Normalize kernel version in labels and image tags (#1135)

Handle uncommon kernel versions with characters such as +, by replacing them with _. Upstream-Commit: 3ac00e3
rh-ecosystem-edge · Jun 3, 2024 · baaf1e8 · baaf1e8
1 parent 048f65c
commit baaf1e8
Show file tree

Hide file tree

Showing 22 changed files with 116 additions and 46 deletions.
diff --git a/bundle-hub/manifests/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml b/bundle-hub/manifests/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml
@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   creationTimestamp: null
   labels:
     app.kubernetes.io/component: kmm-hub

diff --git a/bundle-hub/manifests/kernel-module-management-hub.clusterserviceversion.yaml b/bundle-hub/manifests/kernel-module-management-hub.clusterserviceversion.yaml
@@ -32,7 +32,7 @@ metadata:
         }
       ]
     capabilities: Basic Install
-    createdAt: "2024-04-04T09:19:49Z"
+    createdAt: "2024-06-03T15:27:54Z"
     operatorframework.io/suggested-namespace: openshift-kmm-hub
     operators.operatorframework.io/builder: operator-sdk-v1.32.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3

diff --git a/bundle/manifests/kernel-module-management.clusterserviceversion.yaml b/bundle/manifests/kernel-module-management.clusterserviceversion.yaml
@@ -48,7 +48,7 @@ metadata:
         }
       ]
     capabilities: Basic Install
-    createdAt: "2024-04-04T09:19:49Z"
+    createdAt: "2024-06-03T15:27:53Z"
     operatorframework.io/suggested-namespace: openshift-kmm
     operators.operatorframework.io/builder: operator-sdk-v1.32.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v3

diff --git a/bundle/manifests/kmm.sigs.x-k8s.io_modules.yaml b/bundle/manifests/kmm.sigs.x-k8s.io_modules.yaml
@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   creationTimestamp: null
   labels:
     app.kubernetes.io/component: kmm

diff --git a/bundle/manifests/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/bundle/manifests/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml
@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   creationTimestamp: null
   labels:
     app.kubernetes.io/component: kmm

diff --git a/bundle/manifests/kmm.sigs.x-k8s.io_preflightvalidations.yaml b/bundle/manifests/kmm.sigs.x-k8s.io_preflightvalidations.yaml
@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   creationTimestamp: null
   labels:
     app.kubernetes.io/component: kmm

diff --git a/bundle/manifests/kmm.sigs.x-k8s.io_preflightvalidationsocp.yaml b/bundle/manifests/kmm.sigs.x-k8s.io_preflightvalidationsocp.yaml
@@ -2,7 +2,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   creationTimestamp: null
   labels:
     app.kubernetes.io/component: kmm

diff --git a/config/crd-hub/bases/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml b/config/crd-hub/bases/hub.kmm.sigs.x-k8s.io_managedclustermodules.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   name: managedclustermodules.hub.kmm.sigs.x-k8s.io
 spec:
   group: hub.kmm.sigs.x-k8s.io

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_modules.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_modules.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   name: modules.kmm.sigs.x-k8s.io
 spec:
   group: kmm.sigs.x-k8s.io

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_nodemodulesconfigs.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   name: nodemodulesconfigs.kmm.sigs.x-k8s.io
 spec:
   group: kmm.sigs.x-k8s.io

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_preflightvalidations.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_preflightvalidations.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   name: preflightvalidations.kmm.sigs.x-k8s.io
 spec:
   group: kmm.sigs.x-k8s.io

diff --git a/config/crd/bases/kmm.sigs.x-k8s.io_preflightvalidationsocp.yaml b/config/crd/bases/kmm.sigs.x-k8s.io_preflightvalidationsocp.yaml
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.14.0
+    controller-gen.kubebuilder.io/version: v0.15.0
   name: preflightvalidationsocp.kmm.sigs.x-k8s.io
 spec:
   group: kmm.sigs.x-k8s.io

