feat(controller): engine-agnostic CA extraction for the tenant trust anchor - #3299
feat(controller): engine-agnostic CA extraction for the tenant trust anchor#3299Aleksei Sviridkin (lexfrei) wants to merge 38 commits into
Conversation
github-actions
Bot
added
area/uncategorized
|
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:
📝 WalkthroughWalkthroughAdds an ApplicationDefinition CA source contract, a controller that creates key-free ChangesCA certificate extraction
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant HelmRelease
participant CACertReconciler
participant ApplicationDefinition
participant SourceSecret
participant TenantCASecret
HelmRelease->>CACertReconciler: reconcile application release
CACertReconciler->>ApplicationDefinition: read caCert source declaration
CACertReconciler->>SourceSecret: read declared or labelled CA data
CACertReconciler->>CACertReconciler: validate PEM and retain ca.crt
CACertReconciler->>TenantCASecret: create or update key-free projection
Possibly related issues
Possibly related PRs
Suggested labels: 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 |
github-actions
Bot
added
the
size/XXL
dosubot
Bot
added
the
area/platform
Aleksei Sviridkin (lexfrei)
removed
the
area/uncategorized
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 implements a secure, engine-agnostic framework for distributing CA certificates to tenants. By decoupling the trust anchor from the underlying operator-managed secrets, the platform prevents the accidental exposure of private key material. The solution provides a robust, fail-closed extraction path that supports both label-based and explicitly declared sources, ensuring consistent and secure trust anchor delivery across various managed services. 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
|
![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 introduces a CA-extraction controller (cacert) that projects the TLS trust anchor of managed applications into a canonical, key-free Secret (<release>-tenant-ca) in the release namespace, allowing tenants to securely read CA certificates without exposing private keys. It updates the ApplicationDefinition CRD and CRD schema to support authoritative caCert declarations, configures the Postgres application definition to use this new mechanism, and updates the cozy-lib library chart helper to align with the new canonical naming and labelling. Additionally, it introduces a ValidatingAdmissionPolicy to restrict writes to CA-source Secrets to cert-manager only, updates controller RBAC permissions, and adds comprehensive unit and integration tests. I have no feedback to provide as there are no review comments to assess.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@internal/controller/cacert/reconciler.go`:
- Around line 565-576: Treat any source Secret already occupying the canonical
projection name as a collision, regardless of whether it contains private key
material: update reconcileSourceAtCanonicalName to emit the required collision
warning and requeue instead of silently succeeding. Update
internal/controller/cacert/reconciler_test.go lines 524-550 to assert the
collision warning and resulting behavior rather than a successful no-op.
- Around line 885-895: Validate the namespace parsed from SourceRefAnnotation
against hr.Namespace before calling r.Reader.Get in the existing source-lookup
block. Reject or ignore references where ns differs from hr.Namespace, ensuring
no cross-namespace Secret read occurs while preserving same-namespace lookup
behavior.
- Around line 495-502: Update the reconciliation path around the src == nil
branch and pruneProjection so the persisted source mode remains available when
the declared Secret disappears, and pass the current ApplicationDefinition into
pruning to distinguish a removed declaration from a temporarily absent source.
Ensure removing spec.caCert withdraws the previously projected trust anchor, and
add a regression test covering source disappearance before declaration removal.
- Around line 960-968: Update projectionData to fully decode and parse every PEM
block as a certificate, rejecting malformed or truncated payloads, trailing
bytes, and non-certificate blocks while preserving private-key rejection. In
internal/controller/cacert/reconciler.go lines 960-968, replace header matching
with complete validation; in internal/controller/cacert/reconciler_test.go lines
66-67, use valid certificate fixtures; and in
internal/controller/cacert/reconciler_test.go lines 1383-1398, add malformed,
trailing-data, and non-certificate-block cases.
- Around line 1085-1089: Update the rendered Secret name validation after
strings.TrimSpace in the surrounding name-rendering function to reject values
that are not valid DNS1123 subdomains, using the existing
CACertDeclarationInvalid error/result convention instead of the generic error.
Preserve the empty-name check and return the validated name only when it
satisfies the Kubernetes naming rules.
🪄 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: 8f836b39-3c2b-4df5-a46a-93f85ee43477
📒 Files selected for processing (16)
api/v1alpha1/applicationdefinitions_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/cozystack-controller/main.gointernal/controller/cacert/postgres_wiring_test.gointernal/controller/cacert/reconciler.gointernal/controller/cacert/reconciler_test.gopackages/library/cozy-lib/templates/_tls.tplpackages/system/application-definition-crd/definition/cozystack.io_applicationdefinitions.yamlpackages/system/cozystack-basics/templates/publish-ca-cert-writer-policy.yamlpackages/system/cozystack-basics/tests/publish-ca-cert-writer-policy_test.yamlpackages/system/cozystack-controller/Makefilepackages/system/cozystack-controller/templates/rbac.yamlpackages/system/cozystack-controller/tests/rbac_test.yamlpackages/system/postgres-rd/cozyrds/postgres.yamlpackages/tests/cozy-lib-tests/templates/tests/tls-cacert.yamlpackages/tests/cozy-lib-tests/tests/tls_cacert_test.yaml
myasnikovdaniil
left a comment
myasnikovdaniil
left a comment
There was a problem hiding this comment.
Reviewed the whole controller, the VAP, the RBAC delta, the cozy-lib guard, and the tests as the tree rather than the diff, and traced both source legs end to end against the tenant read path. The security story holds: tenants read trust anchors only through the label-gated, read-only tenantsecrets API and cannot write raw Secrets, the label leg is admission-gated to cert-manager, and the declared leg stays authoritative over the forgeable label leg — so the change fails closed on every tenant-influenceable path, is namespace-confined with no cross-tenant vector, and is unusually well tested (forged-label override, forged-projection healing, non-Opaque collision, recreate-rehome, poisoned rotation, prune discrimination).
Approving — everything below is non-blocking. The two follow-ups most worth doing are the x509-parse hardening of the write-path guard (it currently validates by header substring only) and tightening the prune/canonical-name corner, where a retired anchor can linger if a declaration is removed while its source is momentarily absent. Two nits I did not inline: selectorsDigest keeps only the low 64 bits of the SHA-256 (reconciler.go:952), and applicationDefinition returns the first kind match without flagging duplicate kinds (reconciler.go:696) — both harmless with platform-authored input.
| if containsPrivateKey(pem) { | ||
| return nil, errPrivateKey | ||
| } | ||
| if !certificateHeader.MatchString(pem) { |
There was a problem hiding this comment.
Non-blocking, defense-in-depth. This guard validates the trust anchor by header substring — BEGIN CERTIFICATE present and no BEGIN … PRIVATE KEY — but never decodes the PEM or runs x509.ParseCertificate, so certificate armor wrapped around non-certificate bytes (e.g. headerless DER) passes and is copied verbatim. It is not reachable from a shipped path: both legs' sources come from trusted writers (the operator-created <release>-ca, or cert-manager, which the writer VAP gates) and a tenant can write neither. But since this is the stated fail-closed boundary, worth making the check match the claim: decode with encoding/pem and assert every block is a CERTIFICATE that x509.ParseCertificate accepts, with no trailing remainder. The mirrored chart guard in _tls.tpl shares the limitation.
There was a problem hiding this comment.
The Go guard does decode: certificateChainPEM runs pem.Decode + x509.ParseCertificate on every block and rejects any trailing remainder, and 15b7442 goes further — the projection is rebuilt from the re-encoded parsed DER, so any byte the guard did not parse as a certificate (headerless DER, a JWK, preamble) is structurally unreachable rather than copied. The chart mirror in _tls.tpl is anchored to the whole value (bff1e3f), which is the most a Helm template can enforce without a parser.
| } | ||
| // Key-free and already canonical: the engine publishes its own trust | ||
| // anchor. Leave it alone. | ||
| return ctrl.Result{}, nil |
There was a problem hiding this comment.
Two small asymmetries on the key-free canonical branch, both misconfig-only (no shipped engine declares <release>-tenant-ca as its source). It returns ctrl.Result{} with no RequeueAfter, so on the unwatched name-driven leg a self-source that later gains key material is never re-examined, whereas every other path here requeues on resyncInterval; and unlike the shared write path it does not assert the single-ca.crt/Opaque contract. It stamps no tenant-ca/tenantresource label, so it grants no tenant visibility on its own — but return ctrl.Result{RequeueAfter: resyncInterval} plus a warning on extra keys would make this corner symmetric with the rest.
There was a problem hiding this comment.
Both asymmetries are closed. The key-free canonical branch returns ctrl.Result{RequeueAfter: resyncInterval}, so a declaration-driven self-source that later gains key material is re-examined on the unwatched name leg rather than served with a key forever. And it no longer skips the contract: canonicalContractDeviation asserts the Opaque / single-ca.crt shape and warns on extra keys. Covered by TestReconcile_SourceAtCanonicalName_KeyFree_Resyncs and _ContractDeviation.
| err := r.Reader.Get(ctx, types.NamespacedName{Namespace: ns, Name: name}, recorded) | ||
| switch { | ||
| case apierrors.IsNotFound(err): | ||
| // MERELY ABSENT. Hold the trust anchor and wait for the source to come |
There was a problem hiding this comment.
Correctness edge worth a look. If a declared source disappears and spec.caCert is then removed, resolveSource falls through to the empty label leg and returns nil, so pruneProjection runs — but because the recorded source is still absent it lands in this MERELY ABSENT branch and holds+requeues the projection indefinitely, even though removing the declaration is a definitive opt-out. The retired anchor stays tenant-readable until the source name reappears or the HelmRelease is deleted. The absent-vs-opt-out discriminator keys off the source Secret's existence, which cannot tell 'source rotating' apart from 'declaration withdrawn while the source happens to be gone'. Same-namespace and low-severity, but the withdrawal never fires.
There was a problem hiding this comment.
Addressed. pruneProjection now discriminates on the recorded source LEG, not the source's existence: a declared-leg projection (SourceModeAnnotation == modeDeclared) with no standing declaration is withdrawn without reading the source at all, so removing spec.caCert after the source has already vanished is a definitive opt-out and the retired anchor no longer lingers. Covered by TestReconcile_DeclarationRemovedAfterSourceVanished_WithdrawsProjection; the reissue case (source gone, declaration standing) still holds via TestReconcile_VanishedDeclaredSource_KeepsProjection.
| # platform controller's own ServiceAccount; no tenant RBAC is widened. | ||
| - apiGroups: [""] | ||
| resources: ["secrets"] | ||
| verbs: ["get", "list", "watch", "create", "update", "patch", "delete"] |
There was a problem hiding this comment.
This widens the controller ServiceAccount from cluster-wide Secret read (already covered by the '*'/'*': get,list,watch catch-all) to cluster-wide create/update/patch/delete. It is genuinely required — projections land in tenant namespaces and RBAC cannot scope a verb to a name suffix — but it enlarges the blast radius if this SA is compromised, now co-located with cozystack.io/*: *. Non-blocking; worth a threat-model line, or later splitting the projection writes onto a dedicated SA that holds only the Secret verbs.
There was a problem hiding this comment.
Agreed it is inherent and non-blocking: RBAC cannot scope a verb to a name suffix and projections land in arbitrary tenant namespaces, so cluster-wide create/update/patch/delete on Secrets is required. I am treating it as an accepted residual; splitting the projection writes onto a dedicated ServiceAccount that holds only the Secret verbs is a reasonable later hardening, out of scope for this change.
| failurePolicy is Fail: a Secret whose write cannot be evaluated is denied, | ||
| not admitted — fail-closed, as a trust-anchor control must be. | ||
| */}} | ||
| {{- $certManagerWriter := "system:serviceaccount:cozy-cert-manager:cert-manager" }} |
There was a problem hiding this comment.
Two non-blocking operational notes on this policy. The allowed writer is pinned as a literal, so a cert-manager repackage/rename must move it in lockstep; it fails closed (denies the new identity, killing the label leg) rather than open, and since postgres uses the declared leg the policy gates nothing live yet, so the drift would only surface when the first label-leg engine converges — deriving the identity from the cert-manager package values at render time would remove the seam. Separately, failurePolicy: Fail + [Deny] with no namespace/object selector means a Velero or kubectl restore of a labelled cert-manager Secret during DR is denied (the restorer is not cert-manager); cert-manager re-mints so it is recoverable, but it is worth a runbook note.
There was a problem hiding this comment.
Both are acknowledged as accepted operational residuals. The writer literal fails closed by design — a cert-manager rename denies the new identity rather than silently widening — and since the shipped engine is on the declared leg the policy gates nothing live yet, so the drift would only surface when the first label-leg engine converges. The DR case is recoverable because cert-manager re-mints the Secret. Deriving the identity from cert-manager package values at render time and a restore runbook note are fair follow-ups, out of scope here.
98df611
c5d95dc to
98df611
Compare
github-actions
Bot
added
the
area/uncategorized
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/controller/cacert/reconciler.go`:
- Around line 791-821: Update the projection reconciliation logic around
hasOwner to require exactly one OwnerReference matching owner, rather than
merely checking whether the desired owner exists. Treat any list with extra,
stale, or differing references as non-compliant and replace
existing.OwnerReferences with a single owner entry, while preserving the
early-return path only when labels, annotations, data, and ownership all match
exactly.
🪄 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: 4dc4b7c7-ff0d-4dbc-be35-e80204e60fe1
📒 Files selected for processing (15)
api/v1alpha1/applicationdefinitions_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/cozystack-controller/main.gointernal/controller/cacert/postgres_wiring_test.gointernal/controller/cacert/reconciler.gointernal/controller/cacert/reconciler_test.gopackages/library/cozy-lib/templates/_tls.tplpackages/system/application-definition-crd/definition/cozystack.io_applicationdefinitions.yamlpackages/system/cozystack-basics/templates/publish-ca-cert-writer-policy.yamlpackages/system/cozystack-basics/tests/publish-ca-cert-writer-policy_test.yamlpackages/system/cozystack-controller/templates/rbac.yamlpackages/system/cozystack-controller/tests/rbac_test.yamlpackages/system/postgres-rd/cozyrds/postgres.yamlpackages/tests/cozy-lib-tests/templates/tests/tls-cacert.yamlpackages/tests/cozy-lib-tests/tests/tls_cacert_test.yaml
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/system/application-definition-crd/definition/cozystack.io_applicationdefinitions.yaml
- packages/tests/cozy-lib-tests/templates/tests/tls-cacert.yaml
- internal/controller/cacert/postgres_wiring_test.go
- packages/system/cozystack-basics/tests/publish-ca-cert-writer-policy_test.yaml
- packages/library/cozy-lib/templates/_tls.tpl
- api/v1alpha1/zz_generated.deepcopy.go
- packages/system/cozystack-controller/tests/rbac_test.yaml
- api/v1alpha1/applicationdefinitions_types.go
- cmd/cozystack-controller/main.go
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM. The engine-agnostic cacert controller is fail-closed on every security-critical path: the declared source takes priority over the forgeable label leg, the write path copies exactly one whitelisted ca.crt and rejects any private-key/non-cert PEM, owner-ref GC is native, and Secret.type immutability is handled. The new VAP is a GA in-apiserver policy with no webhook pod (no cold-start trap) and rejects no existing Flux write (no chart writes the publish-ca-cert label). Additive CRD field with committed deepcopy/schema, no migration; cozyrds selects the key-free tenant-ca projection, so there is no TLS key leak.
Non-blocking follow-ups (already tracked): x509-parse the guard instead of header-match only; distinguish declaration-removed from source-temporarily-absent in pruneProjection; harden the cross-ns source lookup; the hardcoded cert-manager SA in the VAP is a documented fragile seam.
myasnikovdaniil
left a comment
myasnikovdaniil
left a comment
There was a problem hiding this comment.
LGTM — re-reviewed after the rebase onto current main. The core controller, _tls.tpl, the API types, the CRD, the postgres wiring, and the ValidatingAdmissionPolicy's rendered output are byte-identical to the previously-approved revision. The force-push added two targeted tests and two documentation clarifications, on top of the environment change from the wildcard-secret controller landing on main.
What I re-verified
- The only behavioral change to this controller's environment is that the manager's shared Secret informer is now scoped (by the wildcard-secret controller) where it was previously unscoped. That makes the invariant "every Secret read goes through the uncached
Readeror this controller's private, scopedcaSourceClustercache — never the shared cached Client" load-bearing rather than a review-only guarantee. Audited every Secret access: the five Secret reads (reconciler.go:613viaCache;:677,:719,:869,:887viaReader) all bypass the shared cache; the remaining cachedGet/Listcalls (:424,:692,:1243,:1272) are HelmRelease / ApplicationDefinition, not Secrets; writes (:736,:830,:907) bypass the cache. The scoping does not regress this controller. - The two new tests (
TestReconcile_NameDrivenSource_ReadThroughUncachedReader,TestReconcile_UpsertExistingCheck_ReadThroughUncachedReader) lock exactly that invariant; both pass.go test ./internal/controller/cacert/...andgo vetare green, andhelm unittestpasses for the cozystack-basics VAP and the cozystack-controller RBAC. - The doc-only changes are accurate: the VAP comment explaining why DELETE is intentionally ungated (integrity-only policy; a DELETE-matching VAP would also gate the garbage-collector and namespace-controller and wedge namespace teardown), and the RBAC comment now attributing the shared Secret write grant to both reconcilers (verbs unchanged).
- The two red CI checks are unrelated to this PR:
Build packages/apps/kubernetesfails on an unresolvable upstreamcentos:stream9image digest (that package is untouched here), andAnalyze (go)was canceled mid-run after its Go build step passed.
Non-blocking follow-ups
- Single-owner enforcement on the projection (
reconciler.go:795,:813) — see inline comment. - Carried over from the earlier review, still applicable (these files are unchanged):
- decode and
x509.ParseCertificatethe write-path guard instead of validating by PEM-header substring (reconciler.go:965, mirrored in_tls.tpl); - the declaration-withdrawn-while-source-absent prune corner leaves a retired anchor tenant-readable until the source reappears or the release is deleted (
:890); - the key-free canonical branch returns no
RequeueAfterand skips the single-ca.crt/Opaque assertion (:576); selectorsDigestretains only the low 64 bits of the SHA-256 (:952);applicationDefinitionreturns the first kind match without flagging duplicate kinds (:696);- the cluster-wide Secret write grant enlarges the controller ServiceAccount's blast radius (
rbac.yaml); - the VAP's cert-manager writer is pinned as a literal, and a DR restore of a labelled source is denied (
publish-ca-cert-writer-policy.yaml).
- decode and
None of these block merge.
| } | ||
| existing.Annotations[SourceRefAnnotation] = ref | ||
| existing.Annotations[SelectorsDigestAnnotation] = digest | ||
| if !hasOwner(existing.OwnerReferences, owner) { |
There was a problem hiding this comment.
Non-blocking hardening. The up-to-date short-circuit (:795) and this normalization both check only that the desired HelmRelease owner is present (hasOwner), not that it is the sole owner. A namespace actor able to update the projection could append a second, non-controller owner reference pointing at a long-lived object; it survives reconciliation, and once the HelmRelease is deleted the garbage collector keeps the Secret because that other owner still exists — the retired trust anchor lingers indefinitely.
Impact is bounded: the projection is key-free, same-namespace, and self-inflicted — the same class as the known limitation already noted in the PR description — so this is not a blocker. Cheap to close by gating both sites on len(existing.OwnerReferences) == 1 && hasOwner(...); worth a line in the known-limitations note either way.
There was a problem hiding this comment.
Both sites now gate on ownedSolelyBy (len(refs) == 1 && hasOwner), not bare hasOwner: a second appended owner reference fails that check, so the projection is re-homed back to exactly one controller ref and garbage collection cannot be kept alive by a squatted second owner. Covered by TestReconcile_ExtraOwnerReferenceIsStripped and TestOwnedSolelyBy.
IvanHunters
left a comment
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM. Reviewed with the cozy-review methodology (build + Go tests + helm unit tests for all three affected charts pass; load-bearing claims verified against sources).
No blocking defects. The change is additive: a new reconciler in the existing cozystack-controller binary creates a new Opaque <release>-tenant-ca Secret from the existing CNPG <release>-ca — nothing existing is mutated or pruned, so there is no Secret.type immutability trap and no resource-policy: keep needed. The secrets RBAC grant is pre-existing (WildcardSecretReconciler), not new. Fresh install adds no cold-start dependencies: the controller creates no Certificate, so no dependsOn on cert-manager is required, and the ValidatingAdmissionPolicy is inert on a cold cluster. make generate artifacts (CRD + deepcopy) are present.
Verified: the second cluster.Cluster is registered via mgr.Add and lands in the Caches runnable group gated by WaitForCacheSync (controller-runtime runnable_group.go), so the controller starts only after its cache syncs.
One non-blocking follow-up: the ValidatingAdmissionPolicy in publish-ca-cert-writer-policy.yaml hardcodes the allowed writer as system:serviceaccount:cozy-cert-manager:cert-manager, duplicating an identity whose source of truth is the cert-manager package (the author flagged this as a "FRAGILE SEAM"). Harmless today while the label-leg is dormant, but before the first label-leg engine converges it would be worth deriving the cert-manager namespace/SA from a shared value so a cert-manager repackaging can't silently break the seam in either direction.
377cd66
98df611 to
377cd66
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@internal/controller/cacert/reconciler.go`:
- Around line 1578-1582: The hasOwner comparison omits BlockOwnerDeletion,
allowing blocking owner references to be treated as matches. Update hasOwner in
internal/controller/cacert/reconciler.go at lines 1578-1582 to compare
BlockOwnerDeletion alongside the existing OwnerReference fields, and add a test
case in internal/controller/cacert/reconciler_test.go at lines 1700-1723
verifying differing BlockOwnerDeletion values are not considered a match.
In `@packages/library/cozy-lib/templates/_tls.tpl`:
- Around line 185-198: Add an admission-policy gate for the
internal.cozystack.io/tenant-ca label so only the authorized publish-ca-cert
path may set it to true. Update the policy governing namespace Secret writers,
preserving the helper-rendered labels in the labels merge while preventing other
Secret writers from surfacing objects through the tenant read path.
🪄 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: 16628456-a3f3-4782-a7e5-569fa99278f3
📒 Files selected for processing (23)
.gitignoreapi/v1alpha1/applicationdefinitions_types.goapi/v1alpha1/zz_generated.deepcopy.gocmd/cozystack-controller/main.gohack/check-publish-ca-cert-writer-pin.batshack/e2e-chainsaw/postgres/chainsaw-test.yamlinternal/controller/cacert/postgres_wiring_test.gointernal/controller/cacert/reconciler.gointernal/controller/cacert/reconciler_test.gopackages/apps/postgres/README.mdpackages/apps/postgres/templates/dashboard-resourcemap.yamlpackages/apps/postgres/templates/db.yamlpackages/apps/postgres/tests/tenant_ca_rbac_test.yamlpackages/library/cozy-lib/templates/_tls.tplpackages/system/application-definition-crd/definition/cozystack.io_applicationdefinitions.yamlpackages/system/cozystack-basics/templates/publish-ca-cert-writer-policy.yamlpackages/system/cozystack-basics/tests/publish-ca-cert-writer-policy_test.yamlpackages/system/cozystack-controller/templates/rbac.yamlpackages/system/cozystack-controller/tests/rbac_test.yamlpackages/system/postgres-rd/cozyrds/postgres.yamlpackages/tests/cozy-lib-tests/templates/tests/tls-cacert.yamlpackages/tests/cozy-lib-tests/tests/tls_cacert_test.yamlpackages/tests/cozy-lib-tests/tests/tls_cacert_values.yaml
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/tests/cozy-lib-tests/templates/tests/tls-cacert.yaml
- packages/system/cozystack-controller/templates/rbac.yaml
- packages/system/application-definition-crd/definition/cozystack.io_applicationdefinitions.yaml
- packages/system/cozystack-basics/tests/publish-ca-cert-writer-policy_test.yaml
- packages/system/cozystack-controller/tests/rbac_test.yaml
- api/v1alpha1/applicationdefinitions_types.go
- packages/system/postgres-rd/cozyrds/postgres.yaml
- cmd/cozystack-controller/main.go
- internal/controller/cacert/postgres_wiring_test.go
The only watches on the projection Secret were the source-Secret index (keyed on
spec.projections.sourceSecretName) and the sentinel itself. The projection is
named <release>.tenant-ca, which is no sentinel's source, so an in-place rewrite
of its ca.crt was corrected only by the five-minute resync — a window in which a
forged trust anchor is served.
Map the projection back to its owning sentinel through the controller owner
reference it carries, on the same dedicated metadata cache the source watch uses.
Owns(&corev1.Secret{}) would register against the manager cache, whose Secret
informer is label-scoped to the WildcardSecret replicas, so a projection carrying
no wildcard label would never be delivered — it would compile and pass a fake
client yet never fire in a real cluster.
Signed-off-by: Aleksei Sviridkin <f@lex.la>
…ases Two gaps in the sentinel-to-canonical-name lifecycle: Withdrawal — the reconciler published each declared entry but never deleted one. A sentinel that keeps existing with its CACert entry removed (a chart gating it on tls.enabled, say) left the old <release>.tenant-ca served forever, because owner-reference GC only fires when the whole sentinel is pruned. A sentinel that reconciles with zero CACert entries now deletes the projection at the canonical name, but only when it still carries that sentinel's owner reference, so a foreign Secret or another sentinel's projection is never touched. Contention — two sentinels in one namespace with the same release label both resolve to the single canonical name. Whichever reconciled first would publish an arbitrary CA and the two could flap ownership of one projection. Refuse both, mirroring the more-than-one-CACert-entry refusal, so nothing is published until the declaration lives on exactly one sentinel. The ClusterRole comments claimed the reconciler never deletes a Secret; corrected to describe the owner-scoped withdrawal delete. Signed-off-by: Aleksei Sviridkin <f@lex.la>
The projection carries two annotations whose keys still named a publish-ca-cert discovery leg that no longer exists in the tree. Rename them to internal.cozystack.io/ca-cert-source and internal.cozystack.io/ca-cert-selectors while the API is unshipped; both stay functional (the selectors digest drives the re-admission drift check, the source ref is a drift input and a traceability record). Resolving the ApplicationDefinition matched on Kind alone from a List of unspecified order. Gate the match on the release's group label too, so a release stamped with an unexpected group cannot resolve a same-kind definition by accident. Signed-off-by: Aleksei Sviridkin <f@lex.la>
select-e2e.sh mapped reverse dependencies to Chainsaw suites only through *-application sources and a hardcoded external-dns, so a change confined to the tenant-projection writer policy (shipped by cozystack-basics) selected every suite that depends on it but not cacert — whose Chainsaw is the policy's only live deny/allow proof. Add the cozystack-basics reverse mapping, mirroring the external-dns precedent, and pin both directions: a cozystack-basics change selects cacert, and a cacert run installs cozystack-basics. Signed-off-by: Aleksei Sviridkin <f@lex.la>
The dependsOn comment warned that a capability gate renders the policy out at first install and never re-adds it, which read as contradicting the .Capabilities.APIVersions.Has guard the policy templates now carry. They do not conflict: the warning is about gating on the matched types' CRDs, which may not exist yet at first install, while the template guard names the ValidatingAdmission Policy API itself — GA on every supported cluster, so always present. Spell out that a maybe-absent matched-type CRD needs dependsOn ordering while an always-present core API can be gated in the template. Signed-off-by: Aleksei Sviridkin <f@lex.la>
Both suites compared the projection's ca.crt against the source PEM byte for byte. The controller re-encodes ca.crt from the DER it parsed, so the two match today only because openssl and the re-encoder both emit header-free 64-column PEM; a header line or column-width change upstream would redden a correct projection. Compare the parsed certificates via openssl x509 -noout -fingerprint instead, which is identity-stable across any re-encoding. Signed-off-by: Aleksei Sviridkin <f@lex.la>
The two-sentinel refusal returned without a RequeueAfter. NoRelease and MultipleCACert can do that because their fix edits the same sentinel, which the For() watch delivers; a contest is cleared by deleting the SIBLING sentinel, and nothing enqueues this one when a sibling changes. So after an operator removed the duplicate, the surviving sentinel withheld its anchor until the informer's global resync — hours. Return RequeueAfter: resyncInterval on the contest branch so recovery rides the resync backstop, and clarify that an anchor an earlier uncontested reconcile already published is left in place rather than withdrawn. Signed-off-by: Aleksei Sviridkin <f@lex.la>
…draw note The selectors digest kept only the first 8 bytes of the SHA-256. A collision skips the re-admission the doc calls load-bearing — the revocation direction, where a tenant would silently keep read access after a definition stopped selecting the anchor — and the annotation has no length limit, so store the full hash. Also correct the withdraw-branch comment: MinItems=1 prevents a single-entry sentinel from emptying its list, so the branch is reached when a future non-CACert entry replaces the CACert one, not by gating a lone entry off. Signed-off-by: Aleksei Sviridkin <f@lex.la>
withdrawProjection recognised its projection by an exact owner-reference match while adoption uses a name match, so the two disagreed about what counts as "ours" — and withdrawal was the stricter, which is backwards: "safe to overwrite" is a stronger claim than "safe to delete". Two projections a name match adopts and rewrites were read as foreign by the exact match and left in place: one carrying a previous incarnation's UID (delete-and-recreate under the same name), which owner-reference garbage collection never reaps once the CACert entry is gone, so the retired anchor stayed tenant-readable forever; and one whose BlockOwnerDeletion flag had drifted, the very drift the adoption path normalizes. Decide ownership with isOurProjection so both paths answer the question the same way. Signed-off-by: Aleksei Sviridkin <f@lex.la>
An entry with an empty sourceSecretName sent the reconciler to Get a Secret with an empty name. The apiserver answers that with "resource name may not be empty" — not NotFound — so it escaped the not-found branch, was returned from Reconcile, and wedged the sentinel on exponential backoff. The error path returns before any status write, so no Ready condition was ever recorded: the silent retry the sentinel was introduced to make visible. Pin sourceSecretName to MinLength=1 so a live apiserver rejects it at admission, and guard the reconcile so an object admitted by an older CRD resolves to Ready=False/SourceInvalid instead of an error. Signed-off-by: Aleksei Sviridkin <f@lex.la>
The package doc claimed there is no withdrawal logic and that a retired declaration is only ever a deleted sentinel. withdrawProjection has since become the second mechanism: it deletes a projection when the sentinel outlives its CACert entry, which owner-reference garbage collection never reaps because the owner still exists. State both paths. Signed-off-by: Aleksei Sviridkin <f@lex.la>
The ApplicationDefinition mapping fans out to every TenantProjection in the cluster on every definition event, including Flux's periodic no-op re-apply of the many *-rd definitions. The selectors-digest drift check already suppresses the resulting writes, but not the load: each enqueued reconcile still issues several uncached reads. Gate the watch with a predicate that delivers an update only when spec.secrets — the field the selectors digest is computed from — changed, while letting creates and deletes through. Comparing the same field the digest digests keeps the gate and the digest from drifting apart. Signed-off-by: Aleksei Sviridkin <f@lex.la>
A key-free Secret already sitting at the canonical trust-anchor name is left untouched and reported Ready. But tenant visibility comes from the internal.cozystack.io/tenant-ca label, and an engine-owned Secret at that name carries none, so the lineage webhook marks it tenantresource=false and the tenant is locked out while the controller reports "published". Add the missing label to the canonical-name contract deviations so the warning names it, and collect all deviations rather than only the first so a single warning describes every way the object falls short. Signed-off-by: Aleksei Sviridkin <f@lex.la>
Go 1.26's extended new() builtin takes an expression, so new(true) and new(false) replace ptr.To for the owner reference's Controller and BlockOwnerDeletion flags. ptr.Deref, which has no builtin equivalent, stays. Signed-off-by: Aleksei Sviridkin <f@lex.la>
703b67c to
156ded1
Compare
|
All six addressed on the rebase. Withdrawal now uses the same name-match gate as adoption ( Rebased onto main. Two later commits (kubevirt-csi base image) are orthogonal and touch none of these files, so I left them for the merge rebase. New head 156ded1. |
|
This is not another round of findings. The six from the last round are all fixed, each with the test that pins it, and two of them you found in your own work rather than in the review. I have nothing further on correctness. What I want to raise instead is scope, and I want to be upfront that acting on it costs you a rebase and changes no behaviour whatsoever. It is worth doing anyway, and the reason is that this branch has grown by review: each round added a guard, a test and a comment, and none of the rounds removed anything, so the surface has been ratcheting up even while the sentinel rework was deleting two thousand lines from the middle of it. The clearest symptom is that the current round asks to delete The measurementAgainst the merge base the branch is 43 files, +5133/-74. Three of the touched files have no functional change at all — - resources: ["tenantprojections"]
verbs: ["get", "list", "watch"]
- resources: ["tenantprojections/status"]
verbs: ["get", "update", "patch"]The remaining 63 are an essay about the blast radius of a cluster-wide Secret write grant that already existed for the WildcardSecret reconciler and is unchanged by this branch. Proposed splitPR A — the controller and its CRD. Everything below keeps its current diff unless the note says otherwise.
Reverted to the merge base and not carried anywhere:
PR B — cozy-lib trust-anchor helper. PR C — VAP capability gate. The PR D — the sentinel writer policy, about 209 lines plus the two policy steps in PR E — the dot-free PR F — the On the writer policyWorth a deliberate decision rather than inheritance. Its own comment makes the case against itself: base tenant roles grant no verb on Dropping it also simplifies two things rather than relocating them. The If you want the defence in depth, it is a clean standalone PR. I would not block on either answer. MechanicsDo not rebase the existing commits — they interleave fixes across areas and replaying them by hand is where mistakes happen. Take the final tree and keep only the in-scope paths: git checkout -b feat/ca-extraction-controller-v2 741a5acc4
git checkout <current-head> -- \
api/internalapi internal/controller/cacert cmd/cozystack-controller \
packages/system/cozystack-controller \
packages/apps/postgres/templates/tenant-projection.yaml \
packages/apps/postgres/tests/tenant_projection_test.yaml \
packages/apps/postgres/README.md \
packages/system/postgres-rd/cozyrds/postgres.yaml \
hack/e2e-chainsaw/postgres hack/update-codegen.shThat subset is 16 files and 3876 insertions as it stands today. The other PRs come off the same base by cherry-picking only their own paths; none of them depends on another, so they can land in any order. If the Comment trimmingThe rule I would apply is whether the comment explains something the next person cannot derive from the code in front of them. Keep: function contracts; why the write path rebuilds the payload from parsed DER instead of copying validated input; why the Secret watch needs its own cache rather than Move to the design proposal: the naming history, the rejected alternatives, the records of what previous iterations got wrong, and every paragraph arguing against a design that is no longer in the tree. The 132-line package header is the single biggest win and almost all of it is archaeology. The naming invariant in particular is smaller than its current footprint suggests. It plays no part in tenant selection — delivery is entirely by the // projectionSuffix completes the canonical name "<release>.tenant-ca".
//
// The DOT is load-bearing. Engine CA Secrets are named <prefix><app><suffix>,
// and app names are validated as DNS-1035 labels, which cannot contain a dot
// (pkg/apis/apps/validation); Secret names are DNS-1123 subdomains, which can.
// So no engine can generate this name for any application, and the projection
// cannot collide with an operator-owned object — in either direction, and both
// are unrecoverable. Do not "tidy" the dot into a dash.
projectionSuffix = ".tenant-ca"The same argument is currently restated in What is actually blockingOnly PR A needs to land for the feature to work, and it needs no further correctness work from me — the split is a repackaging of code that is already right. B through F are yours to land whenever suits you, and I will not hold the feature on any of them. |
…nant Secret (#3407) ## What this PR does Adds an engine-agnostic controller that publishes a key-free `<release>.tenant-ca` Secret (only `ca.crt`) for a managed application, so a tenant can verify the application's TLS without ever seeing a private key. The source is declared, not guessed: a chart renders a namespaced `TenantProjection` sentinel (group `internal.cozystack.io`) naming the CA Secret to lift; the controller watches the sentinel, extracts `ca.crt`, and writes the projection owner-referenced to the sentinel, so garbage collection is native. The projection carries the `internal.cozystack.io/tenant-ca` label the lineage webhook turns into tenant visibility. Postgres is the first consumer. The trust boundary is RBAC: no tenant role grants any verb on `internal.cozystack.io`, so a tenant cannot forge a sentinel. A companion `ValidatingAdmissionPolicy` that pins the writer to helm-controller ships separately as defence in depth. This is the controller-and-CRD slice, split out from #3299 for focused review. It is the only piece required for the feature to work; the render-time helper, the writer policy, the capability-gate, and the release-prefix validation land as their own PRs. ### Downstream repositories Walked the trigger map against the diff. This adds an internal CRD and controller and a sentinel to the Postgres chart; no app is added or renamed and no `values.schema.json` changes, so the typed provider and Ansible are unaffected. The one to confirm is the Postgres README edit — if the website mirrors it, it needs a refresh. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: ### Release note ```release-note feat(cozystack-controller): publish a key-free per-application CA trust anchor (`<release>.tenant-ca`) that tenants read to verify managed-application TLS, declared through a new `TenantProjection` CRD. Postgres is the first consumer. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added tenant-facing CA certificate projections for PostgreSQL deployments. * Tenant secrets now provide only the CA certificate, excluding private keys. * Added support for configuring and monitoring tenant CA projections. * **Documentation** * Updated PostgreSQL TLS instructions to retrieve certificates through tenant secrets. * **Bug Fixes** * Improved protection against invalid certificates, secret collisions, and accidental exposure of private key material. * Added validation to ensure projected certificates remain synchronized with their sources. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
) ## What this PR does Constrains `ApplicationDefinition.spec.application.release.prefix` to `^[a-z0-9-]*$`, which excludes the dot. Release names are `<prefix><app>`, and the tenant CA trust anchor is projected to `<release>.tenant-ca`; the dot is the separator that keeps that name unreachable by any engine, so a prefix carrying a dot would break the guarantee. Every in-tree prefix already conforms. Split out from #3299. ```release-note feat(api)!: `ApplicationDefinition.spec.application.release.prefix` must match `^[a-z0-9-]*$` (dot-free). A definition with a prefix containing a dot or any other character is rejected at admission; correct it before applying this CRD. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Added validation for release prefixes to allow only lowercase letters, numbers, and hyphens. * Updated guidance explaining how prefixes affect generated release names and tenant CA trust anchors. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…3409) ## What this PR does Wraps the route, gateway, and ingress hostname `ValidatingAdmissionPolicy` templates in a `.Capabilities.APIVersions.Has` gate so they render only where the VAP API is available. Without it, an operator-generated HelmRelease with drift detection off renders the policies out at first install on a cluster missing the API and never adds them back. Each template gains a sibling test asserting it renders zero documents when the API is absent. Install-robustness fix, split out from #3299. ```release-note fix(cozystack-basics): hostname ValidatingAdmissionPolicies render only where the VAP API is available, so a first install on a cluster without it no longer drops them permanently. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Hostname validation policies now render only when the Kubernetes admission policy API is available, preventing unsupported policy resources on older clusters. * **Tests** * Added/updated unit tests to explicitly simulate both missing and present admission policy API capabilities, ensuring templates render (or not) as expected. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…ame (#3408) ## What this PR does Ships `cozy-lib.tls.caCertSecret`, a render-time helper that emits a key-free trust-anchor Secret carrying only `ca.crt`. It converges the object name on `<release>.tenant-ca`, requires the whole value to be certificate blocks so a stray private-key header or trailing bytes are rejected, and coerces numeric scalars before the guard runs so a numeric value cannot slip past. No chart calls it yet. It is the render-time producer half of the trust-anchor contract; the controller half is #3407, now merged. The doc-comment on the helper shows the one-line call a chart uses, and says outright there is no caller yet, so nobody reads it as wired when it is not. Split out from #3299. Comments trimmed on review. ```release-note fix(cozy-lib): the CA trust-anchor helper emits the canonical `<release>.tenant-ca` name and rejects non-certificate input. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated generated TLS CA Secret naming to use the `<release>.tenant-ca` convention. - Added/ensured a tenant CA label is set alongside the existing tenant resource label. - Continued support for custom labels (caller labels still applied). - **Bug Fixes** - Strengthened CA certificate PEM validation to be fail-closed: requires non-empty, complete `BEGIN/END CERTIFICATE` blocks only. - Rejects private-key material and other malformed or improperly structured PEM input, including unexpected extra content. - **Tests** - Updated TLS CA certificate fixtures and assertions to cover additional negative cases and refined error expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
4 participants
What this PR does
A tenant that connects to a managed TLS endpoint needs the CA certificate (
ca.crt) to verify the server, and nothing more. Today the platform delivers it for no engine except kafka, and the in-flight per-app TLS work would each hand the tenant a Secret that also carries a private key — the cert-manager CA Secret holdstls.key, CloudNativePG's<release>-caholdsca.key. Granting read on one of those to deliver the trust anchor hands over key material too.This adds a controller that publishes, for every engine, one canonical key-free object per release: an Opaque Secret named
<release>.tenant-caholding onlyca.crt.The dot in that name is load-bearing, and it is the third name this object has had.
<release>-ca-certwas rejected because Percona Server for MongoDB creates a Secret of exactly that name and puts a private key in it. Its replacement<release>-tenant-cawas rejected too, for a subtler reason: it collides across releases. For an applicationfoothe projection would bepostgres-foo-tenant-ca, and for a sibling applicationfoo-tenantCloudNativePG's own CA ispostgres-foo-tenant+-ca— the same object. One direction of that collision cannot be guarded: if the projection is written first, CNPG rejects the key-free Secret (missing ca.key secret data) and the sibling's PKI never completes, so that application never starts.<release>.tenant-cais disjoint by character class instead of by survey — application names are DNS-1035 labels and structurally cannot contain a dot, release prefixes are dot-free, and Secret names are DNS-1123 subdomains where a dot is legal, so no prefix + application + operator suffix can ever produce a dotted name, for any operator, now or later.Source discovery has two legs behind one write path, and the declared leg wins. When an ApplicationDefinition declares
spec.caCert, that named source is authoritative and a labelled Secret is not consulted for the engine; the label leg serves only engines with no declaration. This ordering is a security boundary: the declaration lives on a platform object no tenant can write, while a publish label sits on a Secret a namespace writer could create — so a declared engine's trust anchor cannot be swapped for a forged one.The name-driven (declared) leg exists because CloudNativePG and Percona Server for MongoDB create their CA Secret themselves and cannot be made to label it. The label-driven leg serves the cert-manager-minting charts, which opt in through
Certificate.spec.secretTemplate.labels; on it a required labelinternal.cozystack.io/publish-ca-cert-releasenames the release, because a cert-manager-issued Secret carries no OwnerReference (the platform shipsenableCertificateOwnerRef: false) and no Helm metadata, so nothing else can attribute it to a release.The write path is the single place bytes reach a projection, on both legs. It copies exactly one whitelisted key under the canonical
ca.crt, and it parses rather than pattern-matches: every PEM block must decode and be accepted byx509.ParseCertificate, with no trailing remainder and no private-key block anywhere. A header check is not sufficient — PEM armour around arbitrary bytes satisfies it, andpem.Decodedoes not close the gap either, because it validates the armour and the base64 rather than the contents.The projection is owner-referenced to the application's HelmRelease with
BlockOwnerDeletion: false(theapps.cozystack.iokinds are virtual, so there is no application CR in etcd to reference), and the controller requires sole ownership so a second owner reference cannot keep a retired anchor alive after the application is gone. A Secret at the canonical name the controller did not create is never overwritten; the collision surfaces as a Warning Event.Withdrawal distinguishes a removed declaration from a source that is merely absent: deleting a CA Secret is how a cert-manager reissue is forced, so absence holds the last good anchor and waits, while a declaration that is gone withdraws the anchor definitively. The projection records which leg produced it, because that fact cannot be re-derived once the declaration is gone.
The label leg is guarded at admission. A
ValidatingAdmissionPolicyrestricts writes to any Secret carryinginternal.cozystack.io/publish-ca-certto cert-manager's controller ServiceAccount, on both CREATE and UPDATE (checking the old object too, so relabel-away is also caught), and a bats check renders the shipped cert-manager package and asserts the pinned identity still matches, so a repackage cannot silently break the seam.postgres is wired as the first consumer, validating the mechanism against the hardest input — an operator-created, asynchronous, key-bearing CA Secret on the name-driven leg. The anchor reaches tenants through
core.cozystack.io/tenantsecrets, gated on the lineage webhook's verdict rather than a name match, and the chainsaw test asserts the tenant-facing path end to end, including the negative that CNPG's key-bearing CA must not resolve. The remaining engines are tracked in #2814.Known limitation: a tenant who strips the
internal.cozystack.io/ca-cert-copymarker from their own projection makes the controller treat it as foreign and stop updating that one anchor — self-inflicted, same-namespace, no key leak, a direct consequence of never adopting a Secret it cannot prove it owns.Release note
Closes #3286
Summary by CodeRabbit
spec.application.caCertto source the CA trust anchor fromsourceSecretNameand optionalsourceKey(defaultca.crt).<release>.tenant-ca(contains onlyca.crt).spec.release.prefixvalidation pattern for consistent dot-free naming.<release>.tenant-ca.