From ffbc0d7f4db457c0c3b85a6794d123b33db64804 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Mon, 6 Jul 2020 11:13:59 -0700 Subject: [PATCH] Support policies in VS routes and VSR subroutes --- deployments/common/vs-definition.yaml | 11 + deployments/common/vsr-definition.yaml | 11 + .../helm-chart/crds/virtualserver.yaml | 11 + .../helm-chart/crds/virtualserverroute.yaml | 11 + docs-web/configuration/policy-resource.md | 6 +- ...server-and-virtualserverroute-resources.md | 8 + docs-web/troubleshooting.md | 2 +- internal/configs/version2/http.go | 3 + .../version2/nginx-plus.virtualserver.tmpl | 18 ++ .../configs/version2/nginx.virtualserver.tmpl | 18 ++ internal/configs/version2/templates_test.go | 2 + internal/configs/virtualserver.go | 60 +++- internal/configs/virtualserver_test.go | 32 ++ internal/k8s/controller.go | 157 ++++++--- internal/k8s/controller_test.go | 299 +++++++++++++++++- pkg/apis/configuration/v1/types.go | 15 +- .../configuration/v1/zz_generated.deepcopy.go | 5 + .../configuration/validation/virtualserver.go | 25 +- .../validation/virtualserver_test.go | 14 +- 19 files changed, 614 insertions(+), 94 deletions(-) diff --git a/deployments/common/vs-definition.yaml b/deployments/common/vs-definition.yaml index 968eb787d1..0270d8e065 100644 --- 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 index ca3f36debc..2bad3f32f9 100644 --- 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 index 8672499cda..0d08aa5df4 100644 --- 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 index 9da9559f13..67db121ed4 100644 --- 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 index 75c5038bc7..2fae96774e 100644 --- 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 index 4d4834d5e6..ed349c9ba6 100644 --- 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 `_ 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 index 7f5b4f5aa4..1075876846 100644 --- 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 index 9132e353ee..f341370c1d 100644 --- 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 index 3cba134e20..316d2f1a4b 100644 --- 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 index 725e235b66..ae1ab4c04d 100644 --- 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 index fe85896fc3..735e15b43f 100644 --- 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 index 7bb25a8651..d01696deeb 100644 --- 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 index 06e0719654..ccb6ef7e51 100644 --- 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} diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 3ba023cf27..3fa421e4a1 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -2091,20 +2091,35 @@ func findVirtualServersForSecret(virtualServers []*conf_v1.VirtualServer, secret } func (lbc *LoadBalancerController) getVirtualServersForPolicy(policyNamespace string, policyName string) []*conf_v1.VirtualServer { - return findVirtualServersForPolicy(lbc.getVirtualServers(), policyNamespace, policyName) + var result []*conf_v1.VirtualServer + + allVirtualServers := lbc.getVirtualServers() + + // find VirtualServers that reference VirtualServerRoutes that reference the policy + virtualServerRoutes := findVirtualServerRoutesForPolicy(lbc.getVirtualServerRoutes(), policyNamespace, policyName) + for _, vsr := range virtualServerRoutes { + virtualServers := findVirtualServersForVirtualServerRoute(allVirtualServers, vsr) + result = append(result, virtualServers...) + } + + // find VirtualServers that reference the policy + virtualServers := findVirtualServersForPolicy(lbc.getVirtualServers(), policyNamespace, policyName) + result = append(result, virtualServers...) + + return result } func findVirtualServersForPolicy(virtualServers []*conf_v1.VirtualServer, policyNamespace string, policyName string) []*conf_v1.VirtualServer { var result []*conf_v1.VirtualServer for _, vs := range virtualServers { - for _, p := range vs.Spec.Policies { - namespace := p.Namespace - if namespace == "" { - namespace = vs.Namespace - } + if isPolicyReferenced(vs.Spec.Policies, vs.Namespace, policyNamespace, policyName) { + result = append(result, vs) + continue + } - if p.Name == policyName && namespace == policyNamespace { + for _, r := range vs.Spec.Routes { + if isPolicyReferenced(r.Policies, vs.Namespace, policyNamespace, policyName) { result = append(result, vs) break } @@ -2114,6 +2129,36 @@ func findVirtualServersForPolicy(virtualServers []*conf_v1.VirtualServer, policy return result } +func isPolicyReferenced(policies []conf_v1.PolicyReference, resourceNamespace string, policyNamespace string, policyName string) bool { + for _, p := range policies { + namespace := p.Namespace + if namespace == "" { + namespace = resourceNamespace + } + + if p.Name == policyName && namespace == policyNamespace { + return true + } + } + + return false +} + +func findVirtualServerRoutesForPolicy(virtualServerRoutes []*conf_v1.VirtualServerRoute, policyNamespace string, policyName string) []*conf_v1.VirtualServerRoute { + var result []*conf_v1.VirtualServerRoute + + for _, vsr := range virtualServerRoutes { + for _, r := range vsr.Spec.Subroutes { + if isPolicyReferenced(r.Policies, vsr.Namespace, policyNamespace, policyName) { + result = append(result, vsr) + break + } + } + } + + return result +} + func (lbc *LoadBalancerController) getVirtualServers() []*conf_v1.VirtualServer { var virtualServers []*conf_v1.VirtualServer @@ -2459,36 +2504,9 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi } } - policies := make(map[string]*conf_v1alpha1.Policy) - - for _, p := range virtualServer.Spec.Policies { - polNamespace := p.Namespace - if polNamespace == "" { - polNamespace = virtualServer.Namespace - } - - policyKey := fmt.Sprintf("%s/%s", polNamespace, p.Name) - - policyObj, exists, err := lbc.policyLister.GetByKey(policyKey) - if err != nil { - glog.Warningf("Failed to get policy %s for VirtualServer %s/%s: %v", p.Name, polNamespace, virtualServer.Name, err) - continue - } - - if !exists { - glog.Warningf("Policy %s doesn't exist for VirtualServer %s/%s", p.Name, polNamespace, virtualServer.Name) - continue - } - - policy := policyObj.(*conf_v1alpha1.Policy) - - err = validation.ValidatePolicy(policy) - if err != nil { - glog.Warningf("Policy %s is invalid for VirtualServer %s/%s: %v", policyKey, virtualServer.Namespace, virtualServer.Name, err) - continue - } - - policies[policyKey] = policy + policies, policyErrors := lbc.getPolicies(virtualServer.Spec.Policies, virtualServer.Namespace) + for _, err := range policyErrors { + glog.Warningf("Error getting policy for VirtualServer %s/%s: %v", virtualServer.Namespace, virtualServer.Name, err) } endpoints := make(map[string][]string) @@ -2521,8 +2539,13 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi var virtualServerRoutes []*conf_v1.VirtualServerRoute var virtualServerRouteErrors []virtualServerRouteError - // gather all referenced VirtualServerRoutes for _, r := range virtualServer.Spec.Routes { + vsRoutePolicies, policyErrors := lbc.getPolicies(r.Policies, virtualServer.Namespace) + for _, err := range policyErrors { + glog.Warningf("Error getting policy for VirtualServer %s/%s: %v", virtualServer.Namespace, virtualServer.Name, err) + } + policies = append(policies, vsRoutePolicies...) + if r.Route == "" { continue } @@ -2564,6 +2587,14 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi virtualServerRoutes = append(virtualServerRoutes, vsr) + for _, sr := range vsr.Spec.Subroutes { + vsrSubroutePolicies, policyErrors := lbc.getPolicies(sr.Policies, vsr.Namespace) + for _, err := range policyErrors { + glog.Warningf("Error getting policy for VirtualServerRoute %s/%s: %v", vsr.Namespace, vsr.Name, err) + } + policies = append(policies, vsrSubroutePolicies...) + } + for _, u := range vsr.Spec.Upstreams { endpointsKey := configs.GenerateEndpointsKey(vsr.Namespace, u.Service, u.Subselector, u.Port) @@ -2589,11 +2620,59 @@ func (lbc *LoadBalancerController) createVirtualServer(virtualServer *conf_v1.Vi virtualServerEx.Endpoints = endpoints virtualServerEx.VirtualServerRoutes = virtualServerRoutes virtualServerEx.ExternalNameSvcs = externalNameSvcs - virtualServerEx.Policies = policies + virtualServerEx.Policies = createPolicyMap(policies) return &virtualServerEx, virtualServerRouteErrors } +func createPolicyMap(policies []*conf_v1alpha1.Policy) map[string]*conf_v1alpha1.Policy { + result := make(map[string]*conf_v1alpha1.Policy) + + for _, p := range policies { + key := fmt.Sprintf("%s/%s", p.Namespace, p.Name) + result[key] = p + } + + return result +} + +func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReference, defaultNamespace string) ([]*conf_v1alpha1.Policy, []error) { + var result []*conf_v1alpha1.Policy + var errors []error + + for _, p := range policies { + polNamespace := p.Namespace + if polNamespace == "" { + polNamespace = defaultNamespace + } + + policyKey := fmt.Sprintf("%s/%s", polNamespace, p.Name) + + policyObj, exists, err := lbc.policyLister.GetByKey(policyKey) + if err != nil { + errors = append(errors, fmt.Errorf("Failed to get policy %s: %v", policyKey, err)) + continue + } + + if !exists { + errors = append(errors, fmt.Errorf("Policy %s doesn't exist", policyKey)) + continue + } + + policy := policyObj.(*conf_v1alpha1.Policy) + + err = validation.ValidatePolicy(policy) + if err != nil { + errors = append(errors, fmt.Errorf("Policy %s is invalid: %v", policyKey, err)) + continue + } + + result = append(result, policy) + } + + return result, errors +} + func (lbc *LoadBalancerController) createTransportServer(transportServer *conf_v1alpha1.TransportServer) *configs.TransportServerEx { endpoints := make(map[string][]string) diff --git a/internal/k8s/controller_test.go b/internal/k8s/controller_test.go index 7766295207..a6eea7217a 100644 --- a/internal/k8s/controller_test.go +++ b/internal/k8s/controller_test.go @@ -1,8 +1,11 @@ package k8s import ( + "errors" "fmt" "reflect" + "sort" + "strings" "testing" "time" "unsafe" @@ -1666,7 +1669,12 @@ func TestFindVirtualServersForPolicy(t *testing.T) { Namespace: "ns-1", }, Spec: conf_v1.VirtualServerSpec{ - Policies: nil, + Policies: []conf_v1.PolicyReference{ + { + Name: "test-policy", + Namespace: "ns-1", + }, + }, }, } vs2 := conf_v1.VirtualServer{ @@ -1677,7 +1685,8 @@ func TestFindVirtualServersForPolicy(t *testing.T) { Spec: conf_v1.VirtualServerSpec{ Policies: []conf_v1.PolicyReference{ { - Name: "some-policy", + Name: "some-policy", + Namespace: "ns-1", }, }, }, @@ -1688,10 +1697,14 @@ func TestFindVirtualServersForPolicy(t *testing.T) { Namespace: "ns-1", }, Spec: conf_v1.VirtualServerSpec{ - Policies: []conf_v1.PolicyReference{ + Routes: []conf_v1.Route{ { - Name: "test-policy", - Namespace: "some-namespace", + Policies: []conf_v1.PolicyReference{ + { + Name: "test-policy", + Namespace: "ns-1", + }, + }, }, }, }, @@ -1702,36 +1715,144 @@ func TestFindVirtualServersForPolicy(t *testing.T) { Namespace: "ns-1", }, Spec: conf_v1.VirtualServerSpec{ - Policies: []conf_v1.PolicyReference{ + Routes: []conf_v1.Route{ + { + Policies: []conf_v1.PolicyReference{ + { + Name: "some-policy", + Namespace: "ns-1", + }, + }, + }, + }, + }, + } + + virtualServers := []*conf_v1.VirtualServer{&vs1, &vs2, &vs3, &vs4} + + expected := []*conf_v1.VirtualServer{&vs1, &vs3} + + result := findVirtualServersForPolicy(virtualServers, "ns-1", "test-policy") + if !reflect.DeepEqual(result, expected) { + t.Errorf("findVirtualServersForPolicy() returned %v but expected %v", result, expected) + } +} + +func TestIsPolicyIsReferenced(t *testing.T) { + tests := []struct { + policies []conf_v1.PolicyReference + resourceNamespace string + policyNamespace string + policyName string + expected bool + msg string + }{ + { + policies: []conf_v1.PolicyReference{ + { + Name: "test-policy", + }, + }, + resourceNamespace: "ns-1", + policyNamespace: "ns-1", + policyName: "test-policy", + expected: true, + msg: "reference with implicit namespace", + }, + { + policies: []conf_v1.PolicyReference{ { Name: "test-policy", Namespace: "ns-1", }, }, + resourceNamespace: "ns-1", + policyNamespace: "ns-1", + policyName: "test-policy", + expected: true, + msg: "reference with explicit namespace", + }, + { + policies: []conf_v1.PolicyReference{ + { + Name: "test-policy", + }, + }, + resourceNamespace: "ns-2", + policyNamespace: "ns-1", + policyName: "test-policy", + expected: false, + msg: "wrong namespace with implicit namespace", + }, + { + policies: []conf_v1.PolicyReference{ + { + Name: "test-policy", + Namespace: "ns-2", + }, + }, + resourceNamespace: "ns-2", + policyNamespace: "ns-1", + policyName: "test-policy", + expected: false, + msg: "wrong namespace with explicit namespace", }, } - vs5 := conf_v1.VirtualServer{ + + for _, test := range tests { + result := isPolicyReferenced(test.policies, test.resourceNamespace, test.policyNamespace, test.policyName) + if result != test.expected { + t.Errorf("isPolicyReferenced() returned %v but expected %v for the case of %s", result, + test.expected, test.msg) + } + } +} + +func TestFindVirtualServerRoutesForPolicy(t *testing.T) { + vsr1 := conf_v1.VirtualServerRoute{ ObjectMeta: meta_v1.ObjectMeta{ - Name: "vs-5", + Name: "vsr-1", Namespace: "ns-1", }, - Spec: conf_v1.VirtualServerSpec{ - Policies: []conf_v1.PolicyReference{ + Spec: conf_v1.VirtualServerRouteSpec{ + Subroutes: []conf_v1.Route{ { - Name: "test-policy", - Namespace: "", + Policies: []conf_v1.PolicyReference{ + { + Name: "test-policy", + Namespace: "ns-1", + }, + }, + }, + }, + }, + } + vsr2 := conf_v1.VirtualServerRoute{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "vsr-2", + Namespace: "ns-1", + }, + Spec: conf_v1.VirtualServerRouteSpec{ + Subroutes: []conf_v1.Route{ + { + Policies: []conf_v1.PolicyReference{ + { + Name: "some-policy", + Namespace: "ns-1", + }, + }, }, }, }, } - virtualServers := []*conf_v1.VirtualServer{&vs1, &vs2, &vs3, &vs4, &vs5} + virtualServerRoutes := []*conf_v1.VirtualServerRoute{&vsr1, &vsr2} - expected := []*conf_v1.VirtualServer{&vs4, &vs5} + expected := []*conf_v1.VirtualServerRoute{&vsr1} - result := findVirtualServersForPolicy(virtualServers, "ns-1", "test-policy") + result := findVirtualServerRoutesForPolicy(virtualServerRoutes, "ns-1", "test-policy") if !reflect.DeepEqual(result, expected) { - t.Errorf("findVirtualServersForPolicy returned %v but expected %v", result, expected) + t.Errorf("findVirtualServerRoutesForPolicy() returned %v but expected %v", result, expected) } } @@ -1920,3 +2041,149 @@ func TestGetStatusFromEventTitle(t *testing.T) { } } } + +func TestGetPolicies(t *testing.T) { + validPolicy := &conf_v1alpha1.Policy{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "valid-policy", + Namespace: "default", + }, + Spec: conf_v1alpha1.PolicySpec{ + AccessControl: &conf_v1alpha1.AccessControl{ + Allow: []string{"127.0.0.1"}, + }, + }, + } + + invalidPolicy := &conf_v1alpha1.Policy{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "invalid-policy", + Namespace: "default", + }, + Spec: conf_v1alpha1.PolicySpec{}, + } + + lbc := LoadBalancerController{ + policyLister: &cache.FakeCustomStore{ + GetByKeyFunc: func(key string) (item interface{}, exists bool, err error) { + switch key { + case "default/valid-policy": + return validPolicy, true, nil + case "default/invalid-policy": + return invalidPolicy, true, nil + case "nginx-ingress/valid-policy": + return nil, false, nil + default: + return nil, false, errors.New("GetByKey error") + } + }, + }, + } + + policyRefs := []conf_v1.PolicyReference{ + { + Name: "valid-policy", + // Namespace is implicit here + }, + { + Name: "invalid-policy", + Namespace: "default", + }, + { + Name: "valid-policy", // doesn't exist + Namespace: "nginx-ingress", + }, + { + Name: "some-policy", // will make lister return error + Namespace: "nginx-ingress", + }, + } + + expectedPolicies := []*conf_v1alpha1.Policy{validPolicy} + expectedErrors := []error{ + errors.New("Policy default/invalid-policy is invalid: spec: Invalid value: \"\": must specify exactly one of: `accessControl`"), + errors.New("Policy nginx-ingress/valid-policy doesn't exist"), + errors.New("Failed to get policy nginx-ingress/some-policy: GetByKey error"), + } + + result, errors := lbc.getPolicies(policyRefs, "default") + if !reflect.DeepEqual(result, expectedPolicies) { + t.Errorf("lbc.getPolicies() returned %v but expected %v", result, expectedPolicies) + } + if !reflect.DeepEqual(errors, expectedErrors) { + t.Errorf("lbc.getPolicies() returned %v but expected %v", errors, expectedErrors) + } +} + +func TestCreatePolicyMap(t *testing.T) { + policies := []*conf_v1alpha1.Policy{ + { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-1", + Namespace: "default", + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-2", + Namespace: "default", + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-1", + Namespace: "default", + }, + }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-1", + Namespace: "nginx-ingress", + }, + }, + } + + expected := map[string]*conf_v1alpha1.Policy{ + "default/policy-1": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-1", + Namespace: "default", + }, + }, + "default/policy-2": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-2", + Namespace: "default", + }, + }, + "nginx-ingress/policy-1": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "policy-1", + Namespace: "nginx-ingress", + }, + }, + } + + result := createPolicyMap(policies) + if !reflect.DeepEqual(result, expected) { + t.Errorf("createPolicyMap() returned \n%s but expected \n%s", policyMapToString(result), policyMapToString(expected)) + } +} + +func policyMapToString(policies map[string]*conf_v1alpha1.Policy) string { + var keys []string + for k := range policies { + keys = append(keys, k) + } + sort.Strings(keys) + + var b strings.Builder + + b.WriteString("[ ") + for _, k := range keys { + fmt.Fprintf(&b, "%q: '%s/%s', ", k, policies[k].Namespace, policies[k].Name) + } + b.WriteString("]") + + return b.String() +} diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index d52b9d6b29..a5c6562c90 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -119,13 +119,14 @@ type SessionCookie struct { // Route defines a route. type Route struct { - Path string `json:"path"` - Route string `json:"route"` - Action *Action `json:"action"` - Splits []Split `json:"splits"` - Matches []Match `json:"matches"` - ErrorPages []ErrorPage `json:"errorPages"` - LocationSnippets string `json:"location-snippets"` + Path string `json:"path"` + Policies []PolicyReference `json:"policies"` + Route string `json:"route"` + Action *Action `json:"action"` + Splits []Split `json:"splits"` + Matches []Match `json:"matches"` + ErrorPages []ErrorPage `json:"errorPages"` + LocationSnippets string `json:"location-snippets"` } // Action defines an action. diff --git a/pkg/apis/configuration/v1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1/zz_generated.deepcopy.go index 5a178eaa9e..43f4a94ae8 100644 --- a/pkg/apis/configuration/v1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1/zz_generated.deepcopy.go @@ -372,6 +372,11 @@ func (in *ProxyResponseHeaders) DeepCopy() *ProxyResponseHeaders { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Route) DeepCopyInto(out *Route) { *out = *in + if in.Policies != nil { + in, out := &in.Policies, &out.Policies + *out = make([]PolicyReference, len(*in)) + copy(*out, *in) + } if in.Action != nil { in, out := &in.Action, &out.Action *out = new(Action) diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 7721039d3f..91af879033 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -37,7 +37,7 @@ func validateVirtualServerSpec(spec *v1.VirtualServerSpec, fieldPath *field.Path upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"), isPlus) allErrs = append(allErrs, upstreamErrs...) - allErrs = append(allErrs, validateVirtualServerRoutes(spec.Routes, fieldPath.Child("routes"), upstreamNames)...) + allErrs = append(allErrs, validateVirtualServerRoutes(spec.Routes, fieldPath.Child("routes"), upstreamNames, namespace)...) return allErrs } @@ -571,7 +571,7 @@ func validateDNS1035Label(name string, fieldPath *field.Path) field.ErrorList { return allErrs } -func validateVirtualServerRoutes(routes []v1.Route, fieldPath *field.Path, upstreamNames sets.String) field.ErrorList { +func validateVirtualServerRoutes(routes []v1.Route, fieldPath *field.Path, upstreamNames sets.String, namespace string) field.ErrorList { allErrs := field.ErrorList{} allPaths := sets.String{} @@ -580,7 +580,7 @@ func validateVirtualServerRoutes(routes []v1.Route, fieldPath *field.Path, upstr idxPath := fieldPath.Index(i) isRouteFieldForbidden := false - routeErrs := validateRoute(r, idxPath, upstreamNames, isRouteFieldForbidden) + routeErrs := validateRoute(r, idxPath, upstreamNames, isRouteFieldForbidden, namespace) if len(routeErrs) > 0 { allErrs = append(allErrs, routeErrs...) } else if allPaths.Has(r.Path) { @@ -593,10 +593,11 @@ func validateVirtualServerRoutes(routes []v1.Route, fieldPath *field.Path, upstr return allErrs } -func validateRoute(route v1.Route, fieldPath *field.Path, upstreamNames sets.String, isRouteFieldForbidden bool) field.ErrorList { +func validateRoute(route v1.Route, fieldPath *field.Path, upstreamNames sets.String, isRouteFieldForbidden bool, namespace string) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, validateRoutePath(route.Path, fieldPath.Child("path"))...) + allErrs = append(allErrs, validatePolicies(route.Policies, fieldPath.Child("policies"), namespace)...) fieldCount := 0 @@ -1391,17 +1392,19 @@ func isValidMatchValue(value string) []string { // ValidateVirtualServerRoute validates a VirtualServerRoute. func ValidateVirtualServerRoute(virtualServerRoute *v1.VirtualServerRoute, isPlus bool) error { - allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), "", "/", isPlus) + allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), "", "/", isPlus, virtualServerRoute.Namespace) return allErrs.ToAggregate() } // ValidateVirtualServerRouteForVirtualServer validates a VirtualServerRoute for a VirtualServer represented by its host and path prefix. func ValidateVirtualServerRouteForVirtualServer(virtualServerRoute *v1.VirtualServerRoute, virtualServerHost string, vsPath string, isPlus bool) error { - allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), virtualServerHost, vsPath, isPlus) + allErrs := validateVirtualServerRouteSpec(&virtualServerRoute.Spec, field.NewPath("spec"), virtualServerHost, vsPath, isPlus, + virtualServerRoute.Namespace) return allErrs.ToAggregate() } -func validateVirtualServerRouteSpec(spec *v1.VirtualServerRouteSpec, fieldPath *field.Path, virtualServerHost string, vsPath string, isPlus bool) field.ErrorList { +func validateVirtualServerRouteSpec(spec *v1.VirtualServerRouteSpec, fieldPath *field.Path, virtualServerHost string, vsPath string, isPlus bool, + namespace string) field.ErrorList { allErrs := field.ErrorList{} allErrs = append(allErrs, validateVirtualServerRouteHost(spec.Host, virtualServerHost, fieldPath.Child("host"))...) @@ -1409,7 +1412,7 @@ func validateVirtualServerRouteSpec(spec *v1.VirtualServerRouteSpec, fieldPath * upstreamErrs, upstreamNames := validateUpstreams(spec.Upstreams, fieldPath.Child("upstreams"), isPlus) allErrs = append(allErrs, upstreamErrs...) - allErrs = append(allErrs, validateVirtualServerRouteSubroutes(spec.Subroutes, fieldPath.Child("subroutes"), upstreamNames, vsPath)...) + allErrs = append(allErrs, validateVirtualServerRouteSubroutes(spec.Subroutes, fieldPath.Child("subroutes"), upstreamNames, vsPath, namespace)...) return allErrs } @@ -1431,7 +1434,7 @@ func isRegexOrExactMatch(path string) bool { return strings.HasPrefix(path, "~") || strings.HasPrefix(path, "=") } -func validateVirtualServerRouteSubroutes(routes []v1.Route, fieldPath *field.Path, upstreamNames sets.String, vsPath string) field.ErrorList { +func validateVirtualServerRouteSubroutes(routes []v1.Route, fieldPath *field.Path, upstreamNames sets.String, vsPath string, namespace string) field.ErrorList { allErrs := field.ErrorList{} allPaths := sets.String{} @@ -1446,14 +1449,14 @@ func validateVirtualServerRouteSubroutes(routes []v1.Route, fieldPath *field.Pat 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) + return validateRoute(routes[0], idxPath, upstreamNames, true, namespace) } for i, r := range routes { idxPath := fieldPath.Index(i) isRouteFieldForbidden := true - routeErrs := validateRoute(r, idxPath, upstreamNames, isRouteFieldForbidden) + routeErrs := validateRoute(r, idxPath, upstreamNames, isRouteFieldForbidden, namespace) if vsPath != "" && !strings.HasPrefix(r.Path, vsPath) && !isRegexOrExactMatch(r.Path) { msg := fmt.Sprintf("must start with '%s'", vsPath) diff --git a/pkg/apis/configuration/validation/virtualserver_test.go b/pkg/apis/configuration/validation/virtualserver_test.go index 79cd4afeb5..9c5d946cd1 100644 --- a/pkg/apis/configuration/validation/virtualserver_test.go +++ b/pkg/apis/configuration/validation/virtualserver_test.go @@ -645,7 +645,7 @@ func TestValidateVirtualServerRoutes(t *testing.T) { } for _, test := range tests { - allErrs := validateVirtualServerRoutes(test.routes, field.NewPath("routes"), test.upstreamNames) + allErrs := validateVirtualServerRoutes(test.routes, field.NewPath("routes"), test.upstreamNames, "default") if len(allErrs) > 0 { t.Errorf("validateVirtualServerRoutes() returned errors %v for valid input for the case of %s", allErrs, test.msg) } @@ -693,7 +693,7 @@ func TestValidateVirtualServerRoutesFails(t *testing.T) { } for _, test := range tests { - allErrs := validateVirtualServerRoutes(test.routes, field.NewPath("routes"), test.upstreamNames) + allErrs := validateVirtualServerRoutes(test.routes, field.NewPath("routes"), test.upstreamNames, "default") if len(allErrs) == 0 { t.Errorf("validateVirtualServerRoutes() returned no errors for the case of %s", test.msg) } @@ -786,7 +786,7 @@ func TestValidateRoute(t *testing.T) { } for _, test := range tests { - allErrs := validateRoute(test.route, field.NewPath("route"), test.upstreamNames, test.isRouteFieldForbidden) + allErrs := validateRoute(test.route, field.NewPath("route"), test.upstreamNames, test.isRouteFieldForbidden, "default") if len(allErrs) > 0 { t.Errorf("validateRoute() returned errors %v for valid input for the case of %s", allErrs, test.msg) } @@ -917,7 +917,7 @@ func TestValidateRouteFails(t *testing.T) { } for _, test := range tests { - allErrs := validateRoute(test.route, field.NewPath("route"), test.upstreamNames, test.isRouteFieldForbidden) + allErrs := validateRoute(test.route, field.NewPath("route"), test.upstreamNames, test.isRouteFieldForbidden, "default") if len(allErrs) == 0 { t.Errorf("validateRoute() returned no errors for invalid input for the case of %s", test.msg) } @@ -2017,7 +2017,8 @@ func TestValidateVirtualServerRouteSubroutes(t *testing.T) { } for _, test := range tests { - allErrs := validateVirtualServerRouteSubroutes(test.routes, field.NewPath("subroutes"), test.upstreamNames, test.pathPrefix) + allErrs := validateVirtualServerRouteSubroutes(test.routes, field.NewPath("subroutes"), test.upstreamNames, + test.pathPrefix, "default") if len(allErrs) > 0 { t.Errorf("validateVirtualServerRouteSubroutes() returned errors %v for valid input for the case of %s", allErrs, test.msg) } @@ -2082,7 +2083,8 @@ func TestValidateVirtualServerRouteSubroutesFails(t *testing.T) { } for _, test := range tests { - allErrs := validateVirtualServerRouteSubroutes(test.routes, field.NewPath("subroutes"), test.upstreamNames, test.pathPrefix) + allErrs := validateVirtualServerRouteSubroutes(test.routes, field.NewPath("subroutes"), test.upstreamNames, + test.pathPrefix, "default") if len(allErrs) == 0 { t.Errorf("validateVirtualServerRouteSubroutes() returned no errors for the case of %s", test.msg) }