Merge pull request #84351 from wojtek-t/promote_node_lease_to_GA

Promote node lease to GA

plugin/pkg/admission/noderestriction/admission.go

@@ -151,10 +151,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
 		return nil
 
 	case leaseResource:
-		if p.features.Enabled(features.NodeLease) {
-			return p.admitLease(nodeName, a)
-		}
-		return admission.NewForbidden(a, fmt.Errorf("disabled by feature gate %s", features.NodeLease))
+		return p.admitLease(nodeName, a)
 
 	case csiNodeResource:
 		if p.features.Enabled(features.CSINodeInfo) {

plugin/pkg/admission/noderestriction/admission_test.go

@@ -48,8 +48,6 @@ import (
 var (
 	trEnabledFeature           = featuregate.NewFeatureGate()
 	trDisabledFeature          = featuregate.NewFeatureGate()
-	leaseEnabledFeature        = featuregate.NewFeatureGate()
-	leaseDisabledFeature       = featuregate.NewFeatureGate()
 	csiNodeInfoEnabledFeature  = featuregate.NewFeatureGate()
 	csiNodeInfoDisabledFeature = featuregate.NewFeatureGate()
 )
@@ -61,12 +59,6 @@ func init() {
 	if err := trDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: false}}); err != nil {
 		panic(err)
 	}
-	if err := leaseEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.NodeLease: {Default: true}}); err != nil {
-		panic(err)
-	}
-	if err := leaseDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.NodeLease: {Default: false}}); err != nil {
-		panic(err)
-	}
 	if err := csiNodeInfoEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSINodeInfo: {Default: true}}); err != nil {
 		panic(err)
 	}
@@ -1148,64 +1140,49 @@ func Test_nodePlugin_Admit(t *testing.T) {
 			err:        "",
 		},
 		// Node leases
