mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 16:47:20 +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. ```
Chainsaw E2E tests
The Cozystack E2E app suite, written for Kyverno Chainsaw — a declarative, Kubernetes-native E2E test framework (CNCF, part of the Kyverno project; successor to KUTTL).
This directory replaces the per-app BATS suite that used to live in hack/e2e-apps/*.bats. The CI e2e job runs chainsaw test hack/e2e-chainsaw/ after make install-cozystack; cluster bootstrap (hack/e2e-prepare-cluster.bats, hack/e2e-install-cozystack.bats) and the OpenAPI checks (hack/e2e-test-openapi.bats) remain BATS.
Layout
hack/e2e-chainsaw/
├── .chainsaw.yaml # global Configuration (namespace, timeouts, JUnit report)
├── _lib/ # shell reused by the script-heavy suites
│ ├── run-kubernetes.sh # tenant Kubernetes bringup + LB/NFS/ouroboros checks
│ └── remediation-guard.sh
├── <app>/
│ ├── chainsaw-test.yaml # one or more Test definitions
│ └── <app>.yaml # the manifests the test applies
└── ...
Suites: postgres, bucket, mariadb, mongodb, redis, valkey, qdrant, clickhouse, kafka, rabbitmq, etcd, openbao, harbor, foundationdb, external-dns, kuberture, vminstance, gateway, opensearch, kubernetes-latest, kubernetes-previous, securitygroup.
What Chainsaw buys over the BATS suite
- Readable tests — a list of resources and expected states, not shell control flow. The
timeout N sh -ec "until kubectl get ..."+kubectl waitpair that appeared ~100 times collapses into a singleassertthat polls existence and state together. - Distinguishable failure modes — "object never created", "created but never ready", and "wrong status" all surface as a structured diff instead of an opaque shell timeout.
- Automatic diagnostics —
catchblocks dump events,describe, and pod logs on any failure (the BATS suite only did this inharbor.bats, by hand). - Automatic cleanup — resources Chainsaw applied are deleted (and waited on) in reverse order;
finallycovers operator-created leftovers. - CI-ready output — a JUnit
chainsaw-report.xmlis emitted for the test summary.
Test anatomy
Each suite is a directory with a chainsaw-test.yaml (one or more Test docs) plus the manifests it applies. A step has try (operations: apply, assert, error, script, delete, …), optional catch (diagnostics on failure), and optional finally (teardown regardless of outcome).
Declarative suites assert on status.conditions and concrete fields. Inherently imperative suites keep their logic in script steps:
- openbao —
bao operator init/unsealviakubectl exec. - kuberture —
jqon Service annotations + external-dns split-horizon log assertions. - gateway — admission-rejection cases that need
kubectl --asimpersonation or error-message greps; the tenant apex is read from the namespacenamespace.cozystack.io/hostlabel at runtime so they pass on any host. - kubernetes-latest/previous — wrap
_lib/run-kubernetes.sh(Kamaji bringup, port-forward, LB/NFS/ouroboros). The scriptcds to the repo root (the test dir ishack/e2e-chainsaw/<suite>, so repo root is../../..).
Conventions / gotchas (Chainsaw v0.2.15)
- Condition asserts use the filter-as-list form, not the documented
[0]index:Thestatus: (conditions[?type == 'Ready']): - status: "True"(conditions[?type == 'Ready'])[0]: {status: "True"}idiom fails with "field not found in the input object". - A reading of a failure diff: the "actual" side is a projection onto the expected fields —
status: {}means the JMESPath expression failed, not that status is empty. Read the error line above the diff first. errorwith a bare resource ref (name only) asserts the resource does not exist (used for "PVC reclaimed", "no backup schedule", "child has no Gateway HR").scriptruns in the test directory with$NAMESPACEset to the config namespace (tenant-test).
Running locally
Requires a cluster with Cozystack installed and a tenant-test namespace (the environment hack/e2e-install-cozystack.bats produces). bucket additionally needs mc and nc on the host; kuberture/openbao/harbor need jq.
# install chainsaw
go install github.com/kyverno/chainsaw@latest # or: brew install kyverno/chainsaw/chainsaw
# run all suites (config picked up from .chainsaw.yaml)
cd hack/e2e-chainsaw
chainsaw test
# run a single suite
chainsaw test postgres/
kubernetes-latest/kubernetes-previous each bring up a full tenant Kubernetes cluster (~25 min, real worker nodes) and vminstance boots a nested VM — expect a long run for the full set.
CI integration
The e2e job in .github/workflows/pull-requests.yaml runs make -C packages/core/testing test-chainsaw (which execs chainsaw test hack/e2e-chainsaw/ inside the e2e sandbox) and uploads chainsaw-report.xml. The sandbox image (packages/core/testing/images/e2e-sandbox/Dockerfile) ships the chainsaw binary alongside kubectl/helm/mc/jq/nc.