diff --git a/internal/ingress/controller/controller.go b/internal/ingress/controller/controller.go index 728e8b3a6e..6f5b46cf60 100644 --- a/internal/ingress/controller/controller.go +++ b/internal/ingress/controller/controller.go @@ -1162,7 +1162,7 @@ func nonCanaryIngressExists(ingresses []*ingress.Ingress, canaryIngresses []*ing // 2) primary name is not the default upstream // 3) the primary has a server func canMergeBackend(primary *ingress.Backend, alternative *ingress.Backend) bool { - return primary.Name != alternative.Name && primary.Name != defUpstreamName && !primary.NoServer + return alternative != nil && primary.Name != alternative.Name && primary.Name != defUpstreamName && !primary.NoServer } // Performs the merge action and checks to ensure that one two alternative backends do not merge into each other @@ -1199,22 +1199,27 @@ func mergeAlternativeBackends(ing *ingress.Ingress, upstreams map[string]*ingres altUps := upstreams[upsName] - merged := false + if altUps == nil { + klog.Warningf("alternative backend %s has already been removed", upsName) + } else { + + merged := false - for _, loc := range servers[defServerName].Locations { - priUps := upstreams[loc.Backend] + for _, loc := range servers[defServerName].Locations { + priUps := upstreams[loc.Backend] - if canMergeBackend(priUps, altUps) { - klog.V(2).Infof("matching backend %v found for alternative backend %v", - priUps.Name, altUps.Name) + if canMergeBackend(priUps, altUps) { + klog.V(2).Infof("matching backend %v found for alternative backend %v", + priUps.Name, altUps.Name) - merged = mergeAlternativeBackend(priUps, altUps) + merged = mergeAlternativeBackend(priUps, altUps) + } } - } - if !merged { - klog.Warningf("unable to find real backend for alternative backend %v. Deleting.", altUps.Name) - delete(upstreams, altUps.Name) + if !merged { + klog.Warningf("unable to find real backend for alternative backend %v. Deleting.", altUps.Name) + delete(upstreams, altUps.Name) + } } } @@ -1224,6 +1229,11 @@ func mergeAlternativeBackends(ing *ingress.Ingress, upstreams map[string]*ingres altUps := upstreams[upsName] + if altUps == nil { + klog.Warningf("alternative backend %s has already been removed", upsName) + continue + } + merged := false server, ok := servers[rule.Host] diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index 1c9aa9bac9..7a9a237c94 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -758,4 +758,25 @@ var _ = framework.IngressNginxDescribe("Annotations - canary", func() { }) }) }) + + It("does not crash when canary ingress has multiple paths to the same non-matching backend", 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", + } + + paths := []string{"/foo", "/bar"} + ing := framework.NewSingleIngressWithMultiplePaths(canaryIngName, paths, host, f.Namespace, "httpy-svc-canary", 80, &annotations) + f.EnsureIngress(ing) + + ing = framework.NewSingleIngress(host, "/", host, f.Namespace, "http-svc", 80, nil) + f.EnsureIngress(ing) + + f.WaitForNginxServer(host, + func(server string) bool { + return Expect(server).Should(ContainSubstring("server_name foo")) + }) + }) }) diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index cbdb454efb..04fa62704b 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -367,6 +367,32 @@ func NewSingleIngress(name, path, host, ns, service string, port int, annotation return newSingleIngressWithRules(name, path, host, ns, service, port, annotations, nil) } +// NewSingleIngressWithMultiplePaths creates a simple ingress rule with multiple paths +func NewSingleIngressWithMultiplePaths(name string, paths []string, host, ns, service string, port int, annotations *map[string]string) *extensions.Ingress { + spec := extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: host, + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{}, + }, + }, + }, + } + + for _, path := range paths { + spec.Rules[0].IngressRuleValue.HTTP.Paths = append(spec.Rules[0].IngressRuleValue.HTTP.Paths, extensions.HTTPIngressPath{ + Path: path, + Backend: extensions.IngressBackend{ + ServiceName: service, + ServicePort: intstr.FromInt(port), + }, + }) + } + + return newSingleIngress(name, ns, annotations, spec) +} + func newSingleIngressWithRules(name, path, host, ns, service string, port int, annotations *map[string]string, tlsHosts []string) *extensions.Ingress { spec := extensions.IngressSpec{