Skip to content

Commit 9a89583

Browse files
TomerNewmank8s-ci-robot
authored andcommitted
Adding webhook validation for new shipwright build logic
As part of KMM transitioning to Shipwright, KMM's webhook should allow only the following scenarios: 1. DockerfileOCIArtifact is defined and not DockerfileConfigMap. 2. DockerfileConfigMap is defined and not DockerfileOCIArtifact.
1 parent 69bf011 commit 9a89583

File tree

3 files changed

+120
-2
lines changed

3 files changed

+120
-2
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ require (
1919
github.com/spf13/cobra v1.9.1
2020
go.uber.org/mock v0.5.1
2121
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
22+
golang.org/x/net v0.38.0
2223
golang.org/x/text v0.24.0
2324
gopkg.in/yaml.v3 v3.0.1
2425
k8s.io/api v0.32.3
@@ -90,7 +91,6 @@ require (
9091
go.opentelemetry.io/otel/trace v1.33.0 // indirect
9192
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
9293
go.uber.org/automaxprocs v1.6.0 // indirect
93-
golang.org/x/net v0.38.0 // indirect
9494
golang.org/x/oauth2 v0.25.0 // indirect
9595
golang.org/x/sync v0.13.0 // indirect
9696
golang.org/x/sys v0.32.0 // indirect

internal/webhook/module.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,30 @@ func validateModuleLoaderContainerSpec(container kmmv1beta1.ModuleLoaderContaine
173173
(km.InTreeModuleToRemove != "" && container.InTreeModulesToRemove != nil) { //nolint:staticcheck
174174
return fmt.Errorf("only one type if field (InTreeModuleToRemove or InTreeModulesToRemove) can be defined in KenrelMapping or Container")
175175
}
176+
177+
if err := validateModuleLoaderContainerBuildSpec(km.Build); err != nil {
178+
return fmt.Errorf("failed to validate kernelMappings[%d].build: %v", idx, err)
179+
}
180+
}
181+
182+
return validateModuleLoaderContainerBuildSpec(container.Build)
183+
}
184+
185+
func validateModuleLoaderContainerBuildSpec(build *kmmv1beta1.Build) error {
186+
if build == nil {
187+
return nil
188+
}
189+
190+
if build.DockerfileOCIArtifact != "" && build.DockerfileConfigMap != nil {
191+
return fmt.Errorf("only one of the Dockerfile fields: DockerfileOCIArtifact or DockerfileConfigMap can be defined")
192+
}
193+
194+
if build.DockerfileOCIArtifact == "" && build.DockerfileConfigMap == nil {
195+
return fmt.Errorf("one of the Dockerfile fields: DockerfileOCIArtifact or DockerfileConfigMap must be defined")
196+
}
197+
198+
if build.DockerfileOCIArtifact != "" {
199+
return validateImageFormat(build.DockerfileOCIArtifact)
176200
}
177201

178202
return nil

internal/webhook/module_test.go

Lines changed: 95 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ package webhook
1818

1919
import (
2020
"context"
21-
v1 "k8s.io/api/core/v1"
2221
"strings"
2322

23+
v1 "k8s.io/api/core/v1"
24+
2425
kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
2526
"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
2627
. "github.com/onsi/ginkgo/v2"
@@ -232,6 +233,25 @@ var _ = Describe("validateModuleLoaderContainerSpec", func() {
232233
)
233234
})
234235

236+
It("should fail when a kernel-mapping has invalid Build specification", func() {
237+
containerSpec := kmmv1beta1.ModuleLoaderContainerSpec{
238+
KernelMappings: []kmmv1beta1.KernelMapping{
239+
{
240+
Regexp: "^valid-regexp$",
241+
ContainerImage: "image-url",
242+
Build: &kmmv1beta1.Build{
243+
DockerfileOCIArtifact: "registry.example.com/dockerfile:latest",
244+
DockerfileConfigMap: &v1.LocalObjectReference{Name: "dockerfile-configmap"},
245+
},
246+
},
247+
},
248+
}
249+
250+
Expect(
251+
validateModuleLoaderContainerSpec(containerSpec),
252+
).To(HaveOccurred())
253+
})
254+
235255
DescribeTable("should fail when InTreeModulesToRemove and InTreeModuleToRemove both defined",
236256
func(inTreeModulesInContainer, inTreeModuleInContainer, inTreeModulesInKM, inTreeModuleInKM bool) {
237257
containerSpec := kmmv1beta1.ModuleLoaderContainerSpec{}
@@ -262,6 +282,80 @@ var _ = Describe("validateModuleLoaderContainerSpec", func() {
262282

263283
})
264284

285+
var _ = Describe("validateModuleLoaderContainerBuildSpec", func() {
286+
It("should return nil when build is nil", func() {
287+
Expect(validateModuleLoaderContainerBuildSpec(nil)).NotTo(HaveOccurred())
288+
})
289+
290+
It("should pass when only DockerfileOCIArtifact is set with valid image format", func() {
291+
build := &kmmv1beta1.Build{
292+
DockerfileOCIArtifact: "registry.example.com/dockerfile:latest",
293+
}
294+
Expect(validateModuleLoaderContainerBuildSpec(build)).NotTo(HaveOccurred())
295+
})
296+
297+
It("should pass when only DockerfileConfigMap is set", func() {
298+
build := &kmmv1beta1.Build{
299+
DockerfileConfigMap: &v1.LocalObjectReference{
300+
Name: "dockerfile-configmap",
301+
},
302+
}
303+
Expect(validateModuleLoaderContainerBuildSpec(build)).NotTo(HaveOccurred())
304+
})
305+
306+
It("should fail when both DockerfileOCIArtifact and DockerfileConfigMap are set", func() {
307+
build := &kmmv1beta1.Build{
308+
DockerfileOCIArtifact: "registry.example.com/dockerfile:latest",
309+
DockerfileConfigMap: &v1.LocalObjectReference{
310+
Name: "dockerfile-configmap",
311+
},
312+
}
313+
Expect(validateModuleLoaderContainerBuildSpec(build)).To(
314+
MatchError(ContainSubstring("only one of the Dockerfile fields: DockerfileOCIArtifact or DockerfileConfigMap can be defined")),
315+
)
316+
})
317+
318+
It("should fail when neither DockerfileOCIArtifact nor DockerfileConfigMap is set", func() {
319+
build := &kmmv1beta1.Build{}
320+
Expect(validateModuleLoaderContainerBuildSpec(build)).To(
321+
MatchError(ContainSubstring("one of the Dockerfile fields: DockerfileOCIArtifact or DockerfileConfigMap must be defined")),
322+
)
323+
})
324+
325+
It("should fail when DockerfileOCIArtifact has invalid image format", func() {
326+
build := &kmmv1beta1.Build{
327+
DockerfileOCIArtifact: "invalid-image-format",
328+
}
329+
Expect(validateModuleLoaderContainerBuildSpec(build)).To(
330+
MatchError(ContainSubstring("container image must explicitely set a tag or digest")),
331+
)
332+
})
333+
334+
It("should pass when DockerfileOCIArtifact has valid image format with tag", func() {
335+
build := &kmmv1beta1.Build{
336+
DockerfileOCIArtifact: "registry.example.com/dockerfile:v1.0.0",
337+
}
338+
Expect(validateModuleLoaderContainerBuildSpec(build)).NotTo(HaveOccurred())
339+
})
340+
341+
It("should pass when DockerfileOCIArtifact has valid image format with digest", func() {
342+
build := &kmmv1beta1.Build{
343+
DockerfileOCIArtifact: "registry.example.com/dockerfile@sha256:1234567890abcdef",
344+
}
345+
Expect(validateModuleLoaderContainerBuildSpec(build)).NotTo(HaveOccurred())
346+
})
347+
348+
It("should fail when DockerfileOCIArtifact is empty string and DockerfileConfigMap is nil", func() {
349+
build := &kmmv1beta1.Build{
350+
DockerfileOCIArtifact: "",
351+
DockerfileConfigMap: nil,
352+
}
353+
Expect(validateModuleLoaderContainerBuildSpec(build)).To(
354+
MatchError(ContainSubstring("one of the Dockerfile fields: DockerfileOCIArtifact or DockerfileConfigMap must be defined")),
355+
)
356+
})
357+
})
358+
265359
var _ = Describe("validateModprobe", func() {
266360
It("should fail when moduleName and rawArgs are missing", func() {
267361
Expect(

0 commit comments

Comments
 (0)