diff --git a/internal/controller/cacert/reconciler.go b/internal/controller/cacert/reconciler.go index b9e59826e..33b0b8fdb 100644 --- a/internal/controller/cacert/reconciler.go +++ b/internal/controller/cacert/reconciler.go @@ -318,6 +318,7 @@ const ( reasonNoRelease = "NoRelease" reasonUnsupportedProjection = "UnsupportedProjectionType" reasonMultipleCACert = "MultipleCACertProjections" + reasonReleaseContested = "ReleaseContested" ) // privateKeyHeader matches a PEM private-key header of any flavour (PKCS#1, @@ -454,6 +455,38 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, nil } + if caCertEntries == 0 { + // No CACert entry declares the trust anchor any more — a chart that gated its + // CACert entry (say on tls.enabled) while still rendering the sentinel lands + // here. Withdraw a projection a previous declaration published: garbage + // collection only fires when the whole sentinel is pruned, so a sentinel that + // keeps existing with its CACert entry removed would otherwise serve a stale + // anchor forever. Only a projection this sentinel owns is deleted; a foreign + // Secret, or another sentinel's projection at the name, is left untouched. + if err := r.withdrawProjection(ctx, tp, target); err != nil { + return ctrl.Result{}, err + } + } else { + // Exactly one CACert entry. Refuse if another sentinel in this namespace + // declares one for the same release: both resolve to the single canonical + // name, so publishing from either would silently pick an arbitrary winner and + // let the two flap ownership of one projection, while deleting either would + // garbage-collect an anchor the other still declares. Refuse both — neither + // writes — until the declaration lives on exactly one sentinel. This mirrors + // the more-than-one-CACert-entry refusal above, one level up. + other, err := r.anotherSentinelForRelease(ctx, tp, release) + if err != nil { + return ctrl.Result{}, err + } + if other != "" { + if err := r.setReady(ctx, tp, notReady(reasonReleaseContested, + fmt.Sprintf("another TenantProjection %q in this namespace declares a CACert projection for release %q; both resolve to the canonical name %q, so neither is published — declare the trust anchor from exactly one sentinel", other, release, target))); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + } + // The sentinel's Ready condition aggregates its projections: True only when // every declared projection is published, otherwise the first failure. ready := metav1.Condition{ @@ -554,6 +587,58 @@ func (r *Reconciler) reconcileCACert(ctx context.Context, tp *internalv1alpha1.T }, nil } +// withdrawProjection deletes the canonical trust-anchor Secret when the sentinel +// no longer declares one, but ONLY when it still carries this sentinel's owner +// reference. Deleting the whole sentinel is handled by garbage collection through +// that reference; this covers the narrower case of a sentinel that keeps existing +// with its CACert declaration removed, which GC never sees. A foreign Secret at the +// name, or a projection another sentinel owns, is never deleted — the owner- +// reference check is what keeps the withdrawal from destroying data that is not +// this sentinel's to withdraw. +func (r *Reconciler) withdrawProjection(ctx context.Context, tp *internalv1alpha1.TenantProjection, target string) error { + existing := &corev1.Secret{} + if err := r.Reader.Get(ctx, types.NamespacedName{Namespace: tp.Namespace, Name: target}, existing); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + return fmt.Errorf("get CA projection %s/%s: %w", tp.Namespace, target, err) + } + if !hasOwner(existing.OwnerReferences, sentinelOwnerRef(tp)) { + return nil + } + if err := r.Delete(ctx, existing); err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("withdraw CA projection %s/%s: %w", tp.Namespace, target, err) + } + return nil +} + +// anotherSentinelForRelease returns the name of a DIFFERENT sentinel in the same +// namespace that carries the same release label and also declares a CACert +// projection, or "" when this sentinel is the only one for the release. Two such +// sentinels resolve to the same canonical projection name, so the reconcile refuses +// to publish from either; see the caller. +func (r *Reconciler) anotherSentinelForRelease(ctx context.Context, tp *internalv1alpha1.TenantProjection, release string) (string, error) { + list := &internalv1alpha1.TenantProjectionList{} + if err := r.List(ctx, list, client.InNamespace(tp.Namespace)); err != nil { + return "", fmt.Errorf("list TenantProjections in %s: %w", tp.Namespace, err) + } + for i := range list.Items { + other := &list.Items[i] + if other.Name == tp.Name || other.DeletionTimestamp != nil { + continue + } + if other.Labels[helmNameLabel] != release { + continue + } + for j := range other.Spec.Projections { + if other.Spec.Projections[j].Type == internalv1alpha1.ProjectionTypeCACert { + return other.Name, nil + } + } + } + return "", nil +} + // reconcileSourceAtCanonicalName handles the degenerate case where the source // itself sits at the canonical projection name — a Secret cannot be projected // over itself. diff --git a/internal/controller/cacert/reconciler_test.go b/internal/controller/cacert/reconciler_test.go index ee237cb86..6dfd740bf 100644 --- a/internal/controller/cacert/reconciler_test.go +++ b/internal/controller/cacert/reconciler_test.go @@ -505,6 +505,73 @@ func TestReconcile_MultipleCACertEntries_Rejected(t *testing.T) { assertReady(t, c, metav1.ConditionFalse, reasonMultipleCACert) } +// TestReconcile_EntryRemoved_WithdrawsProjection exercises the withdrawal path: a +// sentinel that stops declaring a CACert projection must have the anchor it +// published removed. Garbage collection only fires when the whole sentinel is +// pruned, so without an explicit delete a sentinel that keeps existing with its +// CACert entry gated off (e.g. on tls.enabled) would serve a stale trust anchor +// forever. +func TestReconcile_EntryRemoved_WithdrawsProjection(t *testing.T) { + c := newClient(t, helmRelease(), appDef(), sentinel(), + operatorSecret(testOperatorCA, map[string][]byte{caCertKey: []byte(testCA), caKeyKey: []byte(testKey)}), + ) + mustReconcile(t, c) + if _, ok := getSecret(t, c, testProjection); !ok { + t.Fatalf("the projection must exist while the CACert entry is declared") + } + + // Drop the CACert entry through the internal type. The CRD's MinItems=1 forbids + // an empty projections list at the API server, but a chart that gates the entry + // — or a future non-CACert entry replacing it — leaves the sentinel with zero + // CACert projections while the old anchor still stands. + tp := &internalv1alpha1.TenantProjection{} + if err := c.Get(context.TODO(), types.NamespacedName{Namespace: testNamespace, Name: testSentinel}, tp); err != nil { + t.Fatalf("get sentinel: %v", err) + } + tp.Spec.Projections = nil + if err := c.Update(context.TODO(), tp); err != nil { + t.Fatalf("drop the CACert entry: %v", err) + } + mustReconcile(t, c) + + if _, ok := getSecret(t, c, testProjection); ok { + t.Errorf("withdrawing the last CACert entry must delete the projection it published") + } +} + +// TestReconcile_TwoSentinelsForOneRelease_Refused pins the two-sentinel guard: two +// sentinels in one namespace carrying the same release label both resolve to the +// single canonical name. Without the guard, whichever reconciles first publishes an +// arbitrary CA (a silent winner) and the two flap ownership of one projection. Both +// must refuse instead, so nothing is published until the declaration lives on +// exactly one sentinel. +func TestReconcile_TwoSentinelsForOneRelease_Refused(t *testing.T) { + second := &internalv1alpha1.TenantProjection{ + ObjectMeta: metav1.ObjectMeta{ + Name: testSentinel + "-dup", + Namespace: testNamespace, + UID: types.UID("77777777-8888-9999-aaaa-bbbbbbbbbbbb"), + Labels: map[string]string{helmNameLabel: testRelease}, + }, + Spec: internalv1alpha1.TenantProjectionSpec{ + Projections: []internalv1alpha1.TenantProjectionEntry{{ + Type: internalv1alpha1.ProjectionTypeCACert, + SourceSecretName: testOperatorCA, + SourceKey: caCertKey, + }}, + }, + } + c := newClient(t, helmRelease(), appDef(), sentinel(), second, + operatorSecret(testOperatorCA, map[string][]byte{caCertKey: []byte(testCA), caKeyKey: []byte(testKey)}), + ) + mustReconcile(t, c) + + if _, ok := getSecret(t, c, testProjection); ok { + t.Errorf("two sentinels contesting one release must publish nothing, not an arbitrary winner") + } + assertReady(t, c, metav1.ConditionFalse, reasonReleaseContested) +} + // --- Reconcile: the write-path guards, exercised through a real source --- // TestReconcile_RefusesPrivateKeyUnderLiftedKey pins the reason the controller diff --git a/packages/system/cozystack-controller/templates/rbac.yaml b/packages/system/cozystack-controller/templates/rbac.yaml index adee3d4f2..d8d176930 100644 --- a/packages/system/cozystack-controller/templates/rbac.yaml +++ b/packages/system/cozystack-controller/templates/rbac.yaml @@ -51,10 +51,11 @@ rules: # terminates TLS; tenant ingress controllers / Gateways still read only the # replica in their own namespace. # - CACertReconciler projects an application's CA certificate into the key-free -# ".tenant-ca" Secret of the same namespace. It only ever creates or -# updates that projection; the projection is garbage-collected through its -# owner reference to the TenantProjection sentinel when the application (and so -# the sentinel) is deleted, so this reconciler itself never deletes a Secret. +# ".tenant-ca" Secret of the same namespace. It creates and updates +# that projection, and deletes it to WITHDRAW a retired declaration — but only +# a projection it owns by owner reference, never a foreign Secret. Deleting the +# whole sentinel is handled instead by garbage collection, through the +# projection's owner reference to the TenantProjection. # # The read verbs (get/list/watch) are listed explicitly so the rule fully # describes both reconcilers' Secret access on its own and keeps working if the @@ -82,8 +83,10 @@ rules: # compromise of every Secret in the cluster — including the CA private keys the # CA-extraction controller exists to keep away from tenants. The controller's own # code is what confines it: the CA reconciler lifts exactly one whitelisted key, -# re-guards the bytes on every write, and never deletes a projection at all — -# garbage collection removes it through the sentinel owner reference. That is a +# re-guards the bytes on every write, and deletes only a projection it owns by +# owner reference — to withdraw a retired declaration — never a foreign Secret; +# garbage collection removes the projection through the sentinel owner reference +# when the sentinel itself goes. That is a # code-level confinement of an RBAC-level power, which is # strictly weaker than not holding the power — the honest reading is that the # guard is the boundary, and the RBAC is not.