diff --git a/plugin/pkg/admission/noderestriction/BUILD b/plugin/pkg/admission/noderestriction/BUILD index e89945a13f0..e9dfae55fe9 100644 --- a/plugin/pkg/admission/noderestriction/BUILD +++ b/plugin/pkg/admission/noderestriction/BUILD @@ -29,7 +29,6 @@ go_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/admission/initializer:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/component-base/featuregate:go_default_library", @@ -54,6 +53,7 @@ go_test( "//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", diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 6c896f60fb0..7792048adfb 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" apiserveradmission "k8s.io/apiserver/pkg/admission/initializer" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" corev1lister "k8s.io/client-go/listers/core/v1" "k8s.io/component-base/featuregate" @@ -63,7 +62,6 @@ func NewPlugin(nodeIdentifier nodeidentifier.NodeIdentifier) *Plugin { return &Plugin{ Handler: admission.NewHandler(admission.Create, admission.Update, admission.Delete), nodeIdentifier: nodeIdentifier, - features: utilfeature.DefaultFeatureGate, } } @@ -72,15 +70,25 @@ type Plugin struct { *admission.Handler nodeIdentifier nodeidentifier.NodeIdentifier podsGetter corev1lister.PodLister - // allows overriding for testing - features featuregate.FeatureGate + + tokenRequestEnabled bool + csiNodeInfoEnabled bool + expandPersistentVolumesEnabled bool } var ( - _ = admission.Interface(&Plugin{}) - _ = apiserveradmission.WantsExternalKubeInformerFactory(&Plugin{}) + _ admission.Interface = &Plugin{} + _ apiserveradmission.WantsExternalKubeInformerFactory = &Plugin{} + _ apiserveradmission.WantsFeatures = &Plugin{} ) +// InspectFeatureGates allows setting bools without taking a dep on a global variable +func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { + p.tokenRequestEnabled = featureGates.Enabled(features.TokenRequest) + p.csiNodeInfoEnabled = featureGates.Enabled(features.CSINodeInfo) + p.expandPersistentVolumesEnabled = featureGates.Enabled(features.ExpandPersistentVolumes) +} + // SetExternalKubeInformerFactory registers an informer factory into Plugin func (p *Plugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { p.podsGetter = f.Core().V1().Pods().Lister() @@ -145,7 +153,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. } case svcacctResource: - if p.features.Enabled(features.TokenRequest) { + if p.tokenRequestEnabled { return p.admitServiceAccount(nodeName, a) } return nil @@ -154,7 +162,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission. return p.admitLease(nodeName, a) case csiNodeResource: - if p.features.Enabled(features.CSINodeInfo) { + if p.csiNodeInfoEnabled { return p.admitCSINode(nodeName, a) } return admission.NewForbidden(a, fmt.Errorf("disabled by feature gates %s", features.CSINodeInfo)) @@ -294,7 +302,7 @@ func (p *Plugin) admitPodEviction(nodeName string, a admission.Attributes) error func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error { switch a.GetOperation() { case admission.Update: - if !p.features.Enabled(features.ExpandPersistentVolumes) { + if !p.expandPersistentVolumesEnabled { return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update persistentvolumeclaim metadata", nodeName)) } diff --git a/plugin/pkg/admission/noderestriction/admission_test.go b/plugin/pkg/admission/noderestriction/admission_test.go index d93a4bf5b90..b0463641387 100644 --- a/plugin/pkg/admission/noderestriction/admission_test.go +++ b/plugin/pkg/admission/noderestriction/admission_test.go @@ -28,6 +28,7 @@ import ( 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" @@ -53,18 +54,19 @@ var ( ) func init() { - if err := trEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: true}}); err != nil { - panic(err) - } - if err := trDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: false}}); err != nil { - panic(err) - } - 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) + // 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.TokenRequest: {Default: false}, + features.CSINodeInfo: {Default: false}, + features.ExpandPersistentVolumes: {Default: false}, } + utilruntime.Must(trEnabledFeature.Add(relevantFeatures)) + utilruntime.Must(trDisabledFeature.Add(relevantFeatures)) + utilruntime.Must(csiNodeInfoEnabledFeature.Add(relevantFeatures)) + utilruntime.Must(csiNodeInfoDisabledFeature.Add(relevantFeatures)) + + utilruntime.Must(trEnabledFeature.SetFromMap(map[string]bool{string(features.TokenRequest): true})) + utilruntime.Must(csiNodeInfoEnabledFeature.SetFromMap(map[string]bool{string(features.CSINodeInfo): true})) } func makeTestPod(namespace, name, node string, mirror bool) (*api.Pod, *corev1.Pod) { @@ -1233,7 +1235,7 @@ func Test_nodePlugin_Admit(t *testing.T) { t.Run(tt.name, func(t *testing.T) { c := NewPlugin(nodeidentifier.NewDefaultNodeIdentifier()) if tt.features != nil { - c.features = tt.features + c.InspectFeatureGates(tt.features) } c.podsGetter = tt.podsGetter err := c.Admit(context.TODO(), tt.attributes, nil) diff --git a/plugin/pkg/admission/serviceaccount/BUILD b/plugin/pkg/admission/serviceaccount/BUILD index 49b643db765..0222f20cfd7 100644 --- a/plugin/pkg/admission/serviceaccount/BUILD +++ b/plugin/pkg/admission/serviceaccount/BUILD @@ -27,7 +27,6 @@ go_library( "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", "//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/client-go/informers:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", @@ -42,7 +41,6 @@ go_test( deps = [ "//pkg/apis/core:go_default_library", "//pkg/controller:go_default_library", - "//pkg/features:go_default_library", "//pkg/kubelet/types:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", @@ -54,7 +52,6 @@ go_test( "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", "//staging/src/k8s.io/client-go/listers/core/v1:go_default_library", "//staging/src/k8s.io/client-go/tools/cache:go_default_library", - "//staging/src/k8s.io/component-base/featuregate:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], ) diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 11de1ad1005..0867718cc6c 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -34,14 +34,13 @@ import ( "k8s.io/apiserver/pkg/admission" genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer" "k8s.io/apiserver/pkg/storage/names" - utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/component-base/featuregate" podutil "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" - kubefeatures "k8s.io/kubernetes/pkg/features" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/serviceaccount" ) @@ -92,11 +91,12 @@ type Plugin struct { generateName func(string) string - featureGate featuregate.FeatureGate + boundServiceAccountTokenVolume bool } var _ admission.MutationInterface = &Plugin{} var _ admission.ValidationInterface = &Plugin{} +var _ genericadmissioninitializer.WantsFeatures = &Plugin{} var _ = genericadmissioninitializer.WantsExternalKubeClientSet(&Plugin{}) var _ = genericadmissioninitializer.WantsExternalKubeInformerFactory(&Plugin{}) @@ -117,11 +117,14 @@ func NewServiceAccount() *Plugin { RequireAPIToken: true, generateName: names.SimpleNameGenerator.GenerateName, - - featureGate: utilfeature.DefaultFeatureGate, } } +// InspectFeatureGates allows setting bools without taking a dep on a global variable +func (s *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { + s.boundServiceAccountTokenVolume = featureGates.Enabled(features.BoundServiceAccountTokenVolume) +} + // SetExternalKubeClientSet sets the client for the plugin func (s *Plugin) SetExternalKubeClientSet(cl kubernetes.Interface) { s.client = cl @@ -443,8 +446,8 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount, allVolumeNames := sets.NewString() for _, volume := range pod.Spec.Volumes { allVolumeNames.Insert(volume.Name) - if (!s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) && volume.Secret != nil && volume.Secret.SecretName == serviceAccountToken) || - (s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) && strings.HasPrefix(volume.Name, ServiceAccountVolumeName+"-")) { + if (!s.boundServiceAccountTokenVolume && volume.Secret != nil && volume.Secret.SecretName == serviceAccountToken) || + (s.boundServiceAccountTokenVolume && strings.HasPrefix(volume.Name, ServiceAccountVolumeName+"-")) { tokenVolumeName = volume.Name hasTokenVolume = true break @@ -453,7 +456,7 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount, // Determine a volume name for the ServiceAccountTokenSecret in case we need it if len(tokenVolumeName) == 0 { - if s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) { + if s.boundServiceAccountTokenVolume { tokenVolumeName = s.generateName(ServiceAccountVolumeName + "-") } else { // Try naming the volume the same as the serviceAccountToken, and uniquify if needed @@ -510,7 +513,7 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount, } func (s *Plugin) createVolume(tokenVolumeName, secretName string) api.Volume { - if s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) { + if s.boundServiceAccountTokenVolume { return api.Volume{ Name: tokenVolumeName, VolumeSource: api.VolumeSource{ diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index 029c6a53fdc..7111a77022f 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -33,27 +33,16 @@ import ( "k8s.io/client-go/kubernetes/fake" corev1listers "k8s.io/client-go/listers/core/v1" "k8s.io/client-go/tools/cache" - "k8s.io/component-base/featuregate" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/controller" - kubefeatures "k8s.io/kubernetes/pkg/features" kubelet "k8s.io/kubernetes/pkg/kubelet/types" ) var ( - deprecationDisabledFeature = featuregate.NewFeatureGate() - deprecationEnabledFeature = featuregate.NewFeatureGate() + deprecationDisabledBoundTokenVolume = false + deprecationEnabledBoundTokenVolume = true ) -func init() { - if err := deprecationDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{kubefeatures.BoundServiceAccountTokenVolume: {Default: false}}); err != nil { - panic(err) - } - if err := deprecationEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{kubefeatures.BoundServiceAccountTokenVolume: {Default: true}}); err != nil { - panic(err) - } -} - func TestIgnoresNonCreate(t *testing.T) { for _, op := range []admission.Operation{admission.Delete, admission.Connect} { handler := NewServiceAccount() @@ -290,7 +279,7 @@ func TestAutomountsAPIToken(t *testing.T) { serviceAccountUID := "12345" tokenName := "token-name" - if admit.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) { + if admit.boundServiceAccountTokenVolume { tokenName = generatedVolumeName } @@ -981,7 +970,7 @@ func TestAutomountIsBackwardsCompatible(t *testing.T) { admit := NewServiceAccount() admit.generateName = testGenerateName - admit.featureGate = deprecationEnabledFeature + admit.boundServiceAccountTokenVolume = deprecationEnabledBoundTokenVolume informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) admit.SetExternalKubeInformerFactory(informerFactory) @@ -1074,14 +1063,14 @@ var generatedVolumeName = testGenerateName(ServiceAccountVolumeName + "-") func testBoundServiceAccountTokenVolumePhases(t *testing.T, f func(*testing.T, func(*Plugin) *Plugin)) { t.Run("BoundServiceAccountTokenVolume disabled", func(t *testing.T) { f(t, func(s *Plugin) *Plugin { - s.featureGate = deprecationDisabledFeature + s.boundServiceAccountTokenVolume = deprecationDisabledBoundTokenVolume return s }) }) t.Run("BoundServiceAccountTokenVolume enabled", func(t *testing.T) { f(t, func(s *Plugin) *Plugin { - s.featureGate = deprecationEnabledFeature + s.boundServiceAccountTokenVolume = deprecationEnabledBoundTokenVolume return s }) }) diff --git a/plugin/pkg/admission/storage/storageobjectinuseprotection/BUILD b/plugin/pkg/admission/storage/storageobjectinuseprotection/BUILD index fe9bdee4b89..e1eb709df74 100644 --- a/plugin/pkg/admission/storage/storageobjectinuseprotection/BUILD +++ b/plugin/pkg/admission/storage/storageobjectinuseprotection/BUILD @@ -10,7 +10,8 @@ go_library( "//pkg/features:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library", + "//staging/src/k8s.io/component-base/featuregate:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], ) @@ -21,14 +22,11 @@ go_test( embed = [":go_default_library"], deps = [ "//pkg/apis/core:go_default_library", - "//pkg/features:go_default_library", "//pkg/volume/util:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apiserver/pkg/admission:go_default_library", - "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", - "//staging/src/k8s.io/component-base/featuregate/testing:go_default_library", "//vendor/github.com/davecgh/go-spew/spew:go_default_library", ], ) diff --git a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go index 764e0a815c7..908193d6e5e 100644 --- a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go +++ b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go @@ -20,10 +20,10 @@ import ( "context" "io" - "k8s.io/klog" - "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/util/feature" + "k8s.io/apiserver/pkg/admission/initializer" + "k8s.io/component-base/featuregate" + "k8s.io/klog" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/features" volumeutil "k8s.io/kubernetes/pkg/volume/util" @@ -45,9 +45,12 @@ func Register(plugins *admission.Plugins) { // storageProtectionPlugin holds state for and implements the admission plugin. type storageProtectionPlugin struct { *admission.Handler + + storageObjectInUseProtection bool } var _ admission.Interface = &storageProtectionPlugin{} +var _ initializer.WantsFeatures = &storageProtectionPlugin{} // newPlugin creates a new admission plugin. func newPlugin() *storageProtectionPlugin { @@ -67,7 +70,7 @@ var ( // This prevents users from deleting a PVC that's used by a running pod. // This also prevents admin from deleting a PV that's bound by a PVC func (c *storageProtectionPlugin) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error { - if !feature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) { + if !c.storageObjectInUseProtection { return nil } @@ -126,3 +129,11 @@ func (c *storageProtectionPlugin) admitPVC(a admission.Attributes) error { pvc.Finalizers = append(pvc.Finalizers, volumeutil.PVCProtectionFinalizer) return nil } + +func (c *storageProtectionPlugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { + c.storageObjectInUseProtection = featureGates.Enabled(features.StorageObjectInUseProtection) +} + +func (c *storageProtectionPlugin) ValidateInitialization() error { + return nil +} diff --git a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go index 7b86621c55a..b48ea070d57 100644 --- a/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go +++ b/plugin/pkg/admission/storage/storageobjectinuseprotection/admission_test.go @@ -27,10 +27,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" - utilfeature "k8s.io/apiserver/pkg/util/feature" - featuregatetesting "k8s.io/component-base/featuregate/testing" api "k8s.io/kubernetes/pkg/apis/core" - "k8s.io/kubernetes/pkg/features" volumeutil "k8s.io/kubernetes/pkg/volume/util" ) @@ -117,11 +114,11 @@ func TestAdmit(t *testing.T) { }, } - ctrl := newPlugin() - for _, test := range tests { t.Run(test.name, func(t *testing.T) { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StorageObjectInUseProtection, test.featureEnabled)() + ctrl := newPlugin() + ctrl.storageObjectInUseProtection = test.featureEnabled + obj := test.object.DeepCopyObject() attrs := admission.NewAttributesRecord( obj, // new object