Merge pull request rancher#43201 from maxsokolovsky/fix-cluster-versi…

…on-check-for-psp

Fix cluster version check for PSP and use it in pspdelete controller registration for RKE2

pkg/controllers/managementuser/pspdelete/pspdelete.go

@@ -2,12 +2,16 @@ package pspdelete
 
 import (
 	"context"
+	"errors"
+	"fmt"
 
 	v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
+	"github.com/rancher/rancher/pkg/controllers/managementuser/rbac/podsecuritypolicy"
 	"github.com/rancher/rancher/pkg/controllers/provisioningv2/cluster"
 	provisioningcontrollers "github.com/rancher/rancher/pkg/generated/controllers/provisioning.cattle.io/v1"
 	v1beta12 "github.com/rancher/rancher/pkg/generated/norman/policy/v1beta1"
 	"github.com/rancher/rancher/pkg/types/config"
+	"github.com/sirupsen/logrus"
 	"k8s.io/api/policy/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"
 )
@@ -25,6 +29,17 @@ type handler struct {
 
 func Register(ctx context.Context, userContext *config.UserContext) {
 	starter := userContext.DeferredStart(ctx, func(ctx context.Context) error {
+		clusterName := userContext.ClusterName
+		clusterLister := userContext.Management.Management.Clusters("").Controller().Lister()
+		err := podsecuritypolicy.CheckClusterVersion(clusterName, clusterLister)
+		if err != nil {
+			if errors.Is(err, podsecuritypolicy.ErrClusterVersionIncompatible) {
+				logrus.Debugf("%v - will not register pspdelete controller for cluster [%s].", err, clusterName)
+				return nil
+			}
+			return fmt.Errorf("unable to parse version of cluster %s: %w", clusterName, err)
+		}
+		logrus.Debugf("Cluster [%s] is compatible with PSPs, will run pspdelete controller.", clusterName)
 		registerDeferred(ctx, userContext)
 		return nil
 	})

pkg/controllers/managementuser/rbac/podsecuritypolicy/binding.go

@@ -94,12 +94,12 @@ func (l *lifecycle) sync(obj *v3.PodSecurityPolicyTemplateProjectBinding) (runti
 		return obj, nil
 	}
 
-	err := checkClusterVersion(l.clusterName, l.clusterLister)
+	err := CheckClusterVersion(l.clusterName, l.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for PodSecurityPolicyTemplateProjectBinding controller: %w", err)
 	}
 
 	podSecurityPolicyName := fmt.Sprintf("%v-psp", obj.PodSecurityPolicyTemplateName)

pkg/controllers/managementuser/rbac/podsecuritypolicy/cluster.go

@@ -61,9 +61,9 @@ func (m *clusterManager) sync(key string, obj *v3.Cluster) (runtime.Object, erro
 		return nil, nil
 	}
 
-	err := checkClusterVersion(m.clusterName, m.clusterLister)
+	err := CheckClusterVersion(m.clusterName, m.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			if obj.Status.AppliedPodSecurityPolicyTemplateName != "" {
 				obj = obj.DeepCopy()
 				obj.Status.AppliedPodSecurityPolicyTemplateName = ""
@@ -74,7 +74,7 @@ func (m *clusterManager) sync(key string, obj *v3.Cluster) (runtime.Object, erro
 			}
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for Cluster controller: %w", err)
 	}
 
 	if obj.Spec.DefaultPodSecurityPolicyTemplateName != "" {

pkg/controllers/managementuser/rbac/podsecuritypolicy/clusterrole.go

@@ -39,12 +39,12 @@ func (c *clusterRoleHandler) sync(key string, obj *v1.ClusterRole) (runtime.Obje
 		return obj, nil
 	}
 
-	err := checkClusterVersion(c.clusterName, c.clusterLister)
+	err := CheckClusterVersion(c.clusterName, c.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for ClusterRole controller: %w", err)
 	}
 
 	if templateID, ok := obj.Annotations[podSecurityPolicyTemplateParentAnnotation]; ok {

pkg/controllers/managementuser/rbac/podsecuritypolicy/deferred.go

@@ -3,6 +3,7 @@ package podsecuritypolicy
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 
 	apimgmtv3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
@@ -17,15 +18,15 @@ func Register(ctx context.Context, userContext *config.UserContext) {
 		clusterName := userContext.ClusterName
 		logrus.Infof("Checking cluster [%s] compatibility before registering podsecuritypolicy controllers.", clusterName)
 		clusterLister := userContext.Management.Management.Clusters("").Controller().Lister()
-		err := checkClusterVersion(clusterName, clusterLister)
+		err := CheckClusterVersion(clusterName, clusterLister)
 		if err != nil {
-			if errors.Is(err, errVersionIncompatible) {
-				logrus.Errorf("%v - will not register podsecuritypolicy controllers for cluster [%s].", err, clusterName)
+			if errors.Is(err, ErrClusterVersionIncompatible) {
+				logrus.Infof("%v - will not register podsecuritypolicy controllers for cluster [%s].", err, clusterName)
 				return nil
 			}
-			return err
+			return fmt.Errorf("unable to parse version of cluster %s: %w", clusterName, err)
 		}
-		logrus.Infof("cluster [%s] compatibility for podsecuritypolicy controllers check succeeded.", clusterName)
+		logrus.Infof("Cluster [%s] is compatible with PSPs, will run PSP controllers.", clusterName)
 		registerDeferred(ctx, userContext)
 		return nil
 	})

pkg/controllers/managementuser/rbac/podsecuritypolicy/namespace.go

@@ -43,12 +43,12 @@ func (m *namespaceManager) sync(key string, obj *v1.Namespace) (runtime.Object,
 		return nil, nil
 	}
 
-	err := checkClusterVersion(m.clusterName, m.clusterLister)
+	err := CheckClusterVersion(m.clusterName, m.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for Namespace controller: %w", err)
 	}
 
 	return nil, resyncServiceAccounts(m.serviceAccountLister, m.serviceAccountsController, obj.Name)

pkg/controllers/managementuser/rbac/podsecuritypolicy/podsecuritypolicy.go

@@ -39,12 +39,12 @@ func (p *pspHandler) sync(key string, obj *v1beta1.PodSecurityPolicy) (runtime.O
 		return obj, nil
 	}
 
-	err := checkClusterVersion(p.clusterName, p.clusterLister)
+	err := CheckClusterVersion(p.clusterName, p.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for PodSecurityPolicy controller: %w", err)
 	}
 
 	if templateID, ok := obj.Annotations[podSecurityPolicyTemplateParentAnnotation]; ok {

pkg/controllers/managementuser/rbac/podsecuritypolicy/serviceaccount.go

@@ -118,12 +118,12 @@ func (m *serviceAccountManager) sync(key string, obj *v1.ServiceAccount) (runtim
 		return nil, nil
 	}
 
-	err := checkClusterVersion(m.clusterName, m.clusterLister)
+	err := CheckClusterVersion(m.clusterName, m.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for ServiceAccount controller: %w", err)
 	}
 
 	namespace, err := m.namespaceLister.Get("", obj.Namespace)

pkg/controllers/managementuser/rbac/podsecuritypolicy/template.go

@@ -68,12 +68,12 @@ func (p *psptHandler) sync(key string, obj *v3.PodSecurityPolicyTemplate) (runti
 		return nil, nil
 	}
 
-	err := checkClusterVersion(p.clusterName, p.clusterLister)
+	err := CheckClusterVersion(p.clusterName, p.clusterLister)
 	if err != nil {
-		if errors.Is(err, errVersionIncompatible) {
+		if errors.Is(err, ErrClusterVersionIncompatible) {
 			return obj, nil
 		}
-		return obj, fmt.Errorf(clusterVersionCheckErrorString, err)
+		return obj, fmt.Errorf("error checking cluster version for PodSecurityPolicyTemplate controller: %w", err)
 	}
 
 	if _, ok := obj.Annotations[podSecurityPolicyTemplateUpgrade]; !ok {

pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go

@@ -4,25 +4,33 @@ import (
 	"errors"
 	"fmt"
 
-	mVersion "github.com/mcuadros/go-version"
 	v3 "github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3"
+	"k8s.io/apimachinery/pkg/util/version"
 )
 
-var errVersionIncompatible = errors.New("podsecuritypolicies are not available in Kubernetes v1.25 and above")
-var clusterVersionCheckErrorString = "failed to check cluster version compatibility for podsecuritypolicy controllers: %v"
+var ErrClusterVersionIncompatible = errors.New("podsecuritypolicies are not available in Kubernetes v1.25 and above")
 
-// checkClusterVersion tries to fetch a cluster by name, extract its Kubernetes version,
-// and check if the version is less than v1.25.
-func checkClusterVersion(clusterName string, clusterLister v3.ClusterLister) error {
+// CheckClusterVersion tries to fetch a management.cattle.io Cluster by name, extract its Kubernetes version,
+// and check if the cluster supports PodSecurityPolicies. If the version can be parsed, the function checks if it's
+// at least 1.25.x. If yes, it returns a special ErrClusterVersionIncompatible error.
+func CheckClusterVersion(clusterName string, clusterLister v3.ClusterLister) error {
 	cluster, err := clusterLister.Get("", clusterName)
 	if err != nil {
-		return fmt.Errorf("failed to get cluster [%s]: %w", clusterName, err)
+		return fmt.Errorf("failed to get cluster %s: %w", clusterName, err)
 	}
 	if cluster.Status.Version == nil {
-		return fmt.Errorf("cannot validate Kubernetes version for podsecuritypolicy capability: cluster [%s] status version is not available yet", clusterName)
+		return fmt.Errorf("cluster %s version is not available yet", clusterName)
 	}
-	if mVersion.Compare(cluster.Status.Version.String(), "v1.25", ">=") {
-		return errVersionIncompatible
+	v, err := version.ParseSemantic(cluster.Status.Version.String())
+	if err != nil {
+		return err
+	}
+	v125, err := version.ParseSemantic("v1.25.0")
+	if err != nil {
+		return err
+	}
+	if v.AtLeast(v125) {
+		return ErrClusterVersionIncompatible
 	}
 	return nil
 }

pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go

@@ -0,0 +1,156 @@
+package podsecuritypolicy_test
+
+import (
+	"fmt"
+	"testing"
+
+	v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3"
+	"github.com/rancher/rancher/pkg/controllers/managementuser/rbac/podsecuritypolicy"
+	"github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3/fakes"
+	"github.com/stretchr/testify/require"
+	"k8s.io/apimachinery/pkg/version"
+)
+
+func newClusterLister(kubernetesVersion string) *fakes.ClusterListerMock {
+	return &fakes.ClusterListerMock{
+		GetFunc: func(namespace, name string) (*v3.Cluster, error) {
+			if name == "invalid" {
+				return nil, fmt.Errorf("invalid cluster: %s", name)
+			} else if name == "not ready" {
+				return &v3.Cluster{Status: v3.ClusterStatus{}}, nil
+			} else {
+				return &v3.Cluster{Status: v3.ClusterStatus{Version: &version.Info{GitVersion: kubernetesVersion}}}, nil
+			}
+		},
+	}
+}
+
+func TestCheckClusterVersionFailsForVersionsThatCannotBeParsed(t *testing.T) {
+	t.Parallel()
+	tests := []string{"", "⌘⌘⌘", "v1.2", "v1.24", "v1.24.a", "1.24", "1.24.a", "v1.24+rke2r1"}
+	for _, v := range tests {
+		v := v
+		t.Run(v, func(t *testing.T) {
+			t.Parallel()
+			clusterLister := newClusterLister(v)
+			err := podsecuritypolicy.CheckClusterVersion("test", clusterLister)
+			require.Error(t, err)
+			require.NotErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible)
+		})
+	}
+}
+
+func TestCheckClusterVersionInspectsValidVersionsForCompatibilityWithPSP(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		version string
+		wantErr bool
+	}{
+		// regular version strings
+		{
+			version: "1.24.9",
+			wantErr: false,
+		},
+		{
+			version: "v1.24.9",
+			wantErr: false,
+		},
+		{
+			version: "1.25.9",
+			wantErr: true,
+		},
+		{
+			version: "v1.25.9",
+			wantErr: true,
+		},
+		{
+			version: "v1.26.9",
+			wantErr: true,
+		},
+		// k3s version strings
+		{
+			version: "v1.24.9+k3s1",
+			wantErr: false,
+		},
+		{
+			version: "v1.25.9+k3s1",
+			wantErr: true,
+		},
+		{
+			version: "v1.26.9+k3s1",
+			wantErr: true,
+		},
+		// rke1 version strings
+		{
+			version: "v1.24.9-rancher1-1",
+			wantErr: false,
+		},
+		{
+			version: "v1.25.9-rancher1-1",
+			wantErr: true,
+		},
+		{
+			version: "v1.26.9-rancher1-1",
+			wantErr: true,
+		},
+		// rke2 version strings
+		{
+			version: "v1.24.9+rke2r1",
+			wantErr: false,
+		},
+		{
+			version: "v1.25.9+rke2r1",
+			wantErr: true,
+		},
+		{
+			version: "v1.26.9+rke2r1",
+			wantErr: true,
+		},
+		// cloud provider version strings
+		{
+			version: "v1.27.3-gke.100",
+			wantErr: true,
+		},
+		{
+			version: "v1.26.9-eks-f8587cb",
+			wantErr: true,
+		},
+		{
+			version: "v1.24.9-eks-f8587cb",
+			wantErr: false,
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.version, func(t *testing.T) {
+			t.Parallel()
+			clusterLister := newClusterLister(tt.version)
+			err := podsecuritypolicy.CheckClusterVersion("test", clusterLister)
+			if tt.wantErr {
+				require.Error(t, err)
+				require.ErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible)
+			} else {
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestCheckClusterVersionFailsWhenItCannotFetchVersion(t *testing.T) {
+	t.Parallel()
+	t.Run("version check fails when it can't get cluster", func(t *testing.T) {
+		t.Parallel()
+		clusterLister := newClusterLister("")
+		err := podsecuritypolicy.CheckClusterVersion("invalid", clusterLister)
+		require.Error(t, err)
+		require.NotErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible)
+	})
+
+	t.Run("version check fails when the version is not yet known", func(t *testing.T) {
+		t.Parallel()
+		clusterLister := newClusterLister("")
+		err := podsecuritypolicy.CheckClusterVersion("not ready", clusterLister)
+		require.Error(t, err)
+		require.NotErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible)
+	})
+}