mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 08:37:31 +09:00
Aleksei Sviridkin
4bf95815a8
## What this PR does Tenant clusters with `spec.oidc.mode: System` or `CustomConfig` authenticate users fine, but a UID does not survive the hop to an aggregated API server. The tenant kube-apiserver publishes `requestheader-uid-headers` into the `extension-apiserver-authentication` ConfigMap only when the flag is set, and the chart never set it. The aggregation layer still sends `X-Remote-Uid`, but the extension server trusts only the header names published in that ConfigMap, so it ignores the one it gets and the call arrives without a UID. The field documentation even told operators to add the flag by hand via `controlPlane.apiServer.extraArgs`. It forwards a UID rather than creating one. ServiceAccount tokens carry one, and for those the aggregation hop starts working. Users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only; giving them one means mapping a claim, which changes the identity of users who have none today, and is tracked separately. The chart now renders `--requestheader-uid-headers=X-Remote-Uid` on v1.32 and newer, and adds `--feature-gates=RemoteRequestHeaderUID=true` on v1.32, where the gate is still alpha and the apiserver rejects the flag without it. From v1.33 the gate is beta and on by default, so only the header flag is rendered. v1.31 gets nothing: the flag does not exist there, and writing it by hand stops the apiserver from starting. `--emulated-version` in the same passthrough moves that decision, because the gate resolves through the versioned spec at the emulated version. Rendering the gate on every version from v1.32 rather than on v1.32 alone was the alternative, and it would remove the migration step entirely, because an opt-out would then drop two entries instead of one and move the entry count by itself. It loses on what it leaves behind: a redundant gate term on every cluster, which stops the apiserver from starting on the day the gate is retired from the known list. The design also prefers rendering over failing for the same reason the fail branches are narrow, since a cluster stranded on a HelmRelease that no longer converges is a worse outcome than one that never gets the UID fix. The chart reads an operator entry the way the apiserver reads it: header list elements matched whole and untrimmed and folded as `EqualFold` folds them, gate names compared exactly, gate values resolved as `strconv.ParseBool` resolves them, `AllAlpha` and `AllBeta` blankets honoured, an empty value treated as no entry. On v1.32 an entry that says nothing about `RemoteRequestHeaderUID` gets a second one carrying the operator's own terms plus `RemoteRequestHeaderUID=true`. Carrying those terms into the merged copy is what makes it land correctly whether the tenant control plane collapses duplicate flags into the last one or hands both to the apiserver, which merges them in the order it reads them. Tests cover the render matrix over mode, version and operator input: every fail path with its exact message, the ownership and merge paths, duplicate entries in both orders, blanket gates, empty values, and case variants on the header name and on the gate value. The latest-version tenant lane creates its tenant with `spec.oidc.mode: System` and asserts the flag on the rendered KamajiControlPlane, so the tenant apiserver starts with the flag in CI. Nothing asserts the UID arriving at an aggregated apiserver; that needs an extension server inside the tenant and is not in this change. The layer where the count-keyed reset actually bites is the tenant Deployment's arguments, and asserting there needs a live tenant control plane, so it is not in this change either. The rule this follows is that a configuration which renders and boots today keeps rendering. Both flags are new, so an `extraArgs` shape reaching this code was rendering before it existed, and stranding a running tenant on a HelmRelease that no longer converges is a worse outcome than one that never gets the fix. A few shapes do stop rendering, and reaching any of them takes writing `--requestheader-uid-headers` into the passthrough by hand, attempting exactly what the chart now does and getting it wrong; in each the kube-apiserver fails option validation and never starts, so nothing that was working is lost. `docs/oidc-tenant.md` covers these shapes. One known limitation is in there and worth naming here: on v1.33 and newer, adding the documented opt-out is count-neutral in `extraArgs`, and so is removing it again, and on v1.33 and v1.34 adding or removing `--emulated-version=1.31`. The pinned Kamaji build rewrites a running apiserver's arguments only when that count changes and otherwise keeps every flag that has left the list, so either the header flag stays beside a now-disabled gate or the disabled gate stays beside the header flag, and the apiserver refuses to start. Changing the entry count in the same edit is the mitigation, and the fix belongs to the pin rather than to this chart, tracked in #3541. The paths that need no operator edit are clear: turning the feature on for a default tenant moves the count, and a tenant already carrying the opt-out renders byte-identically before and after. Out of scope: `mode: None` still publishes no UID header even though ServiceAccount tokens carry UIDs. That is pre-existing, and widening the condition changes the render for every default tenant. Tracked separately, along with the claim mapping above. Closes #3377 ### Screenshots Not a UI change. ### Downstream repositories - [x] No downstream repository is affected by this change Checked the trigger map against the diff: no package added or removed, no schema field, enum or default changed (description text only), no ApplicationDefinition surface touched, no hack/ layout moved. The managed-apps reference page picks up the new field description through release-time README regeneration. ### Release note ```release-note fix(kubernetes): tenant clusters with `spec.oidc.mode` set to `System` or `CustomConfig` now render `--requestheader-uid-headers=X-Remote-Uid` on the tenant kube-apiserver, plus the `RemoteRequestHeaderUID` feature gate on v1.32, so that an aggregated API server can read the caller's UID out of the request headers. Without that flag the tenant apiserver never publishes `requestheader-uid-headers`, and an aggregated API server ignores the UID header even for callers that have one, ServiceAccount tokens among them. It forwards a UID rather than creating one: users authenticated by `spec.oidc.mode: System` still have none, because the generated AuthenticationConfiguration maps `username` and `groups` only, and that is tracked separately. Adding the flag by hand through `controlPlane.apiServer.extraArgs` is no longer necessary, and an entry of your own is respected: the chart steps aside when yours already carries `X-Remote-Uid` or already enables the gate, merges `RemoteRequestHeaderUID=true` into a copy of a v1.32 `--feature-gates` entry that says nothing about the gate, and renders neither flag when you switch the gate off. The render fails where a `--requestheader-uid-headers` list of your own would stop the kube-apiserver from starting anyway: one that omits `X-Remote-Uid`, one with an empty element, and a list of your own while the gate is off. On v1.33 and newer there is a migration step, and it applies in both directions: the pinned Kamaji build rewrites a running apiserver's arguments only when the number of `extraArgs` entries changes, and otherwise keeps every flag that has left the list. Adding the documented opt-out keeps that number the same, so the header flag already on the Deployment stays beside the now-disabled gate. Removing the opt-out later keeps it the same too, so the disabled gate stays on the Deployment beside the header flag the chart renders again, although your spec no longer shows it. Adding or removing `--emulated-version=1.31` has the same shape both ways on v1.33 and v1.34. The apiserver refuses to start in each case, so change the entry count in the same edit. ```