Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exact PathMatch support for HTTPRoutes #603

Merged
merged 7 commits into from
May 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 }} {
pleshakov marked this conversation as resolved.
Show resolved Hide resolved
{{ 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
}{
{
sjberman marked this conversation as resolved.
Show resolved Hide resolved
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