From 4e0f7e2d1158097a1ef619ace47ac1fa95337163 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Fri, 10 Apr 2026 12:54:00 +0300 Subject: [PATCH 1/2] fix: restore init container uses configured registry auth secret name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When backing up, CopySecret always named the copied secret with the hardcoded constant DevWorkspaceBackupAuthSecretName ("devworkspace- backup-registry-auth") regardless of the admin-configured name (e.g. "quay-backup-auth"). On restore, GetNamespaceRegistryAuthSecret was called with an empty operatorConfigNamespace, so it only searched the workspace namespace, did not find the secret there (it was either not copied at all or copied under the wrong name), and returned nil — causing the workspace-restore init container to start without registry auth and immediately crash with CrashLoopBackOff. Changes: - CopySecret: add secretName parameter; use it for the copy's ObjectMeta Name instead of the hardcoded constant. - HandleRegistryAuthSecret: pass secretName (the configured auth secret name) to CopySecret. - GetNamespaceRegistryAuthSecret: add operatorConfigNamespace parameter and thread it through to HandleRegistryAuthSecret. - pkg/library/restore: resolve the operator namespace via infrastructure.GetNamespace() and pass it to GetNamespaceRegistryAuthSecret so the lookup can fall back to the operator namespace when the secret is absent from the workspace NS. Migration note: any existing copies named "devworkspace-backup-registry- auth" become orphaned under their DevWorkspace ownerReference; Kubernetes garbage-collects them when the workspace is deleted. No active cleanup code is needed. Assisted-by: Claude Sonnet 4.6 (1M context) Signed-off-by: Oleksii Kurinnyi --- pkg/library/restore/restore.go | 15 +- pkg/secrets/backup.go | 18 +-- pkg/secrets/backup_test.go | 252 +++++++++++++++++++++++++++++++++ 3 files changed, 272 insertions(+), 13 deletions(-) create mode 100644 pkg/secrets/backup_test.go 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)) + }) + }) +}) From 7c3743cd5afba7f37c0b21ddee8801f13eae3950 Mon Sep 17 00:00:00 2001 From: Oleksii Kurinnyi Date: Fri, 10 Apr 2026 18:10:45 +0300 Subject: [PATCH 2/2] refactor: remove unused DevWorkspaceBackupAuthSecretName constant The constant was used by CopySecret to hardcode the copy name regardless of the configured secret name. Now that CopySecret takes an explicit secretName parameter, the constant has no remaining callers and can be removed. Assisted-by: Claude Sonnet 4.6 (1M context) Signed-off-by: Oleksii Kurinnyi --- pkg/constants/metadata.go | 2 -- 1 file changed, 2 deletions(-) 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"