Skip to content

Commit 14c2d07

Browse files
sdowellsdowell
committed
fix: use correct path to PodSpec for CronJob
CronJob has a different path to the PodSpec from the other currently supported types of ImageRegistryTransform. This fixes the image transform logic to find the correct path.
1 parent 83bd9c0 commit 14c2d07

File tree

3 files changed

+299
-6
lines changed

3 files changed

+299
-6
lines changed

‎pkg/patterns/declarative/image.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
// ImageRegistryTransform modifies all Pods to use registry for the image source and adds the imagePullSecret
3030
func ImageRegistryTransform(registry, imagePullSecret string) ObjectTransform {
3131
return func(c context.Context, o DeclarativeObject, m *manifest.Objects) error {
32-
return applyImageRegistry(c, o, m, registry, imagePullSecret, applyPrivateRegistryToImage)
32+
return applyImageRegistry(c, m, registry, imagePullSecret, applyPrivateRegistryToImage)
3333
}
3434
}
3535

@@ -38,11 +38,11 @@ type ImageFunc func(registry, image string) string
3838
// PrivateRegistryTransform modifies all Pods to use registry for the image source and adds the imagePullSecret
3939
func PrivateRegistryTransform(registry, imagePullSecret string, imageFunc ImageFunc) ObjectTransform {
4040
return func(c context.Context, o DeclarativeObject, m *manifest.Objects) error {
41-
return applyImageRegistry(c, o, m, registry, imagePullSecret, imageFunc)
41+
return applyImageRegistry(c, m, registry, imagePullSecret, imageFunc)
4242
}
4343
}
4444

