diff --git a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go index 60a2df5d4..8283440ff 100644 --- a/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go +++ b/apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go @@ -80,13 +80,31 @@ type RegistryConfig struct { // AuthSecret is the name of a Kubernetes secret of // type kubernetes.io/dockerconfigjson. // The secret is expected to be in the same namespace the workspace is running in. - // If secret is not found in the workspace namespace, the operator will look for the secret - // in the namespace where the operator is running in. - // as the DevWorkspaceOperatorCongfig. + // If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + // the operator will copy the secret from the operator namespace to the workspace namespace. // The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be // recognized by the operator. // +kubebuilder:validation:Optional AuthSecret string `json:"authSecret,omitempty"` + // CopyOperatorAuthSecret controls whether the operator should copy the authentication + // secret from the operator namespace to the workspace namespace when it's not found + // in the workspace namespace. + // + // If true: The operator will copy the secret from the operator namespace + // if it's not found in the workspace namespace. This provides automatic configuration + // but exposes operator-level credentials to workspace users. + // + // If false (default): The operator will not copy the secret. Users must manually create a secret + // with the configured name in their workspace namespace. This is more secure as it allows + // users to provide scoped credentials with minimal privileges. + // + // Note: Regardless of this setting, if a secret already exists in the workspace namespace, + // it will never be overwritten. User-provided secrets are always respected. + // + // Defaults to false. + // +kubebuilder:validation:Optional + // +kubebuilder:default=false + CopyOperatorAuthSecret *bool `json:"copyOperatorAuthSecret,omitempty"` } type OrasConfig struct { diff --git a/apis/controller/v1alpha1/zz_generated.deepcopy.go b/apis/controller/v1alpha1/zz_generated.deepcopy.go index f795f3d43..223c2b8ff 100644 --- a/apis/controller/v1alpha1/zz_generated.deepcopy.go +++ b/apis/controller/v1alpha1/zz_generated.deepcopy.go @@ -58,7 +58,7 @@ func (in *BackupCronJobConfig) DeepCopyInto(out *BackupCronJobConfig) { if in.Registry != nil { in, out := &in.Registry, &out.Registry *out = new(RegistryConfig) - **out = **in + (*in).DeepCopyInto(*out) } if in.OrasConfig != nil { in, out := &in.OrasConfig, &out.OrasConfig @@ -669,6 +669,11 @@ func (in *Proxy) DeepCopy() *Proxy { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RegistryConfig) DeepCopyInto(out *RegistryConfig) { *out = *in + if in.CopyOperatorAuthSecret != nil { + in, out := &in.CopyOperatorAuthSecret, &out.CopyOperatorAuthSecret + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RegistryConfig. diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index 134bdb17e..a2b0a8323 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -121,7 +121,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: pointer.Bool(true), Schedule: "* * * * *", Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -148,7 +149,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -173,7 +175,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -206,7 +209,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule1, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -242,7 +246,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule1, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -268,7 +273,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -303,8 +309,9 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", - AuthSecret: "non-existent", + Path: "fake-registry", + AuthSecret: "non-existent", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -335,7 +342,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, OrasConfig: &controllerv1alpha1.OrasConfig{ ExtraArgs: "--extra-arg1", @@ -392,7 +400,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Schedule: schedule, BackoffLimit: &backoffLimit, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -428,7 +437,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -469,7 +479,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -500,8 +511,9 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "my-registry:5000", - AuthSecret: "my-secret", + Path: "my-registry:5000", + AuthSecret: "my-secret", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -536,8 +548,9 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "my-registry:5000", - AuthSecret: "my-secret", + Path: "my-registry:5000", + AuthSecret: "my-secret", + CopyOperatorAuthSecret: pointer.Bool(true), }, }, }, @@ -572,7 +585,8 @@ var _ = Describe("BackupCronJobReconciler", func() { Enable: &enabled, Schedule: schedule, Registry: &controllerv1alpha1.RegistryConfig{ - Path: "fake-registry", + Path: "fake-registry", + CopyOperatorAuthSecret: pointer.Bool(true), }, OrasConfig: &controllerv1alpha1.OrasConfig{ ExtraArgs: "--extra-arg1", diff --git a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 79a1844f0..7af1aff11 100644 --- a/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/bundle/manifests/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -214,12 +214,31 @@ spec: AuthSecret is the name of a Kubernetes secret of type kubernetes.io/dockerconfigjson. The secret is expected to be in the same namespace the workspace is running in. - If secret is not found in the workspace namespace, the operator will look for the secret - in the namespace where the operator is running in. - as the DevWorkspaceOperatorCongfig. + If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + the operator will copy the secret from the operator namespace to the workspace namespace. The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be recognized by the operator. type: string + copyOperatorAuthSecret: + default: false + description: |- + CopyOperatorAuthSecret controls whether the operator should copy the authentication + secret from the operator namespace to the workspace namespace when it's not found + in the workspace namespace. + + If true: The operator will copy the secret from the operator namespace + if it's not found in the workspace namespace. This provides automatic configuration + but exposes operator-level credentials to workspace users. + + If false (default): The operator will not copy the secret. Users must manually create a secret + with the configured name in their workspace namespace. This is more secure as it allows + users to provide scoped credentials with minimal privileges. + + Note: Regardless of this setting, if a secret already exists in the workspace namespace, + it will never be overwritten. User-provided secrets are always respected. + + Defaults to false. + type: boolean path: description: |- A registry where backup images are stored. Images are stored diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index e19401db5..86fccc025 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -218,12 +218,31 @@ spec: AuthSecret is the name of a Kubernetes secret of type kubernetes.io/dockerconfigjson. The secret is expected to be in the same namespace the workspace is running in. - If secret is not found in the workspace namespace, the operator will look for the secret - in the namespace where the operator is running in. - as the DevWorkspaceOperatorCongfig. + If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + the operator will copy the secret from the operator namespace to the workspace namespace. The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be recognized by the operator. type: string + copyOperatorAuthSecret: + default: false + description: |- + CopyOperatorAuthSecret controls whether the operator should copy the authentication + secret from the operator namespace to the workspace namespace when it's not found + in the workspace namespace. + + If true: The operator will copy the secret from the operator namespace + if it's not found in the workspace namespace. This provides automatic configuration + but exposes operator-level credentials to workspace users. + + If false (default): The operator will not copy the secret. Users must manually create a secret + with the configured name in their workspace namespace. This is more secure as it allows + users to provide scoped credentials with minimal privileges. + + Note: Regardless of this setting, if a secret already exists in the workspace namespace, + it will never be overwritten. User-provided secrets are always respected. + + Defaults to false. + type: boolean path: description: |- A registry where backup images are stored. Images are stored diff --git a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 3319d3bdf..c3973e2b4 100644 --- a/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -218,12 +218,31 @@ spec: AuthSecret is the name of a Kubernetes secret of type kubernetes.io/dockerconfigjson. The secret is expected to be in the same namespace the workspace is running in. - If secret is not found in the workspace namespace, the operator will look for the secret - in the namespace where the operator is running in. - as the DevWorkspaceOperatorCongfig. + If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + the operator will copy the secret from the operator namespace to the workspace namespace. The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be recognized by the operator. type: string + copyOperatorAuthSecret: + default: false + description: |- + CopyOperatorAuthSecret controls whether the operator should copy the authentication + secret from the operator namespace to the workspace namespace when it's not found + in the workspace namespace. + + If true: The operator will copy the secret from the operator namespace + if it's not found in the workspace namespace. This provides automatic configuration + but exposes operator-level credentials to workspace users. + + If false (default): The operator will not copy the secret. Users must manually create a secret + with the configured name in their workspace namespace. This is more secure as it allows + users to provide scoped credentials with minimal privileges. + + Note: Regardless of this setting, if a secret already exists in the workspace namespace, + it will never be overwritten. User-provided secrets are always respected. + + Defaults to false. + type: boolean path: description: |- A registry where backup images are stored. Images are stored diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 448227d7f..0e0437d93 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -218,12 +218,31 @@ spec: AuthSecret is the name of a Kubernetes secret of type kubernetes.io/dockerconfigjson. The secret is expected to be in the same namespace the workspace is running in. - If secret is not found in the workspace namespace, the operator will look for the secret - in the namespace where the operator is running in. - as the DevWorkspaceOperatorCongfig. + If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + the operator will copy the secret from the operator namespace to the workspace namespace. The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be recognized by the operator. type: string + copyOperatorAuthSecret: + default: false + description: |- + CopyOperatorAuthSecret controls whether the operator should copy the authentication + secret from the operator namespace to the workspace namespace when it's not found + in the workspace namespace. + + If true: The operator will copy the secret from the operator namespace + if it's not found in the workspace namespace. This provides automatic configuration + but exposes operator-level credentials to workspace users. + + If false (default): The operator will not copy the secret. Users must manually create a secret + with the configured name in their workspace namespace. This is more secure as it allows + users to provide scoped credentials with minimal privileges. + + Note: Regardless of this setting, if a secret already exists in the workspace namespace, + it will never be overwritten. User-provided secrets are always respected. + + Defaults to false. + type: boolean path: description: |- A registry where backup images are stored. Images are stored diff --git a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml index 3319d3bdf..c3973e2b4 100644 --- a/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml +++ b/deploy/deployment/openshift/objects/devworkspaceoperatorconfigs.controller.devfile.io.CustomResourceDefinition.yaml @@ -218,12 +218,31 @@ spec: AuthSecret is the name of a Kubernetes secret of type kubernetes.io/dockerconfigjson. The secret is expected to be in the same namespace the workspace is running in. - If secret is not found in the workspace namespace, the operator will look for the secret - in the namespace where the operator is running in. - as the DevWorkspaceOperatorCongfig. + If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + the operator will copy the secret from the operator namespace to the workspace namespace. The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be recognized by the operator. type: string + copyOperatorAuthSecret: + default: false + description: |- + CopyOperatorAuthSecret controls whether the operator should copy the authentication + secret from the operator namespace to the workspace namespace when it's not found + in the workspace namespace. + + If true: The operator will copy the secret from the operator namespace + if it's not found in the workspace namespace. This provides automatic configuration + but exposes operator-level credentials to workspace users. + + If false (default): The operator will not copy the secret. Users must manually create a secret + with the configured name in their workspace namespace. This is more secure as it allows + users to provide scoped credentials with minimal privileges. + + Note: Regardless of this setting, if a secret already exists in the workspace namespace, + it will never be overwritten. User-provided secrets are always respected. + + Defaults to false. + type: boolean path: description: |- A registry where backup images are stored. Images are stored diff --git a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml index 78b1a9275..b6fa3f3b4 100644 --- a/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml +++ b/deploy/templates/crd/bases/controller.devfile.io_devworkspaceoperatorconfigs.yaml @@ -216,12 +216,31 @@ spec: AuthSecret is the name of a Kubernetes secret of type kubernetes.io/dockerconfigjson. The secret is expected to be in the same namespace the workspace is running in. - If secret is not found in the workspace namespace, the operator will look for the secret - in the namespace where the operator is running in. - as the DevWorkspaceOperatorCongfig. + If secret is not found in the workspace namespace and copyOperatorAuthSecret is true, + the operator will copy the secret from the operator namespace to the workspace namespace. The secret must contain "controller.devfile.io/watch-secret=true" label so that it can be recognized by the operator. type: string + copyOperatorAuthSecret: + default: false + description: |- + CopyOperatorAuthSecret controls whether the operator should copy the authentication + secret from the operator namespace to the workspace namespace when it's not found + in the workspace namespace. + + If true: The operator will copy the secret from the operator namespace + if it's not found in the workspace namespace. This provides automatic configuration + but exposes operator-level credentials to workspace users. + + If false (default): The operator will not copy the secret. Users must manually create a secret + with the configured name in their workspace namespace. This is more secure as it allows + users to provide scoped credentials with minimal privileges. + + Note: Regardless of this setting, if a secret already exists in the workspace namespace, + it will never be overwritten. User-provided secrets are always respected. + + Defaults to false. + type: boolean path: description: |- A registry where backup images are stored. Images are stored diff --git a/pkg/config/sync.go b/pkg/config/sync.go index bf474e986..84ca5f6dc 100644 --- a/pkg/config/sync.go +++ b/pkg/config/sync.go @@ -477,6 +477,9 @@ func mergeConfig(from, to *controller.OperatorConfiguration) { if from.Workspace.BackupCronJob.Registry.AuthSecret != "" { to.Workspace.BackupCronJob.Registry.AuthSecret = from.Workspace.BackupCronJob.Registry.AuthSecret } + if from.Workspace.BackupCronJob.Registry.CopyOperatorAuthSecret != nil { + to.Workspace.BackupCronJob.Registry.CopyOperatorAuthSecret = from.Workspace.BackupCronJob.Registry.CopyOperatorAuthSecret + } } if from.Workspace.BackupCronJob.OrasConfig != nil { if to.Workspace.BackupCronJob.OrasConfig == nil { diff --git a/pkg/secrets/backup.go b/pkg/secrets/backup.go index 1498a6a3e..e597faca8 100644 --- a/pkg/secrets/backup.go +++ b/pkg/secrets/backup.go @@ -24,12 +24,12 @@ import ( "github.com/devfile/devworkspace-operator/pkg/constants" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + k8sErrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - - "github.com/devfile/devworkspace-operator/pkg/provision/sync" ) // GetRegistryAuthSecret retrieves the registry authentication secret for accessing backup images @@ -54,8 +54,14 @@ func HandleRegistryAuthSecret(ctx context.Context, c client.Client, workspace *d return nil, nil } + // Extract flag value (default: false, users must provide their own secret) + copyOperatorAuthSecret := pointer.BoolDeref( + dwOperatorConfig.Workspace.BackupCronJob.Registry.CopyOperatorAuthSecret, + false, + ) + // On the restore path (operatorConfigNamespace == ""), look for the predefined name - // that CopySecret always uses. On the backup path, look for the configured name + // that EnsureRegistryAuthSecret always uses. On the backup path, look for the configured name // because the secret may exist directly in the workspace namespace under that name. lookupName := secretName if operatorConfigNamespace == "" { @@ -88,13 +94,46 @@ func HandleRegistryAuthSecret(ctx context.Context, c client.Client, workspace *d log.Error(err, "Failed to get registry auth secret for backup job", "secretName", secretName) return nil, err } - log.Info("Successfully retrieved registry auth secret for backup job", "secretName", secretName) - return CopySecret(ctx, c, workspace, registryAuthSecret, scheme, log) + log.Info("Successfully retrieved registry auth secret from operator namespace", "secretName", secretName) + return EnsureRegistryAuthSecret(ctx, c, workspace, registryAuthSecret, scheme, log, copyOperatorAuthSecret) } -// 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) { - // Construct the desired secret state +// EnsureRegistryAuthSecret copies the given secret from the operator namespace to the workspace namespace. +// If copyOperatorAuthSecret is false and the secret doesn't exist, it returns an error. +// If a secret already exists in the workspace namespace, it is never overwritten. +func EnsureRegistryAuthSecret(ctx context.Context, c client.Client, workspace *dw.DevWorkspace, sourceSecret *corev1.Secret, scheme *runtime.Scheme, log logr.Logger, copyOperatorAuthSecret bool) (namespaceSecret *corev1.Secret, err error) { + // Check if secret already exists (user-provided) + existingSecret := &corev1.Secret{} + err = c.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspace.Namespace, + }, existingSecret) + + if err == nil { + // User provided their own secret - always respect it, never overwrite + log.Info("Using existing registry auth secret from workspace namespace", + "secretName", constants.DevWorkspaceBackupAuthSecretName) + return existingSecret, nil + } + + if client.IgnoreNotFound(err) != nil { + return nil, err + } + + // Secret doesn't exist - check if we should copy + if !copyOperatorAuthSecret { + return nil, fmt.Errorf( + "registry auth secret %q not found in workspace namespace %q. "+ + "copyOperatorAuthSecret is set to false, so the operator will not copy the secret. "+ + "Please manually create a secret of type kubernetes.io/dockerconfigjson with the name %q "+ + "in the workspace namespace", + constants.DevWorkspaceBackupAuthSecretName, + workspace.Namespace, + constants.DevWorkspaceBackupAuthSecretName, + ) + } + + // copyOperatorAuthSecret is true - create it from operator's secret desiredSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: constants.DevWorkspaceBackupAuthSecretName, @@ -111,27 +150,27 @@ func CopySecret(ctx context.Context, c client.Client, workspace *dw.DevWorkspace return nil, err } - // Use the sync mechanism - clusterAPI := sync.ClusterAPI{ - Client: c, - Scheme: scheme, - Logger: log, - Ctx: ctx, - } - - syncedObj, err := sync.SyncObjectWithCluster(desiredSecret, clusterAPI) + // Use direct Create instead of SyncObjectWithCluster to avoid overwrites + err = c.Create(ctx, desiredSecret) if err != nil { - if _, ok := err.(*sync.NotInSyncError); !ok { - return nil, err + if k8sErrors.IsAlreadyExists(err) { + // Race condition - secret was created between Get and Create + // Fetch and return it (respect what's there) + if err := c.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspace.Namespace, + }, existingSecret); err != nil { + return nil, err + } + log.Info("Registry auth secret was created concurrently, using existing secret", + "secretName", constants.DevWorkspaceBackupAuthSecretName) + return existingSecret, nil } - // NotInSyncError means the sync operation was successful but triggered a change - log.Info("Successfully synced secret", "name", desiredSecret.Name, "namespace", workspace.Namespace) - } - - // If syncedObj is nil (due to NotInSyncError), return the desired object - if syncedObj == nil { - return desiredSecret, nil + return nil, err } - return syncedObj.(*corev1.Secret), nil + log.Info("Created registry auth secret from operator credentials", + "secretName", constants.DevWorkspaceBackupAuthSecretName, + "namespace", workspace.Namespace) + return desiredSecret, nil } diff --git a/pkg/secrets/backup_test.go b/pkg/secrets/backup_test.go index e89133b18..5c29db10b 100644 --- a/pkg/secrets/backup_test.go +++ b/pkg/secrets/backup_test.go @@ -75,6 +75,21 @@ func makeConfig(authSecretName string) *controllerv1alpha1.OperatorConfiguration } } +// makeConfigWithCopyFlag returns an OperatorConfiguration with copyOperatorAuthSecret flag set. +func makeConfigWithCopyFlag(authSecretName string, copyFlag *bool) *controllerv1alpha1.OperatorConfiguration { + return &controllerv1alpha1.OperatorConfiguration{ + Workspace: &controllerv1alpha1.WorkspaceConfig{ + BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{ + Registry: &controllerv1alpha1.RegistryConfig{ + Path: "example.registry.io/org", + AuthSecret: authSecretName, + CopyOperatorAuthSecret: copyFlag, + }, + }, + }, + } +} + // makeSecret returns a corev1.Secret with the given name and namespace. func makeSecret(name, namespace string) *corev1.Secret { return &corev1.Secret{ @@ -162,3 +177,268 @@ func (e *errorOnNameClient) Get(ctx context.Context, key client.ObjectKey, obj c // Ensure errorOnNameClient satisfies client.Client at compile time. var _ client.Client = &errorOnNameClient{} + +var _ = Describe("HandleRegistryAuthSecret (backup path: operatorConfigNamespace set)", func() { + const ( + workspaceNS = "user-namespace" + operatorNS = "devworkspace-controller" + secretName = "registry-auth-secret" + ) + + var ( + ctx context.Context + scheme *runtime.Scheme + log = zap.New(zap.UseDevMode(true)).WithName("SecretsTest") + ) + + BeforeEach(func() { + ctx = context.Background() + scheme = buildScheme() + }) + + Context("when copyOperatorAuthSecret is nil (default: false)", func() { + It("returns error when secret not found in workspace namespace", func() { + By("creating operator secret but no workspace secret") + operatorSecret := makeSecret(secretName, operatorNS) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + config := makeConfig(secretName) // copyOperatorAuthSecret is nil (default) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).To(HaveOccurred()) + Expect(result).To(BeNil()) + + By("verifying error message is clear and actionable") + Expect(err.Error()).To(ContainSubstring("copyOperatorAuthSecret is set to false")) + Expect(err.Error()).To(ContainSubstring("not found in workspace namespace")) + Expect(err.Error()).To(ContainSubstring("Please manually create")) + Expect(err.Error()).To(ContainSubstring("kubernetes.io/dockerconfigjson")) + }) + + It("returns existing workspace secret without overwriting", func() { + By("creating both operator secret and user-provided workspace secret with different data") + operatorSecret := makeSecret(secretName, operatorNS) + operatorSecret.Data = map[string][]byte{"auth": []byte("operator-creds")} + + userSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS) + userSecret.Data = map[string][]byte{"auth": []byte("user-creds")} + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret, userSecret).Build() + + workspace := makeWorkspace(workspaceNS) + config := makeConfig(secretName) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName)) + + By("verifying user's secret data is unchanged") + Expect(result.Data["auth"]).To(Equal([]byte("user-creds"))) + + By("verifying the secret in the cluster still has user's data") + fetchedSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspaceNS, + }, fetchedSecret) + Expect(err).NotTo(HaveOccurred()) + Expect(fetchedSecret.Data["auth"]).To(Equal([]byte("user-creds"))) + }) + }) + + Context("when copyOperatorAuthSecret is true", func() { + It("creates secret from operator namespace when not in workspace namespace", func() { + By("creating operator secret but no workspace secret") + operatorSecret := makeSecret(secretName, operatorNS) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + trueVal := true + config := makeConfigWithCopyFlag(secretName, &trueVal) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName)) + Expect(result.Namespace).To(Equal(workspaceNS)) + + By("verifying the secret was created in workspace namespace") + createdSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspaceNS, + }, createdSecret) + Expect(err).NotTo(HaveOccurred()) + Expect(createdSecret.Data).To(Equal(operatorSecret.Data)) + }) + + It("returns existing workspace secret without overwriting", func() { + By("creating both operator secret and user-provided workspace secret with different data") + operatorSecret := makeSecret(secretName, operatorNS) + operatorSecret.Data = map[string][]byte{"auth": []byte("operator-creds")} + + userSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS) + userSecret.Data = map[string][]byte{"auth": []byte("user-creds")} + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret, userSecret).Build() + + workspace := makeWorkspace(workspaceNS) + trueVal := true + config := makeConfigWithCopyFlag(secretName, &trueVal) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + + By("verifying user's secret data is unchanged") + Expect(result.Data["auth"]).To(Equal([]byte("user-creds"))) + }) + }) + + Context("when copyOperatorAuthSecret is false", func() { + It("returns error when secret not found in workspace namespace", func() { + By("creating operator secret but no workspace secret") + operatorSecret := makeSecret(secretName, operatorNS) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + falseVal := false + config := makeConfigWithCopyFlag(secretName, &falseVal) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).To(HaveOccurred()) + Expect(result).To(BeNil()) + + By("verifying error message is clear and actionable") + Expect(err.Error()).To(ContainSubstring("copyOperatorAuthSecret is set to false")) + Expect(err.Error()).To(ContainSubstring("not found in workspace namespace")) + Expect(err.Error()).To(ContainSubstring("Please manually create")) + Expect(err.Error()).To(ContainSubstring("kubernetes.io/dockerconfigjson")) + }) + + It("returns existing workspace secret when present", func() { + By("creating both operator secret and user-provided workspace secret") + operatorSecret := makeSecret(secretName, operatorNS) + operatorSecret.Data = map[string][]byte{"auth": []byte("operator-creds")} + + userSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS) + userSecret.Data = map[string][]byte{"auth": []byte("user-creds")} + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret, userSecret).Build() + + workspace := makeWorkspace(workspaceNS) + falseVal := false + config := makeConfigWithCopyFlag(secretName, &falseVal) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + + By("verifying user's secret is returned") + Expect(result.Data["auth"]).To(Equal([]byte("user-creds"))) + }) + + It("does not create secret even if operator secret exists", func() { + By("creating operator secret but no workspace secret") + operatorSecret := makeSecret(secretName, operatorNS) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + falseVal := false + config := makeConfigWithCopyFlag(secretName, &falseVal) + + _, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).To(HaveOccurred()) + + By("verifying no secret was created in workspace namespace") + createdSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspaceNS, + }, createdSecret) + Expect(k8sErrors.IsNotFound(err)).To(BeTrue()) + }) + }) + + Context("secret creation edge cases", func() { + It("sets proper labels on created secret", func() { + By("creating operator secret") + operatorSecret := makeSecret(secretName, operatorNS) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + trueVal := true + config := makeConfigWithCopyFlag(secretName, &trueVal) + + _, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + + By("verifying the created secret has the watch label") + createdSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspaceNS, + }, createdSecret) + Expect(err).NotTo(HaveOccurred()) + Expect(createdSecret.Labels).To(HaveKey(constants.DevWorkspaceWatchSecretLabel)) + Expect(createdSecret.Labels[constants.DevWorkspaceWatchSecretLabel]).To(Equal("true")) + }) + + It("sets controller reference on created secret", func() { + By("creating operator secret") + operatorSecret := makeSecret(secretName, operatorNS) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + trueVal := true + config := makeConfigWithCopyFlag(secretName, &trueVal) + + _, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + + By("verifying the created secret has owner reference to workspace") + createdSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspaceNS, + }, createdSecret) + Expect(err).NotTo(HaveOccurred()) + Expect(createdSecret.OwnerReferences).To(HaveLen(1)) + Expect(createdSecret.OwnerReferences[0].Name).To(Equal(workspace.Name)) + Expect(createdSecret.OwnerReferences[0].Kind).To(Equal("DevWorkspace")) + }) + }) + + Context("when secret exists in workspace namespace with configured name", func() { + It("returns the existing secret without copying from operator namespace", func() { + By("creating a secret in workspace namespace with the configured name") + workspaceSecret := makeSecret(secretName, workspaceNS) + workspaceSecret.Data = map[string][]byte{"auth": []byte("workspace-specific-creds")} + + operatorSecret := makeSecret(secretName, operatorNS) + operatorSecret.Data = map[string][]byte{"auth": []byte("operator-creds")} + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(workspaceSecret, operatorSecret).Build() + + workspace := makeWorkspace(workspaceNS) + config := makeConfig(secretName) + + result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, operatorNS, scheme, log) + Expect(err).NotTo(HaveOccurred()) + Expect(result).NotTo(BeNil()) + Expect(result.Name).To(Equal(secretName)) + Expect(result.Namespace).To(Equal(workspaceNS)) + Expect(result.Data["auth"]).To(Equal([]byte("workspace-specific-creds"))) + + By("verifying no secret with predefined name was created") + predefinedSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, client.ObjectKey{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: workspaceNS, + }, predefinedSecret) + Expect(k8sErrors.IsNotFound(err)).To(BeTrue()) + }) + }) +})