🌱 Add kubeadm cluster role (minimal backport of kubernetes-sigs#13664)

controlplane/kubeadm/internal/controllers/controller_test.go

@@ -34,8 +34,11 @@ import (
 	"github.com/pkg/errors"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/tools/clientcmd"
@@ -3743,6 +3746,15 @@ func TestObjectsPendingDelete(t *testing.T) {
 // test utils.
 
 func newFakeClient(initObjs ...client.Object) client.Client {
+	// Use a new scheme to avoid side effects if multiple tests are sharing the same global scheme.
+	scheme := runtime.NewScheme()
+	_ = appsv1.AddToScheme(scheme)
+	_ = corev1.AddToScheme(scheme)
+	_ = rbacv1.AddToScheme(scheme)
+	_ = apiextensionsv1.AddToScheme(scheme)
+	_ = clusterv1.AddToScheme(scheme)
+	_ = bootstrapv1.AddToScheme(scheme)
+	_ = controlplanev1.AddToScheme(scheme)
 	return &fakeClient{
 		startTime: time.Now(),
 		Client:    fake.NewClientBuilder().WithObjects(initObjs...).WithStatusSubresource(&controlplanev1.KubeadmControlPlane{}).Build(),

controlplane/kubeadm/internal/controllers/upgrade.go

@@ -69,8 +69,8 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane(
 		return ctrl.Result{}, errors.Wrap(err, "failed to set role and role binding for kubeadm")
 	}
 
-	// Ensure kubeadm clusterRoleBinding for v1.29+ as per https://github.com/kubernetes/kubernetes/pull/121305
-	if err := workloadCluster.AllowClusterAdminPermissions(ctx, parsedVersion); err != nil {
+	// Creates ClusterRoleBinding and ClusterRoles introduced by new versions of kubeadm.
+	if err := workloadCluster.EnsureKubeadmPermissions(ctx, parsedVersion); err != nil {
 		return ctrl.Result{}, errors.Wrap(err, "failed to set cluster-admin ClusterRoleBinding for kubeadm")
 	}
 

controlplane/kubeadm/internal/controllers/upgrade_test.go

@@ -88,6 +88,9 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleUp(t *testing.T) {
 			Management: &internal.Management{Client: env},
 			Workload: &fakeWorkloadCluster{
 				Status: internal.ClusterStatus{Nodes: 1},
+				Workload: &internal.Workload{
+					Client: env,
+				},
 			},
 		},
 		managementClusterUncached: &fakeManagementCluster{
@@ -229,6 +232,7 @@ func TestKubeadmControlPlaneReconciler_RolloutStrategy_ScaleDown(t *testing.T) {
 	}
 	fakeClient := newFakeClient(objs...)
 	fmc.Reader = fakeClient
+	fmc.Workload.Workload = &internal.Workload{Client: fakeClient}
 	r := &KubeadmControlPlaneReconciler{
 		Client:                    fakeClient,
 		SecretCachingClient:       fakeClient,

controlplane/kubeadm/internal/workload_cluster.go

@@ -114,7 +114,7 @@ type WorkloadCluster interface {
 	RemoveEtcdMemberForMachine(ctx context.Context, machine *clusterv1.Machine) error
 	ForwardEtcdLeadership(ctx context.Context, machine *clusterv1.Machine, leaderCandidate *clusterv1.Machine) error
 	AllowBootstrapTokensToGetNodes(ctx context.Context) error
-	AllowClusterAdminPermissions(ctx context.Context, version semver.Version) error
+	EnsureKubeadmPermissions(ctx context.Context, version semver.Version) error
 	UpdateClusterConfiguration(ctx context.Context, version semver.Version, mutators ...func(*bootstrapv1.ClusterConfiguration)) error
 
 	// State recovery tasks.

controlplane/kubeadm/internal/workload_cluster_rbac.go

@@ -53,6 +53,15 @@ const (
 
 	// UnversionedKubeletConfigMapName defines base kubelet configuration ConfigMap for kubeadm >= 1.24.
 	UnversionedKubeletConfigMapName = "kubelet-config"
+
+	// KubeletAPIAdminClusterRoleBindingName is the name of the ClusterRoleBinding for the apiserver kubelet client.
+	KubeletAPIAdminClusterRoleBindingName = "kubeadm:apiserver-kubelet-client"
+
+	// KubeletAPIAdminClusterRoleName is the name of the built-in ClusterRole for kubelet API access.
+	KubeletAPIAdminClusterRoleName = "system:kubelet-api-admin"
+
+	// APIServerKubeletClientCertCommonName defines kubelet client certificate common name (CN).
+	APIServerKubeletClientCertCommonName = "kube-apiserver-kubelet-client"
 )
 
 // EnsureResource creates a resoutce if the target resource doesn't exist. If the resource exists already, this function will ignore the resource instead.
@@ -73,15 +82,22 @@ func (w *Workload) EnsureResource(ctx context.Context, obj client.Object) error
 	return nil
 }
 
-// AllowClusterAdminPermissions creates ClusterRoleBinding rules to use the kubeadm:cluster-admins Cluster Role created in Kubeadm v1.29.
-func (w *Workload) AllowClusterAdminPermissions(ctx context.Context, targetVersion semver.Version) error {
-	// We intentionally only parse major/minor/patch so that the subsequent code
-	// also already applies to pre-release versions of new releases.
-	// Do nothing for Kubernetes < 1.29.
-	if version.Compare(targetVersion, semver.Version{Major: 1, Minor: 29, Patch: 0}, version.WithoutPreReleases()) < 0 {
+// EnsureKubeadmPermissions creates ClusterRoleBinding and ClusterRoles introduced by new versions of kubeadm.
+func (w *Workload) EnsureKubeadmPermissions(ctx context.Context, targetVersion semver.Version) error {
+	// Note: this code mimics the changes that kubeadm upgrade is doing.
+	// Cluster API must run the corresponding code when the user are upgrading to the minor where kubeadm introduced the change,
+	// including also patch releases. This is why the upper bound is the minor where a change was introduced plus one.
+	// Also, Cluster API applies new cluster roles when upgrading to releases older than when the changes
+	// have been introduced to kubeadm, so upgrade will keep working also in case the changes are backported to older versions.
+	if version.Compare(targetVersion, semver.Version{Major: 1, Minor: 38, Patch: 0}, version.WithoutPreReleases()) >= 0 {
 		return nil
 	}
-	return w.EnsureResource(ctx, &rbacv1.ClusterRoleBinding{
+
+	// Kubeadm added this role with K8s 1.29 when introducing
+	// a cleaner split between kubeadm:cluster-admins and system:masters.
+	// Rif https://github.com/kubernetes/kubernetes/pull/121305
+	// This change can be dropped when the min supported Kubernetes version in Cluster API >= 1.30.
+	err := w.EnsureResource(ctx, &rbacv1.ClusterRoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: ClusterAdminsGroupAndClusterRoleBinding,
 		},
@@ -96,8 +112,31 @@ func (w *Workload) AllowClusterAdminPermissions(ctx context.Context, targetVersi
 				Name: ClusterAdminsGroupAndClusterRoleBinding,
 			},
 		},
-	},
-	)
+	})
+	if err != nil {
+		return err
+	}
+
+	// Kubeadm introduced this role with K8s 1.37 when reducing
+	// the scope of the credential provided to the API server for accessing kubelet.
+	// Rif https://github.com/kubernetes/kubernetes/pull/138957.
+	// This change can be dropped when the min supported Kubernetes version in Cluster API >=1.38.
+	return w.EnsureResource(ctx, &rbacv1.ClusterRoleBinding{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: KubeletAPIAdminClusterRoleBindingName,
+		},
+		RoleRef: rbacv1.RoleRef{
+			APIGroup: rbacv1.GroupName,
+			Kind:     "ClusterRole",
+			Name:     KubeletAPIAdminClusterRoleName,
+		},
+		Subjects: []rbacv1.Subject{
+			{
+				Kind: rbacv1.UserKind,
+				Name: APIServerKubeletClientCertCommonName,
+			},
+		},
+	})
 }
 
 // AllowBootstrapTokensToGetNodes creates RBAC rules to allow Node Bootstrap Tokens to list nodes.

controlplane/kubeadm/internal/workload_cluster_rbac_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/blang/semver/v4"
+	"github.com/google/go-cmp/cmp"
 	. "github.com/onsi/gomega"
 	rbacv1 "k8s.io/api/rbac/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -227,3 +228,145 @@ func TestCluster_AllowBootstrapTokensToGetNodes_Error(t *testing.T) {
 		})
 	}
 }
+func TestEnsureKubeadmPermissions(t *testing.T) {
+	tests := []struct {
+		name          string
+		objs          []ctrlclient.Object
+		targetVersion semver.Version
+		wantObjs      []ctrlclient.Object
+	}{
+		{
+			name:          "Add kubeadm:cluster-admins and kubeadm:apiserver-kubelet-client ClusterRoleBinding for K8s <= 1.37",
+			objs:          nil,
+			targetVersion: semver.MustParse("1.37.5"),
+			wantObjs: []ctrlclient.Object{
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: ClusterAdminsGroupAndClusterRoleBinding,
+					},
+					RoleRef: rbacv1.RoleRef{
+						APIGroup: rbacv1.GroupName,
+						Kind:     "ClusterRole",
+						Name:     "cluster-admin",
+					},
+					Subjects: []rbacv1.Subject{
+						{
+							Kind: rbacv1.GroupKind,
+							Name: ClusterAdminsGroupAndClusterRoleBinding,
+						},
+					},
+				},
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: KubeletAPIAdminClusterRoleBindingName,
+					},
+					RoleRef: rbacv1.RoleRef{
+						APIGroup: rbacv1.GroupName,
+						Kind:     "ClusterRole",
+						Name:     KubeletAPIAdminClusterRoleName,
+					},
+					Subjects: []rbacv1.Subject{
+						{
+							Kind: rbacv1.UserKind,
+							Name: APIServerKubeletClientCertCommonName,
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "Ignore kubeadm:cluster-admins and kubeadm:apiserver-kubelet-client ClusterRoleBinding it they already exist for K8s <= 1.37 (kubeadm started adding those roles in patch versions)",
+			objs: []ctrlclient.Object{
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: ClusterAdminsGroupAndClusterRoleBinding,
+					},
+					// Intentionally using a different ClusterRoleBinding to check that it is not changed.
+				},
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: KubeletAPIAdminClusterRoleBindingName,
+					},
+					// Intentionally using a different ClusterRoleBinding to check that it is not changed.
+				},
+			},
+			targetVersion: semver.MustParse("1.37.0"),
+			wantObjs: []ctrlclient.Object{
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: ClusterAdminsGroupAndClusterRoleBinding,
+					},
+				},
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: KubeletAPIAdminClusterRoleBindingName,
+					},
+				},
+			},
+		},
+		{
+			name: "Ignore kubeadm:cluster-admins and kubeadm:apiserver-kubelet-client ClusterRoleBinding it they already exist for K8s >= 1.38 (kubeadm should add those roles or KCP in a previous update)",
+			objs: []ctrlclient.Object{
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: ClusterAdminsGroupAndClusterRoleBinding,
+					},
+					// Intentionally using a different ClusterRoleBinding to check that it is not changed.
+				},
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: KubeletAPIAdminClusterRoleBindingName,
+					},
+					// Intentionally using a different ClusterRoleBinding to check that it is not changed.
+				},
+			},
+			targetVersion: semver.MustParse("1.38.0"),
+			wantObjs: []ctrlclient.Object{
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: ClusterAdminsGroupAndClusterRoleBinding,
+					},
+				},
+				&rbacv1.ClusterRoleBinding{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: KubeletAPIAdminClusterRoleBindingName,
+					},
+				},
+			},
+		},
+		{
+			name:          "Do not add kubeadm:cluster-admins and kubeadm:apiserver-kubelet-client ClusterRoleBinding for K8s >= 1.38 (this should never happen, kubeadm should add those roles or KCP in a previous update)",
+			objs:          nil,
+			targetVersion: semver.MustParse("1.38.0"),
+			wantObjs:      nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			fakeClient := fake.NewClientBuilder().WithObjects(tt.objs...).Build()
+
+			w := &Workload{
+				Client: fakeClient,
+			}
+			err := w.EnsureKubeadmPermissions(ctx, tt.targetVersion)
+			g.Expect(err).ToNot(HaveOccurred())
+
+			crbList := &rbacv1.ClusterRoleBindingList{}
+			err = fakeClient.List(ctx, crbList)
+			g.Expect(err).ToNot(HaveOccurred())
+
+			g.Expect(crbList.Items).To(HaveLen(len(tt.wantObjs)))
+
+			for _, o := range tt.wantObjs {
+				obj := o.DeepCopyObject().(ctrlclient.Object)
+				err := fakeClient.Get(ctx, ctrlclient.ObjectKeyFromObject(obj), obj)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				o.SetResourceVersion(obj.GetResourceVersion())
+				g.Expect(obj).To(Equal(o), cmp.Diff(obj, o))
+			}
+		})
+	}
+}