feat(apps/kubernetes): per-cluster OIDC selector for tenant kube-apiserver (Phase 1) - #3044
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a Phase 1 tenant OIDC authentication feature to the Kubernetes chart: new ChangesTenant OIDC authentication and RBAC
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Helm as cluster.yaml
participant Values as spec.oidc.mode
participant KCP as KamajiControlPlane
Helm->>Values: read oidc.mode
alt mode != None
Helm->>Helm: validate extraArgs/extraVolumes conflicts
Helm->>KCP: inject --authentication-config flag
Helm->>KCP: mount authentication-config volume
else mode == None
Helm->>KCP: render extraArgs/extraVolumeMounts as empty arrays
end
sequenceDiagram
participant HelmHook as oidc-bootstrap Job
participant TenantAPI as Tenant Kube-apiserver
participant KamajiCP as KamajiControlPlane
participant KubeconfigSecret as oidc-kubeconfig Secret
HelmHook->>HelmHook: wait for tenant admin kubeconfig
alt OIDC_MODE == None
HelmHook->>KamajiCP: patch extraVolumes to remove authentication-config
HelmHook->>HelmHook: delete OIDC/Keycloak objects
else OIDC_MODE == System or CustomConfig
HelmHook->>TenantAPI: apply ClusterRoleBinding per oidc.users entry
HelmHook->>TenantAPI: prune stale ClusterRoleBindings
opt OIDC_MODE == System
HelmHook->>KubeconfigSecret: patch with generated oidc-login kubeconfig
end
end
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
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 introduces per-tenant OIDC authentication by allowing each tenant to provision its own Keycloak realm. It enhances security by isolating tenant identity from the platform's management realm and provides a streamlined, automated workflow for granting cluster-admin access via Keycloak group membership. 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
|
There was a problem hiding this comment.
Code Review
This pull request introduces OIDC authentication for tenant Kubernetes clusters, allowing integration with per-tenant Keycloak realms. It adds OIDC configuration options to the Tenant and Kubernetes APIs, updates the respective Helm charts to provision Keycloak clients/groups and configure the kube-apiserver, and adds a bootstrap Job to set up the in-cluster RBAC bindings. The review feedback highlights a critical key mismatch in the OIDC realm inheritance logic ($parentNamespace.oidc instead of oidc-realm) and several potential nil-pointer rendering errors when accessing nested properties (e.g., $kub.spec.oidc.enabled), recommending the use of the dig function. Additionally, the feedback suggests adding a request timeout and error handling to the pre-delete cleanup job to prevent unreachable tenant API servers from blocking Helm uninstallation.
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.
| {{- $oidcRealm := $parentNamespace.oidc | default "" }} | ||
| {{- $childKubernetesNeedsOidc := false }} | ||
| {{- range $kub := (lookup "apps.cozystack.io/v1alpha1" "Kubernetes" $tenantName "").items | default list }} | ||
| {{- if and $kub.spec $kub.spec.oidc $kub.spec.oidc.enabled }} | ||
| {{- $childKubernetesNeedsOidc = true }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
There is a critical bug in the OIDC realm inheritance logic. The parent namespace publishes the realm name under the key oidc-realm in the cozystack-values Secret, but here it is being read as $parentNamespace.oidc. Because of this mismatch, descendant tenants will never inherit the parent's OIDC realm, breaking multi-tenant realm inheritance.
Additionally, evaluating $kub.spec.oidc.enabled directly inside the and function is unsafe because Go templates evaluate all arguments of a function before executing it. If any child Kubernetes CR has an empty or missing spec (which is common during creation or migration), this will cause a nil-pointer rendering error and crash the entire Tenant reconciliation.
Using dig and the correct oidc-realm key resolves both issues safely and elegantly.
{{- $oidcRealm := index $parentNamespace "oidc-realm" | default "" }}
{{- $childKubernetesNeedsOidc := false }}
{{- range $kub := (lookup "apps.cozystack.io/v1alpha1" "Kubernetes" $tenantName "").items | default list }}
{{- if dig "spec" "oidc" "enabled" false $kub }}
{{- $childKubernetesNeedsOidc = true }}
{{- end }}
{{- end }}References
- Helm template correctness and avoiding nil dereferences / logic errors. (link)
| {{- $childKubernetesNeedsOidc := false }} | ||
| {{- range $kub := (lookup "apps.cozystack.io/v1alpha1" "Kubernetes" $tenantName "").items | default list }} | ||
| {{- if and $kub.spec $kub.spec.oidc $kub.spec.oidc.enabled }} | ||
| {{- $childKubernetesNeedsOidc = true }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Evaluating $kub.spec.oidc.enabled directly inside the and function is unsafe because Go templates evaluate all arguments of a function before executing it. If any child Kubernetes CR has an empty or missing spec, this will cause a nil-pointer rendering error and crash the entire Tenant reconciliation.
Using dig safely navigates the nested map and avoids any nil-pointer evaluation errors.
{{- $childKubernetesNeedsOidc := false }}
{{- range $kub := (lookup "apps.cozystack.io/v1alpha1" "Kubernetes" $tenantName "").items | default list }}
{{- if dig "spec" "oidc" "enabled" false $kub }}
{{- $childKubernetesNeedsOidc = true }}
{{- end }}
{{- end }}References
- Avoiding nil dereferences and ensuring Helm template correctness. (link)
| kubectl delete clusterrolebinding \ | ||
| --selector cozystack.io/oidc-cluster={{ $cluster }} \ | ||
| --ignore-not-found |
There was a problem hiding this comment.
If the tenant apiserver is unreachable or already terminating when the pre-delete hook runs, kubectl delete will block or fail. Because of set -eu, any failure in kubectl delete will cause the hook job to exit with a non-zero code, which blocks the entire Helm uninstallation of the Kubernetes CR.
Adding a --request-timeout and handling the error gracefully ensures that an unreachable tenant apiserver does not block the uninstallation of the control plane.
kubectl delete clusterrolebinding \
--selector cozystack.io/oidc-cluster={{ $cluster }} \
--ignore-not-found \
--request-timeout=15s || echo "Tenant apiserver unreachable, skipping cleanup"| {{- $platformOidc := index .Values._cluster "oidc-enabled" | default "" | toString }} | ||
| {{- if ne $platformOidc "true" }} | ||
| {{- fail "oidc.enabled=true on this Kubernetes CR but the platform-level OIDC flag is not enabled (authentication.oidc.enabled=false at the cozystack platform → _cluster.oidc-enabled is not 'true'). The Keycloak Operator and ClusterKeycloak CRDs required by the per-tenant realm are only installed when the platform flag is true — enable authentication.oidc.enabled in the platform values first." }} | ||
| {{- end }} |
There was a problem hiding this comment.
To enforce defensive programming and prevent potential nil-pointer rendering errors if _cluster is missing or nil, use dig to safely retrieve the oidc-enabled flag.
{{- $platformOidc := dig "_cluster" "oidc-enabled" "" .Values | toString }}
{{- if ne $platformOidc "true" }}
{{- fail "oidc.enabled=true on this Kubernetes CR but the platform-level OIDC flag is not enabled (authentication.oidc.enabled=false at the cozystack platform → _cluster.oidc-enabled is not 'true'). The Keycloak Operator and ClusterKeycloak CRDs required by the per-tenant realm are only installed when the platform flag is true — enable authentication.oidc.enabled in the platform values first." }}
{{- end }}| {{- $oidcRealm := "" }} | ||
| {{- if .Values.oidc.enabled }} | ||
| {{- $oidcRealm = index .Values._namespace "oidc-realm" | default "" }} | ||
| {{- end }} |
There was a problem hiding this comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/apps/kubernetes/templates/cluster.yaml (1)
228-234: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueConsider setting
--oidc-username-prefixto avoid username collisions/impersonation.With
--oidc-username-claim=preferred_usernameand no prefix, OIDC usernames share the namespace with other authenticators (e.g. an OIDCpreferred_usernameofsystem:mastersor a name colliding with an X509 CN). Setting a prefix (e.g.oidc:) isolates OIDC identities. Optional given the per-realm/per-audience isolation, but it's a cheap hardening step.🤖 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 228 - 234, Add an OIDC username prefix to the auth args in the cluster template so `preferred_username` identities do not share the same namespace as other authenticators. Update the `extraArgs` block near the `--oidc-username-claim` and `--oidc-groups-claim` flags to include `--oidc-username-prefix` with a stable value such as `oidc:`. Keep the change scoped to the `.Values.oidc.enabled` path and the existing `kubernetes.oidcIssuerUrl` / `kubernetes.oidcClientId` configuration.
🤖 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/oidc-keycloak.yaml`:
- Around line 49-73: The KeycloakClientScope definition for the audience scope
currently sets default: true, which makes the scope global across the realm and
can leak the included.client.audience mapper to all clients. Remove the default
flag from the KeycloakClientScope template while keeping the existing explicit
reference from the client resource’s spec.defaultClientScopes so the scope
remains per-client and cluster-isolated.
In `@packages/apps/tenant/templates/namespace.yaml`:
- Line 86: The namespace template is reading the wrong parent value for OIDC
realm inheritance: $parentNamespace.oidc will never resolve the published
oidc-realm key. Update the inheritance lookup in namespace.yaml to use the
hyphenated key via index, matching the pattern used in oidc-keycloak.yaml and
ensuring descendant tenants inherit the realm correctly.
---
Nitpick comments:
In `@packages/apps/kubernetes/templates/cluster.yaml`:
- Around line 228-234: Add an OIDC username prefix to the auth args in the
cluster template so `preferred_username` identities do not share the same
namespace as other authenticators. Update the `extraArgs` block near the
`--oidc-username-claim` and `--oidc-groups-claim` flags to include
`--oidc-username-prefix` with a stable value such as `oidc:`. Keep the change
scoped to the `.Values.oidc.enabled` path and the existing
`kubernetes.oidcIssuerUrl` / `kubernetes.oidcClientId` configuration.
🪄 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: 174e4eab-78d5-4be9-8c0e-33258cb8225f
📒 Files selected for processing (21)
api/apps/v1alpha1/kubernetes/types.goapi/apps/v1alpha1/kubernetes/zz_generated.deepcopy.goapi/apps/v1alpha1/tenant/types.goapi/apps/v1alpha1/tenant/zz_generated.deepcopy.godocs/oidc-tenant.mdpackages/apps/kubernetes/README.mdpackages/apps/kubernetes/templates/NOTES.txtpackages/apps/kubernetes/templates/_helpers.tplpackages/apps/kubernetes/templates/cluster.yamlpackages/apps/kubernetes/templates/oidc-keycloak.yamlpackages/apps/kubernetes/templates/oidc-rbac-job.yamlpackages/apps/kubernetes/tests/oidc_test.yamlpackages/apps/kubernetes/values.schema.jsonpackages/apps/kubernetes/values.yamlpackages/apps/tenant/README.mdpackages/apps/tenant/templates/keycloakrealm.yamlpackages/apps/tenant/templates/namespace.yamlpackages/apps/tenant/values.schema.jsonpackages/apps/tenant/values.yamlpackages/system/kubernetes-rd/cozyrds/kubernetes.yamlpackages/system/tenant-rd/cozyrds/tenant.yaml
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
hack/e2e-apps/kubernetes-oidc.bats (1)
206-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove cleanup/reset out of test body into framework teardown.
Resetting tenant OIDC and deleting temp artifacts only at the end of this test means failures earlier can leak state into subsequent suites. Move this cleanup to
teardown_file/shared framework cleanup.Based on learnings, E2E cleanup under
hack/e2e-appsshould use framework teardown rather than inline per-test cleanup.🤖 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 `@hack/e2e-apps/kubernetes-oidc.bats` around lines 206 - 211, Move the inline cleanup/reset logic out of the test body into the shared bats teardown path so it always runs, even on failures. Relocate the OIDC reset, port-forward kill, and temp file removal currently in kubernetes-oidc.bats to teardown_file or the existing shared framework cleanup used by this suite. Keep the cleanup behavior the same, but ensure it is centralized so other e2e apps under hack/e2e-apps follow the teardown pattern consistently.Source: Learnings
🤖 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 `@hack/e2e-apps/kubernetes-oidc.bats`:
- Around line 31-33: The HelmRelease readiness checks in the e2e script can race
because kubectl wait may run before the HelmRelease exists. Update the
kubernetes-oidc.bats flow around the tenant-test and ${RELEASE_NAME} waits to
first poll for existence with a timeout-based until kubectl get hr/... check,
then run kubectl wait only after the HelmRelease is confirmed present. Use the
existing hr/tenant-test and hr/${RELEASE_NAME} wait blocks as the fix points.
In `@hack/e2e-apps/tenant-oidc.bats`:
- Around line 32-33: The `kubectl wait` calls for `hr/tenant-test` in the tenant
OIDC e2e flow can race the HR materialization, so add a short polling backstop
before each readiness wait. In the `tenant-oidc.bats` flow, update both `kubectl
wait hr/tenant-test -n tenant-root` sites to first run a brief `timeout ...
until kubectl get hr/tenant-test -n tenant-root ...` check, then proceed with
the existing wait once the resource exists.
---
Nitpick comments:
In `@hack/e2e-apps/kubernetes-oidc.bats`:
- Around line 206-211: Move the inline cleanup/reset logic out of the test body
into the shared bats teardown path so it always runs, even on failures. Relocate
the OIDC reset, port-forward kill, and temp file removal currently in
kubernetes-oidc.bats to teardown_file or the existing shared framework cleanup
used by this suite. Keep the cleanup behavior the same, but ensure it is
centralized so other e2e apps under hack/e2e-apps follow the teardown pattern
consistently.
🪄 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: 82b8e051-4362-4178-9f0a-bedc8dc8861d
📒 Files selected for processing (2)
hack/e2e-apps/kubernetes-oidc.batshack/e2e-apps/tenant-oidc.bats
71470ae to
e9cc894
Compare
Timofei Larkin (lllamnyp)
left a comment
There was a problem hiding this comment.
Stepping back on the identity model before this locks in
The mechanics here are solid, but the design commits to one Keycloak realm per tenant, and that is a much larger thing than the kube-apiserver OIDC wiring it is in service of. A realm is not a trust domain like a client or an audience — it is a whole identity directory and service. Before we bake that in, it is worth comparing to how others solve tenant cluster auth, spelling out why a realm is the wrong unit here, and listing the decisions that should be settled first.
How others solve cluster auth
Most of the prior art separates one flat identity source from per-cluster binding, rather than standing up a directory per tenant. AWS EKS is the clearest reference, so a couple of its commands are worth unpacking for anyone who hasn't used them:
- Flat IAM + a webhook. When a user runs
aws eks get-token(the EKS CLI command that produces a credential forkubectl), it does not return a normal OIDC JWT. It returns a short-lived, signed AWS API request — a presignedsts:GetCallerIdentityURL — which the user presents as their bearer token. The cluster doesn't validate this itself; it hands the token to a token-review webhook (aws-iam-authenticator) that "replays" the signed request against AWS to learn who the caller is. Crucially, that signed request is bound to one specific cluster via a signedx-k8s-aws-idheader, so a token minted for cluster A is rejected by cluster B — no cross-cluster replay, even though AWS IAM is a single, flat, account-wide directory. The mapping from IAM identity to Kubernetes RBAC is kept out-of-band (EKS Access Entries, or the olderaws-authConfigMap). - Bring-your-own OIDC.
aws eks associate-identity-provider-configis an admin command that attaches an external OIDC provider to an existing cluster. Under the hood it simply sets the managed apiserver's--oidc-*flags to point at the customer's own issuer (Okta, Entra, a self-run Keycloak, etc.). AWS hosts no identity of its own in this mode — it is pure apiserver plumbing to an issuer the customer already operates. - AWS Cognito is the separate product for "a managed user directory / IdP as a service." It is deliberately not part of EKS. That product — a directory you fill with users and groups — is what a per-tenant realm actually resembles.
- GKE and AKS follow the same shape: delegate to the cloud's flat IAM via a webhook, or let the customer bring their own OIDC.
- On the client side,
kubectl oidc-loginuses the OAuth native-app pattern (RFC 8252): a public client + PKCE, redirecting tohttp://localhost:8000on the user's own machine. AWS Client VPN does the same with a127.0.0.1listener — localhost redirect is a blessed pattern, not a smell, and it means redirect URLs don't drive this design (they are localhost regardless of how the client is scoped).
Takeaway: the mainstream answer is one flat identity source + a per-cluster audience/binding, never one directory per tenant.
Why a realm is the wrong unit
The critical distinction the current design glosses over: a separate client is just a trust/audience domain inside an already-populated directory — cheap. A separate realm is an empty directory that someone has to fill.
- A realm has no users and no groups when it is created. For tenant logins to work, each per-tenant realm must be provisioned with identities — every user who should reach the tenant's clusters needs an account in that realm, with credentials, password policy, MFA, recovery, and group membership maintained there. None of that is a Helm template; it is an ongoing identity-administration workload.
- That forces one of two unhappy answers, and neither is in this PR: either the same human now has duplicate accounts (one in
cozy, one in every tenant realm they touch) and logs into a different place per tenant, or the platform has to build and operate user/group synchronization or federation from a source of truth into each realm. Both are substantial services in their own right — this is the Cognito-shaped product hiding inside a "wire up OIDC flags" PR. - The platform already has the flat
cozyrealm, already populated, already driving mgmt-cluster RBAC via<tenant>-*groups. It is a perfectly good identity source for tenant clusters too. Per-cluster isolation does not require a new directory — it requires a new audience, which is a client, not a realm. - The realm-per-tenant choice is also what creates the lifecycle problems this PR documents: because auto-provisioning keys off a Helm
lookup, helm-controller never re-renders when the lookup result changes, so orphaned realms cannot be cleaned up (hence the "toggleTenant.spec.oidctwice" workaround). Drop the per-tenant realm and that entire class of problem disappears.
In short: the per-tenant realm should come out. It buys an isolation property that a per-cluster client already provides, while taking on the cost of running a directory service per tenant.
A lighter design
Keep identity in the flat cozy realm and move the per-cluster boundary to audience, not to a directory:
- Provision a per-cluster public client
kubernetes-<cluster>incozy. Since an id_token'saudis the client that requested it, and the apiserver matches--oidc-client-idagainstaud, a token minted for one cluster is rejected by every other cluster for free — no audience mapper, no second directory. - This removes the per-tenant realm,
Tenant.spec.oidc, thelookup-based auto-provision, the_namespace.oidc-realmpropagation, and the cleanup limitation in one stroke — while keeping the isolation property intact.
On RBAC — a separate objection to the PR as written: binding the OIDC group to cluster-admin by default is the wrong default. Cluster-admin is already available out-of-band via the admin kubeconfig the platform provisions. OIDC users should get least-privilege RBAC defined per cluster (or nothing by default), with admins composing Roles/RoleBindings for OIDC identities as needed — not a blanket cluster-admin grant baked into the chart. An OIDC user does not have to be an admin, and usually should not be.
Why per-cluster audience isolation matters regardless of how careful the RBAC is: even an identity with zero tenant-specific RBAC still lands in system:authenticated, which carries real default permissions (system:basic-user, system:discovery, system:public-info-viewer — self-subject reviews, API discovery, etc.). And tenant clusters are operated by potentially untrusted parties, so a token presented to one tenant's apiserver could be captured and replayed against another's. Per-cluster audience binding is therefore required independent of the RBAC model — and a single global client shared across all tenant clusters is, for the same reason, not viable.
BYO-OIDC is a real requirement, and flags won't cover it
Many users will want their tenant clusters to trust their own corporate IdP (Okta, Entra, an existing Keycloak) rather than any platform-hosted directory — exactly what EKS's associate-identity-provider-config exists for. The current approach wires a single issuer via --oidc-* flags in apiServer.extraArgs, and that is a structural dead end:
- The classic
--oidc-*flags trust exactly one issuer. A platform-hosted client and a tenant's BYO issuer cannot coexist on one apiserver via flags. - The flag path cannot trust a self-signed / private-CA issuer (this PR notes the limitation), and a lot of on-prem IdPs are exactly that.
Both are solved by Structured Authentication Configuration (--authentication-config, beta since 1.30, GA in recent releases): a list of jwt authenticators (platform + BYO together), inline certificateAuthority for private CAs, CEL-based claim mapping/validation, and hot reload. The catch is plumbing: structured config is a file the apiserver must mount, not a flag — so it depends on what KamajiControlPlane exposes for mounting a config file/secret into the apiserver pod, versus only appending args. Worth confirming early, since it likely decides whether the flag approach has any future at all.
A token-exchange option (a custom credential plugin is acceptable)
We might be willing to ship a custom client.authentication.k8s.io exec credential plugin, which opens a clean third path: the user logs into the flat cozy realm once, and the plugin performs an OAuth 2.0 Token Exchange (RFC 8693) to trade that token for a per-cluster, audience-scoped token — with Keycloak enforcing "is this user entitled to this cluster?" at exchange time. That folds both the per-cluster binding and the entitlement check into the IdP while keeping a single login and a single directory, instead of N static clients or a bespoke webhook. More moving parts than stock kubelogin, but a tidy model given we are comfortable building our own plugin.
For completeness, the EKS-style webhook path (one flat client + a platform-hosted token-review webhook keyed by per-cluster mTLS) gives the richest policy — deny by tenant membership before RBAC, central audit, instant global revocation — at the cost of an HA, security-critical service on every cluster's auth hot path.
Decisions to settle first — let's write a proposal
This is enough architectural surface that it deserves a short design proposal in cozystack/community rather than being settled inline on this PR. The open decisions (note that "per-tenant realm" is intentionally not among them — it should be ruled out):
- Identity source: the flat
cozyrealm, BYO-OIDC, or both — and in what priority. - Per-cluster isolation mechanism: per-cluster audience (a client/scope in
cozy) vs. central token-review webhook vs. token-exchange credential plugin. - Default RBAC for OIDC identities: least-privilege / none vs. anything broader, and how it relates to the admin kubeconfig the platform already hands out.
- Multi-issuer / BYO + private CA: commit to structured auth config? If so, resolve the
KamajiControlPlanefile-mount question. - Lifecycle: how per-cluster clients/groups are provisioned and cleaned up without the
lookupre-render gap. - Headless/CI token acquisition: device-code vs. ROPC vs. token exchange.
686535d to
4ac477d
Compare
|
CodeRabbit (@coderabbitai) full review |
|
Gemini (@gemini-code-assist) review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request implements Phase 1 of OIDC authentication and per-user RBAC for tenant Kubernetes clusters, supporting both a platform-level "System" mode and a tenant-supplied "CustomConfig" mode. The changes include new API types, Helm templates, documentation, and comprehensive E2E and unit tests. The review feedback is highly constructive, pointing out opportunities to prevent potential nil pointer dereferences in Helm templates by using the dig function for nested values, and suggesting a more portable base64 syntax in the bootstrap script to ensure compatibility across different BusyBox versions.
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.
| {{- if and (eq .Values.oidc.mode "CustomConfig") (ne .Values.oidc.customConfig.secretRef.name "") }} | ||
| {{- $oidcAuthnSecret = .Values.oidc.customConfig.secretRef.name }} | ||
| {{- end }} |
There was a problem hiding this comment.
To prevent potential nil pointer dereference errors when customConfig or secretRef is omitted or set to null in user-supplied values, use the dig function to safely navigate the nested fields.
{{- $secretRefName := dig "customConfig" "secretRef" "name" "" .Values.oidc }}
{{- if and (eq .Values.oidc.mode "CustomConfig") (ne $secretRefName "") }}
{{- $oidcAuthnSecret = $secretRefName }}
{{- end }}| {{- range $arg := .Values.controlPlane.apiServer.extraArgs }} | ||
| {{- if hasPrefix "--oidc-" $arg }} | ||
| {{- fail (printf "controlPlane.apiServer.extraArgs contains %q while spec.oidc.mode is %q — the kube-apiserver refuses to boot with both --authentication-config (injected by spec.oidc) and legacy --oidc-* flags. Remove the --oidc-* flags from controlPlane.apiServer.extraArgs and use spec.oidc.mode: CustomConfig with a customConfig.config carrying the same issuer/audience if you need to override, or spec.oidc.mode: System for the platform cozy default." $arg $.Values.oidc.mode) }} | ||
| {{- end }} | ||
| {{- if hasPrefix "--authentication-config" $arg }} | ||
| {{- fail (printf "controlPlane.apiServer.extraArgs contains %q while spec.oidc.mode is %q — the chart already injects --authentication-config= for OIDC. Drop your explicit flag and let spec.oidc.mode: CustomConfig own the AuthenticationConfiguration (customConfig.config or customConfig.secretRef.name), or leave spec.oidc.mode: None and keep managing the flag yourself." $arg $.Values.oidc.mode) }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
To prevent potential nil pointer dereference errors when controlPlane or apiServer is omitted or set to null in user-supplied values, use the dig function to safely navigate the nested fields.
{{- $extraArgs := dig "controlPlane" "apiServer" "extraArgs" (list) .Values }}
{{- range $arg := $extraArgs }}
{{- if hasPrefix "--oidc-" $arg }}
{{- fail (printf "controlPlane.apiServer.extraArgs contains %q while spec.oidc.mode is %q — the kube-apiserver refuses to boot with both --authentication-config (injected by spec.oidc) and legacy --oidc-* flags. Remove the --oidc-* flags from controlPlane.apiServer.extraArgs and use spec.oidc.mode: CustomConfig with a customConfig.config carrying the same issuer/audience if you need to override, or spec.oidc.mode: System for the platform cozy default." $arg $.Values.oidc.mode) }}
{{- end }}
{{- if hasPrefix "--authentication-config" $arg }}
{{- fail (printf "controlPlane.apiServer.extraArgs contains %q while spec.oidc.mode is %q — the chart already injects --authentication-config= for OIDC. Drop your explicit flag and let spec.oidc.mode: CustomConfig own the AuthenticationConfiguration (customConfig.config or customConfig.secretRef.name), or leave spec.oidc.mode: None and keep managing the flag yourself." $arg $.Values.oidc.mode) }}
{{- end }}
{{- end }}| {{- $hasInline := ne .Values.oidc.customConfig.config "" }} | ||
| {{- $hasSecretRef := ne .Values.oidc.customConfig.secretRef.name "" }} |
There was a problem hiding this comment.
To prevent potential nil pointer dereference errors when customConfig or secretRef is omitted or set to null in user-supplied values, use the dig function to safely navigate the nested fields.
{{- $hasInline := ne (dig "customConfig" "config" "" .Values.oidc) "" }}
{{- $hasSecretRef := ne (dig "customConfig" "secretRef" "name" "" .Values.oidc) "" }}| cluster: $RELEASE | ||
| user: $RELEASE-oidc | ||
| EOF | ||
| KUBECONFIG_B64=$(base64 -w 0 < "$KUBECONFIG_FILE") |
Add the plumbing needed for a Phase-1 OIDC integration on Grafana, matching the shape of #3044 for the tenant kube-apiserver: - values.yaml grows a top-level oidc block with a three-value mode enum (None/System/CustomConfig) and a customConfig payload with the XOR-inline/secretRef surface. - monitoring-rd advertises the new selector via its openAPISchema and keysOrder so operators see it on the Monitoring CR. - templates/_helpers.tpl introduces the naming helpers reused by every downstream OIDC template: clientId, audienceScopeName, clientSecretName, grafanaHost, redirectUri, systemIssuerURL, roleAttributePath, allowAssignGrafanaAdmin (platform-only), assertSystemEnabled, assertCustomConfigXor. No behaviour change on default values — mode: None short-circuits and no OIDC objects are rendered. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…mode New oidc-keycloak.yaml template mirrors the tenant kube-apiserver Phase-1 shape (#3044) with three Grafana-specific adjustments: - Client is confidential (public: false, directAccess: false); Grafana is server-side so a client_secret is required. The value is wired via the EDP-Keycloak `$<secret>:<key>` reference; the backing Secret is created by the next commit. - redirectUri is https://grafana.<host>/login/generic_oauth — the Grafana generic_oauth callback path. - Three KeycloakRealmGroups (`<clientId>-{admin,editor,viewer}`) are chart-owned. Membership is set by an operator through the Keycloak UI or a KeycloakRealmUser CR; grafana.yaml will map group membership to Admin / Editor / Viewer via role_attribute_path. Guarded by mode == System AND _cluster.oidc-enabled == true (via the assertSystemEnabled helper). Renders nothing for None or CustomConfig. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…stance New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from #3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…p + reconcile Job Replace the group-based authorization model on the Phase-1 Grafana OIDC selector with an app-side users map, mirroring the tenant kube-apiserver sibling in #3044. What is removed - The three chart-owned KeycloakRealmGroup objects in the cozy realm (`<clientId>-{admin,editor,viewer}`). A Helm release must not own directory objects — toggling `spec.oidc.mode: System -> None` or deleting the Monitoring release would Helm-delete the groups, and with them any membership curated out-of-band. Per-instance groups in the platform realm is exactly the design question left open at the end of cozystack/community#24; establishing it as precedent from a monitoring PR is out of scope. - Dash-separated group naming (`<namespace>-<release>-<role>`) was also collision-prone against tenant/subtenant paths, since both the namespace and release names can themselves contain dashes. - `role_attribute_path` in the Grafana `generic_oauth` block and the default-Viewer fallback, which handed org-Viewer to every cozy realm identity that logged in — cross-tenant metrics read across every opted-in Grafana. Also removed the `groups` client scope from the KeycloakClient defaults. - `monitoring.oidc.allowAssignGrafanaAdmin` helper and the internal `oidc.grafanaAdmin` values key: the feature was already deferred and the helper's release-name-only guard was tenant-spoofable. What replaces it - `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` on the Monitoring chart values. - A chart-owned post-install/post-upgrade Job (`templates/grafana/oidc-users-job.yaml`) that reconciles that list into Grafana's Main Org. via the admin API: pre-provisions a local Grafana account per listed email so the reconcile can set the org role before the operator's first OIDC login, adds the account to the org, PATCHes the role to converge across re-runs, and prunes every non-admin Main-Org member whose email is not in the list (handles both entry removal and `mode: None`). - `skip_org_role_sync = true` + `oauth_allow_insecure_email_lookup = true` in the generic_oauth block. The former stops a login from overwriting the Job's assignments; the latter binds the OIDC identity to the pre-provisioned local account. - Job talks to Grafana over the in-cluster Service and reads admin creds from the chart-managed `grafana-admin-password` Secret via `envFrom`: no ServiceAccount / Role / RoleBinding. `activeDeadlineSeconds: 900` caps a stuck Job so a broken Grafana cannot hold the release in `pending-upgrade`. Defensive rendering - `.Values.oidc | default dict` in the four templates that dereference the `oidc` block so `oidc: ~` or an omitted block no longer nil-derefs. - `oidc-client-secret.yaml` falls back to a fresh random when the existing Secret lookup succeeds but `data.client-secret` is empty. Rationale: #3176 review from @lllamnyp (architectural CHANGES_REQUESTED points 1, 2, 3 + the default-Viewer leak). The three points stack: dropping the groups incidentally fixes the naming-collision and the default-Viewer leak, and the users-Job gives operators the same UX story as the tenant kube-apiserver PR across both sides of the platform. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
…ster OIDC Companion design proposal to cozystack/cozystack#3044. Central decision: per-tenant Keycloak realm (tenant-<ns>) as the identity unit for tenant Kubernetes cluster OIDC, with per-cluster public clients providing audience-bound token isolation. The platform-admin cozy realm and tenant user directories serve different populations and trust models; this proposal keeps them separate while delivering per-cluster isolation through audience binding rather than a separate directory. Covers realm provisioning via the EDP Keycloak operator, propagation through the cozystack-basics namespace-values channel, per-cluster KeycloakClient + view/admin KeycloakRealmGroups, KamajiControlPlane oidc flag wiring, an OIDC kubeconfig Secret exposed via cozyrds, plus lifecycle, rollout, security, failure modes, testing, and alternatives considered (including the flat cozy + per-cluster-client path). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add the plumbing needed for a Phase-1 OIDC integration on Grafana, matching the shape of #3044 for the tenant kube-apiserver: - values.yaml grows a top-level oidc block with a three-value mode enum (None/System/CustomConfig) and a customConfig payload with the XOR-inline/secretRef surface. - monitoring-rd advertises the new selector via its openAPISchema and keysOrder so operators see it on the Monitoring CR. - templates/_helpers.tpl introduces the naming helpers reused by every downstream OIDC template: clientId, audienceScopeName, clientSecretName, grafanaHost, redirectUri, systemIssuerURL, roleAttributePath, allowAssignGrafanaAdmin (platform-only), assertSystemEnabled, assertCustomConfigXor. No behaviour change on default values — mode: None short-circuits and no OIDC objects are rendered. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…mode New oidc-keycloak.yaml template mirrors the tenant kube-apiserver Phase-1 shape (#3044) with three Grafana-specific adjustments: - Client is confidential (public: false, directAccess: false); Grafana is server-side so a client_secret is required. The value is wired via the EDP-Keycloak `$<secret>:<key>` reference; the backing Secret is created by the next commit. - redirectUri is https://grafana.<host>/login/generic_oauth — the Grafana generic_oauth callback path. - Three KeycloakRealmGroups (`<clientId>-{admin,editor,viewer}`) are chart-owned. Membership is set by an operator through the Keycloak UI or a KeycloakRealmUser CR; grafana.yaml will map group membership to Admin / Editor / Viewer via role_attribute_path. Guarded by mode == System AND _cluster.oidc-enabled == true (via the assertSystemEnabled helper). Renders nothing for None or CustomConfig. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…stance New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from #3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…p + reconcile Job Replace the group-based authorization model on the Phase-1 Grafana OIDC selector with an app-side users map, mirroring the tenant kube-apiserver sibling in #3044. What is removed - The three chart-owned KeycloakRealmGroup objects in the cozy realm (`<clientId>-{admin,editor,viewer}`). A Helm release must not own directory objects — toggling `spec.oidc.mode: System -> None` or deleting the Monitoring release would Helm-delete the groups, and with them any membership curated out-of-band. Per-instance groups in the platform realm is exactly the design question left open at the end of cozystack/community#24; establishing it as precedent from a monitoring PR is out of scope. - Dash-separated group naming (`<namespace>-<release>-<role>`) was also collision-prone against tenant/subtenant paths, since both the namespace and release names can themselves contain dashes. - `role_attribute_path` in the Grafana `generic_oauth` block and the default-Viewer fallback, which handed org-Viewer to every cozy realm identity that logged in — cross-tenant metrics read across every opted-in Grafana. Also removed the `groups` client scope from the KeycloakClient defaults. - `monitoring.oidc.allowAssignGrafanaAdmin` helper and the internal `oidc.grafanaAdmin` values key: the feature was already deferred and the helper's release-name-only guard was tenant-spoofable. What replaces it - `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` on the Monitoring chart values. - A chart-owned post-install/post-upgrade Job (`templates/grafana/oidc-users-job.yaml`) that reconciles that list into Grafana's Main Org. via the admin API: pre-provisions a local Grafana account per listed email so the reconcile can set the org role before the operator's first OIDC login, adds the account to the org, PATCHes the role to converge across re-runs, and prunes every non-admin Main-Org member whose email is not in the list (handles both entry removal and `mode: None`). - `skip_org_role_sync = true` + `oauth_allow_insecure_email_lookup = true` in the generic_oauth block. The former stops a login from overwriting the Job's assignments; the latter binds the OIDC identity to the pre-provisioned local account. - Job talks to Grafana over the in-cluster Service and reads admin creds from the chart-managed `grafana-admin-password` Secret via `envFrom`: no ServiceAccount / Role / RoleBinding. `activeDeadlineSeconds: 900` caps a stuck Job so a broken Grafana cannot hold the release in `pending-upgrade`. Defensive rendering - `.Values.oidc | default dict` in the four templates that dereference the `oidc` block so `oidc: ~` or an omitted block no longer nil-derefs. - `oidc-client-secret.yaml` falls back to a fresh random when the existing Secret lookup succeeds but `data.client-secret` is empty. Rationale: #3176 review from @lllamnyp (architectural CHANGES_REQUESTED points 1, 2, 3 + the default-Viewer leak). The three points stack: dropping the groups incidentally fixes the naming-collision and the default-Viewer leak, and the users-Job gives operators the same UX story as the tenant kube-apiserver PR across both sides of the platform. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
… CEL claim gate Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add the plumbing needed for a Phase-1 OIDC integration on Grafana, matching the shape of #3044 for the tenant kube-apiserver: - values.yaml grows a top-level oidc block with a three-value mode enum (None/System/CustomConfig) and a customConfig payload with the XOR-inline/secretRef surface. - monitoring-rd advertises the new selector via its openAPISchema and keysOrder so operators see it on the Monitoring CR. - templates/_helpers.tpl introduces the naming helpers reused by every downstream OIDC template: clientId, audienceScopeName, clientSecretName, grafanaHost, redirectUri, systemIssuerURL, roleAttributePath, allowAssignGrafanaAdmin (platform-only), assertSystemEnabled, assertCustomConfigXor. No behaviour change on default values — mode: None short-circuits and no OIDC objects are rendered. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…mode New oidc-keycloak.yaml template mirrors the tenant kube-apiserver Phase-1 shape (#3044) with three Grafana-specific adjustments: - Client is confidential (public: false, directAccess: false); Grafana is server-side so a client_secret is required. The value is wired via the EDP-Keycloak `$<secret>:<key>` reference; the backing Secret is created by the next commit. - redirectUri is https://grafana.<host>/login/generic_oauth — the Grafana generic_oauth callback path. - Three KeycloakRealmGroups (`<clientId>-{admin,editor,viewer}`) are chart-owned. Membership is set by an operator through the Keycloak UI or a KeycloakRealmUser CR; grafana.yaml will map group membership to Admin / Editor / Viewer via role_attribute_path. Guarded by mode == System AND _cluster.oidc-enabled == true (via the assertSystemEnabled helper). Renders nothing for None or CustomConfig. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…stance New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from #3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…p + reconcile Job Replace the group-based authorization model on the Phase-1 Grafana OIDC selector with an app-side users map, mirroring the tenant kube-apiserver sibling in #3044. What is removed - The three chart-owned KeycloakRealmGroup objects in the cozy realm (`<clientId>-{admin,editor,viewer}`). A Helm release must not own directory objects — toggling `spec.oidc.mode: System -> None` or deleting the Monitoring release would Helm-delete the groups, and with them any membership curated out-of-band. Per-instance groups in the platform realm is exactly the design question left open at the end of cozystack/community#24; establishing it as precedent from a monitoring PR is out of scope. - Dash-separated group naming (`<namespace>-<release>-<role>`) was also collision-prone against tenant/subtenant paths, since both the namespace and release names can themselves contain dashes. - `role_attribute_path` in the Grafana `generic_oauth` block and the default-Viewer fallback, which handed org-Viewer to every cozy realm identity that logged in — cross-tenant metrics read across every opted-in Grafana. Also removed the `groups` client scope from the KeycloakClient defaults. - `monitoring.oidc.allowAssignGrafanaAdmin` helper and the internal `oidc.grafanaAdmin` values key: the feature was already deferred and the helper's release-name-only guard was tenant-spoofable. What replaces it - `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` on the Monitoring chart values. - A chart-owned post-install/post-upgrade Job (`templates/grafana/oidc-users-job.yaml`) that reconciles that list into Grafana's Main Org. via the admin API: pre-provisions a local Grafana account per listed email so the reconcile can set the org role before the operator's first OIDC login, adds the account to the org, PATCHes the role to converge across re-runs, and prunes every non-admin Main-Org member whose email is not in the list (handles both entry removal and `mode: None`). - `skip_org_role_sync = true` + `oauth_allow_insecure_email_lookup = true` in the generic_oauth block. The former stops a login from overwriting the Job's assignments; the latter binds the OIDC identity to the pre-provisioned local account. - Job talks to Grafana over the in-cluster Service and reads admin creds from the chart-managed `grafana-admin-password` Secret via `envFrom`: no ServiceAccount / Role / RoleBinding. `activeDeadlineSeconds: 900` caps a stuck Job so a broken Grafana cannot hold the release in `pending-upgrade`. Defensive rendering - `.Values.oidc | default dict` in the four templates that dereference the `oidc` block so `oidc: ~` or an omitted block no longer nil-derefs. - `oidc-client-secret.yaml` falls back to a fresh random when the existing Secret lookup succeeds but `data.client-secret` is empty. Rationale: #3176 review from @lllamnyp (architectural CHANGES_REQUESTED points 1, 2, 3 + the default-Viewer leak). The three points stack: dropping the groups incidentally fixes the naming-collision and the default-Viewer leak, and the users-Job gives operators the same UX story as the tenant kube-apiserver PR across both sides of the platform. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
… CEL claim gate Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add the plumbing needed for a Phase-1 OIDC integration on Grafana, matching the shape of #3044 for the tenant kube-apiserver: - values.yaml grows a top-level oidc block with a three-value mode enum (None/System/CustomConfig) and a customConfig payload with the XOR-inline/secretRef surface. - monitoring-rd advertises the new selector via its openAPISchema and keysOrder so operators see it on the Monitoring CR. - templates/_helpers.tpl introduces the naming helpers reused by every downstream OIDC template: clientId, audienceScopeName, clientSecretName, grafanaHost, redirectUri, systemIssuerURL, roleAttributePath, allowAssignGrafanaAdmin (platform-only), assertSystemEnabled, assertCustomConfigXor. No behaviour change on default values — mode: None short-circuits and no OIDC objects are rendered. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…mode New oidc-keycloak.yaml template mirrors the tenant kube-apiserver Phase-1 shape (#3044) with three Grafana-specific adjustments: - Client is confidential (public: false, directAccess: false); Grafana is server-side so a client_secret is required. The value is wired via the EDP-Keycloak `$<secret>:<key>` reference; the backing Secret is created by the next commit. - redirectUri is https://grafana.<host>/login/generic_oauth — the Grafana generic_oauth callback path. - Three KeycloakRealmGroups (`<clientId>-{admin,editor,viewer}`) are chart-owned. Membership is set by an operator through the Keycloak UI or a KeycloakRealmUser CR; grafana.yaml will map group membership to Admin / Editor / Viewer via role_attribute_path. Guarded by mode == System AND _cluster.oidc-enabled == true (via the assertSystemEnabled helper). Renders nothing for None or CustomConfig. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…stance New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from #3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…p + reconcile Job Replace the group-based authorization model on the Phase-1 Grafana OIDC selector with an app-side users map, mirroring the tenant kube-apiserver sibling in #3044. What is removed - The three chart-owned KeycloakRealmGroup objects in the cozy realm (`<clientId>-{admin,editor,viewer}`). A Helm release must not own directory objects — toggling `spec.oidc.mode: System -> None` or deleting the Monitoring release would Helm-delete the groups, and with them any membership curated out-of-band. Per-instance groups in the platform realm is exactly the design question left open at the end of cozystack/community#24; establishing it as precedent from a monitoring PR is out of scope. - Dash-separated group naming (`<namespace>-<release>-<role>`) was also collision-prone against tenant/subtenant paths, since both the namespace and release names can themselves contain dashes. - `role_attribute_path` in the Grafana `generic_oauth` block and the default-Viewer fallback, which handed org-Viewer to every cozy realm identity that logged in — cross-tenant metrics read across every opted-in Grafana. Also removed the `groups` client scope from the KeycloakClient defaults. - `monitoring.oidc.allowAssignGrafanaAdmin` helper and the internal `oidc.grafanaAdmin` values key: the feature was already deferred and the helper's release-name-only guard was tenant-spoofable. What replaces it - `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` on the Monitoring chart values. - A chart-owned post-install/post-upgrade Job (`templates/grafana/oidc-users-job.yaml`) that reconciles that list into Grafana's Main Org. via the admin API: pre-provisions a local Grafana account per listed email so the reconcile can set the org role before the operator's first OIDC login, adds the account to the org, PATCHes the role to converge across re-runs, and prunes every non-admin Main-Org member whose email is not in the list (handles both entry removal and `mode: None`). - `skip_org_role_sync = true` + `oauth_allow_insecure_email_lookup = true` in the generic_oauth block. The former stops a login from overwriting the Job's assignments; the latter binds the OIDC identity to the pre-provisioned local account. - Job talks to Grafana over the in-cluster Service and reads admin creds from the chart-managed `grafana-admin-password` Secret via `envFrom`: no ServiceAccount / Role / RoleBinding. `activeDeadlineSeconds: 900` caps a stuck Job so a broken Grafana cannot hold the release in `pending-upgrade`. Defensive rendering - `.Values.oidc | default dict` in the four templates that dereference the `oidc` block so `oidc: ~` or an omitted block no longer nil-derefs. - `oidc-client-secret.yaml` falls back to a fresh random when the existing Secret lookup succeeds but `data.client-secret` is empty. Rationale: #3176 review from @lllamnyp (architectural CHANGES_REQUESTED points 1, 2, 3 + the default-Viewer leak). The three points stack: dropping the groups incidentally fixes the naming-collision and the default-Viewer leak, and the users-Job gives operators the same UX story as the tenant kube-apiserver PR across both sides of the platform. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
… CEL claim gate Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Add the plumbing needed for a Phase-1 OIDC integration on Grafana, matching the shape of #3044 for the tenant kube-apiserver: - values.yaml grows a top-level oidc block with a three-value mode enum (None/System/CustomConfig) and a customConfig payload with the XOR-inline/secretRef surface. - monitoring-rd advertises the new selector via its openAPISchema and keysOrder so operators see it on the Monitoring CR. - templates/_helpers.tpl introduces the naming helpers reused by every downstream OIDC template: clientId, audienceScopeName, clientSecretName, grafanaHost, redirectUri, systemIssuerURL, roleAttributePath, allowAssignGrafanaAdmin (platform-only), assertSystemEnabled, assertCustomConfigXor. No behaviour change on default values — mode: None short-circuits and no OIDC objects are rendered. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…mode New oidc-keycloak.yaml template mirrors the tenant kube-apiserver Phase-1 shape (#3044) with three Grafana-specific adjustments: - Client is confidential (public: false, directAccess: false); Grafana is server-side so a client_secret is required. The value is wired via the EDP-Keycloak `$<secret>:<key>` reference; the backing Secret is created by the next commit. - redirectUri is https://grafana.<host>/login/generic_oauth — the Grafana generic_oauth callback path. - Three KeycloakRealmGroups (`<clientId>-{admin,editor,viewer}`) are chart-owned. Membership is set by an operator through the Keycloak UI or a KeycloakRealmUser CR; grafana.yaml will map group membership to Admin / Editor / Viewer via role_attribute_path. Guarded by mode == System AND _cluster.oidc-enabled == true (via the assertSystemEnabled helper). Renders nothing for None or CustomConfig. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…stance New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from #3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
…p + reconcile Job Replace the group-based authorization model on the Phase-1 Grafana OIDC selector with an app-side users map, mirroring the tenant kube-apiserver sibling in #3044. What is removed - The three chart-owned KeycloakRealmGroup objects in the cozy realm (`<clientId>-{admin,editor,viewer}`). A Helm release must not own directory objects — toggling `spec.oidc.mode: System -> None` or deleting the Monitoring release would Helm-delete the groups, and with them any membership curated out-of-band. Per-instance groups in the platform realm is exactly the design question left open at the end of cozystack/community#24; establishing it as precedent from a monitoring PR is out of scope. - Dash-separated group naming (`<namespace>-<release>-<role>`) was also collision-prone against tenant/subtenant paths, since both the namespace and release names can themselves contain dashes. - `role_attribute_path` in the Grafana `generic_oauth` block and the default-Viewer fallback, which handed org-Viewer to every cozy realm identity that logged in — cross-tenant metrics read across every opted-in Grafana. Also removed the `groups` client scope from the KeycloakClient defaults. - `monitoring.oidc.allowAssignGrafanaAdmin` helper and the internal `oidc.grafanaAdmin` values key: the feature was already deferred and the helper's release-name-only guard was tenant-spoofable. What replaces it - `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` on the Monitoring chart values. - A chart-owned post-install/post-upgrade Job (`templates/grafana/oidc-users-job.yaml`) that reconciles that list into Grafana's Main Org. via the admin API: pre-provisions a local Grafana account per listed email so the reconcile can set the org role before the operator's first OIDC login, adds the account to the org, PATCHes the role to converge across re-runs, and prunes every non-admin Main-Org member whose email is not in the list (handles both entry removal and `mode: None`). - `skip_org_role_sync = true` + `oauth_allow_insecure_email_lookup = true` in the generic_oauth block. The former stops a login from overwriting the Job's assignments; the latter binds the OIDC identity to the pre-provisioned local account. - Job talks to Grafana over the in-cluster Service and reads admin creds from the chart-managed `grafana-admin-password` Secret via `envFrom`: no ServiceAccount / Role / RoleBinding. `activeDeadlineSeconds: 900` caps a stuck Job so a broken Grafana cannot hold the release in `pending-upgrade`. Defensive rendering - `.Values.oidc | default dict` in the four templates that dereference the `oidc` block so `oidc: ~` or an omitted block no longer nil-derefs. - `oidc-client-secret.yaml` falls back to a fresh random when the existing Secret lookup succeeds but `data.client-secret` is empty. Rationale: #3176 review from @lllamnyp (architectural CHANGES_REQUESTED points 1, 2, 3 + the default-Viewer leak). The three points stack: dropping the groups incidentally fixes the naming-collision and the default-Viewer leak, and the users-Job gives operators the same UX story as the tenant kube-apiserver PR across both sides of the platform. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
… CEL claim gate Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
… (Phase 1) (#3176) <!-- Follow-up to #3044 — same design family, different subject. --> ## What this PR does Adds a Phase-1 OIDC selector to the Grafana instance shipped by `packages/system/monitoring/`, mirroring the shape of the tenant kube-apiserver PR ([#3044](#3044)) so that operators can use one mental model for both. Rationale in [cozystack/community#24](cozystack/community#24) — per-cluster (here: per-Monitoring-release) audience isolation on the flat `cozy` realm, no per-tenant realm. Authorization is **app-side** via a chart-owned `users:` map reconciled by a post-install/post-upgrade Job into Grafana org membership. The chart does NOT own Keycloak directory objects (no `KeycloakRealmGroup`s) and does NOT rely on Grafana's `role_attribute_path` — same design shape as #3044's ClusterRoleBinding reconcile, adapted for Grafana orgs. See `docs/oidc-grafana.md` for the full rationale. The `Monitoring` CR grows a new field: ```yaml spec: oidc: mode: None | System | CustomConfig # default None customConfig: # only for CustomConfig config: {} # inline grafana.ini keys, or... secretRef: name: "" # ...an operator-managed Secret users: # optional; ignored in mode=None - email: alice@example.com role: Admin | Editor | Viewer ``` ### Modes - **`None`** (default) — no OIDC; `admin_user` / `admin_password` Secret is the only auth path. Existing releases render byte-identical. - **`System`** — chart provisions, in the release namespace of the Monitoring CR: - `KeycloakClient` (`public: false`, confidential, `secret` sourced from a chart-owned Secret via the EDP `$<secret>:<key>` reference; `redirectUris` locked to `https://grafana.<host>/login/generic_oauth`). - `KeycloakClientScope` with an `oidc-audience-mapper` pinning `id_token.aud` to the per-release clientId — the isolation primitive. - Persistent Kubernetes Secret carrying the confidential `client-secret` (random on first install, preserved via `lookup`+fallback, same pattern as `packages/system/dashboard`). - Grafana CR's `spec.config.auth.generic_oauth` section wired to the `cozy` realm issuer + per-release audience scope, with two chart-forced settings the users-Job depends on: `skip_org_role_sync: "true"` (login never overwrites the Job's org-role assignments) and `oauth_allow_insecure_email_lookup: "true"` (OIDC identity binds to the pre-provisioned local account by email). - `GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET` env on the Grafana Deployment sourced from the Secret. - Post-install / post-upgrade **users-Job** reconciling `spec.oidc.users` into Grafana's Main Org.: pre-provisions a local account per listed email, `POST /api/orgs/1/users` + `PATCH /api/orgs/1/users/{userId}` to converge role changes, prunes every non-admin Main-Org member whose email is not in the list. - Server-level `GrafanaAdmin` promotion (`allow_assign_grafana_admin`) is out of scope for Phase 1. Every Grafana instance (platform and tenant) caps at org-level `Admin`. - **`CustomConfig`** — tenant supplies the whole `[auth.generic_oauth]` payload; `cozy` is not in the path. Two mutually exclusive paths: - `customConfig.config: {...}` — inline map of `grafana.ini` keys. The chart **merges** the two chart-forced settings (`skip_org_role_sync`, `oauth_allow_insecure_email_lookup`) on top so the users-Job contract holds. The operator can NOT override those two (merge wins). - `customConfig.secretRef.name: ...` — operator-owned Secret with an `auth.ini` fragment; the chart mounts it under `/etc/grafana/oidc` and wires `GF_PATHS_CUSTOM_INI`, and does NOT emit `auth.generic_oauth` under `spec.config` (the ini fragment is authoritative). Because the chart cannot inject the two settings into a mounted ini, `spec.oidc.users` is NOT supported in this branch — the chart hard-fails render on that combination. ### Fail-close guards - `mode: System` without `_cluster.oidc-enabled == "true"` (the platform-level `authentication.oidc.enabled` flag) → hard-fail with an actionable message pointing at the platform values. - `mode: System` on a cluster where the EDP Keycloak operator CRDs (`v1.edp.epam.com/v1`) are not registered → hard-fail (symmetric on `oidc-keycloak.yaml` and `grafana.yaml`). - `mode: CustomConfig` with both or neither of `config` / `secretRef.name` → hard-fail. - `mode: CustomConfig` with `secretRef.name` AND non-empty `users` → hard-fail (chart cannot inject the two forced settings into an operator-mounted ini; forcing an explicit choice avoids silent contract loss). - `spec.oidc.users[]` entry with missing / unknown `role` or malformed `email` → hard-fail at render time so the users-Job never fires with a body that would 400 on Grafana's admin API. ### Break-glass posture `admin_user` / `admin_password` and `disable_login_form: false` are unchanged in every mode. Locking the form off under `mode: System` is a documented follow-up hardening (see `docs/oidc-grafana.md`). ### What's NOT in this PR (documented as out of scope) - Per-tenant Keycloak realms (Phase 2 territory, tracked in community#24). - Full-logout through Keycloak's end-session endpoint (`backend-logout-url` and matching Keycloak client attribute). - CEL `claimValidationRules` gate on `email_verified` — layered guarantees are described in `docs/oidc-grafana.md`. - Multi-issuer composition on one Monitoring release (mutually exclusive modes). - Server-level `GrafanaAdmin` promotion. ### Docs - New `docs/oidc-grafana.md` — full operator guide (parallel to `docs/oidc-tenant.md`). - Website page follows in a paired PR against cozystack/website (`docs/monitoring-oidc-authentication` branch, under `content/en/docs/next/operations/services/monitoring/oidc-authentication.md`). ### Tests - 33 `helm-unittest` cases in `tests/oidc_test.yaml` + `tests/oidc_crd_missing_test.yaml` cover: all three modes; defensive nil paths (`oidc: ~`); System-mode Keycloak client + audience scope shape; CustomConfig chart-forced merge (positive assertion) AND `mode=CustomConfig inline — operator cannot override chart-forced OIDC settings` (regression guard); CustomConfig secretRef + `users` incompat fail-fast; `spec.oidc.users[].role` missing/unknown fail-fast; users-Job shape (hook annotations, RBAC-free, activeDeadlineSeconds, `ttlSecondsAfterFinished: 3600`, admin credential wiring); no chart-owned `KeycloakRealmGroup`s in any mode; no `role_attribute_path` in Grafana config. - Render-side bats in `hack/e2e-apps/monitoring-oidc-{system,customconfig}.bats` — mirror the tenant kube-apiserver's e2e shape (no live browser flow — deferred to a follow-up integration suite, same as #3044). Asserts on live cluster: users-Job appears with `DESIRED_USERS_JSON` populated, Grafana config carries `skip_org_role_sync=true` and no `role_attribute_path`, no chart-owned `KeycloakRealmGroup`s land in `cozy`. ### Screenshots N/A — no UI change on default values; the visual delta is Grafana's own "Sign in with Keycloak" button appearing under the login form when a release opts in. ### Release note ```release-note feat(monitoring): Each `Monitoring` CR can now opt its Grafana instance into OIDC via a flat selector — `spec.oidc.mode: System | CustomConfig | None` (default `None`). `System` trusts the platform `cozy` realm via a per-instance confidential Keycloak client + audience binding. Authorization is app-side: `spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` is reconciled into Grafana's Main Org. by a chart-owned post-install/post-upgrade Job (pre-provision → org add → PATCH role → prune stale). `CustomConfig` accepts a tenant-supplied `[auth.generic_oauth]` payload — inline as a map (chart merges the two contract-critical settings `skip_org_role_sync` + `oauth_allow_insecure_email_lookup` on top) or as a Secret with an `auth.ini` key (in which case `spec.oidc.users` is not supported and the chart fails render on that combination). Server-level `GrafanaAdmin` promotion is out of scope for Phase 1. The `admin_user`/`admin_password` Secret stays a documented break-glass path in every mode. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added OIDC authentication for Monitoring Grafana with `None`, platform-managed `System`, and tenant-controlled `CustomConfig` modes. * System mode provisions platform-backed OIDC for each instance (including persistent client credentials) and can optionally promote matching users to server-level `GrafanaAdmin`. * CustomConfig supports inline OAuth settings or Secret-backed `auth.ini` (with mutual exclusivity validation).
What this PR does
Implements Phase 1 of cozystack/community#24: a per-cluster OIDC selector on the
KubernetesCR that authenticates kubectl users without provisioning a new Keycloak realm per tenant. Per-cluster audience binding stays as the isolation primitive; per-tenant identity ownership (Phase 2) is explicitly deferred.apps/kubernetes —
Kubernetes.spec.oidcA flat selector + per-user RBAC list:
None— current behaviour. Only the static admin kubeconfig works.System— trust the platformcozyrealm via a per-cluster publicKeycloakClient(<ns>-<release>) plus aKeycloakClientScopecarrying anoidc-audience-mapperpinned to that clientId. The cross-cluster token replay path is closed by audience binding.CustomConfig— accept a tenant-suppliedAuthenticationConfiguration, either inline (customConfig.config) or via an existing Secret (customConfig.secretRef.name); the two are mutually exclusive and the chart fails the render when neither — or both — are set.The kube-apiserver is wired with the structured
apiserver.config.k8s.io/v1beta1 AuthenticationConfiguration(file form), not the legacy--oidc-*flags. The Secret is mounted on theKamajiControlPlaneat/etc/kubernetes/authentication-config/config.yaml— the design-spec paths and names.Per-user RBAC + OIDC kubeconfig Secret
A post-install/post-upgrade Job (Helm hook, scoped ServiceAccount,
policy.cozystack.io/allow-to-apiserver: "true") reconcilesusers[]into the tenant cluster:ClusterRoleBindingper entry, labelledapp.kubernetes.io/managed-by=cozystack-oidc.adminmaps toClusterRole/cluster-admin;viewmaps toClusterRole/view. TheUser:subject is the literalemail.users[]revokes access on the next reconcile.mode: System, the same Job patches<release>-oidc-kubeconfigon the management cluster (server URL, per-cluster client-id, issuer URL, tenant CA extracted from the Kamaji-issued admin kubeconfig,kubectl oidc-loginexec block). The Secret is pre-rendered as a chart-managed placeholder sohelm uninstallreaps it.packages/system/kubernetes-rd/cozyrds/kubernetes.yamlexposes the new Secret name alongside the existing admin kubeconfig so the dashboard surfaces it.modetoggle safety — the normalize pathThe Job is always rendered — not gated on
mode != None. Inmode=Noneit takes a cleanup branch instead:kubectl patch --type=mergethe KamajiControlPlane to remove OIDC-only entries fromspec.apiServer.{extraArgs,extraVolumeMounts}andspec.deployment.extraVolumes, andkubectl deleteany lingering chart-ownedKeycloakClient,KeycloakClientScope,oidc-authn-configSecret, andoidc-kubeconfigSecret.This is not decorative — it fixes a real upgrade-time bug uncovered on dev3. Kamaji's controller writes back to the
KamajiControlPlanewhen syncing to theTenantControlPlane, taking Server-Side Apply field ownership of those spec fields. Onmode=System → None, Helm's subsequent apply relinquishes ownership but the fields keep their previous OIDC-containing values, and helm-controller's SSA-based delete on absent namespaced resources doesn't reap the chart-owned Secrets and Keycloak objects either. Without the normalize path, the tenant apiserver would keep the dangling--authentication-configflag pointing at a Secret the chart had already reaped — refusing to start on the next kubelet restart. With it,mode=Noneleaves zero chart-owned OIDC leftovers.What is intentionally NOT here
Tenant.spec.oidcand no_namespace.oidc-realmpropagation. Phase 1 doesn't introduce tenant-hierarchy walk-up.packages/extra/oidcsub-chart, nopackages/system/oidc-rd. Per-tenant realms are Phase 2.system/keycloak-configurechanges. The existing<tenant>-*cozy realm groups continue to drive management-cluster RBAC unchanged.Open Question 1 — CustomConfig API shape
The design proposal left the CustomConfig API surface open. This PR resolves it as both
customConfig.config(inline string) andcustomConfig.secretRef.name(existing Secret reference) — mutually exclusive, with a render-time fail when neither or both are set. Inline is the zero-config path; the secretRef escape hatch lets the operator manage the AuthenticationConfiguration under their own GitOps without copying it into aKubernetesCR. The BYO audience replaces (rather than composes with) the per-cluster audience —cozyis not in the path in CustomConfig mode, so no per-cluster Keycloak client is provisioned and the tenant defines the audience inside their own AuthenticationConfiguration.Backward compatibility
oidc.modedefaults toNone. ExistingKubernetesCRs get an unchanged KamajiControlPlane surface — the kube-apiserverextraArgsandextraVolumeMountsare byte-identical tomain(verified viahelm templatediff withtests/values/common.yaml). Because Helm's JSON-merge-patch on the KCP CRD replaces arrays wholesale, the chart also renders a small normalize path inmode: None— aServiceAccount,Role,RoleBinding, and one-shot<release>-oidc-bootstrapJob — so a future toggle fromSystem/CustomConfigback toNonecleanly reverts the KCP arrays instead of leaving stale entries.oidc.enabled: truefield from an earlier iteration of this PR keep working: the field is silently ignored by the Phase-1 chart (it reads onlyoidc.mode), so no CR migration is required.oidc-kubeconfigSecret placeholder,oidc-authn-configSecret,KeycloakClient,KeycloakClientScope— are all release-owned sohelm uninstallreaps them.Test plan
make generateinpackages/apps/kubernetes— schema, README, types, cozyrds embedded schema all regenerated; no drift.hack/update-codegen.sh— deepcopy/openapi/conversion/applyconfig/client all regenerated; no drift.make testinpackages/apps/kubernetes— 135 + 25 OIDC = 160 cases pass.helm templatediff vsorigin/mainwith default values = 0 bytes on the OIDC surface.hack/e2e-apps/kubernetes-oidc-system.bats— System-mode render-side assertions.hack/e2e-apps/kubernetes-oidc-customconfig.bats— CustomConfig render-side assertions.tenantsecretsendpoint exposing both admin and oidc kubeconfigs,mode=System → Nonecleanup path leaving zero leftovers.kubectl oidc-loginagainst a Keycloak-backed tenant cluster — explicitly out of scope per the design proposal.Bugs caught only by the dev3 e2e — none would have been surfaced by
helm unittest:standardFlowfield absent from the EDP KeycloakClient CRD → Helm install failedrm --force,grep --line-regexp— BusyBoxcoreutilsin the alpine/k8s image only supports short flags--request-timeouton the pre-delete cleanup kubectl (unreachable tenant apiserver would hang)digfor_cluster.{oidc-enabled,root-host}— nil-safe when the platform bundle omits the keykubectl wait secretin bats (avoid immediate NotFound flakes)extraArgs/extraVolumeMountsemitted as[]on the off-path — otherwise Helm's SSA doesn't send a REPLACE patch and stale OIDC values survivemode=System → Nonemode=None— force-clean chart-owned OIDC objects Kamaji's SSA ownership keeps Helm from reapinggetverb on the<release>-oidc-kubeconfigSecret —kubectl patch --type=mergefetches before mergingresourceNameson the Keycloak delete verbsAuthenticationConfiguration.claimMappings.username.claimfrompreferred_usernametoemail— end-to-end OIDC on dev3 revealed apiserver identifies user bypreferred_username(= Keycloak usernamekvaps,i.okhotnikov), NOT the emailusers[].emailsupplies; every user whose Keycloak username differs from their email would silently 403 after successful token auth--oidc-extra-scope=emailto the chart-generatedkubectl oidc-loginblock — did not depend on realm-level default-scope hygienespec.oidc.mode != None+ user-supplied--oidc-*/--authentication-config=incontrolPlane.apiServer.extraArgs— kube-apiserver refuses to boot with both, so an existing customer who wired OIDC by hand would BOOT-FAIL on upgradeCompanion
OIDC for Grafana follows in a separate PR against
packages/extra/monitoringonce this lands (a per-clustergrafana-<cluster>client +auth.generic_oauthblock mapping the same per-cluster group). Phase 2 (managed multi-tenant identity — Keycloak Organizations vs per-tenant realm) is a separate proposal as scoped in the design doc.Release note
Summary by CodeRabbit
New Features
Bug Fixes
Documentation