fix: Set TLS certificate annotation only on gRPC service #5715
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
This PR adds
!isRestServicecheck in thegrpcEnabled && restEnabledbranch to prevent setting the annotation on the REST service.When both gRPC and REST APIs are enabled for the registry service with OpenShift TLS, the TLS certificate annotation (
service.beta.openshift.io/serving-cert-secret-name) was being set on both the gRPC and REST services. This caused OpenShift's service serving certificate controller to potentially create a certificate with the REST service hostname as the Common Name (CN) instead of the gRPC service hostname.This resulted in gRPC client connections failing with the error:
InactiveRpcError: Peer name feast-registry.tests.svc.cluster.local is not in peer certificateThe error occurred because:
feast-registry.tests.svc.cluster.local)feast-registry-rest.tests.svc.cluster.local)Root Cause
The
setService()function is called twice when both services are enabled:isRestService=false) viacreateService()isRestService=true) viacreateRestService()The original code set the TLS annotation on both services when
grpcEnabled && restEnabledwas true, without checking which service was being processed. OpenShift's certificate controller creates certificates based on the service where the annotation is set, and if the REST service annotation was processed first or separately, it would create a certificate with the wrong CN.So, Modified the
setService()function to only set the TLS annotation on the gRPC service when both services are enabled by adding a!isRestServicecheck. This ensures:serving-cert-sansannotation