Skip to content

Commit

Permalink
Exact PathMatch support for HTTPRoutes (nginxinc#603)
Browse files Browse the repository at this point in the history
* 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".
  • Loading branch information
sjberman authored May 4, 2023
1 parent 768cbf5 commit 3b11e8e
Show file tree
Hide file tree
Showing 15 changed files with 434 additions and 141 deletions.
2 changes: 1 addition & 1 deletion docs/gateway-api-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion examples/cafe-example/cafe-routes.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ spec:
rules:
- matches:
- path:
type: PathPrefix
type: Exact
value: /tea
backendRefs:
- name: tea
Expand Down
1 change: 1 addition & 0 deletions internal/nginx/config/http/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type Location struct {
ProxyPass string
HTTPMatchVar string
Internal bool
Exact bool
}

// Return represents an HTTP return.
Expand Down
10 changes: 6 additions & 4 deletions internal/nginx/config/servers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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),
}

Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/nginx/config/servers_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
136 changes: 118 additions & 18 deletions internal/nginx/config/servers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
},
Expand All @@ -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),
},
},
},
Expand All @@ -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{
Expand Down Expand Up @@ -245,6 +249,7 @@ func TestCreateServers(t *testing.T) {
{
Path: &v1beta1.HTTPPathMatch{
Value: helpers.GetStringPointer("/path-only"),
Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix),
},
},
},
Expand All @@ -255,6 +260,7 @@ func TestCreateServers(t *testing.T) {
{
Path: &v1beta1.HTTPPathMatch{
Value: helpers.GetStringPointer("/redirect-implicit-port"),
Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix),
},
},
},
Expand All @@ -266,6 +272,7 @@ func TestCreateServers(t *testing.T) {
{
Path: &v1beta1.HTTPPathMatch{
Value: helpers.GetStringPointer("/redirect-explicit-port"),
Type: helpers.GetPointer(v1beta1.PathMatchPathPrefix),
},
},
},
Expand All @@ -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),
},
},
},
},
},
}
Expand Down Expand Up @@ -338,7 +369,8 @@ func TestCreateServers(t *testing.T) {

cafePathRules := []dataplane.PathRule{
{
Path: "/",
Path: "/",
PathType: dataplane.PathTypePrefix,
MatchRules: []dataplane.MatchRule{
{
MatchIdx: 0,
Expand All @@ -361,7 +393,8 @@ func TestCreateServers(t *testing.T) {
},
},
{
Path: "/test",
Path: "/test",
PathType: dataplane.PathTypePrefix,
MatchRules: []dataplane.MatchRule{
{
MatchIdx: 0,
Expand All @@ -372,7 +405,8 @@ func TestCreateServers(t *testing.T) {
},
},
{
Path: "/path-only",
Path: "/path-only",
PathType: dataplane.PathTypePrefix,
MatchRules: []dataplane.MatchRule{
{
MatchIdx: 0,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -416,7 +452,8 @@ func TestCreateServers(t *testing.T) {
},
},
{
Path: "/invalid-filter",
Path: "/invalid-filter",
PathType: dataplane.PathTypePrefix,
MatchRules: []dataplane.MatchRule{
{
MatchIdx: 0,
Expand All @@ -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{
Expand Down Expand Up @@ -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",
},
}

Expand All @@ -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",
},
Expand All @@ -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",
},
Expand Down Expand Up @@ -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,
},
}
}

Expand Down Expand Up @@ -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),
},
},
},
Expand All @@ -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),
},
})
}
Expand Down Expand Up @@ -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))
}
}
Loading

0 comments on commit 3b11e8e

Please sign in to comment.