Skip to content

Commit 180b383

Browse files
author
Kate Osborn
committed
Always generate a root "/" location block
1 parent 37d98ed commit 180b383

File tree

2 files changed

+188
-1
lines changed

2 files changed

+188
-1
lines changed

internal/nginx/config/servers.go

+20-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414

1515
var serversTemplate = gotemplate.Must(gotemplate.New("servers").Parse(serversTemplateText))
1616

17+
const rootPath = "/"
18+
1719
func executeServers(conf state.Configuration) []byte {
1820
servers := createServers(conf.HTTPServers, conf.SSLServers)
1921

@@ -64,14 +66,20 @@ func createLocations(pathRules []state.PathRule, listenerPort int) []http.Locati
6466
lenPathRules := len(pathRules)
6567

6668
if lenPathRules == 0 {
67-
return []http.Location{{Path: "/", Return: &http.Return{Code: http.StatusNotFound}}}
69+
return []http.Location{createDefaultRootLocation()}
6870
}
6971

7072
locs := make([]http.Location, 0, lenPathRules) // FIXME(pleshakov): expand with rule.Routes
7173

74+
rootPathExists := false
75+
7276
for _, rule := range pathRules {
7377
matches := make([]httpMatch, 0, len(rule.MatchRules))
7478

79+
if rule.Path == rootPath {
80+
rootPathExists = true
81+
}
82+
7583
for matchRuleIdx, r := range rule.MatchRules {
7684
m := r.GetMatch()
7785

@@ -130,6 +138,10 @@ func createLocations(pathRules []state.PathRule, listenerPort int) []http.Locati
130138
}
131139
}
132140

141+
if !rootPathExists {
142+
locs = append(locs, createDefaultRootLocation())
143+
}
144+
133145
return locs
134146
}
135147

@@ -279,3 +291,10 @@ func createMatchLocation(path string) http.Location {
279291
func createPathForMatch(path string, routeIdx int) string {
280292
return fmt.Sprintf("%s_route%d", path, routeIdx)
281293
}
294+
295+
func createDefaultRootLocation() http.Location {
296+
return http.Location{
297+
Path: "/",
298+
Return: &http.Return{Code: http.StatusNotFound},
299+
}
300+
}

internal/nginx/config/servers_test.go

+168
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"testing"
88

99
"github.com./google/go-cmp/cmp"
10+
. "github.com./onsi/gomega"
1011
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1112
"k8s.io/apimachinery/pkg/types"
1213
"sigs.k8s.io/gateway-api/apis/v1beta1"
@@ -527,6 +528,173 @@ func TestCreateServers(t *testing.T) {
527528
}
528529
}
529530

531+
func TestCreateLocationsRootPath(t *testing.T) {
532+
g := NewGomegaWithT(t)
533+
534+
createRoute := func(rootPath bool) *v1beta1.HTTPRoute {
535+
route := &v1beta1.HTTPRoute{
536+
ObjectMeta: metav1.ObjectMeta{
537+
Namespace: "test",
538+
Name: "route1",
539+
},
540+
Spec: v1beta1.HTTPRouteSpec{
541+
Hostnames: []v1beta1.Hostname{
542+
"cafe.example.com",
543+
},
544+
Rules: []v1beta1.HTTPRouteRule{
545+
{
546+
Matches: []v1beta1.HTTPRouteMatch{
547+
{
548+
Path: &v1beta1.HTTPPathMatch{
549+
Value: helpers.GetStringPointer("/path-1"),
550+
},
551+
},
552+
{
553+
Path: &v1beta1.HTTPPathMatch{
554+
Value: helpers.GetStringPointer("/path-2"),
555+
},
556+
},
557+
},
558+
},
559+
},
560+
},
561+
}
562+
563+
if rootPath {
564+
route.Spec.Rules[0].Matches = append(route.Spec.Rules[0].Matches, v1beta1.HTTPRouteMatch{
565+
Path: &v1beta1.HTTPPathMatch{
566+
Value: helpers.GetStringPointer("/"),
567+
},
568+
})
569+
}
570+
571+
return route
572+
}
573+
574+
hrWithRootPathRule := createRoute(true)
575+
576+
hrWithoutRootPathRule := createRoute(false)
577+
578+
hrNsName := types.NamespacedName{Namespace: "test", Name: "route1"}
579+
580+
fooGroup := state.BackendGroup{
581+
Source: hrNsName,
582+
RuleIdx: 0,
583+
Backends: []state.BackendRef{
584+
{
585+
Name: "test_foo_80",
586+
Valid: true,
587+
Weight: 1,
588+
},
589+
},
590+
}
591+
592+
getPathRules := func(source *v1beta1.HTTPRoute, rootPath bool) []state.PathRule {
593+
rules := []state.PathRule{
594+
{
595+
Path: "/path-1",
596+
MatchRules: []state.MatchRule{
597+
{
598+
Source: source,
599+
BackendGroup: fooGroup,
600+
MatchIdx: 0,
601+
RuleIdx: 0,
602+
},
603+
},
604+
},
605+
{
606+
Path: "/path-2",
607+
MatchRules: []state.MatchRule{
608+
{
609+
Source: source,
610+
BackendGroup: fooGroup,
611+
MatchIdx: 1,
612+
RuleIdx: 0,
613+
},
614+
},
615+
},
616+
}
617+
618+
if rootPath {
619+
rules = append(rules, state.PathRule{
620+
Path: "/",
621+
MatchRules: []state.MatchRule{
622+
{
623+
Source: source,
624+
BackendGroup: fooGroup,
625+
MatchIdx: 2,
626+
RuleIdx: 0,
627+
},
628+
},
629+
})
630+
}
631+
632+
return rules
633+
}
634+
635+
tests := []struct {
636+
name string
637+
pathRules []state.PathRule
638+
expLocations []http.Location
639+
}{
640+
{
641+
name: "path rules with no root path should generate a default 404 root location",
642+
pathRules: getPathRules(hrWithoutRootPathRule, false),
643+
expLocations: []http.Location{
644+
{
645+
Path: "/path-1",
646+
ProxyPass: "http://test_foo_80",
647+
},
648+
{
649+
Path: "/path-2",
650+
ProxyPass: "http://test_foo_80",
651+
},
652+
{
653+
Path: "/",
654+
Return: &http.Return{
655+
Code: http.StatusNotFound,
656+
},
657+
},
658+
},
659+
},
660+
{
661+
name: "path rules with a root path should not generate a default 404 root path",
662+
pathRules: getPathRules(hrWithRootPathRule, true),
663+
expLocations: []http.Location{
664+
{
665+
Path: "/path-1",
666+
ProxyPass: "http://test_foo_80",
667+
},
668+
{
669+
Path: "/path-2",
670+
ProxyPass: "http://test_foo_80",
671+
},
672+
{
673+
Path: "/",
674+
ProxyPass: "http://test_foo_80",
675+
},
676+
},
677+
},
678+
{
679+
name: "nil path rules should generate a default 404 root path",
680+
pathRules: nil,
681+
expLocations: []http.Location{
682+
{
683+
Path: "/",
684+
Return: &http.Return{
685+
Code: http.StatusNotFound,
686+
},
687+
},
688+
},
689+
},
690+
}
691+
692+
for _, test := range tests {
693+
locs := createLocations(test.pathRules, 80)
694+
g.Expect(locs).To(Equal(test.expLocations), fmt.Sprintf("test case: %s", test.name))
695+
}
696+
}
697+
530698
func TestCreateReturnValForRedirectFilter(t *testing.T) {
531699
const listenerPort = 123
532700

0 commit comments

Comments
 (0)