fix(backups): refresh stale credentials even when the routed objects are unreadable

The credentials endpoint comparison needs only the credentials Secret and
the COSI Secret it is rendered from, yet its forced render was issued only
after the BackupClass and every routed object had been read. A missing
BackupClass or one unreadable Strategy therefore kept the old endpoint in
the credentials Secret, which the projector hands to every driver, not
just the one whose object failed.

Assisted-by: LLM
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>

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)

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)
+			}
+		})
+	}
+}