Repository navigation
feat(kubernetes): expose tenant apiserver authentication in app values - #3123
Conversation
Tenant Kubernetes clusters had no way to configure the kube-apiserver authentication through the app values: no knob for OIDC, the structured AuthenticationConfiguration, extra apiserver flags, or extra volumes. The only workaround was patching the underlying KamajiControlPlane out-of-band, which is invisible to GitOps and lost on recreation. Add three opt-in fields under controlPlane.apiServer, all empty by default so existing clusters render unchanged: - extraArgs -> KamajiControlPlane spec.apiServer.extraArgs - extraVolumeMounts -> spec.apiServer.extraVolumeMounts - extraVolumes -> appended to spec.deployment.extraVolumes This unblocks OIDC SSO via the --oidc-* flags and, more importantly, structured authentication: --authentication-config mounted from a ConfigMap/Secret plus --requestheader-uid-headers, which is the only way to map a user UID for aggregated/extension apiservers. Because the control-plane Deployment runs on the management cluster, the passthrough is validated to preserve tenant<->management isolation: - extraVolumes allow only configMap and secret sources; host-reaching sources (hostPath, csi, nfs, persistentVolumeClaim) and token projection (projected) are rejected, so a tenant with self-service access to the Kubernetes CR cannot mount management-node paths or project arbitrary tokens into the apiserver pod. - each extraVolume must have a unique, non-empty name and exactly one source; the reserved names talos-ca and talos-tls-cert are rejected to avoid a duplicate volume name in the rendered podspec. - each extraVolumeMount must reference a user-declared extraVolume; the chart-managed talos secret volumes cannot be mounted. Regenerate values.schema.json, README, the Go types and deepcopy, and the ApplicationDefinition. Add helm unittest coverage for the default (no-op) render, each field's passthrough, the name/source guards, and the mount-reference guard (valid and rejected inputs). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
github-actions
Bot
added
size/XL
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the managed Kubernetes app by providing necessary configuration knobs for tenant kube-apiserver authentication. By exposing Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
📝 WalkthroughWalkthroughAdds three optional fields— ChangesAPIServer Extension Fields
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for custom API Server configuration in the tenant Kubernetes cluster by introducing extraArgs, extraVolumes, and extraVolumeMounts fields. These fields allow configuring OIDC SSO or structured authentication. Strict validation is added to the Helm templates to ensure tenant isolation by restricting volume sources to configMap and secret only, and rejecting host-reaching or token-projection sources. The changes are fully documented, reflected in the schemas, and covered by a new test suite. There are no review comments to address, so no additional feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@packages/apps/kubernetes/templates/cluster.yaml`:
- Around line 212-239: The extra volume validation in cluster.yaml should reject
Kubernetes-invalid names during render, not just empty/reserved/duplicate ones.
In the extraVolumes loop, validate $volName against the pod-spec DNS_LABEL rules
before appending it to $userVolumeNames, and fail with a clear message if it is
invalid. Keep the check alongside the existing name validation in the
controlPlane.apiServer.extraVolumes block so malformed names are caught early.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a3566319-2fe4-4e76-a7e9-6bac437bfd36
📒 Files selected for processing (8)
api/apps/v1alpha1/kubernetes/types.goapi/apps/v1alpha1/kubernetes/zz_generated.deepcopy.gopackages/apps/kubernetes/README.mdpackages/apps/kubernetes/templates/cluster.yamlpackages/apps/kubernetes/tests/apiserver_authentication_test.yamlpackages/apps/kubernetes/values.schema.jsonpackages/apps/kubernetes/values.yamlpackages/system/kubernetes-rd/cozyrds/kubernetes.yaml
| {{- $volName := $vol.name | default "" | toString }} | ||
| {{- if eq $volName "" }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: every volume must set a non-empty name" $i) }} | ||
| {{- end }} | ||
| {{- if has $volName $reservedVolumeNames }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume name %q is reserved by the chart for the talos control-plane secrets — rename your volume to avoid a duplicate volume name in the control-plane Deployment" $volName) }} | ||
| {{- end }} | ||
| {{- if has $volName $userVolumeNames }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumes: duplicate volume name %q — each extra volume must have a unique name" $volName) }} | ||
| {{- end }} | ||
| {{- $userVolumeNames = append $userVolumeNames $volName }} | ||
| {{- $sources := without (keys $vol) "name" | sortAlpha }} | ||
| {{- if ne (len $sources) 1 }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume %q must declare exactly one volume source, got %d (%v)" $volName (len $sources) $sources) }} | ||
| {{- end }} | ||
| {{- range $source := $sources }} | ||
| {{- if not (has $source $allowedVolumeSources) }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume %q uses a %q source, which is not permitted on a management-cluster control-plane pod — only configMap and secret sources are allowed" $volName $source) }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- end }} | ||
| {{- range $i, $mount := .Values.controlPlane.apiServer.extraVolumeMounts }} | ||
| {{- $mountName := $mount.name | default "" | toString }} | ||
| {{- if eq $mountName "" }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumeMounts[%d]: every mount must set a non-empty name" $i) }} | ||
| {{- end }} | ||
| {{- if not (has $mountName $userVolumeNames) }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumeMounts: mount %q references no volume declared in controlPlane.apiServer.extraVolumes — the chart-managed talos volumes cannot be mounted" $mountName) }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject invalid Kubernetes volume names during render.
extraVolumes only checks for non-empty, reserved, and duplicate names. Names that violate the pod-spec DNS_LABEL rules still render and then fail later during reconciliation, which weakens the whole “fail early” validation block. Validate $volName before adding it to $userVolumeNames.
Suggested fix
{{- $volName := $vol.name | default "" | toString }}
{{- if eq $volName "" }}
{{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: every volume must set a non-empty name" $i) }}
{{- end }}
+{{- if or (gt (len $volName) 63) (not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$" $volName)) }}
+{{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: volume name %q must be a valid DNS label" $i $volName) }}
+{{- end }}
{{- if has $volName $reservedVolumeNames }}
{{- fail (printf "controlPlane.apiServer.extraVolumes: volume name %q is reserved by the chart for the talos control-plane secrets — rename your volume to avoid a duplicate volume name in the control-plane Deployment" $volName) }}
{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- $volName := $vol.name | default "" | toString }} | |
| {{- if eq $volName "" }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: every volume must set a non-empty name" $i) }} | |
| {{- end }} | |
| {{- if has $volName $reservedVolumeNames }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume name %q is reserved by the chart for the talos control-plane secrets — rename your volume to avoid a duplicate volume name in the control-plane Deployment" $volName) }} | |
| {{- end }} | |
| {{- if has $volName $userVolumeNames }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: duplicate volume name %q — each extra volume must have a unique name" $volName) }} | |
| {{- end }} | |
| {{- $userVolumeNames = append $userVolumeNames $volName }} | |
| {{- $sources := without (keys $vol) "name" | sortAlpha }} | |
| {{- if ne (len $sources) 1 }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume %q must declare exactly one volume source, got %d (%v)" $volName (len $sources) $sources) }} | |
| {{- end }} | |
| {{- range $source := $sources }} | |
| {{- if not (has $source $allowedVolumeSources) }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume %q uses a %q source, which is not permitted on a management-cluster control-plane pod — only configMap and secret sources are allowed" $volName $source) }} | |
| {{- end }} | |
| {{- end }} | |
| {{- end }} | |
| {{- range $i, $mount := .Values.controlPlane.apiServer.extraVolumeMounts }} | |
| {{- $mountName := $mount.name | default "" | toString }} | |
| {{- if eq $mountName "" }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumeMounts[%d]: every mount must set a non-empty name" $i) }} | |
| {{- end }} | |
| {{- if not (has $mountName $userVolumeNames) }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumeMounts: mount %q references no volume declared in controlPlane.apiServer.extraVolumes — the chart-managed talos volumes cannot be mounted" $mountName) }} | |
| {{- $volName := $vol.name | default "" | toString }} | |
| {{- if eq $volName "" }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: every volume must set a non-empty name" $i) }} | |
| {{- end }} | |
| {{- if or (gt (len $volName) 63) (not (regexMatch "^[a-z0-9]([-a-z0-9]*[a-z0-9])?$" $volName)) }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: volume name %q must be a valid DNS label" $i $volName) }} | |
| {{- end }} | |
| {{- if has $volName $reservedVolumeNames }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume name %q is reserved by the chart for the talos control-plane secrets — rename your volume to avoid a duplicate volume name in the control-plane Deployment" $volName) }} | |
| {{- end }} | |
| {{- if has $volName $userVolumeNames }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: duplicate volume name %q — each extra volume must have a unique name" $volName) }} | |
| {{- end }} | |
| {{- $userVolumeNames = append $userVolumeNames $volName }} | |
| {{- $sources := without (keys $vol) "name" | sortAlpha }} | |
| {{- if ne (len $sources) 1 }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume %q must declare exactly one volume source, got %d (%v)" $volName (len $sources) $sources) }} | |
| {{- end }} | |
| {{- range $source := $sources }} | |
| {{- if not (has $source $allowedVolumeSources) }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumes: volume %q uses a %q source, which is not permitted on a management-cluster control-plane pod — only configMap and secret sources are allowed" $volName $source) }} | |
| {{- end }} | |
| {{- end }} | |
| {{- end }} | |
| {{- range $i, $mount := .Values.controlPlane.apiServer.extraVolumeMounts }} | |
| {{- $mountName := $mount.name | default "" | toString }} | |
| {{- if eq $mountName "" }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumeMounts[%d]: every mount must set a non-empty name" $i) }} | |
| {{- end }} | |
| {{- if not (has $mountName $userVolumeNames) }} | |
| {{- fail (printf "controlPlane.apiServer.extraVolumeMounts: mount %q references no volume declared in controlPlane.apiServer.extraVolumes — the chart-managed talos volumes cannot be mounted" $mountName) }} |
🤖 Prompt for 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.
In `@packages/apps/kubernetes/templates/cluster.yaml` around lines 212 - 239, The
extra volume validation in cluster.yaml should reject Kubernetes-invalid names
during render, not just empty/reserved/duplicate ones. In the extraVolumes loop,
validate $volName against the pod-spec DNS_LABEL rules before appending it to
$userVolumeNames, and fail with a clear message if it is invalid. Keep the check
alongside the existing name validation in the
controlPlane.apiServer.extraVolumes block so malformed names are caught early.
myasnikovdaniil
left a comment
myasnikovdaniil
left a comment
There was a problem hiding this comment.
LGTM on the code.
Default render is unchanged: every new block is {{- with <empty list> }} or a range over an empty list, so with the default values the rendered KamajiControlPlane/Deployment is identical to main. The volume source allow-list (configMap/secret only; rejects hostPath/csi/nfs/pvc/projected/emptyDir) is the isolation control that matters, and each rejection path is unit-tested; secret/configMap references are namespace-local, so there is no cross-tenant reach. Generated code is in sync.
Note on CI: the earlier E2E failure was the kubernetes-latest bringup (apiserver readyz connection-refused / liveness 403), which is unrelated to this change — the e2e tenant never sets the new fields, so its render is byte-identical to main. It is re-running now; please confirm it goes green before merge.
Non-blocking:
- The CodeRabbit suggestion to validate volume names as DNS labels at render time is worth taking — it keeps the fail-early block from letting a malformed name through to an opaque apply-time error.
extraArgsis an unvalidated passthrough. Acceptable (it is the tenant's own control plane, and the pod is NetworkPolicy-restricted to etcd), but a docs note that it is an advanced, unguarded knob would help.- See the inline note on the reserved-name guard.
| {{- if eq $volName "" }} | ||
| {{- fail (printf "controlPlane.apiServer.extraVolumes[%d]: every volume must set a non-empty name" $i) }} | ||
| {{- end }} | ||
| {{- if has $volName $reservedVolumeNames }} |
There was a problem hiding this comment.
Observation, not a blocker. This rejects the volume names talos-ca/talos-tls-cert, but not the referenced secretName — a tenant can still mount the chart-managed secrets (<release>-talos-ca, <release>-talos-tls-cert, <release>-talos-secrets) from a differently-named volume. I could not turn this into an exfiltration: the control-plane pod runs on the management cluster where the tenant has no exec/filesystem access, and these are the tenant's own cluster secrets, so mounting is not reading. Worth either a one-line note documenting the trust boundary, or also rejecting references to the reserved secret names, as defense-in-depth.
2 participants
What this PR does
The managed
Kubernetesapp (Kamaji-backed tenant control planes) exposed no way to configure the tenantkube-apiserverauthentication through the application values — there was no knob for OIDC, the structuredAuthenticationConfiguration, extra apiserver flags, or extra volumes. The only workaround was patching the underlyingKamajiControlPlaneout-of-band, which is invisible to GitOps and lost when the cluster is recreated from values.This adds three opt-in fields under
controlPlane.apiServer, all empty by default so existing clusters render unchanged:extraArgs→KamajiControlPlane.spec.apiServer.extraArgsextraVolumeMounts→spec.apiServer.extraVolumeMountsextraVolumes→ appended tospec.deployment.extraVolumesThis unblocks OIDC SSO via the
--oidc-*flags and, more importantly, structured authentication:--authentication-configmounted from a ConfigMap/Secret plus--requestheader-uid-headers, which is the only way to map a user UID for aggregated/extension apiservers.Because the control-plane Deployment runs on the management cluster, the passthrough is validated to keep tenant and management isolated:
extraVolumesaccept onlyconfigMap/secretsources (host-reaching sources such ashostPath/csiand token projection viaprojectedare rejected); each volume needs a unique, non-empty name and exactly one source; the reservedtalos-ca/talos-tls-certnames are rejected; and eachextraVolumeMountmust reference a user-declaredextraVolume, so the chart-managed talos secret volumes cannot be mounted. Secret/ConfigMap references resolve in the tenant's own management-side namespace and are intentionally allowed — this stays within the tenant's existing security domain.Generated artifacts (
values.schema.json, README, Go types + deepcopy, ApplicationDefinition) are regenerated from the values annotations. Helm unit tests cover the default no-op render, each field's passthrough, and every rejection path.Addresses #3074.
Screenshots
N/A — no UI changes.
Release note
Summary by CodeRabbit
New Features
Bug Fixes
Tests