45-
func applyImageRegistry(ctx context.Context, operatorObject DeclarativeObject, manifest *manifest.Objects, registry, secret string, imageFunc ImageFunc) error {
45+
func applyImageRegistry(ctx context.Context, manifest *manifest.Objects, registry, secret string, imageFunc ImageFunc) error {
4646
log := log.FromContext(ctx)
4747
if registry == "" && secret == "" {
4848
return nil

‎pkg/patterns/declarative/image_test.go

+280
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,280 @@
1+
/*
2+
Copyright 2024 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package declarative
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
"github.com/google/go-cmp/cmp"
24+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
25+
"sigs.k8s.io/kubebuilder-declarative-pattern/pkg/patterns/declarative/pkg/manifest"
26+
)
27+
28+
func Test_ImageRegistryTransform(t *testing.T) {
29+
inputManifest := `---
30+
apiVersion: apps/v1
31+
kind: Deployment
32+
metadata:
33+
labels:
34+
app: test-app
35+
name: frontend
36+
spec:
37+
replicas: 1
38+
selector:
39+
matchLabels:
40+
app: test-app
41+
strategy: {}
42+
template:
43+
metadata:
44+
labels:
45+
app: test-app
46+
spec:
47+
containers:
48+
- image: busybox
49+
name: busybox
50+
---
51+
apiVersion: batch/v1
52+
kind: CronJob
53+
metadata:
54+
name: hello
55+
spec:
56+
schedule: "* * * * *"
57+
jobTemplate:
58+
spec:
59+
template:
60+
spec:
61+
containers:
62+
- name: hello
63+
image: busybox:1.28
64+
imagePullPolicy: IfNotPresent
65+
command:
66+
- /bin/sh
67+
- -c
68+
- date; echo Hello from the Kubernetes cluster
69+
restartPolicy: OnFailure`
70+
var testCases = []struct {
71+
name string
72+
registry string
73+
imagePullSecret string
74+
inputManifest string
75+
expectedManifest string
76+
}{
77+
{
78+
name: "replace registry only",
79+
registry: "gcr.io/foo/bar",
80+
imagePullSecret: "",
81+
inputManifest: inputManifest,
82+
expectedManifest: `---
83+
apiVersion: apps/v1
84+
kind: Deployment
85+
metadata:
86+
labels:
87+
app: test-app
88+
name: frontend
89+
spec:
90+
replicas: 1
91+
selector:
92+
matchLabels:
93+
app: test-app
94+
strategy: {}
95+
template:
96+
metadata:
97+
labels:
98+
app: test-app
99+
spec:
100+
containers:
101+
- image: gcr.io/foo/bar/busybox
102+
name: busybox
103+
---
104+
apiVersion: batch/v1
105+
kind: CronJob
106+
metadata:
107+
name: hello
108+
spec:
109+
schedule: "* * * * *"
110+
jobTemplate:
111+
spec:
112+
template:
113+
spec:
114+
containers:
115+
- name: hello
116+
image: gcr.io/foo/bar/busybox:1.28
117+
imagePullPolicy: IfNotPresent
118+
command:
119+
- /bin/sh
120+
- -c
121+
- date; echo Hello from the Kubernetes cluster
122+
restartPolicy: OnFailure`,
123+
},
124+
{
125+
name: "replace imagePullSecrets only",
126+
registry: "",
127+
imagePullSecret: "some-secret",
128+
inputManifest: inputManifest,
129+
expectedManifest: `---
130+
apiVersion: apps/v1
131+
kind: Deployment
132+
metadata:
133+
labels:
134+
app: test-app
135+
name: frontend
136+
spec:
137+
replicas: 1
138+
selector:
139+
matchLabels:
140+
app: test-app
141+
strategy: {}
142+
template:
143+
metadata:
144+
labels:
145+
app: test-app
146+
spec:
147+
containers:
148+
- image: busybox
149+
name: busybox
150+
imagePullSecrets:
151+
- name: some-secret
152+
---
153+
apiVersion: batch/v1
154+
kind: CronJob
155+
metadata:
156+
name: hello
157+
spec:
158+
schedule: "* * * * *"
159+
jobTemplate:
160+
spec:
161+
template:
162+
spec:
163+
containers:
164+
- name: hello
165+
image: busybox:1.28
166+
imagePullPolicy: IfNotPresent
167+
command:
168+
- /bin/sh
169+
- -c
170+
- date; echo Hello from the Kubernetes cluster
171+
imagePullSecrets:
172+
- name: some-secret
173+
restartPolicy: OnFailure`,
174+
},
175+
{
176+
name: "replace registry and imagePullSecrets",
177+
registry: "gcr.io/foo/bar",
178+
imagePullSecret: "some-secret",
179+
inputManifest: inputManifest,
180+
expectedManifest: `---
181+
apiVersion: apps/v1
182+
kind: Deployment
183+
metadata:
184+
labels:
185+
app: test-app
186+
name: frontend
187+
spec:
188+
replicas: 1
189+
selector:
190+
matchLabels:
191+
app: test-app
192+
strategy: {}
193+
template:
194+
metadata:
195+
labels:
196+
app: test-app
197+
spec:
198+
containers:
199+
- image: gcr.io/foo/bar/busybox
200+
name: busybox
201+
imagePullSecrets:
202+
- name: some-secret
203+
---
204+
apiVersion: batch/v1
205+
kind: CronJob
206+
metadata:
207+
name: hello
208+
spec:
209+
schedule: "* * * * *"
210+
jobTemplate:
211+
spec:
212+
template:
213+
spec:
214+
containers:
215+
- name: hello
216+
image: gcr.io/foo/bar/busybox:1.28
217+
imagePullPolicy: IfNotPresent
218+
command:
219+
- /bin/sh
220+
- -c
221+
- date; echo Hello from the Kubernetes cluster
222+
imagePullSecrets:
223+
- name: some-secret
224+
restartPolicy: OnFailure`,
225+
},
226+
{
227+
name: "replace nothing",
228+
registry: "",
229+
imagePullSecret: "",
230+
inputManifest: inputManifest,
231+
expectedManifest: inputManifest,
232+
},
233+
}
234+
for _, tc := range testCases {
235+
t.Run(tc.name, func(t *testing.T) {
236+
237+
dummyDeclarative := &TestResource{
238+
TypeMeta: metav1.TypeMeta{
239+
Kind: "TestResource",
240+
APIVersion: "addons.example.org/v1alpha1",
241+
},
242+
ObjectMeta: metav1.ObjectMeta{
243+
Name: "test-instance",
244+
},
245+
}
246+
247+
ctx := context.Background()
248+
249+
objects, err := manifest.ParseObjects(ctx, tc.inputManifest)
250+
if err != nil {
251+
t.Fatalf("unexpected error: %v", err)
252+
}
253+
254+
fn := ImageRegistryTransform(tc.registry, tc.imagePullSecret)
255+
err = fn(ctx, dummyDeclarative, objects)
256+
if err != nil {
257+
t.Fatalf("unexpected error: %v", err)
258+
}
259+
260+
expectedObjects, err := manifest.ParseObjects(ctx, tc.expectedManifest)
261+
if err != nil {
262+
t.Fatalf("unexpected error: %v", err)
263+
}
264+
265+
if len(expectedObjects.Items) != len(objects.Items) {
266+
t.Fatal("expected number of objects does not equal number of objects")
267+
}
268+
269+
for idx := range expectedObjects.Items {
270+
diff := cmp.Diff(
271+
expectedObjects.Items[idx].UnstructuredObject().Object,
272+
objects.Items[idx].UnstructuredObject().Object)
273+
if diff != "" {
274+
t.Errorf("result mismatch (-want +got):\n%s", diff)
275+
}
276+
}
277+
278+
})
279+
}
280+
}

‎pkg/patterns/declarative/pkg/manifest/objects.go

+16-3
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,24 @@ func nestedFieldNoCopy(obj map[string]interface{}, fields ...string) (interface{
161161
return val, true, nil
162162
}
163163

164+
func (o *Object) podSpecPath() []string {
165+
switch o.object.GetKind() {
166+
case "CronJob":
167+
return []string{"spec", "jobTemplate", "spec", "template", "spec"}
168+
default: // Default to try the path used by common types such as Deployment, StatefulSet, etc.
169+
return []string{"spec", "template", "spec"}
170+
}
171+
}
172+
164173
func (o *Object) MutateContainers(fn func(map[string]interface{}) error) error {
165174
if o.object.Object == nil {
166175
o.object.Object = make(map[string]interface{})
167176
}
168177

169-
containers, found, err := nestedFieldNoCopy(o.object.Object, "spec", "template", "spec", "containers")
178+
podSpecPath := o.podSpecPath()
179+
180+
containersPath := append(podSpecPath, "containers")
181+
containers, found, err := nestedFieldNoCopy(o.object.Object, containersPath...)
170182
if err != nil {
171183
return fmt.Errorf("error reading containers: %v", err)
172184
}
@@ -180,7 +192,8 @@ func (o *Object) MutateContainers(fn func(map[string]interface{}) error) error {
180192
return fmt.Errorf("containers was not a list")
181193
}
182194

183-
initContainers, found, err := nestedFieldNoCopy(o.object.Object, "spec", "template", "spec", "initContainers")
195+
initContainersPath := append(podSpecPath, "initContainers")
196+
initContainers, found, err := nestedFieldNoCopy(o.object.Object, initContainersPath...)
184197
if err != nil {
185198
return fmt.Errorf("error reading init containers: %v", err)
186199
}
@@ -215,7 +228,7 @@ func (o *Object) MutatePodSpec(fn func(map[string]interface{}) error) error {
215228
o.object.Object = make(map[string]interface{})
216229
}
217230

218-
sp, found, err := nestedFieldNoCopy(o.object.Object, "spec", "template", "spec")
231+
sp, found, err := nestedFieldNoCopy(o.object.Object, o.podSpecPath()...)
219232
if err != nil {
220233
return fmt.Errorf("error reading containers: %v", err)
221234
}

0 commit comments

Comments
 (0)