diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 4102fe57b..c05e0af77 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -940,8 +940,13 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, // default upstream name un := du.Name + if anns.Canary.Enabled { + klog.V(2).Infof("Ingress %v is marked as Canary, ignoring", ingKey) + continue + } + if ing.Spec.Backend != nil { - defUpstream := fmt.Sprintf("%v-%v-%v", ing.Namespace, ing.Spec.Backend.ServiceName, ing.Spec.Backend.ServicePort.String()) + defUpstream := upstreamName(ing.Namespace, ing.Spec.Backend.ServiceName, ing.Spec.Backend.ServicePort) if backendUpstream, ok := upstreams[defUpstream]; ok { // use backend specified in Ingress as the default backend for all its rules @@ -1016,6 +1021,11 @@ func (n *NGINXController) createServers(data []*ingress.Ingress, ingKey := k8s.MetaNamespaceKey(ing) anns := ing.ParsedAnnotations + if anns.Canary.Enabled { + klog.V(2).Infof("Ingress %v is marked as Canary, ignoring", ingKey) + continue + } + for _, rule := range ing.Spec.Rules { host := rule.Host if host == "" { diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index 55ce67fec..fe67b9823 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -20,12 +20,24 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "fmt" + "time" + "testing" + + "github.com/eapache/channels" + "k8s.io/api/core/v1" extensions "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/fake" + "k8s.io/ingress-nginx/internal/file" "k8s.io/ingress-nginx/internal/ingress" - "testing" + "k8s.io/ingress-nginx/internal/ingress/annotations" + "k8s.io/ingress-nginx/internal/ingress/annotations/canary" + ngx_config "k8s.io/ingress-nginx/internal/ingress/controller/config" + "k8s.io/ingress-nginx/internal/ingress/controller/store" + "k8s.io/ingress-nginx/internal/k8s" ) func TestMergeAlternativeBackends(t *testing.T) { @@ -586,6 +598,337 @@ func TestExtractTLSSecretName(t *testing.T) { } } +func TestGetBackendServers(t *testing.T) { + ctl := newNGINXController(t) + + testCases := []struct { + Ingresses []*ingress.Ingress + Validate func(servers []*ingress.Server) + }{ + { + Ingresses: []*ingress.Ingress{ + { + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: "http-svc-canary", + ServicePort: intstr.IntOrString{ + IntVal: 80, + }, + }, + }, + }, + ParsedAnnotations: &annotations.Ingress{ + Canary: canary.Config{ + Enabled: true, + }, + }, + }, + }, + Validate: func(servers []*ingress.Server) { + if len(servers) != 1 { + t.Errorf("servers count should be 1, got %d", len(servers)) + return + } + + s := servers[0] + if s.Hostname != "_" { + t.Errorf("server hostname should be '_', got '%s'", s.Hostname) + } + if !s.Locations[0].IsDefBackend { + t.Errorf("server location 0 should be default backend") + } + + if s.Locations[0].Backend != defUpstreamName { + t.Errorf("location backend should be '%s', got '%s'", defUpstreamName, s.Locations[0].Backend) + } + }, + }, + { + Ingresses: []*ingress.Ingress{ + { + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: "http-svc-canary", + ServicePort: intstr.IntOrString{ + IntVal: 80, + }, + }, + }, + }, + ParsedAnnotations: &annotations.Ingress{ + Canary: canary.Config{ + Enabled: true, + }, + }, + }, + { + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.IntOrString{ + IntVal: 80, + }, + }, + }, + }, + ParsedAnnotations: &annotations.Ingress{ + Canary: canary.Config{ + Enabled: false, + }, + }, + }, + }, + Validate: func(servers []*ingress.Server) { + if len(servers) != 1 { + t.Errorf("servers count should be 1, got %d", len(servers)) + return + } + + s := servers[0] + if s.Hostname != "_" { + t.Errorf("server hostname should be '_', got '%s'", s.Hostname) + } + if s.Locations[0].IsDefBackend { + t.Errorf("server location 0 should not be default backend") + } + + if s.Locations[0].Backend != "example-http-svc-80" { + t.Errorf("location backend should be 'example-http-svc-80', got '%s'", s.Locations[0].Backend) + } + }, + }, + { + Ingresses: []*ingress.Ingress{ + { + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "example.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "http-svc-canary", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + ParsedAnnotations: &annotations.Ingress{ + Canary: canary.Config{ + Enabled: true, + }, + }, + }, + }, + Validate: func(servers []*ingress.Server) { + if len(servers) != 1 { + t.Errorf("servers count should be 1, got %d", len(servers)) + return + } + + s := servers[0] + if s.Hostname != "_" { + t.Errorf("server hostname should be '_', got '%s'", s.Hostname) + } + if !s.Locations[0].IsDefBackend { + t.Errorf("server location 0 should be default backend") + } + + if s.Locations[0].Backend != defUpstreamName { + t.Errorf("location backend should be '%s', got '%s'", defUpstreamName, s.Locations[0].Backend) + } + }, + }, + { + Ingresses: []*ingress.Ingress{ + { + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example", + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "example.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + ParsedAnnotations: &annotations.Ingress{ + Canary: canary.Config{ + Enabled: false, + }, + }, + }, + { + Ingress: extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-canary", + Namespace: "example", + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "example.com", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "http-svc-canary", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + ParsedAnnotations: &annotations.Ingress{ + Canary: canary.Config{ + Enabled: true, + }, + }, + }, + }, + Validate: func(servers []*ingress.Server) { + if len(servers) != 2 { + t.Errorf("servers count should be 2, got %d", len(servers)) + return + } + + s := servers[0] + if s.Hostname != "_" { + t.Errorf("server hostname should be '_', got '%s'", s.Hostname) + } + if !s.Locations[0].IsDefBackend { + t.Errorf("server location 0 should be default backend") + } + + if s.Locations[0].Backend != defUpstreamName { + t.Errorf("location backend should be '%s', got '%s'", defUpstreamName, s.Locations[0].Backend) + } + + s = servers[1] + if s.Hostname != "example.com" { + t.Errorf("server hostname should be 'example.com', got '%s'", s.Hostname) + } + + if s.Locations[0].Backend != "example-http-svc-80" { + t.Errorf("location backend should be 'example-http-svc-80', got '%s'", s.Locations[0].Backend) + } + }, + }, + } + + for _, testCase := range testCases { + _, servers := ctl.getBackendServers(testCase.Ingresses) + testCase.Validate(servers) + } +} + +func newNGINXController(t *testing.T) *NGINXController { + ns := v1.NamespaceDefault + pod := &k8s.PodInfo{ + Name: "testpod", + Namespace: ns, + Labels: map[string]string{ + "pod-template-hash": "1234", + }, + } + + clientSet := fake.NewSimpleClientset() + configMap := &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "config", + SelfLink: fmt.Sprintf("/api/v1/namespaces/%s/configmaps/config", ns), + }, + } + _, err := clientSet.CoreV1().ConfigMaps(ns).Create(configMap) + if err != nil { + t.Fatalf("error creating the configuration map: %v", err) + } + + fs, err := file.NewFakeFS() + if err != nil { + t.Fatalf("error: %v", err) + } + + storer := store.New(true, + ns, + fmt.Sprintf("%v/config", ns), + fmt.Sprintf("%v/tcp", ns), + fmt.Sprintf("%v/udp", ns), + "", + 10*time.Minute, + clientSet, + fs, + channels.NewRingChannel(10), + false, + pod) + + config := &Configuration{ + ListenPorts: &ngx_config.ListenPorts{ + Default: 80, + }, + } + + return &NGINXController{ + store: storer, + cfg: config, + } +} + var oidExtensionSubjectAltName = asn1.ObjectIdentifier{2, 5, 29, 17} func fakeX509Cert(dnsNames []string) *x509.Certificate { diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index 67fab180d..bb9502fc5 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -192,4 +192,54 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { Expect(body).ShouldNot(ContainSubstring("http-svc-canary")) }) }) + + Context("Single canary Ingress", func() { + It("should not use canary as a catch-all server", func() { + host := "foo" + canaryIngName := fmt.Sprintf("%v-canary", host) + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + ing := framework.NewSingleCatchAllIngress(canaryIngName, f.IngressController.Namespace, "http-svc-canary", 80, &annotations) + f.EnsureIngress(ing) + + ing = framework.NewSingleCatchAllIngress(host, f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer("_", + func(server string) bool { + upstreamName := fmt.Sprintf(`set $proxy_upstream_name "%s-%s-%s";`, f.IngressController.Namespace, "http-svc", "80") + canaryUpstreamName := fmt.Sprintf(`set $proxy_upstream_name "%s-%s-%s";`, f.IngressController.Namespace, "http-svc-canary", "80") + return Expect(server).Should(ContainSubstring(`set $ingress_name "`+host+`";`)) && + Expect(server).ShouldNot(ContainSubstring(`set $proxy_upstream_name "upstream-default-backend";`)) && + Expect(server).ShouldNot(ContainSubstring(canaryUpstreamName)) && + Expect(server).Should(ContainSubstring(upstreamName)) + }) + }) + + It("should not use canary with domain as a server", func() { + host := "foo" + canaryIngName := fmt.Sprintf("%v-canary", host) + annotations := map[string]string{ + "nginx.ingress.kubernetes.io/canary": "true", + "nginx.ingress.kubernetes.io/canary-by-header": "CanaryByHeader", + } + + ing := framework.NewSingleIngress(canaryIngName, "/", host, f.IngressController.Namespace, "http-svc-canary", 80, &annotations) + f.EnsureIngress(ing) + + otherHost := "bar" + ing = framework.NewSingleIngress(otherHost, "/", otherHost, f.IngressController.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + time.Sleep(waitForLuaSync) + + f.WaitForNginxConfiguration(func(cfg string) bool { + return Expect(cfg).Should(ContainSubstring("server_name "+otherHost)) && + Expect(cfg).ShouldNot(ContainSubstring("server_name "+host)) + }) + }) + }) }) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 75efa16c6..d45197b0e 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -354,38 +354,28 @@ func UpdateDeployment(kubeClientSet kubernetes.Interface, namespace string, name // NewSingleIngressWithTLS creates a simple ingress rule with TLS spec included func NewSingleIngressWithTLS(name, path, host, ns, service string, port int, annotations *map[string]string) *extensions.Ingress { - return newSingleIngress(name, path, host, ns, service, port, annotations, true) + return newSingleIngressWithRules(name, path, host, ns, service, port, annotations, true) } // NewSingleIngress creates a simple ingress rule func NewSingleIngress(name, path, host, ns, service string, port int, annotations *map[string]string) *extensions.Ingress { - return newSingleIngress(name, path, host, ns, service, port, annotations, false) + return newSingleIngressWithRules(name, path, host, ns, service, port, annotations, false) } -func newSingleIngress(name, path, host, ns, service string, port int, annotations *map[string]string, withTLS bool) *extensions.Ingress { - if annotations == nil { - annotations = &map[string]string{} - } +func newSingleIngressWithRules(name, path, host, ns, service string, port int, annotations *map[string]string, withTLS bool) *extensions.Ingress { - ing := &extensions.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - Annotations: *annotations, - }, - Spec: extensions.IngressSpec{ - Rules: []extensions.IngressRule{ - { - Host: host, - IngressRuleValue: extensions.IngressRuleValue{ - HTTP: &extensions.HTTPIngressRuleValue{ - Paths: []extensions.HTTPIngressPath{ - { - Path: path, - Backend: extensions.IngressBackend{ - ServiceName: service, - ServicePort: intstr.FromInt(port), - }, + spec := extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: host, + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: path, + Backend: extensions.IngressBackend{ + ServiceName: service, + ServicePort: intstr.FromInt(port), }, }, }, @@ -396,7 +386,7 @@ func newSingleIngress(name, path, host, ns, service string, port int, annotation } if withTLS { - ing.Spec.TLS = []extensions.IngressTLS{ + spec.TLS = []extensions.IngressTLS{ { Hosts: []string{host}, SecretName: host, @@ -404,5 +394,33 @@ func newSingleIngress(name, path, host, ns, service string, port int, annotation } } + return newSingleIngress(name, ns, annotations, spec) +} + +// NewSingleCatchAllIngress creates a simple ingress with a catch-all backend +func NewSingleCatchAllIngress(name, ns, service string, port int, annotations *map[string]string) *extensions.Ingress { + spec := extensions.IngressSpec{ + Backend: &extensions.IngressBackend{ + ServiceName: service, + ServicePort: intstr.FromInt(port), + }, + } + return newSingleIngress(name, ns, annotations, spec) +} + +func newSingleIngress(name, ns string, annotations *map[string]string, spec extensions.IngressSpec) *extensions.Ingress { + if annotations == nil { + annotations = &map[string]string{} + } + + ing := &extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + Annotations: *annotations, + }, + Spec: spec, + } + return ing }