Skip to content

Commit c9f7339

Browse files
committed
fix: fall back to canonical backup auth secret name on restore
When CopySecret writes an auth secret into the workspace namespace, it always uses DevWorkspaceBackupAuthSecretName ("devworkspace-backup-registry-auth") regardless of what name the admin configured (e.g. "quay-backup-auth"). GetNamespaceRegistryAuthSecret (the restore path) calls HandleRegistryAuthSecret with operatorConfigNamespace="" and looked only for the configured name, found nothing, and returned nil — leaving the workspace-restore init container without credentials and causing CrashLoopBackOff on private registries. Fix: when operatorConfigNamespace is empty and the configured name is not found, attempt a second Get using the canonical DevWorkspaceBackupAuthSecretName. Skip the extra lookup when the configured name already equals the canonical name to avoid a redundant API call. Propagate any non-NotFound error from the fallback. Assisted-by: Claude Sonnet 4.6 (1M context) Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
1 parent fcee0cd commit c9f7339

File tree

2 files changed

+224
-1
lines changed

2 files changed

+224
-1
lines changed

pkg/secrets/backup.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,30 @@ func HandleRegistryAuthSecret(ctx context.Context, c client.Client, workspace *d
6666
if client.IgnoreNotFound(err) != nil {
6767
return nil, err
6868
}
69-
// If we don't provide an operator namespace, don't attempt to look there
69+
// If we don't provide an operator namespace, don't attempt to look there.
70+
// However, CopySecret always writes the secret under DevWorkspaceBackupAuthSecretName,
71+
// regardless of the configured name. If the configured name differs, attempt a fallback
72+
// lookup under the canonical name so that the restore path can find it.
7073
if operatorConfigNamespace == "" {
74+
if secretName == constants.DevWorkspaceBackupAuthSecretName {
75+
// The configured name IS the canonical name; it was already looked up and not
76+
// found above, so there is nothing else to try.
77+
return nil, nil
78+
}
79+
fallbackSecret := &corev1.Secret{}
80+
fallbackErr := c.Get(ctx, client.ObjectKey{
81+
Name: constants.DevWorkspaceBackupAuthSecretName,
82+
Namespace: workspace.Namespace,
83+
}, fallbackSecret)
84+
if fallbackErr == nil {
85+
log.Info("Registry auth secret not found under configured name in workspace namespace; using canonical backup auth secret as fallback",
86+
"configuredName", secretName,
87+
"fallbackName", constants.DevWorkspaceBackupAuthSecretName)
88+
return fallbackSecret, nil
89+
}
90+
if client.IgnoreNotFound(fallbackErr) != nil {
91+
return nil, fallbackErr
92+
}
7193
return nil, nil
7294
}
7395
log.Info("Registry auth secret not found in workspace namespace, checking operator namespace", "secretName", secretName)

pkg/secrets/backup_test.go

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
//
2+
// Copyright (c) 2019-2026 Red Hat, Inc.
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
//
15+
16+
package secrets_test
17+
18+
import (
19+
"context"
20+
"errors"
21+
"testing"
22+
23+
. "github.com/onsi/ginkgo/v2"
24+
. "github.com/onsi/gomega"
25+
26+
dwv2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
27+
controllerv1alpha1 "github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
28+
corev1 "k8s.io/api/core/v1"
29+
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
30+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
31+
"k8s.io/apimachinery/pkg/runtime"
32+
"k8s.io/apimachinery/pkg/runtime/schema"
33+
"sigs.k8s.io/controller-runtime/pkg/client"
34+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
35+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
36+
37+
"github.com/devfile/devworkspace-operator/pkg/constants"
38+
"github.com/devfile/devworkspace-operator/pkg/secrets"
39+
)
40+
41+
func TestSecrets(t *testing.T) {
42+
RegisterFailHandler(Fail)
43+
RunSpecs(t, "Secrets Suite")
44+
}
45+
46+
// buildScheme returns a minimal runtime.Scheme for tests: core v1 and the DWO API types.
47+
func buildScheme() *runtime.Scheme {
48+
scheme := runtime.NewScheme()
49+
Expect(corev1.AddToScheme(scheme)).To(Succeed())
50+
Expect(dwv2.AddToScheme(scheme)).To(Succeed())
51+
Expect(controllerv1alpha1.AddToScheme(scheme)).To(Succeed())
52+
return scheme
53+
}
54+
55+
// makeWorkspace returns a minimal DevWorkspace in the given namespace.
56+
func makeWorkspace(namespace string) *dwv2.DevWorkspace {
57+
return &dwv2.DevWorkspace{
58+
ObjectMeta: metav1.ObjectMeta{
59+
Name: "test-workspace",
60+
Namespace: namespace,
61+
},
62+
}
63+
}
64+
65+
// makeConfig returns an OperatorConfiguration with BackupCronJob configured to use the given auth secret name.
66+
func makeConfig(authSecretName string) *controllerv1alpha1.OperatorConfiguration {
67+
return &controllerv1alpha1.OperatorConfiguration{
68+
Workspace: &controllerv1alpha1.WorkspaceConfig{
69+
BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
70+
Registry: &controllerv1alpha1.RegistryConfig{
71+
Path: "example.registry.io/org",
72+
AuthSecret: authSecretName,
73+
},
74+
},
75+
},
76+
}
77+
}
78+
79+
// makeSecret returns a corev1.Secret with the given name and namespace.
80+
func makeSecret(name, namespace string) *corev1.Secret {
81+
return &corev1.Secret{
82+
ObjectMeta: metav1.ObjectMeta{
83+
Name: name,
84+
Namespace: namespace,
85+
},
86+
Data: map[string][]byte{"auth": []byte("dXNlcjpwYXNz")},
87+
Type: corev1.SecretTypeDockerConfigJson,
88+
}
89+
}
90+
91+
var _ = Describe("HandleRegistryAuthSecret (restore path: operatorConfigNamespace='')", func() {
92+
const workspaceNS = "user-namespace"
93+
94+
var (
95+
ctx context.Context
96+
scheme *runtime.Scheme
97+
log = zap.New(zap.UseDevMode(true)).WithName("SecretsTest")
98+
)
99+
100+
BeforeEach(func() {
101+
ctx = context.Background()
102+
scheme = buildScheme()
103+
})
104+
105+
It("returns the secret directly when the configured name is present in the workspace namespace", func() {
106+
By("creating a workspace-namespace secret whose name matches the configured auth secret name")
107+
configuredName := "quay-backup-auth"
108+
secret := makeSecret(configuredName, workspaceNS)
109+
110+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret).Build()
111+
workspace := makeWorkspace(workspaceNS)
112+
config := makeConfig(configuredName)
113+
114+
result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
115+
Expect(err).NotTo(HaveOccurred())
116+
Expect(result).NotTo(BeNil())
117+
Expect(result.Name).To(Equal(configuredName))
118+
})
119+
120+
It("returns the canonical backup auth secret as fallback when configured name is absent (the bug fix case)", func() {
121+
By("creating only the canonical DevWorkspaceBackupAuthSecretName secret in the workspace namespace")
122+
canonicalSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS)
123+
124+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(canonicalSecret).Build()
125+
workspace := makeWorkspace(workspaceNS)
126+
// Configured name is something like "quay-backup-auth" — different from the canonical name.
127+
config := makeConfig("quay-backup-auth")
128+
129+
result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
130+
Expect(err).NotTo(HaveOccurred())
131+
Expect(result).NotTo(BeNil(), "should fall back to the canonical secret copied by CopySecret")
132+
Expect(result.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName))
133+
})
134+
135+
It("returns nil, nil when neither the configured name nor the canonical name exists in the workspace namespace", func() {
136+
By("using a fake client with no secrets at all")
137+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build()
138+
workspace := makeWorkspace(workspaceNS)
139+
config := makeConfig("quay-backup-auth")
140+
141+
result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
142+
Expect(err).NotTo(HaveOccurred())
143+
Expect(result).To(BeNil())
144+
})
145+
146+
It("returns the secret on the first lookup when the configured name IS the canonical name (no duplicate query)", func() {
147+
By("creating the canonical secret in the workspace namespace and configuring the same name")
148+
canonicalSecret := makeSecret(constants.DevWorkspaceBackupAuthSecretName, workspaceNS)
149+
150+
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(canonicalSecret).Build()
151+
workspace := makeWorkspace(workspaceNS)
152+
// Configured name equals the canonical constant — must be found on the first Get.
153+
config := makeConfig(constants.DevWorkspaceBackupAuthSecretName)
154+
155+
result, err := secrets.HandleRegistryAuthSecret(ctx, fakeClient, workspace, config, "", scheme, log)
156+
Expect(err).NotTo(HaveOccurred())
157+
Expect(result).NotTo(BeNil())
158+
Expect(result.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName))
159+
})
160+
161+
It("propagates a real (non-NotFound) error from the fallback lookup", func() {
162+
By("wrapping a fake client so that the fallback lookup returns a server error")
163+
// The configured name differs from the canonical name, so the code will attempt the fallback
164+
// lookup. We simulate that lookup returning a non-NotFound error.
165+
errClient := &errorOnNameClient{
166+
Client: fake.NewClientBuilder().WithScheme(scheme).Build(),
167+
failName: constants.DevWorkspaceBackupAuthSecretName,
168+
failErr: k8sErrors.NewInternalError(errors.New("simulated etcd timeout")),
169+
}
170+
workspace := makeWorkspace(workspaceNS)
171+
config := makeConfig("quay-backup-auth")
172+
173+
result, err := secrets.HandleRegistryAuthSecret(ctx, errClient, workspace, config, "", scheme, log)
174+
Expect(err).To(HaveOccurred())
175+
Expect(result).To(BeNil())
176+
Expect(err.Error()).To(ContainSubstring("simulated etcd timeout"))
177+
})
178+
})
179+
180+
// errorOnNameClient is a thin client wrapper that injects an error for a specific secret name.
181+
type errorOnNameClient struct {
182+
client.Client
183+
failName string
184+
failErr error
185+
}
186+
187+
func (e *errorOnNameClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error {
188+
if secret, ok := obj.(*corev1.Secret); ok {
189+
_ = secret
190+
if key.Name == e.failName {
191+
return e.failErr
192+
}
193+
}
194+
return e.Client.Get(ctx, key, obj, opts...)
195+
}
196+
197+
// Ensure errorOnNameClient satisfies client.Client at compile time.
198+
var _ client.Client = &errorOnNameClient{}
199+
200+
// secretGR is the GroupResource for secrets, used when constructing test errors.
201+
var secretGR = schema.GroupResource{Group: "", Resource: "secrets"} //nolint:unused

0 commit comments

Comments
 (0)