Rearranged feature flags

2026-01-05 23:47:50 +00:00 · 2018-09-06 15:45:50 -07:00
parent becc6a9c19
commit 94d649b590
13 changed files with 217 additions and 116 deletions
--- a/plugin/pkg/admission/noderestriction/admission.go
+++ b/plugin/pkg/admission/noderestriction/admission.go
@@ -147,10 +147,10 @@ func (c *nodePlugin) Admit(a admission.Attributes) error {
 		return admission.NewForbidden(a, fmt.Errorf("disabled by feature gate %s", features.NodeLease))

 	case csiNodeInfoResource:
-		if c.features.Enabled(features.KubeletPluginsWatcher) {
+		if c.features.Enabled(features.KubeletPluginsWatcher) && c.features.Enabled(features.CSINodeInfo) {
 			return c.admitCSINodeInfo(nodeName, a)
 		}
-		return admission.NewForbidden(a, fmt.Errorf("disabled by feature gate %s", features.KubeletPluginsWatcher))
+		return admission.NewForbidden(a, fmt.Errorf("disabled by feature gates %s and %s", features.KubeletPluginsWatcher, features.CSINodeInfo))

 	default:
 		return nil
--- a/plugin/pkg/admission/noderestriction/admission_test.go
+++ b/plugin/pkg/admission/noderestriction/admission_test.go
@@ -21,6 +21,7 @@ import (
 	"testing"
 	"time"

+	"fmt"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -41,12 +42,12 @@ import (
 )

 var (
-	trEnabledFeature              = utilfeature.NewFeatureGate()
-	trDisabledFeature             = utilfeature.NewFeatureGate()
-	leaseEnabledFeature           = utilfeature.NewFeatureGate()
-	leaseDisabledFeature          = utilfeature.NewFeatureGate()
-	pluginsWatcherEnabledFeature  = utilfeature.NewFeatureGate()
-	pluginsWatcherDisabledFeature = utilfeature.NewFeatureGate()
+	trEnabledFeature           = utilfeature.NewFeatureGate()
+	trDisabledFeature          = utilfeature.NewFeatureGate()
+	leaseEnabledFeature        = utilfeature.NewFeatureGate()
+	leaseDisabledFeature       = utilfeature.NewFeatureGate()
+	csiNodeInfoEnabledFeature  = utilfeature.NewFeatureGate()
+	csiNodeInfoDisabledFeature = utilfeature.NewFeatureGate()
 )

 func init() {
@@ -62,10 +63,16 @@ func init() {
 	if err := leaseDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.NodeLease: {Default: false}}); err != nil {
 		panic(err)
 	}
-	if err := pluginsWatcherEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: true}}); err != nil {
+	if err := csiNodeInfoEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: true}}); err != nil {
 		panic(err)
 	}
-	if err := pluginsWatcherDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: false}}); err != nil {
+	if err := csiNodeInfoEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.CSINodeInfo: {Default: true}}); err != nil {
+		panic(err)
+	}
+	if err := csiNodeInfoDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: false}}); err != nil {
+		panic(err)
+	}
+	if err := csiNodeInfoDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.CSINodeInfo: {Default: false}}); err != nil {
 		panic(err)
 	}
 }
