FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

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

/ feast Public

fix: Set TLS certificate annotation only on gRPC service - #5715

Merged
ntkathole merged 2 commits into
feast-dev:masterfrom
ntkathole:fix_tls_san
Nov 17, 2025
ConversationCommits (2)ChecksFiles changed
Merged

fix: Set TLS certificate annotation only on gRPC service#5715
ntkathole merged 2 commits into
feast-dev:masterfrom
ntkathole:fix_tls_san

Conversation

ntkathole commented Nov 6, 2025

Copy link
Copy Markdown
Member

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)

ntkathole requested a review from a team as a code owner November 6, 2025 13:43
ntkathole self-assigned this Nov 6, 2025
ntkathole marked this pull request as draft November 6, 2025 13:44
ntkathole marked this pull request as ready for review November 13, 2025 05:32
ntkathole force-pushed the fix_tls_san branch 2 times, most recently from 86ac153 to be7c6e6 Compare November 13, 2025 06:22
Srihari1192 approved these changes Nov 13, 2025
View reviewed changes

Srihari1192 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Tested feature store instance creation with multiple configurations by enabling grpc: true. Verified that Feast Workbench connections and Feast method invocations are working as expected.

fix: Set TLS certificate annotation only on gRPC service
818a031 
Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
ntkathole merged commit 75d13db into feast-dev:master Nov 17, 2025
23 of 25 checks passed
franciscojavierarceo pushed a commit that referenced this pull request Dec 16, 2025
chore(release): release 0.58.0
b601ef2 
# [0.58.0](v0.57.0...v0.58.0) (2025-12-16)

### Bug Fixes

