diff --git a/internal/backupcontroller/default_objects_gate.go b/internal/backupcontroller/default_objects_gate.go index 8fb58c65e..b99f67b41 100644 --- a/internal/backupcontroller/default_objects_gate.go +++ b/internal/backupcontroller/default_objects_gate.go @@ -378,17 +378,29 @@ func (g *DefaultObjectsGate) Check(ctx context.Context) ([]string, bool, error) return g.reconcileCredentials(ctx) } + // The credentials comparison needs only the two Secrets, so its force goes + // out before the object reads: an unreadable BackupClass or Strategy must + // not keep the stale endpoint in the Secret every driver reads. + var needed []string + var forced bool staleCreds, credsErr := g.staleCredentials(ctx, src) + errs := []error{credsErr} if credsErr != nil { defaultObjectsCheckErrors.WithLabelValues(g.BackupClassName).Inc() } + if staleCreds != "" { + _, credsForced, err := g.force(ctx, g.CredentialsHelmRelease, &g.lastCredentialsForce, []string{staleCreds}) + needed = append(needed, staleCreds) + forced = credsForced + errs = append(errs, err) + } backupClass := &backupsv1alpha1.BackupClass{} if err := g.Client.Get(ctx, client.ObjectKey{Name: g.BackupClassName}, backupClass); err != nil { if credsErr == nil { defaultObjectsCheckErrors.WithLabelValues(g.BackupClassName).Inc() } - return nil, false, errors.Join(credsErr, fmt.Errorf("get BackupClass %s: %w", g.BackupClassName, err)) + return needed, forced, errors.Join(append(errs, fmt.Errorf("get BackupClass %s: %w", g.BackupClassName, err))...) } // With provisionBucket=false (no bucket release) the chart renders @@ -403,19 +415,10 @@ func (g *DefaultObjectsGate) Check(ctx context.Context) ([]string, bool, error) if credsErr == nil { defaultObjectsCheckErrors.WithLabelValues(g.BackupClassName).Inc() } - return nil, false, errors.Join(credsErr, err) + return needed, forced, errors.Join(append(errs, err)...) } defaultObjectsMissing.WithLabelValues(g.BackupClassName).Set(float64(len(state.missing))) - var needed []string - var forced bool - errs := []error{credsErr} - if staleCreds != "" { - _, credsForced, err := g.force(ctx, g.CredentialsHelmRelease, &g.lastCredentialsForce, []string{staleCreds}) - needed = append(needed, staleCreds) - forced = credsForced - errs = append(errs, err) - } if objects := append(state.missing, state.stale...); len(objects) > 0 { sort.Strings(objects) _, objectsForced, err := g.force(ctx, g.HelmRelease, &g.lastForce, objects) diff --git a/internal/backupcontroller/default_objects_gate_test.go b/internal/backupcontroller/default_objects_gate_test.go index fac76e132..befaedabe 100644 --- a/internal/backupcontroller/default_objects_gate_test.go +++ b/internal/backupcontroller/default_objects_gate_test.go @@ -1283,3 +1283,53 @@ func TestCheckSurfacesCredentialsPatchFailure(t *testing.T) { t.Error("lastCredentialsForce advanced despite the patch failing") } } + +// TestCheckForcesStaleCredentialsWhenObjectsUnreadable pins that the +// credentials comparison needs only the two Secrets: a BackupClass or a routed +// object the gate cannot read must not keep the stale endpoint in the +// credentials Secret, which every other driver reads through the projector. +func TestCheckForcesStaleCredentialsWhenObjectsUnreadable(t *testing.T) { + for _, tc := range []struct { + name string + backupClass bool + forbidCNPG bool + }{ + {"BackupClass absent", false, false}, + {"routed Strategy unreadable", true, true}, + } { + t.Run(tc.name, func(t *testing.T) { + defaultObjectsCheckErrors.Reset() + ctrlObjs := []client.Object{sourceSecret("bucket-1a2b"), cosiSecret("https://s3.new.example.com")} + if tc.backupClass { + ctrlObjs = append(ctrlObjs, cozyDefaultBackupClass()) + } + objs := append([]runtime.Object{helmReleaseObject(), credentialsHelmReleaseObject()}, allDefaultObjects()...) + g, dyn := newGate(t, ctrlObjs, objs...) + if tc.forbidCNPG { + dyn.PrependReactor("get", "cnpgs", func(k8stesting.Action) (bool, runtime.Object, error) { + return true, nil, apierrors.NewForbidden(schema.GroupResource{Group: strategyAPIGroup, Resource: "cnpgs"}, "cozy-default-cnpg", nil) + }) + } + + missing, forced, err := g.Check(context.Background()) + if err == nil { + t.Fatal("Check succeeded although the routed objects could not be evaluated") + } + if !forced { + t.Error("forced = false, want the bucket release forced for its stale credentials Secret") + } + if ann := credentialsForceAnnotations(t, dyn); ann["reconcile.fluxcd.io/forceAt"] == "" { + t.Errorf("bucket release not forced: %v", ann) + } + if ann := forceAnnotations(t, dyn); len(ann) != 0 { + t.Errorf("own HelmRelease forced without a conclusive object check: %v", ann) + } + if len(missing) != 1 || !strings.HasPrefix(missing[0], "Secret/bucket-cozy-backups-system-credentials") { + t.Errorf("missing = %v, want the credentials Secret", missing) + } + if got := testutil.ToFloat64(defaultObjectsCheckErrors.WithLabelValues("cozy-default")); got != 1 { + t.Errorf("check errors = %v, want 1", got) + } + }) + } +}