Merge pull request #79309 from draveness/feature/cleanup-CSIPersistentVolume-feature-gates

feat: cleanup feature gates for CSIPersistentVolume

cmd/kube-controller-manager/app/plugins.go

@@ -77,9 +77,7 @@ func ProbeAttachableVolumePlugins() []volume.VolumePlugin {
 	allPlugins = append(allPlugins, fc.ProbeVolumePlugins()...)
 	allPlugins = append(allPlugins, iscsi.ProbeVolumePlugins()...)
 	allPlugins = append(allPlugins, rbd.ProbeVolumePlugins()...)
-	if utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
+	allPlugins = append(allPlugins, csi.ProbeVolumePlugins()...)
-		allPlugins = append(allPlugins, csi.ProbeVolumePlugins()...)
-	}
 	return allPlugins
 }
 

cmd/kubelet/app/plugins.go

@@ -52,11 +52,9 @@ import (
 	"k8s.io/kubernetes/pkg/volume/secret"
 	"k8s.io/kubernetes/pkg/volume/storageos"
 	"k8s.io/kubernetes/pkg/volume/vsphere_volume"
+
 	// Cloud providers
 	_ "k8s.io/kubernetes/pkg/cloudprovider/providers"
-	// features check
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
-	"k8s.io/kubernetes/pkg/features"
 )
 
 // ProbeVolumePlugins collects all volume plugins into an easy to use list.
@@ -94,9 +92,7 @@ func ProbeVolumePlugins() []volume.VolumePlugin {
 	allPlugins = append(allPlugins, scaleio.ProbeVolumePlugins()...)
 	allPlugins = append(allPlugins, local.ProbeVolumePlugins()...)
 	allPlugins = append(allPlugins, storageos.ProbeVolumePlugins()...)
-	if utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
+	allPlugins = append(allPlugins, csi.ProbeVolumePlugins()...)
-		allPlugins = append(allPlugins, csi.ProbeVolumePlugins()...)
-	}
 	return allPlugins
 }
 

pkg/apis/core/validation/validation_test.go

@@ -2037,8 +2037,6 @@ func TestValidateCSIVolumeSource(t *testing.T) {
 		},
 	}
 
-	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CSIPersistentVolume, true)()
-
 	for i, tc := range testCases {
 		errs := validateCSIPersistentVolumeSource(tc.csi, field.NewPath("field"))
 

pkg/features/kube_features.go

@@ -180,12 +180,6 @@ const (
 	// Enable running mount utilities in containers.
 	MountContainers featuregate.Feature = "MountContainers"
 
-	// owner: @vladimirvivien
-	// GA: v1.13
-	//
-	// Enable mount/attachment of Container Storage Interface (CSI) backed PVs
-	CSIPersistentVolume featuregate.Feature = "CSIPersistentVolume"
-
 	// owner: @saad-ali
 	// alpha: v1.12
 	// beta:  v1.14
@@ -487,7 +481,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 	CPUCFSQuotaPeriod:                           {Default: false, PreRelease: featuregate.Alpha},
 	ServiceNodeExclusion:                        {Default: false, PreRelease: featuregate.Alpha},
 	MountContainers:                             {Default: false, PreRelease: featuregate.Alpha},
-	CSIPersistentVolume:                         {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.16
 	CSIDriverRegistry:                           {Default: true, PreRelease: featuregate.Beta},
 	CSINodeInfo:                                 {Default: true, PreRelease: featuregate.Beta},
 	BlockVolume:                                 {Default: true, PreRelease: featuregate.Beta},

plugin/pkg/auth/authorizer/node/graph_populator.go

@@ -64,13 +64,11 @@ func AddGraphEventHandlers(
 		DeleteFunc: g.deletePV,
 	})
 
-	if utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
+	attachments.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
-		attachments.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc:    g.addVolumeAttachment,
-			AddFunc:    g.addVolumeAttachment,
+		UpdateFunc: g.updateVolumeAttachment,
-			UpdateFunc: g.updateVolumeAttachment,
+		DeleteFunc: g.deleteVolumeAttachment,
-			DeleteFunc: g.deleteVolumeAttachment,
+	})
-		})
-	}
 }
 
 func (g *graphPopulator) addNode(obj interface{}) {

plugin/pkg/auth/authorizer/node/node_authorizer.go

@@ -108,10 +108,7 @@ func (r *NodeAuthorizer) Authorize(attrs authorizer.Attributes) (authorizer.Deci
 		case pvResource:
 			return r.authorizeGet(nodeName, pvVertexType, attrs)
 		case vaResource:
-			if r.features.Enabled(features.CSIPersistentVolume) {
+			return r.authorizeGet(nodeName, vaVertexType, attrs)
-				return r.authorizeGet(nodeName, vaVertexType, attrs)
-			}
-			return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.CSIPersistentVolume), nil
 		case svcAcctResource:
 			if r.features.Enabled(features.TokenRequest) {
 				return r.authorizeCreateToken(nodeName, serviceAccountVertexType, attrs)

plugin/pkg/auth/authorizer/node/node_authorizer_test.go

@@ -40,8 +40,6 @@ import (
 )
 
 var (
-	csiEnabledFeature          = featuregate.NewFeatureGate()
-	csiDisabledFeature         = featuregate.NewFeatureGate()
 	trEnabledFeature           = featuregate.NewFeatureGate()
 	trDisabledFeature          = featuregate.NewFeatureGate()
 	leaseEnabledFeature        = featuregate.NewFeatureGate()
@@ -51,12 +49,6 @@ var (
 )
 
 func init() {
-	if err := csiEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSIPersistentVolume: {Default: true}}); err != nil {
-		panic(err)
-	}
-	if err := csiDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSIPersistentVolume: {Default: false}}); err != nil {
-		panic(err)
-	}
 	if err := trEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: true}}); err != nil {
 		panic(err)
 	}
