Sorry, something went wrong.
There was a problem hiding this comment.
The Kubernetes auth path sets a status condition (AuthorizationReadyType) to indicate RBAC provisioning success/failure. The OIDC path does not. This means operators have no visibility into whether the OIDC RBAC was actually created.
The existing OIDC test (featurestore_controller_oidc_auth_test.go) verifies that the Kubernetes-auth Role/RoleBinding are absent, but it does not verify that the new feast-oidc-token-review ClusterRole and per-instance ClusterRoleBinding are created with the correct rules. It also doesn't test the cleanup path (switching from OIDC to no-auth should delete the CRB).
Sorry, something went wrong.
| func (authz *FeastAuthorization) getLabels() map[string]string { | ||
| return map[string]string{ | ||
| services.NameLabelKey: authz.Handler.FeatureStore.Name, |
There was a problem hiding this comment.
getLabels() stamps the FeatureStore-specific name. But the ClusterRole feast-oidc-token-review is shared across all OIDC FeatureStore instances. The last instance to reconcile overwrites the labels with its own name. This creates misleading audit trails — the ClusterRole appears to belong to one FeatureStore when it actually serves all of them.
Recommendation: Either use instance-independent labels for the shared ClusterRole, or use an aggregated label approach.
Sorry, something went wrong.
There was a problem hiding this comment.
Addressed by using instance-independent labels for the shared ClusterRole
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM, small comment.
Sorry, something went wrong.
| apimeta.RemoveStatusCondition(&authz.Handler.FeatureStore.Status.Conditions, feastKubernetesAuthConditions[metav1.ConditionTrue].Type) | ||
|
|
||
| // Clean up cluster-scoped Kubernetes auth CRB (handles Kubernetes→OIDC or Kubernetes→no-auth transitions) | ||
| authz.cleanupKubernetesClusterRbac() |
There was a problem hiding this comment.
want to run this on every OIDC reconcile ?
Sorry, something went wrong.
There was a problem hiding this comment.
Check how DeleteOwnedFeastObj in handler.go already does the same thing - get first, early-return on IsNotFound, then Delete.
Sorry, something went wrong.
There was a problem hiding this comment.
lgtm
Sorry, something went wrong.
What this PR does / why we need it:
When `authz: oidc` is configured, the Feast server delegates Kubernetes service account (SA) tokens to a lightweight TokenReview for validation and namespace extraction. This requires the server SA to have `tokenreviews/create` permission. Previously, this RBAC was not provisioned automatically by the operator for OIDC deployments (only for `authz: kubernetes`), requiring manual ClusterRole creation.Operator: OIDC TokenReview RBAC
The operator now provisions a dedicated feast-oidc-token-review ClusterRole and ClusterRoleBinding when authz: oidc is configured. The ClusterRole contains exactly one rule:
This is the minimum permission needed for the SA token delegation path. No additional RBAC queries (rolebindings, clusterroles, namespaces) are granted, unlike the authz: kubernetes path which needs broader permissions for KubernetesTokenParser.
Cleanup is handled automatically when switching auth types:
SDK: SSL Error Logging
When verify_ssl: true is set but the OIDC provider uses self-signed certificates without a configured ca_cert_path, the server fails to reach the JWKS/discovery endpoints. Previously, this produced a generic "Invalid token" log with no indication of the root cause. The token parser now detects SSL errors in the exception chain and logs a clear, actionable message:
This applies to both the discovery endpoint (_validate_token) and the JWKS endpoint (_decode_token) error paths.
Which issue(s) this PR fixes:
Follow up to #6089
Checks
Testing Strategy
Misc