diff --git a/internal/api/moduleloaderdata.go b/internal/api/moduleloaderdata.go
@@ -16,6 +16,11 @@ import (
 type ModuleLoaderData struct {
 	// kernel version
 	KernelVersion string
+
+	// KernelNormalizedVersion is the kernel version with some characters replaced with '_' so that it can be used in
+	// a Kubernetes label or a container image tag.
+	KernelNormalizedVersion string
+
 	// Repo secret for DS images
 	ImageRepoSecret *v1.LocalObjectReference
 

diff --git a/internal/kernel/kernel.go b/internal/kernel/kernel.go
@@ -0,0 +1,15 @@
+package kernel
+
+import "strings"
+
+func replaceInvalidChar(r rune) rune {
+	if (r < 'A' || r > 'Z') && (r < 'a' || r > 'z') && (r < '0' || r > '9') && r != '-' && r != '.' && r != '_' {
+		return '_'
+	}
+
+	return r
+}
+
+func NormalizeVersion(version string) string {
+	return strings.Map(replaceInvalidChar, version)
+}
diff --git a/internal/kernel/kernel_test.go b/internal/kernel/kernel_test.go
@@ -0,0 +1,26 @@
+package kernel
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("NormalizeVersion", func() {
+	DescribeTable(
+		"should work as expected",
+		func(version, expected string) {
+			Expect(
+				NormalizeVersion(version),
+			).To(
+				Equal(expected),
+			)
+		},
+		Entry(nil, "1.2.3-4", "1.2.3-4"),
+		Entry(nil, "1.2.3+4", "1.2.3_4"),
+		Entry(nil, "1.2.3_4", "1.2.3_4"),
+		Entry(nil, "1.2.3&4", "1.2.3_4"),
+		Entry(nil, "1.2.3%4", "1.2.3_4"),
+		Entry(nil, "1.2.3?4", "1.2.3_4"),
+		Entry(nil, "1.2.3 4", "1.2.3_4"),
+	)
+})
diff --git a/internal/kernel/suite_test.go b/internal/kernel/suite_test.go
@@ -0,0 +1,13 @@
+package kernel
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestSuite(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Kernel Suite")
+}
diff --git a/internal/module/kernelmapper.go b/internal/module/kernelmapper.go
@@ -8,6 +8,7 @@ import (
 	kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/api"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/build"
+	"github.com/rh-ecosystem-edge/kernel-module-management/internal/kernel"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/sign"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/utils"
 )
@@ -134,6 +135,7 @@ func (kh *kernelMapperHelper) prepareModuleLoaderData(mapping *kmmv1beta1.Kernel
 	}
 
 	mld.KernelVersion = kernelVersion
+	mld.KernelNormalizedVersion = kernel.NormalizeVersion(kernelVersion)
 	mld.Name = mod.Name
 	mld.Namespace = mod.Namespace
 	mld.ImageRepoSecret = mod.Spec.ImageRepoSecret
@@ -148,7 +150,7 @@ func (kh *kernelMapperHelper) prepareModuleLoaderData(mapping *kmmv1beta1.Kernel
 }
 
 func (kh *kernelMapperHelper) replaceTemplates(mld *api.ModuleLoaderData) error {
-	osConfigEnvVars := utils.KernelComponentsAsEnvVars(mld.KernelVersion)
+	osConfigEnvVars := utils.KernelComponentsAsEnvVars(mld.KernelNormalizedVersion)
 	osConfigEnvVars = append(osConfigEnvVars, "MOD_NAME="+mld.Name, "MOD_NAMESPACE="+mld.Namespace)
 
 	replacedContainerImage, err := utils.ReplaceInTemplates(osConfigEnvVars, mld.ContainerImage)

diff --git a/internal/module/kernelmapper_test.go b/internal/module/kernelmapper_test.go
@@ -192,15 +192,16 @@ var _ = Describe("prepareModuleLoaderData", func() {
 		}
 
 		mld := api.ModuleLoaderData{
-			Name:               mod.Name,
-			Namespace:          mod.Namespace,
-			ImageRepoSecret:    mod.Spec.ImageRepoSecret,
-			Owner:              &mod,
-			Selector:           mod.Spec.Selector,
-			ServiceAccountName: mod.Spec.ModuleLoader.ServiceAccountName,
-			Modprobe:           mod.Spec.ModuleLoader.Container.Modprobe,
-			ImagePullPolicy:    mod.Spec.ModuleLoader.Container.ImagePullPolicy,
-			KernelVersion:      kernelVersion,
+			Name:                    mod.Name,
+			Namespace:               mod.Namespace,
+			ImageRepoSecret:         mod.Spec.ImageRepoSecret,
+			Owner:                   &mod,
+			Selector:                mod.Spec.Selector,
+			ServiceAccountName:      mod.Spec.ModuleLoader.ServiceAccountName,
+			Modprobe:                mod.Spec.ModuleLoader.Container.Modprobe,
+			ImagePullPolicy:         mod.Spec.ModuleLoader.Container.ImagePullPolicy,
+			KernelVersion:           kernelVersion,
+			KernelNormalizedVersion: kernelVersion,
 		}
 
 		if buildExistsInMapping {
@@ -255,15 +256,16 @@ var _ = Describe("prepareModuleLoaderData", func() {
 	// [TODO] remove this unit test once InTreeModuleToRemove depricated field is removed from CRD
 	DescribeTable("prepare InTreeModules based on InTreeModule", func(inTreeModuleInContainer, inTreeModuleInMapping bool, expectedInTreeModules []string) {
 		mld := api.ModuleLoaderData{
-			Name:               mod.Name,
-			Namespace:          mod.Namespace,
-			ImageRepoSecret:    mod.Spec.ImageRepoSecret,
-			Owner:              &mod,
-			Selector:           mod.Spec.Selector,
-			ServiceAccountName: mod.Spec.ModuleLoader.ServiceAccountName,
-			Modprobe:           mod.Spec.ModuleLoader.Container.Modprobe,
-			ImagePullPolicy:    mod.Spec.ModuleLoader.Container.ImagePullPolicy,
-			KernelVersion:      kernelVersion,
+			Name:                    mod.Name,
+			Namespace:               mod.Namespace,
+			ImageRepoSecret:         mod.Spec.ImageRepoSecret,
+			Owner:                   &mod,
+			Selector:                mod.Spec.Selector,
+			ServiceAccountName:      mod.Spec.ModuleLoader.ServiceAccountName,
+			Modprobe:                mod.Spec.ModuleLoader.Container.Modprobe,
+			ImagePullPolicy:         mod.Spec.ModuleLoader.Container.ImagePullPolicy,
+			KernelVersion:           kernelVersion,
+			KernelNormalizedVersion: kernelVersion,
 		}
 		mld.RegistryTLS = &mod.Spec.ModuleLoader.Container.RegistryTLS
 		mld.ContainerImage = mod.Spec.ModuleLoader.Container.ContainerImage
@@ -280,7 +282,7 @@ var _ = Describe("prepareModuleLoaderData", func() {
 
 		res, err := kh.prepareModuleLoaderData(&mapping, &mod, kernelVersion)
 		Expect(err).NotTo(HaveOccurred())
-		Expect(*res).To(Equal(mld))
+		Expect(*res).To(BeComparableTo(mld))
 	},
 		Entry("inTreeModule not defined", false, false, nil),
 		Entry("inTreeModule defined in container", true, false, []string{"inTreeModuleToRemoveInContainer"}),
@@ -296,8 +298,9 @@ var _ = Describe("replaceTemplates", func() {
 
 	It("error input", func() {
 		mld := api.ModuleLoaderData{
-			ContainerImage: "some image:${KERNEL_XYZ",
-			KernelVersion:  kernelVersion,
+			ContainerImage:          "some image:${KERNEL_XYZ",
+			KernelVersion:           kernelVersion,
+			KernelNormalizedVersion: kernelVersion,
 		}
 		err := kh.replaceTemplates(&mld)
 		Expect(err).To(HaveOccurred())
@@ -313,7 +316,8 @@ var _ = Describe("replaceTemplates", func() {
 				},
 				DockerfileConfigMap: &v1.LocalObjectReference{},
 			},
-			KernelVersion: kernelVersion,
+			KernelVersion:           kernelVersion,
+			KernelNormalizedVersion: kernelVersion,
 		}
 		expectMld := api.ModuleLoaderData{
 			ContainerImage: "some image:5.8.18",
@@ -324,7 +328,8 @@ var _ = Describe("replaceTemplates", func() {
 				},
 				DockerfileConfigMap: &v1.LocalObjectReference{},
 			},
-			KernelVersion: kernelVersion,
+			KernelVersion:           kernelVersion,
+			KernelNormalizedVersion: kernelVersion,
 		}
 
 		err := kh.replaceTemplates(&mld)

diff --git a/internal/sign/helper.go b/internal/sign/helper.go
@@ -2,6 +2,7 @@ package sign
 
 import (
 	kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1"
+	"github.com/rh-ecosystem-edge/kernel-module-management/internal/kernel"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/utils"
 )
 
@@ -18,7 +19,7 @@ func NewSignerHelper() Helper {
 	return &helper{}
 }
 
-func (m *helper) GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernel string) (*kmmv1beta1.Sign, error) {
+func (m *helper) GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1beta1.Sign, kernelVersion string) (*kmmv1beta1.Sign, error) {
 	var signConfig *kmmv1beta1.Sign
 	if moduleSign == nil {
 		// km.Sign cannot be nil in case mod.Sign is nil, checked above
@@ -41,7 +42,10 @@ func (m *helper) GetRelevantSign(moduleSign *kmmv1beta1.Sign, mappingSign *kmmv1
 		//append (not overwrite) any files in the km to the defaults
 		signConfig.FilesToSign = append(signConfig.FilesToSign, mappingSign.FilesToSign...)
 	}
-	osConfigEnvVars := utils.KernelComponentsAsEnvVars(kernel)
+
+	osConfigEnvVars := utils.KernelComponentsAsEnvVars(
+		kernel.NormalizeVersion(kernelVersion),
+	)
 	unsignedImage, err := utils.ReplaceInTemplates(osConfigEnvVars, signConfig.UnsignedImage)
 	if err != nil {
 		return nil, err

diff --git a/internal/sign/helper_test.go b/internal/sign/helper_test.go
@@ -17,7 +17,7 @@ var _ = Describe("GetRelevantSign", func() {
 		keySecret     = "securebootkey"
 		certSecret    = "securebootcert"
 		filesToSign   = "/modules/simple-kmod.ko:/modules/simple-procfs-kmod.ko"
-		kernelVersion = "1.2.3"
+		kernelVersion = "1.2.3+4"
 	)
 
 	var (

diff --git a/internal/utils/ocpbuild/utils.go b/internal/utils/ocpbuild/utils.go
@@ -17,7 +17,7 @@ func GetOCPBuildAnnotations(hash uint64) map[string]string {
 }
 
 func GetOCPBuildLabels(mld *api.ModuleLoaderData, buildType string) map[string]string {
-	labels := moduleKernelLabels(mld.Name, mld.KernelVersion, buildType)
+	labels := moduleKernelLabels(mld.Name, mld.KernelNormalizedVersion, buildType)
 
 	labels["app.kubernetes.io/name"] = "kmm"
 	labels["app.kubernetes.io/component"] = buildType

diff --git a/internal/utils/ocpbuild/utils_test.go b/internal/utils/ocpbuild/utils_test.go
@@ -11,21 +11,21 @@ import (
 
 var _ = Describe("GetOCPBuildLabels", func() {
 	const (
-		kernelVersion = "some-kernel-version"
-		moduleName    = "some-module"
-		buildType     = "some-build-type"
+		kernelNormalizedVersion = "1.2.3_4"
+		moduleName              = "some-module"
+		buildType               = "some-build-type"
 	)
 
 	It("should work as expected", func() {
 		mld := &api.ModuleLoaderData{
-			KernelVersion: kernelVersion,
-			Name:          moduleName,
+			KernelNormalizedVersion: kernelNormalizedVersion,
+			Name:                    moduleName,
 		}
 
 		expected := map[string]string{
 			constants.ModuleNameLabel:     moduleName,
 			constants.BuildTypeLabel:      buildType,
-			constants.TargetKernelTarget:  kernelVersion,
+			constants.TargetKernelTarget:  kernelNormalizedVersion,
 			"app.kubernetes.io/name":      "kmm",
 			"app.kubernetes.io/component": buildType,
 			"app.kubernetes.io/part-of":   "kmm",