feat(platform, ingress): propagate operator wildcard certificate to per-tenant termination points - #2990

Actionable comments posted: 2

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/wildcardsecret/reconciler_test.go`:
- Around line 476-489: The test TestReconcile_NoPlatformValuesIsNoOp currently
locks in destructive prune-on-missing-config behavior by asserting that an
absent values channel causes the wildcard-tls secret to not exist. This
conflicts with the intended design where secrets should only be pruned when
wildcard-secret-name is explicitly cleared, not when the values channel is
transiently absent. Modify the test to create an existing wildcard-tls secret in
the setup, then verify that the reconciler preserves it unchanged when the
values channel is missing, rather than asserting the secret should not exist.

In `@packages/extra/gateway/README.md`:
- Line 60: The documentation describes replica garbage collection as occurring
when "the source is removed," but the actual controller behavior only performs
garbage collection on explicit disable (clearing `wildcardSecretName`). Replicas
persist even when the source Secret is absent or mistyped. Update the wording in
the paragraph to clarify that replicas are garbage-collected only when
`wildcardSecretName` is explicitly cleared or disabled, not during transient
gaps or when the source Secret is temporarily unavailable, to accurately reflect
the controller's reconciliation logic and prevent operator confusion during
certificate rotation scenarios.

Code Review

This pull request introduces a new WildcardSecret controller that replicates the operator-provided wildcard TLS Secret into tenant namespaces that terminate TLS. This allows per-tenant ingress controllers and Gateways to serve the wildcard certificate from their own namespaces without requiring cross-namespace Secret access or widening tenant RBAC. The changes include the reconciler implementation, comprehensive unit tests, RBAC updates, and adjustments to the ingress and gateway charts to utilize the replicated secrets. Feedback is provided to remove the unused Scheme field from the Reconciler struct to simplify the code and clean up unused imports.

Actionable comments posted: 2

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/wildcardsecret/reconciler.go`:
- Line 168: The pruneCopies function is being called without protecting the
configured source Secret from being pruned. Modify the pruneCopies calls (at
line 168 and line 213) to pass the configured source namespace as a parameter so
that it can be excluded from the pruning logic. Additionally, update the
pruneCopies function implementation (around lines 342-357) to accept the source
namespace parameter and add it to the keepSet to ensure the source Secret is
never deleted even if it carries the wildcard-secret-copy label.
- Around line 200-216: The pruning operation via r.pruneCopies is being executed
unconditionally even when transient upsert failures have been collected in the
errs slice, which can cause previously managed Secrets to be deleted before
replacement copies are successfully created. Add a check to only call
r.pruneCopies when the errs slice is empty, ensuring that pruning is deferred
until all retryable upserts have succeeded.

---

Nitpick comments:
In `@internal/controller/wildcardsecret/reconciler.go`:
- Around line 142-146: The documentation comments for the Reconcile method at
lines 142-146 and the similar comments at lines 338-341 are stale and
incorrectly describe the behavior. The comments state that replicas are removed
when the feature is off, the source is absent, or the source is not a TLS
Secret, but the actual implementation preserves replicas unless the wildcard
name is explicitly cleared. Update both comment blocks to accurately reflect
that replicas are only removed when the wildcard name is explicitly cleared, not
when the source is merely absent or not a TLS Secret.

