feat: Pass optional OIDC audience and issuer through the operator - #6677
Merged
ntkathole merged 3 commits intoJul 31, 2026
Merged
Conversation
larrysingleton007
marked this pull request as draft
July 30, 2026 15:27
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6677 +/- ##
=======================================
Coverage 46.46% 46.46%
=======================================
Files 414 414
Lines 50138 50138
Branches 7173 7173
=======================================
Hits 23295 23295
Misses 25204 25204
Partials 1639 1639
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
larrysingleton007
marked this pull request as ready for review
July 30, 2026 16:23
Contributor
Author
|
@ntkathole this is the operator follow-up you asked for on #6670, ready for review. audience and issuer flow from the referenced OIDC Secret into the generated feature_store.yaml like the existing keys. A pre-submit review caught one real bug worth knowing about: an all-digits audience would arrive as a YAML int and fail the SDK's string validation at server startup, so the copy now coerces the two claim keys to strings, with a regression test. Also documented the operational caveats (pre-existing Secret keys take effect on upgrade, the IdP has to mint matching claims for feast's own client flows, Secret edits apply on next reconcile). CI is green. |
ntkathole
approved these changes
Jul 31, 2026
Follow-up to feast-dev#6670, requested in review: add audience and issuer to OidcOptionalSecretProperties so operators can set them in the referenced OIDC Secret and have them flow into the generated feature_store.yaml auth section, enabling the new opt-in claim verification on the feature server. Absent keys change nothing. Documents the two optional Secret keys in the operator security guide with a pointer to the OIDC authorization page for the token-claims vs discovery-metadata caveat. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
Secret values are YAML-parsed on extraction, so an all-digits audience (e.g. a numeric IdP application ID) arrives as a Go int, renders unquoted in the generated feature_store.yaml, and fails the SDK's Optional[str] validation at server startup. Coerce the two claim keys back to strings in the allowlist copy, with a regression test. Also extend the reconcile-level envtest fixtures with both keys so they exercise real Secret extraction and YAML serialization rather than only the mocked property map. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
State precisely which claims are and are not checked, scope the checks to the standard OIDC/JWKS path (ServiceAccount and intra-server tokens follow separate paths), distinguish the Secret's issuer key from the CR's issuerUrl, and document three operational caveats: pre-existing Secret keys activate on operator upgrade, the IdP must mint matching claims for Feast's own client token flows, and Secret edits apply on the next reconcile. Point the two dead ../reference/auth links at real pages and add the new optional keys to the linked sample Secret as commented lines. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
ntkathole
force-pushed
the
feat/operator-oidc-audience-issuer
branch
from
July 31, 2026 04:53
4f8d6db to
d8ef562
Compare
This was referenced Jul 31, 2026
jyejare
pushed a commit
to opendatahub-io/feast
that referenced
this pull request
Aug 5, 2026
…ast-dev#6677) * feat: Pass optional OIDC audience and issuer through the operator Follow-up to feast-dev#6670, requested in review: add audience and issuer to OidcOptionalSecretProperties so operators can set them in the referenced OIDC Secret and have them flow into the generated feature_store.yaml auth section, enabling the new opt-in claim verification on the feature server. Absent keys change nothing. Documents the two optional Secret keys in the operator security guide with a pointer to the OIDC authorization page for the token-claims vs discovery-metadata caveat. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * fix: Coerce numeric OIDC audience and issuer Secret values to strings Secret values are YAML-parsed on extraction, so an all-digits audience (e.g. a numeric IdP application ID) arrives as a Go int, renders unquoted in the generated feature_store.yaml, and fails the SDK's Optional[str] validation at server startup. Coerce the two claim keys back to strings in the allowlist copy, with a regression test. Also extend the reconcile-level envtest fixtures with both keys so they exercise real Secret extraction and YAML serialization rather than only the mocked property map. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> * docs: Sharpen operator OIDC audience/issuer docs and fix dead links State precisely which claims are and are not checked, scope the checks to the standard OIDC/JWKS path (ServiceAccount and intra-server tokens follow separate paths), distinguish the Secret's issuer key from the CR's issuerUrl, and document three operational caveats: pre-existing Secret keys activate on operator upgrade, the IdP must mint matching claims for Feast's own client token flows, and Secret edits apply on the next reconcile. Point the two dead ../reference/auth links at real pages and add the new optional keys to the linked sample Secret as commented lines. Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com> --------- Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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
Follow-up to #6670, requested in review there: expose the new optional OIDC
audienceandissuerverification fields through the feast-operator.Adds
audienceandissuertoOidcOptionalSecretProperties, so operators can set them in the referenced OIDC Secret and they flow into the generatedfeature_store.yamlauth section the same wayclient_id,client_secret,username, andpasswordalready do. When the keys are absent, nothing changes.Also documents the two optional Secret keys in the operator security guide, with a pointer to the main OIDC authorization page for the caveat about token claims versus discovery metadata.
Scope note:
verify_sslandca_cert_pathare deliberately not added to the whitelist. They're already exposed as first-class CR fields (verifySSL,caCertConfigMap) that feed the same generated auth section, so whitelisting them would create two sources for the same keys. The Secret bag carries IdP-coupled values (auth_discovery_url,client_id, ...), which is whereaudienceandissuerbelong; a first-class CR field forissuerwas considered and rejected because the existingissuerUrlfield already means the discovery base URL, not the tokenissclaim.Testing: extended the repo config suite to include both keys in the mocked Secret and assert they arrive in
OidcParameterswith their values intact, plus the reconcile-level envtest fixtures so the keys go through real Secret extraction and YAML serialization.go build ./...andgo test ./internal/controller/...pass.A pre-submit review pass surfaced two things worth calling out:
audience(a numeric IdP application ID) would arrive as an int, render unquoted, and fail the SDK'sOptional[str]validation at server startup. The copy now coerces the two claim keys to strings, with a regression test.audienceorissuer(previously ignored), they take effect after this change. That and two other operational caveats (the IdP must mint matching claims for Feast's own client token flows, and Secret edits apply on the next reconcile rather than immediately) are documented in the security guide.Deliberately left out:
ui_client_idis similarly absent from the whitelist and consumed by the UI server; left for a separate change if wanted, to keep this one to the review request.Which issue(s) this PR fixes
Fixes #6676
Related to #6669 (main change in #6670).