-
Notifications
You must be signed in to change notification settings - Fork 69
fix: restore init container uses configured registry auth secret name #1615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
akurinnoy
wants to merge
2
commits into
devfile:main
Choose a base branch
from
akurinnoy:fix/restore-auth-secret-name
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+272
−15
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)) | ||
| }) | ||
| }) | ||
| }) | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, test description a bit inaccurate.