Actionable comments posted: 2

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/wildcardsecret/reconciler_test.go`:
- Around line 611-612: The disabled-path assertion that checks `res.RequeueAfter
!= 0` is incomplete and could miss a bug where Requeue is set to true. In
addition to the existing RequeueAfter check, add a second assertion to verify
that `res.Requeue` is false, ensuring that a disabled reconcile neither requeues
immediately nor schedules a requeue after a delay.

In `@internal/controller/wildcardsecret/reconciler.go`:
- Around line 159-163: The platformValues struct uses WildcardSecretName as a
string type that defaults to empty string when the key is missing from the YAML,
causing readConfig to incorrectly return present=true and triggering destructive
pruning behavior. Change WildcardSecretName from a string type to a pointer to
string (*string) so that nil represents a missing key while an explicit empty
string represents a disabled state. Apply this same fix to the similar field at
lines 288-298 in the ExposeIngress or related configuration. Update any code
that reads or uses these fields to check for nil before dereferencing to
properly handle the distinction between missing and explicitly empty values.

APPROVE — thorough static review (Go build/vet + 18 reconciler unit tests + both helm-unittest suites + pre-commit all pass locally; not deployed: a storage roll is in progress on the dev cluster and this would require an operator redeploy).

This is the right trust model and it's implemented and documented carefully. Highlights I verified:

Security model (the make-or-break). The private-key replica lands only in namespaces that genuinely own a termination point — ownsTerminationPoint keys on the owner label equalling the namespace's own name, and that label is platform-written by packages/apps/tenant/templates/namespace.yaml (set to own-name only when the tenant runs its own ingress/Gateway), not tenant-editable. So the target set isn't spoofable in the normal trust model, inheriting namespaces are correctly excluded, and the publishing namespace is excluded so the source is never self-overwritten. The blast radius (a terminating tenant holding the wildcard key can impersonate any subdomain under the apex) is the same exposure as the per-host ACME Secret it replaces, and it's spelled out plainly in both values.yaml and the gateway README. Default-off (wildcardSecretName: ""), so existing installs are unchanged.

Cache safety. Scoping the manager-wide Secret informer to replicas + the values channel is safe here because none of the five sibling controllers (WorkloadMonitor, Workload, ApplicationDefinition×2, TenantGateway) nor the telemetry collector read Secrets via the cached client, and the other Secret-reading controllers (fluxplunger, backupcontroller, cozyvaluesreplicator) live in separate binaries. TestSecretCacheByObject_* pins the scope against future widening.

Correctness. Non-destructive on transient source/channel absence, prune-everything only on explicit disable, foreign-Secret collisions never clobber and never block siblings, rotation via bounded resync (source name is dynamic, so it can't be watched), replica self-heal. The test matrix is excellent.

Composition with #2988/#2989. Under wildcard mode the gateway renders certMode: existingSecret and mints no cert-manager Certificate, so there's no competing same-named Secret and no double-management.

make generate. The +kubebuilder:rbac markers are documentation only in this repo (no controller-gen role generation); RBAC is hand-maintained in the chart and correctly updated. No drift.

The only red CI check is the known cilium "IP already in use" E2E flake, unrelated to this change — the cozystack-controller HelmRelease upgraded successfully in that same run.

One optional, non-blocking suggestion left inline about surfacing the foreign-collision case as an Event.

Code Review

This pull request introduces a new wildcardsecret controller that replicates the operator-provided wildcard TLS Secret into tenant namespaces that terminate TLS, allowing per-tenant ingress controllers and Gateways to serve it locally without cross-namespace reads. The implementation includes the reconciler logic, extensive unit tests, Helm chart updates, and RBAC adjustments. The review feedback suggests optimizing the reconciler by utilizing the cached client (r.Get and r.List) instead of the uncached API reader (r.Reader) for resources that are already cached, such as the platform values Secret, Namespaces, and copy-labeled Secrets, thereby reducing unnecessary API server load.

Code Review

This pull request introduces a new wildcardsecret controller that replicates the operator-provided wildcard TLS Secret into tenant namespaces terminating TLS, ensuring secure local access without cross-namespace reads. The changes include the reconciler implementation, extensive unit tests, RBAC updates, and integration with ingress and gateway packages. The review feedback recommends a robustness improvement to validate that the source Secret contains non-empty tls.crt and tls.key data before replication to prevent propagating malformed secrets and causing cluster-wide TLS outages.

Approve.

The wildcard-secret replication controller is mechanically sound and the tenant-isolation invariant holds. ownsTerminationPoint selects a namespace only when the operator-controlled namespace.cozystack.io/{ingress,gateway} label equals the namespace's own name; that label value derives from .Release.Namespace via Flux, and tenants set only boolean flags, so a tenant cannot redirect replication. Prune only touches Secrets carrying the copy label, and the informer cache is scoped to copy-labeled replicas plus the values Secret. go test passes (24 cases incl. foreign-collision no-overwrite, prune-on-demotion, cache-scope pin) and helm unittest passes.

Non-blocking:

• RBAC: the ClusterRole gains cluster-wide secrets write. With dynamic target namespaces this can't be narrowed via resourceNames, and the reconciler only writes copy-labeled Secrets, but the grant is broad — worth a short justifying comment in the manifest.
• Suggest an operator-facing upgrade note: on upgrade, child tenants that terminate TLS switch from per-host ACME to the replicated wildcard, and stale per-host ACME Secrets are left in place (inert), not pruned.

Review scope note: verified statically + unit tests. The upgrade-time claim that ingress-nginx picks up the replica without a restart or connection drop was reasoned from how nginx watches the Secret, not observed on a live cluster.

Code Review

This pull request introduces a new wildcardsecret controller that replicates the operator-provided wildcard TLS Secret into tenant namespaces that terminate TLS. This allows per-tenant ingress controllers and Gateways to serve TLS from their own namespaces without cross-namespace Secret reads or widened RBAC. The controller uses a scoped Secret cache to minimize memory footprint. Corresponding updates are made to the ingress and gateway packages, RBAC permissions, and documentation. The review feedback suggests a minor optimization in cloneData to return nil directly if the input map is empty, avoiding unnecessary map allocation.