Merge pull request #61866 from davidz627/fix/CSIe2e

Automatic merge from submit-queue (batch tested with PRs 62192, 61866, 62206, 62360). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add CSI External Components ClusterRole to bootstrapped roles 

Added CSI External Components ClusterRole to bootstrapped roles and removed creation from failing e2e test

Fixes: #61781

/sig storage
/kind bug
/assign @liggitt @saad-ali 

```release-note
NONE
```

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go

@@ -458,6 +458,26 @@ func ClusterRoles() []rbac.ClusterRole {
 				eventsRule(),
 			},
 		},
+		{
+			// a role for the csi external provisioner
+			ObjectMeta: metav1.ObjectMeta{Name: "system:csi-external-provisioner"},
+			Rules: []rbac.PolicyRule{
+				rbac.NewRule("create", "delete", "list", "watch").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(),
+				rbac.NewRule("get", "list", "watch", "update", "patch").Groups(legacyGroup).Resources("persistentvolumeclaims").RuleOrDie(),
+				rbac.NewRule("list", "watch").Groups(storageGroup).Resources("storageclasses").RuleOrDie(),
+				rbac.NewRule("get", "list", "watch", "create", "update", "patch").Groups(legacyGroup).Resources("events").RuleOrDie(),
+			},
+		},
+		{
+			// a role for the csi external attacher
+			ObjectMeta: metav1.ObjectMeta{Name: "system:csi-external-attacher"},
+			Rules: []rbac.PolicyRule{
+				rbac.NewRule("get", "list", "watch", "update", "patch").Groups(legacyGroup).Resources("persistentvolumes").RuleOrDie(),
+				rbac.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
+				rbac.NewRule("get", "list", "watch", "update", "patch").Groups(storageGroup).Resources("volumeattachments").RuleOrDie(),
+				rbac.NewRule("get", "list", "watch", "create", "update", "patch").Groups(legacyGroup).Resources("events").RuleOrDie(),
+			},
+		},
 		{
 			ObjectMeta: metav1.ObjectMeta{Name: "system:aws-cloud-provider"},
 			Rules: []rbac.PolicyRule{

plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml

@@ -619,6 +619,102 @@ items:
     - certificatesigningrequests/selfnodeclient
     verbs:
     - create
+- apiVersion: rbac.authorization.k8s.io/v1
+  kind: ClusterRole
+  metadata:
+    annotations:
+      rbac.authorization.kubernetes.io/autoupdate: "true"
+    creationTimestamp: null
+    labels:
+      kubernetes.io/bootstrapping: rbac-defaults
+    name: system:csi-external-attacher
+  rules:
+  - apiGroups:
+    - ""
+    resources:
+    - persistentvolumes
+    verbs:
+    - get
+    - list
+    - patch
+    - update
+    - watch
+  - apiGroups:
+    - ""
+    resources:
+    - nodes
+    verbs:
+    - get
+    - list
+    - watch
+  - apiGroups:
+    - storage.k8s.io
+    resources:
+    - volumeattachments
+    verbs:
+    - get
+    - list
+    - patch
+    - update
+    - watch
+  - apiGroups:
+    - ""
+    resources:
+    - events
+    verbs:
+    - create
+    - get
+    - list
+    - patch
+    - update
+    - watch
+- apiVersion: rbac.authorization.k8s.io/v1
+  kind: ClusterRole
+  metadata:
+    annotations:
+      rbac.authorization.kubernetes.io/autoupdate: "true"
+    creationTimestamp: null
+    labels:
+      kubernetes.io/bootstrapping: rbac-defaults
+    name: system:csi-external-provisioner
+  rules:
+  - apiGroups:
+    - ""
+    resources:
+    - persistentvolumes
+    verbs:
+    - create
+    - delete
+    - list
+    - watch
+  - apiGroups:
+    - ""
+    resources:
+    - persistentvolumeclaims
+    verbs:
+    - get
+    - list
+    - patch
+    - update
+    - watch
+  - apiGroups:
+    - storage.k8s.io
+    resources:
+    - storageclasses
+    verbs:
+    - list
+    - watch
+  - apiGroups:
+    - ""
+    resources:
+    - events
+    verbs:
+    - create
+    - get
+    - list
+    - patch
+    - update
+    - watch
 - apiVersion: rbac.authorization.k8s.io/v1
   kind: ClusterRole
   metadata:

test/e2e/storage/BUILD

@@ -76,6 +76,7 @@ go_library(
         "//vendor/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes/typed/core/v1:go_default_library",
+        "//vendor/k8s.io/client-go/rest:go_default_library",
     ],
 )
 

test/e2e/storage/csi_volumes.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	clientset "k8s.io/client-go/kubernetes"
+	restclient "k8s.io/client-go/rest"
 	"k8s.io/kubernetes/test/e2e/framework"
 	"k8s.io/kubernetes/test/e2e/storage/utils"
 
@@ -34,17 +35,68 @@ import (
 )
 
 const (
-	csiExternalAttacherImage    string = "quay.io/k8scsi/csi-attacher:v0.2.0"
+	csiExternalAttacherImage              string = "quay.io/k8scsi/csi-attacher:v0.2.0"
-	csiExternalProvisionerImage string = "quay.io/k8scsi/csi-provisioner:v0.2.0"
+	csiExternalProvisionerImage           string = "quay.io/k8scsi/csi-provisioner:v0.2.0"
-	csiDriverRegistrarImage     string = "quay.io/k8scsi/driver-registrar:v0.2.0"
+	csiDriverRegistrarImage               string = "quay.io/k8scsi/driver-registrar:v0.2.0"
+	csiExternalProvisionerClusterRoleName string = "system:csi-external-provisioner"
+	csiExternalAttacherClusterRoleName    string = "system:csi-external-attacher"
+	csiDriverRegistrarClusterRoleName     string = "csi-driver-registrar"
 )
 
+// Create the driver registrar cluster role if it doesn't exist, no teardown so that tests
+// are parallelizable. This role will be shared with many of the CSI tests.
+func csiDriverRegistrarClusterRole(
+	config framework.VolumeTestConfig,
+) *rbacv1.ClusterRole {
+	// TODO(Issue: #62237) Remove impersonation workaround and cluster role when issue resolved
+	By("Creating an impersonating superuser kubernetes clientset to define cluster role")
+	rc, err := framework.LoadConfig()
+	framework.ExpectNoError(err)
+	rc.Impersonate = restclient.ImpersonationConfig{
+		UserName: "superuser",
+		Groups:   []string{"system:masters"},
+	}
+	superuserClientset, err := clientset.NewForConfig(rc)
+	By("Creating the CSI driver registrar cluster role")
+	clusterRoleClient := superuserClientset.RbacV1().ClusterRoles()
+	role := &rbacv1.ClusterRole{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: csiDriverRegistrarClusterRoleName,
+		},
+		Rules: []rbacv1.PolicyRule{
+
+			{
+				APIGroups: []string{""},
+				Resources: []string{"events"},
+				Verbs:     []string{"get", "list", "watch", "create", "update", "patch"},
+			},
+			{
+				APIGroups: []string{""},
+				Resources: []string{"nodes"},
+				Verbs:     []string{"get", "update", "patch"},
+			},
+		},
+	}
+
+	ret, err := clusterRoleClient.Create(role)
+	if err != nil {
+		if apierrs.IsAlreadyExists(err) {
+			return ret
+		}
+		framework.ExpectNoError(err, "Failed to create %s cluster role: %v", role.GetName(), err)
+	}
+
+	return ret
+}
+
 func csiServiceAccount(
 	client clientset.Interface,
 	config framework.VolumeTestConfig,
+	componentName string,
 	teardown bool,
 ) *v1.ServiceAccount {
-	serviceAccountName := config.Prefix + "-service-account"
+	By("Creating a CSI service account")
+	serviceAccountName := config.Prefix + "-" + componentName + "-service-account"
 	serviceAccountClient := client.CoreV1().ServiceAccounts(config.Namespace)
 	sa := &v1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
@@ -71,117 +123,51 @@ func csiServiceAccount(
 	return ret
 }
 
-func csiClusterRole(
+func csiClusterRoleBindings(
-	client clientset.Interface,
-	config framework.VolumeTestConfig,
-	teardown bool,
-) *rbacv1.ClusterRole {
-	clusterRoleClient := client.RbacV1().ClusterRoles()
-	role := &rbacv1.ClusterRole{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: config.Prefix + "-cluster-role",
-		},
-		Rules: []rbacv1.PolicyRule{
-			{
-				APIGroups: []string{""},
-				Resources: []string{"persistentvolumes"},
-				Verbs:     []string{"create", "delete", "get", "list", "watch", "update"},
-			},
-			{
-				APIGroups: []string{""},
-				Resources: []string{"persistentvolumeclaims"},
-				Verbs:     []string{"get", "list", "watch", "update"},
-			},
-			{
-				APIGroups: []string{""},
-				Resources: []string{"events"},
-				Verbs:     []string{"get", "list", "watch", "create", "update", "patch"},
-			},
-			{
-				APIGroups: []string{""},
-				Resources: []string{"secrets"},
-				Verbs:     []string{"get", "list"},
-			},
-			{
-				APIGroups: []string{""},
-				Resources: []string{"nodes"},
-				Verbs:     []string{"get", "list", "watch", "update"},
-			},
-			{
-				APIGroups: []string{"storage.k8s.io"},
-				Resources: []string{"volumeattachments"},
-				Verbs:     []string{"get", "list", "watch", "update"},
-			},
-			{
-				APIGroups: []string{"storage.k8s.io"},
-				Resources: []string{"storageclasses"},
-				Verbs:     []string{"get", "list", "watch"},
-			},
-		},
-	}
-
-	clusterRoleClient.Delete(role.GetName(), &metav1.DeleteOptions{})
-	err := wait.Poll(2*time.Second, 10*time.Minute, func() (bool, error) {
-		_, err := clusterRoleClient.Get(role.GetName(), metav1.GetOptions{})
-		return apierrs.IsNotFound(err), nil
-	})
-	framework.ExpectNoError(err, "Timed out waiting for deletion: %v", err)
-
-	if teardown {
-		return nil
-	}
-
-	ret, err := clusterRoleClient.Create(role)
-	if err != nil {
-		framework.ExpectNoError(err, "Failed to create %s cluster role: %v", role.GetName(), err)
-	}
-
-	return ret
-}
-
-func csiClusterRoleBinding(
 	client clientset.Interface,
 	config framework.VolumeTestConfig,
 	teardown bool,
 	sa *v1.ServiceAccount,
-	clusterRole *rbacv1.ClusterRole,
+	clusterRolesNames []string,
-) *rbacv1.ClusterRoleBinding {
+) {
+	By("Binding cluster roles to the CSI service account")
 	clusterRoleBindingClient := client.RbacV1().ClusterRoleBindings()
-	binding := &rbacv1.ClusterRoleBinding{
+	for _, clusterRoleName := range clusterRolesNames {
-		ObjectMeta: metav1.ObjectMeta{
+
-			Name: config.Prefix + "-role-binding",
+		binding := &rbacv1.ClusterRoleBinding{
-		},
+			ObjectMeta: metav1.ObjectMeta{
-		Subjects: []rbacv1.Subject{
+				Name: config.Prefix + "-" + clusterRoleName + "-" + config.Namespace + "-role-binding",
-			{
-				Kind:      "ServiceAccount",
-				Name:      sa.GetName(),
-				Namespace: sa.GetNamespace(),
 			},
-		},
+			Subjects: []rbacv1.Subject{
-		RoleRef: rbacv1.RoleRef{
+				{
-			Kind:     "ClusterRole",
+					Kind:      "ServiceAccount",
-			Name:     clusterRole.GetName(),
+					Name:      sa.GetName(),
-			APIGroup: "rbac.authorization.k8s.io",
+					Namespace: sa.GetNamespace(),
-		},
+				},
+			},
+			RoleRef: rbacv1.RoleRef{
+				Kind:     "ClusterRole",
+				Name:     clusterRoleName,
+				APIGroup: "rbac.authorization.k8s.io",
+			},
+		}
+
+		clusterRoleBindingClient.Delete(binding.GetName(), &metav1.DeleteOptions{})
+		err := wait.Poll(2*time.Second, 10*time.Minute, func() (bool, error) {
+			_, err := clusterRoleBindingClient.Get(binding.GetName(), metav1.GetOptions{})
+			return apierrs.IsNotFound(err), nil
+		})
+		framework.ExpectNoError(err, "Timed out waiting for deletion: %v", err)
+
+		if teardown {
+			return
+		}
+
+		_, err = clusterRoleBindingClient.Create(binding)
+		if err != nil {
+			framework.ExpectNoError(err, "Failed to create %s role binding: %v", binding.GetName(), err)
+		}
 	}
-
-	clusterRoleBindingClient.Delete(binding.GetName(), &metav1.DeleteOptions{})
-	err := wait.Poll(2*time.Second, 10*time.Minute, func() (bool, error) {
-		_, err := clusterRoleBindingClient.Get(binding.GetName(), metav1.GetOptions{})
-		return apierrs.IsNotFound(err), nil
-	})
-	framework.ExpectNoError(err, "Timed out waiting for deletion: %v", err)
-
-	if teardown {
-		return nil
-	}
-
-	ret, err := clusterRoleBindingClient.Create(binding)
-	if err != nil {
-		framework.ExpectNoError(err, "Failed to create %s role binding: %v", binding.GetName(), err)
-	}
-
-	return ret
 }
 
 var _ = utils.SIGDescribe("CSI Volumes [Flaky]", func() {
@@ -206,31 +192,33 @@ var _ = utils.SIGDescribe("CSI Volumes [Flaky]", func() {
 			ServerNodeName:    node.Name,
 			WaitForCompletion: true,
 		}
+		csiDriverRegistrarClusterRole(config)
 	})
 
 	// Create one of these for each of the drivers to be tested
 	// CSI hostPath driver test
 	Describe("Sanity CSI plugin test using hostPath CSI driver", func() {
-
 		var (
-			clusterRole    *rbacv1.ClusterRole
+			serviceAccount           *v1.ServiceAccount
-			serviceAccount *v1.ServiceAccount
+			combinedClusterRoleNames []string = []string{
+				csiExternalAttacherClusterRoleName,
+				csiExternalProvisionerClusterRoleName,
+				csiDriverRegistrarClusterRoleName,
+			}
 		)
 
 		BeforeEach(func() {
 			By("deploying csi hostpath driver")
-			clusterRole = csiClusterRole(cs, config, false)
+			serviceAccount = csiServiceAccount(cs, config, "hostpath", false)
-			serviceAccount = csiServiceAccount(cs, config, false)
+			csiClusterRoleBindings(cs, config, false, serviceAccount, combinedClusterRoleNames)
-			csiClusterRoleBinding(cs, config, false, serviceAccount, clusterRole)
 			csiHostPathPod(cs, config, false, f, serviceAccount)
 		})
 
 		AfterEach(func() {
 			By("uninstalling csi hostpath driver")
 			csiHostPathPod(cs, config, true, f, serviceAccount)
-			csiClusterRoleBinding(cs, config, true, serviceAccount, clusterRole)
+			csiClusterRoleBindings(cs, config, true, serviceAccount, combinedClusterRoleNames)
-			serviceAccount = csiServiceAccount(cs, config, true)
+			csiServiceAccount(cs, config, "hostpath", true)
-			clusterRole = csiClusterRole(cs, config, true)
 		})
 
 		It("should provision storage with a hostPath CSI driver", func() {