diff --git a/cmd/manager-hub/main.go b/cmd/manager-hub/main.go index 0505be09c..465a7bd9e 100644 --- a/cmd/manager-hub/main.go +++ b/cmd/manager-hub/main.go @@ -19,6 +19,8 @@ package main import ( "flag" + "github.com/kubernetes-sigs/kernel-module-management/internal/api" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -36,19 +38,15 @@ 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" "github.com/kubernetes-sigs/kernel-module-management/internal/constants" "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" //+kubebuilder:scaffold:imports ) @@ -105,20 +103,26 @@ func main() { metricsAPI.Register() registryAPI := registry.NewRegistry() - jobHelperAPI := utils.NewJobHelper(client) - buildHelper := build.NewHelper() + jobHelperAPI := imgbuild.NewJobHelper(client) + builderImage := cmd.GetEnvOrFatalError(constants.BuildImageEnvVar, setupLogger) - buildAPI := job.NewBuildManager( + buildAPI := imgbuild.NewBuildManager( client, - job.NewMaker(client, buildHelper, jobHelperAPI, scheme), jobHelperAPI, + build.NewJobMaker(client, builderImage, jobHelperAPI, scheme), registryAPI, ) - signAPI := signjob.NewSignJobManager( + signAPI := imgbuild.NewSignManager( client, - signjob.NewSigner(client, scheme, jobHelperAPI), jobHelperAPI, + sign.NewJobMaker( + client, + jobHelperAPI, + builderImage, + cmd.GetEnvOrFatalError(constants.SignImageEnvVar, setupLogger), + scheme, + ), registryAPI, ) @@ -130,7 +134,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.NewModuleLoaderDataFactory(), buildAPI, signAPI, operatorNamespace), statusupdater.NewManagedClusterModuleStatusUpdater(client), filterAPI, ) diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 50b60a410..6fa3fa4a9 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -22,6 +22,8 @@ import ( "os" "strconv" + "github.com/kubernetes-sigs/kernel-module-management/internal/api" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" @@ -38,19 +40,15 @@ 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" //+kubebuilder:scaffold:imports ) @@ -113,32 +111,38 @@ func main() { metricsAPI.Register() registryAPI := registry.NewRegistry() - jobHelperAPI := utils.NewJobHelper(client) - buildHelperAPI := build.NewHelper() + jobHelperAPI := imgbuild.NewJobHelper(client) + buildImage := cmd.GetEnvOrFatalError(constants.BuildImageEnvVar, setupLogger) - buildAPI := job.NewBuildManager( + buildAPI := imgbuild.NewBuildManager( client, - job.NewMaker(client, buildHelperAPI, jobHelperAPI, scheme), jobHelperAPI, + build.NewJobMaker(client, buildImage, jobHelperAPI, scheme), registryAPI, ) - signAPI := signjob.NewSignJobManager( + signAPI := imgbuild.NewSignManager( client, - signjob.NewSigner(client, scheme, jobHelperAPI), jobHelperAPI, + sign.NewJobMaker( + client, + jobHelperAPI, + buildImage, + cmd.GetEnvOrFatalError(constants.SignImageEnvVar, setupLogger), + scheme, + ), registryAPI, ) daemonAPI := daemonset.NewCreator(client, constants.KernelLabel, scheme) - kernelAPI := module.NewKernelMapper(buildHelperAPI, sign.NewSignerHelper()) + mldFactory := api.NewModuleLoaderDataFactory() mc := controllers.NewModuleReconciler( client, buildAPI, signAPI, daemonAPI, - kernelAPI, + mldFactory, metricsAPI, filterAPI, statusupdater.NewModuleStatusUpdater(client), @@ -160,7 +164,7 @@ func main() { } preflightStatusUpdaterAPI := statusupdater.NewPreflightStatusUpdater(client) - preflightAPI := preflight.NewPreflightAPI(client, buildAPI, signAPI, registryAPI, preflightStatusUpdaterAPI, kernelAPI) + preflightAPI := preflight.NewPreflightAPI(client, buildAPI, signAPI, registryAPI, preflightStatusUpdaterAPI, mldFactory) 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..55d8fc219 100644 --- a/controllers/module_reconciler.go +++ b/controllers/module_reconciler.go @@ -26,8 +26,8 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/build" "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/imgbuild" "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" @@ -64,9 +64,9 @@ type ModuleReconciler struct { func NewModuleReconciler( client client.Client, buildAPI build.Manager, - signAPI sign.SignManager, + signAPI sign.Manager, daemonAPI daemonset.DaemonSetCreator, - kernelAPI module.KernelMapper, + kernelAPI api.ModuleLoaderDataFactory, metricsAPI metrics.Metrics, filter *filter.Filter, statusUpdaterAPI statusupdater.ModuleStatusUpdater, @@ -198,17 +198,17 @@ type hashData struct { type moduleReconcilerHelper struct { client client.Client buildAPI build.Manager - signAPI sign.SignManager + signAPI sign.Manager daemonAPI daemonset.DaemonSetCreator - kernelAPI module.KernelMapper + kernelAPI api.ModuleLoaderDataFactory metricsAPI metrics.Metrics } func newModuleReconcilerHelper(client client.Client, buildAPI build.Manager, - signAPI sign.SignManager, + signAPI sign.Manager, daemonAPI daemonset.DaemonSetCreator, - kernelAPI module.KernelMapper, + kernelAPI api.ModuleLoaderDataFactory, metricsAPI metrics.Metrics) moduleReconcilerHelperAPI { return &moduleReconcilerHelper{ client: client, @@ -243,7 +243,7 @@ func (mrh *moduleReconcilerHelper) getRelevantKernelMappingsAndNodes(ctx context continue } - mld, err := mrh.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) + mld, err := mrh.kernelAPI.FromModule(mod, kernelVersion) if err != nil { nodeLogger.Error(err, "failed to get and process kernel mapping") continue @@ -301,9 +301,9 @@ func (mrh *moduleReconcilerHelper) handleBuild(ctx context.Context, mld *api.Mod completedSuccessfully := false switch buildStatus { - case utils.StatusCompleted: + case imgbuild.StatusCompleted: completedSuccessfully = true - case utils.StatusFailed: + case imgbuild.StatusFailed: logger.Info(utils.WarnString("Build job has failed. If the fix is not in Module CR, then delete job after the fix in order to restart the job")) } @@ -320,25 +320,19 @@ 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) - } - logger := log.FromContext(ctx).WithValues("kernel version", mld.KernelVersion, "image", mld.ContainerImage) signCtx := log.IntoContext(ctx, logger) - signStatus, err := mrh.signAPI.Sync(signCtx, mld, previousImage, true, mld.Owner) + signStatus, err := mrh.signAPI.Sync(signCtx, mld, true, mld.Owner) if err != nil { return false, fmt.Errorf("could not synchronize the signing: %w", err) } completedSuccessfully := false switch signStatus { - case utils.StatusCompleted: + case imgbuild.StatusCompleted: completedSuccessfully = true - case utils.StatusFailed: + case imgbuild.StatusFailed: logger.Info(utils.WarnString("Sign job has failed. If the fix is not in Module CR, then delete job after the fix in order to restart the job")) } diff --git a/controllers/module_reconciler_test.go b/controllers/module_reconciler_test.go index c24a67ef0..dd1734460 100644 --- a/controllers/module_reconciler_test.go +++ b/controllers/module_reconciler_test.go @@ -8,14 +8,11 @@ import ( "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/client" "github.com/kubernetes-sigs/kernel-module-management/internal/daemonset" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" "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" "github.com/mitchellh/hashstructure/v2" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -79,10 +76,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { DescribeTable("check error flows", func(getModuleError, getNodesError, getMappingsError, getDSError, handleBuildError, handleSignError, handleDCError, handlePluginError, gcError bool) { mod := kmmv1beta1.Module{} - selectNodesList := []v1.Node{v1.Node{}} - kernelNodesList := []v1.Node{v1.Node{}} - mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}} - moduleDS := []appsv1.DaemonSet{appsv1.DaemonSet{}} + selectNodesList := []v1.Node{{}} + kernelNodesList := []v1.Node{{}} + mappings := map[string]*api.ModuleLoaderData{"kernelVersion": {}} + moduleDS := []appsv1.DaemonSet{{}} returnedError := fmt.Errorf("some error") if getModuleError { mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(nil, returnedError) @@ -144,7 +141,7 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { Entry("getRelevantKernelMappingsAndNodes failed", false, false, true, false, false, false, false, false, false), Entry("ModuleDaemonSetsByKernelVersion failed", false, false, false, true, false, false, false, false, false), Entry("handleBuild failed ", false, false, false, false, true, false, false, false, false), - Entry("handleSig failedn", false, false, false, false, false, true, false, false, false), + Entry("handleSig failed", false, false, false, false, false, true, false, false, false), Entry("handleDriverContainer failed", false, false, false, false, false, false, true, false, false), Entry("handleDevicePlugin failed", false, false, false, false, false, false, false, true, false), Entry("garbageCollect failed", false, false, false, false, false, false, false, false, true), @@ -153,10 +150,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { It("Build has not completed successfully", func() { mod := kmmv1beta1.Module{} - selectNodesList := []v1.Node{v1.Node{}} - kernelNodesList := []v1.Node{v1.Node{}} - mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}} - moduleDS := []appsv1.DaemonSet{appsv1.DaemonSet{}} + selectNodesList := []v1.Node{{}} + kernelNodesList := []v1.Node{{}} + mappings := map[string]*api.ModuleLoaderData{"kernelVersion": {}} + moduleDS := []appsv1.DaemonSet{{}} gomock.InOrder( mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(&mod, nil), mockReconHelper.EXPECT().setKMMOMetrics(ctx), @@ -178,10 +175,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { It("Signing has not completed successfully", func() { mod := kmmv1beta1.Module{} - selectNodesList := []v1.Node{v1.Node{}} - kernelNodesList := []v1.Node{v1.Node{}} - mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}} - moduleDS := []appsv1.DaemonSet{appsv1.DaemonSet{}} + selectNodesList := []v1.Node{{}} + kernelNodesList := []v1.Node{{}} + mappings := map[string]*api.ModuleLoaderData{"kernelVersion": {}} + moduleDS := []appsv1.DaemonSet{{}} gomock.InOrder( mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(&mod, nil), mockReconHelper.EXPECT().setKMMOMetrics(ctx), @@ -203,10 +200,10 @@ var _ = Describe("ModuleReconciler_Reconcile", func() { It("Good flow", func() { mod := kmmv1beta1.Module{} - selectNodesList := []v1.Node{v1.Node{}} - kernelNodesList := []v1.Node{v1.Node{}} - mappings := map[string]*api.ModuleLoaderData{"kernelVersion": &api.ModuleLoaderData{}} - moduleDS := []appsv1.DaemonSet{appsv1.DaemonSet{}} + selectNodesList := []v1.Node{{}} + kernelNodesList := []v1.Node{{}} + mappings := map[string]*api.ModuleLoaderData{"kernelVersion": {}} + moduleDS := []appsv1.DaemonSet{{}} gomock.InOrder( mockReconHelper.EXPECT().getRequestedModule(ctx, nsn).Return(&mod, nil), mockReconHelper.EXPECT().setKMMOMetrics(ctx), @@ -254,7 +251,7 @@ var _ = Describe("ModuleReconciler_getNodesListBySelector", func() { node1 := v1.Node{ Spec: v1.NodeSpec{ Taints: []v1.Taint{ - v1.Taint{ + { Effect: v1.TaintEffectNoSchedule, }, }, @@ -264,7 +261,7 @@ var _ = Describe("ModuleReconciler_getNodesListBySelector", func() { node3 := v1.Node{ Spec: v1.NodeSpec{ Taints: []v1.Taint{ - v1.Taint{ + { Effect: v1.TaintEffectPreferNoSchedule, }, }, @@ -286,15 +283,15 @@ var _ = Describe("ModuleReconciler_getNodesListBySelector", func() { var _ = Describe("ModuleReconciler_getRelevantKernelMappingsAndNodes", func() { var ( - ctrl *gomock.Controller - mockKM *module.MockKernelMapper - mhr moduleReconcilerHelperAPI + ctrl *gomock.Controller + mockModuleLoaderDataFactory *api.MockModuleLoaderDataFactory + mhr moduleReconcilerHelperAPI ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockKM = module.NewMockKernelMapper(ctrl) - mhr = newModuleReconcilerHelper(nil, nil, nil, nil, mockKM, nil) + mockModuleLoaderDataFactory = api.NewMockModuleLoaderDataFactory(ctrl) + mhr = newModuleReconcilerHelper(nil, nil, nil, nil, mockModuleLoaderDataFactory, nil) }) node1 := v1.Node{ @@ -327,8 +324,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), + mockModuleLoaderDataFactory.EXPECT().FromModule(&kmmv1beta1.Module{}, node1.Status.NodeInfo.KernelVersion).Return(&mld1, nil), + mockModuleLoaderDataFactory.EXPECT().FromModule(&kmmv1beta1.Module{}, node2.Status.NodeInfo.KernelVersion).Return(&mld2, nil), ) mappings, resNodes, err := mhr.getRelevantKernelMappingsAndNodes(context.Background(), &kmmv1beta1.Module{}, nodes) @@ -344,8 +341,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")), + mockModuleLoaderDataFactory.EXPECT().FromModule(&kmmv1beta1.Module{}, node1.Status.NodeInfo.KernelVersion).Return(&mld1, nil), + mockModuleLoaderDataFactory.EXPECT().FromModule(&kmmv1beta1.Module{}, node2.Status.NodeInfo.KernelVersion).Return(nil, fmt.Errorf("some error")), ) mappings, resNodes, err := mhr.getRelevantKernelMappingsAndNodes(context.Background(), &kmmv1beta1.Module{}, nodes) @@ -361,14 +358,14 @@ var _ = Describe("ModuleReconciler_getRelevantKernelMappingsAndNodes", func() { var _ = Describe("ModuleReconciler_handleBuild", func() { var ( ctrl *gomock.Controller - mockBM *build.MockManager + mockBM *imgbuild.MockJobManager mockMetrics *metrics.MockMetrics mhr moduleReconcilerHelperAPI ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockBM = build.NewMockManager(ctrl) + mockBM = imgbuild.NewMockJobManager(ctrl) mockMetrics = metrics.NewMockMetrics(ctrl) mhr = newModuleReconcilerHelper(nil, mockBM, nil, nil, nil, mockMetrics) }) @@ -402,7 +399,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { gomock.InOrder( mockBM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), - mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mld.Owner).Return(utils.Status(utils.StatusCreated), nil), + mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mld.Owner).Return(imgbuild.StatusCreated, nil), ) completed, err := mhr.handleBuild(context.Background(), &mld) @@ -422,7 +419,7 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { } gomock.InOrder( mockBM.EXPECT().ShouldSync(gomock.Any(), mld).Return(true, nil), - mockBM.EXPECT().Sync(gomock.Any(), mld, true, mld.Owner).Return(utils.Status(utils.StatusCompleted), nil), + mockBM.EXPECT().Sync(gomock.Any(), mld, true, mld.Owner).Return(imgbuild.StatusCompleted, nil), ) completed, err := mhr.handleBuild(context.Background(), mld) @@ -435,14 +432,14 @@ var _ = Describe("ModuleReconciler_handleBuild", func() { var _ = Describe("ModuleReconciler_handleSigning", func() { var ( ctrl *gomock.Controller - mockSM *sign.MockSignManager + mockSM *imgbuild.MockJobManager mockMetrics *metrics.MockMetrics mhr moduleReconcilerHelperAPI ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockSM = sign.NewMockSignManager(ctrl) + mockSM = imgbuild.NewMockJobManager(ctrl) mockMetrics = metrics.NewMockMetrics(ctrl) mhr = newModuleReconcilerHelper(nil, nil, mockSM, nil, nil, mockMetrics) }) @@ -480,7 +477,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { gomock.InOrder( mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), - mockSM.EXPECT().Sync(gomock.Any(), &mld, "", true, mld.Owner).Return(utils.Status(utils.StatusCreated), nil), + mockSM.EXPECT().Sync(gomock.Any(), &mld, true, mld.Owner).Return(imgbuild.StatusCreated, nil), ) completed, err := mhr.handleSigning(context.Background(), &mld) @@ -500,7 +497,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { gomock.InOrder( mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(true, nil), - mockSM.EXPECT().Sync(gomock.Any(), &mld, "", true, mld.Owner).Return(utils.Status(utils.StatusCompleted), nil), + mockSM.EXPECT().Sync(gomock.Any(), &mld, true, mld.Owner).Return(imgbuild.StatusCompleted, nil), ) completed, err := mhr.handleSigning(context.Background(), &mld) @@ -522,8 +519,7 @@ var _ = Describe("ModuleReconciler_handleSigning", func() { gomock.InOrder( mockSM.EXPECT().ShouldSync(gomock.Any(), mld).Return(true, nil), - mockSM.EXPECT().Sync(gomock.Any(), mld, imageName+":"+namespace+"_"+moduleName+"_kmm_unsigned", true, mld.Owner). - Return(utils.Status(utils.StatusCompleted), nil), + mockSM.EXPECT().Sync(gomock.Any(), mld, true, mld.Owner).Return(imgbuild.StatusCompleted, nil), ) completed, err := mhr.handleSigning(context.Background(), mld) @@ -722,16 +718,16 @@ var _ = Describe("ModuleReconciler_handleDevicePlugin", func() { var _ = Describe("ModuleReconciler_garbageCollect", func() { var ( ctrl *gomock.Controller - mockBM *build.MockManager - mockSM *sign.MockSignManager + mockBM *imgbuild.MockJobManager + mockSM *imgbuild.MockJobManager mockDC *daemonset.MockDaemonSetCreator mhr moduleReconcilerHelperAPI ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockBM = build.NewMockManager(ctrl) - mockSM = sign.NewMockSignManager(ctrl) + mockBM = imgbuild.NewMockJobManager(ctrl) + mockSM = imgbuild.NewMockJobManager(ctrl) mockDC = daemonset.NewMockDaemonSetCreator(ctrl) mhr = newModuleReconcilerHelper(nil, mockBM, mockSM, mockDC, nil, nil) }) @@ -745,9 +741,9 @@ var _ = Describe("ModuleReconciler_garbageCollect", func() { It("good flow", func() { mldMappings := map[string]*api.ModuleLoaderData{ - "kernelVersion1": &api.ModuleLoaderData{}, "kernelVersion2": &api.ModuleLoaderData{}, + "kernelVersion1": {}, "kernelVersion2": {}, } - existingDS := []appsv1.DaemonSet{appsv1.DaemonSet{}, appsv1.DaemonSet{}} + existingDS := make([]appsv1.DaemonSet, 2) kernelSet := sets.New[string]("kernelVersion1", "kernelVersion2") gomock.InOrder( mockDC.EXPECT().GarbageCollect(context.Background(), mod, existingDS, kernelSet).Return(nil, nil), @@ -763,9 +759,9 @@ var _ = Describe("ModuleReconciler_garbageCollect", func() { DescribeTable("check error flows", func(dcError, buildError bool) { returnedError := fmt.Errorf("some error") mldMappings := map[string]*api.ModuleLoaderData{ - "kernelVersion1": &api.ModuleLoaderData{}, "kernelVersion2": &api.ModuleLoaderData{}, + "kernelVersion1": {}, "kernelVersion2": {}, } - existingDS := []appsv1.DaemonSet{appsv1.DaemonSet{}, appsv1.DaemonSet{}} + existingDS := make([]appsv1.DaemonSet, 2) kernelSet := sets.New[string]("kernelVersion1", "kernelVersion2") if dcError { mockDC.EXPECT().GarbageCollect(context.Background(), mod, existingDS, kernelSet).Return(nil, returnedError) diff --git a/go.mod b/go.mod index fe470cc85..cc8e9a674 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,6 @@ require ( github.com/onsi/ginkgo/v2 v2.9.2 github.com/onsi/gomega v1.27.5 github.com/prometheus/client_golang v1.14.0 - golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4 k8s.io/api v0.26.3 k8s.io/apimachinery v0.26.3 k8s.io/client-go v0.26.3 diff --git a/go.sum b/go.sum index df689ae61..25d89b71c 100644 --- a/go.sum +++ b/go.sum @@ -350,8 +350,6 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4= golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= -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/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/internal/api/factory.go b/internal/api/factory.go new file mode 100644 index 000000000..acb9b0edc --- /dev/null +++ b/internal/api/factory.go @@ -0,0 +1,199 @@ +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=factory.go -package=api -destination=mock_factory.go ModuleLoaderDataFactory,moduleLoaderDataFactoryHelper + +type ModuleLoaderDataFactory interface { + FromModule(mod *kmmv1beta1.Module, kernelVersion string) (*ModuleLoaderData, error) +} + +type moduleLoaderDataFactory struct { + helper moduleLoaderDataFactoryHelper +} + +func NewModuleLoaderDataFactory() ModuleLoaderDataFactory { + return &moduleLoaderDataFactory{ + helper: &moduleLoaderDataFactoryHelperImpl{ + sectionGetter: §ionGetterImpl{}, + }, + } +} + +func (f *moduleLoaderDataFactory) 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 moduleLoaderDataFactoryHelper 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 moduleLoaderDataFactoryHelperImpl struct { + sectionGetter sectionGetter +} + +func (kh *moduleLoaderDataFactoryHelperImpl) 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 *moduleLoaderDataFactoryHelperImpl) 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.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 *moduleLoaderDataFactoryHelperImpl) 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/factory_test.go b/internal/api/factory_test.go new file mode 100644 index 000000000..284aa7974 --- /dev/null +++ b/internal/api/factory_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("moduleLoaderDataFactoryHelper_findKernelMapping", func() { + const kernelVersion = "1.2.3" + + var ( + ctrl *gomock.Controller + mldfh moduleLoaderDataFactoryHelper + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + mldfh = &moduleLoaderDataFactoryHelperImpl{ + 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("moduleLoaderDataFactoryHelper_prepareModuleLoaderData", func() { + const kernelVersion = "1.2.3" + + var ( + ctrl *gomock.Controller + mod kmmv1beta1.Module + mapping kmmv1beta1.KernelMapping + mldfh *moduleLoaderDataFactoryHelperImpl + sg *MocksectionGetter + ) + + BeforeEach(func() { + ctrl = gomock.NewController(GinkgoT()) + sg = NewMocksectionGetter(ctrl) + mldfh = &moduleLoaderDataFactoryHelperImpl{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("moduleLoaderDataFactoryHelper_replaceTemplates", func() { + const kernelVersion = "5.8.18-100.fc31.x86_64" + + mdlfh := moduleLoaderDataFactoryHelperImpl{} + + 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_factory.go b/internal/api/mock_factory.go new file mode 100644 index 000000000..ee2398ee0 --- /dev/null +++ b/internal/api/mock_factory.go @@ -0,0 +1,169 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: factory.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" +) + +// MockModuleLoaderDataFactory is a mock of ModuleLoaderDataFactory interface. +type MockModuleLoaderDataFactory struct { + ctrl *gomock.Controller + recorder *MockModuleLoaderDataFactoryMockRecorder +} + +// MockModuleLoaderDataFactoryMockRecorder is the mock recorder for MockModuleLoaderDataFactory. +type MockModuleLoaderDataFactoryMockRecorder struct { + mock *MockModuleLoaderDataFactory +} + +// NewMockModuleLoaderDataFactory creates a new mock instance. +func NewMockModuleLoaderDataFactory(ctrl *gomock.Controller) *MockModuleLoaderDataFactory { + mock := &MockModuleLoaderDataFactory{ctrl: ctrl} + mock.recorder = &MockModuleLoaderDataFactoryMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockModuleLoaderDataFactory) EXPECT() *MockModuleLoaderDataFactoryMockRecorder { + return m.recorder +} + +// FromModule mocks base method. +func (m *MockModuleLoaderDataFactory) 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 *MockModuleLoaderDataFactoryMockRecorder) FromModule(mod, kernelVersion interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FromModule", reflect.TypeOf((*MockModuleLoaderDataFactory)(nil).FromModule), mod, kernelVersion) +} + +// MockmoduleLoaderDataFactoryHelper is a mock of moduleLoaderDataFactoryHelper interface. +type MockmoduleLoaderDataFactoryHelper struct { + ctrl *gomock.Controller + recorder *MockmoduleLoaderDataFactoryHelperMockRecorder +} + +// MockmoduleLoaderDataFactoryHelperMockRecorder is the mock recorder for MockmoduleLoaderDataFactoryHelper. +type MockmoduleLoaderDataFactoryHelperMockRecorder struct { + mock *MockmoduleLoaderDataFactoryHelper +} + +// NewMockmoduleLoaderDataFactoryHelper creates a new mock instance. +func NewMockmoduleLoaderDataFactoryHelper(ctrl *gomock.Controller) *MockmoduleLoaderDataFactoryHelper { + mock := &MockmoduleLoaderDataFactoryHelper{ctrl: ctrl} + mock.recorder = &MockmoduleLoaderDataFactoryHelperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockmoduleLoaderDataFactoryHelper) EXPECT() *MockmoduleLoaderDataFactoryHelperMockRecorder { + return m.recorder +} + +// findKernelMapping mocks base method. +func (m *MockmoduleLoaderDataFactoryHelper) 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 *MockmoduleLoaderDataFactoryHelperMockRecorder) findKernelMapping(mappings, kernelVersion interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "findKernelMapping", reflect.TypeOf((*MockmoduleLoaderDataFactoryHelper)(nil).findKernelMapping), mappings, kernelVersion) +} + +// prepareModuleLoaderData mocks base method. +func (m *MockmoduleLoaderDataFactoryHelper) 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 *MockmoduleLoaderDataFactoryHelperMockRecorder) prepareModuleLoaderData(mapping, mod, kernelVersion interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "prepareModuleLoaderData", reflect.TypeOf((*MockmoduleLoaderDataFactoryHelper)(nil).prepareModuleLoaderData), mapping, mod, kernelVersion) +} + +// replaceTemplates mocks base method. +func (m *MockmoduleLoaderDataFactoryHelper) 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 *MockmoduleLoaderDataFactoryHelperMockRecorder) replaceTemplates(mld interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "replaceTemplates", reflect.TypeOf((*MockmoduleLoaderDataFactoryHelper)(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 25621bd72..8c4f3f0a2 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 @@ -54,3 +58,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) IntermediateImageName() (string, error) { + return image.SetOrAppendTag( + mld.ContainerImage, + fmt.Sprintf("%s_%s_kmm_unsigned", mld.Namespace, mld.Name), + "_", + ) +} + +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 +} diff --git a/internal/api/moduleloaderdata_test.go b/internal/api/moduleloaderdata_test.go new file mode 100644 index 000000000..586500e10 --- /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_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, + ), + ) +}) + +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"), + ) + }) + }) +}) diff --git a/internal/build/job/suite_test.go b/internal/api/suite_test.go similarity index 90% rename from internal/build/job/suite_test.go rename to internal/api/suite_test.go index f82b1fdd7..11aff0ae2 100644 --- a/internal/build/job/suite_test.go +++ b/internal/api/suite_test.go @@ -1,4 +1,4 @@ -package job +package api import ( "testing" @@ -19,5 +19,5 @@ func TestSuite(t *testing.T) { scheme, err = test.TestScheme() Expect(err).NotTo(HaveOccurred()) - RunSpecs(t, "Job Suite") + 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/manager.go b/internal/build/job/manager.go deleted file mode 100644 index 467954e4e..000000000 --- a/internal/build/job/manager.go +++ /dev/null @@ -1,135 +0,0 @@ -package job - -import ( - "context" - "errors" - "fmt" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "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/utils" -) - -type jobManager struct { - client client.Client - maker Maker - jobHelper utils.JobHelper - registry registry.Registry -} - -func NewBuildManager( - client client.Client, - maker Maker, - jobHelper utils.JobHelper, - registry registry.Registry) *jobManager { - return &jobManager{ - client: client, - maker: maker, - jobHelper: jobHelper, - registry: registry, - } -} - -func (jbm *jobManager) GarbageCollect(ctx context.Context, modName, namespace string, owner metav1.Object) ([]string, error) { - jobs, err := jbm.jobHelper.GetModuleJobs(ctx, modName, namespace, utils.JobTypeBuild, owner) - if err != nil { - return nil, fmt.Errorf("failed to get build jobs for module %s: %v", modName, err) - } - - deleteNames := make([]string, 0, len(jobs)) - for _, job := range jobs { - if job.Status.Succeeded == 1 { - err = jbm.jobHelper.DeleteJob(ctx, &job) - if err != nil { - return nil, fmt.Errorf("failed to delete build job %s: %v", job.Name, err) - } - deleteNames = append(deleteNames, job.Name) - } - } - return deleteNames, nil -} - -func (jbm *jobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { - - // if there is no build specified skip - if !module.ShouldBeBuilt(mld) { - 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) - } - - // build is specified and targetImage is either the final image or the intermediate image - // tag, depending on whether sign is specified or not. Either way, if targetImage exists - // we can skip building it - exists, err := module.ImageExists(ctx, jbm.client, jbm.registry, mld, mld.Namespace, targetImage) - if err != nil { - return false, fmt.Errorf("failed to check existence of image %s: %w", targetImage, err) - } - - return !exists, nil -} - -func (jbm *jobManager) Sync( - ctx context.Context, - mld *api.ModuleLoaderData, - pushImage bool, - owner metav1.Object) (utils.Status, error) { - - logger := log.FromContext(ctx) - - logger.Info("Building in-cluster") - - jobTemplate, err := jbm.maker.MakeJobTemplate(ctx, mld, owner, pushImage) - if err != nil { - return "", fmt.Errorf("could not make Job template: %v", err) - } - - job, err := jbm.jobHelper.GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, mld.KernelVersion, utils.JobTypeBuild, owner) - if err != nil { - if !errors.Is(err, utils.ErrNoMatchingJob) { - return "", fmt.Errorf("error getting the build: %v", err) - } - - logger.Info("Creating job") - err = jbm.jobHelper.CreateJob(ctx, jobTemplate) - if err != nil { - return "", fmt.Errorf("could not create Job: %v", err) - } - - return utils.StatusCreated, nil - } - - changed, err := jbm.jobHelper.IsJobChanged(job, jobTemplate) - if err != nil { - return "", fmt.Errorf("could not determine if job has changed: %v", err) - } - - if changed { - logger.Info("The module's build spec has been changed, deleting the current job so a new one can be created", "name", job.Name) - err = jbm.jobHelper.DeleteJob(ctx, job) - if err != nil { - logger.Info(utils.WarnString(fmt.Sprintf("failed to delete build job %s: %v", job.Name, err))) - } - return utils.StatusInProgress, nil - } - - logger.Info("Returning job status", "name", job.Name, "namespace", job.Namespace) - - statusmsg, err := jbm.jobHelper.GetJobStatus(job) - if err != nil { - return "", err - } - - return statusmsg, nil -} diff --git a/internal/build/job/manager_test.go b/internal/build/job/manager_test.go deleted file mode 100644 index b07055bef..000000000 --- a/internal/build/job/manager_test.go +++ /dev/null @@ -1,393 +0,0 @@ -package job - -import ( - "context" - "errors" - "fmt" - - "github.com/golang/mock/gomock" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - batchv1 "k8s.io/api/batch/v1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - 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/client" - "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "github.com/kubernetes-sigs/kernel-module-management/internal/registry" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" -) - -var _ = Describe("ShouldSync", func() { - var ( - ctrl *gomock.Controller - clnt *client.MockClient - reg *registry.MockRegistry - ) - const ( - moduleName = "module-name" - imageName = "image-name" - namespace = "some-namespace" - kernelVersion = "1.2.3" - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - reg = registry.NewMockRegistry(ctrl) - }) - - It("should return false if there was not build section", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{} - - mgr := NewBuildManager(clnt, nil, nil, reg) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).ToNot(HaveOccurred()) - Expect(shouldSync).To(BeFalse()) - }) - - It("should return false if image already exists", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - Build: &kmmv1beta1.Build{}, - ContainerImage: imageName, - ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, - } - - gomock.InOrder( - reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(true, nil), - ) - - mgr := NewBuildManager(clnt, nil, nil, reg) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).ToNot(HaveOccurred()) - Expect(shouldSync).To(BeFalse()) - }) - - It("should return false and an error if image check fails", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - Build: &kmmv1beta1.Build{}, - ContainerImage: imageName, - ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, - } - - gomock.InOrder( - reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, errors.New("generic-registry-error")), - ) - - mgr := NewBuildManager(clnt, nil, nil, reg) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("generic-registry-error")) - Expect(shouldSync).To(BeFalse()) - }) - - It("should return true if image does not exist", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - Build: &kmmv1beta1.Build{}, - ContainerImage: imageName, - ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, - } - - gomock.InOrder( - reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, nil), - ) - - mgr := NewBuildManager(clnt, nil, nil, reg) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).ToNot(HaveOccurred()) - Expect(shouldSync).To(BeTrue()) - }) -}) - -var _ = Describe("Sync", func() { - var ( - ctrl *gomock.Controller - clnt *client.MockClient - maker *MockMaker - jobhelper *utils.MockJobHelper - reg *registry.MockRegistry - ) - - const ( - imageName = "image-name" - namespace = "some-namespace" - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - maker = NewMockMaker(ctrl) - jobhelper = utils.NewMockJobHelper(ctrl) - reg = registry.NewMockRegistry(ctrl) - }) - - const ( - moduleName = "module-name" - kernelVersion = "1.2.3" - jobName = "some-job" - ) - - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{Name: moduleName}, - } - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Build: &kmmv1beta1.Build{}, - ContainerImage: imageName, - Owner: &mod, - KernelVersion: kernelVersion, - } - - DescribeTable("should return the correct status depending on the job status", - func(jobStatus utils.Status, expectsErr bool) { - j := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"label key": "some label"}, - Namespace: namespace, - Annotations: map[string]string{constants.JobHashAnnotation: "some hash"}, - }, - } - ctx := context.Background() - - gomock.InOrder( - maker.EXPECT().MakeJobTemplate(ctx, mld, mld.Owner, true).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeBuild, mld.Owner).Return(&j, nil), - jobhelper.EXPECT().IsJobChanged(&j, &j).Return(false, nil), - jobhelper.EXPECT().GetJobStatus(&j).Return(jobStatus, nil), - ) - - mgr := NewBuildManager(clnt, maker, jobhelper, reg) - - res, err := mgr.Sync(ctx, mld, true, mld.Owner) - - if expectsErr { - Expect(err).To(HaveOccurred()) - return - } - - Expect(res).To(Equal(jobStatus)) - }, - Entry("active", utils.Status(utils.StatusInProgress), false), - Entry("succeeded", utils.Status(utils.StatusCompleted), false), - Entry("failed", utils.Status(utils.StatusFailed), false), - ) - - It("should return an error if there was an error creating the job template", func() { - ctx := context.Background() - - gomock.InOrder( - maker.EXPECT().MakeJobTemplate(ctx, mld, mld.Owner, true).Return(nil, errors.New("random error")), - ) - - mgr := NewBuildManager(clnt, maker, jobhelper, reg) - - Expect( - mgr.Sync(ctx, mld, true, mld.Owner), - ).Error().To( - HaveOccurred(), - ) - }) - - It("should return an error if there was an error creating the job", func() { - ctx := context.Background() - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - }, - } - - gomock.InOrder( - maker.EXPECT().MakeJobTemplate(ctx, mld, mld.Owner, true).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeBuild, mld.Owner).Return(nil, utils.ErrNoMatchingJob), - jobhelper.EXPECT().CreateJob(ctx, &j).Return(errors.New("some error")), - ) - - mgr := NewBuildManager(clnt, maker, jobhelper, reg) - - Expect( - mgr.Sync(ctx, mld, true, mld.Owner), - ).Error().To( - HaveOccurred(), - ) - }) - - It("should create the job if there was no error making it", func() { - ctx := context.Background() - - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - }, - } - - gomock.InOrder( - maker.EXPECT().MakeJobTemplate(ctx, mld, mld.Owner, true).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeBuild, mld.Owner).Return(nil, utils.ErrNoMatchingJob), - jobhelper.EXPECT().CreateJob(ctx, &j).Return(nil), - ) - - mgr := NewBuildManager(clnt, maker, jobhelper, reg) - - Expect( - mgr.Sync(ctx, mld, true, mld.Owner), - ).To( - Equal(utils.Status(utils.StatusCreated)), - ) - }) - - It("should delete the job if it was edited", func() { - ctx := context.Background() - - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - Annotations: map[string]string{constants.JobHashAnnotation: "some hash"}, - }, - } - - newJob := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - Annotations: map[string]string{constants.JobHashAnnotation: "new hash"}, - }, - } - - gomock.InOrder( - maker.EXPECT().MakeJobTemplate(ctx, mld, mld.Owner, true).Return(&newJob, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeBuild, mld.Owner).Return(&j, nil), - jobhelper.EXPECT().IsJobChanged(&j, &newJob).Return(true, nil), - jobhelper.EXPECT().DeleteJob(ctx, &j).Return(nil), - ) - - mgr := NewBuildManager(clnt, maker, jobhelper, reg) - - Expect( - mgr.Sync(ctx, mld, true, mld.Owner), - ).To( - Equal(utils.Status(utils.StatusInProgress)), - ) - }) -}) - -var _ = Describe("GarbageCollect", func() { - var ( - ctrl *gomock.Controller - clnt *client.MockClient - maker *MockMaker - jobhelper *utils.MockJobHelper - reg *registry.MockRegistry - mgr *jobManager - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - maker = NewMockMaker(ctrl) - jobhelper = utils.NewMockJobHelper(ctrl) - reg = registry.NewMockRegistry(ctrl) - mgr = NewBuildManager(clnt, maker, jobhelper, reg) - }) - - mod := kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{Name: "moduleName"}, - } - - DescribeTable("should return the correct error and names of the collected jobs", - func(jobStatus1 batchv1.JobStatus, jobStatus2 batchv1.JobStatus, expectsErr bool) { - job1 := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "jobName1", - }, - Status: jobStatus1, - } - job2 := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "jobName2", - }, - Status: jobStatus2, - } - expectedNames := []string{} - if !expectsErr { - if job1.Status.Succeeded == 1 { - expectedNames = append(expectedNames, "jobName1") - } - if job2.Status.Succeeded == 1 { - expectedNames = append(expectedNames, "jobName2") - } - } - returnedError := fmt.Errorf("some error") - if !expectsErr { - returnedError = nil - } - - jobhelper.EXPECT().GetModuleJobs(context.Background(), mod.Name, mod.Namespace, utils.JobTypeBuild, &mod).Return([]batchv1.Job{job1, job2}, returnedError) - if !expectsErr { - if job1.Status.Succeeded == 1 { - jobhelper.EXPECT().DeleteJob(context.Background(), &job1).Return(nil) - } - if job2.Status.Succeeded == 1 { - jobhelper.EXPECT().DeleteJob(context.Background(), &job2).Return(nil) - } - } - - names, err := mgr.GarbageCollect(context.Background(), mod.Name, mod.Namespace, &mod) - - if expectsErr { - Expect(err).To(HaveOccurred()) - Expect(names).To(BeNil()) - } else { - Expect(err).NotTo(HaveOccurred()) - Expect(expectedNames).To(Equal(names)) - } - }, - Entry("all jobs succeeded", batchv1.JobStatus{Succeeded: 1}, batchv1.JobStatus{Succeeded: 1}, false), - Entry("1 job succeeded", batchv1.JobStatus{Succeeded: 1}, batchv1.JobStatus{Succeeded: 0}, false), - Entry("0 job succeeded", batchv1.JobStatus{Succeeded: 0}, batchv1.JobStatus{Succeeded: 0}, false), - Entry("error occured", batchv1.JobStatus{Succeeded: 0}, batchv1.JobStatus{Succeeded: 0}, true), - ) -}) diff --git a/internal/build/job/mock_maker.go b/internal/build/job/mock_maker.go deleted file mode 100644 index 11c36af8c..000000000 --- a/internal/build/job/mock_maker.go +++ /dev/null @@ -1,53 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: maker.go - -// Package job is a generated GoMock package. -package job - -import ( - context "context" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - api "github.com/kubernetes-sigs/kernel-module-management/internal/api" - v1 "k8s.io/api/batch/v1" - v10 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// MockMaker is a mock of Maker interface. -type MockMaker struct { - ctrl *gomock.Controller - recorder *MockMakerMockRecorder -} - -// MockMakerMockRecorder is the mock recorder for MockMaker. -type MockMakerMockRecorder struct { - mock *MockMaker -} - -// NewMockMaker creates a new mock instance. -func NewMockMaker(ctrl *gomock.Controller) *MockMaker { - mock := &MockMaker{ctrl: ctrl} - mock.recorder = &MockMakerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockMaker) EXPECT() *MockMakerMockRecorder { - return m.recorder -} - -// MakeJobTemplate mocks base method. -func (m *MockMaker) MakeJobTemplate(ctx context.Context, mld *api.ModuleLoaderData, owner v10.Object, pushImage bool) (*v1.Job, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MakeJobTemplate", ctx, mld, owner, pushImage) - ret0, _ := ret[0].(*v1.Job) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// MakeJobTemplate indicates an expected call of MakeJobTemplate. -func (mr *MockMakerMockRecorder) MakeJobTemplate(ctx, mld, owner, pushImage interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MakeJobTemplate", reflect.TypeOf((*MockMaker)(nil).MakeJobTemplate), ctx, mld, owner, pushImage) -} diff --git a/internal/build/job/maker.go b/internal/build/jobmaker.go similarity index 79% rename from internal/build/job/maker.go rename to internal/build/jobmaker.go index 253a21c7b..b10c5a5c8 100644 --- a/internal/build/job/maker.go +++ b/internal/build/jobmaker.go @@ -1,11 +1,11 @@ -package job +package build import ( "context" "fmt" - "os" - "strings" + "github.com/kubernetes-sigs/kernel-module-management/internal/build/utils" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" "github.com/mitchellh/hashstructure/v2" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -19,65 +19,50 @@ 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" + "github.com/kubernetes-sigs/kernel-module-management/internal/utils/image" ) const ( dockerfileVolumeName = "dockerfile" + kanikoContainerName = "kaniko" ) -//go:generate mockgen -source=maker.go -package=job -destination=mock_maker.go - -type Maker interface { - MakeJobTemplate( - ctx context.Context, - mld *api.ModuleLoaderData, - owner metav1.Object, - pushImage bool) (*batchv1.Job, error) +type buildMaker struct { + builderImage string + client client.Client + jobHelper imgbuild.JobHelper + scheme *runtime.Scheme } -type maker struct { - client client.Client - helper build.Helper - jobHelper utils.JobHelper - scheme *runtime.Scheme -} +var _ imgbuild.JobMaker = &buildMaker{} -type hashData struct { - Dockerfile string - PodTemplate *v1.PodTemplateSpec -} - -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, +func NewJobMaker(client client.Client, builderImage string, jobHelper imgbuild.JobHelper, scheme *runtime.Scheme) imgbuild.JobMaker { + return &buildMaker{ + builderImage: builderImage, + client: client, + jobHelper: jobHelper, + scheme: scheme, } } -func (m *maker) MakeJobTemplate( +func (m *buildMaker) MakeJob( ctx context.Context, mld *api.ModuleLoaderData, 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 generate the destination image for the build: %v", err) + } + + specTemplate, err := m.specTemplate(mld, containerImage, pushImage) + if err != nil { + return nil, fmt.Errorf("could not generate then spec template: %v", err) } - specTemplate := m.specTemplate(mld, containerImage, pushImage) specTemplateHash, err := m.getHashAnnotationValue( ctx, mld.Build.DockerfileConfigMap.Name, @@ -92,8 +77,8 @@ func (m *maker) MakeJobTemplate( ObjectMeta: metav1.ObjectMeta{ GenerateName: mld.Name + "-build-", Namespace: mld.Namespace, - Labels: m.jobHelper.JobLabels(mld.Name, mld.KernelVersion, utils.JobTypeBuild), - Annotations: map[string]string{constants.JobHashAnnotation: fmt.Sprintf("%d", specTemplateHash)}, + Labels: m.jobHelper.JobLabels(mld.Name, mld.KernelVersion, imgbuild.JobTypeBuild), + Annotations: map[string]string{imgbuild.JobHashAnnotation: fmt.Sprintf("%d", specTemplateHash)}, }, Spec: batchv1.JobSpec{ Completions: pointer.Int32(1), @@ -102,32 +87,33 @@ func (m *maker) MakeJobTemplate( }, } - if err := controllerutil.SetControllerReference(owner, job, m.scheme); err != nil { + if err = controllerutil.SetControllerReference(owner, job, m.scheme); err != nil { return nil, fmt.Errorf("could not set the owner reference: %v", err) } return job, nil } -func (m *maker) specTemplate(mld *api.ModuleLoaderData, containerImage string, pushImage bool) v1.PodTemplateSpec { +func (m *buildMaker) specTemplate(mld *api.ModuleLoaderData, containerImage string, pushImage bool) (v1.PodTemplateSpec, error) { buildConfig := mld.Build - kanikoImage := os.Getenv("RELATED_IMAGES_BUILD") + kanikoImage := m.builderImage if buildConfig.KanikoParams != nil && buildConfig.KanikoParams.Tag != "" { - if idx := strings.IndexAny(kanikoImage, "@:"); idx != -1 { - kanikoImage = kanikoImage[0:idx] - } + var err error - kanikoImage += ":" + buildConfig.KanikoParams.Tag + kanikoImage, err = image.SetTag(kanikoImage, buildConfig.KanikoParams.Tag) + if err != nil { + return v1.PodTemplateSpec{}, fmt.Errorf("could not generate the kaniko image name: %v", err) + } } - return v1.PodTemplateSpec{ + podSpec := v1.PodTemplateSpec{ Spec: v1.PodSpec{ Containers: []v1.Container{ { Args: m.containerArgs(buildConfig, mld, containerImage, pushImage), - Name: "kaniko", + Name: kanikoContainerName, Image: kanikoImage, VolumeMounts: volumeMounts(mld.ImageRepoSecret, buildConfig), }, @@ -137,9 +123,11 @@ func (m *maker) specTemplate(mld *api.ModuleLoaderData, containerImage string, p Volumes: volumes(mld.ImageRepoSecret, buildConfig), }, } + + return podSpec, nil } -func (m *maker) containerArgs( +func (m *buildMaker) containerArgs( buildConfig *kmmv1beta1.Build, mld *api.ModuleLoaderData, containerImage string, @@ -148,6 +136,14 @@ func (m *maker) containerArgs( args := []string{} if pushImage { args = append(args, "--destination", containerImage) + + if mld.RegistryTLS.Insecure { + args = append(args, "--insecure") + } + + if mld.RegistryTLS.InsecureSkipTLSVerify { + args = append(args, "--skip-tls-verify") + } } else { args = append(args, "--no-push") } @@ -157,7 +153,7 @@ func (m *maker) containerArgs( {Name: "MOD_NAME", Value: mld.Name}, {Name: "MOD_NAMESPACE", Value: mld.Namespace}, } - buildArgs := m.helper.ApplyBuildArgOverrides( + buildArgs := utils.ApplyBuildArgOverrides( buildConfig.BuildArgs, overrides..., ) @@ -174,20 +170,10 @@ func (m *maker) containerArgs( args = append(args, "--skip-tls-verify-pull") } - if pushImage { - if mld.RegistryTLS.Insecure { - args = append(args, "--insecure") - } - - if mld.RegistryTLS.InsecureSkipTLSVerify { - args = append(args, "--skip-tls-verify") - } - } - return args } -func (m *maker) getHashAnnotationValue(ctx context.Context, configMapName, namespace string, podTemplate *v1.PodTemplateSpec) (uint64, error) { +func (m *buildMaker) getHashAnnotationValue(ctx context.Context, configMapName, namespace string, podTemplate *v1.PodTemplateSpec) (uint64, error) { dockerfileCM := &corev1.ConfigMap{} namespacedName := types.NamespacedName{Name: configMapName, Namespace: namespace} if err := m.client.Get(ctx, namespacedName, dockerfileCM); err != nil { @@ -244,8 +230,13 @@ func dockerfileVolumeMount(name string) v1.VolumeMount { } } +type buildHashData struct { + Dockerfile string + PodTemplate *v1.PodTemplateSpec +} + func getHashValue(podTemplate *v1.PodTemplateSpec, dockerfile string) (uint64, error) { - dataToHash := hashData{ + dataToHash := buildHashData{ Dockerfile: dockerfile, PodTemplate: podTemplate, } diff --git a/internal/build/job/maker_test.go b/internal/build/jobmaker_test.go similarity index 83% rename from internal/build/job/maker_test.go rename to internal/build/jobmaker_test.go index 6cda3cd19..db0dba72d 100644 --- a/internal/build/job/maker_test.go +++ b/internal/build/jobmaker_test.go @@ -1,15 +1,14 @@ -package job +package build import ( "context" "fmt" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" - "golang.org/x/exp/slices" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -19,17 +18,15 @@ 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" ) -var _ = Describe("MakeJobTemplate", func() { +var _ = Describe("MakeJob", func() { const ( image = "my.registry/my/image" dockerfile = "FROM test" - kanikoImage = "some-kaniko-image:some-tag" + builderImage = "some-build-image:some-tag" kernelVersion = "1.2.3" moduleName = "module-name" namespace = "some-namespace" @@ -39,21 +36,15 @@ var _ = Describe("MakeJobTemplate", func() { var ( ctrl *gomock.Controller clnt *client.MockClient - m Maker - mh *build.MockHelper - jobhelper *utils.MockJobHelper + m imgbuild.JobMaker + jobhelper *imgbuild.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) - }) - - AfterEach(func() { - ctrl.Finish() + jobhelper = imgbuild.NewMockJobHelper(ctrl) + m = NewJobMaker(clnt, builderImage, jobhelper, scheme) }) mod := kmmv1beta1.Module{ @@ -71,7 +62,7 @@ var _ = Describe("MakeJobTemplate", func() { dockerfileCMData := map[string]string{constants.DockerfileCMKey: dockerfile} DescribeTable("should set fields correctly", func(buildSecrets []v1.LocalObjectReference, imagePullSecret *v1.LocalObjectReference) { - GinkgoT().Setenv(relatedImageEnvVar, kanikoImage) + GinkgoT().Setenv(relatedImageEnvVar, builderImage) ctx := context.Background() nodeSelector := map[string]string{"arch": "x64"} @@ -126,7 +117,7 @@ var _ = Describe("MakeJobTemplate", func() { "--build-arg", "MOD_NAMESPACE=" + namespace, }, Name: "kaniko", - Image: kanikoImage, + Image: builderImage, VolumeMounts: []v1.VolumeMount{ { Name: "dockerfile", @@ -217,27 +208,20 @@ var _ = Describe("MakeJobTemplate", func() { } hash, err := getHashValue(&expected.Spec.Template, dockerfile) Expect(err).NotTo(HaveOccurred()) - annotations := map[string]string{constants.JobHashAnnotation: fmt.Sprintf("%d", hash)} + annotations := map[string]string{imgbuild.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 return nil }, ), - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, utils.JobTypeBuild).Return(labels), + jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, imgbuild.JobTypeBuild).Return(labels), ) - actual, err := m.MakeJobTemplate(ctx, &mld, mld.Owner, true) + actual, err := m.MakeJob(ctx, &mld, mld.Owner, true) Expect(err).NotTo(HaveOccurred()) Expect( @@ -282,17 +266,16 @@ 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 return nil }, ), - jobhelper.EXPECT().JobLabels(mod.Name, kernelVersion, utils.JobTypeBuild).Return(map[string]string{}), + jobhelper.EXPECT().JobLabels(mod.Name, kernelVersion, imgbuild.JobTypeBuild).Return(map[string]string{}), ) - actual, err := m.MakeJobTemplate(ctx, &mld, mld.Owner, pushImage) + actual, err := m.MakeJob(ctx, &mld, mld.Owner, pushImage) Expect(err).NotTo(HaveOccurred()) Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement(kanikoFlag)) @@ -359,19 +342,17 @@ 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 return nil }, ), - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, utils.JobTypeBuild).Return(map[string]string{}), + jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, imgbuild.JobTypeBuild).Return(map[string]string{}), ) - actual, err := m.MakeJobTemplate(ctx, &mld, mld.Owner, false) + actual, err := m.MakeJob(ctx, &mld, mld.Owner, false) Expect(err).NotTo(HaveOccurred()) Expect(actual.Spec.Template.Spec.Containers[0].Image).To(Equal("some-build-image:" + customTag)) @@ -380,7 +361,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, @@ -398,17 +378,16 @@ 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 return nil }, ), - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, utils.JobTypeBuild).Return(map[string]string{}), + jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, imgbuild.JobTypeBuild).Return(map[string]string{}), ) - actual, err := m.MakeJobTemplate(ctx, &mld, mld.Owner, true) + actual, err := m.MakeJob(ctx, &mld, mld.Owner, true) Expect(err).NotTo(HaveOccurred()) Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement("--destination")) diff --git a/internal/build/manager.go b/internal/build/manager.go index 87402c90a..4b34458ca 100644 --- a/internal/build/manager.go +++ b/internal/build/manager.go @@ -1,24 +1,5 @@ package build -import ( - "context" +import "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" -) - -//go:generate mockgen -source=manager.go -package=build -destination=mock_manager.go - -type Manager interface { - GarbageCollect(ctx context.Context, modName, namespace string, owner metav1.Object) ([]string, error) - - ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) - - Sync( - ctx context.Context, - mld *api.ModuleLoaderData, - pushImage bool, - owner metav1.Object) (utils.Status, error) -} +type Manager = imgbuild.JobManager diff --git a/internal/build/mock_helper.go b/internal/build/mock_helper.go deleted file mode 100644 index ae844c7ac..000000000 --- a/internal/build/mock_helper.go +++ /dev/null @@ -1,68 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: helper.go - -// Package build is a generated GoMock package. -package build - -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 -} - -// 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() - ret := m.ctrl.Call(m, "GetRelevantBuild", moduleBuild, mappingBuild) - ret0, _ := ret[0].(*v1beta1.Build) - return ret0 -} - -// GetRelevantBuild indicates an expected call of GetRelevantBuild. -func (mr *MockHelperMockRecorder) GetRelevantBuild(moduleBuild, mappingBuild interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRelevantBuild", reflect.TypeOf((*MockHelper)(nil).GetRelevantBuild), moduleBuild, mappingBuild) -} diff --git a/internal/build/mock_manager.go b/internal/build/mock_manager.go deleted file mode 100644 index c41971773..000000000 --- a/internal/build/mock_manager.go +++ /dev/null @@ -1,83 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: manager.go - -// Package build is a generated GoMock package. -package build - -import ( - context "context" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - api "github.com/kubernetes-sigs/kernel-module-management/internal/api" - utils "github.com/kubernetes-sigs/kernel-module-management/internal/utils" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// MockManager is a mock of Manager interface. -type MockManager struct { - ctrl *gomock.Controller - recorder *MockManagerMockRecorder -} - -// MockManagerMockRecorder is the mock recorder for MockManager. -type MockManagerMockRecorder struct { - mock *MockManager -} - -// NewMockManager creates a new mock instance. -func NewMockManager(ctrl *gomock.Controller) *MockManager { - mock := &MockManager{ctrl: ctrl} - mock.recorder = &MockManagerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockManager) EXPECT() *MockManagerMockRecorder { - return m.recorder -} - -// GarbageCollect mocks base method. -func (m *MockManager) GarbageCollect(ctx context.Context, modName, namespace string, owner v1.Object) ([]string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GarbageCollect", ctx, modName, namespace, owner) - ret0, _ := ret[0].([]string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GarbageCollect indicates an expected call of GarbageCollect. -func (mr *MockManagerMockRecorder) GarbageCollect(ctx, modName, namespace, owner interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GarbageCollect", reflect.TypeOf((*MockManager)(nil).GarbageCollect), ctx, modName, namespace, owner) -} - -// ShouldSync mocks base method. -func (m *MockManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ShouldSync", ctx, mld) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ShouldSync indicates an expected call of ShouldSync. -func (mr *MockManagerMockRecorder) ShouldSync(ctx, mld interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldSync", reflect.TypeOf((*MockManager)(nil).ShouldSync), ctx, mld) -} - -// Sync mocks base method. -func (m *MockManager) Sync(ctx context.Context, mld *api.ModuleLoaderData, pushImage bool, owner v1.Object) (utils.Status, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Sync", ctx, mld, pushImage, owner) - ret0, _ := ret[0].(utils.Status) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Sync indicates an expected call of Sync. -func (mr *MockManagerMockRecorder) Sync(ctx, mld, pushImage, owner interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sync", reflect.TypeOf((*MockManager)(nil).Sync), ctx, mld, pushImage, owner) -} 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..b7a87a06e 100644 --- a/internal/cluster/cluster.go +++ b/internal/cluster/cluster.go @@ -7,6 +7,7 @@ import ( "strings" hubv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -18,9 +19,7 @@ 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" ) //go:generate mockgen -source=cluster.go -package=cluster -destination=mock_cluster.go @@ -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 + kernelAPI api.ModuleLoaderDataFactory + buildManager build.Manager + signManager sign.Manager + namespace string } func NewClusterAPI( client client.Client, - kernelAPI module.KernelMapper, + kernelAPI api.ModuleLoaderDataFactory, buildAPI build.Manager, - signAPI sign.SignManager, + signAPI sign.Manager, defaultJobNamespace string) ClusterAPI { return &clusterAPI{ - client: client, - kernelAPI: kernelAPI, - buildAPI: buildAPI, - signAPI: signAPI, - namespace: defaultJobNamespace, + client: client, + kernelAPI: kernelAPI, + buildManager: buildAPI, + signManager: signAPI, + namespace: defaultJobNamespace, } } @@ -141,7 +140,7 @@ func (c *clusterAPI) BuildAndSign( } func (c *clusterAPI) GarbageCollectBuilds(ctx context.Context, mcm hubv1beta1.ManagedClusterModule) ([]string, error) { - return c.buildAPI.GarbageCollect(ctx, mcm.Name, c.namespace, &mcm) + return c.buildManager.GarbageCollect(ctx, mcm.Name, c.namespace, &mcm) } func (c *clusterAPI) kernelMappingsByKernelVersion( @@ -169,7 +168,7 @@ func (c *clusterAPI) kernelMappingsByKernelVersion( continue } - mld, err := c.kernelAPI.GetModuleLoaderDataForKernel(mod, kernelVersion) + mld, err := c.kernelAPI.FromModule(mod, kernelVersion) if err != nil { kernelVersionLogger.Info("no suitable container image found; skipping kernel version") continue @@ -201,7 +200,7 @@ func (c *clusterAPI) build( mld *api.ModuleLoaderData, mcm *hubv1beta1.ManagedClusterModule) (bool, error) { - shouldSync, err := c.buildAPI.ShouldSync(ctx, mld) + shouldSync, err := c.buildManager.ShouldSync(ctx, mld) if err != nil { return false, fmt.Errorf("could not check if build synchronization is needed: %v", err) } @@ -214,12 +213,12 @@ func (c *clusterAPI) build( "image", mld.ContainerImage) buildCtx := log.IntoContext(ctx, logger) - buildStatus, err := c.buildAPI.Sync(buildCtx, mld, true, mcm) + buildStatus, err := c.buildManager.Sync(buildCtx, mld, true, mcm) if err != nil { return false, fmt.Errorf("could not synchronize the build: %w", err) } - if buildStatus == utils.StatusCompleted { + if buildStatus == imgbuild.StatusCompleted { return true, nil } return false, nil @@ -230,7 +229,7 @@ func (c *clusterAPI) sign( mld *api.ModuleLoaderData, mcm *hubv1beta1.ManagedClusterModule) (bool, error) { - shouldSync, err := c.signAPI.ShouldSync(ctx, mld) + shouldSync, err := c.signManager.ShouldSync(ctx, mld) if err != nil { return false, fmt.Errorf("could not check if signing synchronization is needed: %v", err) } @@ -238,24 +237,17 @@ 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) - } - logger := log.FromContext(ctx).WithValues( "kernel version", mld.KernelVersion, "image", mld.ContainerImage) signCtx := log.IntoContext(ctx, logger) - signStatus, err := c.signAPI.Sync(signCtx, mld, previousImage, true, mcm) + signStatus, err := c.signManager.Sync(signCtx, mld, true, mcm) if err != nil { return false, fmt.Errorf("could not synchronize the signing: %w", err) } - if signStatus == utils.StatusCompleted { + if signStatus == imgbuild.StatusCompleted { return true, nil } return false, nil diff --git a/internal/cluster/cluster_test.go b/internal/cluster/cluster_test.go index f4d775d3f..88e0c919d 100644 --- a/internal/cluster/cluster_test.go +++ b/internal/cluster/cluster_test.go @@ -5,6 +5,7 @@ import ( "errors" "github.com/golang/mock/gomock" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -15,29 +16,25 @@ import ( hubv1beta1 "github.com/kubernetes-sigs/kernel-module-management/api-hub/v1beta1" 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/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 + mockKM *api.MockModuleLoaderDataFactory + mockBM *imgbuild.MockJobManager + mockSM *imgbuild.MockJobManager ) BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mockKM = module.NewMockKernelMapper(ctrl) - mockBM = build.NewMockManager(ctrl) - mockSM = sign.NewMockSignManager(ctrl) + mockKM = api.NewMockModuleLoaderDataFactory(ctrl) + mockBM = imgbuild.NewMockJobManager(ctrl) + mockSM = imgbuild.NewMockJobManager(ctrl) }) const ( @@ -222,7 +219,7 @@ var _ = Describe("ClusterAPI", func() { It("should do nothing when no kernel mappings are found", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(nil, errors.New("generic-error")), + mockKM.EXPECT().FromModule(&mod, kernelVersion).Return(nil, errors.New("generic-error")), ) c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) @@ -253,7 +250,7 @@ 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), + mockKM.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), ) @@ -267,9 +264,9 @@ var _ = Describe("ClusterAPI", func() { It("should run build sync if needed", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockKM.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), + mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.StatusCompleted, nil), mockSM.EXPECT().ShouldSync(gomock.Any(), &mld).Return(false, nil), ) @@ -282,9 +279,9 @@ var _ = Describe("ClusterAPI", func() { It("should return an error when build sync errors", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockKM.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")), + mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.Status(""), errors.New("test-error")), ) c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) @@ -296,10 +293,10 @@ var _ = Describe("ClusterAPI", func() { It("should run sign sync if needed", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockKM.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), + mockSM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.StatusInProgress, nil), ) c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) @@ -311,10 +308,10 @@ var _ = Describe("ClusterAPI", func() { It("should return an error when sign sync errors", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockKM.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")), + mockSM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.Status(""), errors.New("test-error")), ) c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) @@ -326,9 +323,9 @@ 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), + mockKM.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), + mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.StatusInProgress, nil), ) c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) @@ -340,11 +337,11 @@ var _ = Describe("ClusterAPI", func() { It("should run both build sync and sign sync when build is completed", func() { gomock.InOrder( - mockKM.EXPECT().GetModuleLoaderDataForKernel(&mod, kernelVersion).Return(&mld, nil), + mockKM.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), + mockBM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.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), + mockSM.EXPECT().Sync(gomock.Any(), &mld, true, mcm).Return(imgbuild.StatusCompleted, nil), ) c := NewClusterAPI(clnt, mockKM, mockBM, mockSM, namespace) diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 89ea14239..8e762a3b8 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -6,7 +6,6 @@ const ( TargetKernelTarget = "kmm.node.kubernetes.io/target-kernel" DaemonSetRole = "kmm.node.kubernetes.io/role" JobType = "kmm.node.kubernetes.io/job-type" - JobHashAnnotation = "kmm.node.kubernetes.io/last-hash" KernelLabel = "kmm.node.kubernetes.io/kernel-version.full" ModuleLoaderVersionLabelPrefix = "beta.kmm.node.kubernetes.io/version-module-loader" @@ -19,5 +18,7 @@ const ( PublicSignDataKey = "cert" PrivateSignDataKey = "key" + BuildImageEnvVar = "RELATED_IMAGES_BUILD" OperatorNamespaceEnvVar = "OPERATOR_NAMESPACE" + SignImageEnvVar = "RELATED_IMAGES_SIGN" ) diff --git a/internal/utils/jobhelper.go b/internal/imgbuild/jobhelper.go similarity index 77% rename from internal/utils/jobhelper.go rename to internal/imgbuild/jobhelper.go index 5fcc45c9c..89c038127 100644 --- a/internal/utils/jobhelper.go +++ b/internal/imgbuild/jobhelper.go @@ -1,4 +1,4 @@ -package utils +package imgbuild import ( "context" @@ -12,27 +12,17 @@ import ( "github.com/kubernetes-sigs/kernel-module-management/internal/constants" ) -type Status string - -const ( - JobTypeBuild = "build" - JobTypeSign = "sign" - - StatusCompleted = "completed" - StatusCreated = "created" - StatusInProgress = "in progress" - StatusFailed = "failed" -) +const JobHashAnnotation = "kmm.node.kubernetes.io/last-hash" var ErrNoMatchingJob = errors.New("no matching job") -//go:generate mockgen -source=jobhelper.go -package=utils -destination=mock_jobhelper.go +//go:generate mockgen -source=jobhelper.go -package=imgbuild -destination=mock_jobhelper.go type JobHelper interface { IsJobChanged(existingJob *batchv1.Job, newJob *batchv1.Job) (bool, error) - JobLabels(modName string, targetKernel string, jobType string) map[string]string - GetModuleJobByKernel(ctx context.Context, modName, namespace, targetKernel, jobType string, owner metav1.Object) (*batchv1.Job, error) - GetModuleJobs(ctx context.Context, modName, namespace, jobType string, owner metav1.Object) ([]batchv1.Job, error) + JobLabels(modName string, targetKernel string, jobType JobType) map[string]string + GetModuleJobByKernel(ctx context.Context, modName, namespace, targetKernel string, jobType JobType, owner metav1.Object) (*batchv1.Job, error) + GetModuleJobs(ctx context.Context, modName, namespace string, jobType JobType, owner metav1.Object) ([]batchv1.Job, error) DeleteJob(ctx context.Context, job *batchv1.Job) error CreateJob(ctx context.Context, jobTemplate *batchv1.Job) error GetJobStatus(job *batchv1.Job) (Status, error) @@ -51,20 +41,19 @@ func NewJobHelper(client client.Client) JobHelper { func (jh *jobHelper) IsJobChanged(existingJob *batchv1.Job, newJob *batchv1.Job) (bool, error) { existingAnnotations := existingJob.GetAnnotations() newAnnotations := newJob.GetAnnotations() + if existingAnnotations == nil { return false, fmt.Errorf("annotations are not present in the existing job %s", existingJob.Name) } - if existingAnnotations[constants.JobHashAnnotation] == newAnnotations[constants.JobHashAnnotation] { - return false, nil - } - return true, nil + + return existingAnnotations[JobHashAnnotation] != newAnnotations[JobHashAnnotation], nil } -func (jh *jobHelper) JobLabels(modName string, targetKernel string, jobType string) map[string]string { +func (jh *jobHelper) JobLabels(modName string, targetKernel string, jobType JobType) map[string]string { return moduleKernelLabels(modName, targetKernel, jobType) } -func (jh *jobHelper) GetModuleJobByKernel(ctx context.Context, modName, namespace, targetKernel, jobType string, owner metav1.Object) (*batchv1.Job, error) { +func (jh *jobHelper) GetModuleJobByKernel(ctx context.Context, modName, namespace, targetKernel string, jobType JobType, owner metav1.Object) (*batchv1.Job, error) { matchLabels := moduleKernelLabels(modName, targetKernel, jobType) jobs, err := jh.getJobs(ctx, namespace, matchLabels) if err != nil { @@ -82,7 +71,7 @@ func (jh *jobHelper) GetModuleJobByKernel(ctx context.Context, modName, namespac return &moduleOwnedJobs[0], nil } -func (jh *jobHelper) GetModuleJobs(ctx context.Context, modName, namespace, jobType string, owner metav1.Object) ([]batchv1.Job, error) { +func (jh *jobHelper) GetModuleJobs(ctx context.Context, modName, namespace string, jobType JobType, owner metav1.Object) ([]batchv1.Job, error) { matchLabels := moduleLabels(modName, jobType) jobs, err := jh.getJobs(ctx, namespace, matchLabels) if err != nil { @@ -139,25 +128,27 @@ func (jh *jobHelper) getJobs(ctx context.Context, namespace string, labels map[s return jobList.Items, nil } -func moduleKernelLabels(moduleName, targetKernel, jobType string) map[string]string { +func moduleKernelLabels(moduleName, targetKernel string, jobType JobType) map[string]string { labels := moduleLabels(moduleName, jobType) labels[constants.TargetKernelTarget] = targetKernel return labels } -func moduleLabels(moduleName, jobType string) map[string]string { +func moduleLabels(moduleName string, jobType JobType) map[string]string { return map[string]string{ constants.ModuleNameLabel: moduleName, - constants.JobType: jobType, + constants.JobType: string(jobType), } } func filterJobsByOwner(jobs []batchv1.Job, owner metav1.Object) []batchv1.Job { - ownedJobs := []batchv1.Job{} + ownedJobs := make([]batchv1.Job, 0, len(jobs)) + for _, job := range jobs { if metav1.IsControlledBy(&job, owner) { ownedJobs = append(ownedJobs, job) } } + return ownedJobs } diff --git a/internal/utils/jobhelper_test.go b/internal/imgbuild/jobhelper_test.go similarity index 96% rename from internal/utils/jobhelper_test.go rename to internal/imgbuild/jobhelper_test.go index d22395a43..f7f403020 100644 --- a/internal/utils/jobhelper_test.go +++ b/internal/imgbuild/jobhelper_test.go @@ -1,4 +1,4 @@ -package utils +package imgbuild import ( "context" @@ -413,7 +413,7 @@ var _ = Describe("JobStatus", func() { }) DescribeTable("should return the correct status depending on the job status", - func(s *batchv1.Job, jobStatus string, expectsErr bool) { + func(s *batchv1.Job, jobStatus Status, expectsErr bool) { res, err := jh.GetJobStatus(s) if expectsErr { @@ -421,16 +421,16 @@ var _ = Describe("JobStatus", func() { return } - Expect(string(res)).To(Equal(jobStatus)) + Expect(res).To(Equal(jobStatus)) }, Entry("succeeded", &batchv1.Job{Status: batchv1.JobStatus{Succeeded: 1}}, StatusCompleted, false), Entry("in progress", &batchv1.Job{Status: batchv1.JobStatus{Active: 1}}, StatusInProgress, false), Entry("Failed", &batchv1.Job{Status: batchv1.JobStatus{Failed: 1}}, StatusFailed, false), - Entry("unknown", &batchv1.Job{Status: batchv1.JobStatus{Failed: 2}}, "", true), + Entry("unknown", &batchv1.Job{Status: batchv1.JobStatus{Failed: 2}}, Status(""), true), ) }) -var _ = Describe("IsJobChnaged", func() { +var _ = Describe("IsJobChanged", func() { var ( ctrl *gomock.Controller clnt *client.MockClient @@ -452,10 +452,9 @@ var _ = Describe("IsJobChnaged", func() { } newJob := batchv1.Job{ ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{constants.JobHashAnnotation: "some hash"}, + Annotations: map[string]string{JobHashAnnotation: "some hash"}, }, } - fmt.Println(existingJob.GetAnnotations()) changed, err := jh.IsJobChanged(&existingJob, &newJob) @@ -467,7 +466,7 @@ var _ = Describe("IsJobChnaged", func() { }, Entry("should error if job has no annotations", nil, false, true), - Entry("should return true if job has changed", map[string]string{constants.JobHashAnnotation: "some other hash"}, true, false), - Entry("should return false is job has not changed ", map[string]string{constants.JobHashAnnotation: "some hash"}, false, false), + Entry("should return true if job has changed", map[string]string{JobHashAnnotation: "some other hash"}, true, false), + Entry("should return false is job has not changed ", map[string]string{JobHashAnnotation: "some hash"}, false, false), ) }) diff --git a/internal/imgbuild/jobmanager.go b/internal/imgbuild/jobmanager.go new file mode 100644 index 000000000..ede70e193 --- /dev/null +++ b/internal/imgbuild/jobmanager.go @@ -0,0 +1,165 @@ +package imgbuild + +import ( + "context" + "errors" + "fmt" + + "github.com/kubernetes-sigs/kernel-module-management/internal/api" + "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/utils" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +type JobType string + +const ( + JobTypeBuild JobType = "build" + JobTypeSign JobType = "sign" +) + +type BaseJobManager struct { + client client.Client + jobHelper JobHelper + jobType JobType + maker JobMaker + registry registry.Registry +} + +func (bjm *BaseJobManager) GarbageCollect(ctx context.Context, modName, namespace string, owner metav1.Object) ([]string, error) { + jobs, err := bjm.jobHelper.GetModuleJobs(ctx, modName, namespace, bjm.jobType, owner) + if err != nil { + return nil, fmt.Errorf("failed to get build jobs for module %s: %v", modName, err) + } + + deleteNames := make([]string, 0, len(jobs)) + for _, job := range jobs { + if job.Status.Succeeded == 1 { + err = bjm.jobHelper.DeleteJob(ctx, &job) + if err != nil { + return nil, fmt.Errorf("failed to delete build job %s: %v", job.Name, err) + } + deleteNames = append(deleteNames, job.Name) + } + } + return deleteNames, nil +} + +func (bjm *BaseJobManager) Sync(ctx context.Context, mld *api.ModuleLoaderData, pushImage bool, owner metav1.Object) (Status, error) { + logger := log.FromContext(ctx).WithValues("type", bjm.jobType) + + logger.Info("Syncing job") + + jobTemplate, err := bjm.maker.MakeJob(ctx, mld, owner, pushImage) + if err != nil { + return "", fmt.Errorf("could not make Job template: %v", err) + } + + job, err := bjm.jobHelper.GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, mld.KernelVersion, bjm.jobType, owner) + if err != nil { + if !errors.Is(err, ErrNoMatchingJob) { + return "", fmt.Errorf("error getting the build: %v", err) + } + + logger.Info("Creating job") + err = bjm.jobHelper.CreateJob(ctx, jobTemplate) + if err != nil { + return "", fmt.Errorf("could not create Job: %v", err) + } + + return StatusCreated, nil + } + + changed, err := bjm.jobHelper.IsJobChanged(job, jobTemplate) + if err != nil { + return "", fmt.Errorf("could not determine if job has changed: %v", err) + } + + if changed { + logger.Info("The Job spec has changed, deleting the current job so a new one can be created", "name", job.Name) + err = bjm.jobHelper.DeleteJob(ctx, job) + if err != nil { + logger.Info(utils.WarnString(fmt.Sprintf("failed to delete build job %s: %v", job.Name, err))) + } + return StatusInProgress, nil + } + + logger.Info("Returning job status", "name", job.Name, "namespace", job.Namespace) + + statusmsg, err := bjm.jobHelper.GetJobStatus(job) + if err != nil { + return "", err + } + + return statusmsg, nil +} + +type buildJobManager struct { + *BaseJobManager +} + +func NewBuildManager(client client.Client, helper JobHelper, maker JobMaker, registry registry.Registry) JobManager { + return &buildJobManager{ + BaseJobManager: &BaseJobManager{ + client: client, + jobHelper: helper, + jobType: JobTypeBuild, + maker: maker, + registry: registry, + }, + } +} + +func (bjm *buildJobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { + // if there is no build specified skip + if !mld.BuildConfigured() { + return false, nil + } + + destinationImage, err := mld.BuildDestinationImage() + if err != nil { + return false, fmt.Errorf("could not determine the build destination image: %v", err) + } + + // build is specified and targetImage is either the final image or the intermediate image + // tag, depending on whether sign is specified or not. Either way, if targetImage exists + // we can skip building it + exists, err := module.ImageExists(ctx, bjm.client, bjm.registry, mld, mld.Namespace, destinationImage) + if err != nil { + return false, fmt.Errorf("failed to check existence of image %s: %w", destinationImage, err) + } + + return !exists, nil +} + +type signJobManager struct { + *BaseJobManager +} + +func NewSignManager(client client.Client, helper JobHelper, maker JobMaker, registry registry.Registry) JobManager { + return &signJobManager{ + BaseJobManager: &BaseJobManager{ + client: client, + jobHelper: helper, + jobType: JobTypeSign, + maker: maker, + registry: registry, + }, + } +} + +func (sjm *signJobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { + if !mld.SignConfigured() { + return false, nil + } + + exists, err := module.ImageExists(ctx, sjm.client, sjm.registry, mld, mld.Namespace, mld.ContainerImage) + if err != nil { + return false, fmt.Errorf("failed to check existence of image %s: %w", mld.ContainerImage, err) + } + + return !exists, nil +} diff --git a/internal/imgbuild/mock_interfaces.go b/internal/imgbuild/mock_interfaces.go new file mode 100644 index 000000000..099a5e600 --- /dev/null +++ b/internal/imgbuild/mock_interfaces.go @@ -0,0 +1,121 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: types.go + +// Package imgbuild is a generated GoMock package. +package imgbuild + +import ( + context "context" + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + api "github.com/kubernetes-sigs/kernel-module-management/internal/api" + v1 "k8s.io/api/batch/v1" + v10 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// MockJobMaker is a mock of JobMaker interface. +type MockJobMaker struct { + ctrl *gomock.Controller + recorder *MockJobMakerMockRecorder +} + +// MockJobMakerMockRecorder is the mock recorder for MockJobMaker. +type MockJobMakerMockRecorder struct { + mock *MockJobMaker +} + +// NewMockJobMaker creates a new mock instance. +func NewMockJobMaker(ctrl *gomock.Controller) *MockJobMaker { + mock := &MockJobMaker{ctrl: ctrl} + mock.recorder = &MockJobMakerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockJobMaker) EXPECT() *MockJobMakerMockRecorder { + return m.recorder +} + +// MakeJob mocks base method. +func (m *MockJobMaker) MakeJob(ctx context.Context, mld *api.ModuleLoaderData, owner v10.Object, pushImage bool) (*v1.Job, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "MakeJob", ctx, mld, owner, pushImage) + ret0, _ := ret[0].(*v1.Job) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// MakeJob indicates an expected call of MakeJob. +func (mr *MockJobMakerMockRecorder) MakeJob(ctx, mld, owner, pushImage interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MakeJob", reflect.TypeOf((*MockJobMaker)(nil).MakeJob), ctx, mld, owner, pushImage) +} + +// MockJobManager is a mock of JobManager interface. +type MockJobManager struct { + ctrl *gomock.Controller + recorder *MockJobManagerMockRecorder +} + +// MockJobManagerMockRecorder is the mock recorder for MockJobManager. +type MockJobManagerMockRecorder struct { + mock *MockJobManager +} + +// NewMockJobManager creates a new mock instance. +func NewMockJobManager(ctrl *gomock.Controller) *MockJobManager { + mock := &MockJobManager{ctrl: ctrl} + mock.recorder = &MockJobManagerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockJobManager) EXPECT() *MockJobManagerMockRecorder { + return m.recorder +} + +// GarbageCollect mocks base method. +func (m *MockJobManager) GarbageCollect(ctx context.Context, modName, namespace string, owner v10.Object) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GarbageCollect", ctx, modName, namespace, owner) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GarbageCollect indicates an expected call of GarbageCollect. +func (mr *MockJobManagerMockRecorder) GarbageCollect(ctx, modName, namespace, owner interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GarbageCollect", reflect.TypeOf((*MockJobManager)(nil).GarbageCollect), ctx, modName, namespace, owner) +} + +// ShouldSync mocks base method. +func (m *MockJobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ShouldSync", ctx, mld) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ShouldSync indicates an expected call of ShouldSync. +func (mr *MockJobManagerMockRecorder) ShouldSync(ctx, mld interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldSync", reflect.TypeOf((*MockJobManager)(nil).ShouldSync), ctx, mld) +} + +// Sync mocks base method. +func (m *MockJobManager) Sync(ctx context.Context, mld *api.ModuleLoaderData, pushImage bool, owner v10.Object) (Status, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Sync", ctx, mld, pushImage, owner) + ret0, _ := ret[0].(Status) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Sync indicates an expected call of Sync. +func (mr *MockJobManagerMockRecorder) Sync(ctx, mld, pushImage, owner interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sync", reflect.TypeOf((*MockJobManager)(nil).Sync), ctx, mld, pushImage, owner) +} diff --git a/internal/utils/mock_jobhelper.go b/internal/imgbuild/mock_jobhelper.go similarity index 93% rename from internal/utils/mock_jobhelper.go rename to internal/imgbuild/mock_jobhelper.go index b14b707c2..2597a981e 100644 --- a/internal/utils/mock_jobhelper.go +++ b/internal/imgbuild/mock_jobhelper.go @@ -1,8 +1,8 @@ // Code generated by MockGen. DO NOT EDIT. // Source: jobhelper.go -// Package utils is a generated GoMock package. -package utils +// Package imgbuild is a generated GoMock package. +package imgbuild import ( context "context" @@ -80,7 +80,7 @@ func (mr *MockJobHelperMockRecorder) GetJobStatus(job interface{}) *gomock.Call } // GetModuleJobByKernel mocks base method. -func (m *MockJobHelper) GetModuleJobByKernel(ctx context.Context, modName, namespace, targetKernel, jobType string, owner v10.Object) (*v1.Job, error) { +func (m *MockJobHelper) GetModuleJobByKernel(ctx context.Context, modName, namespace, targetKernel string, jobType JobType, owner v10.Object) (*v1.Job, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetModuleJobByKernel", ctx, modName, namespace, targetKernel, jobType, owner) ret0, _ := ret[0].(*v1.Job) @@ -95,7 +95,7 @@ func (mr *MockJobHelperMockRecorder) GetModuleJobByKernel(ctx, modName, namespac } // GetModuleJobs mocks base method. -func (m *MockJobHelper) GetModuleJobs(ctx context.Context, modName, namespace, jobType string, owner v10.Object) ([]v1.Job, error) { +func (m *MockJobHelper) GetModuleJobs(ctx context.Context, modName, namespace string, jobType JobType, owner v10.Object) ([]v1.Job, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "GetModuleJobs", ctx, modName, namespace, jobType, owner) ret0, _ := ret[0].([]v1.Job) @@ -125,7 +125,7 @@ func (mr *MockJobHelperMockRecorder) IsJobChanged(existingJob, newJob interface{ } // JobLabels mocks base method. -func (m *MockJobHelper) JobLabels(modName, targetKernel, jobType string) map[string]string { +func (m *MockJobHelper) JobLabels(modName, targetKernel string, jobType JobType) map[string]string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "JobLabels", modName, targetKernel, jobType) ret0, _ := ret[0].(map[string]string) diff --git a/internal/imgbuild/suite_test.go b/internal/imgbuild/suite_test.go new file mode 100644 index 000000000..9960982e6 --- /dev/null +++ b/internal/imgbuild/suite_test.go @@ -0,0 +1,23 @@ +package imgbuild + +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, "ImgBuild Suite") +} diff --git a/internal/imgbuild/types.go b/internal/imgbuild/types.go new file mode 100644 index 000000000..7a27175c9 --- /dev/null +++ b/internal/imgbuild/types.go @@ -0,0 +1,30 @@ +package imgbuild + +import ( + "context" + + "github.com/kubernetes-sigs/kernel-module-management/internal/api" + batchv1 "k8s.io/api/batch/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type Status string + +const ( + StatusCompleted Status = "completed" + StatusCreated Status = "created" + StatusInProgress Status = "in progress" + StatusFailed Status = "failed" +) + +//go:generate mockgen -source=types.go -package=imgbuild -destination=mock_interfaces.go + +type JobMaker interface { + MakeJob(ctx context.Context, mld *api.ModuleLoaderData, owner metav1.Object, pushImage bool) (*batchv1.Job, error) +} + +type JobManager interface { + GarbageCollect(ctx context.Context, modName, namespace string, owner metav1.Object) ([]string, error) + ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) + Sync(ctx context.Context, mld *api.ModuleLoaderData, pushImage bool, owner metav1.Object) (Status, error) +} 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 58d4676af..000000000 --- a/internal/module/kernelmapper.go +++ /dev/null @@ -1,140 +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.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.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 ea966cb17..000000000 --- a/internal/module/kernelmapper_test.go +++ /dev/null @@ -1,285 +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" - mapping = kmmv1beta1.KernelMapping{} - }) - - AfterEach(func() { - ctrl.Finish() - }) - - 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 := 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, - 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) - } - - 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), - 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("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 index 3ebdc0e69..6d32ce033 100644 --- a/internal/module/mock_kernelmapper.go +++ b/internal/module/mock_kernelmapper.go @@ -3,116 +3,3 @@ // 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/preflight/preflight.go b/internal/preflight/preflight.go index e2d99eff1..d8bea9e10 100644 --- a/internal/preflight/preflight.go +++ b/internal/preflight/preflight.go @@ -8,7 +8,7 @@ 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/imgbuild" "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" @@ -34,10 +34,10 @@ type PreflightAPI interface { func NewPreflightAPI( client client.Client, buildAPI build.Manager, - signAPI sign.SignManager, + signAPI sign.Manager, registryAPI registry.Registry, statusUpdater statusupdater.PreflightStatusUpdater, - kernelAPI module.KernelMapper) PreflightAPI { + kernelAPI api.ModuleLoaderDataFactory) PreflightAPI { helper := newPreflightHelper(client, buildAPI, signAPI, registryAPI) return &preflight{ kernelAPI: kernelAPI, @@ -47,7 +47,7 @@ func NewPreflightAPI( } type preflight struct { - kernelAPI module.KernelMapper + kernelAPI api.ModuleLoaderDataFactory statusUpdater statusupdater.PreflightStatusUpdater helper preflightHelperAPI } @@ -55,7 +55,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.kernelAPI.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 +70,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 +82,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) @@ -108,10 +105,10 @@ type preflightHelper struct { client client.Client registryAPI registry.Registry buildAPI build.Manager - signAPI sign.SignManager + signAPI sign.Manager } -func newPreflightHelper(client client.Client, buildAPI build.Manager, signAPI sign.SignManager, registryAPI registry.Registry) preflightHelperAPI { +func newPreflightHelper(client client.Client, buildAPI build.Manager, signAPI sign.Manager, registryAPI registry.Registry) preflightHelperAPI { return &preflightHelper{ client: client, buildAPI: buildAPI, @@ -160,7 +157,7 @@ func (p *preflightHelper) verifyBuild(ctx context.Context, pv *kmmv1beta1.Prefli return false, fmt.Sprintf("Failed to verify build for module %s, kernel version %s, error %s", mld.Name, pv.Spec.KernelVersion, err) } - if buildStatus == utils.StatusCompleted { + if buildStatus == imgbuild.StatusCompleted { msg := "build compiles" if pv.Spec.PushBuiltImage { msg += " and image pushed" @@ -174,18 +171,13 @@ 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) - } - // at this stage we know that eiher mapping Sign or Container sign are defined - signStatus, err := p.signAPI.Sync(ctx, mld, previousImage, pv.Spec.PushBuiltImage, pv) + signStatus, err := p.signAPI.Sync(ctx, mld, pv.Spec.PushBuiltImage, pv) if err != nil { return false, fmt.Sprintf("Failed to verify signing for module %s, kernel version %s, error %s", mld.Name, pv.Spec.KernelVersion, err) } - if signStatus == utils.StatusCompleted { + if signStatus == imgbuild.StatusCompleted { msg := "sign completes" if pv.Spec.PushBuiltImage { msg += " and image pushed" diff --git a/internal/preflight/preflight_test.go b/internal/preflight/preflight_test.go index bc8c7b0cd..17fff5ff3 100644 --- a/internal/preflight/preflight_test.go +++ b/internal/preflight/preflight_test.go @@ -9,13 +9,10 @@ import ( v1stream "github.com/google/go-containerregistry/pkg/v1/stream" 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/module" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" "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" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -75,7 +72,7 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { ) var ( ctrl *gomock.Controller - mockKernelAPI *module.MockKernelMapper + mockKernelAPI *api.MockModuleLoaderDataFactory mockStatusUpdater *statusupdater.MockPreflightStatusUpdater preflightHelper *MockpreflightHelperAPI p *preflight @@ -83,7 +80,7 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) - mockKernelAPI = module.NewMockKernelMapper(ctrl) + mockKernelAPI = api.NewMockModuleLoaderDataFactory(ctrl) mockStatusUpdater = statusupdater.NewMockPreflightStatusUpdater(ctrl) preflightHelper = NewMockpreflightHelperAPI(ctrl) p = &preflight{ @@ -100,7 +97,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")) + mockKernelAPI.EXPECT().FromModule(mod, kernelVersion).Return(nil, fmt.Errorf("some error")) res, message := p.PreflightUpgradeCheck(context.Background(), pv, mod) @@ -125,7 +122,7 @@ var _ = Describe("preflight_PreflightUpgradeCheck", func() { mld.Sign = &kmmv1beta1.Sign{} } - mockKernelAPI.EXPECT().GetModuleLoaderDataForKernel(mod, kernelVersion).Return(&mld, nil) + mockKernelAPI.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 { @@ -301,7 +298,7 @@ var _ = Describe("preflightHelper_verifyImage", func() { var _ = Describe("preflightHelper_verifyBuild", func() { var ( ctrl *gomock.Controller - mockBuildAPI *build.MockManager + mockBuildAPI *imgbuild.MockJobManager clnt *client.MockClient ph *preflightHelper ) @@ -309,7 +306,7 @@ var _ = Describe("preflightHelper_verifyBuild", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mockBuildAPI = build.NewMockManager(ctrl) + mockBuildAPI = imgbuild.NewMockJobManager(ctrl) ph = &preflightHelper{ client: clnt, buildAPI: mockBuildAPI, @@ -330,7 +327,7 @@ var _ = Describe("preflightHelper_verifyBuild", func() { } mockBuildAPI.EXPECT().Sync(context.Background(), &mld, pv.Spec.PushBuiltImage, pv). - Return(utils.Status(""), fmt.Errorf("some error")) + Return(imgbuild.Status(""), fmt.Errorf("some error")) res, msg := ph.verifyBuild(context.Background(), pv, &mld) Expect(res).To(BeFalse()) @@ -346,7 +343,7 @@ var _ = Describe("preflightHelper_verifyBuild", func() { } mockBuildAPI.EXPECT().Sync(context.Background(), &mld, pv.Spec.PushBuiltImage, pv). - Return(utils.Status(utils.StatusCompleted), nil) + Return(imgbuild.StatusCompleted, nil) res, msg := ph.verifyBuild(context.Background(), pv, &mld) Expect(res).To(BeTrue()) @@ -362,7 +359,7 @@ var _ = Describe("preflightHelper_verifyBuild", func() { } mockBuildAPI.EXPECT().Sync(context.Background(), &mld, pv.Spec.PushBuiltImage, pv). - Return(utils.Status(utils.StatusInProgress), nil) + Return(imgbuild.StatusInProgress, nil) res, msg := ph.verifyBuild(context.Background(), pv, &mld) Expect(res).To(BeFalse()) @@ -373,7 +370,7 @@ var _ = Describe("preflightHelper_verifyBuild", func() { var _ = Describe("preflightHelper_verifySign", func() { var ( ctrl *gomock.Controller - mockSignAPI *sign.MockSignManager + mockSignAPI *imgbuild.MockJobManager clnt *client.MockClient ph *preflightHelper ) @@ -381,7 +378,7 @@ var _ = Describe("preflightHelper_verifySign", func() { BeforeEach(func() { ctrl = gomock.NewController(GinkgoT()) clnt = client.NewMockClient(ctrl) - mockSignAPI = sign.NewMockSignManager(ctrl) + mockSignAPI = imgbuild.NewMockJobManager(ctrl) ph = &preflightHelper{ client: clnt, signAPI: mockSignAPI, @@ -401,10 +398,8 @@ var _ = Describe("preflightHelper_verifySign", func() { KernelVersion: kernelVersion, } - previousImage := "" - - mockSignAPI.EXPECT().Sync(context.Background(), &mld, previousImage, pv.Spec.PushBuiltImage, pv). - Return(utils.Status(""), fmt.Errorf("some error")) + mockSignAPI.EXPECT().Sync(context.Background(), &mld, pv.Spec.PushBuiltImage, pv). + Return(imgbuild.Status(""), fmt.Errorf("some error")) res, msg := ph.verifySign(context.Background(), pv, &mld) Expect(res).To(BeFalse()) @@ -419,10 +414,8 @@ var _ = Describe("preflightHelper_verifySign", func() { KernelVersion: kernelVersion, } - previousImage := "" - - mockSignAPI.EXPECT().Sync(context.Background(), &mld, previousImage, pv.Spec.PushBuiltImage, pv). - Return(utils.Status(utils.StatusCompleted), nil) + mockSignAPI.EXPECT().Sync(context.Background(), &mld, pv.Spec.PushBuiltImage, pv). + Return(imgbuild.StatusCompleted, nil) res, msg := ph.verifySign(context.Background(), pv, &mld) Expect(res).To(BeTrue()) @@ -437,10 +430,8 @@ var _ = Describe("preflightHelper_verifySign", func() { KernelVersion: kernelVersion, } - previousImage := "" - - mockSignAPI.EXPECT().Sync(context.Background(), &mld, previousImage, pv.Spec.PushBuiltImage, pv). - Return(utils.Status(utils.StatusInProgress), nil) + mockSignAPI.EXPECT().Sync(context.Background(), &mld, pv.Spec.PushBuiltImage, pv). + Return(imgbuild.StatusInProgress, nil) res, msg := ph.verifySign(context.Background(), pv, &mld) Expect(res).To(BeFalse()) 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 deleted file mode 100644 index 1cba3646f..000000000 --- a/internal/sign/job/manager.go +++ /dev/null @@ -1,127 +0,0 @@ -package signjob - -import ( - "context" - "errors" - "fmt" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/log" - - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "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/utils" -) - -type signJobManager struct { - client client.Client - signer Signer - jobHelper utils.JobHelper - registry registry.Registry -} - -func NewSignJobManager( - client client.Client, - signer Signer, - jobHelper utils.JobHelper, - registry registry.Registry) *signJobManager { - return &signJobManager{ - client: client, - signer: signer, - jobHelper: jobHelper, - registry: registry, - } -} - -func (jbm *signJobManager) GarbageCollect(ctx context.Context, modName, namespace string, owner metav1.Object) ([]string, error) { - jobs, err := jbm.jobHelper.GetModuleJobs(ctx, modName, namespace, utils.JobTypeSign, owner) - if err != nil { - return nil, fmt.Errorf("failed to get sign jobs for module %s: %v", modName, err) - } - - deleteNames := make([]string, 0, len(jobs)) - for _, job := range jobs { - if job.Status.Succeeded == 1 { - err = jbm.jobHelper.DeleteJob(ctx, &job) - if err != nil { - return nil, fmt.Errorf("failed to delete build job %s: %v", job.Name, err) - } - deleteNames = append(deleteNames, job.Name) - } - } - return deleteNames, nil -} - -func (jbm *signJobManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { - - // if there is no sign specified skip - if !module.ShouldBeSigned(mld) { - return false, nil - } - - exists, err := module.ImageExists(ctx, jbm.client, jbm.registry, mld, mld.Namespace, mld.ContainerImage) - if err != nil { - return false, fmt.Errorf("failed to check existence of image %s: %w", mld.ContainerImage, err) - } - - return !exists, nil -} - -func (jbm *signJobManager) Sync( - ctx context.Context, - mld *api.ModuleLoaderData, - imageToSign string, - pushImage bool, - owner metav1.Object) (utils.Status, error) { - - logger := log.FromContext(ctx) - - logger.Info("Signing in-cluster") - - labels := jbm.jobHelper.JobLabels(mld.Name, mld.KernelVersion, "sign") - - jobTemplate, err := jbm.signer.MakeJobTemplate(ctx, mld, labels, imageToSign, pushImage, owner) - if err != nil { - return "", fmt.Errorf("could not make Job template: %v", err) - } - - job, err := jbm.jobHelper.GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, mld.KernelVersion, utils.JobTypeSign, owner) - if err != nil { - if !errors.Is(err, utils.ErrNoMatchingJob) { - return "", fmt.Errorf("error getting the signing job: %v", err) - } - - logger.Info("Creating job") - err = jbm.jobHelper.CreateJob(ctx, jobTemplate) - if err != nil { - return "", fmt.Errorf("could not create Signing Job: %v", err) - } - - return utils.StatusCreated, nil - } - // default, there are no errors, and there is a job, check if it has changed - changed, err := jbm.jobHelper.IsJobChanged(job, jobTemplate) - if err != nil { - return "", fmt.Errorf("could not determine if job has changed: %v", err) - } - - if changed { - logger.Info("The module's sign spec has been changed, deleting the current job so a new one can be created", "name", job.Name) - err = jbm.jobHelper.DeleteJob(ctx, job) - if err != nil { - logger.Info(utils.WarnString(fmt.Sprintf("failed to delete signing job %s: %v", job.Name, err))) - } - return utils.StatusInProgress, nil - } - - logger.Info("Returning job status", "name", job.Name, "namespace", job.Namespace) - - statusmsg, err := jbm.jobHelper.GetJobStatus(job) - if err != nil { - return "", err - } - - return statusmsg, nil -} diff --git a/internal/sign/job/manager_test.go b/internal/sign/job/manager_test.go deleted file mode 100644 index 70a02bcef..000000000 --- a/internal/sign/job/manager_test.go +++ /dev/null @@ -1,411 +0,0 @@ -package signjob - -import ( - "context" - "errors" - "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/client" - "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "github.com/kubernetes-sigs/kernel-module-management/internal/registry" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - batchv1 "k8s.io/api/batch/v1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -var _ = Describe("ShouldSync", func() { - var ( - ctrl *gomock.Controller - clnt *client.MockClient - reg *registry.MockRegistry - mgr *signJobManager - ) - - const ( - moduleName = "module-name" - imageName = "image-name" - namespace = "some-namespace" - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - reg = registry.NewMockRegistry(ctrl) - mgr = NewSignJobManager(clnt, nil, nil, reg) - }) - - It("should return false if there was not sign section", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{} - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).ToNot(HaveOccurred()) - Expect(shouldSync).To(BeFalse()) - }) - - It("should return false if image already exists", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, - ContainerImage: imageName, - Sign: &kmmv1beta1.Sign{}, - } - - gomock.InOrder( - reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(true, nil), - ) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).ToNot(HaveOccurred()) - Expect(shouldSync).To(BeFalse()) - }) - - It("should return false and an error if image check fails", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, - ContainerImage: imageName, - Sign: &kmmv1beta1.Sign{}, - } - - gomock.InOrder( - reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, errors.New("generic-registry-error")), - ) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("generic-registry-error")) - Expect(shouldSync).To(BeFalse()) - }) - - It("should return true if image does not exist", func() { - ctx := context.Background() - - mld := &api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - ImageRepoSecret: &v1.LocalObjectReference{Name: "pull-push-secret"}, - ContainerImage: imageName, - Sign: &kmmv1beta1.Sign{}, - } - - gomock.InOrder( - reg.EXPECT().ImageExists(ctx, imageName, nil, gomock.Any()).Return(false, nil), - ) - - shouldSync, err := mgr.ShouldSync(ctx, mld) - - Expect(err).ToNot(HaveOccurred()) - Expect(shouldSync).To(BeTrue()) - }) -}) - -var _ = Describe("Sync", func() { - var ( - ctrl *gomock.Controller - maker *MockSigner - jobhelper *utils.MockJobHelper - mgr *signJobManager - ) - - const ( - imageName = "image-name" - previousImageName = "previous-image" - namespace = "some-namespace" - - moduleName = "module-name" - kernelVersion = "1.2.3" - jobName = "some-job" - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - maker = NewMockSigner(ctrl) - jobhelper = utils.NewMockJobHelper(ctrl) - mgr = NewSignJobManager(nil, maker, jobhelper, nil) - }) - - labels := map[string]string{"kmm.node.kubernetes.io/job-type": "sign", - "kmm.node.kubernetes.io/module.name": moduleName, - "kmm.node.kubernetes.io/target-kernel": kernelVersion, - } - - mld := &api.ModuleLoaderData{ - Name: moduleName, - ContainerImage: imageName, - Build: &kmmv1beta1.Build{}, - Owner: &kmmv1beta1.Module{}, - KernelVersion: kernelVersion, - } - - DescribeTable("should return the correct status depending on the job status", - func(s batchv1.JobStatus, jobStatus utils.Status, expectsErr bool) { - j := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"kmm.node.kubernetes.io/job-type": "sign", - "kmm.node.kubernetes.io/module.name": moduleName, - "kmm.node.kubernetes.io/target-kernel": kernelVersion, - }, - Namespace: namespace, - Annotations: map[string]string{constants.JobHashAnnotation: "some hash"}, - }, - Status: s, - } - newJob := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{"kmm.node.kubernetes.io/job-type": "sign", - "kmm.node.kubernetes.io/module.name": moduleName, - "kmm.node.kubernetes.io/target-kernel": kernelVersion, - }, - Namespace: namespace, - Annotations: map[string]string{constants.JobHashAnnotation: "some hash"}, - }, - Status: s, - } - ctx := context.Background() - - var joberr error - if expectsErr { - joberr = errors.New("random error") - } - - gomock.InOrder( - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(ctx, mld, labels, previousImageName, true, mld.Owner).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeSign, mld.Owner).Return(&newJob, nil), - jobhelper.EXPECT().IsJobChanged(&j, &newJob).Return(false, nil), - jobhelper.EXPECT().GetJobStatus(&newJob).Return(jobStatus, joberr), - ) - - res, err := mgr.Sync(ctx, mld, previousImageName, true, mld.Owner) - - if expectsErr { - Expect(err).To(HaveOccurred()) - return - } - - Expect(res).To(Equal(jobStatus)) - }, - Entry("active", batchv1.JobStatus{Active: 1}, utils.Status(utils.StatusInProgress), false), - Entry("active", batchv1.JobStatus{Active: 1}, utils.Status(utils.StatusInProgress), false), - Entry("succeeded", batchv1.JobStatus{Succeeded: 1}, utils.Status(utils.StatusCompleted), false), - Entry("failed", batchv1.JobStatus{Failed: 1}, utils.Status(""), true), - ) - - It("should return an error if there was an error creating the job template", func() { - ctx := context.Background() - - gomock.InOrder( - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(ctx, mld, labels, previousImageName, true, mld.Owner). - Return(nil, errors.New("random error")), - ) - - Expect( - mgr.Sync(ctx, mld, previousImageName, true, mld.Owner), - ).Error().To( - HaveOccurred(), - ) - }) - - It("should return an error if there was an error getting running jobs", func() { - ctx := context.Background() - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - }, - } - - gomock.InOrder( - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(ctx, mld, labels, previousImageName, true, mld.Owner).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeSign, mld.Owner).Return(nil, errors.New("random error")), - ) - - Expect( - mgr.Sync(ctx, mld, previousImageName, true, mld.Owner), - ).Error().To( - HaveOccurred(), - ) - }) - - It("should return an error if there was an error creating the job", func() { - ctx := context.Background() - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - }, - } - - gomock.InOrder( - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(ctx, mld, labels, previousImageName, true, mld.Owner).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeSign, mld.Owner).Return(nil, utils.ErrNoMatchingJob), - jobhelper.EXPECT().CreateJob(ctx, &j).Return(errors.New("unable to create job")), - ) - - Expect( - mgr.Sync(ctx, mld, previousImageName, true, mld.Owner), - ).Error().To( - HaveOccurred(), - ) - }) - - It("should create the job and return without error if it doesn't exist", func() { - ctx := context.Background() - - j := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - }, - } - - gomock.InOrder( - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(ctx, mld, labels, previousImageName, true, mld.Owner).Return(&j, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeSign, mld.Owner).Return(nil, utils.ErrNoMatchingJob), - jobhelper.EXPECT().CreateJob(ctx, &j).Return(nil), - ) - - Expect( - mgr.Sync(ctx, mld, previousImageName, true, mld.Owner), - ).To( - Equal(utils.Status(utils.StatusCreated)), - ) - }) - - It("should delete the job if it was edited", func() { - ctx := context.Background() - - newJob := batchv1.Job{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "batch/v1", - Kind: "Job", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: jobName, - Namespace: namespace, - Annotations: map[string]string{constants.JobHashAnnotation: "new hash"}, - }, - } - - gomock.InOrder( - jobhelper.EXPECT().JobLabels(mld.Name, kernelVersion, "sign").Return(labels), - maker.EXPECT().MakeJobTemplate(ctx, mld, labels, previousImageName, true, mld.Owner).Return(&newJob, nil), - jobhelper.EXPECT().GetModuleJobByKernel(ctx, mld.Name, mld.Namespace, kernelVersion, utils.JobTypeSign, mld.Owner).Return(&newJob, nil), - jobhelper.EXPECT().IsJobChanged(&newJob, &newJob).Return(true, nil), - jobhelper.EXPECT().DeleteJob(ctx, &newJob).Return(nil), - ) - - Expect( - mgr.Sync(ctx, mld, previousImageName, true, mld.Owner), - ).To( - Equal(utils.Status(utils.StatusInProgress)), - ) - }) -}) - -var _ = Describe("GarbageCollect", func() { - var ( - ctrl *gomock.Controller - clnt *client.MockClient - jobhelper *utils.MockJobHelper - mgr *signJobManager - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - jobhelper = utils.NewMockJobHelper(ctrl) - mgr = NewSignJobManager(clnt, nil, jobhelper, nil) - }) - - mld := api.ModuleLoaderData{ - Name: "moduleName", - Owner: &kmmv1beta1.Module{}, - } - - DescribeTable("should return the correct error and names of the collected jobs", - func(jobStatus1 batchv1.JobStatus, jobStatus2 batchv1.JobStatus, expectsErr bool) { - job1 := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "jobName1", - }, - Status: jobStatus1, - } - job2 := batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - Name: "jobName2", - }, - Status: jobStatus2, - } - expectedNames := []string{} - if !expectsErr { - if job1.Status.Succeeded == 1 { - expectedNames = append(expectedNames, "jobName1") - } - if job2.Status.Succeeded == 1 { - expectedNames = append(expectedNames, "jobName2") - } - } - returnedError := fmt.Errorf("some error") - if !expectsErr { - returnedError = nil - } - - jobhelper.EXPECT().GetModuleJobs(context.Background(), mld.Name, mld.Namespace, utils.JobTypeSign, mld.Owner).Return([]batchv1.Job{job1, job2}, returnedError) - if !expectsErr { - if job1.Status.Succeeded == 1 { - jobhelper.EXPECT().DeleteJob(context.Background(), &job1).Return(nil) - } - if job2.Status.Succeeded == 1 { - jobhelper.EXPECT().DeleteJob(context.Background(), &job2).Return(nil) - } - } - - names, err := mgr.GarbageCollect(context.Background(), mld.Name, mld.Namespace, mld.Owner) - - if expectsErr { - Expect(err).To(HaveOccurred()) - Expect(names).To(BeNil()) - } else { - Expect(err).NotTo(HaveOccurred()) - Expect(expectedNames).To(Equal(names)) - } - }, - Entry("all jobs succeeded", batchv1.JobStatus{Succeeded: 1}, batchv1.JobStatus{Succeeded: 1}, false), - Entry("1 job succeeded", batchv1.JobStatus{Succeeded: 1}, batchv1.JobStatus{Succeeded: 0}, false), - Entry("0 job succeeded", batchv1.JobStatus{Succeeded: 0}, batchv1.JobStatus{Succeeded: 0}, false), - Entry("error occured", batchv1.JobStatus{Succeeded: 0}, batchv1.JobStatus{Succeeded: 0}, true), - ) -}) diff --git a/internal/sign/job/mock_signer.go b/internal/sign/job/mock_signer.go deleted file mode 100644 index 3f87b9135..000000000 --- a/internal/sign/job/mock_signer.go +++ /dev/null @@ -1,53 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: signer.go - -// Package signjob is a generated GoMock package. -package signjob - -import ( - context "context" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - api "github.com/kubernetes-sigs/kernel-module-management/internal/api" - v1 "k8s.io/api/batch/v1" - v10 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// MockSigner is a mock of Signer interface. -type MockSigner struct { - ctrl *gomock.Controller - recorder *MockSignerMockRecorder -} - -// MockSignerMockRecorder is the mock recorder for MockSigner. -type MockSignerMockRecorder struct { - mock *MockSigner -} - -// NewMockSigner creates a new mock instance. -func NewMockSigner(ctrl *gomock.Controller) *MockSigner { - mock := &MockSigner{ctrl: ctrl} - mock.recorder = &MockSignerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockSigner) EXPECT() *MockSignerMockRecorder { - return m.recorder -} - -// MakeJobTemplate mocks base method. -func (m *MockSigner) MakeJobTemplate(ctx context.Context, mld *api.ModuleLoaderData, labels map[string]string, imageToSign string, pushImage bool, owner v10.Object) (*v1.Job, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "MakeJobTemplate", ctx, mld, labels, imageToSign, pushImage, owner) - ret0, _ := ret[0].(*v1.Job) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// MakeJobTemplate indicates an expected call of MakeJobTemplate. -func (mr *MockSignerMockRecorder) MakeJobTemplate(ctx, mld, labels, imageToSign, pushImage, owner interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "MakeJobTemplate", reflect.TypeOf((*MockSigner)(nil).MakeJobTemplate), ctx, mld, labels, imageToSign, pushImage, owner) -} diff --git a/internal/sign/job/signer_test.go b/internal/sign/job/signer_test.go deleted file mode 100644 index 587c50fd8..000000000 --- a/internal/sign/job/signer_test.go +++ /dev/null @@ -1,408 +0,0 @@ -package signjob - -import ( - "context" - "fmt" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - - "github.com/golang/mock/gomock" - "github.com/google/go-cmp/cmp" - batchv1 "k8s.io/api/batch/v1" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/pointer" - ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" - - 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/client" - "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" -) - -var _ = Describe("MakeJobTemplate", func() { - const ( - unsignedImage = "my.registry/my/image" - signedImage = unsignedImage + "-signed" - buildImage = "some-kaniko-image:some-tag" - kernelVersion = "1.2.3" - moduleName = "module-name" - namespace = "some-namespace" - privateKey = "some private key" - publicKey = "some public key" - ) - - var ( - ctrl *gomock.Controller - clnt *client.MockClient - mld api.ModuleLoaderData - m Signer - jobhelper *utils.MockJobHelper - - filesToSign = []string{ - "/modules/simple-kmod.ko", - "/modules/simple-procfs-kmod.ko", - } - ) - - BeforeEach(func() { - ctrl = gomock.NewController(GinkgoT()) - clnt = client.NewMockClient(ctrl) - jobhelper = utils.NewMockJobHelper(ctrl) - m = NewSigner(clnt, scheme, jobhelper) - mld = api.ModuleLoaderData{ - Name: moduleName, - Namespace: namespace, - Owner: &kmmv1beta1.Module{ - ObjectMeta: metav1.ObjectMeta{ - Name: moduleName, - Namespace: namespace, - }, - }, - KernelVersion: kernelVersion, - } - }) - - labels := map[string]string{"kmm.node.kubernetes.io/job-type": "sign", - "kmm.node.kubernetes.io/module.name": moduleName, - "kmm.node.kubernetes.io/target-kernel": kernelVersion, - } - - publicSignData := map[string][]byte{constants.PublicSignDataKey: []byte(publicKey)} - privateSignData := map[string][]byte{constants.PrivateSignDataKey: []byte(privateKey)} - - DescribeTable("should set fields correctly", func(imagePullSecret *v1.LocalObjectReference) { - GinkgoT().Setenv("RELATED_IMAGES_BUILD", buildImage) - GinkgoT().Setenv("RELATED_IMAGES_SIGN", "some-sign-image:some-tag") - - ctx := context.Background() - nodeSelector := map[string]string{"arch": "x64"} - - mld.Sign = &kmmv1beta1.Sign{ - UnsignedImage: signedImage, - KeySecret: &v1.LocalObjectReference{Name: "securebootkey"}, - CertSecret: &v1.LocalObjectReference{Name: "securebootcert"}, - FilesToSign: filesToSign, - } - mld.ContainerImage = signedImage - mld.RegistryTLS = &kmmv1beta1.TLSOptions{} - - secretMount := v1.VolumeMount{ - Name: "secret-securebootcert", - ReadOnly: true, - MountPath: "/run/secrets/cert", - } - certMount := v1.VolumeMount{ - Name: "secret-securebootkey", - ReadOnly: true, - MountPath: "/run/secrets/key", - } - keysecret := v1.Volume{ - Name: "secret-securebootkey", - VolumeSource: v1.VolumeSource{ - Secret: &v1.SecretVolumeSource{ - SecretName: "securebootkey", - Items: []v1.KeyToPath{ - { - Key: "key", - Path: "key.pem", - }, - }, - }, - }, - } - certsecret := v1.Volume{ - Name: "secret-securebootcert", - VolumeSource: v1.VolumeSource{ - Secret: &v1.SecretVolumeSource{ - SecretName: "securebootcert", - Items: []v1.KeyToPath{ - { - Key: "cert", - Path: "cert.pem", - }, - }, - }, - }, - } - - expected := &batchv1.Job{ - ObjectMeta: metav1.ObjectMeta{ - GenerateName: mld.Name + "-sign-", - Namespace: namespace, - Labels: map[string]string{ - constants.ModuleNameLabel: moduleName, - constants.TargetKernelTarget: kernelVersion, - constants.JobType: "sign", - }, - OwnerReferences: []metav1.OwnerReference{ - { - APIVersion: "kmm.sigs.x-k8s.io/v1beta1", - Kind: "Module", - Name: moduleName, - Controller: pointer.Bool(true), - BlockOwnerDeletion: pointer.Bool(true), - }, - }, - }, - Spec: batchv1.JobSpec{ - Completions: pointer.Int32(1), - BackoffLimit: pointer.Int32(0), - Template: v1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{ - "dockerfile": `FROM my.registry/my/image as source - -FROM some-sign-image:some-tag AS signimage - -RUN mkdir -p /tmp/signroot - -COPY --from=source /modules/simple-kmod.ko /tmp/signroot/modules/simple-kmod.ko -RUN /usr/local/bin/sign-file sha256 /run/secrets/key/key.pem /run/secrets/cert/cert.pem /tmp/signroot/modules/simple-kmod.ko -COPY --from=source /modules/simple-procfs-kmod.ko /tmp/signroot/modules/simple-procfs-kmod.ko -RUN /usr/local/bin/sign-file sha256 /run/secrets/key/key.pem /run/secrets/cert/cert.pem /tmp/signroot/modules/simple-procfs-kmod.ko - -FROM source - -COPY --from=signimage /tmp/signroot/modules/simple-kmod.ko /modules/simple-kmod.ko -COPY --from=signimage /tmp/signroot/modules/simple-procfs-kmod.ko /modules/simple-procfs-kmod.ko -`, - }, - }, - Spec: v1.PodSpec{ - Containers: []v1.Container{ - { - Name: "kaniko", - Image: buildImage, - Args: []string{"--destination", signedImage}, - VolumeMounts: []v1.VolumeMount{ - secretMount, - certMount, - { - Name: "dockerfile", - ReadOnly: true, - MountPath: "/workspace", - }, - }, - }, - }, - NodeSelector: nodeSelector, - RestartPolicy: v1.RestartPolicyNever, - Volumes: []v1.Volume{ - keysecret, - certsecret, - { - Name: "dockerfile", - VolumeSource: v1.VolumeSource{ - DownwardAPI: &v1.DownwardAPIVolumeSource{ - Items: []v1.DownwardAPIVolumeFile{ - { - Path: "Dockerfile", - FieldRef: &v1.ObjectFieldSelector{ - FieldPath: "metadata.annotations['dockerfile']", - }, - }, - }, - }, - }, - }, - }, - }, - }, - }, - } - if imagePullSecret != nil { - mld.ImageRepoSecret = imagePullSecret - expected.Spec.Template.Spec.Containers[0].VolumeMounts = - append(expected.Spec.Template.Spec.Containers[0].VolumeMounts, - v1.VolumeMount{ - Name: "secret-pull-push-secret", - ReadOnly: true, - MountPath: "/kaniko/.docker", - }, - ) - - expected.Spec.Template.Spec.Volumes = - append(expected.Spec.Template.Spec.Volumes, - v1.Volume{ - Name: "secret-pull-push-secret", - VolumeSource: v1.VolumeSource{ - Secret: &v1.SecretVolumeSource{ - SecretName: "pull-push-secret", - Items: []v1.KeyToPath{ - {Key: ".dockerconfigjson", Path: "config.json"}, - }, - }, - }, - }, - ) - } - - hash, err := getHashValue(&expected.Spec.Template, []byte(publicKey), []byte(privateKey)) - Expect(err).NotTo(HaveOccurred()) - annotations := map[string]string{constants.JobHashAnnotation: fmt.Sprintf("%d", hash)} - expected.SetAnnotations(annotations) - - mld.Selector = nodeSelector - - gomock.InOrder( - clnt.EXPECT().Get(ctx, types.NamespacedName{Name: mld.Sign.KeySecret.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, secret *v1.Secret, _ ...ctrlclient.GetOption) error { - secret.Data = privateSignData - return nil - }, - ), - clnt.EXPECT().Get(ctx, types.NamespacedName{Name: mld.Sign.CertSecret.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, secret *v1.Secret, _ ...ctrlclient.GetOption) error { - secret.Data = publicSignData - return nil - }, - ), - ) - - actual, err := m.MakeJobTemplate(ctx, &mld, labels, unsignedImage, true, mld.Owner) - Expect(err).NotTo(HaveOccurred()) - - Expect( - cmp.Diff(expected, actual), - ).To( - BeEmpty(), - ) - }, - Entry( - "no secrets at all", - nil, - ), - Entry( - "only imagePullSecrets", - &v1.LocalObjectReference{Name: "pull-push-secret"}, - ), - ) - - DescribeTable("should set correct kmod-signer flags", func(filelist []string, pushImage bool) { - ctx := context.Background() - mld.Sign = &kmmv1beta1.Sign{ - UnsignedImage: signedImage, - KeySecret: &v1.LocalObjectReference{Name: "securebootkey"}, - CertSecret: &v1.LocalObjectReference{Name: "securebootcert"}, - FilesToSign: filelist, - } - mld.ContainerImage = unsignedImage - mld.RegistryTLS = &kmmv1beta1.TLSOptions{} - - gomock.InOrder( - clnt.EXPECT().Get(ctx, types.NamespacedName{Name: mld.Sign.KeySecret.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, secret *v1.Secret, _ ...ctrlclient.GetOption) error { - secret.Data = privateSignData - return nil - }, - ), - clnt.EXPECT().Get(ctx, types.NamespacedName{Name: mld.Sign.CertSecret.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, secret *v1.Secret, _ ...ctrlclient.GetOption) error { - secret.Data = publicSignData - return nil - }, - ), - ) - - actual, err := m.MakeJobTemplate(ctx, &mld, labels, "", pushImage, mld.Owner) - - Expect(err).NotTo(HaveOccurred()) - - if pushImage { - Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement("--destination")) - } else { - Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement("-no-push")) - } - - }, - Entry( - "filelist and push", - []string{"simple-kmod", "complicated-kmod"}, - true, - ), - Entry( - "filelist and no push", - []string{"simple-kmod", "complicated-kmod"}, - false, - ), - Entry( - "all kmods and push", - []string{}, - true, - ), - Entry( - "all kmods and dont push", - []string{}, - false, - ), - ) - - DescribeTable("should set correct kmod-signer TLS flags", func(kmRegistryTLS, - unsignedImageRegistryTLS kmmv1beta1.TLSOptions, expectedFlag string) { - ctx := context.Background() - mld.Sign = &kmmv1beta1.Sign{ - UnsignedImage: signedImage, - UnsignedImageRegistryTLS: unsignedImageRegistryTLS, - KeySecret: &v1.LocalObjectReference{Name: "securebootkey"}, - CertSecret: &v1.LocalObjectReference{Name: "securebootcert"}, - } - mld.RegistryTLS = &kmRegistryTLS - - gomock.InOrder( - clnt.EXPECT().Get(ctx, types.NamespacedName{Name: mld.Sign.KeySecret.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, secret *v1.Secret, _ ...ctrlclient.GetOption) error { - secret.Data = privateSignData - return nil - }, - ), - clnt.EXPECT().Get(ctx, types.NamespacedName{Name: mld.Sign.CertSecret.Name, Namespace: mld.Namespace}, gomock.Any()).DoAndReturn( - func(_ interface{}, _ interface{}, secret *v1.Secret, _ ...ctrlclient.GetOption) error { - secret.Data = publicSignData - return nil - }, - ), - ) - - actual, err := m.MakeJobTemplate(ctx, &mld, labels, "", true, mld.Owner) - - Expect(err).NotTo(HaveOccurred()) - Expect(actual.Spec.Template.Spec.Containers[0].Args).To(ContainElement(expectedFlag)) - }, - Entry( - "filelist and push", - kmmv1beta1.TLSOptions{ - Insecure: true, - }, - kmmv1beta1.TLSOptions{}, - "--insecure", - ), - Entry( - "filelist and push", - kmmv1beta1.TLSOptions{ - InsecureSkipTLSVerify: true, - }, - kmmv1beta1.TLSOptions{}, - "--skip-tls-verify", - ), - Entry( - "filelist and push", - kmmv1beta1.TLSOptions{}, - kmmv1beta1.TLSOptions{ - Insecure: true, - }, - "--insecure-pull", - ), - Entry( - "filelist and push", - kmmv1beta1.TLSOptions{}, - kmmv1beta1.TLSOptions{ - InsecureSkipTLSVerify: true, - }, - "--skip-tls-verify-pull", - ), - ) -}) diff --git a/internal/sign/job/signer.go b/internal/sign/jobmaker.go similarity index 74% rename from internal/sign/job/signer.go rename to internal/sign/jobmaker.go index 5070d450f..77b9712c4 100644 --- a/internal/sign/job/signer.go +++ b/internal/sign/jobmaker.go @@ -1,13 +1,16 @@ -package signjob +package sign import ( "bytes" "context" "embed" "fmt" - "os" "text/template" + "github.com/kubernetes-sigs/kernel-module-management/internal/api" + "github.com/kubernetes-sigs/kernel-module-management/internal/constants" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" + "github.com/kubernetes-sigs/kernel-module-management/internal/utils" "github.com/mitchellh/hashstructure/v2" batchv1 "k8s.io/api/batch/v1" v1 "k8s.io/api/core/v1" @@ -17,78 +20,49 @@ import ( "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/constants" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" ) -type TemplateData struct { - FilesToSign []string - SignImage string - UnsignedImage string -} - -//go:embed templates -var templateFS embed.FS +var ( + //go:embed templates + templateFS embed.FS -var tmpl = template.Must( - template.ParseFS(templateFS, "templates/Dockerfile.gotmpl"), + tmpl = template.Must( + template.ParseFS(templateFS, "templates/Dockerfile.gotmpl"), + ) ) -//go:generate mockgen -source=signer.go -package=signjob -destination=mock_signer.go - -type Signer interface { - MakeJobTemplate( - ctx context.Context, - mld *api.ModuleLoaderData, - labels map[string]string, - imageToSign string, - pushImage bool, - owner metav1.Object, - ) (*batchv1.Job, error) -} +type ( + TemplateData struct { + FilesToSign []string + SignImage string + UnsignedImage string + } -type hashData struct { - PrivateKeyData []byte - PublicKeyData []byte - PodTemplate *v1.PodTemplateSpec -} + jobMaker struct { + buildImage string + client client.Client + jobHelper imgbuild.JobHelper + scheme *runtime.Scheme + signImage string + } +) -type signer struct { - client client.Client - scheme *runtime.Scheme - jobHelper utils.JobHelper -} +var _ imgbuild.JobMaker = &jobMaker{} -func NewSigner( - client client.Client, - scheme *runtime.Scheme, - jobHelper utils.JobHelper) Signer { - return &signer{ - client: client, - scheme: scheme, - jobHelper: jobHelper, +func NewJobMaker(client client.Client, jobHelper imgbuild.JobHelper, buildImage, signImage string, scheme *runtime.Scheme) imgbuild.JobMaker { + return &jobMaker{ + buildImage: buildImage, + client: client, + jobHelper: jobHelper, + scheme: scheme, + signImage: signImage, } } -func (s *signer) MakeJobTemplate( - ctx context.Context, - mld *api.ModuleLoaderData, - labels map[string]string, - imageToSign string, - pushImage bool, - owner metav1.Object) (*batchv1.Job, error) { +func (s *jobMaker) MakeJob(ctx context.Context, mld *api.ModuleLoaderData, owner metav1.Object, pushImage bool) (*batchv1.Job, error) { signConfig := mld.Sign - var buf bytes.Buffer - - td := TemplateData{ - FilesToSign: mld.Sign.FilesToSign, - SignImage: os.Getenv("RELATED_IMAGES_SIGN"), - } - args := make([]string, 0) if pushImage { @@ -104,14 +78,6 @@ func (s *signer) MakeJobTemplate( args = append(args, "-no-push") } - if imageToSign != "" { - td.UnsignedImage = imageToSign - } else if signConfig.UnsignedImage != "" { - td.UnsignedImage = signConfig.UnsignedImage - } else { - return nil, fmt.Errorf("no image to sign given") - } - if signConfig.UnsignedImageRegistryTLS.Insecure { args = append(args, "--insecure-pull") } @@ -174,6 +140,19 @@ func (s *signer) MakeJobTemplate( ) } + var buf bytes.Buffer + + unsignedImage, err := mld.UnsignedImage() + if err != nil { + return nil, fmt.Errorf("could not determine the unsigned image: %v", err) + } + + td := TemplateData{ + FilesToSign: mld.Sign.FilesToSign, + SignImage: s.signImage, + UnsignedImage: unsignedImage, + } + if err := tmpl.Execute(&buf, td); err != nil { return nil, fmt.Errorf("could not execute template: %v", err) } @@ -186,7 +165,7 @@ func (s *signer) MakeJobTemplate( Containers: []v1.Container{ { Name: "kaniko", - Image: os.Getenv("RELATED_IMAGES_BUILD"), + Image: s.buildImage, Args: args, VolumeMounts: volumeMounts, }, @@ -206,8 +185,8 @@ func (s *signer) MakeJobTemplate( ObjectMeta: metav1.ObjectMeta{ GenerateName: mld.Name + "-sign-", Namespace: mld.Namespace, - Labels: labels, - Annotations: map[string]string{constants.JobHashAnnotation: fmt.Sprintf("%d", specTemplateHash)}, + Labels: s.jobHelper.JobLabels(mld.Name, mld.KernelVersion, imgbuild.JobTypeSign), + Annotations: map[string]string{imgbuild.JobHashAnnotation: fmt.Sprintf("%d", specTemplateHash)}, }, Spec: batchv1.JobSpec{ Completions: pointer.Int32(1), @@ -223,7 +202,7 @@ func (s *signer) MakeJobTemplate( return job, nil } -func (s *signer) getHashAnnotationValue(ctx context.Context, privateSecret, publicSecret, namespace string, podTemplate *v1.PodTemplateSpec) (uint64, error) { +func (s *jobMaker) getHashAnnotationValue(ctx context.Context, privateSecret, publicSecret, namespace string, podTemplate *v1.PodTemplateSpec) (uint64, error) { privateKeyData, err := s.getSecretData(ctx, privateSecret, constants.PrivateSignDataKey, namespace) if err != nil { return 0, fmt.Errorf("failed to get private secret %s for signing: %v", privateSecret, err) @@ -236,7 +215,7 @@ func (s *signer) getHashAnnotationValue(ctx context.Context, privateSecret, publ return getHashValue(podTemplate, publicKeyData, privateKeyData) } -func (s *signer) getSecretData(ctx context.Context, secretName, secretDataKey, namespace string) ([]byte, error) { +func (s *jobMaker) getSecretData(ctx context.Context, secretName, secretDataKey, namespace string) ([]byte, error) { secret := v1.Secret{} namespacedName := types.NamespacedName{Name: secretName, Namespace: namespace} err := s.client.Get(ctx, namespacedName, &secret) @@ -250,6 +229,12 @@ func (s *signer) getSecretData(ctx context.Context, secretName, secretDataKey, n return data, nil } +type hashData struct { + PrivateKeyData []byte + PublicKeyData []byte + PodTemplate *v1.PodTemplateSpec +} + func getHashValue(podTemplate *v1.PodTemplateSpec, publicKeyData, privateKeyData []byte) (uint64, error) { dataToHash := hashData{ PrivateKeyData: privateKeyData, diff --git a/internal/sign/manager.go b/internal/sign/manager.go index a6148b2ae..05049e0a5 100644 --- a/internal/sign/manager.go +++ b/internal/sign/manager.go @@ -1,25 +1,7 @@ package sign import ( - "context" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "github.com/kubernetes-sigs/kernel-module-management/internal/api" - "github.com/kubernetes-sigs/kernel-module-management/internal/utils" + "github.com/kubernetes-sigs/kernel-module-management/internal/imgbuild" ) -//go:generate mockgen -source=manager.go -package=sign -destination=mock_manager.go - -type SignManager interface { - GarbageCollect(ctx context.Context, modName, namespace string, owner metav1.Object) ([]string, error) - - ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) - - Sync( - ctx context.Context, - mld *api.ModuleLoaderData, - imageToSign string, - pushImage bool, - owner metav1.Object) (utils.Status, error) -} +type Manager = imgbuild.JobManager 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/sign/mock_manager.go b/internal/sign/mock_manager.go deleted file mode 100644 index 07769999d..000000000 --- a/internal/sign/mock_manager.go +++ /dev/null @@ -1,83 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: manager.go - -// Package sign is a generated GoMock package. -package sign - -import ( - context "context" - reflect "reflect" - - gomock "github.com/golang/mock/gomock" - api "github.com/kubernetes-sigs/kernel-module-management/internal/api" - utils "github.com/kubernetes-sigs/kernel-module-management/internal/utils" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// MockSignManager is a mock of SignManager interface. -type MockSignManager struct { - ctrl *gomock.Controller - recorder *MockSignManagerMockRecorder -} - -// MockSignManagerMockRecorder is the mock recorder for MockSignManager. -type MockSignManagerMockRecorder struct { - mock *MockSignManager -} - -// NewMockSignManager creates a new mock instance. -func NewMockSignManager(ctrl *gomock.Controller) *MockSignManager { - mock := &MockSignManager{ctrl: ctrl} - mock.recorder = &MockSignManagerMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockSignManager) EXPECT() *MockSignManagerMockRecorder { - return m.recorder -} - -// GarbageCollect mocks base method. -func (m *MockSignManager) GarbageCollect(ctx context.Context, modName, namespace string, owner v1.Object) ([]string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GarbageCollect", ctx, modName, namespace, owner) - ret0, _ := ret[0].([]string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GarbageCollect indicates an expected call of GarbageCollect. -func (mr *MockSignManagerMockRecorder) GarbageCollect(ctx, modName, namespace, owner interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GarbageCollect", reflect.TypeOf((*MockSignManager)(nil).GarbageCollect), ctx, modName, namespace, owner) -} - -// ShouldSync mocks base method. -func (m *MockSignManager) ShouldSync(ctx context.Context, mld *api.ModuleLoaderData) (bool, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ShouldSync", ctx, mld) - ret0, _ := ret[0].(bool) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ShouldSync indicates an expected call of ShouldSync. -func (mr *MockSignManagerMockRecorder) ShouldSync(ctx, mld interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ShouldSync", reflect.TypeOf((*MockSignManager)(nil).ShouldSync), ctx, mld) -} - -// Sync mocks base method. -func (m *MockSignManager) Sync(ctx context.Context, mld *api.ModuleLoaderData, imageToSign string, pushImage bool, owner v1.Object) (utils.Status, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Sync", ctx, mld, imageToSign, pushImage, owner) - ret0, _ := ret[0].(utils.Status) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Sync indicates an expected call of Sync. -func (mr *MockSignManagerMockRecorder) Sync(ctx, mld, imageToSign, pushImage, owner interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sync", reflect.TypeOf((*MockSignManager)(nil).Sync), ctx, mld, imageToSign, pushImage, owner) -} diff --git a/internal/sign/job/templates/Dockerfile.gotmpl b/internal/sign/templates/Dockerfile.gotmpl similarity index 91% rename from internal/sign/job/templates/Dockerfile.gotmpl rename to internal/sign/templates/Dockerfile.gotmpl index 27633b20e..aa6d440f3 100644 --- a/internal/sign/job/templates/Dockerfile.gotmpl +++ b/internal/sign/templates/Dockerfile.gotmpl @@ -1,4 +1,4 @@ -{{- /*gotype: github.com/rh-ecosystem-edge/kernel-module-management/internal/sign/job.TemplateData */ -}} +{{- /*gotype: github.com/rh-ecosystem-edge/kernel-module-management/internal/sign.TemplateData */ -}} FROM {{ .UnsignedImage }} as source FROM {{ .SignImage }} AS signimage 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/sign/job/suite_test.go b/internal/utils/image/suite_test.go similarity index 88% rename from internal/sign/job/suite_test.go rename to internal/utils/image/suite_test.go index a6296ef35..5dfef70db 100644 --- a/internal/sign/job/suite_test.go +++ b/internal/utils/image/suite_test.go @@ -1,4 +1,4 @@ -package signjob +package image_test import ( "testing" @@ -19,5 +19,5 @@ func TestSuite(t *testing.T) { scheme, err = test.TestScheme() Expect(err).NotTo(HaveOccurred()) - RunSpecs(t, "Job Suite") + RunSpecs(t, "Image Suite") } diff --git a/internal/utils/suite_test.go b/internal/utils/suite_test.go index 2538dfa4f..3df73d846 100644 --- a/internal/utils/suite_test.go +++ b/internal/utils/suite_test.go @@ -3,21 +3,11 @@ package utils 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, "Job Suite") }