diff --git a/internal/registry/registry.go b/internal/registry/registry.go index a44cdce7d..cbb1cb9c3 100644 --- a/internal/registry/registry.go +++ b/internal/registry/registry.go @@ -129,10 +129,6 @@ func (r *registry) getPullOptions(ctx context.Context, image string, tlsOptions repo = tag[0] } - if repo == "" { - return nil, fmt.Errorf("image url %s is not valid, does not contain hash or tag", image) - } - options := []crane.Option{ crane.WithContext(ctx), } diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go index 286e0fe0c..e77575b2b 100644 --- a/internal/registry/registry_test.go +++ b/internal/registry/registry_test.go @@ -57,14 +57,6 @@ var _ = Describe("ImageExists", func() { Expect(err.Error()).To(ContainSubstring("failed to get pull options for image")) }) - It("should fail if the image name isn't valid", func() { - - _, err = reg.ImageExists(ctx, invalidImage, &kmmv1beta1.TLSOptions{}, nil) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("does not contain hash or tag")) - }) - It("should fail if it cannot get key chain from secret", func() { mockRegistryAuthGetter.EXPECT().GetKeyChain(ctx).Return(nil, errors.New("some error")) @@ -222,14 +214,6 @@ var _ = Describe("GetLayersDigests", func() { Expect(err.Error()).To(ContainSubstring("failed to get pull options for image")) }) - It("should fail if the image name isn't valid", func() { - - _, err = reg.ImageExists(ctx, invalidImage, &kmmv1beta1.TLSOptions{}, nil) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("does not contain hash or tag")) - }) - It("should fail if it cannot get key chain from secret", func() { mockRegistryAuthGetter.EXPECT().GetKeyChain(ctx).Return(nil, errors.New("some error")) @@ -393,13 +377,6 @@ var _ = Describe("GetDigest", func() { Expect(err.Error()).To(ContainSubstring("failed to get pull options for image")) }) - It("should fail if the image name isn't valid", func() { - _, err = reg.GetDigest(ctx, invalidImage, &kmmv1beta1.TLSOptions{}, nil) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("does not contain hash or tag")) - }) - It("should fail if it cannot get key chain from secret", func() { mockRegistryAuthGetter.EXPECT().GetKeyChain(ctx).Return(nil, errors.New("some error")) diff --git a/internal/webhook/module.go b/internal/webhook/module.go index deef0023b..96ac716f1 100644 --- a/internal/webhook/module.go +++ b/internal/webhook/module.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "regexp" + "strings" "github.com/go-logr/logr" kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1" @@ -111,11 +112,27 @@ func validateModule(mod *kmmv1beta1.Module) (admission.Warnings, error) { return nil, validateModprobe(mod.Spec.ModuleLoader.Container.Modprobe) } +func validateImageFormat(img string) error { + + if !strings.Contains(img, ":") && !strings.Contains(img, "@") { + return fmt.Errorf("container image must explicitely set a tag or digest; got: %s", img) + } + + return nil +} + func validateModuleLoaderContainerSpec(container kmmv1beta1.ModuleLoaderContainerSpec) error { + if container.InTreeModulesToRemove != nil && container.InTreeModuleToRemove != "" { //nolint:staticcheck return fmt.Errorf("only one of the Container's fields: InTreeModulesToRemove or InTreeModuleToRemove can be defined") } + if contImg := container.ContainerImage; contImg != "" { + if err := validateImageFormat(contImg); err != nil { + return fmt.Errorf("failed to validate image format: %v", err) + } + } + for idx, km := range container.KernelMappings { if km.Regexp != "" && km.Literal != "" { return fmt.Errorf("regexp and literal are mutually exclusive properties at kernelMappings[%d]", idx) @@ -129,8 +146,12 @@ func validateModuleLoaderContainerSpec(container kmmv1beta1.ModuleLoaderContaine return fmt.Errorf("invalid regexp at index %d: %v", idx, err) } - if container.ContainerImage == "" && km.ContainerImage == "" { - return fmt.Errorf("missing spec.moduleLoader.container.kernelMappings[%d].containerImage", idx) + if kmImg := km.ContainerImage; kmImg == "" { + if container.ContainerImage == "" { + return fmt.Errorf("missing spec.moduleLoader.container.kernelMappings[%d].containerImage", idx) + } + } else if err := validateImageFormat(kmImg); err != nil { + return fmt.Errorf("failed to validate image format: %v", err) } if km.InTreeModulesToRemove != nil && km.InTreeModuleToRemove != "" { //nolint:staticcheck diff --git a/internal/webhook/module_test.go b/internal/webhook/module_test.go index 42487e5b4..37a2e599e 100644 --- a/internal/webhook/module_test.go +++ b/internal/webhook/module_test.go @@ -45,7 +45,7 @@ var ( ModuleName: "mod-name", }, KernelMappings: []kmmv1beta1.KernelMapping{ - {Regexp: "valid-regexp", ContainerImage: "image-url"}, + {Regexp: "valid-regexp", ContainerImage: "image-url:mytag"}, }, }, }, @@ -75,6 +75,27 @@ var _ = Describe("maxCombinedLength", func() { }) }) +var _ = Describe("validateImageFormat", func() { + + It("should fail if no tag or digest were specified", func() { + + Expect( + validateImageFormat("image-ur"), + ).To(HaveOccurred()) + }) + + It("should work if a tag or digest were specified", func() { + + Expect( + validateImageFormat("image-url:tag"), + ).NotTo(HaveOccurred()) + + Expect( + validateImageFormat("image-url@sha256:xxxxxxxxxxxx"), + ).NotTo(HaveOccurred()) + }) +}) + var _ = Describe("validateModuleLoaderContainerSpec", func() { It("should pass when there are not kernel mappings", func() { Expect( @@ -84,18 +105,36 @@ var _ = Describe("validateModuleLoaderContainerSpec", func() { ) }) + It("should fail if the containerImage doesn't contain a sha or tag", func() { + + containerSpec := kmmv1beta1.ModuleLoaderContainerSpec{ + ContainerImage: "image-url", + KernelMappings: []kmmv1beta1.KernelMapping{ + {Regexp: "valid-regexp"}, + }, + } + + Expect( + validateModuleLoaderContainerSpec(containerSpec), + ).To( + MatchError( + ContainSubstring("failed to validate image format"), + ), + ) + }) + It("should pass when regexp,literal and containerImage are valid", func() { containerSpec1 := kmmv1beta1.ModuleLoaderContainerSpec{ KernelMappings: []kmmv1beta1.KernelMapping{ - {Regexp: "valid-regexp", ContainerImage: "image-url"}, - {Regexp: "^.*$", ContainerImage: "image-url"}, + {Regexp: "valid-regexp", ContainerImage: "image-url:mytag"}, + {Regexp: "^.*$", ContainerImage: "image-url:mytag"}, }, } Expect(validateModuleLoaderContainerSpec(containerSpec1)).ToNot(HaveOccurred()) containerSpec2 := kmmv1beta1.ModuleLoaderContainerSpec{ - ContainerImage: "image-url", + ContainerImage: "image-url:mytag", KernelMappings: []kmmv1beta1.KernelMapping{ {Regexp: "valid-regexp"}, }, @@ -104,7 +143,7 @@ var _ = Describe("validateModuleLoaderContainerSpec", func() { Expect(validateModuleLoaderContainerSpec(containerSpec2)).ToNot(HaveOccurred()) containerSpec3 := kmmv1beta1.ModuleLoaderContainerSpec{ - ContainerImage: "image-url", + ContainerImage: "image-url:mytag", KernelMappings: []kmmv1beta1.KernelMapping{ {Literal: "any-value"}, }, @@ -176,6 +215,22 @@ var _ = Describe("validateModuleLoaderContainerSpec", func() { ) }) + It("should fail when a kernel-mapping has invalid containerName", func() { + containerSpec := kmmv1beta1.ModuleLoaderContainerSpec{ + KernelMappings: []kmmv1beta1.KernelMapping{ + {Regexp: "valid-regexp", ContainerImage: "image-url"}, + }, + } + + Expect( + validateModuleLoaderContainerSpec(containerSpec), + ).To( + MatchError( + ContainSubstring("failed to validate image format"), + ), + ) + }) + DescribeTable("should fail when InTreeModulesToRemove and InTreeModuleToRemove both defined", func(inTreeModulesInContainer, inTreeModuleInContainer, inTreeModulesInKM, inTreeModuleInKM bool) { containerSpec := kmmv1beta1.ModuleLoaderContainerSpec{} @@ -412,7 +467,7 @@ var _ = Describe("ValidateUpdate", func() { Modprobe: kmmv1beta1.ModprobeSpec{ ModuleName: "module-name", }, - ContainerImage: "image-url", + ContainerImage: "image-url:mytag", KernelMappings: []kmmv1beta1.KernelMapping{ {Regexp: "valid-regexp"}, }, @@ -430,7 +485,7 @@ var _ = Describe("ValidateUpdate", func() { Load: []string{"arg-1"}, Unload: []string{"arg-1"}, }, }, - ContainerImage: "image-url", + ContainerImage: "image-url:mytag", KernelMappings: []kmmv1beta1.KernelMapping{ {Regexp: "valid-regexp"}, },