Skip to content

Commit

Permalink
Fix nginx ingress variables for definitions with Backend
Browse files Browse the repository at this point in the history
  • Loading branch information
aledbf committed Dec 5, 2020
1 parent 0b6d115 commit 77234fc
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 37 deletions.
66 changes: 34 additions & 32 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,37 +567,40 @@ func (n *NGINXController) getBackendServers(ingresses []*ingress.Ingress) ([]*in

addLoc := true
for _, loc := range server.Locations {
if loc.Path == nginxPath {
// Same paths but different types are allowed
// (same type means overlap in the path definition)
if !apiequality.Semantic.DeepEqual(loc.PathType, path.PathType) {
break
}
if loc.Path != nginxPath {
continue
}

addLoc = false
// Same paths but different types are allowed
// (same type means overlap in the path definition)
if !apiequality.Semantic.DeepEqual(loc.PathType, path.PathType) {
break
}

if !loc.IsDefBackend {
klog.V(3).Infof("Location %q already configured for server %q with upstream %q (Ingress %q)",
loc.Path, server.Hostname, loc.Backend, ingKey)
break
}
addLoc = false

klog.V(3).Infof("Replacing location %q for server %q with upstream %q to use upstream %q (Ingress %q)",
loc.Path, server.Hostname, loc.Backend, ups.Name, ingKey)
if !loc.IsDefBackend {
klog.V(3).Infof("Location %q already configured for server %q with upstream %q (Ingress %q)",
loc.Path, server.Hostname, loc.Backend, ingKey)
break
}

loc.Backend = ups.Name
loc.IsDefBackend = false
loc.Port = ups.Port
loc.Service = ups.Service
loc.Ingress = ing
klog.V(3).Infof("Replacing location %q for server %q with upstream %q to use upstream %q (Ingress %q)",
loc.Path, server.Hostname, loc.Backend, ups.Name, ingKey)

locationApplyAnnotations(loc, anns)
loc.Backend = ups.Name
loc.IsDefBackend = false
loc.Port = ups.Port
loc.Service = ups.Service
loc.Ingress = ing

if loc.Redirect.FromToWWW {
server.RedirectFromToWWW = true
}
break
locationApplyAnnotations(loc, anns)

if loc.Redirect.FromToWWW {
server.RedirectFromToWWW = true
}

break
}

// new location
Expand Down Expand Up @@ -1048,14 +1051,14 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,

// special "catch all" case, Ingress with a backend but no rule
defLoc := servers[defServerName].Locations[0]
defLoc.Backend = backendUpstream.Name
defLoc.Service = backendUpstream.Service
defLoc.Ingress = ing

if defLoc.IsDefBackend && len(ing.Spec.Rules) == 0 {
klog.V(2).Infof("Ingress %q defines a backend but no rule. Using it to configure the catch-all server %q",
ingKey, defServerName)
klog.V(2).Infof("Ingress %q defines a backend but no rule. Using it to configure the catch-all server %q", ingKey, defServerName)

defLoc.IsDefBackend = false
defLoc.Backend = backendUpstream.Name
defLoc.Service = backendUpstream.Service
defLoc.Ingress = ing

// TODO: Redirect and rewrite can affect the catch all behavior, skip for now
originalRedirect := defLoc.Redirect
Expand All @@ -1064,8 +1067,7 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
defLoc.Redirect = originalRedirect
defLoc.Rewrite = originalRewrite
} else {
klog.V(3).Infof("Ingress %q defines both a backend and rules. Using its backend as default upstream for all its rules.",
ingKey)
klog.V(3).Infof("Ingress %q defines both a backend and rules. Using its backend as default upstream for all its rules.", ingKey)
}
}
}
Expand All @@ -1081,12 +1083,12 @@ func (n *NGINXController) createServers(data []*ingress.Ingress,
continue
}

pathTypePrefix := networking.PathTypePrefix
loc := &ingress.Location{
Path: rootLocation,
PathType: &pathTypePrefix,
IsDefBackend: true,
Backend: un,
Ingress: ing,
Service: &apiv1.Service{},
}
locationApplyAnnotations(loc, anns)
Expand Down
17 changes: 12 additions & 5 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -890,14 +890,21 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation {
}

for _, rPath := range rule.HTTP.Paths {
if path == rPath.Path {
info.Service = rPath.Backend.ServiceName
if rPath.Backend.ServicePort.String() != "0" {
info.ServicePort = rPath.Backend.ServicePort.String()
}
if path != rPath.Path {
continue
}

if info.Service != "" && rPath.Backend.ServiceName == "" {
// empty rule. Only contains a Path and PathType
return info
}

info.Service = rPath.Backend.ServiceName
if rPath.Backend.ServicePort.String() != "0" {
info.ServicePort = rPath.Backend.ServicePort.String()
}

return info
}
}

Expand Down
41 changes: 41 additions & 0 deletions test/e2e/ingress/without_host.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (
"strings"

"github.com/onsi/ginkgo"
networking "k8s.io/api/networking/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"

"k8s.io/ingress-nginx/test/e2e/framework"
)
Expand Down Expand Up @@ -49,4 +52,42 @@ var _ = framework.IngressNginxDescribe("[Ingress] definition without host", func
Expect().
Status(http.StatusOK)
})

ginkgo.It("should set ingress details variables for ingresses with host without IngressRuleValue, only Backend", func() {
f.NewEchoDeployment()

ing := &networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "backend",
Namespace: f.Namespace,
},
Spec: networking.IngressSpec{
Backend: &networking.IngressBackend{
ServiceName: framework.EchoService,
ServicePort: intstr.FromInt(80),
},
Rules: []networking.IngressRule{
{
Host: "only-backend",
},
},
},
}
f.EnsureIngress(ing)

f.WaitForNginxServer("only-backend",
func(server string) bool {
return strings.Contains(server, fmt.Sprintf(`set $namespace "%v";`, f.Namespace)) &&
strings.Contains(server, fmt.Sprintf(`set $ingress_name "%v";`, ing.Name)) &&
strings.Contains(server, fmt.Sprintf(`set $service_name "%v";`, framework.EchoService)) &&
strings.Contains(server, `set $service_port "80";`) &&
strings.Contains(server, `set $location_path "/";`)
})

f.HTTPTestClient().
GET("/").
WithHeader("Host", "only-backend").
Expect().
Status(http.StatusOK)
})
})

0 comments on commit 77234fc

Please sign in to comment.