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

Add exact matches and regex support to location paths in VS/VSR #766

Merged
merged 1 commit into from
Nov 22, 2019
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
10 changes: 8 additions & 2 deletions docs/virtualserver-and-virtualserverroute.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ spec:
- path: /coffee
action:
pass: coffee
- path: ~ ^/decaf/.*\\.jpg$
action:
pass: coffee
- path: = /green/tea
action:
pass: tea
```

| Field | Description | Type | Required |
Expand Down Expand Up @@ -113,7 +119,7 @@ The route defines rules for matching client requests to actions like passing a r

| Field | Description | Type | Required |
| ----- | ----------- | ---- | -------- |
| `path` | The path of the route. NGINX will match it against the URI of a request. The path must start with `/` and must not include any whitespace characters, `{`, `}` or `;`. For example, `/`, `/path` are valid. The path must be unique among the paths of all routes of the VirtualServer. | `string` | Yes |
| `path` | The path of the route. NGINX will match it against the URI of a request. Possible values are: a prefix (`/`, `/path`), an exact match (`=/exact/match`), a case insensitive regular expression (`~*^/Bar.*\\.jpg`) or a case sensitive regular expression (`~^/foo.*\\.jpg`). In the case of a prefix (must start with `/`) or an exact match (must start with `=`), the path must not include any whitespace characters, `{`, `}` or `;`. In the case of the regex matches, all double quotes `"` must be escaped and the match can't end in an unescaped backslash `\`. The path must be unique among the paths of all routes of the VirtualServer. Check the [location](http://nginx.org/en/docs/http/ngx_http_core_module.html#location) directive for more information. | `string` | Yes |
| `action` | The default action to perform for a request. | [`action`](#Action) | No* |
| `splits` | The default splits configuration for traffic splitting. Must include at least 2 splits. | [`[]split`](#Split) | No* |
| `matches` | The matching rules for advanced content-based routing. Requires the default `action` or `splits`. Unmatched requests will be handled by the default `action` or `splits`. |[`matches`](#Match) | No |
Expand Down Expand Up @@ -193,7 +199,7 @@ action:

| Field | Description | Type | Required |
| ----- | ----------- | ---- | -------- |
| `path` | The path of the subroute. NGINX will match it against the URI of a request. The path must start with the same path as the path of the route of the VirtualServer that references this resource. It must not include any whitespace characters, `{`, `}` or `;`. The path must be unique among the paths of all subroutes of the VirtualServerRoute. | `string` | Yes |
| `path` | The path of the subroute. NGINX will match it against the URI of a request. Possible values are: a prefix (`/`, `/path`), an exact match (`=/exact/match`), a case insensitive regular expression (`~*^/Bar.*\\.jpg`) or a case sensitive regular expression (`~^/foo.*\\.jpg`). In the case of a prefix, the path must start with the same path as the path of the route of the VirtualServer that references this resource. In the case of an exact or regex match, the path must be the same as the path of the route of the VirtualServer that references this resource. In the case of a prefix or an exact match, the path must not include any whitespace characters, `{`, `}` or `;`. In the case of the regex matches, all double quotes `"` must be escaped and the match can't end in an unescaped backslash `\`. The path must be unique among the paths of all subroutes of the VirtualServerRoute. | `string` | Yes |
| `action` | The default action to perform for a request. | [`action`](#Action) | No* |
| `splits` | The default splits configuration for traffic splitting. Must include at least 2 splits. | [`[]split`](#Split) | No* |
| `matches` | The matching rules for advanced content-based routing. Requires the default `action` or `splits`. Unmatched requests will be handled by the default `action` or `splits`. |[`matches`](#Match) | No |
Expand Down
14 changes: 13 additions & 1 deletion internal/configs/virtualserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,21 @@ func generateBool(s *bool, defaultS bool) bool {
return defaultS
}

func generatePath(path string) string {
// Wrap the regular expression (if present) inside double quotes (") to avoid NGINX parsing errors
if strings.HasPrefix(path, "~*") {
return fmt.Sprintf(`~* "%v"`, strings.TrimPrefix(strings.TrimPrefix(path, "~*"), " "))
Rulox marked this conversation as resolved.
Show resolved Hide resolved
}
if strings.HasPrefix(path, "~") {
return fmt.Sprintf(`~ "%v"`, strings.TrimPrefix(strings.TrimPrefix(path, "~"), " "))
}

return path
}

func generateLocation(path string, upstreamName string, upstream conf_v1alpha1.Upstream, cfgParams *ConfigParams) version2.Location {
return version2.Location{
Path: path,
Path: generatePath(path),
Snippets: cfgParams.LocationSnippets,
ProxyConnectTimeout: generateString(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout),
ProxyReadTimeout: generateString(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout),
Expand Down
31 changes: 31 additions & 0 deletions internal/configs/virtualserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2883,3 +2883,34 @@ func TestGenerateSessionCookie(t *testing.T) {
}
}
}

func TestGeneratePath(t *testing.T) {
tests := []struct {
path string
expected string
}{
{
path: "/",
expected: "/",
},
{
path: "=/exact/match",
expected: "=/exact/match",
},
{
path: `~ *\\.jpg`,
expected: `~ "*\\.jpg"`,
},
{
path: `~* *\\.PNG`,
expected: `~* "*\\.PNG"`,
},
}

for _, test := range tests {
result := generatePath(test.path)
if result != test.expected {
t.Errorf("generatePath() returned %v, but expected %v.", result, test.expected)
}
}
}
76 changes: 67 additions & 9 deletions pkg/apis/configuration/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ func validateVirtualServerRoutes(routes []v1alpha1.Route, fieldPath *field.Path,
func validateRoute(route v1alpha1.Route, fieldPath *field.Path, upstreamNames sets.String, isRouteFieldForbidden bool) field.ErrorList {
allErrs := field.ErrorList{}

allErrs = append(allErrs, validatePath(route.Path, fieldPath.Child("path"))...)
allErrs = append(allErrs, validateRoutePath(route.Path, fieldPath.Child("path"))...)

fieldCount := 0

Expand Down Expand Up @@ -660,7 +660,48 @@ func validateSplits(splits []v1alpha1.Split, fieldPath *field.Path, upstreamName
return allErrs
}

// For now, we only support prefix-based NGINX locations. For example, location /abc { ... }.
// We support prefix-based NGINX locations, positive case-sensitive/insensitive regular expressions matches and exact matches.
// More info http://nginx.org/en/docs/http/ngx_http_core_module.html#location
func validateRoutePath(path string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if path == "" {
return append(allErrs, field.Required(fieldPath, ""))
}

if strings.HasPrefix(path, "~") {
allErrs = append(allErrs, validateRegexPath(path, fieldPath)...)
} else if strings.HasPrefix(path, "/") {
allErrs = append(allErrs, validatePath(path, fieldPath)...)
} else if strings.HasPrefix(path, "=") {
allErrs = append(allErrs, validatePath(strings.TrimPrefix(path, "="), fieldPath)...)
} else {
allErrs = append(allErrs, field.Invalid(fieldPath, path, "must start with /, ~ or ="))
}

return allErrs
}

const regexPathFmt = `([^"\\]|\\.)*`
const regexPathErrMsg = `must have all '"' (double quotes) escaped and must not end with an unescaped '\' (backslash)`

var regexPathRegexp = regexp.MustCompile("^" + regexPathFmt + "$")

func validateRegexPath(path string, fieldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}

if _, err := regexp.Compile(path); err != nil {
return append(allErrs, field.Invalid(fieldPath, path, fmt.Sprintf("must be a valid regular expression: %v", err)))
}

if !regexPathRegexp.MatchString(path) {
msg := validation.RegexError(regexPathErrMsg, regexPathFmt, "*.jpg", "^/images/image_*.png$")
return append(allErrs, field.Invalid(fieldPath, path, msg))
}

return allErrs
}

const pathFmt = `/[^\s{};]*`
const pathErrMsg = "must start with / and must not include any whitespace character, `{`, `}` or `;`"

Expand Down Expand Up @@ -826,20 +867,20 @@ func ValidateVirtualServerRoute(virtualServerRoute *v1alpha1.VirtualServerRoute,
}

// ValidateVirtualServerRouteForVirtualServer validates a VirtualServerRoute for a VirtualServer represented by its host and path prefix.
func ValidateVirtualServerRouteForVirtualServer(virtualServerRoute *v1alpha1.VirtualServerRoute, virtualServerHost string, pathPrefix string, isPlus bool) error {
allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), virtualServerHost, pathPrefix, isPlus)
func ValidateVirtualServerRouteForVirtualServer(virtualServerRoute *v1alpha1.VirtualServerRoute, virtualServerHost string, vsPath string, isPlus bool) error {
allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), virtualServerHost, vsPath, isPlus)
return allErrs.ToAggregate()
}

func validateVirtualServerRouteSpec(spec *v1alpha1.VirtualServerRouteSpec, fieldPath *field.Path, virtualServerHost string, pathPrefix string, isPlus bool) field.ErrorList {
func validateVirtualServerRouteSpec(spec *v1alpha1.VirtualServerRouteSpec, fieldPath *field.Path, virtualServerHost string, vsPath string, isPlus bool) field.ErrorList {
allErrs := field.ErrorList{}

allErrs = append(allErrs, validateVirtualServerRouteHost(spec.Host, virtualServerHost, fieldPath.Child("host"))...)

upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"), isPlus)
allErrs = append(allErrs, upstreamErrs...)

allErrs = append(allErrs, validateVirtualServerRouteSubroutes(spec.Subroutes, fieldPath.Child("subroutes"), upstreamNames, pathPrefix)...)
allErrs = append(allErrs, validateVirtualServerRouteSubroutes(spec.Subroutes, fieldPath.Child("subroutes"), upstreamNames, vsPath)...)

return allErrs
}
Expand All @@ -857,19 +898,36 @@ func validateVirtualServerRouteHost(host string, virtualServerHost string, field
return allErrs
}

func validateVirtualServerRouteSubroutes(routes []v1alpha1.Route, fieldPath *field.Path, upstreamNames sets.String, pathPrefix string) field.ErrorList {
func isRegexOrExactMatch(path string) bool {
Rulox marked this conversation as resolved.
Show resolved Hide resolved
return strings.HasPrefix(path, "~") || strings.HasPrefix(path, "=")
}

func validateVirtualServerRouteSubroutes(routes []v1alpha1.Route, fieldPath *field.Path, upstreamNames sets.String, vsPath string) field.ErrorList {
allErrs := field.ErrorList{}

allPaths := sets.String{}

if isRegexOrExactMatch(vsPath) {
if len(routes) != 1 {
return append(allErrs, field.Invalid(fieldPath, "subroutes", "must have only one subroute if regex match or exact match are being used"))
}

idxPath := fieldPath.Index(0)
if routes[0].Path != vsPath {
return append(allErrs, field.Invalid(idxPath.Child("path"), routes[0].Path, "must have the same path as the referenced VirtualServer route path"))
}

return validateRoute(routes[0], idxPath, upstreamNames, true)
}

for i, r := range routes {
idxPath := fieldPath.Index(i)

isRouteFieldForbidden := true
routeErrs := validateRoute(r, idxPath, upstreamNames, isRouteFieldForbidden)

if pathPrefix != "" && !strings.HasPrefix(r.Path, pathPrefix) {
msg := fmt.Sprintf("must start with '%s'", pathPrefix)
if vsPath != "" && !strings.HasPrefix(r.Path, vsPath) && !isRegexOrExactMatch(r.Path) {
msg := fmt.Sprintf("must start with '%s'", vsPath)
routeErrs = append(routeErrs, field.Invalid(idxPath, r.Path, msg))
}

Expand Down
113 changes: 113 additions & 0 deletions pkg/apis/configuration/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,92 @@ func TestValdateUpstreamFails(t *testing.T) {
}
}

func TestValidateRegexPath(t *testing.T) {
tests := []struct {
regexPath string
msg string
}{
{
regexPath: "~ ^/foo.*\\.jpg",
msg: "case sensitive regexp",
},
{
regexPath: "~* ^/Bar.*\\.jpg",
msg: "case insensitive regexp",
},
{
regexPath: `~ ^/f\"oo.*\\.jpg`,
msg: "regexp with escaped double quotes",
},
}
Rulox marked this conversation as resolved.
Show resolved Hide resolved

for _, test := range tests {
allErrs := validateRegexPath(test.regexPath, field.NewPath("path"))
if len(allErrs) != 0 {
t.Errorf("validateRegexPath(%v) returned errors for valid input for the case of %v", test.regexPath, test.msg)
}
}
}

func TestValidateRegexPathFails(t *testing.T) {
tests := []struct {
regexPath string
msg string
}{
{
regexPath: "~ [{",
msg: "invalid regexp",
},
{
regexPath: `~ /foo"`,
msg: "unescaped double quotes",
},
{
regexPath: `~"`,
msg: "empty regex",
},
{
regexPath: `~ /foo\`,
msg: "ending in backslash",
},
}
Rulox marked this conversation as resolved.
Show resolved Hide resolved

for _, test := range tests {
allErrs := validateRegexPath(test.regexPath, field.NewPath("path"))
if len(allErrs) == 0 {
t.Errorf("validateRegexPath(%v) returned no errors for invalid input for the case of %v", test.regexPath, test.msg)
}
}
}

func TestValidateRoutePath(t *testing.T) {
validPaths := []string{
"/",
"~ /^foo.*\\.jpg",
"~* /^Bar.*\\.jpg",
"=/exact/match",
}

for _, path := range validPaths {
allErrs := validateRoutePath(path, field.NewPath("path"))
if len(allErrs) != 0 {
t.Errorf("validateRoutePath(%v) returned errors for valid input", path)
}
}

invalidPaths := []string{
"",
"invalid",
}

for _, path := range invalidPaths {
allErrs := validateRoutePath(path, field.NewPath("path"))
if len(allErrs) == 0 {
t.Errorf("validateRoutePath(%v) returned no errors for invalid input", path)
}
}
}

func TestValidatePath(t *testing.T) {
validPaths := []string{
"/",
Expand Down Expand Up @@ -2289,3 +2375,30 @@ func TestValidateTLSRedirectStatusCodeFails(t *testing.T) {
}
}
}

func TestIsRegexOrExactMatch(t *testing.T) {
tests := []struct {
path string
expected bool
}{
{
path: "/path",
expected: false,
},
{
path: "~ .*\\.jpg",
expected: true,
},
{
path: "=/exact/match",
expected: true,
},
}

for _, test := range tests {
result := isRegexOrExactMatch(test.path)
if result != test.expected {
t.Errorf("isRegexOrExactMatch(%v) returned %v but expected %v", test.path, result, test.expected)
}
}
}