diff --git a/charts/ingress-nginx/templates/admission-webhooks/validating-webhook.yaml b/charts/ingress-nginx/templates/admission-webhooks/validating-webhook.yaml index a65b0ab04..bdcdbb6dd 100644 --- a/charts/ingress-nginx/templates/admission-webhooks/validating-webhook.yaml +++ b/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 diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index 8aab80d58..f8ff31ba3 100644 --- a/cmd/nginx/main.go +++ b/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) - if !k8s.IsNetworkingIngressAvailable { + var isNetworkingIngressAvailable bool + + 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", diff --git a/internal/admission/controller/main.go b/internal/admission/controller/main.go index 3f134d902..b6b663e8a 100644 --- a/internal/admission/controller/main.go +++ b/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{ - Group: networking.GroupName, - Version: "v1beta1", - Resource: "ingresses", - } - - networkingV1Resource = metav1.GroupVersionResource{ - Group: networking.GroupName, - Version: "v1", - Resource: "ingresses", + ingressResource = metav1.GroupVersionKind{ + Group: networking.GroupName, + Version: "v1beta1", + Kind: "Ingress", } ) @@ -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{} diff --git a/internal/admission/controller/main_test.go b/internal/admission/controller/main_test.go index 7eddeb3af..b745c14a2 100644 --- a/internal/admission/controller/main_test.go +++ b/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"}, - }, - }) - 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"}, + Kind: v1.GroupVersionKind{Group: networking.GroupName, Version: "v1beta1", Kind: "Ingress"}, Object: runtime.RawExtension{ Raw: []byte{0xff}, }, diff --git a/internal/ingress/annotations/class/main.go b/internal/ingress/annotations/class/main.go index 6b6bfa434..0f5284f43 100644 --- a/internal/ingress/annotations/class/main.go +++ b/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 } diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 60c231df8..bcc8f67d6 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/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") diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index f8983cc9e..19eb877c9 100644 --- a/internal/ingress/status/status_test.go +++ b/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 diff --git a/internal/k8s/main.go b/internal/k8s/main.go index d10416c73..c3b9cb350 100644 --- a/internal/k8s/main.go +++ b/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 -var IsNetworkingIngressAvailable bool +// IsIngressV1Beta1Ready indicates if the running Kubernetes version is at least v1.18.0 +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 diff --git a/test/e2e/admission/admission.go b/test/e2e/admission/admission.go index 7c452a8a5..520691942 100644 --- a/test/e2e/admission/admission.go +++ b/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 +} diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index a2d8107f5..f16a8948f 100644 --- a/test/e2e/framework/framework.go +++ b/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" @@ -63,7 +64,8 @@ var ( type Framework struct { BaseName string - IsIngressV1Ready bool + 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.IsIngressV1Ready = isIngressV1Ready + _, f.IsIngressV1Beta1Ready, f.IsIngressV1Ready = k8s.NetworkingIngressAvailable(f.KubeClientSet) } f.Namespace, err = CreateKubeNamespace(f.BaseName, f.KubeClientSet) diff --git a/test/e2e/ingress/pathtype_exact.go b/test/e2e/ingress/pathtype_exact.go index 7f6820537..c3be0d327 100644 --- a/test/e2e/ingress/pathtype_exact.go +++ b/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") } diff --git a/test/e2e/settings/ingress_class.go b/test/e2e/settings/ingress_class.go index 32e42a223..7d7c07cb6 100644 --- a/test/e2e/settings/ingress_class.go +++ b/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