nginxinc · pleshakov · Jul 16, 2020 · Jul 6, 2020
diff --git a/deployments/common/vs-definition.yaml b/deployments/common/vs-definition.yaml
@@ -398,6 +398,17 @@ spec:
                                 type: integer
                   path:
                     type: string
+                  policies:
+                    type: array
+                    items:
+                      description: PolicyReference references a policy by name and
+                        an optional namespace.
+                      type: object
+                      properties:
+                        name:
+                          type: string
+                        namespace:
+                          type: string
                   route:
                     type: string
                   splits:

diff --git a/deployments/common/vsr-definition.yaml b/deployments/common/vsr-definition.yaml
@@ -383,6 +383,17 @@ spec:
                                 type: integer
                   path:
                     type: string
+                  policies:
+                    type: array
+                    items:
+                      description: PolicyReference references a policy by name and
+                        an optional namespace.
+                      type: object
+                      properties:
+                        name:
+                          type: string
+                        namespace:
+                          type: string
                   route:
                     type: string
                   splits:

diff --git a/deployments/helm-chart/crds/virtualserver.yaml b/deployments/helm-chart/crds/virtualserver.yaml
@@ -398,6 +398,17 @@ spec:
                                 type: integer
                   path:
                     type: string
+                  policies:
+                    type: array
+                    items:
+                      description: PolicyReference references a policy by name and
+                        an optional namespace.
+                      type: object
+                      properties:
+                        name:
+                          type: string
+                        namespace:
+                          type: string
                   route:
                     type: string
                   splits:

diff --git a/deployments/helm-chart/crds/virtualserverroute.yaml b/deployments/helm-chart/crds/virtualserverroute.yaml
@@ -385,6 +385,17 @@ spec:
                                 type: integer
                   path:
                     type: string
+                  policies:
+                    type: array
+                    items:
+                      description: PolicyReference references a policy by name and
+                        an optional namespace.
+                      type: object
+                      properties:
+                        name:
+                          type: string
+                        namespace:
+                          type: string
                   route:
                     type: string
                   splits:

diff --git a/docs-web/configuration/policy-resource.md b/docs-web/configuration/policy-resource.md
@@ -1,6 +1,6 @@
 # Policy Resource
 
-The Policy resource allows you to configure features like authentication, rate-limiting, and WAF, which you can add to your [VirtualServer resources](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/). In the initial release, we are introducing support for access control based on the client IP address. 
+The Policy resource allows you to configure features like authentication, rate-limiting, and WAF, which you can add to your [VirtualServer and VirtualServerRoute resources](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/). In the initial release, we are introducing support for access control based on the client IP address. 
 
 The resource is implemented as a [Custom Resource](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/).
 
@@ -23,7 +23,7 @@ This document is the reference documentation for the Policy resource. An example
 
 ## Prerequisites
 
-Policies work together with [VirtualServer resources](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/), which you need to create separately. 
+Policies work together with [VirtualServer and VirtualServerRoute resources](/nginx-ingress-controller/configuration/virtualserver-and-virtualserverroute-resources/), which you need to create separately. 
 
 ## Policy Specification
 
@@ -94,7 +94,7 @@ accessControl:
 
 #### AccessControl Merging Behavior
 
-A VirtualServer can reference multiple access control policies. For example, here we reference two policies, each with configured allow lists:
+A VirtualServer/VirtualServerRoute can reference multiple access control policies. For example, here we reference two policies, each with configured allow lists:
 ```yaml
 policies:
 - name: allow-policy-one

diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md
@@ -217,6 +217,10 @@ The route defines rules for matching client requests to actions like passing a r
      - 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 <https://nginx.org/en/docs/http/ngx_http_core_module.html#location>`_ directive for more information.
      - ``string``
      - Yes
+   * - ``policies``
+     - A list of policies. The policies override the policies of the same type defined in the ``spec`` of the VirtualServer. The overriding is done by NGINX: the route policies are configured in the ``location`` context, which overrides the spec policies of the same type defined in the ``server`` context. 
+     - `[]policy <#virtualserver-policy>`_
+     - No
    * - ``action``
      - The default action to perform for a request.
      - `action <#action>`_
@@ -346,6 +350,10 @@ action:
      - 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
