feat(t8s-cluster/management-cluster): add OIDC settings#2170
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThis PR refactors the Kubernetes API server OIDC authentication configuration from static issuer blocks into a data-driven model. OIDC providers are now defined in a structured Helm values schema and rendered dynamically into authentication configurations, with special handling for hosted versus non-hosted control planes. ChangesOIDC Configuration Data-Driven Migration
Sequence DiagramsequenceDiagram
participant HelmValues as Helm Values
participant NonHostedTemplate as Non-Hosted Template
participant HostedTemplate as Hosted Template
participant ApiServerConfig as API Server Config
HelmValues->>NonHostedTemplate: oidcProviders, controlPlane.hosted=false
NonHostedTemplate->>NonHostedTemplate: Loop over oidcProviders
NonHostedTemplate->>NonHostedTemplate: Render authenticationConfig
NonHostedTemplate->>NonHostedTemplate: Add authentication-config argument
NonHostedTemplate->>NonHostedTemplate: Generate dynamic auth file
NonHostedTemplate->>ApiServerConfig: Complete manifest with auth config
HelmValues->>HostedTemplate: oidcProviders, controlPlane.hosted=true
HostedTemplate->>HostedTemplate: Pass oidcProviders to spec
HostedTemplate->>ApiServerConfig: Hosted plane spec with oidcProviders
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@charts/t8s-cluster/templates/management-cluster/clusterClass/_authenticationConfig.yaml`:
- Around line 12-16: The CEL expression strings for the expression fields are
unquoted and can be mis-parsed as YAML scalars (e.g., true/false, numbers);
update the template where expression: {{ $provider.claimMappings.username }} and
expression: {{ . }} (inside the with $provider.claimMappings.groups block) to
apply the quote filter so both rendered expressions are emitted as YAML strings
(use the Helm quote filter on $provider.claimMappings.username and on the groups
expression placeholder) ensuring the expression values are always quoted in the
generated YAML.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7b9012b0-b5b7-4fd3-a397-29d273a786ff
📒 Files selected for processing (5)
charts/t8s-cluster/templates/management-cluster/clusterClass/_authenticationConfig.yamlcharts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tplcharts/t8s-cluster/templates/management-cluster/clusterClass/hostedControlPlaneTemplate/_hostedControlPlaneTemplateSpec.yamlcharts/t8s-cluster/values.schema.jsoncharts/t8s-cluster/values.yaml
There was a problem hiding this comment.
Code Review
This pull request introduces support for OIDC providers in the t8s-cluster chart, enabling structured JWT authentication. The changes include updates to the values schema, template files, and helper functions to dynamically render OIDC provider configurations. The review identified a bug regarding the argument order in the mustMerge function within _authenticationConfig.yaml and suggested an improvement to include the certificateAuthority field in the template.
There was a problem hiding this comment.
Pull request overview
This PR makes OIDC/JWT authentication provider configuration configurable via chart values so clusters can be pointed at customer-specific OIDC issuers, and wires those settings into both hosted and non-hosted control plane rendering.
Changes:
- Added
oidcProvidersdefaults tovalues.yamland validation tovalues.schema.json. - Rendered
oidcProvidersinto the hosted control plane spec when present. - Updated api-server args and dynamic file generation to skip
--authentication-configfor hosted control planes, while generating anAuthenticationConfigurationfrom values for non-hosted clusters.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/t8s-cluster/values.yaml | Introduces default oidcProviders configuration entries. |
| charts/t8s-cluster/values.schema.json | Adds schema/validation for oidcProviders (audiences, mappings, validation rules, CA). |
| charts/t8s-cluster/templates/management-cluster/clusterClass/hostedControlPlaneTemplate/_hostedControlPlaneTemplateSpec.yaml | Passes oidcProviders into the hosted control plane spec when configured. |
| charts/t8s-cluster/templates/management-cluster/clusterClass/_helpers.tpl | Avoids generating/using authentication-config for hosted control planes. |
| charts/t8s-cluster/templates/management-cluster/clusterClass/_authenticationConfig.yaml | Generates AuthenticationConfiguration jwt authenticators from .Values.oidcProviders. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
that way we can add the customers' OIDC server
33bf835 to
f229971
Compare
that way we can add the customers' OIDC server
Summary by CodeRabbit