Skip to content

Commit 83f6f27

Browse files
deads2kdeads2k
committed
remove global variable dep in admission
1 parent 675c2fb commit 83f6f27

File tree

9 files changed

+70
-65
lines changed

9 files changed

+70
-65
lines changed

‎plugin/pkg/admission/noderestriction/BUILD‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ go_library(
2929
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
3030
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
3131
"//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
32-
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
3332
"//staging/src/k8s.io/client-go/informers:go_default_library",
3433
"//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
3534
"//staging/src/k8s.io/component-base/featuregate:go_default_library",
@@ -54,6 +53,7 @@ go_test(
5453
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
5554
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
5655
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
56+
"//staging/src/k8s.io/apimachinery/pkg/util/runtime:go_default_library",
5757
"//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
5858
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
5959
"//staging/src/k8s.io/apiserver/pkg/authentication/user:go_default_library",

‎plugin/pkg/admission/noderestriction/admission.go‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"k8s.io/apimachinery/pkg/util/sets"
3232
"k8s.io/apiserver/pkg/admission"
3333
apiserveradmission "k8s.io/apiserver/pkg/admission/initializer"
34-
utilfeature "k8s.io/apiserver/pkg/util/feature"
3534
"k8s.io/client-go/informers"
3635
corev1lister "k8s.io/client-go/listers/core/v1"
3736
"k8s.io/component-base/featuregate"
@@ -63,7 +62,6 @@ func NewPlugin(nodeIdentifier nodeidentifier.NodeIdentifier) *Plugin {
6362
return &Plugin{
6463
Handler: admission.NewHandler(admission.Create, admission.Update, admission.Delete),
6564
nodeIdentifier: nodeIdentifier,
66-
features: utilfeature.DefaultFeatureGate,
6765
}
6866
}
6967

@@ -72,15 +70,25 @@ type Plugin struct {
7270
*admission.Handler
7371
nodeIdentifier nodeidentifier.NodeIdentifier
7472
podsGetter corev1lister.PodLister
75-
// allows overriding for testing
76-
features featuregate.FeatureGate
73+
74+
tokenRequestEnabled bool
75+
csiNodeInfoEnabled bool
76+
expandPersistentVolumesEnabled bool
7777
}
7878

7979
var (
80-
_ = admission.Interface(&Plugin{})
81-
_ = apiserveradmission.WantsExternalKubeInformerFactory(&Plugin{})
80+
_ admission.Interface = &Plugin{}
81+
_ apiserveradmission.WantsExternalKubeInformerFactory = &Plugin{}
82+
_ apiserveradmission.WantsFeatures = &Plugin{}
8283
)
8384

85+
// InspectFeatureGates allows setting bools without taking a dep on a global variable
86+
func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
87+
p.tokenRequestEnabled = featureGates.Enabled(features.TokenRequest)
88+
p.csiNodeInfoEnabled = featureGates.Enabled(features.CSINodeInfo)
89+
p.expandPersistentVolumesEnabled = featureGates.Enabled(features.ExpandPersistentVolumes)
90+
}
91+
8492
// SetExternalKubeInformerFactory registers an informer factory into Plugin
8593
func (p *Plugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
8694
p.podsGetter = f.Core().V1().Pods().Lister()
@@ -145,7 +153,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
145153
}
146154

147155
case svcacctResource:
148-
if p.features.Enabled(features.TokenRequest) {
156+
if p.tokenRequestEnabled {
149157
return p.admitServiceAccount(nodeName, a)
150158
}
151159
return nil
@@ -154,7 +162,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
154162
return p.admitLease(nodeName, a)
155163

156164
case csiNodeResource:
157-
if p.features.Enabled(features.CSINodeInfo) {
165+
if p.csiNodeInfoEnabled {
158166
return p.admitCSINode(nodeName, a)
159167
}
160168
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
294302
func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error {
295303
switch a.GetOperation() {
296304
case admission.Update:
297-
if !p.features.Enabled(features.ExpandPersistentVolumes) {
305+
if !p.expandPersistentVolumesEnabled {
298306
return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update persistentvolumeclaim metadata", nodeName))
299307
}
300308

‎plugin/pkg/admission/noderestriction/admission_test.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
"k8s.io/apimachinery/pkg/runtime/schema"
3030
"k8s.io/apimachinery/pkg/types"
31+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
3132
"k8s.io/apimachinery/pkg/util/sets"
3233
"k8s.io/apiserver/pkg/admission"
3334
"k8s.io/apiserver/pkg/authentication/user"
@@ -53,18 +54,19 @@ var (
5354
)
5455

5556
func init() {
56-
if err := trEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: true}}); err != nil {
57-
panic(err)
58-
}
59-
if err := trDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.TokenRequest: {Default: false}}); err != nil {
60-
panic(err)
61-
}
62-
if err := csiNodeInfoEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSINodeInfo: {Default: true}}); err != nil {
63-
panic(err)
64-
}
65-
if err := csiNodeInfoDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{features.CSINodeInfo: {Default: false}}); err != nil {
66-
panic(err)
57+
// all features need to be set on all featuregates for the tests. We set everything and then then the if's below override it.
58+
relevantFeatures := map[featuregate.Feature]featuregate.FeatureSpec{
59+
features.TokenRequest: {Default: false},
60+
features.CSINodeInfo: {Default: false},
61+
features.ExpandPersistentVolumes: {Default: false},
6762
}
63+
utilruntime.Must(trEnabledFeature.Add(relevantFeatures))
64+
utilruntime.Must(trDisabledFeature.Add(relevantFeatures))
65+
utilruntime.Must(csiNodeInfoEnabledFeature.Add(relevantFeatures))
66+
utilruntime.Must(csiNodeInfoDisabledFeature.Add(relevantFeatures))
67+
68+
utilruntime.Must(trEnabledFeature.SetFromMap(map[string]bool{string(features.TokenRequest): true}))
69+
utilruntime.Must(csiNodeInfoEnabledFeature.SetFromMap(map[string]bool{string(features.CSINodeInfo): true}))
6870
}
6971

