From 92506a98fce4174e3ba92f6589ea9d6e8694efda Mon Sep 17 00:00:00 2001 From: "Christopher M. Luciano" Date: Tue, 9 Jun 2020 15:06:11 -0400 Subject: [PATCH] ingress: Update IngressClass feature and admission controller for v1 Signed-off-by: Christopher M. Luciano --- pkg/features/kube_features.go | 3 +- .../pkg/admission/defaultingressclass/BUILD | 6 ++-- .../defaultingressclass/admission.go | 36 ++++--------------- .../defaultingressclass/admission_test.go | 34 +++++++++--------- 4 files changed, 29 insertions(+), 50 deletions(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index e014efae8e5..073b972424d 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -568,6 +568,7 @@ const ( // owner: @robscott // beta: v1.18 + // ga: v1.19 // // Enables DefaultIngressClass admission controller. DefaultIngressClass featuregate.Feature = "DefaultIngressClass" @@ -678,7 +679,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS ServiceTopology: {Default: false, PreRelease: featuregate.Alpha}, ServiceAppProtocol: {Default: true, PreRelease: featuregate.Beta}, ImmutableEphemeralVolumes: {Default: true, PreRelease: featuregate.Beta}, - DefaultIngressClass: {Default: true, PreRelease: featuregate.Beta}, + DefaultIngressClass: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.20 HugePageStorageMediumSize: {Default: false, PreRelease: featuregate.Alpha}, ExternalPolicyForExternalIP: {Default: true, PreRelease: featuregate.GA}, // remove in 1.20 AnyVolumeDataSource: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/plugin/pkg/admission/defaultingressclass/BUILD b/plugin/pkg/admission/defaultingressclass/BUILD index 33d3ad32bb1..367c3cf3b3d 100644 --- a/plugin/pkg/admission/defaultingressclass/BUILD +++ b/plugin/pkg/admission/defaultingressclass/BUILD @@ -7,15 +7,14 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/apis/networking:go_default_library", - "//pkg/features:go_default_library", + "//staging/src/k8s.io/api/networking/v1:go_default_library", "//staging/src/k8s.io/api/networking/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels: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/client-go/informers:go_default_library", - "//staging/src/k8s.io/client-go/listers/networking/v1beta1:go_default_library", - "//staging/src/k8s.io/component-base/featuregate:go_default_library", + "//staging/src/k8s.io/client-go/listers/networking/v1:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", ], ) @@ -28,6 +27,7 @@ go_test( "//pkg/apis/core:go_default_library", "//pkg/apis/networking:go_default_library", "//pkg/controller:go_default_library", + "//staging/src/k8s.io/api/networking/v1:go_default_library", "//staging/src/k8s.io/api/networking/v1beta1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/plugin/pkg/admission/defaultingressclass/admission.go b/plugin/pkg/admission/defaultingressclass/admission.go index fb9f260c323..454e703ed9e 100644 --- a/plugin/pkg/admission/defaultingressclass/admission.go +++ b/plugin/pkg/admission/defaultingressclass/admission.go @@ -21,17 +21,16 @@ import ( "fmt" "io" + networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/labels" "k8s.io/apiserver/pkg/admission" genericadmissioninitializer "k8s.io/apiserver/pkg/admission/initializer" "k8s.io/client-go/informers" - networkingv1beta1listers "k8s.io/client-go/listers/networking/v1beta1" - "k8s.io/component-base/featuregate" + networkingv1listers "k8s.io/client-go/listers/networking/v1" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/apis/networking" - "k8s.io/kubernetes/pkg/features" ) const ( @@ -50,10 +49,7 @@ func Register(plugins *admission.Plugins) { // classDefaulterPlugin holds state for and implements the admission plugin. type classDefaulterPlugin struct { *admission.Handler - lister networkingv1beta1listers.IngressClassLister - - inspectedFeatures bool - defaultIngressClassEnabled bool + lister networkingv1listers.IngressClassLister } var _ admission.Interface = &classDefaulterPlugin{} @@ -67,31 +63,16 @@ func newPlugin() *classDefaulterPlugin { } } -// InspectFeatureGates allows setting bools without taking a dep on a global variable -func (a *classDefaulterPlugin) InspectFeatureGates(featureGates featuregate.FeatureGate) { - a.defaultIngressClassEnabled = featureGates.Enabled(features.DefaultIngressClass) - a.inspectedFeatures = true -} - // SetExternalKubeInformerFactory sets a lister and readyFunc for this // classDefaulterPlugin using the provided SharedInformerFactory. func (a *classDefaulterPlugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { - if !a.defaultIngressClassEnabled { - return - } - informer := f.Networking().V1beta1().IngressClasses() + informer := f.Networking().V1().IngressClasses() a.lister = informer.Lister() a.SetReadyFunc(informer.Informer().HasSynced) } // ValidateInitialization ensures lister is set. func (a *classDefaulterPlugin) ValidateInitialization() error { - if !a.inspectedFeatures { - return fmt.Errorf("InspectFeatureGates was not called") - } - if !a.defaultIngressClassEnabled { - return nil - } if a.lister == nil { return fmt.Errorf("missing lister") } @@ -101,10 +82,7 @@ func (a *classDefaulterPlugin) ValidateInitialization() error { // Admit sets the default value of a Ingress's class if the user did not specify // a class. func (a *classDefaulterPlugin) Admit(ctx context.Context, attr admission.Attributes, o admission.ObjectInterfaces) error { - if !a.defaultIngressClassEnabled { - return nil - } - if attr.GetResource().GroupResource() != networkingv1beta1.Resource("ingresses") { + if attr.GetResource().GroupResource() != networkingv1.Resource("ingresses") { return nil } @@ -147,13 +125,13 @@ func (a *classDefaulterPlugin) Admit(ctx context.Context, attr admission.Attribu } // getDefaultClass returns the default IngressClass from the store, or nil. -func getDefaultClass(lister networkingv1beta1listers.IngressClassLister) (*networkingv1beta1.IngressClass, error) { +func getDefaultClass(lister networkingv1listers.IngressClassLister) (*networkingv1.IngressClass, error) { list, err := lister.List(labels.Everything()) if err != nil { return nil, err } - defaultClasses := []*networkingv1beta1.IngressClass{} + defaultClasses := []*networkingv1.IngressClass{} for _, class := range list { if class.Annotations[networkingv1beta1.AnnotationIsDefaultIngressClass] == "true" { defaultClasses = append(defaultClasses, class) diff --git a/plugin/pkg/admission/defaultingressclass/admission_test.go b/plugin/pkg/admission/defaultingressclass/admission_test.go index 01a7e271606..eea5118f97b 100644 --- a/plugin/pkg/admission/defaultingressclass/admission_test.go +++ b/plugin/pkg/admission/defaultingressclass/admission_test.go @@ -22,6 +22,7 @@ import ( "reflect" "testing" + networkingv1 "k8s.io/api/networking/v1" networkingv1beta1 "k8s.io/api/networking/v1beta1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,7 +36,7 @@ import ( ) func TestAdmission(t *testing.T) { - defaultClass1 := &networkingv1beta1.IngressClass{ + defaultClass1 := &networkingv1.IngressClass{ TypeMeta: metav1.TypeMeta{ Kind: "IngressClass", }, @@ -46,7 +47,7 @@ func TestAdmission(t *testing.T) { }, }, } - defaultClass2 := &networkingv1beta1.IngressClass{ + defaultClass2 := &networkingv1.IngressClass{ ObjectMeta: metav1.ObjectMeta{ Name: "default2", Annotations: map[string]string{ @@ -55,7 +56,7 @@ func TestAdmission(t *testing.T) { }, } // Class that has explicit default = false - classWithFalseDefault := &networkingv1beta1.IngressClass{ + classWithFalseDefault := &networkingv1.IngressClass{ TypeMeta: metav1.TypeMeta{ Kind: "IngressClass", }, @@ -67,7 +68,7 @@ func TestAdmission(t *testing.T) { }, } // Class with missing default annotation (=non-default) - classWithNoDefault := &networkingv1beta1.IngressClass{ + classWithNoDefault := &networkingv1.IngressClass{ TypeMeta: metav1.TypeMeta{ Kind: "IngressClass", }, @@ -76,7 +77,7 @@ func TestAdmission(t *testing.T) { }, } // Class with empty default annotation (=non-default) - classWithEmptyDefault := &networkingv1beta1.IngressClass{ + classWithEmptyDefault := &networkingv1.IngressClass{ TypeMeta: metav1.TypeMeta{ Kind: "IngressClass", }, @@ -90,7 +91,7 @@ func TestAdmission(t *testing.T) { testCases := []struct { name string - classes []*networkingv1beta1.IngressClass + classes []*networkingv1.IngressClass classField *string classAnnotation *string expectedClass *string @@ -98,7 +99,7 @@ func TestAdmission(t *testing.T) { }{ { name: "no default, no modification of Ingress", - classes: []*networkingv1beta1.IngressClass{classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: nil, classAnnotation: nil, expectedClass: nil, @@ -106,7 +107,7 @@ func TestAdmission(t *testing.T) { }, { name: "one default, modify Ingress with class=nil", - classes: []*networkingv1beta1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: nil, classAnnotation: nil, expectedClass: utilpointer.StringPtr(defaultClass1.Name), @@ -114,7 +115,7 @@ func TestAdmission(t *testing.T) { }, { name: "one default, no modification of Ingress with class field=''", - classes: []*networkingv1beta1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: utilpointer.StringPtr(""), classAnnotation: nil, expectedClass: utilpointer.StringPtr(""), @@ -122,7 +123,7 @@ func TestAdmission(t *testing.T) { }, { name: "one default, no modification of Ingress with class field='foo'", - classes: []*networkingv1beta1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: utilpointer.StringPtr("foo"), classAnnotation: nil, expectedClass: utilpointer.StringPtr("foo"), @@ -130,7 +131,7 @@ func TestAdmission(t *testing.T) { }, { name: "one default, no modification of Ingress with class annotation='foo'", - classes: []*networkingv1beta1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{defaultClass1, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: nil, classAnnotation: utilpointer.StringPtr("foo"), expectedClass: nil, @@ -138,15 +139,15 @@ func TestAdmission(t *testing.T) { }, { name: "two defaults, error with Ingress with class field=nil", - classes: []*networkingv1beta1.IngressClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: nil, classAnnotation: nil, expectedClass: nil, - expectedError: errors.NewForbidden(networkingv1beta1.Resource("ingresses"), "testing", errors.NewInternalError(fmt.Errorf("2 default IngressClasses were found, only 1 allowed"))), + expectedError: errors.NewForbidden(networkingv1.Resource("ingresses"), "testing", errors.NewInternalError(fmt.Errorf("2 default IngressClasses were found, only 1 allowed"))), }, { name: "two defaults, no modification with Ingress with class field=''", - classes: []*networkingv1beta1.IngressClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, + classes: []*networkingv1.IngressClass{defaultClass1, defaultClass2, classWithFalseDefault, classWithNoDefault, classWithEmptyDefault}, classField: utilpointer.StringPtr(""), classAnnotation: nil, expectedClass: utilpointer.StringPtr(""), @@ -157,11 +158,10 @@ func TestAdmission(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { ctrl := newPlugin() - ctrl.defaultIngressClassEnabled = true informerFactory := informers.NewSharedInformerFactory(nil, controller.NoResyncPeriodFunc()) ctrl.SetExternalKubeInformerFactory(informerFactory) for _, c := range testCase.classes { - informerFactory.Networking().V1beta1().IngressClasses().Informer().GetStore().Add(c) + informerFactory.Networking().V1().IngressClasses().Informer().GetStore().Add(c) } ingress := &networking.Ingress{ObjectMeta: metav1.ObjectMeta{Name: "testing", Namespace: "testing"}} @@ -178,7 +178,7 @@ func TestAdmission(t *testing.T) { api.Kind("Ingress").WithVersion("version"), ingress.Namespace, ingress.Name, - networkingv1beta1.Resource("ingresses").WithVersion("version"), + networkingv1.Resource("ingresses").WithVersion("version"), "", // subresource admission.Create, &metav1.CreateOptions{},