Enable validation of ingress definitions from extensions package

charts/ingress-nginx/templates/admission-webhooks/validating-webhook.yaml

@@ -13,12 +13,12 @@ metadata:
   name: {{ include "ingress-nginx.fullname" . }}-admission
 webhooks:
   - name: validate.nginx.ingress.kubernetes.io
+    matchPolicy: Equivalent
     rules:
       - apiGroups:
           - networking.k8s.io
         apiVersions:
           - v1beta1
-          - v1
         operations:
           - CREATE
           - UPDATE

cmd/nginx/main.go

@@ -104,12 +104,14 @@ func main() {
 	conf.FakeCertificate = ssl.GetFakeSSLCert()
 	klog.InfoS("SSL fake certificate created", "file", conf.FakeCertificate.PemFileName)
 
-	k8s.IsNetworkingIngressAvailable, k8s.IsIngressV1Ready = k8s.NetworkingIngressAvailable(kubeClient)
+	var isNetworkingIngressAvailable bool
-	if !k8s.IsNetworkingIngressAvailable {
+
+	isNetworkingIngressAvailable, k8s.IsIngressV1Beta1Ready, _ = k8s.NetworkingIngressAvailable(kubeClient)
+	if !isNetworkingIngressAvailable {
 		klog.Fatalf("ingress-nginx requires Kubernetes v1.14.0 or higher")
 	}
 
-	if k8s.IsIngressV1Ready {
+	if k8s.IsIngressV1Beta1Ready {
 		klog.InfoS("Enabling new Ingress features available since Kubernetes v1.18")
 		k8s.IngressClass, err = kubeClient.NetworkingV1beta1().IngressClasses().
 			Get(context.TODO(), class.IngressClass, metav1.GetOptions{})
@@ -206,11 +208,13 @@ func handleSigterm(ngx *controller.NGINXController, exit exiter) {
 // the in-cluster config is missing or fails, we fallback to the default config.
 func createApiserverClient(apiserverHost, rootCAFile, kubeConfig string) (*kubernetes.Clientset, error) {
 	cfg, err := clientcmd.BuildConfigFromFlags(apiserverHost, kubeConfig)
-
 	if err != nil {
 		return nil, err
 	}
 
+	// TODO: remove after k8s v1.22
+	cfg.WarningHandler = rest.NoWarnings{}
+
 	// Configure the User-Agent used for the HTTP requests made to the API server.
 	cfg.UserAgent = fmt.Sprintf(
 		"%s/%s (%s/%s) ingress-nginx/%s",

internal/admission/controller/main.go

@@ -23,6 +23,7 @@ import (
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	networking "k8s.io/api/networking/v1beta1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -43,16 +44,10 @@ type IngressAdmission struct {
 }
 
 var (
-	networkingV1Beta1Resource = metav1.GroupVersionResource{
+	ingressResource = metav1.GroupVersionKind{
 		Group:   networking.GroupName,
 		Version: "v1beta1",
-		Resource: "ingresses",
+		Kind:    "Ingress",
-	}
-
-	networkingV1Resource = metav1.GroupVersionResource{
-		Group:    networking.GroupName,
-		Version:  "v1",
-		Resource: "ingresses",
 	}
 )
 
@@ -75,9 +70,9 @@ func (ia *IngressAdmission) HandleAdmission(obj runtime.Object) (runtime.Object,
 		convertV1beta1AdmissionReviewToAdmissionAdmissionReview(reviewv1beta1, review)
 	}
 
-	if review.Request.Resource != networkingV1Beta1Resource && review.Request.Resource != networkingV1Resource {
+	if !apiequality.Semantic.DeepEqual(review.Request.Kind, ingressResource) {
 		return nil, fmt.Errorf("rejecting admission review because the request does not contain an Ingress resource but %s with name %s in namespace %s",
-			review.Request.Resource.String(), review.Request.Name, review.Request.Namespace)
+			review.Request.Kind.String(), review.Request.Name, review.Request.Namespace)
 	}
 
 	status := &admissionv1.AdmissionResponse{}

internal/admission/controller/main_test.go

@@ -25,7 +25,6 @@ import (
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/json"
-	"k8s.io/kubernetes/pkg/apis/extensions"
 )
 
 const testIngressName = "testIngressName"
@@ -58,7 +57,7 @@ func TestHandleAdmission(t *testing.T) {
 
 	result, err := adm.HandleAdmission(&admissionv1.AdmissionReview{
 		Request: &admissionv1.AdmissionRequest{
-			Resource: v1.GroupVersionResource{Group: "", Version: "v1", Resource: "pod"},
+			Kind: v1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"},
 		},
 	})
 	if err == nil {
@@ -72,16 +71,7 @@ func TestHandleAdmission(t *testing.T) {
 
 	result, err = adm.HandleAdmission(&admissionv1.AdmissionReview{
 		Request: &admissionv1.AdmissionRequest{
-			Resource: v1.GroupVersionResource{Group: extensions.GroupName, Version: "v1beta1", Resource: "ingresses"},
+			Kind: v1.GroupVersionKind{Group: networking.GroupName, Version: "v1beta1", Kind: "Ingress"},
-		},
-	})
-	if err == nil {
-		t.Fatalf("with extensions/v1beta1 Ingress resource, the check should not pass")
-	}
-
-	result, err = adm.HandleAdmission(&admissionv1.AdmissionReview{
-		Request: &admissionv1.AdmissionRequest{
-			Resource: v1.GroupVersionResource{Group: networking.GroupName, Version: "v1beta1", Resource: "ingresses"},
 			Object: runtime.RawExtension{
 				Raw: []byte{0xff},
 			},

internal/ingress/annotations/class/main.go

@@ -53,7 +53,7 @@ func IsValid(ing *networking.Ingress) bool {
 	}
 
 	// 2. k8s < v1.18. Check default annotation
-	if !k8s.IsIngressV1Ready {
+	if !k8s.IsIngressV1Beta1Ready {
 		return IngressClass == DefaultClass
 	}
 

internal/ingress/controller/store/store_test.go

@@ -40,13 +40,10 @@ import (
 	"k8s.io/ingress-nginx/internal/ingress"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/class"
 	"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
-	"k8s.io/ingress-nginx/internal/k8s"
 	"k8s.io/ingress-nginx/test/e2e/framework"
 )
 
 func TestStore(t *testing.T) {
-	k8s.IsNetworkingIngressAvailable = true
-
 	//TODO: move env definition to docker image?
 	os.Setenv("KUBEBUILDER_ASSETS", "/usr/local/bin")
 

internal/ingress/status/status_test.go

@@ -301,9 +301,6 @@ func TestStatusActions(t *testing.T) {
 		t.Fatalf("expected a valid Sync")
 	}
 
-	// assume k8s >= 1.14 as the rest of the test
-	k8s.IsNetworkingIngressAvailable = true
-
 	fk := fkSync.(statusSync)
 
 	// start it and wait for the election and syn actions

internal/k8s/main.go

@@ -128,10 +128,10 @@ func MetaNamespaceKey(obj interface{}) string {
 	return key
 }
 
-// IsNetworkingIngressAvailable indicates if package "k8s.io/api/networking/v1beta1" is available or not
+// IsIngressV1Beta1Ready indicates if the running Kubernetes version is at least v1.18.0
-var IsNetworkingIngressAvailable bool
+var IsIngressV1Beta1Ready bool
 
-// IsIngressV1Ready indicates if the running Kubernetes version is at least v1.18.0
+// IsIngressV1Ready indicates if the running Kubernetes version is at least v1.19.0
 var IsIngressV1Ready bool
 
 // IngressClass indicates the class of the Ingress to use as filter
@@ -143,23 +143,24 @@ const IngressNGINXController = "k8s.io/ingress-nginx"
 
 // NetworkingIngressAvailable checks if the package "k8s.io/api/networking/v1beta1"
 // is available or not and if Ingress V1 is supported (k8s >= v1.18.0)
-func NetworkingIngressAvailable(client clientset.Interface) (bool, bool) {
+func NetworkingIngressAvailable(client clientset.Interface) (bool, bool, bool) {
 	// check kubernetes version to use new ingress package or not
 	version114, _ := version.ParseGeneric("v1.14.0")
 	version118, _ := version.ParseGeneric("v1.18.0")
+	version119, _ := version.ParseGeneric("v1.19.0")
 
 	serverVersion, err := client.Discovery().ServerVersion()
 	if err != nil {
-		return false, false
+		return false, false, false
 	}
 
 	runningVersion, err := version.ParseGeneric(serverVersion.String())
 	if err != nil {
 		klog.ErrorS(err, "unexpected error parsing running Kubernetes version")
-		return false, false
+		return false, false, false
 	}
 
-	return runningVersion.AtLeast(version114), runningVersion.AtLeast(version118)
+	return runningVersion.AtLeast(version114), runningVersion.AtLeast(version118), runningVersion.AtLeast(version119)
 }
 
 // default path type is Prefix to not break existing definitions

test/e2e/admission/admission.go

@@ -17,13 +17,16 @@ limitations under the License.
 package admission
 
 import (
+	"bytes"
 	"context"
 	"fmt"
+	"net/http"
 	"os/exec"
 	"strings"
 
 	"github.com/onsi/ginkgo"
 	"github.com/stretchr/testify/assert"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/ingress-nginx/test/e2e/framework"
@@ -37,6 +40,11 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() {
 		f.NewSlowEchoDeployment()
 	})
 
+	ginkgo.AfterEach(func() {
+		err := uninstallChart(f)
+		assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
+	})
+
 	ginkgo.It("should not allow overlaps of host and paths without canary annotations", func() {
 		host := "admission-test"
 
@@ -52,9 +60,6 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() {
 		secondIngress := framework.NewSingleIngress("second-ingress", "/", host, f.Namespace, framework.EchoService, 80, nil)
 		_, err = f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), secondIngress, metav1.CreateOptions{})
 		assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should return an error")
-
-		err = uninstallChart(f)
-		assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
 	})
 
 	ginkgo.It("should allow overlaps of host and paths with canary annotation", func() {
@@ -76,9 +81,6 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() {
 		secondIngress := framework.NewSingleIngress("second-ingress", "/", host, f.Namespace, framework.SlowEchoService, 80, canaryAnnotations)
 		_, err = f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), secondIngress, metav1.CreateOptions{})
 		assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress with the same host and path should not return an error using a canary annotation")
-
-		err = uninstallChart(f)
-		assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
 	})
 
 	ginkgo.It("should return an error if there is an error validating the ingress definition", func() {
@@ -90,11 +92,65 @@ var _ = framework.IngressNginxDescribe("[Serial] admission controller", func() {
 		firstIngress := framework.NewSingleIngress("first-ingress", "/", host, f.Namespace, framework.EchoService, 80, annotations)
 		_, err := f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Create(context.TODO(), firstIngress, metav1.CreateOptions{})
 		assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error")
-
-		err = uninstallChart(f)
-		assert.Nil(ginkgo.GinkgoT(), err, "uninstalling helm chart")
 	})
 
+	ginkgo.It("should not return an error the ingress definition uses the deprecated extensions package", func() {
+		err := createIngress(f.Namespace, validIngress)
+		assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl")
+
+		f.WaitForNginxConfiguration(func(cfg string) bool {
+			return strings.Contains(cfg, "extensions")
+		})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", "extensions").
+			Expect().
+			Status(http.StatusOK)
+	})
+
+	ginkgo.It("should return an error if the ingress definition uses the deprecated extensions package and invalid annotations", func() {
+		err := createIngress(f.Namespace, invalidIngress)
+		assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl")
+
+		_, err = f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Get(context.TODO(), "extensions", metav1.GetOptions{})
+		if !apierrors.IsNotFound(err) {
+			assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error")
+		}
+	})
+
+	ginkgo.It("should not return an error if the Ingress V1 definition is valid", func() {
+		if !f.IsIngressV1Ready {
+			ginkgo.Skip("Test requires Kubernetes v1.19 or higher")
+		}
+
+		err := createIngress(f.Namespace, validV1Ingress)
+		assert.Nil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl")
+
+		f.WaitForNginxConfiguration(func(cfg string) bool {
+			return strings.Contains(cfg, "extensions")
+		})
+
+		f.HTTPTestClient().
+			GET("/").
+			WithHeader("Host", "extensions").
+			Expect().
+			Status(http.StatusOK)
+	})
+
+	ginkgo.It("should return an error if the Ingress V1 definition contains invalid annotations", func() {
+		if !f.IsIngressV1Ready {
+			ginkgo.Skip("Test requires Kubernetes v1.19 or higher")
+		}
+
+		err := createIngress(f.Namespace, invalidV1Ingress)
+		assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress using kubectl")
+
+		_, err = f.KubeClientSet.NetworkingV1beta1().Ingresses(f.Namespace).Get(context.TODO(), "extensions", metav1.GetOptions{})
+		if !apierrors.IsNotFound(err) {
+			assert.NotNil(ginkgo.GinkgoT(), err, "creating an ingress with invalid configuration should return an error")
+		}
+	})
 })
 
 func uninstallChart(f *framework.Framework) error {
@@ -106,3 +162,104 @@ func uninstallChart(f *framework.Framework) error {
 
 	return nil
 }
+
+const (
+	validIngress = `
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  name: extensions
+spec:
+  rules:
+  - host: extensions
+    http:
+      paths:
+      - path: /
+        backend:
+          serviceName: echo
+          servicePort: 80
+---
+`
+
+	invalidIngress = `
+apiVersion: extensions/v1beta1
+kind: Ingress
+metadata:
+  name: extensions
+  annotations:
+    nginx.ingress.kubernetes.io/configuration-snippet: |
+      invalid directive
+spec:
+  rules:
+  - host: extensions
+    http:
+      paths:
+      - path: /
+        backend:
+          serviceName: echo
+          servicePort: 80
+---
+`
+
+	validV1Ingress = `
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: extensions
+spec:
+  rules:
+  - host: extensions
+    http:
+      paths:
+      - path: /
+        pathType: Prefix
+        backend:
+          service:
+            name: echo
+            port:
+              number: 80
+
+---
+`
+
+	invalidV1Ingress = `
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: extensions
+  annotations:
+    nginx.ingress.kubernetes.io/configuration-snippet: |
+      invalid directive
+spec:
+  rules:
+  - host: extensions
+    http:
+      paths:
+      - path: /
+        pathType: Prefix
+        backend:
+          service:
+            name: echo
+            port:
+              number: 80
+---
+`
+)
+
+func createIngress(namespace, ingressDefinition string) error {
+	var (
+		execErr bytes.Buffer
+	)
+
+	cmd := exec.Command("/bin/bash", "-c", fmt.Sprintf("%v --warnings-as-errors=false apply --namespace %s -f -", framework.KubectlPath, namespace))
+	cmd.Stdin = strings.NewReader(ingressDefinition)
+	cmd.Stderr = &execErr
+
+	err := cmd.Run()
+	if err != nil {
+		stderr := strings.TrimSpace(execErr.String())
+		return fmt.Errorf("Kubectl error: %v\n%v", err, stderr)
+	}
+
+	return nil
+}

test/e2e/framework/framework.go

@@ -38,6 +38,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/rest"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/ingress-nginx/internal/k8s"
 	"k8s.io/klog/v2"
@@ -64,6 +65,7 @@ type Framework struct {
 	BaseName string
 
 	IsIngressV1Ready      bool
+	IsIngressV1Beta1Ready bool
 
 	// A Kubernetes and Service Catalog client
 	KubeClientSet          kubernetes.Interface
@@ -97,11 +99,14 @@ func (f *Framework) BeforeEach() {
 	if f.KubeClientSet == nil {
 		f.KubeConfig, err = kubeframework.LoadConfig()
 		assert.Nil(ginkgo.GinkgoT(), err, "loading a kubernetes client configuration")
+
+		// TODO: remove after k8s v1.22
+		f.KubeConfig.WarningHandler = rest.NoWarnings{}
+
 		f.KubeClientSet, err = kubernetes.NewForConfig(f.KubeConfig)
 		assert.Nil(ginkgo.GinkgoT(), err, "creating a kubernetes client")
 
-		_, isIngressV1Ready := k8s.NetworkingIngressAvailable(f.KubeClientSet)
+		_, f.IsIngressV1Beta1Ready, f.IsIngressV1Ready = k8s.NetworkingIngressAvailable(f.KubeClientSet)
-		f.IsIngressV1Ready = isIngressV1Ready
 	}
 
 	f.Namespace, err = CreateKubeNamespace(f.BaseName, f.KubeClientSet)

test/e2e/ingress/pathtype_exact.go

@@ -35,7 +35,7 @@ var _ = framework.IngressNginxDescribe("[Ingress] [PathType] exact", func() {
 	})
 
 	ginkgo.It("should choose exact location for /exact", func() {
-		if !f.IsIngressV1Ready {
+		if !f.IsIngressV1Beta1Ready {
 			ginkgo.Skip("Test requires Kubernetes v1.18 or higher")
 		}
 

test/e2e/settings/ingress_class.go

@@ -28,6 +28,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	networking "k8s.io/api/networking/v1beta1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"k8s.io/ingress-nginx/internal/ingress/annotations/class"
@@ -40,6 +41,8 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 
 	var doOnce sync.Once
 
+	testIngressClassName := "test-new-ingress-class"
+
 	ginkgo.BeforeEach(func() {
 		f.NewEchoDeploymentWithReplicas(1)
 
@@ -63,6 +66,20 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 					Name:     "ingress-nginx-class",
 				},
 			}, metav1.CreateOptions{})
+
+			_, err := f.KubeClientSet.NetworkingV1beta1().IngressClasses().
+				Create(context.TODO(), &networking.IngressClass{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: testIngressClassName,
+					},
+					Spec: networking.IngressClassSpec{
+						Controller: k8s.IngressNGINXController,
+					},
+				}, metav1.CreateOptions{})
+
+			if !apierrors.IsAlreadyExists(err) {
+				assert.Nil(ginkgo.GinkgoT(), err, "creating IngressClass")
+			}
 		})
 	})
 
@@ -193,28 +210,11 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 	})
 
 	ginkgo.It("check scenarios for IngressClass and ingress.class annotation", func() {
-		if !f.IsIngressV1Ready {
+		if !f.IsIngressV1Beta1Ready {
 			ginkgo.Skip("Test requires Kubernetes v1.18 or higher")
 		}
 
-		ingressClassName := "test-new-ingress-class"
-
-		ingressClass, err := f.KubeClientSet.NetworkingV1beta1().IngressClasses().
-			Create(context.TODO(), &networking.IngressClass{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: ingressClassName,
-				},
-				Spec: networking.IngressClassSpec{
-					Controller: k8s.IngressNGINXController,
-				},
-			}, metav1.CreateOptions{})
-
-		if ingressClass == nil {
-			assert.Nil(ginkgo.GinkgoT(), err, "creating IngressClass")
-		}
-
 		pod := f.GetIngressNGINXPod()
-		serviceAccount := pod.Spec.ServiceAccountName
 
 		crb, err := f.KubeClientSet.RbacV1().ClusterRoleBindings().Get(context.Background(), "ingress-nginx-class", metav1.GetOptions{})
 		assert.Nil(ginkgo.GinkgoT(), err, "searching cluster role binding")
@@ -223,7 +223,7 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 		crb.Subjects = append(crb.Subjects, rbacv1.Subject{
 			APIGroup:  "",
 			Kind:      "ServiceAccount",
-			Name:      serviceAccount,
+			Name:      pod.Spec.ServiceAccountName,
 			Namespace: f.Namespace,
 		})
 
@@ -240,7 +240,7 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 				args = append(args, v)
 			}
 
-			args = append(args, fmt.Sprintf("--ingress-class=%v", ingressClassName))
+			args = append(args, fmt.Sprintf("--ingress-class=%v", testIngressClassName))
 			deployment.Spec.Template.Spec.Containers[0].Args = args
 			_, err := f.KubeClientSet.AppsV1().Deployments(f.Namespace).Update(context.TODO(), deployment, metav1.UpdateOptions{})
 			return err
@@ -251,10 +251,10 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 
 		ginkgo.By("only having IngressClassName")
 		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, nil)
-		ing.Spec.IngressClassName = &ingressClassName
+		ing.Spec.IngressClassName = &testIngressClassName
 		f.EnsureIngress(ing)
 
-		f.WaitForNginxServer(host, func(cfg string) bool {
+		f.WaitForNginxConfiguration(func(cfg string) bool {
 			return strings.Contains(cfg, fmt.Sprintf("server_name %v", host))
 		})
 
@@ -269,7 +269,7 @@ var _ = framework.IngressNginxDescribe("[Flag] ingress-class", func() {
 		assert.Nil(ginkgo.GinkgoT(), err)
 
 		ing.Annotations = map[string]string{
-			class.IngressKey: ingressClassName,
+			class.IngressKey: testIngressClassName,
 		}
 		ing.Spec.IngressClassName = nil