@@ -996,43 +1003,43 @@ func Test_nodePlugin_Admit(t *testing.T) {
 		{
 			name:       "disallowed create CSINodeInfo - feature disabled",
 			attributes: admission.NewAttributesRecord(nodeInfo, nil, csiNodeInfoKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeInfoResource, "", admission.Create, false, mynode),
-			features:   pluginsWatcherDisabledFeature,
-			err:        "forbidden: disabled by feature gate KubeletPluginsWatcher",
+			features:   csiNodeInfoDisabledFeature,
+			err:        fmt.Sprintf("forbidden: disabled by feature gates %s and %s", features.KubeletPluginsWatcher, features.CSINodeInfo),
 		},
 		{
 			name:       "disallowed create another node's CSINodeInfo - feature enabled",
 			attributes: admission.NewAttributesRecord(nodeInfoWrongName, nil, csiNodeInfoKind, nodeInfoWrongName.Namespace, nodeInfoWrongName.Name, csiNodeInfoResource, "", admission.Create, false, mynode),
-			features:   pluginsWatcherEnabledFeature,
+			features:   csiNodeInfoEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed update another node's CSINodeInfo - feature enabled",
 			attributes: admission.NewAttributesRecord(nodeInfoWrongName, nodeInfoWrongName, csiNodeInfoKind, nodeInfoWrongName.Namespace, nodeInfoWrongName.Name, csiNodeInfoResource, "", admission.Update, false, mynode),
-			features:   pluginsWatcherEnabledFeature,
+			features:   csiNodeInfoEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed delete another node's CSINodeInfo - feature enabled",
 			attributes: admission.NewAttributesRecord(nil, nil, csiNodeInfoKind, nodeInfoWrongName.Namespace, nodeInfoWrongName.Name, csiNodeInfoResource, "", admission.Delete, false, mynode),
-			features:   pluginsWatcherEnabledFeature,
+			features:   csiNodeInfoEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "allowed create node CSINodeInfo - feature enabled",
 			attributes: admission.NewAttributesRecord(nodeInfo, nil, csiNodeInfoKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeInfoResource, "", admission.Create, false, mynode),
-			features:   pluginsWatcherEnabledFeature,
+			features:   csiNodeInfoEnabledFeature,
 			err:        "",
 		},
 		{
 			name:       "allowed update node CSINodeInfo - feature enabled",
 			attributes: admission.NewAttributesRecord(nodeInfo, nodeInfo, csiNodeInfoKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeInfoResource, "", admission.Update, false, mynode),
-			features:   pluginsWatcherEnabledFeature,
+			features:   csiNodeInfoEnabledFeature,
 			err:        "",
 		},
 		{
 			name:       "allowed delete node CSINodeInfo - feature enabled",
 			attributes: admission.NewAttributesRecord(nil, nil, csiNodeInfoKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeInfoResource, "", admission.Delete, false, mynode),
-			features:   pluginsWatcherEnabledFeature,
+			features:   csiNodeInfoEnabledFeature,
 			err:        "",
 		},
 	}
--- a/plugin/pkg/auth/authorizer/node/node_authorizer.go
+++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go
@@ -123,10 +123,10 @@ func (r *NodeAuthorizer) Authorize(attrs authorizer.Attributes) (authorizer.Deci
 			}
 			return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.NodeLease), nil
 		case csiNodeInfoResource:
-			if r.features.Enabled(features.KubeletPluginsWatcher) {
+			if r.features.Enabled(features.KubeletPluginsWatcher) && r.features.Enabled(features.CSINodeInfo) {
 				return r.authorizeCSINodeInfo(nodeName, attrs)
 			}
-			return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.KubeletPluginsWatcher), nil
+			return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gates %s and %s", features.KubeletPluginsWatcher, features.CSINodeInfo), nil
 		}

 	}
--- a/plugin/pkg/auth/authorizer/node/node_authorizer_test.go
+++ b/plugin/pkg/auth/authorizer/node/node_authorizer_test.go
@@ -39,14 +39,14 @@ import (
 )

 var (
-	csiEnabledFeature             = utilfeature.NewFeatureGate()
-	csiDisabledFeature            = utilfeature.NewFeatureGate()
-	trEnabledFeature              = utilfeature.NewFeatureGate()
-	trDisabledFeature             = utilfeature.NewFeatureGate()
-	leaseEnabledFeature           = utilfeature.NewFeatureGate()
-	leaseDisabledFeature          = utilfeature.NewFeatureGate()
-	pluginsWatcherEnabledFeature  = utilfeature.NewFeatureGate()
-	pluginsWatcherDisabledFeature = utilfeature.NewFeatureGate()
+	csiEnabledFeature          = utilfeature.NewFeatureGate()
+	csiDisabledFeature         = utilfeature.NewFeatureGate()
+	trEnabledFeature           = utilfeature.NewFeatureGate()
+	trDisabledFeature          = utilfeature.NewFeatureGate()
+	leaseEnabledFeature        = utilfeature.NewFeatureGate()
+	leaseDisabledFeature       = utilfeature.NewFeatureGate()
+	csiNodeInfoEnabledFeature  = utilfeature.NewFeatureGate()
+	csiNodeInfoDisabledFeature = utilfeature.NewFeatureGate()
 )

 func init() {
@@ -68,10 +68,16 @@ func init() {
 	if err := leaseDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.NodeLease: {Default: false}}); err != nil {
 		panic(err)
 	}
