◐ Shell
clean mode source ↗

fix: Set TLS certificate annotation only on gRPC service by ntkathole · Pull Request #5715 · feast-dev/feast

ntkathole

What this PR does / why we need it:

This PR adds !isRestService check in the grpcEnabled && restEnabled branch 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 certificate

The error occurred because:

  • The gRPC client connects using the gRPC service hostname (e.g., feast-registry.tests.svc.cluster.local)
  • But the certificate CN was set to the REST service hostname (e.g., feast-registry-rest.tests.svc.cluster.local)
  • Even though both hostnames were in the SANs, gRPC validates against the CN

Root Cause

The setService() function is called twice when both services are enabled:

  1. Once for the gRPC service (isRestService=false) via createService()
  2. Once for the REST service (isRestService=true) via createRestService()

The original code set the TLS annotation on both services when grpcEnabled && restEnabled was 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 !isRestService check. This ensures:

  1. OpenShift creates the certificate with the gRPC service name as the CN (correct for gRPC connections)
  2. Both hostnames are included as Subject Alternative Names (SANs) via the serving-cert-sans annotation
  3. The REST service can still use the same certificate (mounted in the pod from the gRPC service secret)