Remove CSINodeInfo feature gate

Signed-off-by: ialidzhikov <i.alidjikov@gmail.com>
This commit is contained in:
ialidzhikov
2020-11-13 14:45:33 +02:00
parent ccd29ea264
commit bc432124a2
26 changed files with 117 additions and 267 deletions

View File

@@ -46,13 +46,11 @@ go_test(
"//pkg/apis/policy:go_default_library",
"//pkg/apis/storage:go_default_library",
"//pkg/auth/nodeidentifier:go_default_library",
"//pkg/features:go_default_library",
"//pkg/kubelet/apis:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",

View File

@@ -71,7 +71,6 @@ type Plugin struct {
podsGetter corev1lister.PodLister
nodesGetter corev1lister.NodeLister
csiNodeInfoEnabled bool
expandPersistentVolumesEnabled bool
}
@@ -83,7 +82,6 @@ var (
// InspectFeatureGates allows setting bools without taking a dep on a global variable
func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
p.csiNodeInfoEnabled = featureGates.Enabled(features.CSINodeInfo)
p.expandPersistentVolumesEnabled = featureGates.Enabled(features.ExpandPersistentVolumes)
}
@@ -163,10 +161,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
return p.admitLease(nodeName, a)
case csiNodeResource:
if p.csiNodeInfoEnabled {
return p.admitCSINode(nodeName, a)
}
return admission.NewForbidden(a, fmt.Errorf("disabled by feature gates %s", features.CSINodeInfo))
return p.admitCSINode(nodeName, a)
default:
return nil

View File

@@ -18,18 +18,15 @@ package noderestriction
import (
"context"
"fmt"
"reflect"
"strings"
"testing"
"time"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/admission"
"k8s.io/apiserver/pkg/authentication/user"
@@ -42,28 +39,10 @@ import (
"k8s.io/kubernetes/pkg/apis/policy"
storage "k8s.io/kubernetes/pkg/apis/storage"
"k8s.io/kubernetes/pkg/auth/nodeidentifier"
"k8s.io/kubernetes/pkg/features"
kubeletapis "k8s.io/kubernetes/pkg/kubelet/apis"
"k8s.io/utils/pointer"
)
var (
csiNodeInfoEnabledFeature = featuregate.NewFeatureGate()
csiNodeInfoDisabledFeature = featuregate.NewFeatureGate()
)
func init() {
// all features need to be set on all featuregates for the tests. We set everything and then then the if's below override it.
relevantFeatures := map[featuregate.Feature]featuregate.FeatureSpec{
features.CSINodeInfo: {Default: false},
features.ExpandPersistentVolumes: {Default: false},
}
utilruntime.Must(csiNodeInfoEnabledFeature.Add(relevantFeatures))
utilruntime.Must(csiNodeInfoDisabledFeature.Add(relevantFeatures))
utilruntime.Must(csiNodeInfoEnabledFeature.SetFromMap(map[string]bool{string(features.CSINodeInfo): true}))
}
func makeTestPod(namespace, name, node string, mirror bool) (*api.Pod, *corev1.Pod) {
corePod := &api.Pod{}
corePod.Namespace = namespace
@@ -398,7 +377,7 @@ func Test_nodePlugin_Admit(t *testing.T) {
existingPodsIndex.Add(v1otherpod)
existingPodsIndex.Add(v1unboundpod)
existingNodesIndex.Add(&v1.Node{ObjectMeta: mynodeObjMeta})
existingNodesIndex.Add(&corev1.Node{ObjectMeta: mynodeObjMeta})
sapod, _ := makeTestPod("ns", "mysapod", "mynode", true)
sapod.Spec.ServiceAccountName = "foo"
@@ -1214,45 +1193,33 @@ func Test_nodePlugin_Admit(t *testing.T) {
},
// CSINode
{
name: "disallowed create CSINode - feature disabled",
attributes: admission.NewAttributesRecord(nodeInfo, nil, csiNodeKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
features: csiNodeInfoDisabledFeature,
err: fmt.Sprintf("forbidden: disabled by feature gates %s", features.CSINodeInfo),
},
{
name: "disallowed create another node's CSINode - feature enabled",
name: "disallowed create another node's CSINode",
attributes: admission.NewAttributesRecord(nodeInfoWrongName, nil, csiNodeKind, nodeInfoWrongName.Namespace, nodeInfoWrongName.Name, csiNodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
features: csiNodeInfoEnabledFeature,
err: "forbidden: ",
},
{
name: "disallowed update another node's CSINode - feature enabled",
name: "disallowed update another node's CSINode",
attributes: admission.NewAttributesRecord(nodeInfoWrongName, nodeInfoWrongName, csiNodeKind, nodeInfoWrongName.Namespace, nodeInfoWrongName.Name, csiNodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
features: csiNodeInfoEnabledFeature,
err: "forbidden: ",
},
{
name: "disallowed delete another node's CSINode - feature enabled",
name: "disallowed delete another node's CSINode",
attributes: admission.NewAttributesRecord(nil, nil, csiNodeKind, nodeInfoWrongName.Namespace, nodeInfoWrongName.Name, csiNodeResource, "", admission.Delete, &metav1.DeleteOptions{}, false, mynode),
features: csiNodeInfoEnabledFeature,
err: "forbidden: ",
},
{
name: "allowed create node CSINode - feature enabled",
name: "allowed create node CSINode",
attributes: admission.NewAttributesRecord(nodeInfo, nil, csiNodeKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeResource, "", admission.Create, &metav1.CreateOptions{}, false, mynode),
features: csiNodeInfoEnabledFeature,
err: "",
},
{
name: "allowed update node CSINode - feature enabled",
name: "allowed update node CSINode",
attributes: admission.NewAttributesRecord(nodeInfo, nodeInfo, csiNodeKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeResource, "", admission.Update, &metav1.UpdateOptions{}, false, mynode),
features: csiNodeInfoEnabledFeature,
err: "",
},
{
name: "allowed delete node CSINode - feature enabled",
name: "allowed delete node CSINode",
attributes: admission.NewAttributesRecord(nil, nil, csiNodeKind, nodeInfo.Namespace, nodeInfo.Name, csiNodeResource, "", admission.Delete, &metav1.UpdateOptions{}, false, mynode),
features: csiNodeInfoEnabledFeature,
err: "",
},
}
@@ -1264,11 +1231,11 @@ func Test_nodePlugin_Admit(t *testing.T) {
func Test_nodePlugin_Admit_OwnerReference(t *testing.T) {
expectedNodeIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
expectedNodeIndex.Add(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}})
expectedNodeIndex.Add(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}})
expectedNode := corev1lister.NewNodeLister(expectedNodeIndex)
unexpectedNodeIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
unexpectedNodeIndex.Add(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-unexpected-uid"}})
unexpectedNodeIndex.Add(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-unexpected-uid"}})
unexpectedNode := corev1lister.NewNodeLister(unexpectedNodeIndex)
noNodesIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)

View File

@@ -16,7 +16,6 @@ go_test(
embed = [":go_default_library"],
deps = [
"//pkg/auth/nodeidentifier:go_default_library",
"//pkg/features:go_default_library",
"//plugin/pkg/auth/authorizer/rbac/bootstrappolicy:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/storage/v1:go_default_library",

View File

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

View File

@@ -20,14 +20,13 @@ import (
"context"
"fmt"
"math/rand"
"os"
"runtime"
"runtime/pprof"
"sync/atomic"
"testing"
"time"
"os"
corev1 "k8s.io/api/core/v1"
storagev1 "k8s.io/api/storage/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -37,24 +36,9 @@ import (
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/component-base/featuregate"
"k8s.io/kubernetes/pkg/auth/nodeidentifier"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
)
var (
csiNodeInfoEnabledFeature = featuregate.NewFeatureGate()
csiNodeInfoDisabledFeature = featuregate.NewFeatureGate()
)
func init() {
if err := csiNodeInfoEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSINodeInfo: {Default: true}}); err != nil {
panic(err)
}
if err := csiNodeInfoDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSINodeInfo: {Default: false}}); err != nil {
panic(err)
}
}
func TestAuthorizer(t *testing.T) {
g := NewGraph()
@@ -282,82 +266,64 @@ func TestAuthorizer(t *testing.T) {
},
// CSINode
{
name: "disallowed CSINode - feature disabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoDisabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed CSINode with subresource - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", Subresource: "csiDrivers", APIGroup: "storage.k8s.io", Name: "node0"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed CSINode with subresource - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", Subresource: "csiDrivers", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed get another node's CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed get another node's CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed update another node's CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed update another node's CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed patch another node's CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed patch another node's CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed delete another node's CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed delete another node's CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node1"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed list CSINodes",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "csinodes", APIGroup: "storage.k8s.io"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed list CSINodes - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "list", Resource: "csinodes", APIGroup: "storage.k8s.io"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "disallowed watch CSINodes",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "csinodes", APIGroup: "storage.k8s.io"},
expect: authorizer.DecisionNoOpinion,
},
{
name: "disallowed watch CSINodes - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "watch", Resource: "csinodes", APIGroup: "storage.k8s.io"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionNoOpinion,
name: "allowed get CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed get CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "get", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionAllow,
name: "allowed create CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed create CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "create", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionAllow,
name: "allowed update CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed update CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "update", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionAllow,
name: "allowed patch CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
expect: authorizer.DecisionAllow,
},
{
name: "allowed patch CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "patch", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionAllow,
},
{
name: "allowed delete CSINode - feature enabled",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
features: csiNodeInfoEnabledFeature,
expect: authorizer.DecisionAllow,
name: "allowed delete CSINode",
attrs: authorizer.AttributesRecord{User: node0, ResourceRequest: true, Verb: "delete", Resource: "csinodes", APIGroup: "storage.k8s.io", Name: "node0"},
expect: authorizer.DecisionAllow,
},
}

View File

@@ -74,7 +74,7 @@ func buildControllerRoles() ([]rbacv1.ClusterRole, []rbacv1.ClusterRoleBinding)
}
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) {
if utilfeature.DefaultFeatureGate.Enabled(features.CSIMigration) {
role.Rules = append(role.Rules, rbacv1helpers.NewRule("get", "watch", "list").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie())
}

View File

@@ -169,10 +169,8 @@ func NodeRules() []rbacv1.PolicyRule {
// CSI
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()
nodePolicyRules = append(nodePolicyRules, csiNodeInfoRule)
}
csiNodeInfoRule := rbacv1helpers.NewRule("get", "create", "update", "patch", "delete").Groups("storage.k8s.io").Resources("csinodes").RuleOrDie()
nodePolicyRules = append(nodePolicyRules, csiNodeInfoRule)
// RuntimeClass
nodePolicyRules = append(nodePolicyRules, rbacv1helpers.NewRule("get", "list", "watch").Groups("node.k8s.io").Resources("runtimeclasses").RuleOrDie())