-	if err := pluginsWatcherEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: true}}); err != nil {
+	if err := csiNodeInfoEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: true}}); err != nil {
 		panic(err)
 	}
-	if err := pluginsWatcherDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: false}}); err != nil {
+	if err := csiNodeInfoEnabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.CSINodeInfo: {Default: true}}); err != nil {
+		panic(err)
+	}
+	if err := csiNodeInfoDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.KubeletPluginsWatcher: {Default: false}}); err != nil {
+		panic(err)
+	}
+	if err := csiNodeInfoDisabledFeature.Add(map[utilfeature.Feature]utilfeature.FeatureSpec{features.CSINodeInfo: {Default: false}}); err != nil {
 		panic(err)
 	}
 }
@@ -350,79 +356,79 @@ func TestAuthorizer(t *testing.T) {
 		{
 			name:     "disallowed CSINodeInfo - feature disabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherDisabledFeature,
+			features: csiNodeInfoDisabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed CSINodeInfo with subresource - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodeinfos", Subresource: "csiDrivers", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed get another node's CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node1"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed update another node's CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node1"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed patch another node's CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node1"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed delete another node's CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node1"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed list CSINodeInfos - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "disallowed watch CSINodeInfos - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
 			name:     "allowed get CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionAllow,
 		},
 		{
 			name:     "allowed create CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionAllow,
 		},
 		{
 			name:     "allowed update CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionAllow,
 		},
 		{
 			name:     "allowed patch CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionAllow,
 		},
 		{
 			name:     "allowed delete CSINodeInfo - feature enabled",
 			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "csinodeinfos", APIGroup: "csi.storage.k8s.io", Name: "node0"},
-			features: pluginsWatcherEnabledFeature,
+			features: csiNodeInfoEnabledFeature,
 			expect:   authorizer.DecisionAllow,
 		},
 	}
--- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go
+++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go
@@ -164,7 +164,8 @@ func NodeRules() []rbacv1.PolicyRule {
 			nodePolicyRules = append(nodePolicyRules, csiDriverRule)
 		}
 	}
-	if utilfeature.DefaultFeatureGate.Enabled(features.KubeletPluginsWatcher) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.KubeletPluginsWatcher) &&
+		utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) {
 		csiNodeInfoRule := rbacv1helpers.NewRule("get", "create", "update", "patch", "delete").Groups("csi.storage.k8s.io").Resources("csinodeinfos").RuleOrDie()
 		nodePolicyRules = append(nodePolicyRules, csiNodeInfoRule)
 	}
@@ -507,7 +508,7 @@ func ClusterRoles() []rbacv1.ClusterRole {
 		rbacv1helpers.NewRule("get", "list", "watch", "create", "update", "patch").Groups(legacyGroup).Resources("events").RuleOrDie(),
 		rbacv1helpers.NewRule("get", "list", "watch").Groups(legacyGroup).Resources("nodes").RuleOrDie(),
 	}
-	if utilfeature.DefaultFeatureGate.Enabled(features.KubeletPluginsWatcher) {
+	if utilfeature.DefaultFeatureGate.Enabled(features.CSINodeInfo) {
 		externalProvisionerRules = append(externalProvisionerRules, rbacv1helpers.NewRule("get", "watch", "list").Groups("csi.storage.k8s.io").Resources("csinodeinfos").RuleOrDie())
 	}
 	roles = append(roles, rbacv1.ClusterRole{
--- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml
+++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/testdata/cluster-roles.yaml
@@ -531,14 +531,6 @@ items:
    - get
    - list
    - watch
-  - apiGroups:
-    - csi.storage.k8s.io
-    resources:
-    - csinodeinfos
-    verbs:
-    - get
-    - list
-    - watch
 - apiVersion: rbac.authorization.k8s.io/v1
  kind: ClusterRole
  metadata:
@@ -978,16 +970,6 @@ items:
    - volumeattachments
    verbs:
    - get
-  - apiGroups:
-    - csi.storage.k8s.io
-    resources:
-    - csinodeinfos
-    verbs:
-    - create
-    - delete
-    - get
-    - patch
-    - update
 - apiVersion: rbac.authorization.k8s.io/v1
  kind: ClusterRole
  metadata: