Adding TLS options to the sign logic.

When checking if the `signed` image already exist or when pulling the
image to sign, we should be able to set some TLS options.

This flow follows the same logic we have for builds.

In additions, this commit also update the type of the
`ModuleLoaderContainerSpec.RegistryTLS` API so we use the default values
when not specified.

Signed-off-by: Yoni Bettan <yonibettan@gmail.com>

api/v1beta1/module_types.go

@@ -54,7 +54,7 @@ type Build struct {
 
 	// +optional
 	// BaseImageRegistryTLS contains settings determining how to access registries of the base images in the build-process' Dockerfile.
-	BaseImageRegistryTLS TLSOptions `json:"baseImageRegistryTLS"`
+	BaseImageRegistryTLS TLSOptions `json:"baseImageRegistryTLS,omitempty"`
 
 	// +optional
 	// Secrets is an optional list of secrets to be made available to the build system.
@@ -72,6 +72,10 @@ type Sign struct {
 	// Image to sign, ignored if a Build is present, required otherwise
 	UnsignedImage string `json:"unsignedImage,omitempty"`
 
+	// +optional
+	// UnsignedImageRegistryTLS contains settings determining how to access registries of the unsigned image.
+	UnsignedImageRegistryTLS TLSOptions `json:"unsignedImageRegistryTLS,omitempty"`
+
 	// a secret containing the private key used to sign kernel modules for secureboot
 	KeySecret *v1.LocalObjectReference `json:"keySecret"`
 
@@ -184,7 +188,7 @@ type ModuleLoaderContainerSpec struct {
 
 	// +optional
 	// RegistryTLS set the TLS configs for accessing the registry of the module-loader's image.
-	RegistryTLS *TLSOptions `json:"registryTLS"`
+	RegistryTLS TLSOptions `json:"registryTLS"`
 }
 
 type ModuleLoaderSpec struct {

config/crd/bases/kmm.sigs.k8s.io_managedclustermodules.yaml

@@ -2203,6 +2203,22 @@ spec:
                                       description: Image to sign, ignored if a Build
                                         is present, required otherwise
                                       type: string
+                                    unsignedImageRegistryTLS:
+                                      description: UnsignedImageRegistryTLS contains
+                                        settings determining how to access registries
+                                        of the unsigned image.
+                                      properties:
+                                        insecure:
+                                          description: If Insecure is true, the operator
+                                            will be able to access a registry in an
+                                            insecure (plain HTTP) protocol.
+                                          type: boolean
+                                        insecureSkipTLSVerify:
+                                          description: If InsecureSkipTLSVerify, the
+                                            operator will accept any certificate provided
+                                            by the registry.
+                                          type: boolean
+                                      type: object
                                   required:
                                   - certSecret
                                   - keySecret
@@ -2335,6 +2351,22 @@ spec:
                                 description: Image to sign, ignored if a Build is
                                   present, required otherwise
                                 type: string
+                              unsignedImageRegistryTLS:
+                                description: UnsignedImageRegistryTLS contains settings
+                                  determining how to access registries of the unsigned
+                                  image.
+                                properties:
+                                  insecure:
+                                    description: If Insecure is true, the operator
+                                      will be able to access a registry in an insecure
+                                      (plain HTTP) protocol.
+                                    type: boolean
+                                  insecureSkipTLSVerify:
+                                    description: If InsecureSkipTLSVerify, the operator
+                                      will accept any certificate provided by the
+                                      registry.
+                                    type: boolean
+                                type: object
                             required:
                             - certSecret
                             - keySecret

config/crd/bases/kmm.sigs.k8s.io_modules.yaml

@@ -2108,6 +2108,22 @@ spec:
                                   description: Image to sign, ignored if a Build is
                                     present, required otherwise
                                   type: string
+                                unsignedImageRegistryTLS:
+                                  description: UnsignedImageRegistryTLS contains settings
+                                    determining how to access registries of the unsigned
+                                    image.
+                                  properties:
+                                    insecure:
+                                      description: If Insecure is true, the operator
+                                        will be able to access a registry in an insecure
+                                        (plain HTTP) protocol.
+                                      type: boolean
+                                    insecureSkipTLSVerify:
+                                      description: If InsecureSkipTLSVerify, the operator
+                                        will accept any certificate provided by the
+                                        registry.
+                                      type: boolean
+                                  type: object
                               required:
                               - certSecret
                               - keySecret
@@ -2238,6 +2254,21 @@ spec:
                             description: Image to sign, ignored if a Build is present,
                               required otherwise
                             type: string
+                          unsignedImageRegistryTLS:
+                            description: UnsignedImageRegistryTLS contains settings
+                              determining how to access registries of the unsigned
+                              image.
+                            properties:
+                              insecure:
+                                description: If Insecure is true, the operator will
+                                  be able to access a registry in an insecure (plain
+                                  HTTP) protocol.
+                                type: boolean
+                              insecureSkipTLSVerify:
+                                description: If InsecureSkipTLSVerify, the operator
+                                  will accept any certificate provided by the registry.
+                                type: boolean
+                            type: object
                         required:
                         - certSecret
                         - keySecret

internal/build/job/maker.go

@@ -81,12 +81,13 @@ func (m *maker) MakeJobTemplate(
 		containerImage = module.IntermediateImageName(mod.Name, mod.Namespace, containerImage)
 	}
 
+	registryTLS := module.TLSOptions(mod.Spec, km)
 	specTemplate := m.specTemplate(
 		mod.Spec,
 		buildConfig,
 		targetKernel,
 		containerImage,
-		km.RegistryTLS,
+		registryTLS,
 		pushImage)
 
 	specTemplateHash, err := m.getHashAnnotationValue(ctx, buildConfig.DockerfileConfigMap.Name, mod.Namespace, &specTemplate)
@@ -175,7 +176,7 @@ func (m *maker) containerArgs(
 		args = append(args, "--skip-tls-verify-pull")
 	}
 
-	if registryTLS != nil {
+	if pushImage {
 		if registryTLS.Insecure {
 			args = append(args, "--insecure")
 		}

internal/build/job/maker_test.go

@@ -293,7 +293,7 @@ var _ = Describe("MakeJobTemplate", func() {
 				DockerfileConfigMap:  &dockerfileConfigMap,
 			},
 			"--insecure-pull",
-			true,
+			false,
 		),
 		Entry(
 			"BaseImageRegistryTLS.InsecureSkipTLSVerify",
@@ -317,7 +317,7 @@ var _ = Describe("MakeJobTemplate", func() {
 			&kmmv1beta1.TLSOptions{InsecureSkipTLSVerify: true},
 			&kmmv1beta1.Build{DockerfileConfigMap: &dockerfileConfigMap},
 			"--skip-tls-verify",
-			false,
+			true,
 		),
 	)
 

internal/build/job/manager_test.go

@@ -72,7 +72,7 @@ var _ = Describe("ShouldSync", func() {
 		}
 
 		gomock.InOrder(
-			reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(true, nil),
+			reg.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Any()).Return(true, nil),
 		)
 
 		mgr := NewBuildManager(clnt, nil, nil, reg)
@@ -102,7 +102,7 @@ var _ = Describe("ShouldSync", func() {
 		}
 
 		gomock.InOrder(
-			reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, errors.New("generic-registry-error")),
+			reg.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Any()).Return(false, errors.New("generic-registry-error")),
 		)
 
 		mgr := NewBuildManager(clnt, nil, nil, reg)
@@ -133,7 +133,7 @@ var _ = Describe("ShouldSync", func() {
 		}
 
 		gomock.InOrder(
-			reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, nil),
+			reg.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Any()).Return(false, nil),
 		)
 
 		mgr := NewBuildManager(clnt, nil, nil, reg)

internal/module/helper.go

@@ -17,7 +17,7 @@ func TLSOptions(modSpec kmmv1beta1.ModuleSpec, km kmmv1beta1.KernelMapping) *kmm
 	if km.RegistryTLS != nil {
 		return km.RegistryTLS
 	}
-	return modSpec.ModuleLoader.Container.RegistryTLS
+	return &modSpec.ModuleLoader.Container.RegistryTLS
 }
 
 // AppendToTag adds the specified tag to the image name cleanly, i.e. by avoiding messing up

internal/module/helper_test.go

@@ -40,15 +40,15 @@ var _ = Describe("TLSOptions", func() {
 			Spec: kmmv1beta1.ModuleSpec{
 				ModuleLoader: kmmv1beta1.ModuleLoaderSpec{
 					Container: kmmv1beta1.ModuleLoaderContainerSpec{
-						RegistryTLS: &kmmv1beta1.TLSOptions{},
+						RegistryTLS: kmmv1beta1.TLSOptions{},
 					},
 				},
 			},
 		}
 		km := kmmv1beta1.KernelMapping{}
 
 		Expect(
-			TLSOptions(mod.Spec, km),
+			*TLSOptions(mod.Spec, km),
 		).To(
 			Equal(mod.Spec.ModuleLoader.Container.RegistryTLS),
 		)
@@ -241,7 +241,7 @@ var _ = Describe("ImageExists", func() {
 
 	It("should return true if the image exists", func() {
 		gomock.InOrder(
-			mockRegistry.EXPECT().ImageExists(ctx, imageName, nil, nil).Return(true, nil),
+			mockRegistry.EXPECT().ImageExists(ctx, imageName, gomock.Any(), nil).Return(true, nil),
 		)
 
 		exists, err := ImageExists(ctx, clnt, mockRegistry, mod.Spec, namespace, km, imageName)
@@ -252,7 +252,7 @@ var _ = Describe("ImageExists", func() {
 
 	It("should return false if the image does not exist", func() {
 		gomock.InOrder(
-			mockRegistry.EXPECT().ImageExists(ctx, imageName, nil, nil).Return(false, nil),
+			mockRegistry.EXPECT().ImageExists(ctx, imageName, gomock.Any(), nil).Return(false, nil),
 		)
 
 		exists, err := ImageExists(ctx, clnt, mockRegistry, mod.Spec, namespace, km, imageName)
@@ -263,7 +263,7 @@ var _ = Describe("ImageExists", func() {
 
 	It("should return an error if the registry call fails", func() {
 		gomock.InOrder(
-			mockRegistry.EXPECT().ImageExists(ctx, imageName, nil, nil).Return(false, errors.New("some-error")),
+			mockRegistry.EXPECT().ImageExists(ctx, imageName, gomock.Any(), nil).Return(false, errors.New("some-error")),
 		)
 
 		exists, err := ImageExists(ctx, clnt, mockRegistry, mod.Spec, namespace, km, imageName)
@@ -283,7 +283,7 @@ var _ = Describe("ImageExists", func() {
 		}
 
 		gomock.InOrder(
-			mockRegistry.EXPECT().ImageExists(ctx, imageName, nil, gomock.Not(gomock.Nil())).Return(false, nil),
+			mockRegistry.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Not(gomock.Nil())).Return(false, nil),
 		)
 
 		exists, err := ImageExists(ctx, clnt, mockRegistry, mod.Spec, namespace, km, imageName)

internal/preflight/preflight_test.go

@@ -215,7 +215,8 @@ var _ = Describe("preflightHelper_verifyImage", func() {
 		repoConfig := &registry.RepoPullConfig{}
 		digestLayer := v1stream.Layer{}
 		gomock.InOrder(
-			mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, nil, gomock.Any()).Return(digests, repoConfig, nil),
+			mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, gomock.Any(),
+				gomock.Any()).Return(digests, repoConfig, nil),
 			mockRegistryAPI.EXPECT().GetLayerByDigest(digests[1], repoConfig).Return(&digestLayer, nil),
 			mockRegistryAPI.EXPECT().VerifyModuleExists(&digestLayer, "/opt", kernelVersion, "simple-kmod.ko").Return(true),
 		)
@@ -228,7 +229,8 @@ var _ = Describe("preflightHelper_verifyImage", func() {
 
 	It("get layers digest failed", func() {
 		mapping := kmmv1beta1.KernelMapping{ContainerImage: containerImage}
-		mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, nil, gomock.Any()).Return(nil, nil, fmt.Errorf("some error"))
+		mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, gomock.Any(),
+			gomock.Any()).Return(nil, nil, fmt.Errorf("some error"))
 
 		res, message := ph.verifyImage(context.Background(), &mapping, mod, kernelVersion)
 
@@ -241,7 +243,8 @@ var _ = Describe("preflightHelper_verifyImage", func() {
 		digests := []string{"digest0", "digest1"}
 		repoConfig := &registry.RepoPullConfig{}
 		gomock.InOrder(
-			mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, nil, gomock.Any()).Return(digests, repoConfig, nil),
+			mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, gomock.Any(),
+				gomock.Any()).Return(digests, repoConfig, nil),
 			mockRegistryAPI.EXPECT().GetLayerByDigest(digests[1], repoConfig).Return(nil, fmt.Errorf("some error")),
 		)
 
@@ -257,7 +260,7 @@ var _ = Describe("preflightHelper_verifyImage", func() {
 		repoConfig := &registry.RepoPullConfig{}
 		digestLayer := v1stream.Layer{}
 		gomock.InOrder(
-			mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, nil, gomock.Any()).Return(digests, repoConfig, nil),
+			mockRegistryAPI.EXPECT().GetLayersDigests(context.Background(), containerImage, gomock.Any(), gomock.Any()).Return(digests, repoConfig, nil),
 			mockRegistryAPI.EXPECT().GetLayerByDigest(digests[0], repoConfig).Return(&digestLayer, nil),
 			mockRegistryAPI.EXPECT().VerifyModuleExists(&digestLayer, "/opt", kernelVersion, "simple-kmod.ko").Return(false),
 		)

internal/sign/job/manager_test.go

@@ -71,7 +71,7 @@ var _ = Describe("JobManager", func() {
 			}
 
 			gomock.InOrder(
-				reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(true, nil),
+				reg.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Any()).Return(true, nil),
 			)
 
 			mgr := NewSignJobManager(clnt, nil, nil, reg)
@@ -101,7 +101,7 @@ var _ = Describe("JobManager", func() {
 			}
 
 			gomock.InOrder(
-				reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, errors.New("generic-registry-error")),
+				reg.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Any()).Return(false, errors.New("generic-registry-error")),
 			)
 
 			mgr := NewSignJobManager(clnt, nil, nil, reg)
@@ -132,7 +132,7 @@ var _ = Describe("JobManager", func() {
 			}
 
 			gomock.InOrder(
-				reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, nil),
+				reg.EXPECT().ImageExists(ctx, imageName, gomock.Any(), gomock.Any()).Return(false, nil),
 			)
 
 			mgr := NewSignJobManager(clnt, nil, nil, reg)

internal/sign/job/signer.go

@@ -12,6 +12,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
 	kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/module"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/sign"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
 )
@@ -62,6 +63,14 @@ func (m *signer) MakeJobTemplate(
 
 	if pushImage {
 		args = append(args, "-signedimage", km.ContainerImage)
+
+		registryTLS := module.TLSOptions(mod.Spec, km)
+		if registryTLS.Insecure {
+			args = append(args, "--insecure")
+		}
+		if registryTLS.InsecureSkipTLSVerify {
+			args = append(args, "--skip-tls-verify")
+		}
 	} else {
 		args = append(args, "-no-push")
 	}
@@ -80,6 +89,14 @@ func (m *signer) MakeJobTemplate(
 		args = append(args, "-filestosign", strings.Join(signConfig.FilesToSign, ":"))
 	}
 
+	if signConfig.UnsignedImageRegistryTLS.Insecure {
+		args = append(args, "--insecure-pull")
+	}
+
+	if signConfig.UnsignedImageRegistryTLS.InsecureSkipTLSVerify {
+		args = append(args, "--skip-tls-verify-pull")
+	}
+
 	volumes := []v1.Volume{
 		utils.MakeSecretVolume(signConfig.KeySecret, "key", "key.priv"),
 		utils.MakeSecretVolume(signConfig.CertSecret, "cert", "public.der"),