Skip to content

Commit

Permalink
Adding TLS options to the sign logic.
Browse files Browse the repository at this point in the history 
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 <yonibettan@gmail.com>
  • Loading branch information
@ybettan
ybettan committed Nov 27, 2022
1 parent 2642a7d commit 91dce79
Show file tree
Hide file tree
Showing 16 changed files with 184 additions and 62 deletions.
8 changes: 6 additions & 2 deletions api/v1beta1/module_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 3 additions & 10 deletions api/v1beta1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions config/crd/bases/kmm.sigs.k8s.io_managedclustermodules.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions config/crd/bases/kmm.sigs.k8s.io_modules.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
20 changes: 10 additions & 10 deletions controllers/module_reconciler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
)
Expand All @@ -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),
)
Expand All @@ -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),
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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),
)
Expand All @@ -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),
)
Expand All @@ -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),
)
Expand All @@ -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),
)
Expand All @@ -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),
)
Expand Down
12 changes: 5 additions & 7 deletions internal/build/job/maker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 4 additions & 4 deletions internal/build/job/maker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -285,7 +285,7 @@ var _ = Describe("MakeJobTemplate", func() {
},
Entry(
"BaseImageRegistryTLS.Insecure",
nil,
&kmmv1beta1.TLSOptions{},
kmmv1beta1.Build{
BaseImageRegistryTLS: kmmv1beta1.TLSOptions{Insecure: true},
DockerfileConfigMap: &dockerfileConfigMap,
Expand All @@ -295,7 +295,7 @@ var _ = Describe("MakeJobTemplate", func() {
),
Entry(
"BaseImageRegistryTLS.InsecureSkipTLSVerify",
nil,
&kmmv1beta1.TLSOptions{},
kmmv1beta1.Build{
BaseImageRegistryTLS: kmmv1beta1.TLSOptions{InsecureSkipTLSVerify: true},
DockerfileConfigMap: &dockerfileConfigMap,
Expand Down Expand Up @@ -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))
})
Expand Down
2 changes: 1 addition & 1 deletion internal/build/job/manager.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
10 changes: 5 additions & 5 deletions internal/build/job/manager_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand All @@ -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)
Expand All @@ -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")),
)
Expand Down Expand Up @@ -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),
)
Expand Down Expand Up @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions internal/module/helper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 91dce79

Please sign in to comment.