Gate DRA reconciler on Openshift >= 4.21

The DRA reconciler was registered unconditionally, which would crash the
entire operator on clusters older than 4.21 because the resource.k8s.io/v1
DeviceClass API does not exist there.

Introduce an internal/version package with OCPVersion, DiscoverOCPVersion,
and an AtLeast method to centralise version discovery and comparison. The
manager now checks the cluster version at startup and only registers the DRA
controller when the minimum version is met. The webhook package is updated to
use the shared version package instead of its own local types.

Also fixes a pre-existing bug where /run was missing from the allowed
hostPath prefixes error message.

Author: TomerNewman

cmd/manager/main.go

@@ -48,6 +48,7 @@ import (
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/networkpolicy"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/nmc"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/syncronizedmap"
+	"github.com/rh-ecosystem-edge/kernel-module-management/internal/version"
 	"k8s.io/apimachinery/pkg/api/meta"
 	runtimescheme "k8s.io/apimachinery/pkg/runtime/schema"
 
@@ -126,8 +127,16 @@ func main() {
 	}
 
 	options := cg.GetManagerOptionsFromConfig(cfg, scheme)
+	restCfg := ctrl.GetConfigOrDie()
 
-	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), options)
+	ocpVersion, err := version.DiscoverOCPVersion(restCfg)
+	if err != nil {
+		cmd.FatalError(setupLogger, err, "unable to discover Openshift version")
+	}
+
+	setupLogger.Info("Detected Openshift version", "major", ocpVersion.Major, "minor", ocpVersion.Minor)
+
+	mgr, err := ctrl.NewManager(restCfg, options)
 	if err != nil {
 		cmd.FatalError(setupLogger, err, "unable to create manager")
 	}
@@ -195,8 +204,16 @@ func main() {
 		cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.PodNodeLabelReconcilerName)
 	}
 
