fixup! fix: fall back to canonical backup auth secret name on restore

fixup\! fix: fall back to canonical backup auth secret name on restore

Address review feedback:
- Use canonical DevWorkspaceBackupAuthSecretName for workspace NS lookup
  directly instead of falling back (dkwon17)
- Remove unused secretGR variable and schema import (rohanKanojia)

Assisted-by: Claude Opus 4.6 (1M context)
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>

Author: akurinnoy

pkg/secrets/backup.go

@@ -54,42 +54,23 @@ func HandleRegistryAuthSecret(ctx context.Context, c client.Client, workspace *d
 		return nil, nil
 	}
 
-	// First check the workspace namespace for the secret
+	// First check the workspace namespace for the secret.
+	// CopySecret always writes the copy under DevWorkspaceBackupAuthSecretName,
+	// so look it up by that canonical name.
 	registryAuthSecret := &corev1.Secret{}
 	err := c.Get(ctx, client.ObjectKey{
-		Name:      secretName,
+		Name:      constants.DevWorkspaceBackupAuthSecretName,
 		Namespace: workspace.Namespace}, registryAuthSecret)
 	if err == nil {
-		log.Info("Successfully retrieved registry auth secret for backup from workspace namespace", "secretName", secretName)
+		log.Info("Successfully retrieved registry auth secret for backup from workspace namespace",
+			"secretName", constants.DevWorkspaceBackupAuthSecretName)
 		return registryAuthSecret, nil
 	}
 	if client.IgnoreNotFound(err) != nil {
 		return nil, err
 	}
 	// If we don't provide an operator namespace, don't attempt to look there.
-	// However, CopySecret always writes the secret under DevWorkspaceBackupAuthSecretName,
-	// regardless of the configured name. If the configured name differs, attempt a fallback
-	// lookup under the canonical name so that the restore path can find it.
 	if operatorConfigNamespace == "" {
-		if secretName == constants.DevWorkspaceBackupAuthSecretName {
-			// The configured name IS the canonical name; it was already looked up and not
-			// found above, so there is nothing else to try.
-			return nil, nil
-		}
-		fallbackSecret := &corev1.Secret{}
-		fallbackErr := c.Get(ctx, client.ObjectKey{
-			Name:      constants.DevWorkspaceBackupAuthSecretName,
-			Namespace: workspace.Namespace,
-		}, fallbackSecret)
-		if fallbackErr == nil {
-			log.Info("Registry auth secret not found under configured name in workspace namespace; using canonical backup auth secret as fallback",
-				"configuredName", secretName,
-				"fallbackName", constants.DevWorkspaceBackupAuthSecretName)
-			return fallbackSecret, nil
-		}
-		if client.IgnoreNotFound(fallbackErr) != nil {
-			return nil, fallbackErr
-		}
 		return nil, nil
 	}
 	log.Info("Registry auth secret not found in workspace namespace, checking operator namespace", "secretName", secretName)

pkg/secrets/backup_test.go

@@ -29,7 +29,6 @@ import (
 	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -102,38 +101,22 @@ var _ = Describe("HandleRegistryAuthSecret (restore path: operatorConfigNamespac
 		scheme = buildScheme()
 	})
 
-	It("returns the secret directly when the configured name is present in the workspace namespace", func() {
-		By("creating a workspace-namespace secret whose name matches the configured auth secret name")
-		configuredName := "quay-backup-auth"
-		secret := makeSecret(configuredName, workspaceNS)
-
-		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
-		workspace := makeWorkspace(workspaceNS)
-		config := makeConfig(configuredName)
-
-		result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
-		Expect(err).NotTo(HaveOccurred())
-		Expect(result).NotTo(BeNil())
-		Expect(result.Name).To(Equal(configuredName))
-	})
-
-	It("returns the canonical backup auth secret as fallback when configured name is absent (the bug fix case)", func() {
-		By("creating only the canonical DevWorkspaceBackupAuthSecretName secret in the workspace namespace")
+	It("returns the canonical secret when it exists in the workspace namespace", func() {
+		By("creating the canonical DevWorkspaceBackupAuthSecretName secret in the workspace namespace")
 		canonicalSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS)
 
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(canonicalSecret).Build()
 		workspace := makeWorkspace(workspaceNS)
-		// Configured name is something like "quay-backup-auth" — different from the canonical name.
 		config := makeConfig("quay-backup-auth")
 
 		result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
 		Expect(err).NotTo(HaveOccurred())
-		Expect(result).NotTo(BeNil(), "should fall back to the canonical secret copied by CopySecret")
+		Expect(result).NotTo(BeNil())
 		Expect(result.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName))
 	})
 
-	It("returns nil, nil when neither the configured name nor the canonical name exists in the workspace namespace", func() {
-		By("using a fake client with no secrets at all")
+	It("returns nil when the canonical secret does not exist in the workspace namespace", func() {
+		By("using a fake client with no secrets")
 		fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
 		workspace := makeWorkspace(workspaceNS)
 		config := makeConfig("quay-backup-auth")
@@ -143,25 +126,8 @@ var _ = Describe("HandleRegistryAuthSecret (restore path: operatorConfigNamespac
 		Expect(result).To(BeNil())
 	})
 
-	It("returns the secret on the first lookup when the configured name IS the canonical name (no duplicate query)", func() {
-		By("creating the canonical secret in the workspace namespace and configuring the same name")
-		canonicalSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS)
-
-		fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(canonicalSecret).Build()
-		workspace := makeWorkspace(workspaceNS)
-		// Configured name equals the canonical constant — must be found on the first Get.
-		config := makeConfig(constants.DevWorkspaceBackupAuthSecretName)
-
-		result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
-		Expect(err).NotTo(HaveOccurred())
-		Expect(result).NotTo(BeNil())
-		Expect(result.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName))
-	})
-
-	It("propagates a real (non-NotFound) error from the fallback lookup", func() {
-		By("wrapping a fake client so that the fallback lookup returns a server error")
-		// The configured name differs from the canonical name, so the code will attempt the fallback
-		// lookup. We simulate that lookup returning a non-NotFound error.
+	It("propagates a non-NotFound error from the workspace namespace lookup", func() {
+		By("wrapping a fake client so that the canonical name lookup returns a server error")
 		errClient := &errorOnNameClient{
 			Client:   fake.NewClientBuilder().WithScheme(scheme).Build(),
 			failName: constants.DevWorkspaceBackupAuthSecretName,
@@ -196,6 +162,3 @@ func (e *errorOnNameClient) Get(ctx context.Context, key client.ObjectKey, obj c
 
 // Ensure errorOnNameClient satisfies client.Client at compile time.
 var _ client.Client = &errorOnNameClient{}
-
-// secretGR is the GroupResource for secrets, used when constructing test errors.
-var secretGR = schema.GroupResource{Group: "", Resource: "secrets"} //nolint:unused