From ac9c2a8ec282960f5cae473a5dae179d36a9249f Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Fri, 28 Apr 2023 16:09:57 -0600 Subject: [PATCH 1/7] Exact PathMatch support for HTTPRoutes Add support for Exact PathMatch in an HTTPRoute. Internal locations now include the type of path (prefix, exact, regex) in the path name to distinguish between the possibility of the same path name being used with different path match types. nginx conf is updated to include "= " in the location path if that path has been defined as "Exact". --- docs/gateway-api-compatibility.md | 2 +- internal/nginx/config/http/config.go | 1 + internal/nginx/config/servers.go | 23 ++- internal/nginx/config/servers_template.go | 2 +- internal/nginx/config/servers_test.go | 71 ++++++-- .../nginx/config/validation/http_njs_match.go | 16 +- .../config/validation/http_njs_match_test.go | 6 +- internal/state/change_processor_test.go | 45 +++-- internal/state/dataplane/configuration.go | 29 ++- .../state/dataplane/configuration_test.go | 169 ++++++++++++++---- internal/state/graph/httproute.go | 7 +- internal/state/graph/httproute_test.go | 15 +- .../fake_httpfields_validator.go | 78 ++++---- internal/state/validation/validator.go | 2 +- 14 files changed, 329 insertions(+), 137 deletions(-) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 716f096151..bbfd6d967a 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -83,7 +83,7 @@ Fields: * `hostnames` - partially supported. Wildcard binding is not supported: a hostname like `example.com` will not bind to a listener with the hostname `*.example.com`. However, `example.com` will bind to a listener with the empty hostname. * `rules` * `matches` - * `path` - partially supported. Only `PathPrefix` type. + * `path` - partially supported. Only `PathPrefix` and `Exact` type. * `headers` - partially supported. Only `Exact` type. * `queryParams` - partially supported. Only `Exact` type. * `method` - supported. diff --git a/internal/nginx/config/http/config.go b/internal/nginx/config/http/config.go index f799bfb345..f1538109af 100644 --- a/internal/nginx/config/http/config.go +++ b/internal/nginx/config/http/config.go @@ -16,6 +16,7 @@ type Location struct { ProxyPass string HTTPMatchVar string Internal bool + Exact bool } // Return represents an HTTP return. diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 2c0b1c52c5..f72855f6df 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -102,10 +102,11 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo // generate a standard location block without http_matches. if len(rule.MatchRules) == 1 && isPathOnlyMatch(m) { loc = http.Location{ - Path: rule.Path, + Path: rule.Path, + Exact: *m.Path.Type == v1beta1.PathMatchExact, } } else { - path := createPathForMatch(rule.Path, matchRuleIdx) + path := createPathForMatch(rule.Path, rule.PathType, matchRuleIdx) loc = createMatchLocation(path) matches = append(matches, createHTTPMatch(m, path)) } @@ -151,6 +152,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo pathLoc := http.Location{ Path: rule.Path, + Exact: rule.PathType == v1beta1.PathMatchExact, HTTPMatchVar: string(b), } @@ -304,8 +306,8 @@ func createMatchLocation(path string) http.Location { } } -func createPathForMatch(path string, routeIdx int) string { - return fmt.Sprintf("%s_route%d", path, routeIdx) +func createPathForMatch(path string, pathType v1beta1.PathMatchType, routeIdx int) string { + return fmt.Sprintf("%s_%s_route%d", path, formatPathType(pathType), routeIdx) } func createDefaultRootLocation() http.Location { @@ -314,3 +316,16 @@ func createDefaultRootLocation() http.Location { Return: &http.Return{Code: http.StatusNotFound}, } } + +func formatPathType(pathType v1beta1.PathMatchType) string { + switch pathType { + case v1beta1.PathMatchPathPrefix: + return "prefix" + case v1beta1.PathMatchExact: + return "exact" + case v1beta1.PathMatchRegularExpression: + return "regex" + default: + return "" + } +} diff --git a/internal/nginx/config/servers_template.go b/internal/nginx/config/servers_template.go index f75af840b8..16736564db 100644 --- a/internal/nginx/config/servers_template.go +++ b/internal/nginx/config/servers_template.go @@ -30,7 +30,7 @@ server { server_name {{ $s.ServerName }}; {{ range $l := $s.Locations }} - location {{ $l.Path }} { + location {{ if $l.Exact }}= {{ end }}{{ $l.Path }} { {{ if $l.Internal -}} internal; {{ end }} diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 043835e194..132543f30e 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -181,12 +181,14 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPost), }, { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodPatch), }, @@ -195,6 +197,7 @@ func TestCreateServers(t *testing.T) { Value: helpers.GetStringPointer( "/", // should generate an "any" httpmatch since other matches exists for / ), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, @@ -205,6 +208,7 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/test"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodGet), Headers: []v1beta1.HTTPHeaderMatch{ @@ -245,6 +249,7 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/path-only"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, @@ -255,6 +260,7 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/redirect-implicit-port"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, @@ -266,6 +272,7 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/redirect-explicit-port"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, @@ -277,6 +284,7 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetPointer("/invalid-filter"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, @@ -338,7 +346,8 @@ func TestCreateServers(t *testing.T) { cafePathRules := []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -361,7 +370,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/test", + Path: "/test", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -372,7 +382,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/path-only", + Path: "/path-only", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -383,7 +394,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/redirect-implicit-port", + Path: "/redirect-implicit-port", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -399,7 +411,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/redirect-explicit-port", + Path: "/redirect-explicit-port", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -416,7 +429,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/invalid-filter", + Path: "/invalid-filter", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -461,16 +475,16 @@ func TestCreateServers(t *testing.T) { } slashMatches := []httpMatch{ - {Method: v1beta1.HTTPMethodPost, RedirectPath: "/_route0"}, - {Method: v1beta1.HTTPMethodPatch, RedirectPath: "/_route1"}, - {Any: true, RedirectPath: "/_route2"}, + {Method: v1beta1.HTTPMethodPost, RedirectPath: "/_prefix_route0"}, + {Method: v1beta1.HTTPMethodPatch, RedirectPath: "/_prefix_route1"}, + {Any: true, RedirectPath: "/_prefix_route2"}, } testMatches := []httpMatch{ { Method: v1beta1.HTTPMethodGet, Headers: []string{"Version:V1", "test:foo", "my-header:my-value"}, QueryParams: []string{"GrEat=EXAMPLE", "test=foo=bar"}, - RedirectPath: "/test_route0", + RedirectPath: "/test_prefix_route0", }, } @@ -482,17 +496,17 @@ func TestCreateServers(t *testing.T) { return []http.Location{ { - Path: "/_route0", + Path: "/_prefix_route0", Internal: true, ProxyPass: "http://test_foo_80", }, { - Path: "/_route1", + Path: "/_prefix_route1", Internal: true, ProxyPass: "http://test_foo_80", }, { - Path: "/_route2", + Path: "/_prefix_route2", Internal: true, ProxyPass: "http://test_foo_80", }, @@ -501,7 +515,7 @@ func TestCreateServers(t *testing.T) { HTTPMatchVar: expectedMatchString(slashMatches), }, { - Path: "/test_route0", + Path: "/test_prefix_route0", Internal: true, ProxyPass: "http://$test__route1_rule1", }, @@ -579,11 +593,13 @@ func TestCreateLocationsRootPath(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/path-1"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/path-2"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, @@ -596,6 +612,7 @@ func TestCreateLocationsRootPath(t *testing.T) { route.Spec.Rules[0].Matches = append(route.Spec.Rules[0].Matches, v1beta1.HTTPRouteMatch{ Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetStringPointer("/"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }) } @@ -1089,10 +1106,28 @@ func TestCreateMatchLocation(t *testing.T) { } func TestCreatePathForMatch(t *testing.T) { - expected := "/path_route1" + tests := []struct { + expected string + pathType v1beta1.PathMatchType + }{ + { + expected: "/path_prefix_route1", + pathType: v1beta1.PathMatchPathPrefix, + }, + { + expected: "/path_exact_route1", + pathType: v1beta1.PathMatchExact, + }, + { + expected: "/path_regex_route1", + pathType: v1beta1.PathMatchRegularExpression, + }, + } - result := createPathForMatch("/path", 1) - if result != expected { - t.Errorf("createPathForMatch() returned %q but expected %q", result, expected) + for _, tc := range tests { + result := createPathForMatch("/path", tc.pathType, 1) + if result != tc.expected { + t.Errorf("createPathForMatch() returned %q but expected %q", result, tc.expected) + } } } diff --git a/internal/nginx/config/validation/http_njs_match.go b/internal/nginx/config/validation/http_njs_match.go index 13e68124e0..1e5efc2d6b 100644 --- a/internal/nginx/config/validation/http_njs_match.go +++ b/internal/nginx/config/validation/http_njs_match.go @@ -17,23 +17,23 @@ import ( type HTTPNJSMatchValidator struct{} const ( - prefixPathFmt = `/[^\s{};]*` - prefixPathErrMsg = "must start with / and must not include any whitespace character, `{`, `}` or `;`" + pathFmt = `/[^\s{};]*` + pathErrMsg = "must start with / and must not include any whitespace character, `{`, `}` or `;`" ) var ( - prefixPathRegexp = regexp.MustCompile("^" + prefixPathFmt + "$") - prefixPathExamples = []string{"/", "/path", "/path/subpath-123"} + pathRegexp = regexp.MustCompile("^" + pathFmt + "$") + pathExamples = []string{"/", "/path", "/path/subpath-123"} ) -// ValidatePathInPrefixMatch a prefix path used in the location directive. -func (HTTPNJSMatchValidator) ValidatePathInPrefixMatch(path string) error { +// ValidatePathInMatch a path used in the location directive. +func (HTTPNJSMatchValidator) ValidatePathInMatch(path string) error { if path == "" { return errors.New("cannot be empty") } - if !prefixPathRegexp.MatchString(path) { - msg := k8svalidation.RegexError(prefixPathErrMsg, prefixPathFmt, prefixPathExamples...) + if !pathRegexp.MatchString(path) { + msg := k8svalidation.RegexError(pathErrMsg, pathFmt, pathExamples...) return errors.New(msg) } diff --git a/internal/nginx/config/validation/http_njs_match_test.go b/internal/nginx/config/validation/http_njs_match_test.go index 0f13613efe..ea5d4862b8 100644 --- a/internal/nginx/config/validation/http_njs_match_test.go +++ b/internal/nginx/config/validation/http_njs_match_test.go @@ -4,14 +4,14 @@ import ( "testing" ) -func TestValidatePathInPrefixMatch(t *testing.T) { +func TestValidatePathInMatch(t *testing.T) { validator := HTTPNJSMatchValidator{} - testValidValuesForSimpleValidator(t, validator.ValidatePathInPrefixMatch, + testValidValuesForSimpleValidator(t, validator.ValidatePathInMatch, "/", "/path", "/path/subpath-123") - testInvalidValuesForSimpleValidator(t, validator.ValidatePathInPrefixMatch, + testInvalidValuesForSimpleValidator(t, validator.ValidatePathInMatch, "/ ", "/path{", "/path}", diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index f4e2bf0d76..ae30dc6296 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -377,7 +377,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -399,7 +400,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -491,7 +493,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -513,7 +516,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -605,7 +609,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -627,7 +632,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -718,7 +724,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -740,7 +747,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -828,7 +836,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -849,7 +858,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -935,7 +945,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -957,7 +968,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1064,7 +1076,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "bar.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1086,7 +1099,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -2105,7 +2119,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 30650df628..24d64b0507 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -58,6 +58,8 @@ type SSL struct { type PathRule struct { // Path is a path. For example, '/hello'. Path string + // PathType is a path type. For example, PathPrefix or Exact. + PathType v1beta1.PathMatchType // MatchRules holds routing rules. MatchRules []MatchRule } @@ -185,8 +187,13 @@ func buildServers(listeners map[string]*graph.Listener) (http, ssl []VirtualServ return httpRules.buildServers(), sslRules.buildServers() } +type pathAndType struct { + path string + pathType v1beta1.PathMatchType +} + type hostPathRules struct { - rulesPerHost map[string]map[string]PathRule + rulesPerHost map[string]map[pathAndType]PathRule listenersForHost map[string]*graph.Listener httpsListeners []*graph.Listener listenersExist bool @@ -194,7 +201,7 @@ type hostPathRules struct { func newHostPathRules() *hostPathRules { return &hostPathRules{ - rulesPerHost: make(map[string]map[string]PathRule), + rulesPerHost: make(map[string]map[pathAndType]PathRule), listenersForHost: make(map[string]*graph.Listener), httpsListeners: make([]*graph.Listener, 0), } @@ -220,7 +227,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { hpr.listenersForHost[h] = l if _, exist := hpr.rulesPerHost[h]; !exist { - hpr.rulesPerHost[h] = make(map[string]PathRule) + hpr.rulesPerHost[h] = make(map[pathAndType]PathRule) } } @@ -242,9 +249,15 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { for j, m := range rule.Matches { path := getPath(m.Path) - rule, exist := hpr.rulesPerHost[h][path] + key := pathAndType{ + path: path, + pathType: *m.Path.Type, + } + + rule, exist := hpr.rulesPerHost[h][key] if !exist { rule.Path = path + rule.PathType = *m.Path.Type } rule.MatchRules = append(rule.MatchRules, MatchRule{ @@ -255,7 +268,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { Filters: filters, }) - hpr.rulesPerHost[h][path] = rule + hpr.rulesPerHost[h][key] = rule } } } @@ -288,7 +301,11 @@ func (hpr *hostPathRules) buildServers() []VirtualServer { // We sort the path rules so the order is preserved after reconfiguration. sort.Slice(s.PathRules, func(i, j int) bool { - return s.PathRules[i].Path < s.PathRules[j].Path + if s.PathRules[i].Path != s.PathRules[j].Path { + return s.PathRules[i].Path < s.PathRules[j].Path + } + + return s.PathRules[i].PathType < s.PathRules[j].PathType }) servers = append(servers, s) diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index f313d6ffa3..99de131c35 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -26,14 +26,15 @@ func TestBuildConfiguration(t *testing.T) { invalidFiltersPath = "/not-valid-filters" ) - createRoute := func(name, hostname, listenerName string, paths ...string) *v1beta1.HTTPRoute { + createRoute := func(name, hostname, listenerName string, paths ...pathAndType) *v1beta1.HTTPRoute { rules := make([]v1beta1.HTTPRouteRule, 0, len(paths)) for _, p := range paths { rules = append(rules, v1beta1.HTTPRouteRule{ Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ - Value: helpers.GetStringPointer(p), + Value: helpers.GetStringPointer(p.path), + Type: helpers.GetPointer(p.pathType), }, }, }, @@ -107,12 +108,12 @@ func TestBuildConfiguration(t *testing.T) { } } - createRules := func(hr *v1beta1.HTTPRoute, paths []string) []graph.Rule { + createRules := func(hr *v1beta1.HTTPRoute, paths []pathAndType) []graph.Rule { rules := make([]graph.Rule, len(hr.Spec.Rules)) for i := range paths { - validMatches := paths[i] != invalidMatchesPath - validFilters := paths[i] != invalidFiltersPath + validMatches := paths[i].path != invalidMatchesPath + validFilters := paths[i].path != invalidFiltersPath validRule := validMatches && validFilters rules[i] = graph.Rule{ @@ -127,7 +128,7 @@ func TestBuildConfiguration(t *testing.T) { createInternalRoute := func( source *v1beta1.HTTPRoute, - paths []string, + paths []pathAndType, ) *graph.Route { r := &graph.Route{ Source: source, @@ -136,7 +137,7 @@ func TestBuildConfiguration(t *testing.T) { return r } - createTestResources := func(name, hostname, listenerName string, paths ...string) ( + createTestResources := func(name, hostname, listenerName string, paths ...pathAndType) ( *v1beta1.HTTPRoute, []graph.BackendGroup, *graph.Route, ) { hr := createRoute(name, hostname, listenerName, paths...) @@ -144,12 +145,21 @@ func TestBuildConfiguration(t *testing.T) { return hr, route.GetAllBackendGroups(), route } - hr1, hr1Groups, routeHR1 := createTestResources("hr-1", "foo.example.com", "listener-80-1", "/") - hr2, hr2Groups, routeHR2 := createTestResources("hr-2", "bar.example.com", "listener-80-1", "/") - hr3, hr3Groups, routeHR3 := createTestResources("hr-3", "foo.example.com", "listener-80-1", "/", "/third") - hr4, hr4Groups, routeHR4 := createTestResources("hr-4", "foo.example.com", "listener-80-1", "/fourth", "/") - - hr5, hr5Groups, routeHR5 := createTestResources("hr-5", "foo.example.com", "listener-80-1", "/", invalidFiltersPath) + prefix := v1beta1.PathMatchPathPrefix + hr1, hr1Groups, routeHR1 := createTestResources( + "hr-1", "foo.example.com", "listener-80-1", pathAndType{path: "/", pathType: prefix}) + hr2, hr2Groups, routeHR2 := createTestResources( + "hr-2", "bar.example.com", "listener-80-1", pathAndType{path: "/", pathType: prefix}) + hr3, hr3Groups, routeHR3 := createTestResources( + "hr-3", "foo.example.com", "listener-80-1", + pathAndType{path: "/", pathType: prefix}, pathAndType{path: "/third", pathType: prefix}) + hr4, hr4Groups, routeHR4 := createTestResources( + "hr-4", "foo.example.com", "listener-80-1", + pathAndType{path: "/fourth", pathType: prefix}, pathAndType{path: "/", pathType: prefix}) + + hr5, hr5Groups, routeHR5 := createTestResources( + "hr-5", "foo.example.com", "listener-80-1", + pathAndType{path: "/", pathType: prefix}, pathAndType{path: invalidFiltersPath, pathType: prefix}) redirect := v1beta1.HTTPRouteFilter{ Type: v1beta1.HTTPRouteFilterRequestRedirect, RequestRedirect: &v1beta1.HTTPRequestRedirectFilter{ @@ -162,50 +172,56 @@ func TestBuildConfiguration(t *testing.T) { "hr-6", "foo.example.com", "listener-80-1", - "/valid", invalidMatchesPath, + pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: invalidMatchesPath, pathType: prefix}, + ) + + hr7, hr7Groups, routeHR7 := createTestResources( + "hr-7", + "foo.example.com", + "listener-80-1", + pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: "/valid", pathType: v1beta1.PathMatchExact}, ) httpsHR1, httpsHR1Groups, httpsRouteHR1 := createTestResources( "https-hr-1", "foo.example.com", "listener-443-1", - "/", + pathAndType{path: "/", pathType: prefix}, ) httpsHR2, httpsHR2Groups, httpsRouteHR2 := createTestResources( "https-hr-2", "bar.example.com", "listener-443-1", - "/", + pathAndType{path: "/", pathType: prefix}, ) httpsHR3, httpsHR3Groups, httpsRouteHR3 := createTestResources( "https-hr-3", "foo.example.com", "listener-443-1", - "/", "/third", + pathAndType{path: "/", pathType: prefix}, pathAndType{path: "/third", pathType: prefix}, ) httpsHR4, httpsHR4Groups, httpsRouteHR4 := createTestResources( "https-hr-4", "foo.example.com", "listener-443-1", - "/fourth", "/", + pathAndType{path: "/fourth", pathType: prefix}, pathAndType{path: "/", pathType: prefix}, ) httpsHR5, httpsHR5Groups, httpsRouteHR5 := createTestResources( "https-hr-5", "example.com", "listener-443-with-hostname", - "/", + pathAndType{path: "/", pathType: prefix}, ) httpsHR6, httpsHR6Groups, httpsRouteHR6 := createTestResources( "https-hr-6", "foo.example.com", "listener-443-1", - "/valid", - invalidMatchesPath, + pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: invalidMatchesPath, pathType: prefix}, ) listener80 := v1beta1.Listener{ @@ -421,7 +437,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "bar.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -437,7 +454,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -507,7 +525,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "bar.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -526,7 +545,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -545,7 +565,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -620,7 +641,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -637,7 +659,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/fourth", + Path: "/fourth", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -648,7 +671,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/third", + Path: "/third", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -672,7 +696,8 @@ func TestBuildConfiguration(t *testing.T) { }, PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -689,7 +714,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/fourth", + Path: "/fourth", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -700,7 +726,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/third", + Path: "/third", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -830,7 +857,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -844,7 +872,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: invalidFiltersPath, + Path: invalidFiltersPath, + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -911,7 +940,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/valid", + Path: "/valid", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -935,7 +965,8 @@ func TestBuildConfiguration(t *testing.T) { }, PathRules: []PathRule{ { - Path: "/valid", + Path: "/valid", + PathType: v1beta1.PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -960,6 +991,72 @@ func TestBuildConfiguration(t *testing.T) { }, msg: "one http and one https listener with routes with valid and invalid rules", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1beta1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1beta1.Gateway{}, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-7"}: routeHR7, + }, + AcceptedHostnames: map[string]struct{}{ + "foo.example.com": {}, + }, + }, + }, + }, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "hr-7"}: routeHR7, + }, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, + { + Hostname: "foo.example.com", + PathRules: []PathRule{ + { + Path: "/valid", + PathType: v1beta1.PathMatchExact, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: hr7Groups[1], + Source: hr7, + }, + }, + }, + { + Path: "/valid", + PathType: v1beta1.PathMatchPathPrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: hr7Groups[0], + Source: hr7, + }, + }, + }, + }, + }, + }, + SSLServers: []VirtualServer{}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []graph.BackendGroup{hr7Groups[0], hr7Groups[1]}, + }, + msg: "duplicate paths with different types", + }, } for _, test := range tests { diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index b7f0b1dbd9..27fbcb8256 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -523,12 +523,13 @@ func validatePathMatch( panicForBrokenWebhookAssumption(errors.New("path value cannot be nil")) } - if *path.Type != v1beta1.PathMatchPathPrefix { - valErr := field.NotSupported(fieldPath.Child("type"), *path.Type, []string{string(v1beta1.PathMatchPathPrefix)}) + if *path.Type != v1beta1.PathMatchPathPrefix && *path.Type != v1beta1.PathMatchExact { + valErr := field.NotSupported(fieldPath.Child("type"), *path.Type, + []string{string(v1beta1.PathMatchExact), string(v1beta1.PathMatchPathPrefix)}) allErrs = append(allErrs, valErr) } - if err := validator.ValidatePathInPrefixMatch(*path.Value); err != nil { + if err := validator.ValidatePathInMatch(*path.Value); err != nil { valErr := field.Invalid(fieldPath.Child("value"), *path.Value, err.Error()) allErrs = append(allErrs, valErr) } diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 292a0c3ac7..e9cfe6c649 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -423,7 +423,7 @@ func TestBuildRoute(t *testing.T) { addFilterToPath(hrInvalidValidRules, "/filter", invalidFilter) validatorInvalidFieldsInRule := &validationfakes.FakeHTTPFieldsValidator{ - ValidatePathInPrefixMatchStub: func(path string) error { + ValidatePathInMatchStub: func(path string) error { if path == invalidPath { return errors.New("invalid path") } @@ -1171,6 +1171,17 @@ func TestValidateMatch(t *testing.T) { expectErrCount: 0, name: "valid", }, + { + validator: createAllValidValidator(), + match: v1beta1.HTTPRouteMatch{ + Path: &v1beta1.HTTPPathMatch{ + Type: helpers.GetPointer(v1beta1.PathMatchExact), + Value: helpers.GetPointer("/"), + }, + }, + expectErrCount: 0, + name: "valid", + }, { validator: createAllValidValidator(), match: v1beta1.HTTPRouteMatch{ @@ -1185,7 +1196,7 @@ func TestValidateMatch(t *testing.T) { { validator: func() *validationfakes.FakeHTTPFieldsValidator { validator := createAllValidValidator() - validator.ValidatePathInPrefixMatchReturns(errors.New("invalid path value")) + validator.ValidatePathInMatchReturns(errors.New("invalid path value")) return validator }(), match: v1beta1.HTTPRouteMatch{ diff --git a/internal/state/validation/validationfakes/fake_httpfields_validator.go b/internal/state/validation/validationfakes/fake_httpfields_validator.go index 2040aa6655..98e6f9d4b3 100644 --- a/internal/state/validation/validationfakes/fake_httpfields_validator.go +++ b/internal/state/validation/validationfakes/fake_httpfields_validator.go @@ -43,15 +43,15 @@ type FakeHTTPFieldsValidator struct { result1 bool result2 []string } - ValidatePathInPrefixMatchStub func(string) error - validatePathInPrefixMatchMutex sync.RWMutex - validatePathInPrefixMatchArgsForCall []struct { + ValidatePathInMatchStub func(string) error + validatePathInMatchMutex sync.RWMutex + validatePathInMatchArgsForCall []struct { arg1 string } - validatePathInPrefixMatchReturns struct { + validatePathInMatchReturns struct { result1 error } - validatePathInPrefixMatchReturnsOnCall map[int]struct { + validatePathInMatchReturnsOnCall map[int]struct { result1 error } ValidateQueryParamNameInMatchStub func(string) error @@ -314,16 +314,16 @@ func (fake *FakeHTTPFieldsValidator) ValidateMethodInMatchReturnsOnCall(i int, r }{result1, result2} } -func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatch(arg1 string) error { - fake.validatePathInPrefixMatchMutex.Lock() - ret, specificReturn := fake.validatePathInPrefixMatchReturnsOnCall[len(fake.validatePathInPrefixMatchArgsForCall)] - fake.validatePathInPrefixMatchArgsForCall = append(fake.validatePathInPrefixMatchArgsForCall, struct { +func (fake *FakeHTTPFieldsValidator) ValidatePathInMatch(arg1 string) error { + fake.validatePathInMatchMutex.Lock() + ret, specificReturn := fake.validatePathInMatchReturnsOnCall[len(fake.validatePathInMatchArgsForCall)] + fake.validatePathInMatchArgsForCall = append(fake.validatePathInMatchArgsForCall, struct { arg1 string }{arg1}) - stub := fake.ValidatePathInPrefixMatchStub - fakeReturns := fake.validatePathInPrefixMatchReturns - fake.recordInvocation("ValidatePathInPrefixMatch", []interface{}{arg1}) - fake.validatePathInPrefixMatchMutex.Unlock() + stub := fake.ValidatePathInMatchStub + fakeReturns := fake.validatePathInMatchReturns + fake.recordInvocation("ValidatePathInMatch", []interface{}{arg1}) + fake.validatePathInMatchMutex.Unlock() if stub != nil { return stub(arg1) } @@ -333,44 +333,44 @@ func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatch(arg1 string) erro return fakeReturns.result1 } -func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchCallCount() int { - fake.validatePathInPrefixMatchMutex.RLock() - defer fake.validatePathInPrefixMatchMutex.RUnlock() - return len(fake.validatePathInPrefixMatchArgsForCall) +func (fake *FakeHTTPFieldsValidator) ValidatePathInMatchCallCount() int { + fake.validatePathInMatchMutex.RLock() + defer fake.validatePathInMatchMutex.RUnlock() + return len(fake.validatePathInMatchArgsForCall) } -func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchCalls(stub func(string) error) { - fake.validatePathInPrefixMatchMutex.Lock() - defer fake.validatePathInPrefixMatchMutex.Unlock() - fake.ValidatePathInPrefixMatchStub = stub +func (fake *FakeHTTPFieldsValidator) ValidatePathInMatchCalls(stub func(string) error) { + fake.validatePathInMatchMutex.Lock() + defer fake.validatePathInMatchMutex.Unlock() + fake.ValidatePathInMatchStub = stub } -func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchArgsForCall(i int) string { - fake.validatePathInPrefixMatchMutex.RLock() - defer fake.validatePathInPrefixMatchMutex.RUnlock() - argsForCall := fake.validatePathInPrefixMatchArgsForCall[i] +func (fake *FakeHTTPFieldsValidator) ValidatePathInMatchArgsForCall(i int) string { + fake.validatePathInMatchMutex.RLock() + defer fake.validatePathInMatchMutex.RUnlock() + argsForCall := fake.validatePathInMatchArgsForCall[i] return argsForCall.arg1 } -func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchReturns(result1 error) { - fake.validatePathInPrefixMatchMutex.Lock() - defer fake.validatePathInPrefixMatchMutex.Unlock() - fake.ValidatePathInPrefixMatchStub = nil - fake.validatePathInPrefixMatchReturns = struct { +func (fake *FakeHTTPFieldsValidator) ValidatePathInMatchReturns(result1 error) { + fake.validatePathInMatchMutex.Lock() + defer fake.validatePathInMatchMutex.Unlock() + fake.ValidatePathInMatchStub = nil + fake.validatePathInMatchReturns = struct { result1 error }{result1} } -func (fake *FakeHTTPFieldsValidator) ValidatePathInPrefixMatchReturnsOnCall(i int, result1 error) { - fake.validatePathInPrefixMatchMutex.Lock() - defer fake.validatePathInPrefixMatchMutex.Unlock() - fake.ValidatePathInPrefixMatchStub = nil - if fake.validatePathInPrefixMatchReturnsOnCall == nil { - fake.validatePathInPrefixMatchReturnsOnCall = make(map[int]struct { +func (fake *FakeHTTPFieldsValidator) ValidatePathInMatchReturnsOnCall(i int, result1 error) { + fake.validatePathInMatchMutex.Lock() + defer fake.validatePathInMatchMutex.Unlock() + fake.ValidatePathInMatchStub = nil + if fake.validatePathInMatchReturnsOnCall == nil { + fake.validatePathInMatchReturnsOnCall = make(map[int]struct { result1 error }) } - fake.validatePathInPrefixMatchReturnsOnCall[i] = struct { + fake.validatePathInMatchReturnsOnCall[i] = struct { result1 error }{result1} } @@ -756,8 +756,8 @@ func (fake *FakeHTTPFieldsValidator) Invocations() map[string][][]interface{} { defer fake.validateHeaderValueInMatchMutex.RUnlock() fake.validateMethodInMatchMutex.RLock() defer fake.validateMethodInMatchMutex.RUnlock() - fake.validatePathInPrefixMatchMutex.RLock() - defer fake.validatePathInPrefixMatchMutex.RUnlock() + fake.validatePathInMatchMutex.RLock() + defer fake.validatePathInMatchMutex.RUnlock() fake.validateQueryParamNameInMatchMutex.RLock() defer fake.validateQueryParamNameInMatchMutex.RUnlock() fake.validateQueryParamValueInMatchMutex.RLock() diff --git a/internal/state/validation/validator.go b/internal/state/validation/validator.go index 14aff31382..75d68c1000 100644 --- a/internal/state/validation/validator.go +++ b/internal/state/validation/validator.go @@ -13,7 +13,7 @@ type Validators struct { // //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . HTTPFieldsValidator type HTTPFieldsValidator interface { - ValidatePathInPrefixMatch(path string) error + ValidatePathInMatch(path string) error ValidateHeaderNameInMatch(name string) error ValidateHeaderValueInMatch(value string) error ValidateQueryParamNameInMatch(name string) error From 14d7be52422179974aa8a740d97373ea95bdf297 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Tue, 2 May 2023 12:08:32 -0600 Subject: [PATCH 2/7] Address code review comments --- docs/gateway-api-compatibility.md | 2 +- examples/cafe-example/cafe-routes.yaml | 2 +- internal/nginx/config/servers.go | 8 +- internal/nginx/config/servers_test.go | 86 +++++++++++++++++-- internal/state/change_processor_test.go | 34 ++++---- internal/state/dataplane/configuration.go | 4 +- .../state/dataplane/configuration_test.go | 34 ++++---- internal/state/graph/httproute_test.go | 2 +- 8 files changed, 120 insertions(+), 52 deletions(-) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index bbfd6d967a..a2f3dfe964 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -83,7 +83,7 @@ Fields: * `hostnames` - partially supported. Wildcard binding is not supported: a hostname like `example.com` will not bind to a listener with the hostname `*.example.com`. However, `example.com` will bind to a listener with the empty hostname. * `rules` * `matches` - * `path` - partially supported. Only `PathPrefix` and `Exact` type. + * `path` - Core (`PathPrefix` and `Exact` types) -- supported; Implementation-specific (`RegularExpression` type) -- not supported. * `headers` - partially supported. Only `Exact` type. * `queryParams` - partially supported. Only `Exact` type. * `method` - supported. diff --git a/examples/cafe-example/cafe-routes.yaml b/examples/cafe-example/cafe-routes.yaml index e47a50d0ef..1fd9e95a88 100644 --- a/examples/cafe-example/cafe-routes.yaml +++ b/examples/cafe-example/cafe-routes.yaml @@ -30,7 +30,7 @@ spec: rules: - matches: - path: - type: PathPrefix + type: Exact value: /tea backendRefs: - name: tea diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index f72855f6df..69a2eafac7 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -106,7 +106,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo Exact: *m.Path.Type == v1beta1.PathMatchExact, } } else { - path := createPathForMatch(rule.Path, rule.PathType, matchRuleIdx) + path := createPathForMatch(rule.Path, v1beta1.PathMatchType(rule.PathType), matchRuleIdx) loc = createMatchLocation(path) matches = append(matches, createHTTPMatch(m, path)) } @@ -152,7 +152,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo pathLoc := http.Location{ Path: rule.Path, - Exact: rule.PathType == v1beta1.PathMatchExact, + Exact: v1beta1.PathMatchType(rule.PathType) == v1beta1.PathMatchExact, HTTPMatchVar: string(b), } @@ -324,8 +324,8 @@ func formatPathType(pathType v1beta1.PathMatchType) string { case v1beta1.PathMatchExact: return "exact" case v1beta1.PathMatchRegularExpression: - return "regex" + panic(fmt.Sprintf("unsupported path type: %s", pathType)) default: - return "" + panic(fmt.Sprintf("unsupported path type: %s", pathType)) } } diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 132543f30e..d00e023a80 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -289,6 +289,29 @@ func TestCreateServers(t *testing.T) { }, }, }, + { + // A match using type Exact + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetPointer("/exact"), + Type: helpers.GetPointer(v1beta1.PathMatchExact), + }, + }, + }, + }, + { + // A match using type Exact with method + Matches: []v1beta1.HTTPRouteMatch{ + { + Path: &v1beta1.HTTPPathMatch{ + Value: helpers.GetPointer("/test"), + Type: helpers.GetPointer(v1beta1.PathMatchExact), + }, + Method: helpers.GetHTTPMethodPointer(v1beta1.HTTPMethodGet), + }, + }, + }, }, }, } @@ -347,7 +370,7 @@ func TestCreateServers(t *testing.T) { cafePathRules := []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -371,7 +394,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/test", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -383,7 +406,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/path-only", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -395,7 +418,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/redirect-implicit-port", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -412,7 +435,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/redirect-explicit-port", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -430,7 +453,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/invalid-filter", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -443,6 +466,30 @@ func TestCreateServers(t *testing.T) { }, }, }, + { + Path: "/exact", + PathType: string(v1beta1.PathMatchExact), + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 6, + Source: hr, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/test", + PathType: string(v1beta1.PathMatchExact), + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 7, + Source: hr, + BackendGroup: fooGroup, + }, + }, + }, } httpServers := []dataplane.VirtualServer{ @@ -487,6 +534,12 @@ func TestCreateServers(t *testing.T) { RedirectPath: "/test_prefix_route0", }, } + exactMatches := []httpMatch{ + { + Method: v1beta1.HTTPMethodGet, + RedirectPath: "/test_exact_route0", + }, + } getExpectedLocations := func(isHTTPS bool) []http.Location { port := 80 @@ -547,6 +600,21 @@ func TestCreateServers(t *testing.T) { Code: http.StatusInternalServerError, }, }, + { + Path: "/exact", + ProxyPass: "http://test_foo_80", + Exact: true, + }, + { + Path: "/test_exact_route0", + ProxyPass: "http://test_foo_80", + Internal: true, + }, + { + Path: "/test", + HTTPMatchVar: expectedMatchString(exactMatches), + Exact: true, + }, } } @@ -1106,6 +1174,8 @@ func TestCreateMatchLocation(t *testing.T) { } func TestCreatePathForMatch(t *testing.T) { + g := NewGomegaWithT(t) + tests := []struct { expected string pathType v1beta1.PathMatchType @@ -1126,8 +1196,6 @@ func TestCreatePathForMatch(t *testing.T) { for _, tc := range tests { result := createPathForMatch("/path", tc.pathType, 1) - if result != tc.expected { - t.Errorf("createPathForMatch() returned %q but expected %q", result, tc.expected) - } + g.Expect(result).To(Equal(tc.expected)) } } diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index ae30dc6296..80f753711e 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -76,7 +76,7 @@ func createRoute( Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ - Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(v1beta1.PathMatchPathPrefix))), + Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(string(v1beta1.PathMatchPathPrefix)))), Value: helpers.GetStringPointer("/"), }, }, @@ -378,7 +378,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -401,7 +401,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -494,7 +494,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -517,7 +517,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -610,7 +610,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -633,7 +633,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -725,7 +725,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -748,7 +748,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -837,7 +837,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -859,7 +859,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -946,7 +946,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -969,7 +969,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1077,7 +1077,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1100,7 +1100,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -2032,7 +2032,7 @@ var _ = Describe("ChangeProcessor", func() { Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ - Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(v1beta1.PathMatchPathPrefix))), + Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(string(v1beta1.PathMatchPathPrefix)))), Value: helpers.GetStringPointer("/"), }, }, @@ -2120,7 +2120,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 24d64b0507..c138c180b9 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -59,7 +59,7 @@ type PathRule struct { // Path is a path. For example, '/hello'. Path string // PathType is a path type. For example, PathPrefix or Exact. - PathType v1beta1.PathMatchType + PathType string // MatchRules holds routing rules. MatchRules []MatchRule } @@ -257,7 +257,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { rule, exist := hpr.rulesPerHost[h][key] if !exist { rule.Path = path - rule.PathType = *m.Path.Type + rule.PathType = string(*m.Path.Type) } rule.MatchRules = append(rule.MatchRules, MatchRule{ diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 99de131c35..0ab2d7dc65 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -438,7 +438,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -455,7 +455,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -526,7 +526,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -546,7 +546,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -566,7 +566,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -642,7 +642,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -660,7 +660,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/fourth", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -672,7 +672,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/third", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -697,7 +697,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -715,7 +715,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/fourth", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -727,7 +727,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/third", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -858,7 +858,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -873,7 +873,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: invalidFiltersPath, - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -941,7 +941,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -966,7 +966,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1026,7 +1026,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: v1beta1.PathMatchExact, + PathType: string(v1beta1.PathMatchExact), MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1038,7 +1038,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/valid", - PathType: v1beta1.PathMatchPathPrefix, + PathType: string(v1beta1.PathMatchPathPrefix), MatchRules: []MatchRule{ { MatchIdx: 0, diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index e9cfe6c649..8e44082aaf 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -1180,7 +1180,7 @@ func TestValidateMatch(t *testing.T) { }, }, expectErrCount: 0, - name: "valid", + name: "valid exact match", }, { validator: createAllValidValidator(), From f682fb0c20f774d9cb3611e211ad73ea698c1b42 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Tue, 2 May 2023 15:25:28 -0600 Subject: [PATCH 3/7] Fix lint and test --- internal/nginx/config/servers_test.go | 4 ---- internal/state/change_processor_test.go | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index d00e023a80..cf2857ea6d 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -1188,10 +1188,6 @@ func TestCreatePathForMatch(t *testing.T) { expected: "/path_exact_route1", pathType: v1beta1.PathMatchExact, }, - { - expected: "/path_regex_route1", - pathType: v1beta1.PathMatchRegularExpression, - }, } for _, tc := range tests { diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 80f753711e..f6c3e68a7b 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -76,7 +76,7 @@ func createRoute( Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ - Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(string(v1beta1.PathMatchPathPrefix)))), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), Value: helpers.GetStringPointer("/"), }, }, @@ -2032,7 +2032,7 @@ var _ = Describe("ChangeProcessor", func() { Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ - Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(string(v1beta1.PathMatchPathPrefix)))), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), Value: helpers.GetStringPointer("/"), }, }, From 4ff9664bc205ea00aec27f64f42299b097ebfb92 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Tue, 2 May 2023 16:26:39 -0600 Subject: [PATCH 4/7] Remove regex panic --- internal/nginx/config/servers.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 69a2eafac7..936031a182 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -323,8 +323,6 @@ func formatPathType(pathType v1beta1.PathMatchType) string { return "prefix" case v1beta1.PathMatchExact: return "exact" - case v1beta1.PathMatchRegularExpression: - panic(fmt.Sprintf("unsupported path type: %s", pathType)) default: panic(fmt.Sprintf("unsupported path type: %s", pathType)) } From 1966ec330194570c8690e4b1998b5947ff4feb51 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Wed, 3 May 2023 12:35:03 -0600 Subject: [PATCH 5/7] Change path type to constant --- internal/nginx/config/servers.go | 8 ++--- internal/nginx/config/servers_test.go | 22 ++++++------ internal/state/change_processor_test.go | 30 ++++++++-------- internal/state/dataplane/configuration.go | 6 +++- .../state/dataplane/configuration_test.go | 34 +++++++++---------- 5 files changed, 52 insertions(+), 48 deletions(-) diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index 936031a182..c6101abe49 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -106,7 +106,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo Exact: *m.Path.Type == v1beta1.PathMatchExact, } } else { - path := createPathForMatch(rule.Path, v1beta1.PathMatchType(rule.PathType), matchRuleIdx) + path := createPathForMatch(rule.Path, rule.PathType, matchRuleIdx) loc = createMatchLocation(path) matches = append(matches, createHTTPMatch(m, path)) } @@ -306,7 +306,7 @@ func createMatchLocation(path string) http.Location { } } -func createPathForMatch(path string, pathType v1beta1.PathMatchType, routeIdx int) string { +func createPathForMatch(path, pathType string, routeIdx int) string { return fmt.Sprintf("%s_%s_route%d", path, formatPathType(pathType), routeIdx) } @@ -317,8 +317,8 @@ func createDefaultRootLocation() http.Location { } } -func formatPathType(pathType v1beta1.PathMatchType) string { - switch pathType { +func formatPathType(pathType string) string { + switch v1beta1.PathMatchType(pathType) { case v1beta1.PathMatchPathPrefix: return "prefix" case v1beta1.PathMatchExact: diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index cf2857ea6d..9ed1c32bb9 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -370,7 +370,7 @@ func TestCreateServers(t *testing.T) { cafePathRules := []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -394,7 +394,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/test", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -406,7 +406,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/path-only", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -418,7 +418,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/redirect-implicit-port", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -435,7 +435,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/redirect-explicit-port", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -453,7 +453,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/invalid-filter", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -468,7 +468,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/exact", - PathType: string(v1beta1.PathMatchExact), + PathType: dataplane.PathMatchExact, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -480,7 +480,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/test", - PathType: string(v1beta1.PathMatchExact), + PathType: dataplane.PathMatchExact, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1178,15 +1178,15 @@ func TestCreatePathForMatch(t *testing.T) { tests := []struct { expected string - pathType v1beta1.PathMatchType + pathType string }{ { expected: "/path_prefix_route1", - pathType: v1beta1.PathMatchPathPrefix, + pathType: dataplane.PathMatchPathPrefix, }, { expected: "/path_exact_route1", - pathType: v1beta1.PathMatchExact, + pathType: dataplane.PathMatchExact, }, } diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index f6c3e68a7b..75e6042336 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -378,7 +378,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -401,7 +401,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -494,7 +494,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -517,7 +517,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -610,7 +610,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -633,7 +633,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -725,7 +725,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -748,7 +748,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -837,7 +837,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -859,7 +859,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -946,7 +946,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -969,7 +969,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1077,7 +1077,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1100,7 +1100,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -2120,7 +2120,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: dataplane.PathMatchPathPrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index c138c180b9..15158bb743 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -11,7 +11,11 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" ) -const wildcardHostname = "~^" +const ( + wildcardHostname = "~^" + PathMatchPathPrefix = string(v1beta1.PathMatchPathPrefix) + PathMatchExact = string(v1beta1.PathMatchExact) +) // Configuration is an intermediate representation of dataplane configuration. type Configuration struct { diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 0ab2d7dc65..34bcbd20d4 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -438,7 +438,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -455,7 +455,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -526,7 +526,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -546,7 +546,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -566,7 +566,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -642,7 +642,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -660,7 +660,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/fourth", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -672,7 +672,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/third", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -697,7 +697,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -715,7 +715,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/fourth", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -727,7 +727,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/third", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -858,7 +858,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -873,7 +873,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: invalidFiltersPath, - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -941,7 +941,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -966,7 +966,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1026,7 +1026,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: string(v1beta1.PathMatchExact), + PathType: PathMatchExact, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1038,7 +1038,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/valid", - PathType: string(v1beta1.PathMatchPathPrefix), + PathType: PathMatchPathPrefix, MatchRules: []MatchRule{ { MatchIdx: 0, From c5113c0d8a6a63d99b8e54e0c9a5331ec529804b Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 4 May 2023 11:59:20 -0600 Subject: [PATCH 6/7] Revert doc update --- docs/gateway-api-compatibility.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index a2f3dfe964..8346361c43 100644 --- a/docs/gateway-api-compatibility.md +++ b/docs/gateway-api-compatibility.md @@ -83,7 +83,7 @@ Fields: * `hostnames` - partially supported. Wildcard binding is not supported: a hostname like `example.com` will not bind to a listener with the hostname `*.example.com`. However, `example.com` will bind to a listener with the empty hostname. * `rules` * `matches` - * `path` - Core (`PathPrefix` and `Exact` types) -- supported; Implementation-specific (`RegularExpression` type) -- not supported. + * `path` - partially supported. Only `PathPrefix` and `Exact` types. * `headers` - partially supported. Only `Exact` type. * `queryParams` - partially supported. Only `Exact` type. * `method` - supported. From dba51b6bdd5f55a5eef0610645b2ded95bbe08f0 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 4 May 2023 14:05:53 -0600 Subject: [PATCH 7/7] Use defined path type --- internal/nginx/config/servers.go | 19 ++---- internal/nginx/config/servers_test.go | 23 +++---- internal/state/change_processor_test.go | 30 ++++----- internal/state/dataplane/configuration.go | 25 +++++-- .../state/dataplane/configuration_test.go | 66 ++++++++++++++----- 5 files changed, 99 insertions(+), 64 deletions(-) diff --git a/internal/nginx/config/servers.go b/internal/nginx/config/servers.go index c6101abe49..cbb28db872 100644 --- a/internal/nginx/config/servers.go +++ b/internal/nginx/config/servers.go @@ -103,7 +103,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo if len(rule.MatchRules) == 1 && isPathOnlyMatch(m) { loc = http.Location{ Path: rule.Path, - Exact: *m.Path.Type == v1beta1.PathMatchExact, + Exact: rule.PathType == dataplane.PathTypeExact, } } else { path := createPathForMatch(rule.Path, rule.PathType, matchRuleIdx) @@ -152,7 +152,7 @@ func createLocations(pathRules []dataplane.PathRule, listenerPort int) []http.Lo pathLoc := http.Location{ Path: rule.Path, - Exact: v1beta1.PathMatchType(rule.PathType) == v1beta1.PathMatchExact, + Exact: rule.PathType == dataplane.PathTypeExact, HTTPMatchVar: string(b), } @@ -306,8 +306,8 @@ func createMatchLocation(path string) http.Location { } } -func createPathForMatch(path, pathType string, routeIdx int) string { - return fmt.Sprintf("%s_%s_route%d", path, formatPathType(pathType), routeIdx) +func createPathForMatch(path string, pathType dataplane.PathType, routeIdx int) string { + return fmt.Sprintf("%s_%s_route%d", path, pathType, routeIdx) } func createDefaultRootLocation() http.Location { @@ -316,14 +316,3 @@ func createDefaultRootLocation() http.Location { Return: &http.Return{Code: http.StatusNotFound}, } } - -func formatPathType(pathType string) string { - switch v1beta1.PathMatchType(pathType) { - case v1beta1.PathMatchPathPrefix: - return "prefix" - case v1beta1.PathMatchExact: - return "exact" - default: - panic(fmt.Sprintf("unsupported path type: %s", pathType)) - } -} diff --git a/internal/nginx/config/servers_test.go b/internal/nginx/config/servers_test.go index 9ed1c32bb9..bbd7c5e796 100644 --- a/internal/nginx/config/servers_test.go +++ b/internal/nginx/config/servers_test.go @@ -370,7 +370,7 @@ func TestCreateServers(t *testing.T) { cafePathRules := []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -394,7 +394,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/test", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -406,7 +406,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/path-only", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -418,7 +418,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/redirect-implicit-port", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -435,7 +435,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/redirect-explicit-port", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -453,7 +453,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/invalid-filter", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -468,7 +468,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/exact", - PathType: dataplane.PathMatchExact, + PathType: dataplane.PathTypeExact, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -480,7 +480,7 @@ func TestCreateServers(t *testing.T) { }, { Path: "/test", - PathType: dataplane.PathMatchExact, + PathType: dataplane.PathTypeExact, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1178,15 +1178,16 @@ func TestCreatePathForMatch(t *testing.T) { tests := []struct { expected string - pathType string + pathType dataplane.PathType + panic bool }{ { expected: "/path_prefix_route1", - pathType: dataplane.PathMatchPathPrefix, + pathType: dataplane.PathTypePrefix, }, { expected: "/path_exact_route1", - pathType: dataplane.PathMatchExact, + pathType: dataplane.PathTypeExact, }, } diff --git a/internal/state/change_processor_test.go b/internal/state/change_processor_test.go index 75e6042336..c9ad15ec88 100644 --- a/internal/state/change_processor_test.go +++ b/internal/state/change_processor_test.go @@ -378,7 +378,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -401,7 +401,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -494,7 +494,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -517,7 +517,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -610,7 +610,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -633,7 +633,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -725,7 +725,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -748,7 +748,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -837,7 +837,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -859,7 +859,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -946,7 +946,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -969,7 +969,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1077,7 +1077,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1100,7 +1100,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -2120,7 +2120,7 @@ var _ = Describe("ChangeProcessor", func() { PathRules: []dataplane.PathRule{ { Path: "/", - PathType: dataplane.PathMatchPathPrefix, + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 15158bb743..311105484c 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -11,10 +11,12 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" ) +type PathType string + const ( - wildcardHostname = "~^" - PathMatchPathPrefix = string(v1beta1.PathMatchPathPrefix) - PathMatchExact = string(v1beta1.PathMatchExact) + wildcardHostname = "~^" + PathTypePrefix PathType = "prefix" + PathTypeExact PathType = "exact" ) // Configuration is an intermediate representation of dataplane configuration. @@ -62,8 +64,8 @@ type SSL struct { type PathRule struct { // Path is a path. For example, '/hello'. Path string - // PathType is a path type. For example, PathPrefix or Exact. - PathType string + // PathType is simplified path type. For example, prefix or exact. + PathType PathType // MatchRules holds routing rules. MatchRules []MatchRule } @@ -261,7 +263,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { rule, exist := hpr.rulesPerHost[h][key] if !exist { rule.Path = path - rule.PathType = string(*m.Path.Type) + rule.PathType = convertPathType(*m.Path.Type) } rule.MatchRules = append(rule.MatchRules, MatchRule{ @@ -421,3 +423,14 @@ func createFilters(filters []v1beta1.HTTPRouteFilter) Filters { return result } + +func convertPathType(pathType v1beta1.PathMatchType) PathType { + switch pathType { + case v1beta1.PathMatchPathPrefix: + return PathTypePrefix + case v1beta1.PathMatchExact: + return PathTypeExact + default: + panic(fmt.Sprintf("unsupported path type: %s", pathType)) + } +} diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 34bcbd20d4..96da054a00 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -438,7 +438,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -455,7 +455,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -526,7 +526,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -546,7 +546,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -566,7 +566,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -642,7 +642,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -660,7 +660,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/fourth", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -672,7 +672,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/third", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -697,7 +697,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -715,7 +715,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/fourth", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -727,7 +727,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/third", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -858,7 +858,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -873,7 +873,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: invalidFiltersPath, - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -941,7 +941,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -966,7 +966,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1026,7 +1026,7 @@ func TestBuildConfiguration(t *testing.T) { PathRules: []PathRule{ { Path: "/valid", - PathType: PathMatchExact, + PathType: PathTypeExact, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1038,7 +1038,7 @@ func TestBuildConfiguration(t *testing.T) { }, { Path: "/valid", - PathType: PathMatchPathPrefix, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -1594,3 +1594,35 @@ func TestUpstreamsMapToSlice(t *testing.T) { t.Errorf("upstreamMapToSlice() mismatch (-want +got):\n%s", diff) } } + +func TestConvertPathType(t *testing.T) { + g := NewGomegaWithT(t) + + tests := []struct { + expected PathType + pathType v1beta1.PathMatchType + panic bool + }{ + { + expected: PathTypePrefix, + pathType: v1beta1.PathMatchPathPrefix, + }, + { + expected: PathTypeExact, + pathType: v1beta1.PathMatchExact, + }, + { + pathType: v1beta1.PathMatchRegularExpression, + panic: true, + }, + } + + for _, tc := range tests { + if tc.panic { + g.Expect(func() { convertPathType(tc.pathType) }).To(Panic()) + } else { + result := convertPathType(tc.pathType) + g.Expect(result).To(Equal(tc.expected)) + } + } +}