@@ -198,22 +190,9 @@ func TestAuthorizer(t *testing.T) {
 			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed attachment - no relationship",
+			name:   "allowed attachment",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node1"},
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node0"},
-			features: csiEnabledFeature,
+			expect: authorizer.DecisionAllow,
-			expect:   authorizer.DecisionNoOpinion,
-		},
-		{
-			name:     "disallowed attachment - feature disabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node0"},
-			features: csiDisabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
-		},
-		{
-			name:     "allowed attachment - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node0"},
-			features: csiEnabledFeature,
-			expect:   authorizer.DecisionAllow,
 		},
 		{
 			name:     "allowed svcacct token create - feature enabled",
@@ -771,22 +750,14 @@ func BenchmarkAuthorization(b *testing.B) {
 			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed attachment - no relationship",
+			name:   "disallowed attachment - no relationship",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node1"},
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node1"},
-			features: csiEnabledFeature,
+			expect: authorizer.DecisionNoOpinion,
-			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed attachment - feature disabled",
+			name:   "allowed attachment",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node0"},
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node0"},
-			features: csiDisabledFeature,
+			expect: authorizer.DecisionAllow,
-			expect:   authorizer.DecisionNoOpinion,
-		},
-		{
-			name:     "allowed attachment - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "volumeattachments", APIGroup: "storage.k8s.io", Name: "attachment0-node0"},
-			features: csiEnabledFeature,
-			expect:   authorizer.DecisionAllow,
 		},
 	}
 

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

@@ -68,17 +68,15 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
 				rbacv1helpers.NewRule("patch", "update").Groups(legacyGroup).Resources("nodes/status").RuleOrDie(),
 				rbacv1helpers.NewRule("list", "watch").Groups(legacyGroup).Resources("pods").RuleOrDie(),
 				eventsRule(),
+				rbacv1helpers.NewRule("get", "create", "delete", "list", "watch").Groups(storageGroup).Resources("volumeattachments").RuleOrDie(),
 			},
 		}
 
-		if utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
+		if utilfeature.DefaultFeatureGate.Enabled(features.CSIDriverRegistry) {
-			role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "create", "delete", "list", "watch").Groups(storageGroup).Resources("volumeattachments").RuleOrDie())
+			role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csidrivers").RuleOrDie())
-			if utilfeature.DefaultFeatureGate.Enabled(features.CSIDriverRegistry) {
+		}
-				role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csidrivers").RuleOrDie())
+		if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) {
-			}
+			role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie())
-			if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) && utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) {
-				role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie())
-			}
 		}
 
 		return role

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

@@ -139,6 +139,9 @@ func NodeRules() []rbacv1.PolicyRule {
 		// Used to create a certificatesigningrequest for a node-specific client certificate, and watch
 		// for it to be signed. This allows the kubelet to rotate it's own certificate.
 		rbacv1helpers.NewRule("create", "get", "list", "watch").Groups(certificatesGroup).Resources("certificatesigningrequests").RuleOrDie(),
+
+		// CSI
+		rbacv1helpers.NewRule("get").Groups(storageGroup).Resources("volumeattachments").RuleOrDie(),
 	}
 
 	if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) {
@@ -156,13 +159,9 @@ func NodeRules() []rbacv1.PolicyRule {
 	}
 
 	// CSI
-	if utilfeature.DefaultFeatureGate.Enabled(features.CSIPersistentVolume) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.CSIDriverRegistry) {
-		volAttachRule := rbacv1helpers.NewRule("get").Groups(storageGroup).Resources("volumeattachments").RuleOrDie()
+		csiDriverRule := rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csidrivers").RuleOrDie()
-		nodePolicyRules = append(nodePolicyRules, volAttachRule)
+		nodePolicyRules = append(nodePolicyRules, csiDriverRule)
-		if utilfeature.DefaultFeatureGate.Enabled(features.CSIDriverRegistry) {
-			csiDriverRule := rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csidrivers").RuleOrDie()
-			nodePolicyRules = append(nodePolicyRules, csiDriverRule)
-		}
 	}
 	if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) {
 		csiNodeInfoRule := rbacv1helpers.NewRule("get", "create", "update", "patch", "delete").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie()

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

@@ -957,6 +957,12 @@ items:
     - get
     - list
     - watch
+  - apiGroups:
+    - storage.k8s.io
+    resources:
+    - volumeattachments
+    verbs:
+    - get
   - apiGroups:
     - ""
     resources:
@@ -971,12 +977,6 @@ items:
     - serviceaccounts/token
     verbs:
     - create
-  - apiGroups:
-    - storage.k8s.io
-    resources:
-    - volumeattachments
-    verbs:
-    - get
   - apiGroups:
     - storage.k8s.io
     resources: