Skip to content

Commit

Permalink
Fix duplicate location in NGINX config (#10090)
Browse files Browse the repository at this point in the history
When creating ingresses that use the same path with a mix of
Exact and Prefix path types nginx-ingress-controller can
generate an NGINX config with a duplicate location:

    nginx: [emerg] duplicate location "/a/" in /tmp/nginx/nginx-cfg2287376726:693
    nginx: configuration file /tmp/nginx/nginx-cfg2287376726 test failed

The issue is that nginx-ingress-controller stops looking for
duplicates once it sees the same path with a different path type.
  • Loading branch information
ailurarctos committed Feb 23, 2024
1 parent 536c4e7 commit 4f2950f
Show file tree
Hide file tree
Showing 2 changed files with 144 additions and 1 deletion.
2 changes: 1 addition & 1 deletion internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -795,7 +795,7 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in
// Same paths but different types are allowed
// (same type means overlap in the path definition)
if !apiequality.Semantic.DeepEqual(loc.PathType, path.PathType) {
break
continue
}

addLoc = false
Expand Down
143 changes: 143 additions & 0 deletions internal/ingress/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2468,6 +2468,149 @@ func TestGetBackendServers(t *testing.T) {
}
},
},
{
Ingresses: []*ingress.Ingress{
{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: "example",
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "example.com",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/a",
PathType: &pathTypeExact,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc-1",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
},
},
ParsedAnnotations: &annotations.Ingress{},
},
{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: "example",
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "example.com",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/a",
PathType: &pathTypePrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc-2",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
},
},
ParsedAnnotations: &annotations.Ingress{},
},
{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Namespace: "example",
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "example.com",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/a",
PathType: &pathTypePrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc-3",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
},
},
ParsedAnnotations: &annotations.Ingress{},
},
},
Validate: func(ingresses []*ingress.Ingress, upstreams []*ingress.Backend, 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 len(s.Locations) != 1 {
t.Errorf("servers locations count should be 1, got %d", len(s.Locations))
return
}
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 len(s.Locations) != 3 {
t.Errorf("servers locations count should be 2, got %d", len(s.Locations))
return
}
if s.Locations[0].Backend != "example-http-svc-1-80" {
t.Errorf("location 0 backend should be 'example-http-svc-1-80', got '%s'", s.Locations[0].Backend)
}
if s.Locations[1].Backend != "example-http-svc-2-80" {
t.Errorf("location 1 backend should be 'example-http-svc-2-80', got '%s'", s.Locations[1].Backend)
}
if s.Locations[2].Backend != defUpstreamName {
t.Errorf("location 2 backend should be '%s', got '%s'", defUpstreamName, s.Locations[2].Backend)
}
},
SetConfigMap: testConfigMap,
},
}

for _, testCase := range testCases {
Expand Down

0 comments on commit 4f2950f

Please sign in to comment.