remove global variable dep in admission

This commit is contained in:
David Eads 2019-11-05 14:51:38 -05:00
parent 675c2fb924
commit 83f6f2717e
9 changed files with 70 additions and 65 deletions

View File

@ -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",

View File

@ -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))
}

View File

@ -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)

View File

@ -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",
],
)

View File

@ -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{

View File

@ -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
})
})

View File

@ -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",
],
)

View File

@ -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
}

View File

@ -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