diff --git a/docs/gateway-api-compatibility.md b/docs/gateway-api-compatibility.md index 716f09615..8346361c4 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` types. * `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 e47a50d0e..1fd9e95a8 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/http/config.go b/internal/nginx/config/http/config.go index f799bfb34..f1538109a 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 2c0b1c52c..cbb28db87 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: rule.PathType == dataplane.PathTypeExact, } } 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 == dataplane.PathTypeExact, 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 dataplane.PathType, routeIdx int) string { + return fmt.Sprintf("%s_%s_route%d", path, pathType, routeIdx) } func createDefaultRootLocation() http.Location { diff --git a/internal/nginx/config/servers_template.go b/internal/nginx/config/servers_template.go index f75af840b..16736564d 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 043835e19..bbd7c5e79 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,10 +284,34 @@ func TestCreateServers(t *testing.T) { { Path: &v1beta1.HTTPPathMatch{ Value: helpers.GetPointer("/invalid-filter"), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), }, }, }, }, + { + // 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), + }, + }, + }, }, }, } @@ -338,7 +369,8 @@ func TestCreateServers(t *testing.T) { cafePathRules := []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -361,7 +393,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/test", + Path: "/test", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -372,7 +405,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/path-only", + Path: "/path-only", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -383,7 +417,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/redirect-implicit-port", + Path: "/redirect-implicit-port", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -399,7 +434,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/redirect-explicit-port", + Path: "/redirect-explicit-port", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -416,7 +452,8 @@ func TestCreateServers(t *testing.T) { }, }, { - Path: "/invalid-filter", + Path: "/invalid-filter", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -429,6 +466,30 @@ func TestCreateServers(t *testing.T) { }, }, }, + { + Path: "/exact", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 6, + Source: hr, + BackendGroup: fooGroup, + }, + }, + }, + { + Path: "/test", + PathType: dataplane.PathTypeExact, + MatchRules: []dataplane.MatchRule{ + { + MatchIdx: 0, + RuleIdx: 7, + Source: hr, + BackendGroup: fooGroup, + }, + }, + }, } httpServers := []dataplane.VirtualServer{ @@ -461,16 +522,22 @@ 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", + }, + } + exactMatches := []httpMatch{ + { + Method: v1beta1.HTTPMethodGet, + RedirectPath: "/test_exact_route0", }, } @@ -482,17 +549,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 +568,7 @@ func TestCreateServers(t *testing.T) { HTTPMatchVar: expectedMatchString(slashMatches), }, { - Path: "/test_route0", + Path: "/test_prefix_route0", Internal: true, ProxyPass: "http://$test__route1_rule1", }, @@ -533,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, + }, } } @@ -579,11 +661,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 +680,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 +1174,25 @@ func TestCreateMatchLocation(t *testing.T) { } func TestCreatePathForMatch(t *testing.T) { - expected := "/path_route1" + g := NewGomegaWithT(t) - result := createPathForMatch("/path", 1) - if result != expected { - t.Errorf("createPathForMatch() returned %q but expected %q", result, expected) + tests := []struct { + expected string + pathType dataplane.PathType + panic bool + }{ + { + expected: "/path_prefix_route1", + pathType: dataplane.PathTypePrefix, + }, + { + expected: "/path_exact_route1", + pathType: dataplane.PathTypeExact, + }, + } + + for _, tc := range tests { + result := createPathForMatch("/path", tc.pathType, 1) + g.Expect(result).To(Equal(tc.expected)) } } diff --git a/internal/nginx/config/validation/http_njs_match.go b/internal/nginx/config/validation/http_njs_match.go index 13e68124e..1e5efc2d6 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 0f13613ef..ea5d4862b 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 f4e2bf0d7..c9ad15ec8 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: helpers.GetPointer(v1beta1.PathMatchPathPrefix), Value: helpers.GetStringPointer("/"), }, }, @@ -377,7 +377,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -399,7 +400,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -491,7 +493,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -513,7 +516,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -605,7 +609,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -627,7 +632,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -718,7 +724,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -740,7 +747,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -828,7 +836,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -849,7 +858,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -935,7 +945,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -957,7 +968,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1064,7 +1076,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "bar.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -1086,7 +1099,8 @@ var _ = Describe("ChangeProcessor", func() { SSL: &dataplane.SSL{CertificatePath: certificatePath}, PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, @@ -2018,7 +2032,7 @@ var _ = Describe("ChangeProcessor", func() { Matches: []v1beta1.HTTPRouteMatch{ { Path: &v1beta1.HTTPPathMatch{ - Type: (*v1beta1.PathMatchType)(helpers.GetStringPointer(string(v1beta1.PathMatchPathPrefix))), + Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix), Value: helpers.GetStringPointer("/"), }, }, @@ -2105,7 +2119,8 @@ var _ = Describe("ChangeProcessor", func() { Hostname: "foo.example.com", PathRules: []dataplane.PathRule{ { - Path: "/", + Path: "/", + PathType: dataplane.PathTypePrefix, MatchRules: []dataplane.MatchRule{ { MatchIdx: 0, diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 30650df62..311105484 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -11,7 +11,13 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/resolver" ) -const wildcardHostname = "~^" +type PathType string + +const ( + wildcardHostname = "~^" + PathTypePrefix PathType = "prefix" + PathTypeExact PathType = "exact" +) // Configuration is an intermediate representation of dataplane configuration. type Configuration struct { @@ -58,6 +64,8 @@ type SSL struct { type PathRule struct { // Path is a path. For example, '/hello'. Path string + // PathType is simplified path type. For example, prefix or exact. + PathType PathType // MatchRules holds routing rules. MatchRules []MatchRule } @@ -185,8 +193,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 +207,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 +233,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 +255,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 = convertPathType(*m.Path.Type) } rule.MatchRules = append(rule.MatchRules, MatchRule{ @@ -255,7 +274,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { Filters: filters, }) - hpr.rulesPerHost[h][path] = rule + hpr.rulesPerHost[h][key] = rule } } } @@ -288,7 +307,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) @@ -400,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 f313d6ffa..96da054a0 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: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -437,7 +454,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -507,7 +525,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "bar.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -526,7 +545,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -545,7 +565,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -620,7 +641,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -637,7 +659,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/fourth", + Path: "/fourth", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -648,7 +671,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/third", + Path: "/third", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -672,7 +696,8 @@ func TestBuildConfiguration(t *testing.T) { }, PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -689,7 +714,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/fourth", + Path: "/fourth", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -700,7 +726,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: "/third", + Path: "/third", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -830,7 +857,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/", + Path: "/", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -844,7 +872,8 @@ func TestBuildConfiguration(t *testing.T) { }, }, { - Path: invalidFiltersPath, + Path: invalidFiltersPath, + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -911,7 +940,8 @@ func TestBuildConfiguration(t *testing.T) { Hostname: "foo.example.com", PathRules: []PathRule{ { - Path: "/valid", + Path: "/valid", + PathType: PathTypePrefix, MatchRules: []MatchRule{ { MatchIdx: 0, @@ -935,7 +965,8 @@ func TestBuildConfiguration(t *testing.T) { }, PathRules: []PathRule{ { - Path: "/valid", + Path: "/valid", + PathType: PathTypePrefix, 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: PathTypeExact, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 1, + BackendGroup: hr7Groups[1], + Source: hr7, + }, + }, + }, + { + Path: "/valid", + PathType: PathTypePrefix, + 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 { @@ -1497,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)) + } + } +} diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index b7f0b1dbd..27fbcb825 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 292a0c3ac..8e44082aa 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 exact match", + }, { 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 2040aa665..98e6f9d4b 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 14aff3138..75d68c100 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