From 52e6d5e75b66b73466e34f7dac1ac6f67bd1bc8c Mon Sep 17 00:00:00 2001 From: Greg Sidelinger Date: Wed, 4 Oct 2023 12:16:46 -0400 Subject: [PATCH 1/3] Adding a version test suite for podsecuritypolicy Fxing podsecuritypolicy version checks for RKE2 Signed-off-by: Greg Sidelinger --- .../rbac/podsecuritypolicy/version.go | 5 +- .../rbac/podsecuritypolicy/version_test.go | 105 ++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go index b9f1cb3c3b5..7c093906a01 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go +++ b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go @@ -21,7 +21,10 @@ func checkClusterVersion(clusterName string, clusterLister v3.ClusterLister) 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) } - if mVersion.Compare(cluster.Status.Version.String(), "v1.25", ">=") { + if len(cluster.Status.Version.String()) < 5 { + return fmt.Errorf("cannot validate Kubernetes version for podsecuritypolicy capability: cluster [%s] status version [%s] is too small", clusterName, cluster.Status.Version.String()) + } + if mVersion.Compare(cluster.Status.Version.String()[0:5], "v1.25", ">=") { return errVersionIncompatible } return nil diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go new file mode 100644 index 00000000000..dca19064c30 --- /dev/null +++ b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go @@ -0,0 +1,105 @@ +package podsecuritypolicy + +import ( + "fmt" + "testing" + + apimgmtv3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" + mgmtv3 "github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3" + "github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3/fakes" + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/version" +) + +func newClusterListerWithVersion(kubernetesVersion string) *fakes.ClusterListerMock { + return &fakes.ClusterListerMock{ + GetFunc: func(namespace, name string) (*mgmtv3.Cluster, error) { + if name == "test" { + cluster := mgmtv3.Cluster{ + Status: apimgmtv3.ClusterStatus{ + Version: &version.Info{ + GitVersion: kubernetesVersion, + }, + }, + } + return &cluster, nil + } + return nil, fmt.Errorf("invalid cluster: %s", name) + }, + } +} + +func TestCheckClusterVersion(t *testing.T) { + + tests := []*struct { + version string + wantErr bool + setup func() + }{ + // tests for version string size + { + version: "", + wantErr: true, + }, + { + version: "v1.24", + wantErr: false, + }, + { + version: "v1.2", + wantErr: true, + }, + // rke1 version strings + { + version: "v1.24.9", + wantErr: false, + }, + { + 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, + }, + // rke2 version strings + { + version: "v1.24.9+rke2r1", + wantErr: false, + }, + { + version: "v1.25.9+rke2r1", + wantErr: true, + }, + { + version: "v1.26.9+rke2r1", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.version, func(t *testing.T) { + clusterLister := newClusterListerWithVersion(tt.version) + println(tt.version) + err := checkClusterVersion("test", clusterLister) + if tt.wantErr { + println(err.Error()) + assert.Error(t, err, "Expected checkClusterVersion to raise error.") + return + } + }) + } + +} From f2bdf8595740d6f42d60db5b643c6f8b92e918c0 Mon Sep 17 00:00:00 2001 From: Max Sokolovsky Date: Fri, 13 Oct 2023 10:42:10 -0400 Subject: [PATCH 2/3] Rewrite cluster version check for PSPs by using a k8s.io package --- .../rbac/podsecuritypolicy/version.go | 20 ++-- .../rbac/podsecuritypolicy/version_test.go | 104 ++++++++++++------ 2 files changed, 84 insertions(+), 40 deletions(-) diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go index 7c093906a01..e2a2a2920ea 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go +++ b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go @@ -4,27 +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" + +const clusterVersionCheckErrorString = "failed to check cluster version compatibility for podsecuritypolicy controllers: %v" // 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 { 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) + } + v, err := version.ParseSemantic(cluster.Status.Version.String()) + if err != nil { + return err } - if len(cluster.Status.Version.String()) < 5 { - return fmt.Errorf("cannot validate Kubernetes version for podsecuritypolicy capability: cluster [%s] status version [%s] is too small", clusterName, cluster.Status.Version.String()) + v125, err := version.ParseSemantic("v1.25.0") + if err != nil { + return err } - if mVersion.Compare(cluster.Status.Version.String()[0:5], "v1.25", ">=") { + if v.AtLeast(v125) { return errVersionIncompatible } return nil diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go index dca19064c30..08703575386 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go +++ b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go @@ -4,56 +4,60 @@ import ( "fmt" "testing" - apimgmtv3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" - mgmtv3 "github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3" + v3 "github.com/rancher/rancher/pkg/apis/management.cattle.io/v3" "github.com/rancher/rancher/pkg/generated/norman/management.cattle.io/v3/fakes" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/version" ) -func newClusterListerWithVersion(kubernetesVersion string) *fakes.ClusterListerMock { +func newClusterLister(kubernetesVersion string) *fakes.ClusterListerMock { return &fakes.ClusterListerMock{ - GetFunc: func(namespace, name string) (*mgmtv3.Cluster, error) { - if name == "test" { - cluster := mgmtv3.Cluster{ - Status: apimgmtv3.ClusterStatus{ - Version: &version.Info{ - GitVersion: kubernetesVersion, - }, - }, - } - return &cluster, nil + 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 } - return nil, fmt.Errorf("invalid cluster: %s", name) }, } } -func TestCheckClusterVersion(t *testing.T) { +func TestCheckClusterVersionFailsForVersionsThatCannotBeParsed(t *testing.T) { + t.Parallel() + tests := []string{"", "⌘⌘⌘", "v1.2", "v1.24", "v1.24.a", "1.24", "1.24.a"} + for _, v := range tests { + v := v + t.Run(v, func(t *testing.T) { + t.Parallel() + clusterLister := newClusterLister(v) + err := checkClusterVersion("test", clusterLister) + require.Error(t, err) + require.NotErrorIs(t, err, errVersionIncompatible) + }) + } +} - tests := []*struct { +func TestCheckClusterVersionInspectsValidVersionsForCompatibilityWithPSP(t *testing.T) { + t.Parallel() + tests := []struct { version string wantErr bool - setup func() }{ - // tests for version string size + // regular version strings { - version: "", - wantErr: true, + version: "1.24.9", + wantErr: false, }, { - version: "v1.24", + version: "v1.24.9", wantErr: false, }, { - version: "v1.2", + version: "1.25.9", wantErr: true, }, - // rke1 version strings - { - version: "v1.24.9", - wantErr: false, - }, { version: "v1.25.9", wantErr: true, @@ -75,6 +79,19 @@ func TestCheckClusterVersion(t *testing.T) { 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", @@ -90,16 +107,37 @@ func TestCheckClusterVersion(t *testing.T) { }, } for _, tt := range tests { + tt := tt t.Run(tt.version, func(t *testing.T) { - clusterLister := newClusterListerWithVersion(tt.version) - println(tt.version) + t.Parallel() + clusterLister := newClusterLister(tt.version) err := checkClusterVersion("test", clusterLister) if tt.wantErr { - println(err.Error()) - assert.Error(t, err, "Expected checkClusterVersion to raise error.") - return + require.Error(t, err) + require.ErrorIs(t, err, errVersionIncompatible) + } + if !tt.wantErr { + 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 := checkClusterVersion("invalid", clusterLister) + require.Error(t, err) + require.NotErrorIs(t, err, errVersionIncompatible) + }) + t.Run("version check fails when the version is not yet known", func(t *testing.T) { + t.Parallel() + clusterLister := newClusterLister("") + err := checkClusterVersion("not ready", clusterLister) + require.Error(t, err) + require.NotErrorIs(t, err, errVersionIncompatible) + }) } From df6f162e25aa5fc56d2caf9ebe8e666f6e70bd46 Mon Sep 17 00:00:00 2001 From: Max Sokolovsky Date: Tue, 17 Oct 2023 19:13:57 -0400 Subject: [PATCH 3/3] Do not register pspdelete controller if the cluster version is >= 1.25 --- .../managementuser/pspdelete/pspdelete.go | 15 ++++++++ .../rbac/podsecuritypolicy/binding.go | 6 +-- .../rbac/podsecuritypolicy/cluster.go | 6 +-- .../rbac/podsecuritypolicy/clusterrole.go | 6 +-- .../rbac/podsecuritypolicy/deferred.go | 11 +++--- .../rbac/podsecuritypolicy/namespace.go | 6 +-- .../podsecuritypolicy/podsecuritypolicy.go | 6 +-- .../rbac/podsecuritypolicy/serviceaccount.go | 6 +-- .../rbac/podsecuritypolicy/template.go | 6 +-- .../rbac/podsecuritypolicy/version.go | 13 +++---- .../rbac/podsecuritypolicy/version_test.go | 37 +++++++++++++------ 11 files changed, 73 insertions(+), 45 deletions(-) diff --git a/pkg/controllers/managementuser/pspdelete/pspdelete.go b/pkg/controllers/managementuser/pspdelete/pspdelete.go index 4da0f441531..2b36bc800ab 100644 --- a/pkg/controllers/managementuser/pspdelete/pspdelete.go +++ b/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 }) diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/binding.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/binding.go index d5691514ed0..120b43087b7 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/binding.go +++ b/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) diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/cluster.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/cluster.go index 6cf69190050..1898480346d 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/cluster.go +++ b/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 != "" { diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/clusterrole.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/clusterrole.go index ab7ff2851d9..4b9f2dc6a62 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/clusterrole.go +++ b/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 { diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/deferred.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/deferred.go index 62ec1d4d754..1a26862872a 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/deferred.go +++ b/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 }) diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/namespace.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/namespace.go index 4704b43a5df..51a589eb7b3 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/namespace.go +++ b/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) diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/podsecuritypolicy.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/podsecuritypolicy.go index 77adc849944..dbd9a496a3c 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/podsecuritypolicy.go +++ b/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 { diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/serviceaccount.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/serviceaccount.go index 3da2fd39bd0..f37ab88c8ad 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/serviceaccount.go +++ b/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) diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/template.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/template.go index 37219d7bcbb..2769d2d2b82 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/template.go +++ b/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 { diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go index e2a2a2920ea..0187aa1f9d9 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go +++ b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version.go @@ -8,13 +8,12 @@ import ( "k8s.io/apimachinery/pkg/util/version" ) -var errVersionIncompatible = errors.New("podsecuritypolicies are not available in Kubernetes v1.25 and above") +var ErrClusterVersionIncompatible = errors.New("podsecuritypolicies are not available in Kubernetes v1.25 and above") -const clusterVersionCheckErrorString = "failed to check cluster version compatibility for podsecuritypolicy controllers: %v" - -// 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) @@ -31,7 +30,7 @@ func checkClusterVersion(clusterName string, clusterLister v3.ClusterLister) err return err } if v.AtLeast(v125) { - return errVersionIncompatible + return ErrClusterVersionIncompatible } return nil } diff --git a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go index 08703575386..d9d6bab1a24 100644 --- a/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go +++ b/pkg/controllers/managementuser/rbac/podsecuritypolicy/version_test.go @@ -1,10 +1,11 @@ -package podsecuritypolicy +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" @@ -26,15 +27,15 @@ func newClusterLister(kubernetesVersion string) *fakes.ClusterListerMock { func TestCheckClusterVersionFailsForVersionsThatCannotBeParsed(t *testing.T) { t.Parallel() - tests := []string{"", "⌘⌘⌘", "v1.2", "v1.24", "v1.24.a", "1.24", "1.24.a"} + 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 := checkClusterVersion("test", clusterLister) + err := podsecuritypolicy.CheckClusterVersion("test", clusterLister) require.Error(t, err) - require.NotErrorIs(t, err, errVersionIncompatible) + require.NotErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible) }) } } @@ -105,18 +106,30 @@ func TestCheckClusterVersionInspectsValidVersionsForCompatibilityWithPSP(t *test 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 := checkClusterVersion("test", clusterLister) + err := podsecuritypolicy.CheckClusterVersion("test", clusterLister) if tt.wantErr { require.Error(t, err) - require.ErrorIs(t, err, errVersionIncompatible) - } - if !tt.wantErr { + require.ErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible) + } else { require.NoError(t, err) } }) @@ -128,16 +141,16 @@ func TestCheckClusterVersionFailsWhenItCannotFetchVersion(t *testing.T) { t.Run("version check fails when it can't get cluster", func(t *testing.T) { t.Parallel() clusterLister := newClusterLister("") - err := checkClusterVersion("invalid", clusterLister) + err := podsecuritypolicy.CheckClusterVersion("invalid", clusterLister) require.Error(t, err) - require.NotErrorIs(t, err, errVersionIncompatible) + 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 := checkClusterVersion("not ready", clusterLister) + err := podsecuritypolicy.CheckClusterVersion("not ready", clusterLister) require.Error(t, err) - require.NotErrorIs(t, err, errVersionIncompatible) + require.NotErrorIs(t, err, podsecuritypolicy.ErrClusterVersionIncompatible) }) }