7072
func makeTestPod(namespace, name, node string, mirror bool) (*api.Pod, *corev1.Pod) {
@@ -1233,7 +1235,7 @@ func Test_nodePlugin_Admit(t *testing.T) {
12331235
t.Run(tt.name, func(t *testing.T) {
12341236
c := NewPlugin(nodeidentifier.NewDefaultNodeIdentifier())
12351237
if tt.features != nil {
1236-
c.features = tt.features
1238+
c.InspectFeatureGates(tt.features)
12371239
}
12381240
c.podsGetter = tt.podsGetter
12391241
err := c.Admit(context.TODO(), tt.attributes, nil)

‎plugin/pkg/admission/serviceaccount/BUILD‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ go_library(
2727
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
2828
"//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
2929
"//staging/src/k8s.io/apiserver/pkg/storage/names:go_default_library",
30-
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
3130
"//staging/src/k8s.io/client-go/informers:go_default_library",
3231
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
3332
"//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
@@ -42,7 +41,6 @@ go_test(
4241
deps = [
4342
"//pkg/apis/core:go_default_library",
4443
"//pkg/controller:go_default_library",
45-
"//pkg/features:go_default_library",
4644
"//pkg/kubelet/types:go_default_library",
4745
"//staging/src/k8s.io/api/core/v1:go_default_library",
4846
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
@@ -54,7 +52,6 @@ go_test(
5452
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
5553
"//staging/src/k8s.io/client-go/listers/core/v1:go_default_library",
5654
"//staging/src/k8s.io/client-go/tools/cache:go_default_library",
57-
"//staging/src/k8s.io/component-base/featuregate:go_default_library",
5855
"//vendor/github.com/stretchr/testify/assert:go_default_library",
5956
],
6057
)

‎plugin/pkg/admission/serviceaccount/admission.go‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,13 @@ import (
3434
"k8s.io/apiserver/pkg/admission"
3535
genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer"
3636
"k8s.io/apiserver/pkg/storage/names"
37-
utilfeature "k8s.io/apiserver/pkg/util/feature"
3837
"k8s.io/client-go/informers"
3938
"k8s.io/client-go/kubernetes"
4039
corev1listers "k8s.io/client-go/listers/core/v1"
4140
"k8s.io/component-base/featuregate"
4241
podutil "k8s.io/kubernetes/pkg/api/pod"
4342
api "k8s.io/kubernetes/pkg/apis/core"
44-
kubefeatures "k8s.io/kubernetes/pkg/features"
43+
"k8s.io/kubernetes/pkg/features"
4544
"k8s.io/kubernetes/pkg/serviceaccount"
4645
)
4746

@@ -92,11 +91,12 @@ type Plugin struct {
9291

9392
generateName func(string) string
9493

95-
featureGate featuregate.FeatureGate
94+
boundServiceAccountTokenVolume bool
9695
}
9796

9897
var _ admission.MutationInterface = &Plugin{}
9998
var _ admission.ValidationInterface = &Plugin{}
99+
var _ genericadmissioninitializer.WantsFeatures = &Plugin{}
100100
var _ = genericadmissioninitializer.WantsExternalKubeClientSet(&Plugin{})
101101
var _ = genericadmissioninitializer.WantsExternalKubeInformerFactory(&Plugin{})
102102

@@ -117,11 +117,14 @@ func NewServiceAccount() *Plugin {
117117
RequireAPIToken: true,
118118

119119
generateName: names.SimpleNameGenerator.GenerateName,
120-
121-
featureGate: utilfeature.DefaultFeatureGate,
122120
}
123121
}
124122

123+
// InspectFeatureGates allows setting bools without taking a dep on a global variable
124+
func (s *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
125+
s.boundServiceAccountTokenVolume = featureGates.Enabled(features.BoundServiceAccountTokenVolume)
126+
}
127+
125128
// SetExternalKubeClientSet sets the client for the plugin
126129
func (s *Plugin) SetExternalKubeClientSet(cl kubernetes.Interface) {
127130
s.client = cl
@@ -443,8 +446,8 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount,
443446
allVolumeNames := sets.NewString()
444447
for _, volume := range pod.Spec.Volumes {
445448
allVolumeNames.Insert(volume.Name)
446-
if (!s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) && volume.Secret != nil && volume.Secret.SecretName == serviceAccountToken) ||
447-
(s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) && strings.HasPrefix(volume.Name, ServiceAccountVolumeName+"-")) {
449+
if (!s.boundServiceAccountTokenVolume && volume.Secret != nil && volume.Secret.SecretName == serviceAccountToken) ||
450+
(s.boundServiceAccountTokenVolume && strings.HasPrefix(volume.Name, ServiceAccountVolumeName+"-")) {
448451
tokenVolumeName = volume.Name
449452
hasTokenVolume = true
450453
break
@@ -453,7 +456,7 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount,
453456

454457
// Determine a volume name for the ServiceAccountTokenSecret in case we need it
455458
if len(tokenVolumeName) == 0 {
456-
if s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) {
459+
if s.boundServiceAccountTokenVolume {
457460
tokenVolumeName = s.generateName(ServiceAccountVolumeName + "-")
458461
} else {
459462
// Try naming the volume the same as the serviceAccountToken, and uniquify if needed
@@ -510,7 +513,7 @@ func (s *Plugin) mountServiceAccountToken(serviceAccount *corev1.ServiceAccount,
510513
}
511514

512515
func (s *Plugin) createVolume(tokenVolumeName, secretName string) api.Volume {
513-
if s.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) {
516+
if s.boundServiceAccountTokenVolume {
514517
return api.Volume{
515518
Name: tokenVolumeName,
516519
VolumeSource: api.VolumeSource{

‎plugin/pkg/admission/serviceaccount/admission_test.go‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -33,27 +33,16 @@ import (
3333
"k8s.io/client-go/kubernetes/fake"
3434
corev1listers "k8s.io/client-go/listers/core/v1"
3535
"k8s.io/client-go/tools/cache"
36-
"k8s.io/component-base/featuregate"
3736
api "k8s.io/kubernetes/pkg/apis/core"
3837
"k8s.io/kubernetes/pkg/controller"
39-
kubefeatures "k8s.io/kubernetes/pkg/features"
4038
kubelet "k8s.io/kubernetes/pkg/kubelet/types"
4139
)
4240

4341
var (
44-
deprecationDisabledFeature = featuregate.NewFeatureGate()
45-
deprecationEnabledFeature = featuregate.NewFeatureGate()
42+
deprecationDisabledBoundTokenVolume = false
43+
deprecationEnabledBoundTokenVolume = true
4644
)
4745

48-
func init() {
49-
if err := deprecationDisabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{kubefeatures.BoundServiceAccountTokenVolume: {Default: false}}); err != nil {
50-
panic(err)
51-
}
52-
if err := deprecationEnabledFeature.Add(map[featuregate.Feature]featuregate.FeatureSpec{kubefeatures.BoundServiceAccountTokenVolume: {Default: true}}); err != nil {
53-
panic(err)
54-
}
55-
}
56-
5746
func TestIgnoresNonCreate(t *testing.T) {
5847
for _, op := range []admission.Operation{admission.Delete, admission.Connect} {
5948
handler := NewServiceAccount()
@@ -290,7 +279,7 @@ func TestAutomountsAPIToken(t *testing.T) {
290279
serviceAccountUID := "12345"
291280

292281
tokenName := "token-name"
293-
if admit.featureGate.Enabled(kubefeatures.BoundServiceAccountTokenVolume) {
282+
if admit.boundServiceAccountTokenVolume {
294283
tokenName = generatedVolumeName
295284
}
296285

@@ -981,7 +970,7 @@ func TestAutomountIsBackwardsCompatible(t *testing.T) {
981970

982971
admit := NewServiceAccount()
983972
admit.generateName = testGenerateName
984-
admit.featureGate = deprecationEnabledFeature
973+
admit.boundServiceAccountTokenVolume = deprecationEnabledBoundTokenVolume
985974

986975
informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc())
987976
admit.SetExternalKubeInformerFactory(informerFactory)
@@ -1074,14 +1063,14 @@ var generatedVolumeName = testGenerateName(ServiceAccountVolumeName + "-")
10741063
func testBoundServiceAccountTokenVolumePhases(t *testing.T, f func(*testing.T, func(*Plugin) *Plugin)) {
10751064
t.Run("BoundServiceAccountTokenVolume disabled", func(t *testing.T) {
10761065
f(t, func(s *Plugin) *Plugin {
1077-
s.featureGate = deprecationDisabledFeature
1066+
s.boundServiceAccountTokenVolume = deprecationDisabledBoundTokenVolume
10781067
return s
10791068
})
10801069
})
10811070

10821071
t.Run("BoundServiceAccountTokenVolume enabled", func(t *testing.T) {
10831072
f(t, func(s *Plugin) *Plugin {
1084-
s.featureGate = deprecationEnabledFeature
1073+
s.boundServiceAccountTokenVolume = deprecationEnabledBoundTokenVolume
10851074
return s
10861075
})
10871076
})

‎plugin/pkg/admission/storage/storageobjectinuseprotection/BUILD‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ go_library(
1010
"//pkg/features:go_default_library",
1111
"//pkg/volume/util:go_default_library",
1212
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
13-
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
13+
"//staging/src/k8s.io/apiserver/pkg/admission/initializer:go_default_library",
14+
"//staging/src/k8s.io/component-base/featuregate:go_default_library",
1415
"//vendor/k8s.io/klog:go_default_library",
1516
],
1617
)
@@ -21,14 +22,11 @@ go_test(
2122
embed = [":go_default_library"],
2223
deps = [
2324
"//pkg/apis/core:go_default_library",
24-
"//pkg/features:go_default_library",
2525
"//pkg/volume/util:go_default_library",
2626
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
2727
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
2828
"//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
2929
"//staging/src/k8s.io/apiserver/pkg/admission:go_default_library",
30-
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
31-
"//staging/src/k8s.io/component-base/featuregate/testing:go_default_library",
3230
"//vendor/github.com/davecgh/go-spew/spew:go_default_library",
3331
],
3432
)

‎plugin/pkg/admission/storage/storageobjectinuseprotection/admission.go‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ import (
2020
"context"
2121
"io"
2222

23-
"k8s.io/klog"
24-
2523
"k8s.io/apiserver/pkg/admission"
26-
"k8s.io/apiserver/pkg/util/feature"
24+
"k8s.io/apiserver/pkg/admission/initializer"
25+
"k8s.io/component-base/featuregate"
26+
"k8s.io/klog"
2727
api "k8s.io/kubernetes/pkg/apis/core"
2828
"k8s.io/kubernetes/pkg/features"
2929
volumeutil "k8s.io/kubernetes/pkg/volume/util"
@@ -45,9 +45,12 @@ func Register(plugins *admission.Plugins) {
4545
// storageProtectionPlugin holds state for and implements the admission plugin.
4646
type storageProtectionPlugin struct {
4747
*admission.Handler
48+
49+
storageObjectInUseProtection bool
4850
}
4951

5052
var _ admission.Interface = &storageProtectionPlugin{}
53+
var _ initializer.WantsFeatures = &storageProtectionPlugin{}
5154

5255
// newPlugin creates a new admission plugin.
5356
func newPlugin() *storageProtectionPlugin {
@@ -67,7 +70,7 @@ var (
6770
// This prevents users from deleting a PVC that's used by a running pod.
6871
// This also prevents admin from deleting a PV that's bound by a PVC
6972
func (c *storageProtectionPlugin) Admit(ctx context.Context, a admission.Attributes, o admission.ObjectInterfaces) error {
70-
if !feature.DefaultFeatureGate.Enabled(features.StorageObjectInUseProtection) {
73+
if !c.storageObjectInUseProtection {
7174
return nil
7275
}
7376

@@ -126,3 +129,11 @@ func (c *storageProtectionPlugin) admitPVC(a admission.Attributes) error {
126129
pvc.Finalizers = append(pvc.Finalizers, volumeutil.PVCProtectionFinalizer)
127130
return nil
128131
}
132+
133+
func (c *storageProtectionPlugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
134+
c.storageObjectInUseProtection = featureGates.Enabled(features.StorageObjectInUseProtection)
135+
}
136+
137+
func (c *storageProtectionPlugin) ValidateInitialization() error {
138+
return nil
139+
}

0 commit comments

Comments
 (0)