docs(unified-tls-pki): deliver the trust anchor through a declared sentinel#41
Conversation
…ntinel
Rewrite the interior delivery mechanism (§5) from a source-selection
label stamped on the CA Secret to a declared, namespaced sentinel CRD
(working name TenantProjection, group internal.cozystack.io) rendered by
the app chart. The chart names the source Secret and key explicitly
(spec.projections: [{type: CACert, sourceSecretName, sourceKey}]) rather
than coaxing a label onto operator-created output.
Why the sentinel is better than the label:
- It is a first-class object, not object semantics emulated with a
boolean. The projection owner-refs the sentinel; the sentinel carries
no ownerReferences and is attributed to its release by Flux's
helm.toolkit.fluxcd.io/name label, so lineage resolves
Secret -> sentinel -> HelmRelease -> app unchanged and GC is native.
- status.conditions makes a broken declaration observable
(Ready=False, Reason=SourceNotFound) and alertable; the label leg
could only requeue silently.
- A VAP pins the writer to helm-controller as defense-in-depth on top of
RBAC, which already denies tenants write to internal.cozystack.io.
- Naming the source by name closes the proposal's own open question
'Labelling operator-created CA sources' — CNPG/PSMDB need not label
their output.
The change is entirely internal: the tenant-facing consume contract in
§4 is untouched (still <release>.tenant-ca, only ca.crt). The two-tier
model, the preference for a key-free source, and the per-engine order
are unchanged. Edge cases, testing, security, rollout, open questions
and alternatives are updated to match.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the unified TLS PKI design proposal, replacing the previous label-based source selection with a first-class TenantProjection custom resource (sentinel) and an extraction controller. This sentinel-based approach resolves release-attribution issues, simplifies handling of operator-created CA Secrets, and improves observability. Feedback on the proposal suggests addressing a potential conflict where multiple projections in the list could overwrite the same target Secret, and recommends adding an administrative escape hatch to the ValidatingAdmissionPolicy to ensure cluster administrators can perform manual troubleshooting.
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.
| projections: | ||
| - type: CACert |
There was a problem hiding this comment.
The schema defines spec.projections as a list, but section (b) states that the controller upserts a Secret named <release>.tenant-ca for each CACert projection.\n\nIf a TenantProjection contains multiple CACert projections in the list, they will target the same Secret name (<release>.tenant-ca) and overwrite each other. To support multiple projections or avoid conflicts, consider either:\n1. Adding an optional targetSecretName or suffix field to the projection item schema.\n2. Simplifying spec.projections to a single object if only one projection is ever intended per TenantProjection sentinel.
|
|
||
| **(c) Attribution, lineage, and garbage collection: the sentinel is a plain chart object.** The sentinel itself carries **no** `ownerReferences`. Flux stamps every object it renders with the `helm.toolkit.fluxcd.io/name` label naming the release, and that label — not an owner-reference — is how the sentinel is attributed to its release, exactly as for any chart-rendered object. So the lineage walk resolves the chain **Secret → sentinel → HelmRelease → application** with no change to the webhook: the projected Secret owner-refs the sentinel, the sentinel's release label names the `HelmRelease`, and the `HelmRelease` backs the application. Garbage collection is native and needs no delete path in the controller: deleting the application deletes the `HelmRelease`, Flux prunes the chart's objects including the sentinel, and Kubernetes then garbage-collects the projection the sentinel owns. Marking the projection tenant-visible is unchanged from §4 — the controller stamps `internal.cozystack.io/tenant-ca: "true"` on it, one generic `ApplicationDefinition.spec.secrets.include` entry (`matchLabels: {internal.cozystack.io/tenant-ca: "true"}`) matches it by label (`internal/lineagecontrollerwebhook/matcher.go`), and the lineage webhook then walks the projection's owner chain and authoritatively stamps `internal.cozystack.io/tenantresource` to `true` (and to `false` should the Secret ever stop matching). | ||
|
|
||
| **(d) Admission: the sentinel's writer is pinned to helm-controller.** A ValidatingAdmissionPolicy pins writes of `TenantProjection` to Flux's helm-controller service account (`system:serviceaccount:cozy-fluxcd:helm-controller`), so the only way a sentinel enters the cluster is by being rendered into a release. This is **defense-in-depth**, not the primary control: `internal.cozystack.io` resources are already not tenant-writable by RBAC, so a tenant cannot create a sentinel to aim the controller at an arbitrary Secret. The VAP makes that guarantee explicit at admission and narrows the writer to exactly the principal that renders charts. |
There was a problem hiding this comment.
Pinning writes of TenantProjection strictly to the helm-controller service account via a ValidatingAdmissionPolicy (VAP) is a strong defense-in-depth measure, but it will block cluster administrators (cluster-admin or members of system:masters) from performing manual troubleshooting, emergency hotfixes, or manual resource cleanup.\n\nConsider updating the VAP rules to allow an escape hatch for administrative users/roles (e.g., by checking the request's user groups or service account) to ensure the system remains maintainable and debuggable during operational incidents.
1 participant
Rewrites section 5 of unified-tls-pki to deliver the trust anchor through a declared
TenantProjectionsentinel instead of the label leg. Matches the controller rework in cozystack/cozystack#3299.Section 4 does not change: a tenant still reads
<release>.tenant-ca, onlyca.crt. Section 5 is the delivery mechanism and only that changes. The app chart declares the source in a namespaced CRD, the controller watches the sentinel, the projection is owner-refed to it, and lineage resolves through thehelm.toolkit.fluxcd.io/namelabel unchanged. The sentinel carries status conditions, so a broken declaration surfaces instead of requeuing silently.Stacks on #36 (canonical-name fix), so it targets that branch. Rebase onto main once #36 lands.