+   * - ``policies``
+     - A list of policies. The policies override *all* policies defined in the route of the VirtualServer that references this resource. This is done by the Ingress Controller: the route policies of the VirtualServer will not be present in the generated configuration. The policies also override the policies of the same type defined in the ``spec`` of the VirtualServer. This overriding is done by NGINX: the subroute policies are configured in the ``location`` context, which overrides the spec policies of the same type defined in the ``server`` context.
+     - `[]policy <#virtualserver-policy>`_
+     - No
    * - ``action``
      - The default action to perform for a request.
      - `action <#action>`_

diff --git a/docs-web/troubleshooting.md b/docs-web/troubleshooting.md
@@ -108,7 +108,7 @@ Events:
 ```
 Note that in the events section, we have a `Normal` event with the `AddedOrUpdated` reason, which informs us that the policy was successfully accepted.
 
-However, the fact that a policy was accepted doesn't guarantee that the NGINX configuration was successfully applied. To confirm that, check the events of the VirtualServer resources that reference that policy.
+However, the fact that a policy was accepted doesn't guarantee that the NGINX configuration was successfully applied. To confirm that, check the events of the VirtualServer and VirtualServerRoute resources that reference that policy.
 
 ### Checking the Events of the ConfigMap Resource
 

diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go
@@ -93,6 +93,9 @@ type Location struct {
 	ErrorPages               []ErrorPage
 	ProxySSLName             string
 	InternalProxyPass        string
+	Allow                     []string
+	Deny                      []string
+	PoliciesErrorReturn       *Return
 }
 
 // ReturnLocation defines a location for returning a fixed response.

diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl
@@ -164,6 +164,24 @@ server {
         {{ $snippet }}
         {{ end }}
 
+        {{ with $l.PoliciesErrorReturn }}
+        return {{ .Code }};
+        {{ end }}
+
+        {{ range $allow := $l.Allow }}
+        allow {{ $allow }};
+        {{ end }}
+        {{ if gt (len $l.Allow) 0 }}
+        deny all;
+        {{ end }}
+
+        {{ range $deny := $l.Deny }}
+        deny {{ $deny }};
+        {{ end }}
+        {{ if gt (len $l.Deny) 0 }}
+        allow all;
+        {{ end }}
+
         {{ range $e := $l.ErrorPages }}
         error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}";
         {{ end }}

diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl
@@ -133,6 +133,24 @@ server {
         {{ $snippet }}
         {{ end }}
 
+        {{ with $l.PoliciesErrorReturn }}
+        return {{ .Code }};
+        {{ end }}
+
+        {{ range $allow := $l.Allow }}
+        allow {{ $allow }};
+        {{ end }}
+        {{ if gt (len $l.Allow) 0 }}
+        deny all;
+        {{ end }}
+
+        {{ range $deny := $l.Deny }}
+        deny {{ $deny }};
+        {{ end }}
+        {{ if gt (len $l.Deny) 0 }}
+        allow all;
+        {{ end }}
+
         {{ range $e := $l.ErrorPages }}
         error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}";
         {{ end }}

diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go
@@ -135,6 +135,8 @@ var virtualServerCfg = VirtualServerConfig{
 			{
 				Path:                     "/",
 				Snippets:                 []string{"# location snippet"},
+				Allow:                    []string{"127.0.0.1"},
+				Deny:                     []string{"127.0.0.1"},
 				ProxyConnectTimeout:      "30s",
 				ProxyReadTimeout:         "31s",
 				ProxySendTimeout:         "32s",

diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go
@@ -257,8 +257,9 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 	var maps []version2.Map
 	var errorPageLocations []version2.ErrorPageLocation
 	var vsrErrorPagesFromVs = make(map[string][]conf_v1.ErrorPage)
-	var vsrLocationSnippetsFromVs = make(map[string]string)
 	var vsrErrorPagesRouteIndex = make(map[string]int)
+	var vsrLocationSnippetsFromVs = make(map[string]string)
+	var vsrPoliciesFromVs = make(map[string][]conf_v1.PolicyReference)
 	matchesRoutes := 0
 
 	variableNamer := newVariableNamer(virtualServerEx.VirtualServer)
@@ -270,32 +271,39 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 
 		// ignore routes that reference VirtualServerRoute
 		if r.Route != "" {
+			name := r.Route
+			if !strings.Contains(name, "/") {
+				name = fmt.Sprintf("%v/%v", virtualServerEx.VirtualServer.Namespace, r.Route)
+			}
+
 			// store route location snippet for the referenced VirtualServerRoute in case they don't define their own
 			if r.LocationSnippets != "" {
-				name := r.Route
-				if !strings.Contains(name, "/") {
-					name = fmt.Sprintf("%v/%v", virtualServerEx.VirtualServer.Namespace, r.Route)
-				}
 				vsrLocationSnippetsFromVs[name] = r.LocationSnippets
 			}
 
 			// store route error pages and route index for the referenced VirtualServerRoute in case they don't define their own
 			if len(r.ErrorPages) > 0 {
-				name := r.Route
-				if !strings.Contains(name, "/") {
-					name = fmt.Sprintf("%v/%v", virtualServerEx.VirtualServer.Namespace, r.Route)
-				}
 				vsrErrorPagesFromVs[name] = r.ErrorPages
 				vsrErrorPagesRouteIndex[name] = errorPageIndex
 			}
+
+			// store route policies for the referenced VirtualServerRoute in case they don't define their own
+			if len(r.Policies) > 0 {
+				vsrPoliciesFromVs[name] = r.Policies
+			}
+
 			continue
 		}
 
 		vsLocSnippets := r.LocationSnippets
+		routePoliciesCfg := vsc.generatePolicies(virtualServerEx.VirtualServer, virtualServerEx.VirtualServer.Namespace,
+			r.Policies, virtualServerEx.Policies)
 
 		if len(r.Matches) > 0 {
 			cfg := generateMatchesConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, matchesRoutes, len(splitClients),
 				vsc.cfgParams, r.ErrorPages, errorPageIndex, vsLocSnippets, vsc.enableSnippets, len(returnLocations))
+			addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations)
+
 			maps = append(maps, cfg.Maps...)
 			locations = append(locations, cfg.Locations...)
 			internalRedirectLocations = append(internalRedirectLocations, cfg.InternalRedirectLocation)
@@ -305,6 +313,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 		} else if len(r.Splits) > 0 {
 			cfg := generateDefaultSplitsConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, len(splitClients),
 				vsc.cfgParams, r.ErrorPages, errorPageIndex, r.Path, vsLocSnippets, vsc.enableSnippets, len(returnLocations))
+			addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations)
 
 			splitClients = append(splitClients, cfg.SplitClients...)
 			locations = append(locations, cfg.Locations...)
@@ -314,8 +323,11 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 			upstreamName := virtualServerUpstreamNamer.GetNameForUpstreamFromAction(r.Action)
 			upstream := crUpstreams[upstreamName]
 			proxySSLName := generateProxySSLName(upstream.Service, virtualServerEx.VirtualServer.Namespace)
+
 			loc, returnLoc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams, r.ErrorPages, false,
 				errorPageIndex, proxySSLName, r.Path, vsLocSnippets, vsc.enableSnippets, len(returnLocations))
+			addPoliciesCfgToLocation(routePoliciesCfg, &loc)
+
 			locations = append(locations, loc)
 			if returnLoc != nil {
 				returnLocations = append(returnLocations, *returnLoc)
@@ -331,7 +343,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 			errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPageIndex, r.ErrorPages)...)
 			errorPages := r.ErrorPages
 			vsrNamespaceName := fmt.Sprintf("%v/%v", vsr.Namespace, vsr.Name)
-			// use referenced VirtualServer error pages if the route does not define any
+			// use the VirtualServer error pages if the route does not define any
 			if r.ErrorPages == nil {
 				if vsErrorPages, ok := vsrErrorPagesFromVs[vsrNamespaceName]; ok {
 					errorPages = vsErrorPages
@@ -340,14 +352,23 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 			}
 
 			locSnippets := r.LocationSnippets
-			// use referenced VirtualServer location snippet if the route does not define any
+			// use the  VirtualServer location snippet if the route does not define any
 			if r.LocationSnippets == "" {
 				locSnippets = vsrLocationSnippetsFromVs[vsrNamespaceName]
 			}
 
+			routePoliciesCfg := vsc.generatePolicies(vsr, vsr.Namespace, r.Policies, virtualServerEx.Policies)
+			// use the VirtualServer route policies if the route does not define any
+			if len(r.Policies) == 0 {
+				routePoliciesCfg = vsc.generatePolicies(virtualServerEx.VirtualServer, virtualServerEx.VirtualServer.Namespace,
+					vsrPoliciesFromVs[vsrNamespaceName], virtualServerEx.Policies)
+			}
+
 			if len(r.Matches) > 0 {
 				cfg := generateMatchesConfig(r, upstreamNamer, crUpstreams, variableNamer, matchesRoutes, len(splitClients),
 					vsc.cfgParams, errorPages, errorPageIndex, locSnippets, vsc.enableSnippets, len(returnLocations))
+				addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations)
+
 				maps = append(maps, cfg.Maps...)
 				locations = append(locations, cfg.Locations...)
 				internalRedirectLocations = append(internalRedirectLocations, cfg.InternalRedirectLocation)
@@ -357,6 +378,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 			} else if len(r.Splits) > 0 {
 				cfg := generateDefaultSplitsConfig(r, upstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams,
 					errorPages, errorPageIndex, r.Path, locSnippets, vsc.enableSnippets, len(returnLocations))
+				addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations)
+
 				splitClients = append(splitClients, cfg.SplitClients...)
 				locations = append(locations, cfg.Locations...)
 				internalRedirectLocations = append(internalRedirectLocations, cfg.InternalRedirectLocation)
@@ -365,8 +388,11 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE
 				upstreamName := upstreamNamer.GetNameForUpstreamFromAction(r.Action)
 				upstream := crUpstreams[upstreamName]
 				proxySSLName := generateProxySSLName(upstream.Service, vsr.Namespace)
+
 				loc, returnLoc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams, errorPages, false,
 					errorPageIndex, proxySSLName, r.Path, locSnippets, vsc.enableSnippets, len(returnLocations))
+				addPoliciesCfgToLocation(routePoliciesCfg, &loc)
+
 				locations = append(locations, loc)
 				if returnLoc != nil {
 					returnLocations = append(returnLocations, *returnLoc)
@@ -459,6 +485,18 @@ func (vsc *virtualServerConfigurator) generatePolicies(owner runtime.Object, own
 	}
 }
 
+func addPoliciesCfgToLocation(cfg policiesCfg, location *version2.Location) {
+	location.Allow = cfg.Allow
+	location.Deny = cfg.Deny
+	location.PoliciesErrorReturn = cfg.ErrorReturn
+}
+
+func addPoliciesCfgToLocations(cfg policiesCfg, locations []version2.Location) {
+	for i := range locations {
+		addPoliciesCfgToLocation(cfg, &locations[i])
+	}
+}
+
 func (vsc *virtualServerConfigurator) generateUpstream(owner runtime.Object, upstreamName string, upstream conf_v1.Upstream, isExternalNameSvc bool, endpoints []string) version2.Upstream {
 	var upsServers []version2.UpstreamServer
 	for _, e := range endpoints {

diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go
@@ -1827,6 +1827,38 @@ func TestGeneratePoliciesFails(t *testing.T) {
 	}
 }
 
+func TestAddPoliciesCfgToLocations(t *testing.T) {
+	cfg := policiesCfg{
+		Allow: []string{"127.0.0.1"},
+		Deny:  []string{"127.0.0.2"},
+		ErrorReturn: &version2.Return{
+			Code: 400,
+		},
+	}
+
+	locations := []version2.Location{
+		{
+			Path: "/",
+		},
+	}
+
+	expectedLocations := []version2.Location{
+		{
+			Path:  "/",
+			Allow: []string{"127.0.0.1"},
+			Deny:  []string{"127.0.0.2"},
+			PoliciesErrorReturn: &version2.Return{
+				Code: 400,
+			},
+		},
+	}
+
+	addPoliciesCfgToLocations(cfg, locations)
+	if !reflect.DeepEqual(locations, expectedLocations) {
+		t.Errorf("addPoliciesCfgToLocations() returned \n%+v but expected \n%+v", locations, expectedLocations)
+	}
+}
+
 func TestGenerateUpstream(t *testing.T) {
 	name := "test-upstream"
 	upstream := conf_v1.Upstream{Service: name, Port: 80}