diff --git a/cmd/manager-hub/main.go b/cmd/manager-hub/main.go index 3826a41ce..200fefbb9 100644 --- a/cmd/manager-hub/main.go +++ b/cmd/manager-hub/main.go @@ -19,6 +19,7 @@ package main import ( "flag" + "github.com/kubernetes-sigs/kernel-module-management/internal/api" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -35,7 +36,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1" "github.com/kubernetes-sigs/kernel-module-management/controllers/hub" - "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/build/job" "github.com/kubernetes-sigs/kernel-module-management/internal/cluster" "github.com/kubernetes-sigs/kernel-module-management/internal/cmd" @@ -43,9 +43,7 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/filter" "github.com/kubernetes-sigs/kernel-module-management/internal/manifestwork" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" - "github.com/kubernetes-sigs/kernel-module-management/internal/module" "github.com/kubernetes-sigs/kernel-module-management/internal/registry" - "github.com/kubernetes-sigs/kernel-module-management/internal/sign" signjob "github.com/kubernetes-sigs/kernel-module-management/internal/sign/job" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" "github.com/kubernetes-sigs/kernel-module-management/internal/utils" @@ -106,11 +104,10 @@ func main() { registryAPI := registry.NewRegistry() jobHelperAPI := utils.NewJobHelper(client) - buildHelper := build.NewHelper() buildAPI := job.NewBuildManager( client, - job.NewMaker(client, buildHelper, jobHelperAPI, scheme), + job.NewMaker(client, jobHelperAPI, scheme), jobHelperAPI, registryAPI, ) @@ -130,7 +127,7 @@ func main() { mcmr := hub.NewManagedClusterModuleReconciler( client, manifestwork.NewCreator(client, scheme), - cluster.NewClusterAPI(client, module.NewKernelMapper(buildHelper, sign.NewSignerHelper()), buildAPI, signAPI, operatorNamespace), + cluster.NewClusterAPI(client, api.NewModuleLoaderDataCreator(), buildAPI, signAPI, operatorNamespace), statusupdater.NewManagedClusterModuleStatusUpdater(client), filterAPI, ) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 39a289719..5a70676c1 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -22,6 +22,7 @@ import ( "os" "strconv" + "github.com/kubernetes-sigs/kernel-module-management/internal/api" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -37,17 +38,14 @@ import ( v1beta12 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" "github.com/kubernetes-sigs/kernel-module-management/controllers" - "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/build/job" "github.com/kubernetes-sigs/kernel-module-management/internal/cmd" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/filter" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" - "github.com/kubernetes-sigs/kernel-module-management/internal/module" "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" signjob "github.com/kubernetes-sigs/kernel-module-management/internal/sign/job" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" "github.com/kubernetes-sigs/kernel-module-management/internal/utils" @@ -114,11 +112,10 @@ func main() { registryAPI := registry.NewRegistry() jobHelperAPI := utils.NewJobHelper(client) - buildHelperAPI := build.NewHelper() buildAPI := job.NewBuildManager( client, - job.NewMaker(client, buildHelperAPI, jobHelperAPI, scheme), + job.NewMaker(client, jobHelperAPI, scheme), jobHelperAPI, registryAPI, ) @@ -131,14 +128,14 @@ func main() { ) daemonAPI := daemonset.NewCreator(client, constants.KernelLabel, scheme) - kernelAPI := module.NewKernelMapper(buildHelperAPI, sign.NewSignerHelper()) + mldCreator := api.NewModuleLoaderDataCreator() mc := controllers.NewModuleReconciler( client, buildAPI, signAPI, daemonAPI, - kernelAPI, + mldCreator, metricsAPI, filterAPI, statusupdater.NewModuleStatusUpdater(client), @@ -164,7 +161,7 @@ func main() { } preflightStatusUpdaterAPI := statusupdater.NewPreflightStatusUpdater(client) - preflightAPI := preflight.NewPreflightAPI(client, buildAPI, signAPI, registryAPI, preflightStatusUpdaterAPI, kernelAPI) + preflightAPI := preflight.NewPreflightAPI(client, buildAPI, signAPI, registryAPI, preflightStatusUpdaterAPI, mldCreator) if err = controllers.NewPreflightValidationReconciler(client, filterAPI, metricsAPI, preflightStatusUpdaterAPI, preflightAPI).SetupWithManager(mgr); err != nil { cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.PreflightValidationReconcilerName) diff --git a/controllers/module_reconciler.go b/controllers/module_reconciler.go index 8fa9154a1..55079d6a1 100644 --- a/controllers/module_reconciler.go +++ b/controllers/module_reconciler.go @@ -27,7 +27,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/filter" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" - "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/statusupdater" "github.com/kubernetes-sigs/kernel-module-management/internal/utils" @@ -66,13 +65,13 @@ func NewModuleReconciler( buildAPI build.Manager, signAPI sign.SignManager, daemonAPI daemonset.DaemonSetCreator, - kernelAPI module.KernelMapper, + mldCreator api.ModuleLoaderDataCreator, metricsAPI metrics.Metrics, filter *filter.Filter, statusUpdaterAPI statusupdater.ModuleStatusUpdater, operatorNamespace string, ) *ModuleReconciler { - reconHelperAPI := newModuleReconcilerHelper(client, buildAPI, signAPI, daemonAPI, kernelAPI, metricsAPI) + reconHelperAPI := newModuleReconcilerHelper(client, buildAPI, signAPI, daemonAPI, mldCreator, metricsAPI) return &ModuleReconciler{ daemonAPI: daemonAPI, reconHelperAPI: reconHelperAPI, @@ -200,7 +199,7 @@ type moduleReconcilerHelper struct { buildAPI build.Manager signAPI sign.SignManager daemonAPI daemonset.DaemonSetCreator - kernelAPI module.KernelMapper + mldCreator api.ModuleLoaderDataCreator metricsAPI metrics.Metrics } @@ -208,14 +207,14 @@ func newModuleReconcilerHelper(client client.Client, buildAPI build.Manager, signAPI sign.SignManager, daemonAPI daemonset.DaemonSetCreator, - kernelAPI module.KernelMapper, + mldCreator api.ModuleLoaderDataCreator, metricsAPI metrics.Metrics) moduleReconcilerHelperAPI { return &moduleReconcilerHelper{ client: client, buildAPI: buildAPI, signAPI: signAPI, daemonAPI: daemonAPI, - kernelAPI: kernelAPI, + mldCreator: mldCreator, metricsAPI: metricsAPI, } } @@ -243,7 +242,7 @@ func (mrh *moduleReconcilerHelper) getRelevantKernelMappingsAndNodes(ctx context continue } - mld, err := mrh.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) + mld, err := mrh.mldCreator.FromModule(mod, kernelVersion) if err != nil { nodeLogger.Error(err, "failed to get and process kernel mapping") continue @@ -320,10 +319,9 @@ func (mrh *moduleReconcilerHelper) handleSigning(ctx context.Context, mld *api.M return true, nil } - // if we need to sign AND we've built, then we must have built the intermediate image so must figure out its name - previousImage := "" - if module.ShouldBeBuilt(mld) { - previousImage = module.IntermediateImageName(mld.Name, mld.Namespace, mld.ContainerImage) + previousImage, err := mld.UnsignedImage() + if err != nil { + return false, fmt.Errorf("could not determine the unsigned image: %v", err) } logger := log.FromContext(ctx).WithValues("kernel version", mld.KernelVersion, "image", mld.ContainerImage) diff --git a/controllers/module_reconciler_test.go b/controllers/module_reconciler_test.go index c24a67ef0..1a1ab7ed8 100644 --- a/controllers/module_reconciler_test.go +++ b/controllers/module_reconciler_test.go @@ -12,7 +12,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/client" "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" "github.com/kubernetes-sigs/kernel-module-management/internal/metrics" - "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/statusupdater" "github.com/kubernetes-sigs/kernel-module-management/internal/utils" @@ -286,15 +285,15 @@ var _ = Describe("ModuleReconciler_getNodesListBySelector", func() { var _ = Describe("ModuleReconciler_getRelevantKernelMappingsAndNodes", func() { var ( - ctrl *gomock.Controller - mockKM *module.MockKernelMapper - mhr moduleReconcilerHelperAPI + ctrl *gomock.Controller + mockMLDCreator *api.MockModuleLoaderDataCreator + mhr moduleReconcilerHelperAPI ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockKM = module.NewMockKernelMapper(ctrl) - mhr = newModuleReconcilerHelper(nil, nil, nil, nil, mockKM, nil) + mockMLDCreator = api.NewMockModuleLoaderDataCreator(ctrl) + mhr = newModuleReconcilerHelper(nil, nil, nil, nil, mockMLDCreator, nil) }) node1 := v1.Node{ @@ -327,8 +326,8 @@ var _ = Describe("ModuleReconciler_getRelevantKernelMappingsAndNodes", func() { expectedNodes := []v1.Node{node1, node2, node3} expectedMappings := map[string]*api.ModuleLoaderData{"kernelVersion1": &mld1, "kernelVersion2": &mld2} gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&kmmv1beta1.Module{}, node1.Status.NodeInfo.KernelVersion).Return(&mld1, nil), - mockKM.EXPECT().GetModuleLoaderDataForKernel(&kmmv1beta1.Module{}, node2.Status.NodeInfo.KernelVersion).Return(&mld2, nil), + mockMLDCreator.EXPECT().FromModule(&kmmv1beta1.Module{}, node1.Status.NodeInfo.KernelVersion).Return(&mld1, nil), + mockMLDCreator.EXPECT().FromModule(&kmmv1beta1.Module{}, node2.Status.NodeInfo.KernelVersion).Return(&mld2, nil), ) mappings, resNodes, err := mhr.getRelevantKernelMappingsAndNodes(context.Background(), &kmmv1beta1.Module{}, nodes) @@ -344,8 +343,8 @@ var _ = Describe("ModuleReconciler_getRelevantKernelMappingsAndNodes", func() { expectedNodes := []v1.Node{node1, node3} expectedMappings := map[string]*api.ModuleLoaderData{"kernelVersion1": &mld1} gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&kmmv1beta1.Module{}, node1.Status.NodeInfo.KernelVersion).Return(&mld1, nil), - mockKM.EXPECT().GetModuleLoaderDataForKernel(&kmmv1beta1.Module{}, node2.Status.NodeInfo.KernelVersion).Return(nil, fmt.Errorf("some error")), + mockMLDCreator.EXPECT().FromModule(&kmmv1beta1.Module{}, node1.Status.NodeInfo.KernelVersion).Return(&mld1, nil), + mockMLDCreator.EXPECT().FromModule(&kmmv1beta1.Module{}, node2.Status.NodeInfo.KernelVersion).Return(nil, fmt.Errorf("some error")), ) mappings, resNodes, err := mhr.getRelevantKernelMappingsAndNodes(context.Background(), &kmmv1beta1.Module{}, nodes) diff --git a/go.mod b/go.mod index f118f2820..6fbe69506 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.19 require ( github.com/a8m/envsubst v1.4.2 github.com/go-logr/logr v1.2.4 + github.com/go-openapi/swag v0.22.3 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.5.9 github.com/google/go-containerregistry v0.14.0 @@ -13,7 +14,6 @@ require ( github.com/onsi/ginkgo/v2 v2.9.2 github.com/onsi/gomega v1.27.6 github.com/prometheus/client_golang v1.15.0 - golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4 k8s.io/api v0.26.3 k8s.io/apimachinery v0.27.1 k8s.io/client-go v0.26.3 @@ -38,7 +38,6 @@ require ( github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-openapi/jsonpointer v0.19.6 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect - github.com/go-openapi/swag v0.22.3 // indirect github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect diff --git a/go.sum b/go.sum index 542077b23..12f5107cf 100644 --- a/go.sum +++ b/go.sum @@ -209,8 +209,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= -golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4 h1:K3x+yU+fbot38x5bQbU2QqUAVyYLEktdNH2GxZLnM3U= -golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4/go.mod h1:lgLbSvA5ygNOMpwM/9anMpWVlVJ7Z+cHWq/eFuinpGE= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= golang.org/x/lint v0.0.0-20190227174305-5b3e6a55c961/go.mod h1:wehouNa3lNwaWXcvxsM5YxQ5yQlVC4a0KAMCusXpPoU= golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc= diff --git a/internal/api/creator.go b/internal/api/creator.go new file mode 100644 index 000000000..cf88c1b70 --- /dev/null +++ b/internal/api/creator.go @@ -0,0 +1,200 @@ +package api + +import ( + "errors" + "fmt" + "regexp" + + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + buildutils "github.com/kubernetes-sigs/kernel-module-management/internal/build/utils" + "github.com/kubernetes-sigs/kernel-module-management/internal/utils" +) + +//go:generate mockgen -source=creator.go -package=api -destination=mock_creator.go ModuleLoaderDataCreator,moduleLoaderDataCreatorHelper + +type ModuleLoaderDataCreator interface { + FromModule(mod *kmmv1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) +} + +type moduleLoaderDataCreator struct { + helper moduleLoaderDataCreatorHelper +} + +func NewModuleLoaderDataCreator() ModuleLoaderDataCreator { + return &moduleLoaderDataCreator{ + helper: &moduleLoaderDataCreatorHelperImpl{ + sectionGetter: §ionGetterImpl{}, + }, + } +} + +func (f *moduleLoaderDataCreator) FromModule(mod *kmmv1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) { + mappings := mod.Spec.ModuleLoader.Container.KernelMappings + foundMapping, err := f.helper.findKernelMapping(mappings, kernelVersion) + if err != nil { + return nil, fmt.Errorf("failed to find mapping for kernel %s: %v", kernelVersion, err) + } + mld, err := f.helper.prepareModuleLoaderData(foundMapping, mod, kernelVersion) + if err != nil { + return nil, fmt.Errorf("failed to prepare module loader data for kernel %s: %v", kernelVersion, err) + } + + err = f.helper.replaceTemplates(mld) + if err != nil { + return nil, fmt.Errorf("failed to replace templates in module loader data for kernel %s: %v", kernelVersion, err) + } + return mld, nil +} + +type moduleLoaderDataCreatorHelper interface { + findKernelMapping(mappings []kmmv1beta1.KernelMapping, kernelVersion string) (*kmmv1beta1.KernelMapping, error) + prepareModuleLoaderData(mapping *kmmv1beta1.KernelMapping, mod *kmmv1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) + replaceTemplates(mld *ModuleLoaderData) error +} + +type moduleLoaderDataCreatorHelperImpl struct { + sectionGetter sectionGetter +} + +func (kh *moduleLoaderDataCreatorHelperImpl) findKernelMapping(mappings []kmmv1beta1.KernelMapping, kernelVersion string) (*kmmv1beta1.KernelMapping, error) { + for _, m := range mappings { + if m.Literal != "" && m.Literal == kernelVersion { + return &m, nil + } + + if m.Regexp == "" { + continue + } + + if matches, err := regexp.MatchString(m.Regexp, kernelVersion); err != nil { + return nil, fmt.Errorf("could not match regexp %q against kernel %q: %v", m.Regexp, kernelVersion, err) + } else if matches { + return &m, nil + } + } + + return nil, errors.New("no suitable mapping found") +} + +func (kh *moduleLoaderDataCreatorHelperImpl) prepareModuleLoaderData(mapping *kmmv1beta1.KernelMapping, mod *kmmv1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) { + var err error + + mld := &ModuleLoaderData{} + // prepare the build + if mapping.Build != nil || mod.Spec.ModuleLoader.Container.Build != nil { + mld.Build = kh.sectionGetter.GetRelevantBuild(mod.Spec.ModuleLoader.Container.Build, mapping.Build) + } + + // prepare the sign + if mapping.Sign != nil || mod.Spec.ModuleLoader.Container.Sign != nil { + mld.Sign, err = kh.sectionGetter.GetRelevantSign(mod.Spec.ModuleLoader.Container.Sign, mapping.Sign, kernelVersion) + if err != nil { + return nil, fmt.Errorf("failed to get the relevant Sign configuration for kernel %s: %v", kernelVersion, err) + } + } + + // prepare TLS options + mld.RegistryTLS = mapping.RegistryTLS + if mapping.RegistryTLS == nil { + mld.RegistryTLS = &mod.Spec.ModuleLoader.Container.RegistryTLS + } + + //prepare container image + mld.ContainerImage = mapping.ContainerImage + if mapping.ContainerImage == "" { + mld.ContainerImage = mod.Spec.ModuleLoader.Container.ContainerImage + } + + mld.KernelVersion = kernelVersion + mld.Name = mod.Name + mld.Namespace = mod.Namespace + mld.ImagePullPolicy = mod.Spec.ModuleLoader.Container.ImagePullPolicy + mld.ImageRepoSecret = mod.Spec.ImageRepoSecret + mld.Selector = mod.Spec.Selector + mld.ServiceAccountName = mod.Spec.ModuleLoader.ServiceAccountName + mld.Modprobe = mod.Spec.ModuleLoader.Container.Modprobe + mld.Owner = mod + + return mld, nil +} + +func (kh *moduleLoaderDataCreatorHelperImpl) replaceTemplates(mld *ModuleLoaderData) error { + osConfigEnvVars := utils.KernelComponentsAsEnvVars(mld.KernelVersion) + osConfigEnvVars = append(osConfigEnvVars, "MOD_NAME="+mld.Name, "MOD_NAMESPACE="+mld.Namespace) + + replacedContainerImage, err := utils.ReplaceInTemplates(osConfigEnvVars, mld.ContainerImage) + if err != nil { + return fmt.Errorf("failed to substitute templates in the ContainerImage field: %v", err) + } + mld.ContainerImage = replacedContainerImage[0] + + return nil +} + +type sectionGetter interface { + GetRelevantBuild(moduleBuild *kmmv1beta1.Build, mappingBuild *kmmv1beta1.Build) *kmmv1beta1.Build + GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernel string) (*kmmv1beta1.Sign, error) +} + +type sectionGetterImpl struct{} + +func (sg *sectionGetterImpl) 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 = buildutils.ApplyBuildArgOverrides(buildConfig.BuildArgs, mappingBuild.BuildArgs...) + + // [TODO] once MGMT-10832 is consolidated, this code must be revisited. We will decide which + // secret and how to use, and if we need to take care of repeated secrets names + buildConfig.Secrets = append(buildConfig.Secrets, mappingBuild.Secrets...) + return buildConfig +} + +func (sg *sectionGetterImpl) GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernel 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) + 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 +} diff --git a/internal/api/creator_test.go b/internal/api/creator_test.go new file mode 100644 index 000000000..a2ef52feb --- /dev/null +++ b/internal/api/creator_test.go @@ -0,0 +1,345 @@ +package api + +import ( + "strings" + + "github.com/golang/mock/gomock" + "github.com/google/go-cmp/cmp" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + v1 "k8s.io/api/core/v1" + + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" +) + +var _ = Describe("moduleLoaderDataCreatorHelper_findKernelMapping", func() { + const kernelVersion = "1.2.3" + + var ( + ctrl *gomock.Controller + mldfh moduleLoaderDataCreatorHelper + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + mldfh = &moduleLoaderDataCreatorHelperImpl{ + sectionGetter: NewMocksectionGetter(ctrl), + } + }) + + It("one literal mapping", func() { + mapping := kmmv1beta1.KernelMapping{ + Literal: "1.2.3", + } + + m, err := mldfh.findKernelMapping([]kmmv1beta1.KernelMapping{mapping}, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(m).To(Equal(&mapping)) + }) + + It("one regexp mapping", func() { + mapping := kmmv1beta1.KernelMapping{ + Regexp: `1\..*`, + } + + m, err := mldfh.findKernelMapping([]kmmv1beta1.KernelMapping{mapping}, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(m).To(Equal(&mapping)) + }) + + It("should return an error if a regex is invalid", func() { + mapping := kmmv1beta1.KernelMapping{ + Regexp: "invalid)", + } + + m, err := mldfh.findKernelMapping([]kmmv1beta1.KernelMapping{mapping}, kernelVersion) + Expect(err).To(HaveOccurred()) + Expect(m).To(BeNil()) + }) + + It("should return an error if no mapping work", func() { + mappings := []kmmv1beta1.KernelMapping{ + { + Regexp: `1.2.2`, + }, + { + Regexp: `0\..*`, + }, + } + + m, err := mldfh.findKernelMapping(mappings, kernelVersion) + Expect(err).To(MatchError("no suitable mapping found")) + Expect(m).To(BeNil()) + }) +}) + +var _ = Describe("moduleLoaderDataCreatorHelper_prepareModuleLoaderData", func() { + const kernelVersion = "1.2.3" + + var ( + ctrl *gomock.Controller + mod kmmv1beta1.Module + mapping kmmv1beta1.KernelMapping + mldfh *moduleLoaderDataCreatorHelperImpl + sg *MocksectionGetter + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + sg = NewMocksectionGetter(ctrl) + mldfh = &moduleLoaderDataCreatorHelperImpl{sectionGetter: sg} + mod = kmmv1beta1.Module{} + mod.Spec.ModuleLoader.Container.ContainerImage = "spec container image" + mapping = kmmv1beta1.KernelMapping{} + }) + + DescribeTable("prepare mapping", func(buildExistsInMapping, buildExistsInModuleSpec, signExistsInMapping, SignExistsInModuleSpec, + registryTLSExistsInMapping, containerImageExistsInMapping bool) { + build := &kmmv1beta1.Build{ + DockerfileConfigMap: &v1.LocalObjectReference{ + Name: "some name", + }, + } + sign := &kmmv1beta1.Sign{ + UnsignedImage: "some unsigned image", + } + registryTSL := &kmmv1beta1.TLSOptions{ + Insecure: true, + } + + mld := ModuleLoaderData{ + Name: mod.Name, + Namespace: mod.Namespace, + ImageRepoSecret: mod.Spec.ImageRepoSecret, + Owner: &mod, + Selector: mod.Spec.Selector, + ServiceAccountName: mod.Spec.ModuleLoader.ServiceAccountName, + Modprobe: mod.Spec.ModuleLoader.Container.Modprobe, + KernelVersion: kernelVersion, + } + + if buildExistsInMapping { + mapping.Build = build + } + if buildExistsInModuleSpec { + mod.Spec.ModuleLoader.Container.Build = build + } + if signExistsInMapping { + mapping.Sign = sign + } + if SignExistsInModuleSpec { + mod.Spec.ModuleLoader.Container.Sign = sign + } + mld.RegistryTLS = &mod.Spec.ModuleLoader.Container.RegistryTLS + if registryTLSExistsInMapping { + mapping.RegistryTLS = registryTSL + mld.RegistryTLS = registryTSL + } + mld.ContainerImage = mod.Spec.ModuleLoader.Container.ContainerImage + if containerImageExistsInMapping { + mapping.ContainerImage = "mapping container image" + mld.ContainerImage = mapping.ContainerImage + } + + if buildExistsInMapping || buildExistsInModuleSpec { + mld.Build = build + sg.EXPECT().GetRelevantBuild(mod.Spec.ModuleLoader.Container.Build, mapping.Build).Return(build) + } + if signExistsInMapping || SignExistsInModuleSpec { + mld.Sign = sign + sg.EXPECT().GetRelevantSign(mod.Spec.ModuleLoader.Container.Sign, mapping.Sign, kernelVersion).Return(sign, nil) + } + + res, err := mldfh.prepareModuleLoaderData(&mapping, &mod, kernelVersion) + Expect(err).NotTo(HaveOccurred()) + Expect(*res).To(Equal(mld)) + }, + Entry("build in mapping only", true, false, false, false, false, false), + Entry("build in spec only", false, true, false, false, false, false), + Entry("sign in mapping only", false, false, true, false, false, false), + Entry("sign in spec only", false, false, false, true, false, false), + Entry("registryTLS in mapping", false, false, false, false, true, false), + Entry("containerImage in mapping", false, false, false, false, false, true), + ) +}) + +var _ = Describe("moduleLoaderDataCreatorHelper_replaceTemplates", func() { + const kernelVersion = "5.8.18-100.fc31.x86_64" + + mdlfh := moduleLoaderDataCreatorHelperImpl{} + + It("error input", func() { + mld := ModuleLoaderData{ + ContainerImage: "some image:${KERNEL_XYZ", + KernelVersion: kernelVersion, + } + err := mdlfh.replaceTemplates(&mld) + Expect(err).To(HaveOccurred()) + }) + + It("should only substitute the ContainerImage field", func() { + mld := ModuleLoaderData{ + ContainerImage: "some image:${KERNEL_XYZ}", + Build: &kmmv1beta1.Build{ + BuildArgs: []kmmv1beta1.BuildArg{ + {Name: "name1", Value: "value1"}, + {Name: "kernel version", Value: "${KERNEL_FULL_VERSION}"}, + }, + DockerfileConfigMap: &v1.LocalObjectReference{}, + }, + KernelVersion: kernelVersion, + } + expectMld := ModuleLoaderData{ + ContainerImage: "some image:5.8.18", + Build: &kmmv1beta1.Build{ + BuildArgs: []kmmv1beta1.BuildArg{ + {Name: "name1", Value: "value1"}, + {Name: "kernel version", Value: "${KERNEL_FULL_VERSION}"}, + }, + DockerfileConfigMap: &v1.LocalObjectReference{}, + }, + KernelVersion: kernelVersion, + } + + err := mdlfh.replaceTemplates(&mld) + Expect(err).NotTo(HaveOccurred()) + Expect(mld).To(Equal(expectMld)) + }) + +}) + +var _ = Describe("sectionGetterImpl_GetRelevantBuild", func() { + sg := sectionGetterImpl{} + + It("kernel mapping build present, module loader build absent", func() { + mappingBuild := &kmmv1beta1.Build{ + DockerfileConfigMap: &v1.LocalObjectReference{Name: "some kernel mapping build name"}, + } + + res := sg.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 := sg.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 := sg.GetRelevantBuild(moduleBuild, mappingBuild) + Expect(res.DockerfileConfigMap).To(Equal(mappingBuild.DockerfileConfigMap)) + Expect(res.BaseImageRegistryTLS).To(Equal(moduleBuild.BaseImageRegistryTLS)) + }) +}) + +var _ = Describe("sectionGetterImpl_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" + ) + + sg := sectionGetterImpl{} + + 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 := sg.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}, + }, + ), + ) + +}) diff --git a/internal/api/mock_creator.go b/internal/api/mock_creator.go new file mode 100644 index 000000000..dc278a163 --- /dev/null +++ b/internal/api/mock_creator.go @@ -0,0 +1,169 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: creator.go + +// Package api is a generated GoMock package. +package api + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + v1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" +) + +// MockModuleLoaderDataCreator is a mock of ModuleLoaderDataCreator interface. +type MockModuleLoaderDataCreator struct { + ctrl *gomock.Controller + recorder *MockModuleLoaderDataCreatorMockRecorder +} + +// MockModuleLoaderDataCreatorMockRecorder is the mock recorder for MockModuleLoaderDataCreator. +type MockModuleLoaderDataCreatorMockRecorder struct { + mock *MockModuleLoaderDataCreator +} + +// NewMockModuleLoaderDataCreator creates a new mock instance. +func NewMockModuleLoaderDataCreator(ctrl *gomock.Controller) *MockModuleLoaderDataCreator { + mock := &MockModuleLoaderDataCreator{ctrl: ctrl} + mock.recorder = &MockModuleLoaderDataCreatorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockModuleLoaderDataCreator) EXPECT() *MockModuleLoaderDataCreatorMockRecorder { + return m.recorder +} + +// FromModule mocks base method. +func (m *MockModuleLoaderDataCreator) FromModule(mod *v1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "FromModule", mod, kernelVersion) + ret0, _ := ret[0].(*ModuleLoaderData) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// FromModule indicates an expected call of FromModule. +func (mr *MockModuleLoaderDataCreatorMockRecorder) FromModule(mod, kernelVersion interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FromModule", reflect.TypeOf((*MockModuleLoaderDataCreator)(nil).FromModule), mod, kernelVersion) +} + +// MockmoduleLoaderDataCreatorHelper is a mock of moduleLoaderDataCreatorHelper interface. +type MockmoduleLoaderDataCreatorHelper struct { + ctrl *gomock.Controller + recorder *MockmoduleLoaderDataCreatorHelperMockRecorder +} + +// MockmoduleLoaderDataCreatorHelperMockRecorder is the mock recorder for MockmoduleLoaderDataCreatorHelper. +type MockmoduleLoaderDataCreatorHelperMockRecorder struct { + mock *MockmoduleLoaderDataCreatorHelper +} + +// NewMockmoduleLoaderDataCreatorHelper creates a new mock instance. +func NewMockmoduleLoaderDataCreatorHelper(ctrl *gomock.Controller) *MockmoduleLoaderDataCreatorHelper { + mock := &MockmoduleLoaderDataCreatorHelper{ctrl: ctrl} + mock.recorder = &MockmoduleLoaderDataCreatorHelperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockmoduleLoaderDataCreatorHelper) EXPECT() *MockmoduleLoaderDataCreatorHelperMockRecorder { + return m.recorder +} + +// findKernelMapping mocks base method. +func (m *MockmoduleLoaderDataCreatorHelper) findKernelMapping(mappings []v1beta1.KernelMapping, kernelVersion string) (*v1beta1.KernelMapping, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "findKernelMapping", mappings, kernelVersion) + ret0, _ := ret[0].(*v1beta1.KernelMapping) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// findKernelMapping indicates an expected call of findKernelMapping. +func (mr *MockmoduleLoaderDataCreatorHelperMockRecorder) findKernelMapping(mappings, kernelVersion interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "findKernelMapping", reflect.TypeOf((*MockmoduleLoaderDataCreatorHelper)(nil).findKernelMapping), mappings, kernelVersion) +} + +// prepareModuleLoaderData mocks base method. +func (m *MockmoduleLoaderDataCreatorHelper) prepareModuleLoaderData(mapping *v1beta1.KernelMapping, mod *v1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "prepareModuleLoaderData", mapping, mod, kernelVersion) + ret0, _ := ret[0].(*ModuleLoaderData) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// prepareModuleLoaderData indicates an expected call of prepareModuleLoaderData. +func (mr *MockmoduleLoaderDataCreatorHelperMockRecorder) prepareModuleLoaderData(mapping, mod, kernelVersion interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "prepareModuleLoaderData", reflect.TypeOf((*MockmoduleLoaderDataCreatorHelper)(nil).prepareModuleLoaderData), mapping, mod, kernelVersion) +} + +// replaceTemplates mocks base method. +func (m *MockmoduleLoaderDataCreatorHelper) replaceTemplates(mld *ModuleLoaderData) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "replaceTemplates", mld) + ret0, _ := ret[0].(error) + return ret0 +} + +// replaceTemplates indicates an expected call of replaceTemplates. +func (mr *MockmoduleLoaderDataCreatorHelperMockRecorder) replaceTemplates(mld interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "replaceTemplates", reflect.TypeOf((*MockmoduleLoaderDataCreatorHelper)(nil).replaceTemplates), mld) +} + +// MocksectionGetter is a mock of sectionGetter interface. +type MocksectionGetter struct { + ctrl *gomock.Controller + recorder *MocksectionGetterMockRecorder +} + +// MocksectionGetterMockRecorder is the mock recorder for MocksectionGetter. +type MocksectionGetterMockRecorder struct { + mock *MocksectionGetter +} + +// NewMocksectionGetter creates a new mock instance. +func NewMocksectionGetter(ctrl *gomock.Controller) *MocksectionGetter { + mock := &MocksectionGetter{ctrl: ctrl} + mock.recorder = &MocksectionGetterMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MocksectionGetter) EXPECT() *MocksectionGetterMockRecorder { + return m.recorder +} + +// GetRelevantBuild mocks base method. +func (m *MocksectionGetter) GetRelevantBuild(moduleBuild, mappingBuild *v1beta1.Build) *v1beta1.Build { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRelevantBuild", moduleBuild, mappingBuild) + ret0, _ := ret[0].(*v1beta1.Build) + return ret0 +} + +// GetRelevantBuild indicates an expected call of GetRelevantBuild. +func (mr *MocksectionGetterMockRecorder) GetRelevantBuild(moduleBuild, mappingBuild interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRelevantBuild", reflect.TypeOf((*MocksectionGetter)(nil).GetRelevantBuild), moduleBuild, mappingBuild) +} + +// GetRelevantSign mocks base method. +func (m *MocksectionGetter) GetRelevantSign(moduleSign, mappingSign *v1beta1.Sign, kernel string) (*v1beta1.Sign, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRelevantSign", moduleSign, mappingSign, kernel) + ret0, _ := ret[0].(*v1beta1.Sign) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRelevantSign indicates an expected call of GetRelevantSign. +func (mr *MocksectionGetterMockRecorder) GetRelevantSign(moduleSign, mappingSign, kernel interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRelevantSign", reflect.TypeOf((*MocksectionGetter)(nil).GetRelevantSign), moduleSign, mappingSign, kernel) +} diff --git a/internal/api/moduleloaderdata.go b/internal/api/moduleloaderdata.go index f6159cf93..423f11772 100644 --- a/internal/api/moduleloaderdata.go +++ b/internal/api/moduleloaderdata.go @@ -1,7 +1,11 @@ package api import ( + "errors" + "fmt" + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + "github.com/kubernetes-sigs/kernel-module-management/internal/utils/image" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -24,7 +28,7 @@ type ModuleLoaderData struct { // Name Name string - // Namspace + // Namespace Namespace string // service account for DS @@ -57,3 +61,43 @@ type ModuleLoaderData struct { // used for setting the owner field of jobs/buildconfigs Owner metav1.Object } + +func (mld *ModuleLoaderData) BuildConfigured() bool { + return mld.Build != nil +} + +func (mld *ModuleLoaderData) BuildDestinationImage() (string, error) { + if !mld.BuildConfigured() { + return "", errors.New("no build configured") + } + + if mld.Sign != nil { + return mld.intermediateImageName() + } + + return mld.ContainerImage, nil +} + +func (mld *ModuleLoaderData) SignConfigured() bool { + return mld.Sign != nil +} + +func (mld *ModuleLoaderData) UnsignedImage() (string, error) { + if !mld.SignConfigured() { + return "", errors.New("signing not configured") + } + + if build := mld.Build; build != nil { + return mld.intermediateImageName() + } + + return mld.Sign.UnsignedImage, nil +} + +func (mld *ModuleLoaderData) intermediateImageName() (string, error) { + return image.SetOrAppendTag( + mld.ContainerImage, + fmt.Sprintf("%s_%s_kmm_unsigned", mld.Namespace, mld.Name), + "_", + ) +} diff --git a/internal/api/moduleloaderdata_test.go b/internal/api/moduleloaderdata_test.go new file mode 100644 index 000000000..a4adde15b --- /dev/null +++ b/internal/api/moduleloaderdata_test.go @@ -0,0 +1,142 @@ +package api + +import ( + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ModuleLoaderData_BuildConfigured", func() { + DescribeTable("works as expected", + func(mld *ModuleLoaderData, expected bool) { + Expect(mld.BuildConfigured()).To(Equal(expected)) + }, + Entry(nil, &ModuleLoaderData{}, false), + Entry(nil, &ModuleLoaderData{Build: &kmmv1beta1.Build{}}, true), + ) +}) + +var _ = Describe("ModuleLoaderData_BuildDestinationImage", func() { + DescribeTable("works as expected", + func(mld *ModuleLoaderData, expected string, returnsError bool) { + expect := Expect(mld.BuildDestinationImage()) + + if returnsError { + expect.Error().To(HaveOccurred()) + return + } + + expect.To(Equal(expected)) + }, + Entry( + "no build configured", + &ModuleLoaderData{}, + "", + true, + ), + Entry( + "build configured, sign not configured", + &ModuleLoaderData{Build: &kmmv1beta1.Build{}, ContainerImage: "test"}, + "test", + false, + ), + Entry( + "build and sign configured", + &ModuleLoaderData{ + Build: &kmmv1beta1.Build{}, + Sign: &kmmv1beta1.Sign{}, + ContainerImage: "test", + Namespace: "ns", + Name: "name", + }, + "test:ns_name_kmm_unsigned", + false, + ), + ) +}) + +var _ = Describe("ModuleLoaderData_SignConfigured", func() { + DescribeTable("works as expected", + func(mld *ModuleLoaderData, expected bool) { + Expect(mld.SignConfigured()).To(Equal(expected)) + }, + Entry(nil, &ModuleLoaderData{}, false), + Entry(nil, &ModuleLoaderData{Sign: &kmmv1beta1.Sign{}}, true), + ) +}) + +var _ = Describe("ModuleLoaderData_UnsignedImage", func() { + When("an unsigned image is specified", func() { + const unsignedImage = "other-repo/other-image:other-tag" + + mld := ModuleLoaderData{ + Name: "test-module", + Namespace: "test-namespace", + ContainerImage: "registry.com/repo/image:tag", + Sign: &kmmv1beta1.Sign{UnsignedImage: unsignedImage}, + } + + It("should return it when no build section is present", func() { + Expect( + mld.UnsignedImage(), + ).To( + Equal(unsignedImage), + ) + }) + + It("should ignore it if a build section is present", func() { + mld.Build = &kmmv1beta1.Build{} + + Expect( + mld.UnsignedImage(), + ).To( + Equal("registry.com/repo/image:tag_test-namespace_test-module_kmm_unsigned"), + ) + }) + }) +}) + +var _ = Describe("ModuleLoaderData_intermediateImageName", func() { + DescribeTable("works as expected", + func(mld *ModuleLoaderData, expected string, returnsError bool) { + expect := Expect(mld.intermediateImageName()) + + if returnsError { + expect.Error().To(HaveOccurred()) + return + } + + expect.To(Equal(expected)) + }, + Entry( + "empty containerImage", + &ModuleLoaderData{}, + "", + true, + ), + Entry( + "bad containerImage", + &ModuleLoaderData{ContainerImage: "bad@name"}, + "", + true, + ), + Entry( + "containerImage without host and label", + &ModuleLoaderData{ContainerImage: "name", Namespace: "ns", Name: "name"}, + "name:ns_name_kmm_unsigned", + false, + ), + Entry( + "containerImage with a host but no label", + &ModuleLoaderData{ContainerImage: "example.org/name", Namespace: "ns", Name: "name"}, + "example.org/name:ns_name_kmm_unsigned", + false, + ), + Entry( + "containerImage with a host and a label", + &ModuleLoaderData{ContainerImage: "example.org/name:test", Namespace: "ns", Name: "name"}, + "example.org/name:test_ns_name_kmm_unsigned", + false, + ), + ) +}) diff --git a/internal/api/suite_test.go b/internal/api/suite_test.go new file mode 100644 index 000000000..11aff0ae2 --- /dev/null +++ b/internal/api/suite_test.go @@ -0,0 +1,23 @@ +package api + +import ( + "testing" + + "github.com/kubernetes-sigs/kernel-module-management/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" +) + +var scheme *runtime.Scheme + +func TestSuite(t *testing.T) { + RegisterFailHandler(Fail) + + var err error + + scheme, err = test.TestScheme() + Expect(err).NotTo(HaveOccurred()) + + RunSpecs(t, "API Suite") +} diff --git a/internal/build/helper.go b/internal/build/helper.go deleted file mode 100644 index 0c7ddeca8..000000000 --- a/internal/build/helper.go +++ /dev/null @@ -1,67 +0,0 @@ -package build - -import ( - "k8s.io/apimachinery/pkg/util/sets" - - kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" -) - -//go:generate mockgen -source=helper.go -package=build -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 -} - -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...) - - // [TODO] once MGMT-10832 is consolidated, this code must be revisited. We will decide which - // secret and how to use, and if we need to take care of repeated secrets names - buildConfig.Secrets = append(buildConfig.Secrets, mappingBuild.Secrets...) - return buildConfig -} diff --git a/internal/build/helper_test.go b/internal/build/helper_test.go deleted file mode 100644 index bd18eeffb..000000000 --- a/internal/build/helper_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package build - -import ( - 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)) - }) -}) diff --git a/internal/build/job/maker.go b/internal/build/job/maker.go index d3f88cef8..e1e2de1c2 100644 --- a/internal/build/job/maker.go +++ b/internal/build/job/maker.go @@ -6,6 +6,7 @@ import ( "os" "strings" + buildutils "github.com/kubernetes-sigs/kernel-module-management/internal/build/utils" "github.com/mitchellh/hashstructure/v2" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -19,9 +20,7 @@ import ( kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "github.com/kubernetes-sigs/kernel-module-management/internal/module" "github.com/kubernetes-sigs/kernel-module-management/internal/utils" ) @@ -41,7 +40,6 @@ type Maker interface { type maker struct { client client.Client - helper build.Helper jobHelper utils.JobHelper scheme *runtime.Scheme } @@ -53,12 +51,10 @@ type hashData struct { func NewMaker( client client.Client, - helper build.Helper, jobHelper utils.JobHelper, scheme *runtime.Scheme) Maker { return &maker{ client: client, - helper: helper, jobHelper: jobHelper, scheme: scheme, } @@ -70,11 +66,9 @@ func (m *maker) MakeJobTemplate( owner metav1.Object, pushImage bool) (*batchv1.Job, error) { - // if build AND sign are specified, then we will build an intermediate image - // and let sign produce the one specified in its targetImage - containerImage := mld.ContainerImage - if module.ShouldBeSigned(mld) { - containerImage = module.IntermediateImageName(mld.Name, mld.Namespace, containerImage) + containerImage, err := mld.BuildDestinationImage() + if err != nil { + return nil, fmt.Errorf("could not determine the destination image: %v", err) } specTemplate := m.specTemplate(mld, containerImage, pushImage) @@ -162,7 +156,7 @@ func (m *maker) containerArgs( {Name: "MOD_NAME", Value: mld.Name}, {Name: "MOD_NAMESPACE", Value: mld.Namespace}, } - buildArgs := m.helper.ApplyBuildArgOverrides( + buildArgs := buildutils.ApplyBuildArgOverrides( buildConfig.BuildArgs, overrides..., ) diff --git a/internal/build/job/maker_test.go b/internal/build/job/maker_test.go index c5d5059c3..4f26e405d 100644 --- a/internal/build/job/maker_test.go +++ b/internal/build/job/maker_test.go @@ -9,7 +9,6 @@ import ( "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" - "golang.org/x/exp/slices" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -19,7 +18,6 @@ import ( kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/client" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" "github.com/kubernetes-sigs/kernel-module-management/internal/utils" @@ -40,16 +38,14 @@ var _ = Describe("MakeJobTemplate", func() { ctrl *gomock.Controller clnt *client.MockClient m Maker - mh *build.MockHelper jobhelper *utils.MockJobHelper ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mh = build.NewMockHelper(ctrl) jobhelper = utils.NewMockJobHelper(ctrl) - m = NewMaker(clnt, mh, jobhelper, scheme) + m = NewMaker(clnt, jobhelper, scheme) }) AfterEach(func() { @@ -228,14 +224,7 @@ var _ = Describe("MakeJobTemplate", func() { annotations := map[string]string{constants.JobHashAnnotation: fmt.Sprintf("%d", hash)} expected.SetAnnotations(annotations) - overrides := []kmmv1beta1.BuildArg{ - {Name: "KERNEL_VERSION", Value: kernelVersion}, - {Name: "MOD_NAME", Value: moduleName}, - {Name: "MOD_NAMESPACE", Value: namespace}, - } - gomock.InOrder( - mh.EXPECT().ApplyBuildArgOverrides(buildArgs, overrides).Return(append(slices.Clone(buildArgs), overrides...)), clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error { cm.Data = dockerfileCMData @@ -300,7 +289,6 @@ var _ = Describe("MakeJobTemplate", func() { } gomock.InOrder( - mh.EXPECT().ApplyBuildArgOverrides(nil, kmmv1beta1.BuildArg{Name: "KERNEL_VERSION", Value: kernelVersion}), clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mod.Namespace}, gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error { cm.Data = dockerfileCMData @@ -377,9 +365,7 @@ var _ = Describe("MakeJobTemplate", func() { KernelVersion: kernelVersion, } - override := kmmv1beta1.BuildArg{Name: "KERNEL_VERSION", Value: kernelVersion} gomock.InOrder( - mh.EXPECT().ApplyBuildArgOverrides(buildArgs, override), clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error { cm.Data = dockerfileCMData @@ -398,7 +384,6 @@ var _ = Describe("MakeJobTemplate", func() { It("should add the kmm_unsigned suffix to the target image if sign is defined", func() { ctx := context.Background() - override := kmmv1beta1.BuildArg{Name: "KERNEL_VERSION", Value: kernelVersion} mld := api.ModuleLoaderData{ Name: mod.Name, Namespace: mod.Namespace, @@ -416,7 +401,6 @@ var _ = Describe("MakeJobTemplate", func() { expectedImageName := mld.ContainerImage + ":" + mld.Namespace + "_" + mld.Name + "_kmm_unsigned" gomock.InOrder( - mh.EXPECT().ApplyBuildArgOverrides(buildArgs, override), clnt.EXPECT().Get(ctx, types.NamespacedName{Name: dockerfileConfigMap.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( func(_ interface{}, _ interface{}, cm *v1.ConfigMap, _ ...ctrlclient.GetOption) error { cm.Data = dockerfileCMData diff --git a/internal/build/job/manager.go b/internal/build/job/manager.go index 467954e4e..9ed892f00 100644 --- a/internal/build/job/manager.go +++ b/internal/build/job/manager.go @@ -57,16 +57,13 @@ func (jbm *jobManager) GarbageCollect(ctx context.Context, modName, namespace st func (jbm *jobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { // if there is no build specified skip - if !module.ShouldBeBuilt(mld) { + if !mld.BuildConfigured() { return false, nil } - targetImage := mld.ContainerImage - - // if build AND sign are specified, then we will build an intermediate image - // and let sign produce the one specified in targetImage - if module.ShouldBeSigned(mld) { - targetImage = module.IntermediateImageName(mld.Name, mld.Namespace, targetImage) + targetImage, err := mld.BuildDestinationImage() + if err != nil { + return false, fmt.Errorf("could not determine the destination image: %v", err) } // build is specified and targetImage is either the final image or the intermediate image diff --git a/internal/build/mock_helper.go b/internal/build/mock_helper.go index ae844c7ac..78a252277 100644 --- a/internal/build/mock_helper.go +++ b/internal/build/mock_helper.go @@ -34,25 +34,6 @@ func (m *MockHelper) EXPECT() *MockHelperMockRecorder { return m.recorder } -// ApplyBuildArgOverrides mocks base method. -func (m *MockHelper) ApplyBuildArgOverrides(args []v1beta1.BuildArg, overrides ...v1beta1.BuildArg) []v1beta1.BuildArg { - m.ctrl.T.Helper() - varargs := []interface{}{args} - for _, a := range overrides { - varargs = append(varargs, a) - } - ret := m.ctrl.Call(m, "ApplyBuildArgOverrides", varargs...) - ret0, _ := ret[0].([]v1beta1.BuildArg) - return ret0 -} - -// ApplyBuildArgOverrides indicates an expected call of ApplyBuildArgOverrides. -func (mr *MockHelperMockRecorder) ApplyBuildArgOverrides(args interface{}, overrides ...interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - varargs := append([]interface{}{args}, overrides...) - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ApplyBuildArgOverrides", reflect.TypeOf((*MockHelper)(nil).ApplyBuildArgOverrides), varargs...) -} - // GetRelevantBuild mocks base method. func (m *MockHelper) GetRelevantBuild(moduleBuild, mappingBuild *v1beta1.Build) *v1beta1.Build { m.ctrl.T.Helper() diff --git a/internal/build/utils/helper.go b/internal/build/utils/helper.go new file mode 100644 index 000000000..e1617d401 --- /dev/null +++ b/internal/build/utils/helper.go @@ -0,0 +1,32 @@ +package utils + +import ( + "k8s.io/apimachinery/pkg/util/sets" + + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" +) + +func 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 +} diff --git a/internal/build/utils/helper_test.go b/internal/build/utils/helper_test.go new file mode 100644 index 000000000..ac720e0d4 --- /dev/null +++ b/internal/build/utils/helper_test.go @@ -0,0 +1,50 @@ +package utils + +import ( + kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ApplyBuildArgOverrides", func() { + 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 := ApplyBuildArgOverrides(args, overrides...) + Expect(res).To(Equal(expected)) + }) +}) diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go index fcff488f6..8030bd23f 100644 --- a/internal/cluster/cluster.go +++ b/internal/cluster/cluster.go @@ -18,7 +18,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/api" "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "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" ) @@ -33,25 +32,25 @@ type ClusterAPI interface { } type clusterAPI struct { - client client.Client - kernelAPI module.KernelMapper - buildAPI build.Manager - signAPI sign.SignManager - namespace string + client client.Client + mldCreator api.ModuleLoaderDataCreator + buildAPI build.Manager + signAPI sign.SignManager + namespace string } func NewClusterAPI( client client.Client, - kernelAPI module.KernelMapper, + mldCreator api.ModuleLoaderDataCreator, buildAPI build.Manager, signAPI sign.SignManager, defaultJobNamespace string) ClusterAPI { return &clusterAPI{ - client: client, - kernelAPI: kernelAPI, - buildAPI: buildAPI, - signAPI: signAPI, - namespace: defaultJobNamespace, + client: client, + mldCreator: mldCreator, + buildAPI: buildAPI, + signAPI: signAPI, + namespace: defaultJobNamespace, } } @@ -169,7 +168,7 @@ func (c *clusterAPI) kernelMappingsByKernelVersion( continue } - mld, err := c.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) + mld, err := c.mldCreator.FromModule(mod, kernelVersion) if err != nil { kernelVersionLogger.Info("no suitable container image found; skipping kernel version") continue @@ -238,11 +237,9 @@ func (c *clusterAPI) sign( return true, nil } - // if we need to sign AND we've built, then we must have built - // the intermediate image so must figure out its name - previousImage := "" - if module.ShouldBeBuilt(mld) { - previousImage = module.IntermediateImageName(mld.Name, mld.Namespace, mld.ContainerImage) + previousImage, err := mld.UnsignedImage() + if err != nil { + return false, fmt.Errorf("could not determine the unsigned image: %v", err) } logger := log.FromContext(ctx).WithValues( diff --git a/internal/cluster/cluster_test.go b/internal/cluster/cluster_test.go index f4d775d3f..29ae84736 100644 --- a/internal/cluster/cluster_test.go +++ b/internal/cluster/cluster_test.go @@ -18,24 +18,23 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/client" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "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" ) var _ = Describe("ClusterAPI", func() { var ( - ctrl *gomock.Controller - clnt *client.MockClient - mockKM *module.MockKernelMapper - mockBM *build.MockManager - mockSM *sign.MockSignManager + ctrl *gomock.Controller + clnt *client.MockClient + mockMLDCreator *api.MockModuleLoaderDataCreator + mockBM *build.MockManager + mockSM *sign.MockSignManager ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mockKM = module.NewMockKernelMapper(ctrl) + mockMLDCreator = api.NewMockModuleLoaderDataCreator(ctrl) mockBM = build.NewMockManager(ctrl) mockSM = sign.NewMockSignManager(ctrl) }) @@ -76,7 +75,7 @@ var _ = Describe("ClusterAPI", func() { ), ) - c := NewClusterAPI(clnt, mockKM, nil, nil, "") + c := NewClusterAPI(clnt, mockMLDCreator, nil, nil, "") res, err := c.RequestedManagedClusterModule(ctx, nsn) @@ -96,7 +95,7 @@ var _ = Describe("ClusterAPI", func() { clnt.EXPECT().Get(ctx, nsn, gomock.Any()).Return(errors.New("generic-error")), ) - c := NewClusterAPI(clnt, mockKM, nil, nil, "") + c := NewClusterAPI(clnt, mockMLDCreator, nil, nil, "") res, err := c.RequestedManagedClusterModule(ctx, nsn) @@ -141,7 +140,7 @@ var _ = Describe("ClusterAPI", func() { ), ) - c := NewClusterAPI(clnt, mockKM, nil, nil, "") + c := NewClusterAPI(clnt, mockMLDCreator, nil, nil, "") res, err := c.SelectedManagedClusters(ctx, mcm) @@ -156,7 +155,7 @@ var _ = Describe("ClusterAPI", func() { clnt.EXPECT().List(ctx, gomock.Any(), gomock.Any()).Return(errors.New("generic-error")), ) - c := NewClusterAPI(clnt, mockKM, nil, nil, "") + c := NewClusterAPI(clnt, mockMLDCreator, nil, nil, "") res, err := c.SelectedManagedClusters(ctx, &hubv1beta1.ManagedClusterModule{}) @@ -174,10 +173,7 @@ var _ = Describe("ClusterAPI", func() { ) var ( - mld = api.ModuleLoaderData{ - ContainerImage: imageName, - KernelVersion: kernelVersion, - } + mld api.ModuleLoaderData mcm = &hubv1beta1.ManagedClusterModule{ ObjectMeta: metav1.ObjectMeta{ @@ -220,12 +216,19 @@ var _ = Describe("ClusterAPI", func() { ctx = context.Background() ) + BeforeEach(func() { + mld = api.ModuleLoaderData{ + ContainerImage: imageName, + KernelVersion: kernelVersion, + } + }) + It("should do nothing when no kernel mappings are found", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, errors.New("generic-error")), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(nil, errors.New("generic-error")), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).ToNot(HaveOccurred()) @@ -244,7 +247,7 @@ var _ = Describe("ClusterAPI", func() { }, } - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).To(HaveOccurred()) @@ -253,12 +256,12 @@ var _ = Describe("ClusterAPI", func() { It("should do nothing when Build and Sign are not needed", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(false, nil), mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(false, nil), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).ToNot(HaveOccurred()) @@ -267,13 +270,13 @@ var _ = Describe("ClusterAPI", func() { It("should run build sync if needed", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(utils.Status(utils.StatusCompleted), nil), mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(false, nil), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).ToNot(HaveOccurred()) @@ -282,12 +285,12 @@ var _ = Describe("ClusterAPI", func() { It("should return an error when build sync errors", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(utils.Status(""), errors.New("test-error")), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).To(HaveOccurred()) @@ -295,14 +298,16 @@ var _ = Describe("ClusterAPI", func() { }) It("should run sign sync if needed", func() { + mld.Sign = &kmmv1beta1.Sign{} + gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(false, nil), mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockSM.EXPECT().Sync(gomock.Any(), &mld, "", true, mcm).Return(utils.Status(utils.StatusInProgress), nil), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).ToNot(HaveOccurred()) @@ -310,14 +315,16 @@ var _ = Describe("ClusterAPI", func() { }) It("should return an error when sign sync errors", func() { + mld.Sign = &kmmv1beta1.Sign{} + gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(false, nil), mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockSM.EXPECT().Sync(gomock.Any(), &mld, "", true, mcm).Return(utils.Status(""), errors.New("test-error")), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).To(HaveOccurred()) @@ -326,12 +333,12 @@ var _ = Describe("ClusterAPI", func() { It("should not run sign sync when build sync does not complete", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(utils.Status(utils.StatusInProgress), nil), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).ToNot(HaveOccurred()) @@ -339,15 +346,17 @@ var _ = Describe("ClusterAPI", func() { }) It("should run both build sync and sign sync when build is completed", func() { + mld.Sign = &kmmv1beta1.Sign{} + gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockMLDCreator.EXPECT().FromModule(&mod, kernelVersion).Return(&mld, nil), mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(utils.Status(utils.StatusCompleted), nil), mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), mockSM.EXPECT().Sync(gomock.Any(), &mld, "", true, mcm).Return(utils.Status(utils.StatusCompleted), nil), ) - c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) + c := NewClusterAPI(clnt, mockMLDCreator, mockBM, mockSM, namespace) completed, err := c.BuildAndSign(ctx, *mcm, clusterList.Items[0]) Expect(err).ToNot(HaveOccurred()) diff --git a/internal/module/helper.go b/internal/module/helper.go index 2f47a9520..f3d8bc58f 100644 --- a/internal/module/helper.go +++ b/internal/module/helper.go @@ -3,7 +3,6 @@ package module import ( "context" "fmt" - "strings" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -13,33 +12,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/registry" ) -// AppendToTag adds the specified tag to the image name cleanly, i.e. by avoiding messing up -// the name or getting "name:-tag" -func AppendToTag(name string, tag string) string { - separator := ":" - if strings.Contains(name, ":") { - separator = "_" - } - return name + separator + tag -} - -// IntermediateImageName returns the image name of the pre-signed module image name -func IntermediateImageName(name, namespace, targetImage string) string { - return AppendToTag(targetImage, namespace+"_"+name+"_kmm_unsigned") -} - -// ShouldBeBuilt indicates whether the specified ModuleLoaderData of the -// Module should be built or not. -func ShouldBeBuilt(mld *api.ModuleLoaderData) bool { - return mld.Build != nil -} - -// ShouldBeSigned indicates whether the specified ModuleLoaderData of the -// Module should be signed or not. -func ShouldBeSigned(mld *api.ModuleLoaderData) bool { - return mld.Sign != nil -} - func ImageExists( ctx context.Context, client client.Client, diff --git a/internal/module/helper_test.go b/internal/module/helper_test.go index d0be2312c..0c8eca08b 100644 --- a/internal/module/helper_test.go +++ b/internal/module/helper_test.go @@ -15,40 +15,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/registry" ) -var _ = Describe("AppendToTag", func() { - It("should append a tag to the image name", func() { - name := "some-container-image-name" - tag := "a-kmm-tag" - - Expect( - AppendToTag(name, tag), - ).To( - Equal(name + ":" + tag), - ) - }) - - It("should add a suffix to the already present tag", func() { - name := "some-container-image-name:with-a-tag" - tag := "a-kmm-tag-suffix" - - Expect( - AppendToTag(name, tag), - ).To( - Equal(name + "_" + tag), - ) - }) -}) - -var _ = Describe("IntermediateImageName", func() { - It("should add the kmm_unsigned suffix to the target image name", func() { - Expect( - IntermediateImageName("module-name", "test-namespace", "some-image-name"), - ).To( - Equal("some-image-name:test-namespace_module-name_kmm_unsigned"), - ) - }) -}) - var _ = Describe("ImageExists", func() { const ( imageName = "image-name" diff --git a/internal/module/kernelmapper.go b/internal/module/kernelmapper.go deleted file mode 100644 index af041c4ac..000000000 --- a/internal/module/kernelmapper.go +++ /dev/null @@ -1,146 +0,0 @@ -package module - -import ( - "errors" - "fmt" - "regexp" - - kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/build" - "github.com/kubernetes-sigs/kernel-module-management/internal/sign" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" -) - -//go:generate mockgen -source=kernelmapper.go -package=module -destination=mock_kernelmapper.go KernelMapper,kernelMapperHelperAPI - -type KernelMapper interface { - GetModuleLoaderDataForKernel(mod *kmmv1beta1.Module, kernelVersion string) (*api.ModuleLoaderData, error) -} - -type kernelMapper struct { - helper kernelMapperHelperAPI -} - -func NewKernelMapper(buildHelper build.Helper, signHelper sign.Helper) KernelMapper { - return &kernelMapper{ - helper: newKernelMapperHelper(buildHelper, signHelper), - } -} - -func (k *kernelMapper) GetModuleLoaderDataForKernel(mod *kmmv1beta1.Module, kernelVersion string) (*api.ModuleLoaderData, error) { - mappings := mod.Spec.ModuleLoader.Container.KernelMappings - foundMapping, err := k.helper.findKernelMapping(mappings, kernelVersion) - if err != nil { - return nil, fmt.Errorf("failed to find mapping for kernel %s: %v", kernelVersion, err) - } - mld, err := k.helper.prepareModuleLoaderData(foundMapping, mod, kernelVersion) - if err != nil { - return nil, fmt.Errorf("failed to prepare module loader data for kernel %s: %v", kernelVersion, err) - } - - err = k.helper.replaceTemplates(mld) - if err != nil { - return nil, fmt.Errorf("failed to replace templates in module loader data for kernel %s: %v", kernelVersion, err) - } - return mld, nil -} - -type kernelMapperHelperAPI interface { - findKernelMapping(mappings []kmmv1beta1.KernelMapping, kernelVersion string) (*kmmv1beta1.KernelMapping, error) - prepareModuleLoaderData(mapping *kmmv1beta1.KernelMapping, mod *kmmv1beta1.Module, kernelVersion string) (*api.ModuleLoaderData, error) - replaceTemplates(mld *api.ModuleLoaderData) error -} - -type kernelMapperHelper struct { - buildHelper build.Helper - signHelper sign.Helper -} - -func newKernelMapperHelper(buildHelper build.Helper, signHelper sign.Helper) kernelMapperHelperAPI { - return &kernelMapperHelper{ - buildHelper: buildHelper, - signHelper: signHelper, - } -} - -func (kh *kernelMapperHelper) findKernelMapping(mappings []kmmv1beta1.KernelMapping, kernelVersion string) (*kmmv1beta1.KernelMapping, error) { - for _, m := range mappings { - if m.Literal != "" && m.Literal == kernelVersion { - return &m, nil - } - - if m.Regexp == "" { - continue - } - - if matches, err := regexp.MatchString(m.Regexp, kernelVersion); err != nil { - return nil, fmt.Errorf("could not match regexp %q against kernel %q: %v", m.Regexp, kernelVersion, err) - } else if matches { - return &m, nil - } - } - - return nil, errors.New("no suitable mapping found") -} - -func (kh *kernelMapperHelper) prepareModuleLoaderData(mapping *kmmv1beta1.KernelMapping, mod *kmmv1beta1.Module, kernelVersion string) (*api.ModuleLoaderData, error) { - var err error - - mld := &api.ModuleLoaderData{} - // prepare the build - if mapping.Build != nil || mod.Spec.ModuleLoader.Container.Build != nil { - mld.Build = kh.buildHelper.GetRelevantBuild(mod.Spec.ModuleLoader.Container.Build, mapping.Build) - } - - // prepare the sign - if mapping.Sign != nil || mod.Spec.ModuleLoader.Container.Sign != nil { - mld.Sign, err = kh.signHelper.GetRelevantSign(mod.Spec.ModuleLoader.Container.Sign, mapping.Sign, kernelVersion) - if err != nil { - return nil, fmt.Errorf("failed to get the relevant Sign configuration for kernel %s: %v", kernelVersion, err) - } - } - - // prepare TLS options - mld.RegistryTLS = mapping.RegistryTLS - if mapping.RegistryTLS == nil { - mld.RegistryTLS = &mod.Spec.ModuleLoader.Container.RegistryTLS - } - - //prepare container image - mld.ContainerImage = mapping.ContainerImage - if mapping.ContainerImage == "" { - mld.ContainerImage = mod.Spec.ModuleLoader.Container.ContainerImage - } - - mld.InTreeRemoval = mod.Spec.ModuleLoader.Container.InTreeRemoval - if mapping.InTreeRemoval != nil { - mld.InTreeRemoval = *mapping.InTreeRemoval - } - - mld.KernelVersion = kernelVersion - mld.Name = mod.Name - mld.Namespace = mod.Namespace - mld.ImageRepoSecret = mod.Spec.ImageRepoSecret - mld.Selector = mod.Spec.Selector - mld.ServiceAccountName = mod.Spec.ModuleLoader.ServiceAccountName - mld.Modprobe = mod.Spec.ModuleLoader.Container.Modprobe - mld.ModuleVersion = mod.Spec.ModuleLoader.Container.Version - mld.ImagePullPolicy = mod.Spec.ModuleLoader.Container.ImagePullPolicy - mld.Owner = mod - - return mld, nil -} - -func (kh *kernelMapperHelper) replaceTemplates(mld *api.ModuleLoaderData) error { - osConfigEnvVars := utils.KernelComponentsAsEnvVars(mld.KernelVersion) - osConfigEnvVars = append(osConfigEnvVars, "MOD_NAME="+mld.Name, "MOD_NAMESPACE="+mld.Namespace) - - replacedContainerImage, err := utils.ReplaceInTemplates(osConfigEnvVars, mld.ContainerImage) - if err != nil { - return fmt.Errorf("failed to substitute templates in the ContainerImage field: %v", err) - } - mld.ContainerImage = replacedContainerImage[0] - - return nil -} diff --git a/internal/module/kernelmapper_test.go b/internal/module/kernelmapper_test.go deleted file mode 100644 index db6653b81..000000000 --- a/internal/module/kernelmapper_test.go +++ /dev/null @@ -1,293 +0,0 @@ -package module - -import ( - "fmt" - "github.com/golang/mock/gomock" - kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/build" - "github.com/kubernetes-sigs/kernel-module-management/internal/sign" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - v1 "k8s.io/api/core/v1" -) - -var _ = Describe("GetMergedMappingForKernel", func() { - const ( - kernelVersion = "1.2.3" - selectedImage = "image1" - ) - - var ( - ctrl *gomock.Controller - kh *MockkernelMapperHelperAPI - km *kernelMapper - mod kmmv1beta1.Module - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - kh = NewMockkernelMapperHelperAPI(ctrl) - km = &kernelMapper{helper: kh} - mod = kmmv1beta1.Module{} - }) - - AfterEach(func() { - ctrl.Finish() - }) - - It("good flow", func() { - mapping := kmmv1beta1.KernelMapping{} - mld := api.ModuleLoaderData{KernelVersion: kernelVersion} - kh.EXPECT().findKernelMapping(mod.Spec.ModuleLoader.Container.KernelMappings, kernelVersion).Return(&mapping, nil) - kh.EXPECT().prepareModuleLoaderData(&mapping, &mod, kernelVersion).Return(&mld, nil) - kh.EXPECT().replaceTemplates(&mld).Return(nil) - res, err := km.GetModuleLoaderDataForKernel(&mod, kernelVersion) - Expect(err).NotTo(HaveOccurred()) - Expect(res).To(Equal(&mld)) - }) - - It("failed to find kernel mapping", func() { - kh.EXPECT().findKernelMapping(mod.Spec.ModuleLoader.Container.KernelMappings, kernelVersion).Return(nil, fmt.Errorf("some error")) - res, err := km.GetModuleLoaderDataForKernel(&mod, kernelVersion) - Expect(err).To(HaveOccurred()) - Expect(res).To(BeNil()) - }) - - It("failed to merge mapping data", func() { - mapping := kmmv1beta1.KernelMapping{} - kh.EXPECT().findKernelMapping(mod.Spec.ModuleLoader.Container.KernelMappings, kernelVersion).Return(&mapping, nil) - kh.EXPECT().prepareModuleLoaderData(&mapping, &mod, kernelVersion).Return(nil, fmt.Errorf("some error")) - res, err := km.GetModuleLoaderDataForKernel(&mod, kernelVersion) - Expect(err).To(HaveOccurred()) - Expect(res).To(BeNil()) - }) - - It("failed to replace templates", func() { - mapping := kmmv1beta1.KernelMapping{} - mld := api.ModuleLoaderData{KernelVersion: kernelVersion} - kh.EXPECT().findKernelMapping(mod.Spec.ModuleLoader.Container.KernelMappings, kernelVersion).Return(&mapping, nil) - kh.EXPECT().prepareModuleLoaderData(&mapping, &mod, kernelVersion).Return(&mld, nil) - kh.EXPECT().replaceTemplates(&mld).Return(fmt.Errorf("some error")) - res, err := km.GetModuleLoaderDataForKernel(&mod, kernelVersion) - Expect(err).To(HaveOccurred()) - Expect(res).To(BeNil()) - }) -}) - -var _ = Describe("findKernelMapping", func() { - const ( - kernelVersion = "1.2.3" - ) - - var ( - ctrl *gomock.Controller - kh kernelMapperHelperAPI - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - kh = newKernelMapperHelper(nil, nil) - }) - - AfterEach(func() { - ctrl.Finish() - }) - - It("one literal mapping", func() { - mapping := kmmv1beta1.KernelMapping{ - Literal: "1.2.3", - } - - m, err := kh.findKernelMapping([]kmmv1beta1.KernelMapping{mapping}, kernelVersion) - Expect(err).NotTo(HaveOccurred()) - Expect(m).To(Equal(&mapping)) - }) - - It("one regexp mapping", func() { - mapping := kmmv1beta1.KernelMapping{ - Regexp: `1\..*`, - } - - m, err := kh.findKernelMapping([]kmmv1beta1.KernelMapping{mapping}, kernelVersion) - Expect(err).NotTo(HaveOccurred()) - Expect(m).To(Equal(&mapping)) - }) - - It("should return an error if a regex is invalid", func() { - mapping := kmmv1beta1.KernelMapping{ - Regexp: "invalid)", - } - - m, err := kh.findKernelMapping([]kmmv1beta1.KernelMapping{mapping}, kernelVersion) - Expect(err).To(HaveOccurred()) - Expect(m).To(BeNil()) - }) - - It("should return an error if no mapping work", func() { - mappings := []kmmv1beta1.KernelMapping{ - { - Regexp: `1.2.2`, - }, - { - Regexp: `0\..*`, - }, - } - - m, err := kh.findKernelMapping(mappings, kernelVersion) - Expect(err).To(MatchError("no suitable mapping found")) - Expect(m).To(BeNil()) - }) -}) - -var _ = Describe("prepareModuleLoaderData", func() { - const ( - kernelVersion = "1.2.3" - selectedImage = "image1" - ) - - var ( - ctrl *gomock.Controller - buildHelper *build.MockHelper - signHelper *sign.MockHelper - kh kernelMapperHelperAPI - mod kmmv1beta1.Module - mapping kmmv1beta1.KernelMapping - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - buildHelper = build.NewMockHelper(ctrl) - signHelper = sign.NewMockHelper(ctrl) - kh = newKernelMapperHelper(buildHelper, signHelper) - mod = kmmv1beta1.Module{} - mod.Spec.ModuleLoader.Container.ContainerImage = "spec container image" - mod.Spec.ModuleLoader.Container.ImagePullPolicy = "Always" - mapping = kmmv1beta1.KernelMapping{} - }) - - AfterEach(func() { - ctrl.Finish() - }) - - DescribeTable("prepare mapping", func(buildExistsInMapping, buildExistsInModuleSpec, signExistsInMapping, SignExistsInModuleSpec, - registryTLSExistsInMapping, containerImageExistsInMapping, inTreeRemovalExistsInMapping bool) { - build := &kmmv1beta1.Build{ - DockerfileConfigMap: &v1.LocalObjectReference{ - Name: "some name", - }, - } - sign := &kmmv1beta1.Sign{ - UnsignedImage: "some unsigned image", - } - registryTSL := &kmmv1beta1.TLSOptions{ - Insecure: true, - } - - mld := api.ModuleLoaderData{ - Name: mod.Name, - Namespace: mod.Namespace, - ImageRepoSecret: mod.Spec.ImageRepoSecret, - Owner: &mod, - Selector: mod.Spec.Selector, - ServiceAccountName: mod.Spec.ModuleLoader.ServiceAccountName, - Modprobe: mod.Spec.ModuleLoader.Container.Modprobe, - ImagePullPolicy: mod.Spec.ModuleLoader.Container.ImagePullPolicy, - KernelVersion: kernelVersion, - } - - if buildExistsInMapping { - mapping.Build = build - } - if buildExistsInModuleSpec { - mod.Spec.ModuleLoader.Container.Build = build - } - if signExistsInMapping { - mapping.Sign = sign - } - if SignExistsInModuleSpec { - mod.Spec.ModuleLoader.Container.Sign = sign - } - mld.RegistryTLS = &mod.Spec.ModuleLoader.Container.RegistryTLS - if registryTLSExistsInMapping { - mapping.RegistryTLS = registryTSL - mld.RegistryTLS = registryTSL - } - mld.ContainerImage = mod.Spec.ModuleLoader.Container.ContainerImage - if containerImageExistsInMapping { - mapping.ContainerImage = "mapping container image" - mld.ContainerImage = mapping.ContainerImage - } - - if buildExistsInMapping || buildExistsInModuleSpec { - mld.Build = build - buildHelper.EXPECT().GetRelevantBuild(mod.Spec.ModuleLoader.Container.Build, mapping.Build).Return(build) - } - if signExistsInMapping || SignExistsInModuleSpec { - mld.Sign = sign - signHelper.EXPECT().GetRelevantSign(mod.Spec.ModuleLoader.Container.Sign, mapping.Sign, kernelVersion).Return(sign, nil) - } - if inTreeRemovalExistsInMapping { - mld.InTreeRemoval = true - tmp := true - mapping.InTreeRemoval = &tmp - } - - res, err := kh.prepareModuleLoaderData(&mapping, &mod, kernelVersion) - Expect(err).NotTo(HaveOccurred()) - Expect(*res).To(Equal(mld)) - }, - Entry("build in mapping only", true, false, false, false, false, false, false), - Entry("build in spec only", false, true, false, false, false, false, false), - Entry("sign in mapping only", false, false, true, false, false, false, false), - Entry("sign in spec only", false, false, false, true, false, false, false), - Entry("registryTLS in mapping", false, false, false, false, true, false, false), - Entry("containerImage in mapping", false, false, false, false, false, true, false), - Entry("inTreeRemoval in mapping", false, false, false, false, false, false, true), - ) -}) - -var _ = Describe("replaceTemplates", func() { - const kernelVersion = "5.8.18-100.fc31.x86_64" - - kh := newKernelMapperHelper(nil, nil) - - It("error input", func() { - mld := api.ModuleLoaderData{ - ContainerImage: "some image:${KERNEL_XYZ", - KernelVersion: kernelVersion, - } - err := kh.replaceTemplates(&mld) - Expect(err).To(HaveOccurred()) - }) - - It("should only substitute the ContainerImage field", func() { - mld := api.ModuleLoaderData{ - ContainerImage: "some image:${KERNEL_XYZ}", - Build: &kmmv1beta1.Build{ - BuildArgs: []kmmv1beta1.BuildArg{ - {Name: "name1", Value: "value1"}, - {Name: "kernel version", Value: "${KERNEL_FULL_VERSION}"}, - }, - DockerfileConfigMap: &v1.LocalObjectReference{}, - }, - KernelVersion: kernelVersion, - } - expectMld := api.ModuleLoaderData{ - ContainerImage: "some image:5.8.18", - Build: &kmmv1beta1.Build{ - BuildArgs: []kmmv1beta1.BuildArg{ - {Name: "name1", Value: "value1"}, - {Name: "kernel version", Value: "${KERNEL_FULL_VERSION}"}, - }, - DockerfileConfigMap: &v1.LocalObjectReference{}, - }, - KernelVersion: kernelVersion, - } - - err := kh.replaceTemplates(&mld) - Expect(err).NotTo(HaveOccurred()) - Expect(mld).To(Equal(expectMld)) - }) - -}) diff --git a/internal/module/mock_kernelmapper.go b/internal/module/mock_kernelmapper.go deleted file mode 100644 index 3ebdc0e69..000000000 --- a/internal/module/mock_kernelmapper.go +++ /dev/null @@ -1,118 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: kernelmapper.go - -// Package module is a generated GoMock package. -package module - -import ( - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - v1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" - api "github.com/kubernetes-sigs/kernel-module-management/internal/api" -) - -// MockKernelMapper is a mock of KernelMapper interface. -type MockKernelMapper struct { - ctrl *gomock.Controller - recorder *MockKernelMapperMockRecorder -} - -// MockKernelMapperMockRecorder is the mock recorder for MockKernelMapper. -type MockKernelMapperMockRecorder struct { - mock *MockKernelMapper -} - -// NewMockKernelMapper creates a new mock instance. -func NewMockKernelMapper(ctrl *gomock.Controller) *MockKernelMapper { - mock := &MockKernelMapper{ctrl: ctrl} - mock.recorder = &MockKernelMapperMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockKernelMapper) EXPECT() *MockKernelMapperMockRecorder { - return m.recorder -} - -// GetModuleLoaderDataForKernel mocks base method. -func (m *MockKernelMapper) GetModuleLoaderDataForKernel(mod *v1beta1.Module, kernelVersion string) (*api.ModuleLoaderData, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetModuleLoaderDataForKernel", mod, kernelVersion) - ret0, _ := ret[0].(*api.ModuleLoaderData) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetModuleLoaderDataForKernel indicates an expected call of GetModuleLoaderDataForKernel. -func (mr *MockKernelMapperMockRecorder) GetModuleLoaderDataForKernel(mod, kernelVersion interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleLoaderDataForKernel", reflect.TypeOf((*MockKernelMapper)(nil).GetModuleLoaderDataForKernel), mod, kernelVersion) -} - -// MockkernelMapperHelperAPI is a mock of kernelMapperHelperAPI interface. -type MockkernelMapperHelperAPI struct { - ctrl *gomock.Controller - recorder *MockkernelMapperHelperAPIMockRecorder -} - -// MockkernelMapperHelperAPIMockRecorder is the mock recorder for MockkernelMapperHelperAPI. -type MockkernelMapperHelperAPIMockRecorder struct { - mock *MockkernelMapperHelperAPI -} - -// NewMockkernelMapperHelperAPI creates a new mock instance. -func NewMockkernelMapperHelperAPI(ctrl *gomock.Controller) *MockkernelMapperHelperAPI { - mock := &MockkernelMapperHelperAPI{ctrl: ctrl} - mock.recorder = &MockkernelMapperHelperAPIMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockkernelMapperHelperAPI) EXPECT() *MockkernelMapperHelperAPIMockRecorder { - return m.recorder -} - -// findKernelMapping mocks base method. -func (m *MockkernelMapperHelperAPI) findKernelMapping(mappings []v1beta1.KernelMapping, kernelVersion string) (*v1beta1.KernelMapping, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "findKernelMapping", mappings, kernelVersion) - ret0, _ := ret[0].(*v1beta1.KernelMapping) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// findKernelMapping indicates an expected call of findKernelMapping. -func (mr *MockkernelMapperHelperAPIMockRecorder) findKernelMapping(mappings, kernelVersion interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "findKernelMapping", reflect.TypeOf((*MockkernelMapperHelperAPI)(nil).findKernelMapping), mappings, kernelVersion) -} - -// prepareModuleLoaderData mocks base method. -func (m *MockkernelMapperHelperAPI) prepareModuleLoaderData(mapping *v1beta1.KernelMapping, mod *v1beta1.Module, kernelVersion string) (*api.ModuleLoaderData, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "prepareModuleLoaderData", mapping, mod, kernelVersion) - ret0, _ := ret[0].(*api.ModuleLoaderData) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// prepareModuleLoaderData indicates an expected call of prepareModuleLoaderData. -func (mr *MockkernelMapperHelperAPIMockRecorder) prepareModuleLoaderData(mapping, mod, kernelVersion interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "prepareModuleLoaderData", reflect.TypeOf((*MockkernelMapperHelperAPI)(nil).prepareModuleLoaderData), mapping, mod, kernelVersion) -} - -// replaceTemplates mocks base method. -func (m *MockkernelMapperHelperAPI) replaceTemplates(mld *api.ModuleLoaderData) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "replaceTemplates", mld) - ret0, _ := ret[0].(error) - return ret0 -} - -// replaceTemplates indicates an expected call of replaceTemplates. -func (mr *MockkernelMapperHelperAPIMockRecorder) replaceTemplates(mld interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "replaceTemplates", reflect.TypeOf((*MockkernelMapperHelperAPI)(nil).replaceTemplates), mld) -} diff --git a/internal/module/suite_test.go b/internal/module/suite_test.go index 60f87e2e6..e78f13a80 100644 --- a/internal/module/suite_test.go +++ b/internal/module/suite_test.go @@ -3,21 +3,11 @@ package module import ( "testing" - "github.com/kubernetes-sigs/kernel-module-management/internal/test" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "k8s.io/apimachinery/pkg/runtime" ) -var scheme *runtime.Scheme - func TestSuite(t *testing.T) { RegisterFailHandler(Fail) - - var err error - - scheme, err = test.TestScheme() - Expect(err).NotTo(HaveOccurred()) - RunSpecs(t, "Module Suite") } diff --git a/internal/preflight/preflight.go b/internal/preflight/preflight.go index e2d99eff1..35b363a37 100644 --- a/internal/preflight/preflight.go +++ b/internal/preflight/preflight.go @@ -8,7 +8,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/api" "github.com/kubernetes-sigs/kernel-module-management/internal/auth" "github.com/kubernetes-sigs/kernel-module-management/internal/build" - "github.com/kubernetes-sigs/kernel-module-management/internal/module" "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/sign" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" @@ -37,17 +36,17 @@ func NewPreflightAPI( signAPI sign.SignManager, registryAPI registry.Registry, statusUpdater statusupdater.PreflightStatusUpdater, - kernelAPI module.KernelMapper) PreflightAPI { + mldCreator api.ModuleLoaderDataCreator) PreflightAPI { helper := newPreflightHelper(client, buildAPI, signAPI, registryAPI) return &preflight{ - kernelAPI: kernelAPI, + mldCreator: mldCreator, statusUpdater: statusUpdater, helper: helper, } } type preflight struct { - kernelAPI module.KernelMapper + mldCreator api.ModuleLoaderDataCreator statusUpdater statusupdater.PreflightStatusUpdater helper preflightHelperAPI } @@ -55,7 +54,7 @@ type preflight struct { func (p *preflight) PreflightUpgradeCheck(ctx context.Context, pv *kmmv1beta1.PreflightValidation, mod *kmmv1beta1.Module) (bool, string) { log := ctrlruntime.LoggerFrom(ctx) kernelVersion := pv.Spec.KernelVersion - mld, err := p.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) + mld, err := p.mldCreator.FromModule(mod, kernelVersion) if err != nil { return false, fmt.Sprintf("failed to process kernel mapping in the module %s for kernel version %s", mod.Name, kernelVersion) } @@ -70,10 +69,7 @@ func (p *preflight) PreflightUpgradeCheck(ctx context.Context, pv *kmmv1beta1.Pr return true, msg } - shouldBuild := module.ShouldBeBuilt(mld) - shouldSign := module.ShouldBeSigned(mld) - - if shouldBuild { + if mld.BuildConfigured() { err = p.statusUpdater.PreflightSetVerificationStage(ctx, pv, mld.Name, kmmv1beta1.VerificationStageBuild) if err != nil { log.Info(utils.WarnString("failed to update the stage of Module CR in preflight to build stage"), "module", mld.Name, "error", err) @@ -85,7 +81,7 @@ func (p *preflight) PreflightUpgradeCheck(ctx context.Context, pv *kmmv1beta1.Pr } } - if shouldSign { + if mld.SignConfigured() { err = p.statusUpdater.PreflightSetVerificationStage(ctx, pv, mld.Name, kmmv1beta1.VerificationStageSign) if err != nil { log.Info(utils.WarnString("failed to update the stage of Module CR in preflight to sign stage"), "module", mld.Name, "error", err) @@ -174,9 +170,9 @@ func (p *preflightHelper) verifyBuild(ctx context.Context, pv *kmmv1beta1.Prefli func (p *preflightHelper) verifySign(ctx context.Context, pv *kmmv1beta1.PreflightValidation, mld *api.ModuleLoaderData) (bool, string) { log := ctrlruntime.LoggerFrom(ctx) - previousImage := "" - if module.ShouldBeBuilt(mld) { - previousImage = module.IntermediateImageName(mld.Name, mld.Namespace, mld.ContainerImage) + previousImage, err := mld.UnsignedImage() + if err != nil { + return false, fmt.Sprintf("could not determine the unsigned image: %v", err) } // at this stage we know that eiher mapping Sign or Container sign are defined diff --git a/internal/preflight/preflight_test.go b/internal/preflight/preflight_test.go index bc8c7b0cd..156e74899 100644 --- a/internal/preflight/preflight_test.go +++ b/internal/preflight/preflight_test.go @@ -11,7 +11,6 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/api" "github.com/kubernetes-sigs/kernel-module-management/internal/build" "github.com/kubernetes-sigs/kernel-module-management/internal/client" - "github.com/kubernetes-sigs/kernel-module-management/internal/module" "github.com/kubernetes-sigs/kernel-module-management/internal/registry" "github.com/kubernetes-sigs/kernel-module-management/internal/sign" "github.com/kubernetes-sigs/kernel-module-management/internal/statusupdater" @@ -75,7 +74,7 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { ) var ( ctrl *gomock.Controller - mockKernelAPI *module.MockKernelMapper + mockMLDCreator *api.MockModuleLoaderDataCreator mockStatusUpdater *statusupdater.MockPreflightStatusUpdater preflightHelper *MockpreflightHelperAPI p *preflight @@ -83,11 +82,11 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockKernelAPI = module.NewMockKernelMapper(ctrl) + mockMLDCreator = api.NewMockModuleLoaderDataCreator(ctrl) mockStatusUpdater = statusupdater.NewMockPreflightStatusUpdater(ctrl) preflightHelper = NewMockpreflightHelperAPI(ctrl) p = &preflight{ - kernelAPI: mockKernelAPI, + mldCreator: mockMLDCreator, helper: preflightHelper, statusUpdater: mockStatusUpdater, } @@ -100,7 +99,7 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { It("Failed to process mapping", func() { mod.Spec.ModuleLoader.Container.KernelMappings = []kmmv1beta1.KernelMapping{} - mockKernelAPI.EXPECT().GetModuleLoaderDataForKernel(mod, kernelVersion).Return(nil, fmt.Errorf("some error")) + mockMLDCreator.EXPECT().FromModule(mod, kernelVersion).Return(nil, fmt.Errorf("some error")) res, message := p.PreflightUpgradeCheck(context.Background(), pv, mod) @@ -125,7 +124,7 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { mld.Sign = &kmmv1beta1.Sign{} } - mockKernelAPI.EXPECT().GetModuleLoaderDataForKernel(mod, kernelVersion).Return(&mld, nil) + mockMLDCreator.EXPECT().FromModule(mod, kernelVersion).Return(&mld, nil) mockStatusUpdater.EXPECT().PreflightSetVerificationStage(context.Background(), pv, mld.Name, kmmv1beta1.VerificationStageImage).Return(nil) preflightHelper.EXPECT().verifyImage(ctx, &mld).Return(imageVerified, "image message") if !imageVerified { diff --git a/internal/sign/helper.go b/internal/sign/helper.go deleted file mode 100644 index 743399682..000000000 --- a/internal/sign/helper.go +++ /dev/null @@ -1,58 +0,0 @@ -package sign - -import ( - kmmv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" -) - -//go:generate mockgen -source=helper.go -package=sign -destination=mock_helper.go - -type Helper interface { - GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernel string) (*kmmv1beta1.Sign, error) -} - -type helper struct { -} - -func NewSignerHelper() Helper { - return &helper{} -} - -func (m *helper) GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernel 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) - 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 -} diff --git a/internal/sign/helper_test.go b/internal/sign/helper_test.go deleted file mode 100644 index 0e5829584..000000000 --- a/internal/sign/helper_test.go +++ /dev/null @@ -1,168 +0,0 @@ -package sign - -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("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 = NewSignerHelper() - }) - - 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}, - }, - ), - ) - -}) -var _ = Describe("GetRelevantSign", func() { - - const ( - unsignedImage = "my.registry/my/image" - keySecret = "securebootkey" - certSecret = "securebootcert" - filesToSign = "/modules/${KERNEL_VERSION}/simple-kmod.ko:/modules/${KERNEL_VERSION}/simple-procfs-kmod.ko" - kernelVersion = "1.2.3" - ) - - var ( - h Helper - ) - - BeforeEach(func() { - h = NewSignerHelper() - }) - - expected := &kmmv1beta1.Sign{ - UnsignedImage: unsignedImage + ":" + kernelVersion, - KeySecret: &v1.LocalObjectReference{Name: keySecret}, - CertSecret: &v1.LocalObjectReference{Name: certSecret}, - FilesToSign: strings.Split("/modules/"+kernelVersion+"/simple-kmod.ko:/modules/"+kernelVersion+"/simple-procfs-kmod.ko", ":"), - } - - DescribeTable("should set fields correctly", func(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign) { - actual, _ := h.GetRelevantSign(moduleSign, mappingSign, kernelVersion) - Expect( - cmp.Diff(expected, actual), - ).To( - BeEmpty(), - ) - }, - Entry( - "no km.Sign", - &kmmv1beta1.Sign{ - UnsignedImage: unsignedImage + ":${KERNEL_VERSION}", - KeySecret: &v1.LocalObjectReference{Name: keySecret}, - CertSecret: &v1.LocalObjectReference{Name: certSecret}, - FilesToSign: strings.Split(filesToSign, ":"), - }, - nil, - ), - Entry( - "no container.Sign", - nil, - &kmmv1beta1.Sign{ - UnsignedImage: unsignedImage + ":${KERNEL_VERSION}", - KeySecret: &v1.LocalObjectReference{Name: keySecret}, - CertSecret: &v1.LocalObjectReference{Name: certSecret}, - FilesToSign: strings.Split(filesToSign, ":"), - }, - ), - ) -}) diff --git a/internal/sign/job/manager.go b/internal/sign/job/manager.go index 1cba3646f..a3a8b3779 100644 --- a/internal/sign/job/manager.go +++ b/internal/sign/job/manager.go @@ -57,7 +57,7 @@ func (jbm *signJobManager) GarbageCollect(ctx context.Context, modName, namespac func (jbm *signJobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { // if there is no sign specified skip - if !module.ShouldBeSigned(mld) { + if !mld.SignConfigured() { return false, nil } diff --git a/internal/sign/job/signer.go b/internal/sign/job/signer.go index 5070d450f..1c2708b44 100644 --- a/internal/sign/job/signer.go +++ b/internal/sign/job/signer.go @@ -106,10 +106,13 @@ func (s *signer) MakeJobTemplate( if imageToSign != "" { td.UnsignedImage = imageToSign - } else if signConfig.UnsignedImage != "" { - td.UnsignedImage = signConfig.UnsignedImage } else { - return nil, fmt.Errorf("no image to sign given") + var err error + + td.UnsignedImage, err = mld.UnsignedImage() + if err != nil { + return nil, fmt.Errorf("could not determine the unsigned image: %v", err) + } } if signConfig.UnsignedImageRegistryTLS.Insecure { diff --git a/internal/sign/mock_helper.go b/internal/sign/mock_helper.go deleted file mode 100644 index 64e0528bf..000000000 --- a/internal/sign/mock_helper.go +++ /dev/null @@ -1,50 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: helper.go - -// Package sign is a generated GoMock package. -package sign - -import ( - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - v1beta1 "github.com/kubernetes-sigs/kernel-module-management/api/v1beta1" -) - -// MockHelper is a mock of Helper interface. -type MockHelper struct { - ctrl *gomock.Controller - recorder *MockHelperMockRecorder -} - -// MockHelperMockRecorder is the mock recorder for MockHelper. -type MockHelperMockRecorder struct { - mock *MockHelper -} - -// NewMockHelper creates a new mock instance. -func NewMockHelper(ctrl *gomock.Controller) *MockHelper { - mock := &MockHelper{ctrl: ctrl} - mock.recorder = &MockHelperMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockHelper) EXPECT() *MockHelperMockRecorder { - return m.recorder -} - -// GetRelevantSign mocks base method. -func (m *MockHelper) GetRelevantSign(moduleSign, mappingSign *v1beta1.Sign, kernel string) (*v1beta1.Sign, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetRelevantSign", moduleSign, mappingSign, kernel) - ret0, _ := ret[0].(*v1beta1.Sign) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetRelevantSign indicates an expected call of GetRelevantSign. -func (mr *MockHelperMockRecorder) GetRelevantSign(moduleSign, mappingSign, kernel interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRelevantSign", reflect.TypeOf((*MockHelper)(nil).GetRelevantSign), moduleSign, mappingSign, kernel) -} diff --git a/internal/utils/image/image.go b/internal/utils/image/image.go new file mode 100644 index 000000000..1b91cbe1b --- /dev/null +++ b/internal/utils/image/image.go @@ -0,0 +1,35 @@ +package image + +import ( + "fmt" + "strings" + + "github.com/google/go-containerregistry/pkg/name" +) + +func setTag(imageName, tag string) string { + return fmt.Sprintf("%s:%s", imageName, tag) +} + +func SetTag(imageName, newTag string) (string, error) { + ref, err := name.ParseReference(imageName, name.WithDefaultTag(""), name.WithDefaultRegistry("")) + if err != nil { + return "", fmt.Errorf("could not parse the image name: %v", err) + } + + return setTag(ref.Context().Name(), newTag), nil +} + +func SetOrAppendTag(imageName, newTag, sep string) (string, error) { + ref, err := name.ParseReference(imageName, name.WithDefaultTag(""), name.WithDefaultRegistry("")) + if err != nil { + return "", fmt.Errorf("could not parse the image name: %v", err) + } + + // Only digest names contain a semicolon + if identifier := ref.Identifier(); identifier != "" && !strings.Contains(identifier, ":") { + newTag = identifier + sep + newTag + } + + return setTag(ref.Context().Name(), newTag), nil +} diff --git a/internal/utils/image/image_test.go b/internal/utils/image/image_test.go new file mode 100644 index 000000000..43346702b --- /dev/null +++ b/internal/utils/image/image_test.go @@ -0,0 +1,29 @@ +package image_test + +import ( + "github.com/kubernetes-sigs/kernel-module-management/internal/utils/image" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("SetOrAppendTag", func() { + DescribeTable( + "should work as expected", + func(imageName, tag, sep, outputName string, expectsError bool) { + expect := Expect( + image.SetOrAppendTag(imageName, tag, sep), + ) + + if expectsError { + expect.Error().To(HaveOccurred()) + return + } + + expect.To(Equal(outputName)) + }, + Entry(nil, "example.org/repo/image:my-tag", "extra", "_", "example.org/repo/image:my-tag_extra", false), + Entry(nil, "example.org/repo/image", "extra", "_", "example.org/repo/image:extra", false), + Entry(nil, "", "", "", "", true), + Entry(nil, "example.org/repo/image@sha256:62fa0f2b9e7cabfb58d8ef34eaacd22e17e0d342c15a8e061bf814780aa3427a", "extra", "_", "example.org/repo/image:extra", false), + ) +}) diff --git a/internal/utils/image/suite_test.go b/internal/utils/image/suite_test.go new file mode 100644 index 000000000..5dfef70db --- /dev/null +++ b/internal/utils/image/suite_test.go @@ -0,0 +1,23 @@ +package image_test + +import ( + "testing" + + "github.com/kubernetes-sigs/kernel-module-management/internal/test" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/apimachinery/pkg/runtime" +) + +var scheme *runtime.Scheme + +func TestSuite(t *testing.T) { + RegisterFailHandler(Fail) + + var err error + + scheme, err = test.TestScheme() + Expect(err).NotTo(HaveOccurred()) + + RunSpecs(t, "Image Suite") +}