fix(backups): raise ArtifactNotMeasured only for a Backup this pass created

The Event was recorded before the Backup was created, so a Create that
then failed left the BackupJob Failed next to an Event saying the
backup succeeded, and a Create that collided with a Backup another pass
had made reported on that pass's Backup. The Event is now recorded once
Create succeeds. It also covers the one unmeasured path that had none:
a failed existence check, which skips the probe.

A test now also pins that a CA Secret key the s3-ca volume's items do
not project is never trusted.

Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>

internal/backupcontroller/redis_artifact_size_test.go

@@ -186,6 +186,16 @@ func configMapS3CA(spec *corev1.PodSpec) *corev1.PodSpec {
 	return spec
 }
 
+// nextEvent returns the next recorded event, or "" when there is none.
+func nextEvent(rec *record.FakeRecorder) string {
+	select {
+	case ev := <-rec.Events:
+		return ev
+	default:
+		return ""
+	}
+}
+
 func TestRedisS3Trust(t *testing.T) {
 	caSecret := func(data map[string]string) *corev1.Secret {
 		s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "s3-endpoint-ca"}, Data: map[string][]byte{}}
@@ -267,6 +277,14 @@ func TestRedisS3Trust(t *testing.T) {
 			objs: []client.Object{caSecret(map[string]string{"other": "x"})},
 		},
 		"an s3-ca volume of another source": {spec: func(skip string) *corev1.PodSpec { return configMapS3CA(redisPodSpec("", skip)) }},
+		"a CA Secret whose keys the volume does not project": {
+			spec: func(skip string) *corev1.PodSpec {
+				s := redisPodSpec("s3-endpoint-ca", skip)
+				s.Volumes[0].Secret.Items = []corev1.KeyToPath{{Key: "bundle.pem", Path: "ca.crt"}}
+				return s
+			},
+			objs: []client.Object{caSecret(map[string]string{"tls.crt": "UNPROJECTED"})},
+		},
 	}
 	for name, tc := range refused {
 		for _, skip := range []string{"", "true"} {
@@ -445,6 +463,8 @@ func TestCreateRedisBackupArtifact_RetryCheck(t *testing.T) {
 		l := &fakeS3Lister{statSize: 777}
 		r := redisReconciler(t, l, j, projectedCredsSecret(nil))
 		r.APIReader = failingReads(t, newMariaDBStrategyTestClient(t))
+		rec := record.NewFakeRecorder(4)
+		r.Recorder = rec
 		got, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil)
 		if err != nil || got == nil {
 			t.Fatalf("createRedisBackupArtifact = %v, %v; a transient read must not fail the backup", got, err)
@@ -452,6 +472,37 @@ func TestCreateRedisBackupArtifact_RetryCheck(t *testing.T) {
 		if l.calls != 0 {
 			t.Errorf("S3 probed %d times after the existence check failed", l.calls)
 		}
+		if ev := nextEvent(rec); !strings.HasPrefix(ev, "Normal ArtifactNotMeasured ") || !strings.Contains(ev, "could not be checked") {
+			t.Errorf("event = %q, want ArtifactNotMeasured saying the existence check failed", ev)
+		}
+	})
+
+	t.Run("no event when Create fails", func(t *testing.T) {
+		r := redisReconciler(t, &fakeS3Lister{statErr: errors.New("NoSuchKey")}, j, projectedCredsSecret(nil))
+		r.Client = forbidBackupCreate(t, r.Client)
+		rec := record.NewFakeRecorder(4)
+		r.Recorder = rec
+		if _, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil); err == nil {
+			t.Fatal("createRedisBackupArtifact succeeded although Create was rejected")
+		}
+		if ev := nextEvent(rec); ev != "" {
+			t.Errorf("event %q although the BackupJob fails", ev)
+		}
+	})
+
+	t.Run("no event when Create collides with a Backup another pass made", func(t *testing.T) {
+		backup := &backupsv1alpha1.Backup{ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "bj-1"}}
+		store := newMariaDBStrategyTestClient(t, j, backup, projectedCredsSecret(nil))
+		r := redisReconciler(t, &fakeS3Lister{statErr: errors.New("NoSuchKey")})
+		r.Client, r.APIReader = staleCache(t, store), notFoundOnce(t, store)
+		rec := record.NewFakeRecorder(4)
+		r.Recorder = rec
+		if _, err := r.createRedisBackupArtifact(context.Background(), j, &ResolvedBackupConfig{}, redisTestKey, nil); err != nil {
+			t.Fatalf("createRedisBackupArtifact: %v", err)
+		}
+		if ev := nextEvent(rec); ev != "" {
+			t.Errorf("event %q about a Backup this pass did not create", ev)
+		}
 	})
 
 	t.Run("a Create collision is resolved from the API server past the cache", func(t *testing.T) {

internal/backupcontroller/redisstrategy_controller.go

@@ -498,13 +498,15 @@ func (r *BackupJobReconciler) createRedisBackupArtifact(
 			Phase: backupsv1alpha1.BackupPhaseReady,
 		},
 	}
+	unmeasured := "whether the Backup already exists could not be checked"
 	if probe {
 		artifact, err := r.redisArtifact(ctx, j.Namespace, objectKey, podSpec)
+		unmeasured = ""
 		if err != nil {
 			getLogger(ctx).Info("leaving the Redis backup artifact unset", "backupjob", j.Name, "objectKey", objectKey, "error", err)
 			var probeErr *artifactProbeError
-			if r.Recorder != nil && errors.As(err, &probeErr) {
-				r.Recorder.Eventf(j, corev1.EventTypeNormal, "ArtifactNotMeasured", "The backup succeeded, but its location and size were not recorded: %s", probeErr.reason)
+			if errors.As(err, &probeErr) {
+				unmeasured = probeErr.reason
 			}
 		}
 		backup.Status.Artifact = artifact
@@ -522,6 +524,11 @@ func (r *BackupJobReconciler) createRedisBackupArtifact(
 		}
 		return existing, nil
 	}
+	// Only once the Backup exists: a terminal Create error fails the
+	// BackupJob, and a collision returns a Backup another pass measured.
+	if unmeasured != "" && r.Recorder != nil {
+		r.Recorder.Eventf(j, corev1.EventTypeNormal, "ArtifactNotMeasured", "The backup succeeded, but its location and size were not recorded: %s", unmeasured)
+	}
 	return backup, nil
 }