diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 65c803029..e283f6b19 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -179,8 +179,6 @@ const ( // DevWorkspaceBackupJobLabel is the label key to identify backup jobs created for DevWorkspaces DevWorkspaceBackupJobLabel = "controller.devfile.io/backup-job" - DevWorkspaceBackupAuthSecretName = "devworkspace-backup-registry-auth" - // DevWorkspaceLastBackupSuccessfulAnnotation is an annotation that indicates whether the last backup // attempt for this DevWorkspace was successful. Value is either "true" or "false". DevWorkspaceLastBackupSuccessfulAnnotation = "controller.devfile.io/last-backup-successful" diff --git a/pkg/library/restore/restore.go b/pkg/library/restore/restore.go index b54c01ef1..03d357eb1 100644 --- a/pkg/library/restore/restore.go +++ b/pkg/library/restore/restore.go @@ -22,17 +22,18 @@ import ( "strings" dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" - "github.com/devfile/devworkspace-operator/pkg/common" - devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" - dwResources "github.com/devfile/devworkspace-operator/pkg/library/resources" - "github.com/devfile/devworkspace-operator/pkg/secrets" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/devfile/devworkspace-operator/internal/images" + "github.com/devfile/devworkspace-operator/pkg/common" "github.com/devfile/devworkspace-operator/pkg/constants" + "github.com/devfile/devworkspace-operator/pkg/infrastructure" + devfileConstants "github.com/devfile/devworkspace-operator/pkg/library/constants" + dwResources "github.com/devfile/devworkspace-operator/pkg/library/resources" + "github.com/devfile/devworkspace-operator/pkg/secrets" ) const ( @@ -114,7 +115,11 @@ func GetWorkspaceRestoreInitContainer( MountPath: constants.DefaultProjectsSourcesRoot, }, } - registryAuthSecret, err := secrets.GetNamespaceRegistryAuthSecret(ctx, k8sClient, workspace.DevWorkspace, workspace.Config, scheme, log) + operatorNamespace, err := infrastructure.GetNamespace() + if err != nil { + return nil, nil, fmt.Errorf("failed to get operator namespace for workspace restore: %w", err) + } + registryAuthSecret, err := secrets.GetNamespaceRegistryAuthSecret(ctx, k8sClient, workspace.DevWorkspace, workspace.Config, operatorNamespace, scheme, log) if err != nil { return nil, nil, fmt.Errorf("handling registry auth secret for workspace restore: %w", err) } diff --git a/pkg/secrets/backup.go b/pkg/secrets/backup.go index 18935ad24..828f2a4d5 100644 --- a/pkg/secrets/backup.go +++ b/pkg/secrets/backup.go @@ -32,12 +32,13 @@ import ( "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) -// GetRegistryAuthSecret retrieves the registry authentication secret for accessing backup images -// based on the operator configuration. +// GetNamespaceRegistryAuthSecret retrieves the registry authentication secret for accessing backup images +// based on the operator configuration. operatorConfigNamespace is the namespace where the operator is +// running; if non-empty, the secret is also looked up there and copied to the workspace namespace when found. func GetNamespaceRegistryAuthSecret(ctx context.Context, c client.Client, workspace *dw.DevWorkspace, - dwOperatorConfig *controllerv1alpha1.OperatorConfiguration, scheme *runtime.Scheme, log logr.Logger, + dwOperatorConfig *controllerv1alpha1.OperatorConfiguration, operatorConfigNamespace string, scheme *runtime.Scheme, log logr.Logger, ) (*corev1.Secret, error) { - return HandleRegistryAuthSecret(ctx, c, workspace, dwOperatorConfig, "", scheme, log) + return HandleRegistryAuthSecret(ctx, c, workspace, dwOperatorConfig, operatorConfigNamespace, scheme, log) } func HandleRegistryAuthSecret(ctx context.Context, c client.Client, workspace *dw.DevWorkspace, @@ -81,15 +82,16 @@ func HandleRegistryAuthSecret(ctx context.Context, c client.Client, workspace *d return nil, err } log.Info("Successfully retrieved registry auth secret for backup job", "secretName", secretName) - return CopySecret(ctx, c, workspace, registryAuthSecret, scheme, log) + return CopySecret(ctx, c, workspace, registryAuthSecret, secretName, scheme, log) } -// CopySecret copies the given secret from the operator namespace to the workspace namespace. -func CopySecret(ctx context.Context, c client.Client, workspace *dw.DevWorkspace, sourceSecret *corev1.Secret, scheme *runtime.Scheme, log logr.Logger) (namespaceSecret *corev1.Secret, err error) { +// CopySecret copies the given secret from the operator namespace to the workspace namespace, +// naming the copy secretName (the configured auth secret name from the operator config). +func CopySecret(ctx context.Context, c client.Client, workspace *dw.DevWorkspace, sourceSecret *corev1.Secret, secretName string, scheme *runtime.Scheme, log logr.Logger) (namespaceSecret *corev1.Secret, err error) { // Construct the desired secret state desiredSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: constants.DevWorkspaceBackupAuthSecretName, + Name: secretName, Namespace: workspace.Namespace, Labels: map[string]string{ constants.DevWorkspaceWatchSecretLabel: "true", diff --git a/pkg/secrets/backup_test.go b/pkg/secrets/backup_test.go new file mode 100644 index 000000000..4f4583d2f --- /dev/null +++ b/pkg/secrets/backup_test.go @@ -0,0 +1,252 @@ +// +// Copyright (c) 2019-2026 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +// Generated by Claude Sonnet 4.6 (1M context) + +package secrets_test + +import ( + "context" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1" + "github.com/devfile/devworkspace-operator/pkg/secrets" +) + +func TestSecrets(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Secrets Suite") +} + +var _ = Describe("Backup secrets", func() { + var ( + ctx context.Context + scheme *runtime.Scheme + ) + + const ( + operatorNamespace = "devworkspace-controller" + workspaceNamespace = "user-namespace" + configuredAuthSecretName = "quay-backup-auth" + ) + + BeforeEach(func() { + ctx = context.Background() + + scheme = runtime.NewScheme() + Expect(dw.AddToScheme(scheme)).To(Succeed()) + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + Expect(controllerv1alpha1.AddToScheme(scheme)).To(Succeed()) + }) + + // buildFakeClient creates a fake client with the given objects pre-populated. + buildFakeClient := func(objs ...client.Object) client.Client { + return fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...).Build() + } + + // buildWorkspace creates a minimal DevWorkspace for use in tests. + buildWorkspace := func(name, namespace string) *dw.DevWorkspace { + return &dw.DevWorkspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + UID: "test-uid", + }, + } + } + + // buildConfig creates an OperatorConfiguration with the given registry auth secret name. + buildConfig := func(authSecretName string) *controllerv1alpha1.OperatorConfiguration { + return &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + Registry: &controllerv1alpha1.RegistryConfig{ + Path: "my-registry:5000", + AuthSecret: authSecretName, + }, + }, + }, + } + } + + // buildSecret creates a secret with the given name, namespace and data. + buildSecret := func(name, namespace string, data map[string][]byte) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Data: data, + Type: corev1.SecretTypeDockerConfigJson, + } + } + + log := zap.New(zap.UseDevMode(true)).WithName("SecretsTest") + + Context("CopySecret", func() { + It("creates the copy using the configured secret name, not devworkspace-backup-registry-auth", func() { + workspace := buildWorkspace("my-workspace", workspaceNamespace) + sourceSecret := buildSecret(configuredAuthSecretName, operatorNamespace, map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{}}`), + }) + fakeClient := buildFakeClient(workspace) + + By("calling CopySecret with the configured secret name") + result, err := secrets.CopySecret(ctx, fakeClient, workspace, sourceSecret, configuredAuthSecretName, scheme, log) + Expect(err).NotTo(HaveOccurred()) + // The first sync creates the object and returns desiredSecret (NotInSyncError path) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(configuredAuthSecretName)) + Expect(result.Namespace).To(Equal(workspaceNamespace)) + + By("verifying the secret was created in the workspace namespace with the correct name") + clusterSecret := &corev1.Secret{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: configuredAuthSecretName, + Namespace: workspaceNamespace, + }, clusterSecret)).To(Succeed()) + Expect(clusterSecret.Name).To(Equal(configuredAuthSecretName)) + Expect(clusterSecret.Data).To(Equal(sourceSecret.Data)) + + By("verifying the hardcoded default name was NOT used") + legacySecret := &corev1.Secret{} + err = fakeClient.Get(ctx, types.NamespacedName{ + Name: "devworkspace-backup-registry-auth", + Namespace: workspaceNamespace, + }, legacySecret) + Expect(client.IgnoreNotFound(err)).To(Succeed()) + Expect(err).To(HaveOccurred(), "legacy hardcoded name must not be used") + }) + }) + + Context("HandleRegistryAuthSecret", func() { + It("returns secret directly when it exists in the workspace namespace with the configured name", func() { + workspace := buildWorkspace("my-workspace", workspaceNamespace) + config := buildConfig(configuredAuthSecretName) + // Secret already exists in workspace namespace + localSecret := buildSecret(configuredAuthSecretName, workspaceNamespace, map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"local":{}}}`), + }) + fakeClient := buildFakeClient(workspace, localSecret) + + By("calling HandleRegistryAuthSecret with operator namespace") + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNamespace, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(configuredAuthSecretName)) + Expect(result.Namespace).To(Equal(workspaceNamespace)) + }) + + It("copies secret from operator namespace to workspace namespace with the configured name", func() { + workspace := buildWorkspace("my-workspace", workspaceNamespace) + config := buildConfig(configuredAuthSecretName) + // Secret only exists in the operator namespace + operatorSecret := buildSecret(configuredAuthSecretName, operatorNamespace, map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"operator":{}}}`), + }) + fakeClient := buildFakeClient(workspace, operatorSecret) + + By("calling HandleRegistryAuthSecret — secret missing from workspace NS, present in operator NS") + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNamespace, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + // The result should use the configured name + Expect(result.Name).To(Equal(configuredAuthSecretName)) + Expect(result.Namespace).To(Equal(workspaceNamespace)) + + By("verifying the copy exists in workspace namespace with the configured name") + clusterSecret := &corev1.Secret{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: configuredAuthSecretName, + Namespace: workspaceNamespace, + }, clusterSecret)).To(Succeed()) + + By("verifying the legacy hardcoded name was NOT created") + legacySecret := &corev1.Secret{} + err = fakeClient.Get(ctx, types.NamespacedName{ + Name: "devworkspace-backup-registry-auth", + Namespace: workspaceNamespace, + }, legacySecret) + Expect(client.IgnoreNotFound(err)).To(Succeed()) + Expect(err).To(HaveOccurred(), "legacy hardcoded name must not be created") + }) + + It("returns nil, nil when operator namespace is given but secret is in neither namespace", func() { + workspace := buildWorkspace("my-workspace", workspaceNamespace) + config := buildConfig(configuredAuthSecretName) + fakeClient := buildFakeClient(workspace) // No secrets pre-created + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNamespace, scheme, log) + // Secret not found in operator namespace returns an error (not found) + // The function logs the error and returns it + Expect(err).To(HaveOccurred()) + Expect(result).To(BeNil()) + }) + }) + + Context("GetNamespaceRegistryAuthSecret", func() { + It("end-to-end: with operatorConfigNamespace, copies secret from operator NS and returns it with configured name", func() { + workspace := buildWorkspace("my-workspace", workspaceNamespace) + config := buildConfig(configuredAuthSecretName) + // Secret only in operator namespace + operatorSecret := buildSecret(configuredAuthSecretName, operatorNamespace, map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"e2e":{}}}`), + }) + fakeClient := buildFakeClient(workspace, operatorSecret) + + By("calling GetNamespaceRegistryAuthSecret with a non-empty operatorConfigNamespace") + result, err := secrets.GetNamespaceRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNamespace, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(configuredAuthSecretName)) + Expect(result.Namespace).To(Equal(workspaceNamespace)) + + By("verifying the copy exists with the configured name in the workspace namespace") + clusterSecret := &corev1.Secret{} + Expect(fakeClient.Get(ctx, types.NamespacedName{ + Name: configuredAuthSecretName, + Namespace: workspaceNamespace, + }, clusterSecret)).To(Succeed()) + }) + + It("legacy no-op case: with operatorConfigNamespace empty and secret in workspace NS, returns it directly", func() { + workspace := buildWorkspace("my-workspace", workspaceNamespace) + config := buildConfig(configuredAuthSecretName) + // Secret already exists in workspace namespace + localSecret := buildSecret(configuredAuthSecretName, workspaceNamespace, map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"local":{}}}`), + }) + fakeClient := buildFakeClient(workspace, localSecret) + + By("calling GetNamespaceRegistryAuthSecret with empty operatorConfigNamespace (legacy behavior)") + result, err := secrets.GetNamespaceRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(configuredAuthSecretName)) + Expect(result.Namespace).To(Equal(workspaceNamespace)) + }) + }) +})