Skip to content

Commit 4048446

Browse files
dkoshkindkoshkin
authored
fix: skip kubeadm CA file when Secret doesn't have a CA (#752)
**What problem does this PR solve?**: Found a bug when testing with a mirror registry where it doesn't have the CA key set in the Secret. The handler incorrectly generated a patch to add `/etc/certs/...` file with a Secret source and key `ca.crt`, however its possible that the Secret does not have that key set. Example of an incorrect file: ``` - contentFrom: secret: key: ca.crt name: e2e-aws-ag-cc-test-1719374309-image-registry-mirror-credentials path: /etc/certs/999867407951.dkr.ecr.us-west-2.amazonaws.com-e2e-aws-ag-cc-test-1719374309.pem permissions: "0600" ``` This led to CAPI failing because `ca.crt` did not exist in the Secret. **Which issue(s) this PR fixes**: Fixes # **How Has This Been Tested?**: <!-- Please describe the tests that you ran to verify your changes. Provide output from the tests and any manual steps needed to replicate the tests. --> The first commit contains a failing unit tests to cover this scenario. **Special notes for your reviewer**: <!-- Use this to provide any additional information to the reviewers. This may include: - Best way to review the PR. - Where the author wants the most review attention on. - etc. -->
1 parent 04dce16 commit 4048446

File tree

3 files changed

+206
-22
lines changed

3 files changed

+206
-22
lines changed

‎pkg/handlers/generic/mutation/mirrors/containerd_files.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import (
1919

2020
const (
2121
containerdHostsConfigurationOnRemote = "/etc/containerd/certs.d/_default/hosts.toml"
22-
secretKeyForMirrorCACert = "ca.crt"
22+
secretKeyForCACert = "ca.crt"
2323
)
2424

2525
var (
@@ -159,7 +159,7 @@ func generateRegistryCACertFiles(
159159
ContentFrom: &cabpkv1.FileSource{
160160
Secret: cabpkv1.SecretFileSource{
161161
Name: config.CASecretName,
162-
Key: secretKeyForMirrorCACert,
162+
Key: secretKeyForCACert,
163163
},
164164
},
165165
})

‎pkg/handlers/generic/mutation/mirrors/inject.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"fmt"
99

10+
corev1 "k8s.io/api/core/v1"
1011
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
1112
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1213
"k8s.io/apimachinery/pkg/runtime"
@@ -193,9 +194,9 @@ func containerdConfigFromGlobalMirror(
193194
)
194195
}
195196

196-
if secret != nil {
197+
if secretHasCACert(secret) {
197198
configWithOptionalCACert.CASecretName = secret.Name
198-
configWithOptionalCACert.CACert = string(secret.Data[secretKeyForMirrorCACert])
199+
configWithOptionalCACert.CACert = string(secret.Data[secretKeyForCACert])
199200
}
200201

201202
return configWithOptionalCACert, nil
@@ -225,9 +226,9 @@ func containerdConfigFromImageRegistry(
225226
)
226227
}
227228

