Skip to content

Commit

Permalink
Add regex support to VS/VSR route paths
Browse files Browse the repository at this point in the history
  • Loading branch information
Raul Marrero authored and Rulox committed Nov 22, 2019
1 parent efdd31f commit c5196a8
Show file tree
Hide file tree
Showing 5 changed files with 232 additions and 12 deletions.
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, "~*"), " "))
}
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 {
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",
},
}

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",
},
}

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)
}
}
}

0 comments on commit c5196a8

Please sign in to comment.