-		{
-			name:       "disallowed create lease - feature disabled",
-			attributes: admission.NewAttributesRecord(lease, nil, leaseKind, lease.Namespace, lease.Name, leaseResource, "", admission.Create, &metav1.DeleteOptions{}, false, mynode),
-			features:   leaseDisabledFeature,
-			err:        "forbidden: disabled by feature gate NodeLease",
-		},
 		{
 			name:       "disallowed create lease in namespace other than kube-node-lease - feature enabled",
 			attributes: admission.NewAttributesRecord(leaseWrongNS, nil, leaseKind, leaseWrongNS.Namespace, leaseWrongNS.Name, leaseResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed update lease in namespace other than kube-node-lease - feature enabled",
 			attributes: admission.NewAttributesRecord(leaseWrongNS, leaseWrongNS, leaseKind, leaseWrongNS.Namespace, leaseWrongNS.Name, leaseResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed delete lease in namespace other than kube-node-lease - feature enabled",
 			attributes: admission.NewAttributesRecord(nil, nil, leaseKind, leaseWrongNS.Namespace, leaseWrongNS.Name, leaseResource, "", admission.Delete, &metav1.DeleteOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed create another node's lease - feature enabled",
 			attributes: admission.NewAttributesRecord(leaseWrongName, nil, leaseKind, leaseWrongName.Namespace, leaseWrongName.Name, leaseResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed update another node's lease - feature enabled",
 			attributes: admission.NewAttributesRecord(leaseWrongName, leaseWrongName, leaseKind, leaseWrongName.Namespace, leaseWrongName.Name, leaseResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "disallowed delete another node's lease - feature enabled",
 			attributes: admission.NewAttributesRecord(nil, nil, leaseKind, leaseWrongName.Namespace, leaseWrongName.Name, leaseResource, "", admission.Delete, &metav1.DeleteOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "forbidden: ",
 		},
 		{
 			name:       "allowed create node lease - feature enabled",
 			attributes: admission.NewAttributesRecord(lease, nil, leaseKind, lease.Namespace, lease.Name, leaseResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "",
 		},
 		{
 			name:       "allowed update node lease - feature enabled",
 			attributes: admission.NewAttributesRecord(lease, lease, leaseKind, lease.Namespace, lease.Name, leaseResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "",
 		},
 		{
 			name:       "allowed delete node lease - feature enabled",
 			attributes: admission.NewAttributesRecord(nil, nil, leaseKind, lease.Namespace, lease.Name, leaseResource, "", admission.Delete, &metav1.DeleteOptions{}, false, mynode),
-			features:   leaseEnabledFeature,
 			err:        "",
 		},
 		// CSINode

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

@@ -116,10 +116,7 @@ func (r *NodeAuthorizer) Authorize(ctx context.Context, attrs authorizer.Attribu
 			}
 			return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.TokenRequest), nil
 		case leaseResource:
-			if r.features.Enabled(features.NodeLease) {
-				return r.authorizeLease(nodeName, attrs)
-			}
-			return authorizer.DecisionNoOpinion, fmt.Sprintf("disabled by feature gate %s", features.NodeLease), nil
+			return r.authorizeLease(nodeName, attrs)
 		case csiNodeResource:
 			if r.features.Enabled(features.CSINodeInfo) {
 				return r.authorizeCSINode(nodeName, attrs)

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

@@ -43,8 +43,6 @@ import (
 var (
 	trEnabledFeature           = featuregate.NewFeatureGate()
 	trDisabledFeature          = featuregate.NewFeatureGate()
-	leaseEnabledFeature        = featuregate.NewFeatureGate()
-	leaseDisabledFeature       = featuregate.NewFeatureGate()
 	csiNodeInfoEnabledFeature  = featuregate.NewFeatureGate()
 	csiNodeInfoDisabledFeature = featuregate.NewFeatureGate()
 )
@@ -56,12 +54,6 @@ func init() {
 	if err := trDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: false}}); err != nil {
 		panic(err)
 	}
-	if err := leaseEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.NodeLease: {Default: true}}); err != nil {
-		panic(err)
-	}
-	if err := leaseDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.NodeLease: {Default: false}}); err != nil {
-		panic(err)
-	}
 	if err := csiNodeInfoEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSINodeInfo: {Default: true}}); err != nil {
 		panic(err)
 	}
@@ -226,106 +218,84 @@ func TestAuthorizer(t *testing.T) {
 			expect:   authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed node lease - feature disabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
-			features: leaseDisabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed get lease in namespace other than kube-node-lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed get lease in namespace other than kube-node-lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed create lease in namespace other than kube-node-lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed create lease in namespace other than kube-node-lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed update lease in namespace other than kube-node-lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed update lease in namespace other than kube-node-lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed patch lease in namespace other than kube-node-lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed patch lease in namespace other than kube-node-lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed delete lease in namespace other than kube-node-lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed delete lease in namespace other than kube-node-lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: "foo"},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed get another node's lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed get another node's lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed update another node's lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed update another node's lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed patch another node's lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed patch another node's lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed delete another node's lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed delete another node's lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node1", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed list node leases - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "leases", APIGroup: "coordination.k8s.io", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed list node leases - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "leases", APIGroup: "coordination.k8s.io", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "disallowed watch node leases - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "leases", APIGroup: "coordination.k8s.io", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionNoOpinion,
 		},
 		{
-			name:     "disallowed watch node leases - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "leases", APIGroup: "coordination.k8s.io", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionNoOpinion,
+			name:   "allowed get node lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionAllow,
 		},
 		{
-			name:     "allowed get node lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionAllow,
+			name:   "allowed create node lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionAllow,
 		},
 		{
-			name:     "allowed create node lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionAllow,
+			name:   "allowed update node lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionAllow,
 		},
 		{
-			name:     "allowed update node lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionAllow,
+			name:   "allowed patch node lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionAllow,
 		},
 		{
-			name:     "allowed patch node lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionAllow,
-		},
-		{
-			name:     "allowed delete node lease - feature enabled",
-			attrs:    authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
-			features: leaseEnabledFeature,
-			expect:   authorizer.DecisionAllow,
+			name:   "allowed delete node lease - feature enabled",
+			attrs:  authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "leases", APIGroup: "coordination.k8s.io", Name: "node0", Namespace: corev1.NamespaceNodeLease},
+			expect: authorizer.DecisionAllow,
 		},
 		// CSINode
 		{

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

@@ -143,6 +143,9 @@ func NodeRules() []rbacv1.PolicyRule {
 		// 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(),
 
+		// Leases
+		rbacv1helpers.NewRule("get", "create", "update", "patch", "delete").Groups("coordination.k8s.io").Resources("leases").RuleOrDie(),
+
 		// CSI
 		rbacv1helpers.NewRule("get").Groups(storageGroup).Resources("volumeattachments").RuleOrDie(),
 	}
@@ -171,11 +174,6 @@ func NodeRules() []rbacv1.PolicyRule {
 		nodePolicyRules = append(nodePolicyRules, csiNodeInfoRule)
 	}
 
-	// Node leases
-	if utilfeature.DefaultFeatureGate.Enabled(features.NodeLease) {
-		nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get", "create", "update", "patch", "delete").Groups(coordinationGroup).Resources("leases").RuleOrDie())
-	}
-
 	// RuntimeClass
 	if utilfeature.DefaultFeatureGate.Enabled(features.RuntimeClass) {
 		nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get", "list", "watch").Groups("node.k8s.io").Resources("runtimeclasses").RuleOrDie())

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

@@ -910,6 +910,16 @@ items:
     - get
     - list
     - watch
+  - apiGroups:
+    - coordination.k8s.io
+    resources:
+    - leases
+    verbs:
+    - create
+    - delete
+    - get
+    - patch
+    - update
   - apiGroups:
     - storage.k8s.io
     resources:
@@ -948,16 +958,6 @@ items:
     - get
     - patch
     - update
-  - apiGroups:
-    - coordination.k8s.io
-    resources:
-    - leases
-    verbs:
-    - create
-    - delete
-    - get
-    - patch
-    - update
   - apiGroups:
     - node.k8s.io
     resources: