diff --git a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md index 1799b281fb..196163b0c1 100644 --- a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md @@ -301,7 +301,7 @@ action: {{% table %}} |Field | Description | Type | Required | | ---| ---| ---| --- | -|``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 | +|``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. A matching path of the route of the VirtualServer but in different type is not accepted, e.g. a regex path (`~/match`) cannot be used with a prefix path in VirtualServer (`/match`) 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 | |``policies`` | A list of policies. The policies override *all* policies defined in the route of the VirtualServer that references this resource. The policies also override the policies of the same type defined in the ``spec`` of the VirtualServer. See [Applying Policies](/nginx-ingress-controller/configuration/policy-resource/#applying-policies) for more details. | [[]policy](#virtualserverpolicy) | No | |``action`` | The default action to perform for a request. | [action](#action) | No | |``dos`` | A reference to a DosProtectedResource, setting this enables DOS protection of the VirtualServerRoute subroute. | ``string`` | No | diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 4f260c7f88..cf3fa9907c 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -1466,7 +1466,7 @@ func isValidMatchValue(value string) []string { // ValidateVirtualServerRoute validates a VirtualServerRoute. func (vsv *VirtualServerValidator) ValidateVirtualServerRoute(virtualServerRoute *v1.VirtualServerRoute) error { - allErrs := vsv.validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), "", "/", virtualServerRoute.Namespace) + allErrs := vsv.validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), "", "", virtualServerRoute.Namespace) return allErrs.ToAggregate() } @@ -1527,7 +1527,7 @@ func (vsv *VirtualServerValidator) validateVirtualServerRouteSubroutes(routes [] isRouteFieldForbidden := true routeErrs := vsv.validateRoute(r, idxPath, upstreamNames, isRouteFieldForbidden, namespace) - if vsPath != "" && !strings.HasPrefix(r.Path, vsPath) && !isRegexOrExactMatch(r.Path) { + if vsPath != "" && !strings.HasPrefix(r.Path, vsPath) { msg := fmt.Sprintf("must start with '%s'", vsPath) routeErrs = append(routeErrs, field.Invalid(idxPath, r.Path, msg)) } diff --git a/pkg/apis/configuration/validation/virtualserver_test.go b/pkg/apis/configuration/validation/virtualserver_test.go index 6e46fb59e5..1d3170829f 100644 --- a/pkg/apis/configuration/validation/virtualserver_test.go +++ b/pkg/apis/configuration/validation/virtualserver_test.go @@ -2426,13 +2426,13 @@ func TestValidateVirtualServerRouteSubroutes(t *testing.T) { tests := []struct { routes []v1.Route upstreamNames sets.Set[string] - pathPrefix string + vsPath string msg string }{ { routes: []v1.Route{}, upstreamNames: sets.Set[string]{}, - pathPrefix: "/", + vsPath: "/", msg: "no routes", }, { @@ -2447,8 +2447,74 @@ func TestValidateVirtualServerRouteSubroutes(t *testing.T) { upstreamNames: map[string]sets.Empty{ "test": {}, }, - pathPrefix: "/", - msg: "valid route", + vsPath: "/", + msg: "valid prefix route", + }, + { + routes: []v1.Route{ + { + Path: "/", + Action: &v1.Action{ + Pass: "test", + }, + }, + { + Path: "/test", + Action: &v1.Action{ + Pass: "test", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test": {}, + }, + vsPath: "/", + msg: "valid route prefix with two paths", + }, + { + routes: []v1.Route{ + { + Path: "~/test", + Action: &v1.Action{ + Pass: "test", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test": {}, + }, + vsPath: "~/test", + msg: "valid regex route", + }, + { + routes: []v1.Route{ + { + Path: "~ /regex1/?(.*)", + Action: &v1.Action{ + Pass: "test", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test": {}, + }, + vsPath: "~ /regex1/?(.*)", + msg: "valid regex route", + }, + { + routes: []v1.Route{ + { + Path: "=/test", + Action: &v1.Action{ + Pass: "test", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test": {}, + }, + vsPath: "=/test", + msg: "valid exact route", }, } @@ -2456,7 +2522,7 @@ func TestValidateVirtualServerRouteSubroutes(t *testing.T) { for _, test := range tests { allErrs := vsv.validateVirtualServerRouteSubroutes(test.routes, field.NewPath("subroutes"), test.upstreamNames, - test.pathPrefix, "default") + test.vsPath, "default") if len(allErrs) > 0 { t.Errorf("validateVirtualServerRouteSubroutes() returned errors %v for valid input for the case of %s", allErrs, test.msg) } @@ -2468,7 +2534,7 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { tests := []struct { routes []v1.Route upstreamNames sets.Set[string] - pathPrefix string + vsPath string msg string }{ { @@ -2490,8 +2556,8 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { "test-1": {}, "test-2": {}, }, - pathPrefix: "/", - msg: "duplicated paths", + vsPath: "/", + msg: "duplicated paths", }, { routes: []v1.Route{ @@ -2501,7 +2567,7 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { }, }, upstreamNames: map[string]sets.Empty{}, - pathPrefix: "", + vsPath: "", msg: "invalid route", }, { @@ -2516,8 +2582,131 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { upstreamNames: map[string]sets.Empty{ "test-1": {}, }, - pathPrefix: "/abc", - msg: "invalid prefix", + vsPath: "/abc", + msg: "invalid prefix", + }, + { + routes: []v1.Route{ + { + Path: "/abc", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + { + Path: "~^/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "/abc", + msg: "prefix vs path with both matching prefix and mismatching regex subroute path", + }, + { + routes: []v1.Route{ + { + Path: "~/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "/test", + msg: "prefix vs path with matching regex subroute path", + }, + { + routes: []v1.Route{ + { + Path: "=/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "/test", + msg: "prefix vs path with matching exact subroute path", + }, + { + routes: []v1.Route{ + { + Path: "/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "=/test", + msg: "exact vs path with prefix subroute path", + }, + { + routes: []v1.Route{ + { + Path: "=/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "~/test", + msg: "regex vs path with exact subroute path", + }, + { + routes: []v1.Route{ + { + Path: "=/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + { + Path: "/abc", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "/abc", + msg: "prefix vs path with both exact and matching prefix subroute path", + }, + { + routes: []v1.Route{ + { + Path: "~/abc", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + { + Path: "/test", + Action: &v1.Action{ + Pass: "test-1", + }, + }, + }, + upstreamNames: map[string]sets.Empty{ + "test-1": {}, + }, + vsPath: "/test", + msg: "prefix vs path with both regex and matching prefix subroute path", }, } @@ -2525,7 +2714,7 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { for _, test := range tests { allErrs := vsv.validateVirtualServerRouteSubroutes(test.routes, field.NewPath("subroutes"), test.upstreamNames, - test.pathPrefix, "default") + test.vsPath, "default") if len(allErrs) == 0 { t.Errorf("validateVirtualServerRouteSubroutes() returned no errors for the case of %s", test.msg) }