228-
if secret != nil {
229+
if secretHasCACert(secret) {
229230
configWithOptionalCACert.CASecretName = secret.Name
230-
configWithOptionalCACert.CACert = string(secret.Data[secretKeyForMirrorCACert])
231+
configWithOptionalCACert.CACert = string(secret.Data[secretKeyForCACert])
231232
}
232233

233234
return configWithOptionalCACert, nil
@@ -271,3 +272,12 @@ func needContainerdConfiguration(configs []containerdConfig) bool {
271272

272273
return false
273274
}
275+
276+
func secretHasCACert(secret *corev1.Secret) bool {
277+
if secret == nil {
278+
return false
279+
}
280+
281+
_, ok := secret.Data[secretKeyForCACert]
282+
return ok
283+
}

‎pkg/handlers/generic/mutation/mirrors/inject_test.go

Lines changed: 190 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,8 @@ import (
2222
)
2323

2424
const (
25-
validMirrorCASecretName = "myregistry-mirror-cacert"
26-
//nolint:gosec // Does not contain hard coded credentials.
27-
cpRegistryAsMirrorCreds = "kubeadmControlPlaneRegistryAsMirrorCreds"
28-
//nolint:gosec // Does not contain hard coded credentials.
29-
workerRegistryAsMirrorCreds = "kubeadmConfigTemplateRegistryAsMirrorCreds"
25+
validMirrorCASecretName = "myregistry-mirror-cacert"
26+
validMirrorNoCASecretName = "myregistry-mirror-no-cacert"
3027
)
3128

3229
func TestMirrorsPatch(t *testing.T) {
@@ -50,7 +47,7 @@ var _ = Describe("Generate Global mirror patches", func() {
5047

5148
testDefs := []capitest.PatchTestDef{
5249
{
53-
Name: "files added in KubeadmControlPlaneTemplate for registry with mirror without CA Certificate",
50+
Name: "files added in KubeadmControlPlaneTemplate for registry with mirror without CA Certificate secret",
5451
Vars: []runtimehooksv1.Variable{
5552
capitest.VariableWithValue(
5653
v1alpha1.ClusterConfigVariableName,
@@ -65,7 +62,7 @@ var _ = Describe("Generate Global mirror patches", func() {
6562
{
6663
Operation: "add",
6764
Path: "/spec/template/spec/kubeadmConfigSpec/files",
68-
ValueMatcher: gomega.ContainElements(
65+
ValueMatcher: gomega.HaveExactElements(
6966
gomega.HaveKeyWithValue(
7067
"path", "/etc/containerd/certs.d/_default/hosts.toml",
7168
),
@@ -92,12 +89,12 @@ var _ = Describe("Generate Global mirror patches", func() {
9289
v1alpha1.GlobalMirrorVariableName,
9390
),
9491
},
95-
RequestItem: request.NewKubeadmControlPlaneTemplateRequest("", cpRegistryAsMirrorCreds),
92+
RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""),
9693
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
9794
{
9895
Operation: "add",
9996
Path: "/spec/template/spec/kubeadmConfigSpec/files",
100-
ValueMatcher: gomega.ContainElements(
97+
ValueMatcher: gomega.HaveExactElements(
10198
gomega.HaveKeyWithValue(
10299
"path", "/etc/containerd/certs.d/_default/hosts.toml",
103100
),
@@ -112,7 +109,74 @@ var _ = Describe("Generate Global mirror patches", func() {
112109
},
113110
},
114111
{
115-
Name: "files added in KubeadmConfigTemplate for registry mirror wihthout CA certificate",
112+
Name: "files added in KubeadmControlPlaneTemplate for registry mirror with secret but missing CA certificate key",
113+
Vars: []runtimehooksv1.Variable{
114+
capitest.VariableWithValue(
115+
v1alpha1.ClusterConfigVariableName,
116+
v1alpha1.GlobalImageRegistryMirror{
117+
URL: "https://registry.example.com",
118+
Credentials: &v1alpha1.RegistryCredentials{
119+
SecretRef: &v1alpha1.LocalObjectReference{
120+
Name: validMirrorNoCASecretName,
121+
},
122+
},
123+
},
124+
v1alpha1.GlobalMirrorVariableName,
125+
),
126+
},
127+
RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""),
128+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
129+
{
130+
Operation: "add",
131+
Path: "/spec/template/spec/kubeadmConfigSpec/files",
132+
ValueMatcher: gomega.HaveExactElements(
133+
gomega.HaveKeyWithValue(
134+
"path", "/etc/containerd/certs.d/_default/hosts.toml",
135+
),
136+
gomega.HaveKeyWithValue(
137+
"path", "/etc/caren/containerd/patches/registry-config.toml",
138+
),
139+
),
140+
},
141+
},
142+
},
143+
{
144+
Name: "files added in KubeadmControlPlaneTemplate for image registry with CA Certificate secret",
145+
Vars: []runtimehooksv1.Variable{
146+
capitest.VariableWithValue(
147+
v1alpha1.ClusterConfigVariableName,
148+
[]v1alpha1.ImageRegistry{{
149+
URL: "https://registry.example.com",
150+
Credentials: &v1alpha1.RegistryCredentials{
151+
SecretRef: &v1alpha1.LocalObjectReference{
152+
Name: validMirrorCASecretName,
153+
},
154+
},
155+
}},
156+
v1alpha1.ImageRegistriesVariableName,
157+
),
158+
},
159+
RequestItem: request.NewKubeadmControlPlaneTemplateRequestItem(""),
160+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
161+
{
162+
Operation: "add",
163+
Path: "/spec/template/spec/kubeadmConfigSpec/files",
164+
ValueMatcher: gomega.HaveExactElements(
165+
gomega.HaveKeyWithValue(
166+
"path", "/etc/containerd/certs.d/_default/hosts.toml",
167+
),
168+
gomega.HaveKeyWithValue(
169+
"path", "/etc/certs/registry.example.com.pem",
170+
),
171+
gomega.HaveKeyWithValue(
172+
"path", "/etc/caren/containerd/patches/registry-config.toml",
173+
),
174+
),
175+
},
176+
},
177+
},
178+
{
179+
Name: "files added in KubeadmConfigTemplate for registry mirror without CA certificate secret",
116180
Vars: []runtimehooksv1.Variable{
117181
capitest.VariableWithValue(
118182
v1alpha1.ClusterConfigVariableName,
@@ -135,7 +199,7 @@ var _ = Describe("Generate Global mirror patches", func() {
135199
{
136200
Operation: "add",
137201
Path: "/spec/template/spec/files",
138-
ValueMatcher: gomega.ContainElements(
202+
ValueMatcher: gomega.HaveExactElements(
139203
gomega.HaveKeyWithValue(
140204
"path", "/etc/containerd/certs.d/_default/hosts.toml",
141205
),
@@ -170,12 +234,95 @@ var _ = Describe("Generate Global mirror patches", func() {
170234
},
171235
),
172236
},
173-
RequestItem: request.NewKubeadmConfigTemplateRequest("", workerRegistryAsMirrorCreds),
237+
RequestItem: request.NewKubeadmConfigTemplateRequestItem(""),
238+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
239+
{
240+
Operation: "add",
241+
Path: "/spec/template/spec/files",
242+
ValueMatcher: gomega.HaveExactElements(
243+
gomega.HaveKeyWithValue(
244+
"path", "/etc/containerd/certs.d/_default/hosts.toml",
245+
),
246+
gomega.HaveKeyWithValue(
247+
"path", "/etc/certs/registry.example.com.pem",
248+
),
249+
gomega.HaveKeyWithValue(
250+
"path", "/etc/caren/containerd/patches/registry-config.toml",
251+
),
252+
),
253+
},
254+
},
255+
},
256+
{
257+
Name: "files added in KubeadmConfigTemplate for registry mirror with secret but missing CA certificate key",
258+
Vars: []runtimehooksv1.Variable{
259+
capitest.VariableWithValue(
260+
v1alpha1.ClusterConfigVariableName,
261+
v1alpha1.GlobalImageRegistryMirror{
262+
URL: "https://registry.example.com",
263+
Credentials: &v1alpha1.RegistryCredentials{
264+
SecretRef: &v1alpha1.LocalObjectReference{
265+
Name: validMirrorNoCASecretName,
266+
},
267+
},
268+
},
269+
v1alpha1.GlobalMirrorVariableName,
270+
),
271+
capitest.VariableWithValue(
272+
"builtin",
273+
map[string]any{
274+
"machineDeployment": map[string]any{
275+
"class": names.SimpleNameGenerator.GenerateName("worker-"),
276+
},
277+
},
278+
),
279+
},
280+
RequestItem: request.NewKubeadmConfigTemplateRequestItem(""),
174281
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
175282
{
176283
Operation: "add",
177284
Path: "/spec/template/spec/files",
178-
ValueMatcher: gomega.ContainElements(
285+
ValueMatcher: gomega.HaveExactElements(
286+
gomega.HaveKeyWithValue(
287+
"path", "/etc/containerd/certs.d/_default/hosts.toml",
288+
),
289+
gomega.HaveKeyWithValue(
290+
"path", "/etc/caren/containerd/patches/registry-config.toml",
291+
),
292+
),
293+
},
294+
},
295+
},
296+
{
297+
Name: "files added in KubeadmConfigTemplate for image registry with secret for CA certificate",
298+
Vars: []runtimehooksv1.Variable{
299+
capitest.VariableWithValue(
300+
v1alpha1.ClusterConfigVariableName,
301+
[]v1alpha1.ImageRegistry{{
302+
URL: "https://registry.example.com",
303+
Credentials: &v1alpha1.RegistryCredentials{
304+
SecretRef: &v1alpha1.LocalObjectReference{
305+
Name: validMirrorCASecretName,
306+
},
307+
},
308+
}},
309+
v1alpha1.ImageRegistriesVariableName,
310+
),
311+
capitest.VariableWithValue(
312+
"builtin",
313+
map[string]any{
314+
"machineDeployment": map[string]any{
315+
"class": names.SimpleNameGenerator.GenerateName("worker-"),
316+
},
317+
},
318+
),
319+
},
320+
RequestItem: request.NewKubeadmConfigTemplateRequestItem(""),
321+
ExpectedPatchMatchers: []capitest.JSONPatchMatcher{
322+
{
323+
Operation: "add",
324+
Path: "/spec/template/spec/files",
325+
ValueMatcher: gomega.HaveExactElements(
179326
gomega.HaveKeyWithValue(
180327
"path", "/etc/containerd/certs.d/_default/hosts.toml",
181328
),
@@ -197,7 +344,11 @@ var _ = Describe("Generate Global mirror patches", func() {
197344
gomega.Expect(err).To(gomega.BeNil())
198345
gomega.Expect(client.Create(
199346
ctx,
200-
newMirrorSecret(validMirrorCASecretName, request.Namespace),
347+
newMirrorSecretWithCA(validMirrorCASecretName, request.Namespace),
348+
)).To(gomega.BeNil())
349+
gomega.Expect(client.Create(
350+
ctx,
351+
newMirrorSecretWithoutCA(validMirrorNoCASecretName, request.Namespace),
201352
)).To(gomega.BeNil())
202353
})
203354

@@ -207,7 +358,11 @@ var _ = Describe("Generate Global mirror patches", func() {
207358
gomega.Expect(err).To(gomega.BeNil())
208359
gomega.Expect(client.Delete(
209360
ctx,
210-
newMirrorSecret(validMirrorCASecretName, request.Namespace),
361+
newMirrorSecretWithCA(validMirrorCASecretName, request.Namespace),
362+
)).To(gomega.BeNil())
363+
gomega.Expect(client.Delete(
364+
ctx,
365+
newMirrorSecretWithoutCA(validMirrorNoCASecretName, request.Namespace),
211366
)).To(gomega.BeNil())
212367
})
213368

@@ -220,7 +375,7 @@ var _ = Describe("Generate Global mirror patches", func() {
220375
}
221376
})
222377

223-
func newMirrorSecret(name, namespace string) *corev1.Secret {
378+
func newMirrorSecretWithCA(name, namespace string) *corev1.Secret {
224379
secretData := map[string][]byte{
225380
"ca.crt": []byte("myCACert"),
226381
}
@@ -238,6 +393,25 @@ func newMirrorSecret(name, namespace string) *corev1.Secret {
238393
}
239394
}
240395

396+
func newMirrorSecretWithoutCA(name, namespace string) *corev1.Secret {
397+
secretData := map[string][]byte{
398+
"username": []byte("user"),
399+
"password": []byte("pass"),
400+
}
401+
return &corev1.Secret{
402+
TypeMeta: metav1.TypeMeta{
403+
APIVersion: "v1",
404+
Kind: "Secret",
405+
},
406+
ObjectMeta: metav1.ObjectMeta{
407+
Name: name,
408+
Namespace: namespace,
409+
},
410+
Data: secretData,
411+
Type: corev1.SecretTypeOpaque,
412+
}
413+
}
414+
241415
func Test_needContainerdConfiguration(t *testing.T) {
242416
t.Parallel()
243417
tests := []struct {

0 commit comments

Comments
 (0)