diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 79ae489c73..a6863d31da 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -304,6 +304,42 @@ func New(checkOCSP bool, ) store.listers.Pod.Store = store.informers.Pod.GetStore() + ingDeleteHandler := func(obj interface{}) { + ing, ok := obj.(*extensions.Ingress) + if !ok { + // If we reached here it means the ingress was deleted but its final state is unrecorded. + tombstone, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + klog.Errorf("couldn't get object from tombstone %#v", obj) + return + } + ing, ok = tombstone.Obj.(*extensions.Ingress) + if !ok { + klog.Errorf("Tombstone contained object that is not an Ingress: %#v", obj) + return + } + } + if !class.IsValid(ing) { + klog.Infof("ignoring delete for ingress %v based on annotation %v", ing.Name, class.IngressKey) + return + } + if ing.Spec.Backend != nil && disableCatchAll { + klog.Infof("ignoring delete for catch-all ingress %v/%v because of --disable-catch-all", ing.Namespace, ing.Name) + return + } + recorder.Eventf(ing, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", ing.Namespace, ing.Name)) + + store.listers.IngressWithAnnotation.Delete(ing) + + key := k8s.MetaNamespaceKey(ing) + store.secretIngressMap.Delete(key) + + updateCh.In() <- Event{ + Type: DeleteEvent, + Obj: obj, + } + } + ingEventHandler := cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { ing := obj.(*extensions.Ingress) @@ -327,41 +363,7 @@ func New(checkOCSP bool, Obj: obj, } }, - DeleteFunc: func(obj interface{}) { - ing, ok := obj.(*extensions.Ingress) - if !ok { - // If we reached here it means the ingress was deleted but its final state is unrecorded. - tombstone, ok := obj.(cache.DeletedFinalStateUnknown) - if !ok { - klog.Errorf("couldn't get object from tombstone %#v", obj) - return - } - ing, ok = tombstone.Obj.(*extensions.Ingress) - if !ok { - klog.Errorf("Tombstone contained object that is not an Ingress: %#v", obj) - return - } - } - if !class.IsValid(ing) { - klog.Infof("ignoring delete for ingress %v based on annotation %v", ing.Name, class.IngressKey) - return - } - if ing.Spec.Backend != nil && disableCatchAll { - klog.Infof("ignoring delete for catch-all ingress %v/%v because of --disable-catch-all", ing.Namespace, ing.Name) - return - } - recorder.Eventf(ing, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", ing.Namespace, ing.Name)) - - store.listers.IngressWithAnnotation.Delete(ing) - - key := k8s.MetaNamespaceKey(ing) - store.secretIngressMap.Delete(key) - - updateCh.In() <- Event{ - Type: DeleteEvent, - Obj: obj, - } - }, + DeleteFunc: ingDeleteHandler, UpdateFunc: func(old, cur interface{}) { oldIng := old.(*extensions.Ingress) curIng := cur.(*extensions.Ingress) @@ -377,18 +379,16 @@ func New(checkOCSP bool, recorder.Eventf(curIng, corev1.EventTypeNormal, "CREATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) } else if validOld && !validCur { klog.Infof("removing ingress %v based on annotation %v", curIng.Name, class.IngressKey) - // FIXME: this does not actually delete the Ingress resource. - // The existing one will be updated with latest changes and invalid ingress.class will be missed. - recorder.Eventf(curIng, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + ingDeleteHandler(old) + return } else if validCur && !reflect.DeepEqual(old, cur) { if curIng.Spec.Backend != nil && disableCatchAll { - klog.Infof("ignoring update for catch-all ingress %v/%v because of --disable-catch-all", curIng.Namespace, curIng.Name) - // FIXME: this does not actually delete the Ingress resource. - // The existing one will be updated with latest changes. - recorder.Eventf(curIng, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) - } else { - recorder.Eventf(curIng, corev1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + klog.Infof("ignoring update for catch-all ingress %v/%v and delete old one because of --disable-catch-all", curIng.Namespace, curIng.Name) + ingDeleteHandler(old) + return } + + recorder.Eventf(curIng, corev1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) } else { klog.Infof("ignoring ingress %v based on annotation %v", curIng.Name, class.IngressKey) return diff --git a/test/e2e/settings/disable_catch_all.go b/test/e2e/settings/disable_catch_all.go index 4c58c28346..fefc5183eb 100644 --- a/test/e2e/settings/disable_catch_all.go +++ b/test/e2e/settings/disable_catch_all.go @@ -17,10 +17,16 @@ limitations under the License. package settings import ( + "net/http" "strings" . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "github.com/parnurzeal/gorequest" appsv1beta1 "k8s.io/api/apps/v1beta1" + extensions "k8s.io/api/extensions/v1beta1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -64,46 +70,43 @@ var _ = framework.IngressNginxDescribe("Disabled catch-all", func() { }) }) - // FIXME: This test doesn't work because of a bug in Ingress update handle in store package. - // It("should delete Ingress updated to catch-all", func() { - // host := "foo" - - // ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, nil) - // f.EnsureIngress(ing) - - // f.WaitForNginxServer(host, - // func(server string) bool { - // return strings.Contains(server, "server_name foo") - // }) - - // resp, _, errs := gorequest.New(). - // Get(f.IngressController.HTTPURL). - // Set("Host", host). - // End() - // Expect(errs).To(BeNil()) - // Expect(resp.StatusCode).Should(Equal(http.StatusOK)) - - // err := framework.UpdateIngress(f.KubeClientSet, f.IngressController.Namespace, host, func(ingress *extensions.Ingress) error { - // ingress.Spec.Rules = nil - // ingress.Spec.Backend = &extensions.IngressBackend{ - // ServiceName: "http-svc", - // ServicePort: intstr.FromInt(80), - // } - // return nil - // }) - // Expect(err).ToNot(HaveOccurred()) - - // f.WaitForNginxConfiguration(func(cfg string) bool { - // return !strings.Contains(cfg, "server_name foo") && - // !strings.Contains(cfg, `set $ingress_name "foo"`) && - // !strings.Contains(cfg, `set $service_name "http-svc"`) - // }) - - // resp, _, errs = gorequest.New(). - // Get(f.IngressController.HTTPURL). - // Set("Host", host). - // End() - // Expect(errs).To(BeNil()) - // Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) - // }) + It("should delete Ingress updated to catch-all", func() { + host := "foo" + + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return strings.Contains(server, "server_name foo") + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + err := framework.UpdateIngress(f.KubeClientSet, f.IngressController.Namespace, host, func(ingress *extensions.Ingress) error { + ingress.Spec.Rules = nil + ingress.Spec.Backend = &extensions.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + } + return nil + }) + Expect(err).ToNot(HaveOccurred()) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name foo") + }) + + resp, _, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) + }) }) diff --git a/test/e2e/settings/ingress_class.go b/test/e2e/settings/ingress_class.go new file mode 100644 index 0000000000..69032d3083 --- /dev/null +++ b/test/e2e/settings/ingress_class.go @@ -0,0 +1,160 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package settings + +import ( + "net/http" + "strings" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "github.com/parnurzeal/gorequest" + appsv1beta1 "k8s.io/api/apps/v1beta1" + + "k8s.io/ingress-nginx/test/e2e/framework" +) + +var _ = framework.IngressNginxDescribe("Ingress class", func() { + f := framework.NewDefaultFramework("ingress-class") + + BeforeEach(func() { + f.NewEchoDeploymentWithReplicas(1) + }) + + AfterEach(func() { + }) + + Context("Without a specific ingress-class", func() { + + It("should ignore Ingress with class", func() { + invalidHost := "foo" + annotations := map[string]string{ + "kubernetes.io/ingress.class": "testclass", + } + ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + validHost := "bar" + ing = framework.NewSingleIngress(validHost, "/", validHost, f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name foo") && + strings.Contains(cfg, "server_name bar") + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", invalidHost). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) + + resp, _, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", validHost). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + }) + }) + + Context("With a specific ingress-class", func() { + BeforeEach(func() { + framework.UpdateDeployment(f.KubeClientSet, f.IngressController.Namespace, "nginx-ingress-controller", 1, + func(deployment *appsv1beta1.Deployment) error { + args := deployment.Spec.Template.Spec.Containers[0].Args + args = append(args, "--ingress-class=testclass") + deployment.Spec.Template.Spec.Containers[0].Args = args + _, err := f.KubeClientSet.AppsV1beta1().Deployments(f.IngressController.Namespace).Update(deployment) + + return err + }) + }) + + It("should ignore Ingress with no class", func() { + invalidHost := "bar" + + ing := framework.NewSingleIngress(invalidHost, "/", invalidHost, f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + validHost := "foo" + annotations := map[string]string{ + "kubernetes.io/ingress.class": "testclass", + } + ing = framework.NewSingleIngress(validHost, "/", validHost, f.IngressController.Namespace, "http-svc", 80, &annotations) + f.EnsureIngress(ing) + + f.WaitForNginxServer(validHost, func(cfg string) bool { + return strings.Contains(cfg, "server_name foo") + }) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name bar") + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", validHost). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + resp, _, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", invalidHost). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) + }) + + It("should delete Ingress when class is removed", func() { + host := "foo" + annotations := map[string]string{ + "kubernetes.io/ingress.class": "testclass", + } + ing := framework.NewSingleIngress(host, "/", host, f.IngressController.Namespace, "http-svc", 80, &annotations) + ing = f.EnsureIngress(ing) + + f.WaitForNginxServer(host, func(cfg string) bool { + return strings.Contains(cfg, "server_name foo") + }) + + resp, _, errs := gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusOK)) + + delete(ing.Annotations, "kubernetes.io/ingress.class") + ing = f.EnsureIngress(ing) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return !strings.Contains(cfg, "server_name foo") + }) + + resp, _, errs = gorequest.New(). + Get(f.IngressController.HTTPURL). + Set("Host", host). + End() + Expect(errs).To(BeNil()) + Expect(resp.StatusCode).Should(Equal(http.StatusNotFound)) + }) + }) + +})