-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: Set TLS certificate annotation only on gRPC service #5715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
86ac153 to
be7c6e6
Compare
Srihari1192
approved these changes
Nov 13, 2025
Contributor
Srihari1192
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
jyejare
approved these changes
Nov 13, 2025
be7c6e6 to
543e6fc
Compare
Signed-off-by: ntkathole <nikhilkathole2683@gmail.com>
543e6fc to
818a031
Compare
franciscojavierarceo
approved these changes
Nov 17, 2025
franciscojavierarceo
pushed a commit
that referenced
this pull request
Dec 16, 2025
# [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
# [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
# [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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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