-	if err = controllers.NewDRAReconciler(client, nodeAPI, networkPolicyAPI, scheme, operatorNamespace).SetupWithManager(mgr); err != nil {
-		cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.DRAReconcilerName)
+	if ocpVersion.AtLeast(constants.MinOCPMajorForDRA, constants.MinOCPMinorForDRA) {
+		if err = controllers.NewDRAReconciler(client, nodeAPI, networkPolicyAPI, scheme, operatorNamespace).SetupWithManager(mgr); err != nil {
+			cmd.FatalError(setupLogger, err, "unable to create controller", "name", controllers.DRAReconcilerName)
+		}
+	} else {
+		setupLogger.Info(
+			fmt.Sprintf("Skipping DRA controller; requires Openshift >= %d.%d", constants.MinOCPMajorForDRA, constants.MinOCPMinorForDRA),
+			"detectedMajor", ocpVersion.Major,
+			"detectedMinor", ocpVersion.Minor,
+		)
 	}
 
 	if err = controllers.NewNodeLabelModuleVersionReconciler(client).SetupWithManager(mgr); err != nil {

cmd/webhook-server/main.go

@@ -9,6 +9,7 @@ import (
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/cmd"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/config"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/constants"
+	"github.com/rh-ecosystem-edge/kernel-module-management/internal/version"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/webhook"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/webhook/hub"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -84,7 +85,7 @@ func main() {
 	if enableModule {
 		logger.Info("Enabling Module webhook")
 
-		ocpVersion, err := webhook.DiscoverOCPVersion(restCfg)
+		ocpVersion, err := version.DiscoverOCPVersion(restCfg)
 		if err != nil {
 			cmd.FatalError(setupLogger, err, "unable to discover OpenShift version")
 		}

internal/version/version.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2022.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package version
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/rest"
+)
+
+var ocpVersionRe = regexp.MustCompile(`^v?(\d+)\.(\d+)`)
+
+// OCPVersion holds the parsed major and minor version of a OpenShift cluster.
+type OCPVersion struct {
+	Major int
+	Minor int
+}
+
+// AtLeast returns true if kv is greater than or equal to the given major.minor version.
+func (ov OCPVersion) AtLeast(major, minor int) bool {
+	return ov.Major > major || (ov.Major == major && ov.Minor >= minor)
+}
+
+// DiscoverOCPVersion queries the OpenShift API server and returns its version.
+func DiscoverOCPVersion(cfg *rest.Config) (OCPVersion, error) {
+	discClient, err := discovery.NewDiscoveryClientForConfig(cfg)
+	if err != nil {
+		return OCPVersion{}, fmt.Errorf("failed to create discovery client: %v", err)
+	}
+
+	serverVersion, err := discClient.ServerVersion()
+	if err != nil {
+		return OCPVersion{}, fmt.Errorf("failed to query OpenShift server version: %v", err)
+	}
+
+	return ParseOCPVersion(serverVersion.GitVersion)
+}
+
+// ParseOCPVersion extracts the major and minor version from a OpenShift
+// GitVersion string such as "v4.21.0" or "v4.21.0+ocp-4.21.0".
+func ParseOCPVersion(gitVersion string) (OCPVersion, error) {
+	m := ocpVersionRe.FindStringSubmatch(gitVersion)
+	if m == nil {
+		return OCPVersion{}, fmt.Errorf("cannot parse OpenShift version from %q", gitVersion)
+	}
+
+	major, err := strconv.Atoi(m[1])
+	if err != nil {
+		return OCPVersion{}, fmt.Errorf("cannot parse OpenShift major version from %q: %w", gitVersion, err)
+	}
+	minor, err := strconv.Atoi(m[2])
+	if err != nil {
+		return OCPVersion{}, fmt.Errorf("cannot parse OpenShift minor version from %q: %w", gitVersion, err)
+	}
+	return OCPVersion{Major: major, Minor: minor}, nil
+}

internal/version/version_test.go

@@ -0,0 +1,74 @@
+/*
+Copyright 2022.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package version
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestSuite(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Version Suite")
+}
+
+var _ = Describe("AtLeast", func() {
+	DescribeTable(
+		"should compare versions correctly",
+		func(major, minor, minMajor, minMinor int, expected bool) {
+			ov := OCPVersion{Major: major, Minor: minor}
+			Expect(ov.AtLeast(minMajor, minMinor)).To(Equal(expected))
+		},
+		Entry("exact match", 4, 21, 4, 21, true),
+		Entry("higher minor", 4, 22, 4, 21, true),
+		Entry("lower minor", 4, 20, 4, 21, false),
+		Entry("higher major, lower minor", 5, 0, 4, 21, true),
+		Entry("lower major", 3, 11, 4, 21, false),
+	)
+})
+
+var _ = Describe("ParseKubeVersion", func() {
+	DescribeTable(
+		"should parse valid version strings",
+		func(gitVersion string, expectedMajor, expectedMinor int) {
+			ov, err := ParseOCPVersion(gitVersion)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(ov.Major).To(Equal(expectedMajor))
+			Expect(ov.Minor).To(Equal(expectedMinor))
+		},
+		Entry("standard version", "v4.21.0", 4, 21),
+		Entry("with build metadata", "v4.21.0+ocp-4.21.0", 4, 21),
+		Entry("pre-release version", "v4.20.3-rc.1", 4, 20),
+		Entry("higher minor", "v4.22.1", 4, 22),
+		Entry("without v prefix", "4.21.0", 4, 21),
+		Entry("hypothetical OCP 5.0", "5.0.0", 5, 0),
+		Entry("hypothetical OCP 2.5", "2.5.3", 2, 5),
+	)
+
+	DescribeTable(
+		"should return error for invalid strings",
+		func(gitVersion string) {
+			_, err := ParseOCPVersion(gitVersion)
+			Expect(err).To(HaveOccurred())
+		},
+		Entry("empty string", ""),
+		Entry("garbage", "invalid"),
+		Entry("no minor", "v1"),
+	)
+})

internal/webhook/hub/managedclustermodule_webhook.go

@@ -23,6 +23,7 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/rh-ecosystem-edge/kernel-module-management/api-hub/v1beta1"
 	kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1"
+	"github.com/rh-ecosystem-edge/kernel-module-management/internal/version"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/webhook"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -34,7 +35,7 @@ type ManagedClusterModuleValidator struct {
 	m      admission.CustomValidator
 }
 
-func NewManagedClusterModuleValidator(logger logr.Logger, ocpVersion *webhook.OCPVersion) *ManagedClusterModuleValidator {
+func NewManagedClusterModuleValidator(logger logr.Logger, ocpVersion *version.OCPVersion) *ManagedClusterModuleValidator {
 	return &ManagedClusterModuleValidator{
 		logger: logger,
 		m:      webhook.NewModuleValidator(logger, ocpVersion),

internal/webhook/module.go

@@ -22,21 +22,17 @@ import (
 	"fmt"
 	"path/filepath"
 	"regexp"
-	"strconv"
 	"strings"
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 
 	"github.com/go-logr/logr"
-	configv1 "github.com/openshift/api/config/v1"
 	kmmv1beta1 "github.com/rh-ecosystem-edge/kernel-module-management/api/v1beta1"
 	"github.com/rh-ecosystem-edge/kernel-module-management/internal/constants"
+	"github.com/rh-ecosystem-edge/kernel-module-management/internal/version"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/util/sets"
-	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
@@ -45,66 +41,15 @@ import (
 // 63 (max label key length after slash) - len("version-schedule-plugin.") = 38
 const maxCombinedLength = 38
 
-var ocpVersionRe = regexp.MustCompile(`^v?(\d+)\.(\d+)`)
-
-// OCPVersion holds the parsed major and minor version of an OpenShift cluster.
-type OCPVersion struct {
-	Major int
-	Minor int
-}
-
 type ModuleValidator struct {
 	logger     logr.Logger
-	ocpVersion *OCPVersion
+	ocpVersion *version.OCPVersion
 }
 
-func NewModuleValidator(logger logr.Logger, ocpVersion *OCPVersion) *ModuleValidator {
+func NewModuleValidator(logger logr.Logger, ocpVersion *version.OCPVersion) *ModuleValidator {
 	return &ModuleValidator{logger: logger, ocpVersion: ocpVersion}
 }
 
-// DiscoverOCPVersion queries the OpenShift ClusterVersion resource and returns the cluster version.
-func DiscoverOCPVersion(cfg *rest.Config) (OCPVersion, error) {
-	ocpScheme := runtime.NewScheme()
-	if err := configv1.Install(ocpScheme); err != nil {
-		return OCPVersion{}, fmt.Errorf("failed to register OpenShift config scheme: %v", err)
-	}
-
-	cfgCopy := *cfg
-	cfgCopy.GroupVersion = &schema.GroupVersion{Group: "config.openshift.io", Version: "v1"}
-	cfgCopy.APIPath = "/apis"
-	cfgCopy.NegotiatedSerializer = serializer.NewCodecFactory(ocpScheme)
-
-	client, err := rest.RESTClientFor(&cfgCopy)
-	if err != nil {
-		return OCPVersion{}, fmt.Errorf("failed to create REST client for OpenShift config API: %v", err)
-	}
-
-	cv := &configv1.ClusterVersion{}
-	if err = client.Get().Resource("clusterversions").Name("version").Do(context.Background()).Into(cv); err != nil {
-		return OCPVersion{}, fmt.Errorf("failed to get ClusterVersion: %v", err)
-	}
-
-	return parseOCPVersion(cv.Status.Desired.Version)
-}
-
-// parseOCPVersion extracts the major and minor version from an OpenShift
-// version string such as "4.21.0" or "4.21.0-rc.1".
-func parseOCPVersion(version string) (OCPVersion, error) {
-	m := ocpVersionRe.FindStringSubmatch(version)
-	if m == nil {
-		return OCPVersion{}, fmt.Errorf("cannot parse OpenShift version from %q", version)
-	}
-	major, err := strconv.Atoi(m[1])
-	if err != nil {
-		return OCPVersion{}, fmt.Errorf("cannot parse OpenShift major version from %q: %v", version, err)
-	}
-	minor, err := strconv.Atoi(m[2])
-	if err != nil {
-		return OCPVersion{}, fmt.Errorf("cannot parse OpenShift minor version from %q: %v", version, err)
-	}
-	return OCPVersion{Major: major, Minor: minor}, nil
-}
-
 func (m *ModuleValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	// controller-runtime will set the path to `validate-<group>-<version>-<resource> so we
 	// need to make sure it is set correctly in the +kubebuilder annotation below.
@@ -157,7 +102,7 @@ func (m *ModuleValidator) ValidateDelete(ctx context.Context, obj runtime.Object
 	return nil, NotImplemented
 }
 
-func validateModule(mod *kmmv1beta1.Module, ocpVersion *OCPVersion) (admission.Warnings, error) {
+func validateModule(mod *kmmv1beta1.Module, ocpVersion *version.OCPVersion) (admission.Warnings, error) {
 	nameLength := len(mod.Name + mod.Namespace)
 
 	if nameLength > maxCombinedLength {
@@ -204,18 +149,18 @@ func validateModule(mod *kmmv1beta1.Module, ocpVersion *OCPVersion) (admission.W
 	return nil, validateFilesToSign(mod.Spec.ModuleLoader.Container)
 }
 
-func validateDRA(mod *kmmv1beta1.Module, ocpVersion *OCPVersion) error {
+func validateDRA(mod *kmmv1beta1.Module, ocpVersion *version.OCPVersion) error {
 	if mod.Spec.DRA == nil {
 		return nil
 	}
 
-	if ocpVersion != nil {
-		if ocpVersion.Major < constants.MinOCPMajorForDRA || (ocpVersion.Major == constants.MinOCPMajorForDRA && ocpVersion.Minor < constants.MinOCPMinorForDRA) {
-			return fmt.Errorf(
-				"spec.dra requires OpenShift %d.%d or later; current cluster version is %d.%d",
-				constants.MinOCPMajorForDRA, constants.MinOCPMinorForDRA, ocpVersion.Major, ocpVersion.Minor,
-			)
-		}
+	// Skip version gating when ocpVersion is nil (e.g. hub webhook validating
+	// a ManagedClusterModule — the spoke's own webhook will enforce its version).
+	if ocpVersion != nil && !ocpVersion.AtLeast(constants.MinOCPMajorForDRA, constants.MinOCPMinorForDRA) {
+		return fmt.Errorf(
+			"spec.dra requires OpenShift %d.%d or later; current cluster version is %d.%d",
+			constants.MinOCPMajorForDRA, constants.MinOCPMinorForDRA, ocpVersion.Major, ocpVersion.Minor,
+		)
 	}
 	driverName := mod.Spec.DRA.DriverName
 	if driverName == "" {
@@ -261,7 +206,7 @@ func validateHostPathVolumes(fieldPath string, volumes []corev1.Volume) error {
 	for i, vol := range volumes {
 		if vol.HostPath != nil && !isAllowedHostPath(vol.HostPath.Path) {
 			return fmt.Errorf(
-				"%s.volumes[%d]: hostPath %q is not allowed; only /dev, /sys, /var and /opt paths are permitted",
+				"%s.volumes[%d]: hostPath %q is not allowed; only /dev, /sys, /var, /opt and /run paths are permitted",
 				fieldPath, i, vol.HostPath.Path,
 			)
 		}