Introducing buildsign package

In the current implementation of the build and sign flow, build_sign
controller uses 2 packages, build and sign, to verify whether build/sign
must be executed, to actually create build/sign pod and to run garbage
collector on he successfull pods. In many cases, those 2 packages are
very much the same, especially in where the package manager must decide
what to do (build pod, delete pod, etc)
With the introduction of the MBSC controller, the flow is changing and
it makes sense to unify those packages into one package. There is not
need to decide if the build/sign should be executed, since that decision
is taken by the MIC controller

This PR starts with creating the buildsing package and unifying Helper
interfaces both in sign and build package into one Helper interface:
1) create Helper interface and implementation for buildsign package
2) move the existing code that was previously using build.Helper and
   sign.Helper interface to start using unified interface

Author: yevgeny-shnaidman

cmd/manager-hub/main.go

@@ -36,8 +36,8 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
 	"github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/build"
 	buildpod "github.com/kubernetes-sigs/kernel-module-management/internal/build/pod"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/buildsign"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/cluster"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/cmd"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
@@ -48,7 +48,6 @@ import (
 	"github.com/kubernetes-sigs/kernel-module-management/internal/module"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/nmc"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/sign"
 	signpod "github.com/kubernetes-sigs/kernel-module-management/internal/sign/pod"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater"
 	//+kubebuilder:scaffold:imports
@@ -111,11 +110,11 @@ func main() {
 
 	registryAPI := registry.NewRegistry()
 	buildSignPodAPI := pod.NewBuildSignPodManager(client)
-	buildHelper := build.NewHelper()
+	buildSignHelper := buildsign.NewHelper()
 
 	buildAPI := buildpod.NewBuildManager(
 		client,
-		buildpod.NewMaker(client, buildHelper, buildSignPodAPI, scheme),
+		buildpod.NewMaker(client, buildSignHelper, buildSignPodAPI, scheme),
 		buildSignPodAPI,
 		registryAPI,
 	)
@@ -127,7 +126,7 @@ func main() {
 		registryAPI,
 	)
 
-	kernelAPI := module.NewKernelMapper(buildHelper, sign.NewSignerHelper())
+	kernelAPI := module.NewKernelMapper(buildSignHelper)
 
 	ctrlLogger := setupLogger.WithValues("name", hub.ManagedClusterModuleReconcilerName)
 	ctrlLogger.Info("Adding controller")

cmd/manager/main.go

@@ -28,8 +28,8 @@ import (
 
 	"github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
 	"github.com/kubernetes-sigs/kernel-module-management/api/v1beta2"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/build"
 	buildpod "github.com/kubernetes-sigs/kernel-module-management/internal/build/pod"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/buildsign"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/cmd"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/config"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/constants"
@@ -40,7 +40,6 @@ import (
 	"github.com/kubernetes-sigs/kernel-module-management/internal/nmc"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/preflight"
 	"github.com/kubernetes-sigs/kernel-module-management/internal/registry"
-	"github.com/kubernetes-sigs/kernel-module-management/internal/sign"
 	signpod "github.com/kubernetes-sigs/kernel-module-management/internal/sign/pod"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -120,9 +119,9 @@ func main() {
 	metricsAPI.Register()
 
 	registryAPI := registry.NewRegistry()
-	buildHelperAPI := build.NewHelper()
+	buildSignHelperAPI := buildsign.NewHelper()
 	nodeAPI := node.NewNode(client)
-	kernelAPI := module.NewKernelMapper(buildHelperAPI, sign.NewSignerHelper())
+	kernelAPI := module.NewKernelMapper(buildSignHelperAPI)
 	micAPI := mic.New(client, scheme)
 
 	dpc := controllers.NewDevicePluginReconciler(
@@ -183,7 +182,7 @@ func main() {
 
 		buildAPI := buildpod.NewBuildManager(
 			client,
-			buildpod.NewMaker(client, buildHelperAPI, buildSignPodAPI, scheme),
+			buildpod.NewMaker(client, buildSignHelperAPI, buildSignPodAPI, scheme),
 			buildSignPodAPI,
 			registryAPI,
 		)

internal/buildsign/helper.go

@@ -0,0 +1,109 @@
+package buildsign
+
+import (
+	"k8s.io/apimachinery/pkg/util/sets"
+
+	kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/kernel"
+	"github.com/kubernetes-sigs/kernel-module-management/internal/utils"
+)
+
+//go:generate mockgen -source=helper.go -package=buildsign -destination=mock_helper.go
+
+type Helper interface {
+	ApplyBuildArgOverrides(args []kmmv1beta1.BuildArg, overrides ...kmmv1beta1.BuildArg) []kmmv1beta1.BuildArg
+	GetRelevantBuild(moduleBuild *kmmv1beta1.Build, mappingBuild *kmmv1beta1.Build) *kmmv1beta1.Build
+	GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernel string) (*kmmv1beta1.Sign, error)
+}
+
+type helper struct{}
+
+func NewHelper() Helper {
+	return &helper{}
+}
+
+func (m *helper) ApplyBuildArgOverrides(args []kmmv1beta1.BuildArg, overrides ...kmmv1beta1.BuildArg) []kmmv1beta1.BuildArg {
+	overridesMap := make(map[string]kmmv1beta1.BuildArg, len(overrides))
+
+	for _, o := range overrides {
+		overridesMap[o.Name] = o
+	}
+
+	unusedOverrides := sets.StringKeySet(overridesMap)
+
+	for i := 0; i < len(args); i++ {
+		argName := args[i].Name
+
+		if o, ok := overridesMap[argName]; ok {
+			args[i] = o
+			unusedOverrides.Delete(argName)
+		}
+	}
+
+	for _, overrideName := range unusedOverrides.List() {
+		args = append(args, overridesMap[overrideName])
+	}
+
+	return args
+}
+
+func (m *helper) GetRelevantBuild(moduleBuild *kmmv1beta1.Build, mappingBuild *kmmv1beta1.Build) *kmmv1beta1.Build {
+	if moduleBuild == nil {
+		return mappingBuild.DeepCopy()
+	}
+
+	if mappingBuild == nil {
+		return moduleBuild.DeepCopy()
+	}
+
+	buildConfig := moduleBuild.DeepCopy()
+	if mappingBuild.DockerfileConfigMap != nil {
+		buildConfig.DockerfileConfigMap = mappingBuild.DockerfileConfigMap
+	}
+
+	buildConfig.BuildArgs = m.ApplyBuildArgOverrides(buildConfig.BuildArgs, mappingBuild.BuildArgs...)
+
+	buildConfig.Secrets = append(buildConfig.Secrets, mappingBuild.Secrets...)
+	return buildConfig
+}
+
+func (m *helper) GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernelVersion string) (*kmmv1beta1.Sign, error) {
+	var signConfig *kmmv1beta1.Sign
+	if moduleSign == nil {
+		// km.Sign cannot be nil in case mod.Sign is nil, checked above
+		signConfig = mappingSign.DeepCopy()
+	} else if mappingSign == nil {
+		signConfig = moduleSign.DeepCopy()
+	} else {
+		signConfig = moduleSign.DeepCopy()
+
+		if mappingSign.UnsignedImage != "" {
+			signConfig.UnsignedImage = mappingSign.UnsignedImage
+		}
+
+		if mappingSign.KeySecret != nil {
+			signConfig.KeySecret = mappingSign.KeySecret
+		}
+		if mappingSign.CertSecret != nil {
+			signConfig.CertSecret = mappingSign.CertSecret
+		}
+		//append (not overwrite) any files in the km to the defaults
+		signConfig.FilesToSign = append(signConfig.FilesToSign, mappingSign.FilesToSign...)
+	}
+
+	osConfigEnvVars := utils.KernelComponentsAsEnvVars(
+		kernel.NormalizeVersion(kernelVersion),
+	)
+	unsignedImage, err := utils.ReplaceInTemplates(osConfigEnvVars, signConfig.UnsignedImage)
+	if err != nil {
+		return nil, err
+	}
+	signConfig.UnsignedImage = unsignedImage[0]
+	filesToSign, err := utils.ReplaceInTemplates(osConfigEnvVars, signConfig.FilesToSign...)
+	if err != nil {
+		return nil, err
+	}
+	signConfig.FilesToSign = filesToSign
+
+	return signConfig, nil
+}

internal/buildsign/helper_test.go

@@ -0,0 +1,208 @@
+package buildsign
+
+import (
+	"strings"
+
+	"github.com/google/go-cmp/cmp"
+	kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	v1 "k8s.io/api/core/v1"
+)
+
+var _ = Describe("GetRelevantBuild", func() {
+
+	var nh Helper
+
+	BeforeEach(func() {
+		nh = NewHelper()
+	})
+
+	It("kernel mapping build present, module loader build absent", func() {
+		mappingBuild := &kmmv1beta1.Build{
+			DockerfileConfigMap: &v1.LocalObjectReference{Name: "some kernel mapping build name"},
+		}
+
+		res := nh.GetRelevantBuild(nil, mappingBuild)
+		Expect(res).To(Equal(mappingBuild))
+	})
+
+	It("kernel mapping build absent, module loader build present", func() {
+		moduleBuild := &kmmv1beta1.Build{
+			DockerfileConfigMap: &v1.LocalObjectReference{Name: "some load module build name"},
+		}
+
+		res := nh.GetRelevantBuild(moduleBuild, nil)
+
+		Expect(res).To(Equal(moduleBuild))
+	})
+
+	It("kernel mapping and module loader builds are present, overrides", func() {
+		moduleBuild := &kmmv1beta1.Build{
+			DockerfileConfigMap: &v1.LocalObjectReference{Name: "some load module build name"},
+			BaseImageRegistryTLS: kmmv1beta1.TLSOptions{
+				Insecure:              true,
+				InsecureSkipTLSVerify: true,
+			},
+		}
+		mappingBuild := &kmmv1beta1.Build{
+			DockerfileConfigMap: &v1.LocalObjectReference{Name: "some kernel mapping build name"},
+		}
+
+		res := nh.GetRelevantBuild(moduleBuild, mappingBuild)
+		Expect(res.DockerfileConfigMap).To(Equal(mappingBuild.DockerfileConfigMap))
+		Expect(res.BaseImageRegistryTLS).To(Equal(moduleBuild.BaseImageRegistryTLS))
+	})
+})
+
+var _ = Describe("ApplyBuildArgOverrides", func() {
+
+	var nh Helper
+
+	BeforeEach(func() {
+		nh = NewHelper()
+	})
+
+	It("apply overrides", func() {
+		args := []kmmv1beta1.BuildArg{
+			{
+				Name:  "name1",
+				Value: "value1",
+			},
+			{
+				Name:  "name2",
+				Value: "value2",
+			},
+		}
+		overrides := []kmmv1beta1.BuildArg{
+			{
+				Name:  "name1",
+				Value: "valueOverride1",
+			},
+			{
+				Name:  "overrideName2",
+				Value: "overrideValue2",
+			},
+		}
+
+		expected := []kmmv1beta1.BuildArg{
+			{
+				Name:  "name1",
+				Value: "valueOverride1",
+			},
+			{
+				Name:  "name2",
+				Value: "value2",
+			},
+			{
+				Name:  "overrideName2",
+				Value: "overrideValue2",
+			},
+		}
+
+		res := nh.ApplyBuildArgOverrides(args, overrides...)
+		Expect(res).To(Equal(expected))
+	})
+})
+
+var _ = Describe("GetRelevantSign", func() {
+
+	const (
+		unsignedImage = "my.registry/my/image"
+		keySecret     = "securebootkey"
+		certSecret    = "securebootcert"
+		filesToSign   = "/modules/simple-kmod.ko:/modules/simple-procfs-kmod.ko"
+		kernelVersion = "1.2.3"
+	)
+
+	var (
+		h Helper
+	)
+
+	BeforeEach(func() {
+		h = NewHelper()
+	})
+
+	expected := &kmmv1beta1.Sign{
+		UnsignedImage: unsignedImage,
+		KeySecret:     &v1.LocalObjectReference{Name: keySecret},
+		CertSecret:    &v1.LocalObjectReference{Name: certSecret},
+		FilesToSign:   strings.Split(filesToSign, ":"),
+	}
+
+	DescribeTable("should set fields correctly", func(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign) {
+		actual, err := h.GetRelevantSign(moduleSign, mappingSign, kernelVersion)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			cmp.Diff(expected, actual),
+		).To(
+			BeEmpty(),
+		)
+	},
+		Entry(
+			"no km.Sign",
+			&kmmv1beta1.Sign{
+				UnsignedImage: unsignedImage,
+				KeySecret:     &v1.LocalObjectReference{Name: keySecret},
+				CertSecret:    &v1.LocalObjectReference{Name: certSecret},
+				FilesToSign:   strings.Split(filesToSign, ":"),
+			},
+			nil,
+		),
+		Entry(
+			"no container.Sign",
+			nil,
+			&kmmv1beta1.Sign{
+				UnsignedImage: unsignedImage,
+				KeySecret:     &v1.LocalObjectReference{Name: keySecret},
+				CertSecret:    &v1.LocalObjectReference{Name: certSecret},
+				FilesToSign:   strings.Split(filesToSign, ":"),
+			},
+		),
+		Entry(
+			"default UnsignedImage",
+			&kmmv1beta1.Sign{
+				UnsignedImage: unsignedImage,
+			},
+			&kmmv1beta1.Sign{
+				KeySecret:   &v1.LocalObjectReference{Name: keySecret},
+				CertSecret:  &v1.LocalObjectReference{Name: certSecret},
+				FilesToSign: strings.Split(filesToSign, ":"),
+			},
+		),
+		Entry(
+			"default UnsignedImage and KeySecret",
+			&kmmv1beta1.Sign{
+				UnsignedImage: unsignedImage,
+				KeySecret:     &v1.LocalObjectReference{Name: keySecret},
+			},
+			&kmmv1beta1.Sign{
+				CertSecret:  &v1.LocalObjectReference{Name: certSecret},
+				FilesToSign: strings.Split(filesToSign, ":"),
+			},
+		),
+		Entry(
+			"default UnsignedImage, KeySecret, and CertSecret",
+			&kmmv1beta1.Sign{
+				UnsignedImage: unsignedImage,
+				KeySecret:     &v1.LocalObjectReference{Name: keySecret},
+				CertSecret:    &v1.LocalObjectReference{Name: certSecret},
+			},
+			&kmmv1beta1.Sign{
+				FilesToSign: strings.Split(filesToSign, ":"),
+			},
+		),
+		Entry(
+			"default FilesToSign only",
+			&kmmv1beta1.Sign{
+				FilesToSign: strings.Split(filesToSign, ":"),
+			},
+			&kmmv1beta1.Sign{
+				UnsignedImage: unsignedImage,
+				KeySecret:     &v1.LocalObjectReference{Name: keySecret},
+				CertSecret:    &v1.LocalObjectReference{Name: certSecret},
+			},
+		),
+	)
+
+})