From 9bcfef07187c596a4d4b853fe5c956af8d8e7748 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Tue, 11 Jul 2023 16:14:55 -0600 Subject: [PATCH] Ensure Prefix matching requires trailing slash (#817) Problem: When using a prefix path match, a path such as /foobar would match for the prefix /foo, which should not happen. Prefixes need a "/" delimiter between path segments. Solution: In order to allow just the prefix without a trailing slash, as well as more path segments using a slash delimiter, we now create two location blocks. One uses exact matching for the configured prefix path, and the other uses prefix matching with a trailing slash. --- .../mode/static/nginx/config/http/config.go | 1 - internal/mode/static/nginx/config/servers.go | 211 +++++--- .../static/nginx/config/servers_template.go | 2 +- .../mode/static/nginx/config/servers_test.go | 509 +++++++++++++++++- 4 files changed, 634 insertions(+), 89 deletions(-) diff --git a/internal/mode/static/nginx/config/http/config.go b/internal/mode/static/nginx/config/http/config.go index 7b75004c6..fa917e58f 100644 --- a/internal/mode/static/nginx/config/http/config.go +++ b/internal/mode/static/nginx/config/http/config.go @@ -18,7 +18,6 @@ type Location struct { HTTPMatchVar string ProxySetHeaders []Header Internal bool - Exact bool } // Header defines a HTTP header to be passed to the proxied server. diff --git a/internal/mode/static/nginx/config/servers.go b/internal/mode/static/nginx/config/servers.go index aa604ab1e..63e99b30f 100644 --- a/internal/mode/static/nginx/config/servers.go +++ b/internal/mode/static/nginx/config/servers.go @@ -75,24 +75,9 @@ func createServer(virtualServer dataplane.VirtualServer) http.Server { } func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http.Location { - lenPathRules := len(pathRules) - - if lenPathRules == 0 { - return []http.Location{createDefaultRootLocation()} - } - - // To calculate the maximum number of locations, we need to take into account the following: - // 1. Each match rule for a path rule will have one location. - // 2. Each path rule may have an additional location if it contains non-path-only matches. - // 3. There may be an additional location for the default root path. - maxLocs := 1 - for _, rules := range pathRules { - maxLocs += len(rules.MatchRules) + 1 - } - + maxLocs, pathsAndTypes := getMaxLocationCountAndPathMap(pathRules) locs := make([]http.Location, 0, maxLocs) - - rootPathExists := false + var rootPathExists bool for _, rule := range pathRules { matches := make([]httpMatch, 0, len(rule.MatchRules)) @@ -101,27 +86,23 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http. rootPathExists = true } + extLocations := initializeExternalLocations(rule, pathsAndTypes) + for matchRuleIdx, r := range rule.MatchRules { m := r.GetMatch() - var loc http.Location - - // handle case where the only route is a path-only match - // generate a standard location block without http_matches. - if len(rule.MatchRules) == 1 && isPathOnlyMatch(m) { - loc = http.Location{ - Path: rule.Path, - Exact: rule.PathType == dataplane.PathTypeExact, - } - } else { - path := createPathForMatch(rule.Path, rule.PathType, matchRuleIdx) - loc = createMatchLocation(path) - matches = append(matches, createHTTPMatch(m, path)) + buildLocations := extLocations + if len(rule.MatchRules) != 1 || !isPathOnlyMatch(m) { + intLocation, match := initializeInternalLocation(rule, matchRuleIdx, m) + buildLocations = []http.Location{intLocation} + matches = append(matches, match) } if r.Filters.InvalidFilter != nil { - loc.Return = &http.Return{Code: http.StatusInternalServerError} - locs = append(locs, loc) + for i := range buildLocations { + buildLocations[i].Return = &http.Return{Code: http.StatusInternalServerError} + } + locs = append(locs, buildLocations...) continue } @@ -136,40 +117,32 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http. // RequestRedirect and proxying are mutually exclusive. if r.Filters.RequestRedirect != nil { - loc.Return = createReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort) - locs = append(locs, loc) + ret := createReturnValForRedirectFilter(r.Filters.RequestRedirect, listenerPort) + for i := range buildLocations { + buildLocations[i].Return = ret + } + locs = append(locs, buildLocations...) continue } - backendName := backendGroupName(r.BackendGroup) - loc.ProxySetHeaders = generateProxySetHeaders(r.Filters.RequestHeaderModifiers) - - if backendGroupNeedsSplit(r.BackendGroup) { - loc.ProxyPass = createProxyPassForVar(backendName) - } else { - loc.ProxyPass = createProxyPass(backendName) + proxySetHeaders := generateProxySetHeaders(r.Filters.RequestHeaderModifiers) + for i := range buildLocations { + buildLocations[i].ProxySetHeaders = proxySetHeaders } - locs = append(locs, loc) + proxyPass := createProxyPass(r.BackendGroup) + for i := range buildLocations { + buildLocations[i].ProxyPass = proxyPass + } + locs = append(locs, buildLocations...) } if len(matches) > 0 { - // FIXME(sberman): De-dupe matches and associated locations - // so we don't need nginx/njs to perform unnecessary matching. - // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/662 - b, err := json.Marshal(matches) - if err != nil { - // panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly. - panic(fmt.Errorf("could not marshal http match: %w", err)) + matchesStr := convertMatchesToString(matches) + for i := range extLocations { + extLocations[i].HTTPMatchVar = matchesStr } - - pathLoc := http.Location{ - Path: rule.Path, - Exact: rule.PathType == dataplane.PathTypeExact, - HTTPMatchVar: string(b), - } - - locs = append(locs, pathLoc) + locs = append(locs, extLocations...) } } @@ -180,6 +153,87 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int32) []http. return locs } +// pathAndTypeMap contains a map of paths and any path types defined for that path +// for example, {/foo: {exact: {}, prefix: {}}} +type pathAndTypeMap map[string]map[dataplane.PathType]struct{} + +// To calculate the maximum number of locations, we need to take into account the following: +// 1. Each match rule for a path rule will have one location. +// 2. Each path rule may have an additional location if it contains non-path-only matches. +// 3. Each prefix path rule may have an additional location if it doesn't contain trailing slash. +// 4. There may be an additional location for the default root path. +// We also return a map of all paths and their types. +func getMaxLocationCountAndPathMap(pathRules []dataplane.PathRule) (int, pathAndTypeMap) { + maxLocs := 1 + pathsAndTypes := make(pathAndTypeMap) + for _, rule := range pathRules { + maxLocs += len(rule.MatchRules) + 2 + if pathsAndTypes[rule.Path] == nil { + pathsAndTypes[rule.Path] = map[dataplane.PathType]struct{}{ + rule.PathType: {}, + } + } else { + pathsAndTypes[rule.Path][rule.PathType] = struct{}{} + } + } + + return maxLocs, pathsAndTypes +} + +func initializeExternalLocations( + rule dataplane.PathRule, + pathsAndTypes pathAndTypeMap, +) []http.Location { + extLocations := make([]http.Location, 0, 2) + externalLocPath := createPath(rule) + // If the path type is Prefix and doesn't contain a trailing slash, then we need a second location + // that handles the Exact prefix case (if it doesn't already exist), and the first location is updated + // to handle the trailing slash prefix case (if it doesn't already exist) + if isNonSlashedPrefixPath(rule.PathType, externalLocPath) { + // if Exact path and/or trailing slash Prefix path already exists, this means some routing rule + // configures it. The routing rule location has priority over this location, so we don't try to + // overwrite it and we don't add a duplicate location to NGINX because that will cause an NGINX config error. + _, exactPathExists := pathsAndTypes[rule.Path][dataplane.PathTypeExact] + var trailingSlashPrefixPathExists bool + if pathTypes, exists := pathsAndTypes[rule.Path+"/"]; exists { + _, trailingSlashPrefixPathExists = pathTypes[dataplane.PathTypePrefix] + } + + if exactPathExists && trailingSlashPrefixPathExists { + return []http.Location{} + } + + if !trailingSlashPrefixPathExists { + externalLocTrailing := http.Location{ + Path: externalLocPath + "/", + } + extLocations = append(extLocations, externalLocTrailing) + } + if !exactPathExists { + externalLocExact := http.Location{ + Path: exactPath(externalLocPath), + } + extLocations = append(extLocations, externalLocExact) + } + } else { + externalLoc := http.Location{ + Path: externalLocPath, + } + extLocations = []http.Location{externalLoc} + } + + return extLocations +} + +func initializeInternalLocation( + rule dataplane.PathRule, + matchRuleIdx int, + match v1beta1.HTTPRouteMatch, +) (http.Location, httpMatch) { + path := createPathForMatch(rule.Path, rule.PathType, matchRuleIdx) + return createMatchLocation(path), createHTTPMatch(match, path) +} + func createReturnValForRedirectFilter(filter *v1beta1.HTTPRequestRedirectFilter, listenerPort int32) *http.Return { if filter == nil { return nil @@ -308,12 +362,13 @@ func isPathOnlyMatch(match v1beta1.HTTPRouteMatch) bool { return match.Method == nil && match.Headers == nil && match.QueryParams == nil } -func createProxyPass(address string) string { - return "http://" + address -} +func createProxyPass(backendGroup dataplane.BackendGroup) string { + backendName := backendGroupName(backendGroup) + if backendGroupNeedsSplit(backendGroup) { + return "http://$" + convertStringToSafeVariableName(backendName) + } -func createProxyPassForVar(variable string) string { - return "http://$" + convertStringToSafeVariableName(variable) + return "http://" + backendName } func createMatchLocation(path string) http.Location { @@ -369,6 +424,33 @@ func convertSetHeaders(headers []dataplane.HTTPHeader) []http.Header { return locHeaders } +func convertMatchesToString(matches []httpMatch) string { + // FIXME(sberman): De-dupe matches and associated locations + // so we don't need nginx/njs to perform unnecessary matching. + // https://github.com/nginxinc/nginx-kubernetes-gateway/issues/662 + b, err := json.Marshal(matches) + if err != nil { + // panic is safe here because we should never fail to marshal the match unless we constructed it incorrectly. + panic(fmt.Errorf("could not marshal http match: %w", err)) + } + + return string(b) +} + +func exactPath(path string) string { + return fmt.Sprintf("= %s", path) +} + +// createPath builds the location path depending on the path type. +func createPath(rule dataplane.PathRule) string { + switch rule.PathType { + case dataplane.PathTypeExact: + return exactPath(rule.Path) + default: + return rule.Path + } +} + func createPathForMatch(path string, pathType dataplane.PathType, routeIdx int) string { return fmt.Sprintf("%s_%s_route%d", path, pathType, routeIdx) } @@ -379,3 +461,8 @@ func createDefaultRootLocation() http.Location { Return: &http.Return{Code: http.StatusNotFound}, } } + +// isNonSlashedPrefixPath returns whether or not a path is of type Prefix and does not contain a trailing slash +func isNonSlashedPrefixPath(pathType dataplane.PathType, path string) bool { + return pathType == dataplane.PathTypePrefix && !strings.HasSuffix(path, "/") +} diff --git a/internal/mode/static/nginx/config/servers_template.go b/internal/mode/static/nginx/config/servers_template.go index 4241438c4..09d708014 100644 --- a/internal/mode/static/nginx/config/servers_template.go +++ b/internal/mode/static/nginx/config/servers_template.go @@ -32,7 +32,7 @@ server { server_name {{ $s.ServerName }}; {{ range $l := $s.Locations }} - location {{ if $l.Exact }}= {{ end }}{{ $l.Path }} { + location {{ $l.Path }} { {{ if $l.Internal -}} internal; {{ end }} diff --git a/internal/mode/static/nginx/config/servers_test.go b/internal/mode/static/nginx/config/servers_test.go index eb45a9a61..d524abbb9 100644 --- a/internal/mode/static/nginx/config/servers_test.go +++ b/internal/mode/static/nginx/config/servers_test.go @@ -281,6 +281,24 @@ func TestCreateServers(t *testing.T) { }, // redirect is set in the corresponding state.MatchRule }, + { + // A match with a redirect and header matches + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetStringPointer("/redirect-with-headers"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), + }, + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "redirect", + Value: "this", + }, + }, + }, + }, + }, { // A match with an invalid filter Matches: []v1beta1.HTTPRouteMatch{ @@ -292,6 +310,24 @@ func TestCreateServers(t *testing.T) { }, }, }, + { + // A match with an invalid filter and headers + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetPointer("/invalid-filter-with-headers"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), + }, + Headers: []v1beta1.HTTPHeaderMatch{ + { + Type: helpers.GetHeaderMatchTypePointer(v1beta1.HeaderMatchExact), + Name: "filter", + Value: "this", + }, + }, + }, + }, + }, { // A match using type Exact Matches: []v1beta1.HTTPRouteMatch{ @@ -479,13 +515,46 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/invalid-filter", + Path: "/redirect-with-headers", PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, RuleIdx: 5, Source: hr, + Filters: dataplane.Filters{ + RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ + Hostname: helpers.GetPointer(v1beta1.PreciseHostname("foo.example.com")), + Port: helpers.GetPointer(v1beta1.PortNumber(8080)), + }, + }, + BackendGroup: filterGroup1, + }, + }, + }, + { + Path: "/invalid-filter", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 6, + Source: hr, + Filters: dataplane.Filters{ + InvalidFilter: &dataplane.InvalidFilter{}, + }, + BackendGroup: invalidFilterGroup, + }, + }, + }, + { + Path: "/invalid-filter-with-headers", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 7, + Source: hr, Filters: dataplane.Filters{ InvalidFilter: &dataplane.InvalidFilter{}, }, @@ -499,7 +568,7 @@ func TestCreateServers(t *testing.T) { MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, - RuleIdx: 6, + RuleIdx: 8, Source: hr, BackendGroup: fooGroup, }, @@ -511,7 +580,7 @@ func TestCreateServers(t *testing.T) { MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, - RuleIdx: 7, + RuleIdx: 9, Source: hr, BackendGroup: fooGroup, }, @@ -523,7 +592,7 @@ func TestCreateServers(t *testing.T) { MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, - RuleIdx: 8, + RuleIdx: 10, Source: hr, BackendGroup: fooGroup, Filters: dataplane.Filters{ @@ -593,6 +662,18 @@ func TestCreateServers(t *testing.T) { RedirectPath: "/test_exact_route0", }, } + redirectHeaderMatches := []httpMatch{ + { + Headers: []string{"redirect:this"}, + RedirectPath: "/redirect-with-headers_prefix_route0", + }, + } + invalidFilterHeaderMatches := []httpMatch{ + { + Headers: []string{"filter:this"}, + RedirectPath: "/invalid-filter-with-headers_prefix_route0", + }, + } getExpectedLocations := func(isHTTPS bool) []http.Location { port := 8080 @@ -626,37 +707,91 @@ func TestCreateServers(t *testing.T) { ProxyPass: "http://$test__route1_rule1", }, { - Path: "/test", + Path: "/test/", HTTPMatchVar: expectedMatchString(testMatches), }, { - Path: "/path-only", + Path: "/path-only/", ProxyPass: "http://invalid-backend-ref", }, { - Path: "/redirect-implicit-port", + Path: "= /path-only", + ProxyPass: "http://invalid-backend-ref", + }, + { + Path: "/redirect-implicit-port/", Return: &http.Return{ Code: 302, Body: fmt.Sprintf("$scheme://foo.example.com:%d$request_uri", port), }, }, { - Path: "/redirect-explicit-port", + Path: "= /redirect-implicit-port", + Return: &http.Return{ + Code: 302, + Body: fmt.Sprintf("$scheme://foo.example.com:%d$request_uri", port), + }, + }, + { + Path: "/redirect-explicit-port/", Return: &http.Return{ Code: 302, Body: "$scheme://bar.example.com:8080$request_uri", }, }, { - Path: "/invalid-filter", + Path: "= /redirect-explicit-port", + Return: &http.Return{ + Code: 302, + Body: "$scheme://bar.example.com:8080$request_uri", + }, + }, + { + Path: "/redirect-with-headers_prefix_route0", + Return: &http.Return{ + Body: "$scheme://foo.example.com:8080$request_uri", + Code: 302, + }, + Internal: true, + }, + { + Path: "/redirect-with-headers/", + HTTPMatchVar: expectedMatchString(redirectHeaderMatches), + }, + { + Path: "= /redirect-with-headers", + HTTPMatchVar: expectedMatchString(redirectHeaderMatches), + }, + { + Path: "/invalid-filter/", + Return: &http.Return{ + Code: http.StatusInternalServerError, + }, + }, + { + Path: "= /invalid-filter", Return: &http.Return{ Code: http.StatusInternalServerError, }, }, { - Path: "/exact", + Path: "/invalid-filter-with-headers_prefix_route0", + Return: &http.Return{ + Code: http.StatusInternalServerError, + }, + Internal: true, + }, + { + Path: "/invalid-filter-with-headers/", + HTTPMatchVar: expectedMatchString(invalidFilterHeaderMatches), + }, + { + Path: "= /invalid-filter-with-headers", + HTTPMatchVar: expectedMatchString(invalidFilterHeaderMatches), + }, + { + Path: "= /exact", ProxyPass: "http://test_foo_80", - Exact: true, }, { Path: "/test_exact_route0", @@ -664,12 +799,21 @@ func TestCreateServers(t *testing.T) { Internal: true, }, { - Path: "/test", + Path: "= /test", HTTPMatchVar: expectedMatchString(exactMatches), - Exact: true, }, { - Path: "/proxy-set-headers", + Path: "/proxy-set-headers/", + ProxyPass: "http://test_foo_80", + ProxySetHeaders: []http.Header{ + { + Name: "my-header", + Value: "${my_header_header_var}some-value-123", + }, + }, + }, + { + Path: "= /proxy-set-headers", ProxyPass: "http://test_foo_80", ProxySetHeaders: []http.Header{ { @@ -714,6 +858,294 @@ func TestCreateServers(t *testing.T) { g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) } +func TestCreateServersConflicts(t *testing.T) { + type pathAndType struct { + path string + pathType v1beta1.PathMatchType + } + + createHR := func(pathsAndTypes []pathAndType) *v1beta1.HTTPRoute { + hr := &v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "route", + }, + Spec: v1beta1.HTTPRouteSpec{ + Hostnames: []v1beta1.Hostname{ + "cafe.example.com", + }, + Rules: []v1beta1.HTTPRouteRule{}, + }, + } + for _, pt := range pathsAndTypes { + match := v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetPointer(pt.path), + Type: helpers.GetPointer(pt.pathType), + }, + } + hr.Spec.Rules = append(hr.Spec.Rules, v1beta1.HTTPRouteRule{ + Matches: []v1beta1.HTTPRouteMatch{match}, + }) + } + + return hr + } + + hr1 := createHR([]pathAndType{ + { + path: "/coffee", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + path: "/coffee", + pathType: v1beta1.PathMatchExact, + }, + }) + hr2 := createHR([]pathAndType{ + { + path: "/coffee", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + path: "/coffee/", + pathType: v1beta1.PathMatchPathPrefix, + }, + }) + hr3 := createHR([]pathAndType{ + { + path: "/coffee", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + path: "/coffee/", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + path: "/coffee", + pathType: v1beta1.PathMatchExact, + }, + }) + + fooGroup := dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "route"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + { + UpstreamName: "test_foo_80", + Valid: true, + Weight: 1, + }, + }, + } + barGroup := dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "route"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + { + UpstreamName: "test_bar_80", + Valid: true, + Weight: 1, + }, + }, + } + bazGroup := dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "test", Name: "route"}, + RuleIdx: 0, + Backends: []dataplane.Backend{ + { + UpstreamName: "test_baz_80", + Valid: true, + Weight: 1, + }, + }, + } + + tests := []struct { + name string + rules []dataplane.PathRule + expLocs []http.Location + }{ + { + name: "/coffee prefix, /coffee exact", + rules: []dataplane.PathRule{ + { + Path: "/coffee", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + Source: hr1, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/coffee", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + Source: hr1, + BackendGroup: barGroup, + }, + }, + }, + }, + expLocs: []http.Location{ + { + Path: "/coffee/", + ProxyPass: "http://test_foo_80", + }, + { + Path: "= /coffee", + ProxyPass: "http://test_bar_80", + }, + createDefaultRootLocation(), + }, + }, + { + name: "/coffee prefix, /coffee/ prefix", + rules: []dataplane.PathRule{ + { + Path: "/coffee", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + Source: hr2, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/coffee/", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + Source: hr2, + BackendGroup: barGroup, + }, + }, + }, + }, + expLocs: []http.Location{ + { + Path: "= /coffee", + ProxyPass: "http://test_foo_80", + }, + { + Path: "/coffee/", + ProxyPass: "http://test_bar_80", + }, + createDefaultRootLocation(), + }, + }, + { + name: "/coffee prefix, /coffee/ prefix, /coffee exact", + rules: []dataplane.PathRule{ + { + Path: "/coffee", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + Source: hr3, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/coffee/", + PathType: dataplane.PathTypePrefix, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + Source: hr3, + BackendGroup: barGroup, + }, + }, + }, + { + Path: "/coffee", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 2, + Source: createHR([]pathAndType{ + { + path: "/coffee", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + path: "/coffee/", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + path: "/coffee", + pathType: v1beta1.PathMatchExact, + }, + }), + BackendGroup: bazGroup, + }, + }, + }, + }, + expLocs: []http.Location{ + { + Path: "/coffee/", + ProxyPass: "http://test_bar_80", + }, + { + Path: "= /coffee", + ProxyPass: "http://test_baz_80", + }, + createDefaultRootLocation(), + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + httpServers := []dataplane.VirtualServer{ + { + IsDefault: true, + Port: 8080, + }, + { + Hostname: "cafe.example.com", + PathRules: test.rules, + Port: 8080, + }, + } + expectedServers := []http.Server{ + { + IsDefaultHTTP: true, + Port: 8080, + }, + { + ServerName: "cafe.example.com", + Locations: test.expLocs, + Port: 8080, + }, + } + + g := NewGomegaWithT(t) + + result := createServers(httpServers, []dataplane.VirtualServer{}) + g.Expect(helpers.Diff(expectedServers, result)).To(BeEmpty()) + }) + } +} + func TestCreateLocationsRootPath(t *testing.T) { g := NewGomegaWithT(t) @@ -1301,20 +1733,47 @@ func TestIsPathOnlyMatch(t *testing.T) { } func TestCreateProxyPass(t *testing.T) { - expected := "http://10.0.0.1:80" + g := NewGomegaWithT(t) - result := createProxyPass("10.0.0.1:80") - if result != expected { - t.Errorf("createProxyPass() returned %s but expected %s", result, expected) + tests := []struct { + expected string + grp dataplane.BackendGroup + }{ + { + expected: "http://10.0.0.1:80", + grp: dataplane.BackendGroup{ + Backends: []dataplane.Backend{ + { + UpstreamName: "10.0.0.1:80", + Valid: true, + Weight: 1, + }, + }, + }, + }, + { + expected: "http://$ns1__bg_rule0", + grp: dataplane.BackendGroup{ + Source: types.NamespacedName{Namespace: "ns1", Name: "bg"}, + Backends: []dataplane.Backend{ + { + UpstreamName: "my-variable", + Valid: true, + Weight: 1, + }, + { + UpstreamName: "my-variable2", + Valid: true, + Weight: 1, + }, + }, + }, + }, } -} -func TestCreateProxyPassForVar(t *testing.T) { - expected := "http://$my_variable" - - result := createProxyPassForVar("my-variable") - if result != expected { - t.Errorf("createProxyPassForVar() returned %s but expected %s", result, expected) + for _, tc := range tests { + result := createProxyPass(tc.grp) + g.Expect(result).To(Equal(tc.expected)) } }