* Add java proto ([#5719](#5719)) ([fc3ea20](fc3ea20))
* Add possibility to force full features names for materialize ops ([#5728](#5728)) ([55c9c36](55c9c36))
* Fixed file registry cache sync ([09505d4](09505d4))
* Handle hyphon in sqlite project name ([#5575](#5575)) ([#5749](#5749)) ([b8346ff](b8346ff))
* Pinned substrait to fix protobuf issue ([d0ef4da](d0ef4da))
* Set TLS certificate annotation only on gRPC service ([#5715](#5715)) ([75d13db](75d13db))
* SQLite online store deletes tables from other projects in shared registry scenarios ([#5766](#5766)) ([fabce76](fabce76))
* Validate not existing entity join keys for preventing panic ([0b93559](0b93559))

### Features

* Add annotations for pod templates ([534e647](534e647))
* Add Pytorch template ([#5780](#5780)) ([6afd353](6afd353))
* Add support for extra options for stream source ([#5618](#5618)) ([18956c2](18956c2))
* Added matched_tag field search api results with fuzzy search capabilities ([#5769](#5769)) ([4a9ffae](4a9ffae))
* Added support for enabling metrics in Feast Operator ([#5317](#5317)) ([#5748](#5748)) ([a8498c2](a8498c2))
* Configure CacheTTLSecondscache,CacheMode for file-based registry in Feast Operator([#5708](#5708)) ([#5744](#5744)) ([f25f83b](f25f83b))
* Implemented Tiling Support for Time-Windowed Aggregations ([#5724](#5724)) ([7a99166](7a99166))
* Offline Store historical features retrieval based on datetime range for spark ([#5720](#5720)) ([27ec8ec](27ec8ec))
* Offline Store historical features retrieval based on datetime range in dask ([#5717](#5717)) ([a16582a](a16582a))
* Production ready feast operator with v1 apiversion ([#5771](#5771)) ([49359c6](49359c6))
* Support for Map value data type ([#5768](#5768)) ([#5772](#5772)) ([b99a8a9](b99a8a9))
antznette1 pushed a commit to antznette1/feast that referenced this pull request Jan 3, 2026
chore(release): release 0.58.0
a78a5de 
# [0.58.0](feast-dev/feast@v0.57.0...v0.58.0) (2025-12-16)

### Bug Fixes

* Add java proto ([feast-dev#5719](feast-dev#5719)) ([fc3ea20](feast-dev@fc3ea20))
* Add possibility to force full features names for materialize ops ([feast-dev#5728](feast-dev#5728)) ([55c9c36](feast-dev@55c9c36))
* Fixed file registry cache sync ([09505d4](feast-dev@09505d4))
* Handle hyphon in sqlite project name ([feast-dev#5575](feast-dev#5575)) ([feast-dev#5749](feast-dev#5749)) ([b8346ff](feast-dev@b8346ff))
* Pinned substrait to fix protobuf issue ([d0ef4da](feast-dev@d0ef4da))
* Set TLS certificate annotation only on gRPC service ([feast-dev#5715](feast-dev#5715)) ([75d13db](feast-dev@75d13db))
* SQLite online store deletes tables from other projects in shared registry scenarios ([feast-dev#5766](feast-dev#5766)) ([fabce76](feast-dev@fabce76))
* Validate not existing entity join keys for preventing panic ([0b93559](feast-dev@0b93559))

### Features

* Add annotations for pod templates ([534e647](feast-dev@534e647))
* Add Pytorch template ([feast-dev#5780](feast-dev#5780)) ([6afd353](feast-dev@6afd353))
* Add support for extra options for stream source ([feast-dev#5618](feast-dev#5618)) ([18956c2](feast-dev@18956c2))
* Added matched_tag field search api results with fuzzy search capabilities ([feast-dev#5769](feast-dev#5769)) ([4a9ffae](feast-dev@4a9ffae))
* Added support for enabling metrics in Feast Operator ([feast-dev#5317](feast-dev#5317)) ([feast-dev#5748](feast-dev#5748)) ([a8498c2](feast-dev@a8498c2))
* Configure CacheTTLSecondscache,CacheMode for file-based registry in Feast Operator([feast-dev#5708](feast-dev#5708)) ([feast-dev#5744](feast-dev#5744)) ([f25f83b](feast-dev@f25f83b))
* Implemented Tiling Support for Time-Windowed Aggregations ([feast-dev#5724](feast-dev#5724)) ([7a99166](feast-dev@7a99166))
* Offline Store historical features retrieval based on datetime range for spark ([feast-dev#5720](feast-dev#5720)) ([27ec8ec](feast-dev@27ec8ec))
* Offline Store historical features retrieval based on datetime range in dask ([feast-dev#5717](feast-dev#5717)) ([a16582a](feast-dev@a16582a))
* Production ready feast operator with v1 apiversion ([feast-dev#5771](feast-dev#5771)) ([49359c6](feast-dev@49359c6))
* Support for Map value data type ([feast-dev#5768](feast-dev#5768)) ([feast-dev#5772](feast-dev#5772)) ([b99a8a9](feast-dev@b99a8a9))

Signed-off-by: Anthonette Adanyin <106275232+antznette1@users.noreply.github.com>
franciscojavierarceo pushed a commit that referenced this pull request Jan 5, 2026
chore(release): release 0.58.0
807e80e 
# [0.58.0](v0.57.0...v0.58.0) (2025-12-16)

### Bug Fixes

* Add java proto ([#5719](#5719)) ([fc3ea20](fc3ea20))
* Add possibility to force full features names for materialize ops ([#5728](#5728)) ([55c9c36](55c9c36))
* Fixed file registry cache sync ([09505d4](09505d4))
* Handle hyphon in sqlite project name ([#5575](#5575)) ([#5749](#5749)) ([b8346ff](b8346ff))
* Pinned substrait to fix protobuf issue ([d0ef4da](d0ef4da))
* Set TLS certificate annotation only on gRPC service ([#5715](#5715)) ([75d13db](75d13db))
* SQLite online store deletes tables from other projects in shared registry scenarios ([#5766](#5766)) ([fabce76](fabce76))
* Validate not existing entity join keys for preventing panic ([0b93559](0b93559))

### Features

* Add annotations for pod templates ([534e647](534e647))
* Add Pytorch template ([#5780](#5780)) ([6afd353](6afd353))
* Add support for extra options for stream source ([#5618](#5618)) ([18956c2](18956c2))
* Added matched_tag field search api results with fuzzy search capabilities ([#5769](#5769)) ([4a9ffae](4a9ffae))
* Added support for enabling metrics in Feast Operator ([#5317](#5317)) ([#5748](#5748)) ([a8498c2](a8498c2))
* Configure CacheTTLSecondscache,CacheMode for file-based registry in Feast Operator([#5708](#5708)) ([#5744](#5744)) ([f25f83b](f25f83b))
* Implemented Tiling Support for Time-Windowed Aggregations ([#5724](#5724)) ([7a99166](7a99166))
* Offline Store historical features retrieval based on datetime range for spark ([#5720](#5720)) ([27ec8ec](27ec8ec))
* Offline Store historical features retrieval based on datetime range in dask ([#5717](#5717)) ([a16582a](a16582a))
* Production ready feast operator with v1 apiversion ([#5771](#5771)) ([49359c6](49359c6))
* Support for Map value data type ([#5768](#5768)) ([#5772](#5772)) ([b99a8a9](b99a8a9))

Signed-off-by: Francisco Javier Arceo <farceo@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

franciscojavierarceo franciscojavierarceo approved these changes
jyejare jyejare approved these changes
+1 more reviewer
Srihari1192 Srihari1192 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

ntkathole

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL