From 91dce790e10ae2ea4ebc6b07d15ac42b9a711292 Mon Sep 17 00:00:00 2001 From: Yoni Bettan Date: Thu, 24 Nov 2022 15:56:39 +0200 Subject: [PATCH] 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. Signed-off-by: Yoni Bettan --- api/v1beta1/module_types.go | 8 +++- api/v1beta1/zz_generated.deepcopy.go | 13 ++---- ...kmm.sigs.k8s.io_managedclustermodules.yaml | 32 +++++++++++++++ config/crd/bases/kmm.sigs.k8s.io_modules.yaml | 31 ++++++++++++++ controllers/module_reconciler_test.go | 20 ++++----- internal/build/job/maker.go | 12 +++--- internal/build/job/maker_test.go | 8 ++-- internal/build/job/manager.go | 2 +- internal/build/job/manager_test.go | 10 ++--- internal/module/helper.go | 6 +-- internal/preflight/preflight_test.go | 8 ++-- internal/sign/job/manager.go | 6 ++- internal/sign/job/manager_test.go | 18 +++++--- internal/sign/job/mock_signer.go | 8 ++-- internal/sign/job/signer.go | 23 ++++++++++- internal/sign/job/signer_test.go | 41 ++++++++++++++++++- 16 files changed, 184 insertions(+), 62 deletions(-) diff --git a/api/v1beta1/module_types.go b/api/v1beta1/module_types.go index 7227d8f06..49d0e5b86 100644 --- a/api/v1beta1/module_types.go +++ b/api/v1beta1/module_types.go @@ -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:"baseImageRegistryTLS"` + // a secret containing the private key used to sign kernel modules for secureboot KeySecret *v1.LocalObjectReference `json:"keySecret"` @@ -104,7 +108,7 @@ type KernelMapping 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"` // +optional // Regexp is a regular expression to be match against node kernels. @@ -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 { diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index f7be6f745..37ab570d3 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -199,11 +199,7 @@ func (in *KernelMapping) DeepCopyInto(out *KernelMapping) { *out = new(Sign) (*in).DeepCopyInto(*out) } - if in.RegistryTLS != nil { - in, out := &in.RegistryTLS, &out.RegistryTLS - *out = new(TLSOptions) - **out = **in - } + out.RegistryTLS = in.RegistryTLS } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new KernelMapping. @@ -432,11 +428,7 @@ func (in *ModuleLoaderContainerSpec) DeepCopyInto(out *ModuleLoaderContainerSpec } } in.Modprobe.DeepCopyInto(&out.Modprobe) - if in.RegistryTLS != nil { - in, out := &in.RegistryTLS, &out.RegistryTLS - *out = new(TLSOptions) - **out = **in - } + out.RegistryTLS = in.RegistryTLS } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ModuleLoaderContainerSpec. @@ -622,6 +614,7 @@ func (in *PreflightValidationStatus) DeepCopy() *PreflightValidationStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Sign) DeepCopyInto(out *Sign) { *out = *in + out.UnsignedImageRegistryTLS = in.UnsignedImageRegistryTLS if in.KeySecret != nil { in, out := &in.KeySecret, &out.KeySecret *out = new(v1.LocalObjectReference) diff --git a/config/crd/bases/kmm.sigs.k8s.io_managedclustermodules.yaml b/config/crd/bases/kmm.sigs.k8s.io_managedclustermodules.yaml index 916e6c822..3ccb36485 100644 --- a/config/crd/bases/kmm.sigs.k8s.io_managedclustermodules.yaml +++ b/config/crd/bases/kmm.sigs.k8s.io_managedclustermodules.yaml @@ -2168,6 +2168,22 @@ spec: description: Sign enables in-cluster signing for this mapping properties: + baseImageRegistryTLS: + 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 certSecret: description: a secret containing the public key used to sign kernel modules for secureboot @@ -2301,6 +2317,22 @@ spec: sign: description: Sign provides default kmod signing settings properties: + baseImageRegistryTLS: + 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 certSecret: description: a secret containing the public key used to sign kernel modules for secureboot diff --git a/config/crd/bases/kmm.sigs.k8s.io_modules.yaml b/config/crd/bases/kmm.sigs.k8s.io_modules.yaml index 8f70abc99..a621ef57f 100644 --- a/config/crd/bases/kmm.sigs.k8s.io_modules.yaml +++ b/config/crd/bases/kmm.sigs.k8s.io_modules.yaml @@ -2074,6 +2074,22 @@ spec: description: Sign enables in-cluster signing for this mapping properties: + baseImageRegistryTLS: + 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 certSecret: description: a secret containing the public key used to sign kernel modules for secureboot @@ -2206,6 +2222,21 @@ spec: sign: description: Sign provides default kmod signing settings properties: + baseImageRegistryTLS: + 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 certSecret: description: a secret containing the public key used to sign kernel modules for secureboot diff --git a/controllers/module_reconciler_test.go b/controllers/module_reconciler_test.go index 00b8bf073..47248bf7b 100644 --- a/controllers/module_reconciler_test.go +++ b/controllers/module_reconciler_test.go @@ -624,7 +624,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { }, } - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).DoAndReturn( + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, _ interface{}, registryAuthGetter auth.RegistryAuthGetter) (bool, error) { Expect(registryAuthGetter).ToNot(BeNil()) return true, nil @@ -652,7 +652,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { } buildRes := build.Result{Requeue: true, Status: build.StatusCreated} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), km.ContainerImage, true).Return(buildRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false), ) @@ -678,7 +678,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { } buildRes := build.Result{Requeue: true, Status: build.StatusCreated} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), km.ContainerImage, true).Return(buildRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, false), ) @@ -704,7 +704,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { } buildRes := build.Result{Requeue: false, Status: build.StatusCompleted} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockBM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), km.ContainerImage, true).Return(buildRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.BuildStage, true), ) @@ -781,7 +781,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { }, } - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).DoAndReturn( + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, _ interface{}, registryAuthGetter auth.RegistryAuthGetter) (bool, error) { Expect(registryAuthGetter).ToNot(BeNil()) return true, nil @@ -809,7 +809,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { } signRes := utils.Result{Requeue: true, Status: utils.StatusCreated} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockSM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), "", km.ContainerImage, true).Return(signRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.SignStage, false), ) @@ -835,7 +835,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { } buildRes := utils.Result{Requeue: true, Status: utils.StatusCreated} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockSM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), "", km.ContainerImage, true).Return(buildRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.SignStage, false), ) @@ -861,7 +861,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { } signRes := utils.Result{Requeue: false, Status: build.StatusCompleted} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockSM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), "", km.ContainerImage, true).Return(signRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.SignStage, true), ) @@ -887,7 +887,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { } signRes := utils.Result{Requeue: false, Status: build.StatusCompleted} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName, nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName, gomock.Any(), gomock.Any()).Return(false, nil), mockSM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), imageName+":"+namespace+"_"+moduleName+"_kmm_unsigned", km.ContainerImage, true).Return(signRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.SignStage, true), ) @@ -913,7 +913,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { } signRes := utils.Result{Requeue: false, Status: build.StatusCompleted} gomock.InOrder( - mockRegistry.EXPECT().ImageExists(context.Background(), imageName+":test", nil, gomock.Any()).Return(false, nil), + mockRegistry.EXPECT().ImageExists(context.Background(), imageName+":test", gomock.Any(), gomock.Any()).Return(false, nil), mockSM.EXPECT().Sync(gomock.Any(), *mod, *km, gomock.Any(), imageName+":test_"+namespace+"_"+moduleName+"_kmm_unsigned", km.ContainerImage, true).Return(signRes, nil), mockMetrics.EXPECT().SetCompletedStage(mod.Name, mod.Namespace, kernelVersion, metrics.SignStage, true), ) diff --git a/internal/build/job/maker.go b/internal/build/job/maker.go index 8dcc31d21..199fad4db 100644 --- a/internal/build/job/maker.go +++ b/internal/build/job/maker.go @@ -75,14 +75,12 @@ func (m *maker) MakeJobTemplate(ctx context.Context, mod kmmv1beta1.Module, buil args = append(args, "--skip-tls-verify-pull") } - if registryTLS != nil { - if registryTLS.Insecure { - args = append(args, "--insecure") - } + if registryTLS.Insecure { + args = append(args, "--insecure") + } - if registryTLS.InsecureSkipTLSVerify { - args = append(args, "--skip-tls-verify") - } + if registryTLS.InsecureSkipTLSVerify { + args = append(args, "--skip-tls-verify") } const dockerfileVolumeName = "dockerfile" diff --git a/internal/build/job/maker_test.go b/internal/build/job/maker_test.go index ca5203b38..3fca1d835 100644 --- a/internal/build/job/maker_test.go +++ b/internal/build/job/maker_test.go @@ -220,7 +220,7 @@ var _ = Describe("MakeJobTemplate", func() { jobhelper.EXPECT().JobLabels(*mod, kernelVersion, utils.JobTypeBuild).Return(labels), ) - actual, err := m.MakeJobTemplate(ctx, *mod, km.Build, kernelVersion, km.ContainerImage, true, nil) + actual, err := m.MakeJobTemplate(ctx, *mod, km.Build, kernelVersion, km.ContainerImage, true, &kmmv1beta1.TLSOptions{}) Expect(err).NotTo(HaveOccurred()) Expect( @@ -285,7 +285,7 @@ var _ = Describe("MakeJobTemplate", func() { }, Entry( "BaseImageRegistryTLS.Insecure", - nil, + &kmmv1beta1.TLSOptions{}, kmmv1beta1.Build{ BaseImageRegistryTLS: kmmv1beta1.TLSOptions{Insecure: true}, DockerfileConfigMap: &dockerfileConfigMap, @@ -295,7 +295,7 @@ var _ = Describe("MakeJobTemplate", func() { ), Entry( "BaseImageRegistryTLS.InsecureSkipTLSVerify", - nil, + &kmmv1beta1.TLSOptions{}, kmmv1beta1.Build{ BaseImageRegistryTLS: kmmv1beta1.TLSOptions{InsecureSkipTLSVerify: true}, DockerfileConfigMap: &dockerfileConfigMap, @@ -344,7 +344,7 @@ var _ = Describe("MakeJobTemplate", func() { jobhelper.EXPECT().JobLabels(mod, kernelVersion, utils.JobTypeBuild).Return(map[string]string{}), ) - actual, err := m.MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, false, nil) + actual, err := m.MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, false, &kmmv1beta1.TLSOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(actual.Spec.Template.Spec.Containers[0].Image).To(Equal("gcr.io/kaniko-project/executor:" + customTag)) }) diff --git a/internal/build/job/manager.go b/internal/build/job/manager.go index 36b1e4d3d..271115e89 100644 --- a/internal/build/job/manager.go +++ b/internal/build/job/manager.go @@ -54,7 +54,7 @@ func (jbm *jobManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1b buildConfig := jbm.helper.GetRelevantBuild(mod, m) - jobTemplate, err := jbm.maker.MakeJobTemplate(ctx, mod, buildConfig, targetKernel, targetImage, pushImage, m.RegistryTLS) + jobTemplate, err := jbm.maker.MakeJobTemplate(ctx, mod, buildConfig, targetKernel, targetImage, pushImage, &m.RegistryTLS) if err != nil { return build.Result{}, fmt.Errorf("could not make Job template: %v", err) } diff --git a/internal/build/job/manager_test.go b/internal/build/job/manager_test.go index 6d7c7c6ef..326f493d7 100644 --- a/internal/build/job/manager_test.go +++ b/internal/build/job/manager_test.go @@ -69,7 +69,7 @@ var _ = Describe("Sync", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, nil).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeBuild).Return(&j, nil), jobhelper.EXPECT().IsJobChanged(&j, &j).Return(false, nil), ) @@ -95,7 +95,7 @@ var _ = Describe("Sync", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, nil).Return(nil, errors.New("random error")), + maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, gomock.Any()).Return(nil, errors.New("random error")), ) mgr := NewBuildManager(clnt, maker, helper, jobhelper) @@ -122,7 +122,7 @@ var _ = Describe("Sync", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, nil).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeBuild).Return(nil, utils.ErrNoMatchingJob), jobhelper.EXPECT().CreateJob(ctx, &j).Return(errors.New("some error")), ) @@ -152,7 +152,7 @@ var _ = Describe("Sync", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, nil).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeBuild).Return(nil, utils.ErrNoMatchingJob), jobhelper.EXPECT().CreateJob(ctx, &j).Return(nil), ) @@ -195,7 +195,7 @@ var _ = Describe("Sync", func() { gomock.InOrder( helper.EXPECT().GetRelevantBuild(mod, km).Return(km.Build), - maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, nil).Return(&newJob, nil), + maker.EXPECT().MakeJobTemplate(ctx, mod, km.Build, kernelVersion, km.ContainerImage, true, gomock.Any()).Return(&newJob, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeBuild).Return(&j, nil), jobhelper.EXPECT().IsJobChanged(&j, &newJob).Return(true, nil), jobhelper.EXPECT().DeleteJob(ctx, &j).Return(nil), diff --git a/internal/module/helper.go b/internal/module/helper.go index 394129b95..ab75bef62 100644 --- a/internal/module/helper.go +++ b/internal/module/helper.go @@ -5,8 +5,8 @@ import ( ) func GetRelevantTLSOptions(mod *kmmv1beta1.Module, km *kmmv1beta1.KernelMapping) *kmmv1beta1.TLSOptions { - if km.RegistryTLS != nil { - return km.RegistryTLS + if km.RegistryTLS != (kmmv1beta1.TLSOptions{}) { + return &km.RegistryTLS } - return mod.Spec.ModuleLoader.Container.RegistryTLS + return &mod.Spec.ModuleLoader.Container.RegistryTLS } diff --git a/internal/preflight/preflight_test.go b/internal/preflight/preflight_test.go index 1364fae90..2889d3edb 100644 --- a/internal/preflight/preflight_test.go +++ b/internal/preflight/preflight_test.go @@ -215,7 +215,7 @@ var _ = Describe("preflightHelper_verifyImage", func() { repoConfig := ®istry.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 +228,7 @@ 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 +241,7 @@ var _ = Describe("preflightHelper_verifyImage", func() { digests := []string{"digest0", "digest1"} repoConfig := ®istry.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 +257,7 @@ var _ = Describe("preflightHelper_verifyImage", func() { repoConfig := ®istry.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), ) diff --git a/internal/sign/job/manager.go b/internal/sign/job/manager.go index 1d0df5780..d9ebf5a57 100644 --- a/internal/sign/job/manager.go +++ b/internal/sign/job/manager.go @@ -25,7 +25,8 @@ func NewSignJobManager(signer Signer, helper sign.Helper, jobHelper utils.JobHel } } -func (jbm *signJobManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel string, imageToSign string, targetImage string, pushImage bool) (utils.Result, error) { +func (jbm *signJobManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m kmmv1beta1.KernelMapping, targetKernel, + imageToSign, targetImage string, pushImage bool) (utils.Result, error) { logger := log.FromContext(ctx) logger.Info("Signing in-cluster") @@ -33,7 +34,8 @@ func (jbm *signJobManager) Sync(ctx context.Context, mod kmmv1beta1.Module, m km signConfig := jbm.helper.GetRelevantSign(mod, m) jobLabels := jbm.jobHelper.JobLabels(mod, targetKernel, "sign") - jobTemplate, err := jbm.signer.MakeJobTemplate(mod, signConfig, targetKernel, imageToSign, targetImage, jobLabels, pushImage) + jobTemplate, err := jbm.signer.MakeJobTemplate(mod, signConfig, targetKernel, imageToSign, targetImage, + jobLabels, pushImage, &m.RegistryTLS) if err != nil { return utils.Result{}, fmt.Errorf("could not make Job template: %v", err) } diff --git a/internal/sign/job/manager_test.go b/internal/sign/job/manager_test.go index 54b7cc7e6..546e49618 100644 --- a/internal/sign/job/manager_test.go +++ b/internal/sign/job/manager_test.go @@ -90,7 +90,8 @@ var _ = Describe("JobManager", func() { helper.EXPECT().GetRelevantSign(mod, km).Return(km.Sign), jobhelper.EXPECT().JobLabels(mod, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, labels, true).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, + labels, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeSign).Return(&newJob, nil), jobhelper.EXPECT().IsJobChanged(&j, &newJob).Return(false, nil), jobhelper.EXPECT().GetJobStatus(&newJob).Return(r.Status, r.Requeue, joberr), @@ -119,7 +120,8 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantSign(mod, km).Return(km.Sign), jobhelper.EXPECT().JobLabels(mod, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, labels, true).Return(nil, errors.New("random error")), + maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, + labels, true, gomock.Any()).Return(nil, errors.New("random error")), ) mgr := NewSignJobManager(maker, helper, jobhelper) @@ -147,7 +149,8 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantSign(mod, km).Return(km.Sign), jobhelper.EXPECT().JobLabels(mod, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, labels, true).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, + labels, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeSign).Return(nil, errors.New("random error")), ) @@ -176,7 +179,8 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantSign(mod, km).Return(km.Sign), jobhelper.EXPECT().JobLabels(mod, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, labels, true).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, + labels, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeSign).Return(nil, utils.ErrNoMatchingJob), jobhelper.EXPECT().CreateJob(ctx, &j).Return(errors.New("unable to create job")), ) @@ -207,7 +211,8 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantSign(mod, km).Return(km.Sign), jobhelper.EXPECT().JobLabels(mod, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, labels, true).Return(&j, nil), + maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, + labels, true, gomock.Any()).Return(&j, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeSign).Return(nil, utils.ErrNoMatchingJob), jobhelper.EXPECT().CreateJob(ctx, &j).Return(nil), ) @@ -239,7 +244,8 @@ var _ = Describe("JobManager", func() { gomock.InOrder( helper.EXPECT().GetRelevantSign(mod, km).Return(km.Sign), jobhelper.EXPECT().JobLabels(mod, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, labels, true).Return(&newJob, nil), + maker.EXPECT().MakeJobTemplate(mod, km.Sign, kernelVersion, previousImageName, km.ContainerImage, + labels, true, gomock.Any()).Return(&newJob, nil), jobhelper.EXPECT().GetModuleJobByKernel(ctx, mod, kernelVersion, utils.JobTypeSign).Return(&newJob, nil), jobhelper.EXPECT().IsJobChanged(&newJob, &newJob).Return(true, nil), jobhelper.EXPECT().DeleteJob(ctx, &newJob).Return(nil), diff --git a/internal/sign/job/mock_signer.go b/internal/sign/job/mock_signer.go index 8e41ed796..17ccfe71d 100644 --- a/internal/sign/job/mock_signer.go +++ b/internal/sign/job/mock_signer.go @@ -36,16 +36,16 @@ func (m *MockSigner) EXPECT() *MockSignerMockRecorder { } // MakeJobTemplate mocks base method. -func (m *MockSigner) MakeJobTemplate(mod v1beta1.Module, signConfig *v1beta1.Sign, targetKernel, imageToSign, targetImage string, labels map[string]string, pushImage bool) (*v1.Job, error) { +func (m *MockSigner) MakeJobTemplate(mod v1beta1.Module, signConfig *v1beta1.Sign, targetKernel, imageToSign, targetImage string, labels map[string]string, pushImage bool, registryTLS *v1beta1.TLSOptions) (*v1.Job, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MakeJobTemplate", mod, signConfig, targetKernel, imageToSign, targetImage, labels, pushImage) + ret := m.ctrl.Call(m, "MakeJobTemplate", mod, signConfig, targetKernel, imageToSign, targetImage, labels, pushImage, registryTLS) ret0, _ := ret[0].(*v1.Job) ret1, _ := ret[1].(error) return ret0, ret1 } // MakeJobTemplate indicates an expected call of MakeJobTemplate. -func (mr *MockSignerMockRecorder) MakeJobTemplate(mod, signConfig, targetKernel, imageToSign, targetImage, labels, pushImage interface{}) *gomock.Call { +func (mr *MockSignerMockRecorder) MakeJobTemplate(mod, signConfig, targetKernel, imageToSign, targetImage, labels, pushImage, registryTLS interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MakeJobTemplate", reflect.TypeOf((*MockSigner)(nil).MakeJobTemplate), mod, signConfig, targetKernel, imageToSign, targetImage, labels, pushImage) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MakeJobTemplate", reflect.TypeOf((*MockSigner)(nil).MakeJobTemplate), mod, signConfig, targetKernel, imageToSign, targetImage, labels, pushImage, registryTLS) } diff --git a/internal/sign/job/signer.go b/internal/sign/job/signer.go index d2be26a38..9b85c50b4 100644 --- a/internal/sign/job/signer.go +++ b/internal/sign/job/signer.go @@ -17,7 +17,8 @@ import ( //go:generate mockgen -source=signer.go -package=signjob -destination=mock_signer.go type Signer interface { - MakeJobTemplate(mod kmmv1beta1.Module, signConfig *kmmv1beta1.Sign, targetKernel string, imageToSign string, targetImage string, labels map[string]string, pushImage bool) (*batchv1.Job, error) + MakeJobTemplate(mod kmmv1beta1.Module, signConfig *kmmv1beta1.Sign, targetKernel, imageToSign, targetImage string, + labels map[string]string, pushImage bool, registryTLS *kmmv1beta1.TLSOptions) (*batchv1.Job, error) } type signer struct { @@ -28,7 +29,9 @@ func NewSigner(scheme *runtime.Scheme) Signer { return &signer{scheme: scheme} } -func (m *signer) MakeJobTemplate(mod kmmv1beta1.Module, signConfig *kmmv1beta1.Sign, targetKernel string, imageToSign string, targetImage string, labels map[string]string, pushImage bool) (*batchv1.Job, error) { +func (m *signer) MakeJobTemplate(mod kmmv1beta1.Module, signConfig *kmmv1beta1.Sign, targetKernel, imageToSign, targetImage string, + labels map[string]string, pushImage bool, registryTLS *kmmv1beta1.TLSOptions) (*batchv1.Job, error) { + var args []string if pushImage { @@ -66,6 +69,22 @@ func (m *signer) MakeJobTemplate(mod kmmv1beta1.Module, signConfig *kmmv1beta1.S volumeMounts = append(volumeMounts, utils.MakeSecretVolumeMount(mod.Spec.ImageRepoSecret, "/docker_config")) } + if signConfig.UnsignedImageRegistryTLS.Insecure { + args = append(args, "--insecure-pull") + } + + if signConfig.UnsignedImageRegistryTLS.InsecureSkipTLSVerify { + args = append(args, "--skip-tls-verify-pull") + } + + if registryTLS.Insecure { + args = append(args, "--insecure") + } + + if registryTLS.InsecureSkipTLSVerify { + args = append(args, "--skip-tls-verify") + } + job := &batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ GenerateName: mod.Name + "-sign-", diff --git a/internal/sign/job/signer_test.go b/internal/sign/job/signer_test.go index deae97531..35afb1465 100644 --- a/internal/sign/job/signer_test.go +++ b/internal/sign/job/signer_test.go @@ -186,7 +186,7 @@ var _ = Describe("MakeJobTemplate", func() { mod := mod.DeepCopy() mod.Spec.Selector = nodeSelector - actual, err := m.MakeJobTemplate(*mod, km.Sign, kernelVersion, unsignedImage, signedImage, labels, true) + actual, err := m.MakeJobTemplate(*mod, km.Sign, kernelVersion, unsignedImage, signedImage, labels, true, &kmmv1beta1.TLSOptions{}) Expect(err).NotTo(HaveOccurred()) Expect( @@ -214,7 +214,7 @@ var _ = Describe("MakeJobTemplate", func() { FilesToSign: filelist, } - actual, err := m.MakeJobTemplate(mod, signConfig, kernelVersion, "", signedImage, labels, pushFlag) + actual, err := m.MakeJobTemplate(mod, signConfig, kernelVersion, "", signedImage, labels, pushFlag, &kmmv1beta1.TLSOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement("-unsignedimage")) Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement("-key")) @@ -247,4 +247,41 @@ var _ = Describe("MakeJobTemplate", func() { false, ), ) + + DescribeTable("should set correct kaniko TLS flags", func(registryTLS *kmmv1beta1.TLSOptions, s kmmv1beta1.Sign, kanikoFlag string) { + + actual, err := m.MakeJobTemplate(mod, &s, kernelVersion, unsignedImage, signedImage, labels, false, registryTLS) + Expect(err).NotTo(HaveOccurred()) + Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement(kanikoFlag)) + + }, + Entry( + "UnsignedImageRegistryTLS.Insecure", + &kmmv1beta1.TLSOptions{}, + kmmv1beta1.Sign{ + UnsignedImageRegistryTLS: kmmv1beta1.TLSOptions{Insecure: true}, + }, + "--insecure-pull", + ), + Entry( + "UnsignedImageRegistryTLS.InsecureSkipTLSVerify", + &kmmv1beta1.TLSOptions{}, + kmmv1beta1.Sign{ + UnsignedImageRegistryTLS: kmmv1beta1.TLSOptions{InsecureSkipTLSVerify: true}, + }, + "--skip-tls-verify-pull", + ), + Entry( + "RegistryTLS.Insecure", + &kmmv1beta1.TLSOptions{Insecure: true}, + nil, + "--insecure", + ), + Entry( + "RegistryTLS.InsecureSkipTLSVerify", + &kmmv1beta1.TLSOptions{InsecureSkipTLSVerify: true}, + nil, + "--skip-tls-verify", + ), + ) })