diff --git a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md index 4e2c0fe349..40473abd3d 100644 --- a/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs-web/configuration/virtualserver-and-virtualserverroute-resources.md @@ -28,6 +28,9 @@ This document is the reference documentation for the resources. To see additiona - [Split](#split) - [Match](#match) - [Condition](#condition) + - [ErrorPage](#errorpage) + - [ErrorPage.Redirect](#errorpage-redirect) + - [ErrorPage.Return](#errorpage-return) - [Using VirtualServer and VirtualServerRoute](#using-virtualserver-and-virtualserverroute) - [Validation](#validation) - [Customization via ConfigMap](#customization-via-configmap) @@ -186,6 +189,10 @@ The route defines rules for matching client requests to actions like passing a r - The name of a VirtualServerRoute resource that defines this route. If the VirtualServerRoute belongs to a different namespace than the VirtualServer, you need to include the namespace. For example, ``tea-namespace/tea``. - ``string`` - No* + * - ``errorPages`` + - The custom responses for error codes. NGINX will use those responses instead of returning the error responses from the upstream servers or the default responses generated by NGINX. A custom response can be a redirect or a canned response. For example, a redirect to another URL if an upstream server responded with a 404 status code. + - `[]errorPage <#errorpage>`_ + - No ``` \* -- a route must include exactly one of the following: `action`, `splits`, or `route`. @@ -299,6 +306,10 @@ action: - 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 + * - ``errorPages`` + - The custom responses for error codes. NGINX will use those responses instead of returning the error responses from the upstream servers or the default responses generated by NGINX. A custom response can be a redirect or a canned response. For example, a redirect to another URL if an upstream server responded with a 404 status code. + - `[]errorPage <#errorpage>`_ + - No ``` \* -- a subroute must include exactly one of the following: `action` or `splits`. @@ -930,6 +941,148 @@ The value supports two kinds of matching: **Note**: a value must not include any unescaped double quotes (`"`) and must not end with an unescaped backslash (`\`). For example, the following are invalid values: `some"value`, `somevalue\`. + +### ErrorPage + +The errorPage defines a custom response for a route for the case when either an upstream server responds with (or NGINX generates) an error status code. The custom response can be a redirect or a canned response. See the [error_page](https://nginx.org/en/docs/http/ngx_http_core_module.html#error_page) directive for more information. +```yaml +path: /coffee +errorPages: +- codes: [502, 503] + redirect: + code: 301 + url: https://nginx.org +- codes: [404] + return: + code: 200 + body: "Original resource not found, but success!" +``` + +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + - Required + * - ``codes`` + - A list of error status codes. + - ``[]int`` + - Yes + * - ``redirect`` + - The redirect action for the given status codes. + - `errorPage.Redirect <#errorpage-redirect>`_ + - No* + * - ``return`` + - The canned response action for the given status codes. + - `errorPage.Return <#errorpage-return>`_ + - No* +``` + +\* -- an errorPage must include exactly one of the following: `return` or `redirect`. + +### ErrorPage.Redirect + +The redirect defines a redirect for an errorPage. + +In the example below, NGINX responds with a redirect when a response from an upstream server has a 404 status code. + +```yaml +codes: [404] +redirect: + code: 301 + url: ${scheme}://cafe.example.com/error_${status}.html +``` + +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + - Required + * - ``code`` + - The status code of a redirect. The allowed values are: ``301``\ , ``302``\ , ``307``\ , ``308``. The default is ``301``. + - ``int`` + - No + * - ``url`` + - The URL to redirect the request to. Supported NGINX variables: ``$scheme``\ , ``$status``\ and ``$http_x_forwarded_proto``\. Variables must be inclosed in curly braces. For example: ``${scheme}``. + - ``string`` + - Yes +``` + +### ErrorPage.Return + +The return defines a canned response for an errorPage. + +In the example below, NGINX responds with a canned response when a response from an upstream server has either 401 or 403 status code. + +```yaml +codes: [401, 403] +return: + code: 200 + type: application/json + body: "{\"msg\": \"You don't have permission to do this\"}" + headers: + - name: x-debug-original-status + value: ${status} +``` + +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + - Required + * - ``code`` + - The status code of the response. The default is the status code of the original response. + - ``int`` + - No + * - ``type`` + - The MIME type of the response. The default is ``text/html``. + - ``string`` + - No + * - ``body`` + - The body of the response. Supported NGINX variable: ``$status`` \ . Variables must be inclosed in curly braces. For example: ``${status}``. + - ``string`` + - Yes + * - ``headers`` + - The custom headers of the response. + - `errorPage.Redirect.Header <#errorpage-return-header>`_ + - No +``` + +### ErrorPage.Return.Header + +The header defines an HTTP Header for a canned response in an errorPage: + +```yaml +name: x-debug-original-status +value: ${status} +``` + +```eval_rst +.. list-table:: + :header-rows: 1 + + * - Field + - Description + - Type + - Required + * - ``name`` + - The name of the header. + - ``string`` + - Yes + * - ``value`` + - The value of the header. Supported NGINX variables: ``$status`` \ . Variables must be inclosed in curly braces. For example: ``${status}``. + - ``string`` + - No +``` + ## Using VirtualServer and VirtualServerRoute You can use the usual `kubectl` commands to work with VirtualServer and VirtualServerRoute resources, similar to Ingress resources. diff --git a/internal/configs/version2/config.go b/internal/configs/version2/config.go index c0730c2c83..e78ac8f119 100644 --- a/internal/configs/version2/config.go +++ b/internal/configs/version2/config.go @@ -43,6 +43,7 @@ type Server struct { Snippets []string InternalRedirectLocations []InternalRedirectLocation Locations []Location + ErrorPageLocations []ErrorPageLocation HealthChecks []HealthCheck TLSRedirect *TLSRedirect } @@ -58,6 +59,7 @@ type SSL struct { // Location defines a location. type Location struct { Path string + Internal bool Snippets []string ProxyConnectTimeout string ProxyReadTimeout string @@ -71,9 +73,11 @@ type Location struct { ProxyNextUpstream string ProxyNextUpstreamTimeout string ProxyNextUpstreamTries int + ProxyInterceptErrors bool HasKeepalive bool DefaultType string Return *Return + ErrorPages []ErrorPage } // SplitClient defines a split_clients. @@ -89,6 +93,27 @@ type Return struct { Text string } +// ErrorPage defines an error_page of a location. +type ErrorPage struct { + Name string + Codes string + ResponseCode int +} + +// ErrorPageLocation defines a named location for an error_page directive. +type ErrorPageLocation struct { + Name string + DefaultType string + Return *Return + Headers []Header +} + +// Header defines a header to use with add_header directive. +type Header struct { + Name string + Value string +} + // HealthCheck defines a HealthCheck for an upstream in a Server. type HealthCheck struct { Name string diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 3c71687cd8..430d68eba4 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -88,8 +88,7 @@ server { {{ range $l := $s.InternalRedirectLocations }} location {{ $l.Path }} { - error_page 418 = {{ $l.Destination }}; - return 418; + rewrite ^ {{ $l.Destination }} last; } {{ end }} @@ -107,8 +106,24 @@ server { } {{ end }} + {{ range $e := $s.ErrorPageLocations }} + location {{ $e.Name }} { + {{ if $e.DefaultType }} + default_type "{{ $e.DefaultType }}"; + {{ end }} + {{ range $h := $e.Headers }} + add_header {{ $h.Name }} "{{ $h.Value }}" always; + {{ end }} + # status code is ignored here, using 0 + return 0 "{{ $e.Return.Text }}"; + } + {{ end }} + {{ range $l := $s.Locations }} location {{ $l.Path }} { + {{ if $l.Internal }} + internal; + {{ end }} {{ range $snippet := $l.Snippets }} {{ $snippet }} {{ end }} @@ -121,6 +136,10 @@ server { {{ end }} {{ if $l.ProxyPass }} + {{ range $e := $l.ErrorPages }} + error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}"; + {{ end }} + proxy_connect_timeout {{ $l.ProxyConnectTimeout }}; proxy_read_timeout {{ $l.ProxyReadTimeout }}; proxy_send_timeout {{ $l.ProxySendTimeout }}; @@ -137,7 +156,9 @@ server { {{ if $l.ProxyBufferSize }} proxy_buffer_size {{ $l.ProxyBufferSize }}; {{ end }} - + {{ if $l.ProxyInterceptErrors }} + proxy_intercept_errors on; + {{ end }} proxy_http_version 1.1; set $default_connection_header {{ if $l.HasKeepalive }}""{{ else }}close{{ end }}; diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 75638918c5..20fb178c82 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -71,13 +71,28 @@ server { {{ range $l := $s.InternalRedirectLocations }} location {{ $l.Path }} { - error_page 418 = {{ $l.Destination }}; - return 418; + rewrite ^ {{ $l.Destination }} last; + } + {{ end }} + + {{ range $e := $s.ErrorPageLocations }} + location {{ $e.Name }} { + {{ if $e.DefaultType }} + default_type "{{ $e.DefaultType }}"; + {{ end }} + {{ range $h := $e.Headers }} + add_header {{ $h.Name }} "{{ $h.Value }}" always; + {{ end }} + # status code is ignored here, using 0 + return 0 "{{ $e.Return.Text }}"; } {{ end }} {{ range $l := $s.Locations }} location {{ $l.Path }} { + {{ if $l.Internal }} + internal; + {{ end }} {{ range $snippet := $l.Snippets }} {{ $snippet }} {{ end }} @@ -90,6 +105,10 @@ server { {{ end }} {{ if $l.ProxyPass }} + {{ range $e := $l.ErrorPages }} + error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}"; + {{ end }} + proxy_connect_timeout {{ $l.ProxyConnectTimeout }}; proxy_read_timeout {{ $l.ProxyReadTimeout }}; proxy_send_timeout {{ $l.ProxySendTimeout }}; @@ -106,7 +125,9 @@ server { {{ if $l.ProxyBufferSize }} proxy_buffer_size {{ $l.ProxyBufferSize }}; {{ end }} - + {{ if $l.ProxyInterceptErrors }} + proxy_intercept_errors on; + {{ end }} proxy_http_version 1.1; set $default_connection_header {{ if $l.HasKeepalive }}""{{ else }}close{{ end }}; diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index 23ae5c4d08..ce0b2c0cec 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -139,6 +139,7 @@ var virtualServerCfg = VirtualServerConfig{ ProxyPass: "http://test-upstream", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "5s", + Internal: true, }, { Path: "@loc0", @@ -149,6 +150,19 @@ var virtualServerCfg = VirtualServerConfig{ ProxyPass: "http://coffee-v1", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "5s", + ProxyInterceptErrors: true, + ErrorPages: []ErrorPage{ + { + Name: "@error_page_1", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "@error_page_2", + Codes: "500", + ResponseCode: 0, + }, + }, }, { Path: "@loc1", @@ -181,6 +195,35 @@ var virtualServerCfg = VirtualServerConfig{ ProxyNextUpstreamTimeout: "5s", }, }, + ErrorPageLocations: []ErrorPageLocation{ + { + Name: "@vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0", + DefaultType: "application/json", + Return: &Return{ + Code: 200, + Text: "Hello World", + }, + Headers: nil, + }, + { + Name: "@vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_1", + DefaultType: "", + Return: &Return{ + Code: 200, + Text: "Hello World", + }, + Headers: []Header{ + { + Name: "Set-Cookie", + Value: "cookie1=test", + }, + { + Name: "Set-Cookie", + Value: "cookie2=test; Secure", + }, + }, + }, + }, }, } diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index d9b4ba705c..ef8f5a20e2 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -2,6 +2,7 @@ package configs import ( "fmt" + "strconv" "strings" "github.com/golang/glog" @@ -15,6 +16,7 @@ import ( ) const nginx502Server = "unix:/var/lib/nginx/nginx-502-server.sock" +const internalLocationPrefix = "internal_location_" var incompatibleLBMethodsForSlowStart = map[string]bool{ "random": true, @@ -220,20 +222,33 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE var internalRedirectLocations []version2.InternalRedirectLocation var splitClients []version2.SplitClient var maps []version2.Map - + var errorPageLocations []version2.ErrorPageLocation + var vsrErrorPagesFromVs = make(map[string][]conf_v1.ErrorPage) + var vsrErrorPagesRouteIndex = make(map[string]int) matchesRoutes := 0 variableNamer := newVariableNamer(virtualServerEx.VirtualServer) // generates config for VirtualServer routes for _, r := range virtualServerEx.VirtualServer.Spec.Routes { + errorPageIndex := len(errorPageLocations) + errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPageIndex, r.ErrorPages)...) // ignore routes that reference VirtualServerRoute if r.Route != "" { + // 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 + } continue } if len(r.Matches) > 0 { - cfg := generateMatchesConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, matchesRoutes, len(splitClients), vsc.cfgParams) + cfg := generateMatchesConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, matchesRoutes, len(splitClients), vsc.cfgParams, r.ErrorPages, errorPageIndex) maps = append(maps, cfg.Maps...) locations = append(locations, cfg.Locations...) @@ -242,7 +257,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE matchesRoutes++ } else if len(r.Splits) > 0 { - cfg := generateDefaultSplitsConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams) + cfg := generateDefaultSplitsConfig(r, virtualServerUpstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams, r.ErrorPages, errorPageIndex) splitClients = append(splitClients, cfg.SplitClients...) locations = append(locations, cfg.Locations...) @@ -250,18 +265,29 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE } else { upstreamName := virtualServerUpstreamNamer.GetNameForUpstream(r.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams) + loc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams, r.ErrorPages, false, errorPageIndex) locations = append(locations, loc) } - } // generate config for subroutes of each VirtualServerRoute for _, vsr := range virtualServerEx.VirtualServerRoutes { upstreamNamer := newUpstreamNamerForVirtualServerRoute(virtualServerEx.VirtualServer, vsr) for _, r := range vsr.Spec.Subroutes { + errorPageIndex := len(errorPageLocations) + errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPageIndex, r.ErrorPages)...) + errorPages := r.ErrorPages + // use referenced VirtualServer error pages if the route does not define any + if r.ErrorPages == nil { + vsrNamespaceName := fmt.Sprintf("%v/%v", vsr.Namespace, vsr.Name) + if vsErrorPages, ok := vsrErrorPagesFromVs[vsrNamespaceName]; ok { + errorPages = vsErrorPages + errorPageIndex = vsrErrorPagesRouteIndex[vsrNamespaceName] + } + } + if len(r.Matches) > 0 { - cfg := generateMatchesConfig(r, upstreamNamer, crUpstreams, variableNamer, matchesRoutes, len(splitClients), vsc.cfgParams) + cfg := generateMatchesConfig(r, upstreamNamer, crUpstreams, variableNamer, matchesRoutes, len(splitClients), vsc.cfgParams, errorPages, errorPageIndex) maps = append(maps, cfg.Maps...) locations = append(locations, cfg.Locations...) @@ -270,7 +296,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE matchesRoutes++ } else if len(r.Splits) > 0 { - cfg := generateDefaultSplitsConfig(r, upstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams) + cfg := generateDefaultSplitsConfig(r, upstreamNamer, crUpstreams, variableNamer, len(splitClients), vsc.cfgParams, errorPages, errorPageIndex) splitClients = append(splitClients, cfg.SplitClients...) locations = append(locations, cfg.Locations...) @@ -278,7 +304,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE } else { upstreamName := upstreamNamer.GetNameForUpstream(r.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams) + loc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams, errorPages, false, errorPageIndex) locations = append(locations, loc) } } @@ -303,6 +329,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(virtualServerE Locations: locations, HealthChecks: healthChecks, TLSRedirect: tlsRedirectConfig, + ErrorPageLocations: errorPageLocations, }, } @@ -471,6 +498,16 @@ func upstreamHasKeepalive(upstream conf_v1.Upstream, cfgParams *ConfigParams) bo return cfgParams.Keepalive != 0 } +func generateProxyPass(tlsEnabled bool, upstreamName string, internal bool) string { + proxyPass := fmt.Sprintf("%v://%v", generateProxyPassProtocol(tlsEnabled), upstreamName) + + if internal { + return fmt.Sprintf("%v$request_uri", proxyPass) + } + + return proxyPass +} + func generateProxyPassProtocol(enableTLS bool) string { if enableTLS { return "https" @@ -524,7 +561,8 @@ func generateReturnBlock(text string, code int, defaultCode int) *version2.Retur return returnBlock } -func generateLocation(path string, upstreamName string, upstream conf_v1.Upstream, action *conf_v1.Action, cfgParams *ConfigParams) version2.Location { +func generateLocation(path string, upstreamName string, upstream conf_v1.Upstream, action *conf_v1.Action, + cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, internal bool, errPageIndex int) version2.Location { if action.Redirect != nil { returnBlock := generateReturnBlock(action.Redirect.URL, action.Redirect.Code, 301) return generateLocationForReturnBlock(path, cfgParams.LocationSnippets, returnBlock, "") @@ -539,12 +577,14 @@ func generateLocation(path string, upstreamName string, upstream conf_v1.Upstrea return generateLocationForReturnBlock(path, cfgParams.LocationSnippets, returnBlock, defaultType) } - return generateLocationForProxying(path, upstreamName, upstream, cfgParams) + return generateLocationForProxying(path, upstreamName, upstream, cfgParams, errorPages, internal, errPageIndex) } -func generateLocationForProxying(path string, upstreamName string, upstream conf_v1.Upstream, cfgParams *ConfigParams) version2.Location { +func generateLocationForProxying(path string, upstreamName string, upstream conf_v1.Upstream, + cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, internal bool, errPageIndex int) version2.Location { return version2.Location{ Path: generatePath(path), + Internal: internal, Snippets: cfgParams.LocationSnippets, ProxyConnectTimeout: generateString(upstream.ProxyConnectTimeout, cfgParams.ProxyConnectTimeout), ProxyReadTimeout: generateString(upstream.ProxyReadTimeout, cfgParams.ProxyReadTimeout), @@ -554,14 +594,20 @@ func generateLocationForProxying(path string, upstreamName string, upstream conf ProxyBuffering: generateBool(upstream.ProxyBuffering, cfgParams.ProxyBuffering), ProxyBuffers: generateBuffers(upstream.ProxyBuffers, cfgParams.ProxyBuffers), ProxyBufferSize: generateString(upstream.ProxyBufferSize, cfgParams.ProxyBufferSize), - ProxyPass: fmt.Sprintf("%v://%v", generateProxyPassProtocol(upstream.TLS.Enable), upstreamName), + ProxyPass: generateProxyPass(upstream.TLS.Enable, upstreamName, internal), ProxyNextUpstream: generateString(upstream.ProxyNextUpstream, "error timeout"), ProxyNextUpstreamTimeout: generateString(upstream.ProxyNextUpstreamTimeout, "0s"), ProxyNextUpstreamTries: upstream.ProxyNextUpstreamTries, + ProxyInterceptErrors: generateProxyInterceptErrors(errorPages), HasKeepalive: upstreamHasKeepalive(upstream, cfgParams), + ErrorPages: generateErrorPages(errPageIndex, errorPages), } } +func generateProxyInterceptErrors(errorPages []conf_v1.ErrorPage) bool { + return len(errorPages) > 0 +} + func generateLocationForReturnBlock(path string, locationSnippets []string, r *version2.Return, defaultType string) version2.Location { return version2.Location{ Path: path, @@ -578,13 +624,14 @@ type routingCfg struct { InternalRedirectLocation version2.InternalRedirectLocation } -func generateSplits(splits []conf_v1.Split, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream, variableNamer *variableNamer, scIndex int, cfgParams *ConfigParams) (version2.SplitClient, []version2.Location) { +func generateSplits(splits []conf_v1.Split, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream, + variableNamer *variableNamer, scIndex int, cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, errPageIndex int) (version2.SplitClient, []version2.Location) { var distributions []version2.Distribution for i, s := range splits { d := version2.Distribution{ Weight: fmt.Sprintf("%d%%", s.Weight), - Value: fmt.Sprintf("@splits_%d_split_%d", scIndex, i), + Value: fmt.Sprintf("/%vsplits_%d_split_%d", internalLocationPrefix, scIndex, i), } distributions = append(distributions, d) } @@ -598,18 +645,19 @@ func generateSplits(splits []conf_v1.Split, upstreamNamer *upstreamNamer, crUpst var locations []version2.Location for i, s := range splits { - path := fmt.Sprintf("@splits_%d_split_%d", scIndex, i) + path := fmt.Sprintf("/%vsplits_%d_split_%d", internalLocationPrefix, scIndex, i) upstreamName := upstreamNamer.GetNameForUpstream(s.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(path, upstreamName, upstream, s.Action, cfgParams) + loc := generateLocation(path, upstreamName, upstream, s.Action, cfgParams, errorPages, true, errPageIndex) locations = append(locations, loc) } return splitClient, locations } -func generateDefaultSplitsConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream, variableNamer *variableNamer, scIndex int, cfgParams *ConfigParams) routingCfg { - sc, locs := generateSplits(route.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex, cfgParams) +func generateDefaultSplitsConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream, + variableNamer *variableNamer, scIndex int, cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, errPageIndex int) routingCfg { + sc, locs := generateSplits(route.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex, cfgParams, errorPages, errPageIndex) splitClientVarName := variableNamer.GetNameForSplitClientVariable(scIndex) @@ -626,7 +674,7 @@ func generateDefaultSplitsConfig(route conf_v1.Route, upstreamNamer *upstreamNam } func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream, - variableNamer *variableNamer, index int, scIndex int, cfgParams *ConfigParams) routingCfg { + variableNamer *variableNamer, index int, scIndex int, cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, errPageIndex int) routingCfg { // Generate maps var maps []version2.Map @@ -659,7 +707,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr source += variableNamer.GetNameForVariableForMatchesRouteMap(index, i, 0) v := fmt.Sprintf("~^%s1", strings.Repeat("0", i)) - r := fmt.Sprintf("@matches_%d_match_%d", index, i) + r := fmt.Sprintf("/%vmatches_%d_match_%d", internalLocationPrefix, index, i) if len(m.Splits) > 0 { r = variableNamer.GetNameForSplitClientVariable(scIndex + scLocalIndex) scLocalIndex++ @@ -672,7 +720,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr params = append(params, p) } - defaultResult := fmt.Sprintf("@matches_%d_default", index) + defaultResult := fmt.Sprintf("/%vmatches_%d_default", internalLocationPrefix, index) if len(route.Splits) > 0 { defaultResult = variableNamer.GetNameForSplitClientVariable(scIndex + scLocalIndex) } @@ -699,30 +747,30 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr for i, m := range route.Matches { if len(m.Splits) > 0 { - sc, locs := generateSplits(m.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex+scLocalIndex, cfgParams) + sc, locs := generateSplits(m.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex+scLocalIndex, cfgParams, errorPages, errPageIndex) scLocalIndex++ splitClients = append(splitClients, sc) locations = append(locations, locs...) } else { - path := fmt.Sprintf("@matches_%d_match_%d", index, i) + path := fmt.Sprintf("/%vmatches_%d_match_%d", internalLocationPrefix, index, i) upstreamName := upstreamNamer.GetNameForUpstream(m.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(path, upstreamName, upstream, m.Action, cfgParams) + loc := generateLocation(path, upstreamName, upstream, m.Action, cfgParams, errorPages, true, errPageIndex) locations = append(locations, loc) } } // Generate default splits or default action if len(route.Splits) > 0 { - sc, locs := generateSplits(route.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex+scLocalIndex, cfgParams) + sc, locs := generateSplits(route.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex+scLocalIndex, cfgParams, errorPages, errPageIndex) splitClients = append(splitClients, sc) locations = append(locations, locs...) } else { - path := fmt.Sprintf("@matches_%d_default", index) + path := fmt.Sprintf("/%vmatches_%d_default", internalLocationPrefix, index) upstreamName := upstreamNamer.GetNameForUpstream(route.Action.Pass) upstream := crUpstreams[upstreamName] - loc := generateLocation(path, upstreamName, upstream, route.Action, cfgParams) + loc := generateLocation(path, upstreamName, upstream, route.Action, cfgParams, errorPages, true, errPageIndex) locations = append(locations, loc) } @@ -932,3 +980,80 @@ func generateQueueForPlus(upstreamQueue *conf_v1.UpstreamQueue, defaultTimeout s Timeout: generateString(upstreamQueue.Timeout, defaultTimeout), } } + +func generateErrorPageName(errPageIndex int, index int) string { + return fmt.Sprintf("@error_page_%v_%v", errPageIndex, index) +} + +func generateErrorPageCodes(codes []int) string { + var c []string + for _, code := range codes { + c = append(c, strconv.Itoa(code)) + } + return strings.Join(c, " ") +} + +func generateErrorPages(errPageIndex int, errorPages []conf_v1.ErrorPage) []version2.ErrorPage { + var ePages []version2.ErrorPage + + for i, e := range errorPages { + var code int + var name string + + if e.Redirect != nil { + code = 301 + if e.Redirect.Code != 0 { + code = e.Redirect.Code + } + name = e.Redirect.URL + } else { + code = e.Return.Code + name = generateErrorPageName(errPageIndex, i) + } + + ep := version2.ErrorPage{ + Name: name, + Codes: generateErrorPageCodes(e.Codes), + ResponseCode: code, + } + + ePages = append(ePages, ep) + } + + return ePages +} + +func generateErrorPageLocations(errPageIndex int, errorPages []conf_v1.ErrorPage) []version2.ErrorPageLocation { + var errorPageLocations []version2.ErrorPageLocation + for i, e := range errorPages { + if e.Redirect != nil { + // Redirects are handled in the error_page of the location directly, no need for a named location. + continue + } + + var headers []version2.Header + + for _, h := range e.Return.Headers { + headers = append(headers, version2.Header{ + Name: h.Name, + Value: h.Value, + }) + } + + defaultType := "text/html" + if e.Return.Type != "" { + defaultType = e.Return.Type + } + + epl := version2.ErrorPageLocation{ + Name: generateErrorPageName(errPageIndex, i), + DefaultType: defaultType, + Return: generateReturnBlock(e.Return.Body, 0, 0), + Headers: headers, + } + + errorPageLocations = append(errorPageLocations, epl) + } + + return errorPageLocations +} diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index aa8f9a69af..3f84e06130 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -195,6 +195,11 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Subselector: map[string]string{"version": "v1"}, Port: 80, }, + { + Name: "coffee", + Service: "coffee-svc", + Port: 80, + }, }, Routes: []conf_v1.Route{ { @@ -217,6 +222,38 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Path: "/subtea", Route: "default/subtea", }, + { + Path: "/coffee-errorpage", + Action: &conf_v1.Action{ + Pass: "coffee", + }, + ErrorPages: []conf_v1.ErrorPage{ + { + Codes: []int{401, 403}, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 301, + }, + }, + }, + }, + }, + { + Path: "/coffee-errorpage-subroute", + Route: "default/subcoffee", + ErrorPages: []conf_v1.ErrorPage{ + { + Codes: []int{401, 403}, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 301, + }, + }, + }, + }, + }, }, }, }, @@ -284,6 +321,48 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, }, }, + { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "subcoffee", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerRouteSpec{ + Host: "cafe.example.com", + Upstreams: []conf_v1.Upstream{ + { + Name: "coffee", + Service: "coffee-svc", + Port: 80, + }, + }, + Subroutes: []conf_v1.Route{ + { + Path: "/coffee-errorpage-subroute", + Action: &conf_v1.Action{ + Pass: "coffee", + }, + }, + { + Path: "/coffee-errorpage-subroute-defined", + Action: &conf_v1.Action{ + Pass: "coffee", + }, + ErrorPages: []conf_v1.ErrorPage{ + { + Codes: []int{502, 503}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "text/plain", + Body: "All Good", + }, + }, + }, + }, + }, + }, + }, + }, }, } @@ -317,6 +396,15 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, Keepalive: 16, }, + { + Name: "vs_default_cafe_coffee", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.40:80", + }, + }, + Keepalive: 16, + }, { Name: "vs_default_cafe_vsr_default_coffee_coffee", Servers: []version2.UpstreamServer{ @@ -335,6 +423,15 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, Keepalive: 16, }, + { + Name: "vs_default_cafe_vsr_default_subcoffee_coffee", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.40:80", + }, + }, + Keepalive: 16, + }, }, Server: version2.Server{ ServerName: "cafe.example.com", @@ -362,6 +459,23 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ProxyNextUpstreamTries: 0, HasKeepalive: true, }, + // Order changes here because we generate first all the VS Routes and then all the VSR Subroutes (separated for loops) + { + Path: "/coffee-errorpage", + ProxyPass: "http://vs_default_cafe_coffee", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, + ProxyInterceptErrors: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "http://nginx.com", + Codes: "401 403", + ResponseCode: 301, + }, + }, + }, { Path: "/coffee", ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee", @@ -378,6 +492,48 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ProxyNextUpstreamTries: 0, HasKeepalive: true, }, + + { + Path: "/coffee-errorpage-subroute", + ProxyPass: "http://vs_default_cafe_vsr_default_subcoffee_coffee", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, + ProxyInterceptErrors: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "http://nginx.com", + Codes: "401 403", + ResponseCode: 301, + }, + }, + }, + { + Path: "/coffee-errorpage-subroute-defined", + ProxyPass: "http://vs_default_cafe_vsr_default_subcoffee_coffee", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: true, + ProxyInterceptErrors: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "502 503", + ResponseCode: 200, + }, + }, + }, + }, + ErrorPageLocations: []version2.ErrorPageLocation{ + { + Name: "@error_page_0_0", + DefaultType: "text/plain", + Return: &version2.Return{ + Text: "All Good", + }, + }, }, }, } @@ -388,7 +544,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, tlsPemFileName) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -543,11 +699,11 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "90%", - Value: "@splits_0_split_0", + Value: "/internal_location_splits_0_split_0", }, { Weight: "10%", - Value: "@splits_0_split_1", + Value: "/internal_location_splits_0_split_1", }, }, }, @@ -557,11 +713,11 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "40%", - Value: "@splits_1_split_0", + Value: "/internal_location_splits_1_split_0", }, { Weight: "60%", - Value: "@splits_1_split_1", + Value: "/internal_location_splits_1_split_1", }, }, }, @@ -581,32 +737,36 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@splits_0_split_0", - ProxyPass: "http://vs_default_cafe_tea-v1", + Path: "/internal_location_splits_0_split_0", + ProxyPass: "http://vs_default_cafe_tea-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@splits_0_split_1", - ProxyPass: "http://vs_default_cafe_tea-v2", + Path: "/internal_location_splits_0_split_1", + ProxyPass: "http://vs_default_cafe_tea-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@splits_1_split_0", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1", + Path: "/internal_location_splits_1_split_0", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@splits_1_split_1", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2", + Path: "/internal_location_splits_1_split_1", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, }, }, @@ -618,7 +778,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, tlsPemFileName) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -792,11 +952,11 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Parameters: []version2.Parameter{ { Value: "~^1", - Result: "@matches_0_match_0", + Result: "/internal_location_matches_0_match_0", }, { Value: "default", - Result: "@matches_0_default", + Result: "/internal_location_matches_0_default", }, }, }, @@ -820,11 +980,11 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Parameters: []version2.Parameter{ { Value: "~^1", - Result: "@matches_1_match_0", + Result: "/internal_location_matches_1_match_0", }, { Value: "default", - Result: "@matches_1_default", + Result: "/internal_location_matches_1_default", }, }, }, @@ -844,32 +1004,36 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@matches_0_match_0", - ProxyPass: "http://vs_default_cafe_tea-v2", + Path: "/internal_location_matches_0_match_0", + ProxyPass: "http://vs_default_cafe_tea-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@matches_0_default", - ProxyPass: "http://vs_default_cafe_tea-v1", + Path: "/internal_location_matches_0_default", + ProxyPass: "http://vs_default_cafe_tea-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@matches_1_match_0", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2", + Path: "/internal_location_matches_1_match_0", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@matches_1_default", - ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1", + Path: "/internal_location_matches_1_default", + ProxyPass: "http://vs_default_cafe_vsr_default_coffee_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, }, }, @@ -881,7 +1045,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, tlsPemFileName) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%v but expected \n%v", result, expected) + t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -1027,6 +1191,47 @@ func TestGenerateUpstreamForExternalNameService(t *testing.T) { } } +func TestGenerateProxyPass(t *testing.T) { + tests := []struct { + tlsEnabled bool + upstreamName string + internal bool + expected string + }{ + { + tlsEnabled: false, + upstreamName: "test-upstream", + internal: false, + expected: "http://test-upstream", + }, + { + tlsEnabled: true, + upstreamName: "test-upstream", + internal: false, + expected: "https://test-upstream", + }, + { + tlsEnabled: false, + upstreamName: "test-upstream", + internal: true, + expected: "http://test-upstream$request_uri", + }, + { + tlsEnabled: true, + upstreamName: "test-upstream", + internal: true, + expected: "https://test-upstream$request_uri", + }, + } + + for _, test := range tests { + result := generateProxyPass(test.tlsEnabled, test.upstreamName, test.internal) + if result != test.expected { + t.Errorf("generateProxyPass(%v, %v, %v) returned %v but expected %v", test.tlsEnabled, test.upstreamName, test.internal, result, test.expected) + } + } +} + func TestGenerateProxyPassProtocol(t *testing.T) { tests := []struct { upstream conf_v1.Upstream @@ -1132,7 +1337,7 @@ func TestGenerateLocationForProxying(t *testing.T) { ProxyNextUpstreamTries: 0, } - result := generateLocationForProxying(path, upstreamName, conf_v1.Upstream{}, &cfgParams) + result := generateLocationForProxying(path, upstreamName, conf_v1.Upstream{}, &cfgParams, nil, false, 0) if !reflect.DeepEqual(result, expected) { t.Errorf("generateLocationForProxying() returned %v but expected %v", result, expected) } @@ -1574,6 +1779,35 @@ func TestGenerateSplits(t *testing.T) { scIndex := 1 cfgParams := ConfigParams{} crUpstreams := make(map[string]conf_v1.Upstream) + errorPages := []conf_v1.ErrorPage{ + { + Codes: []int{400, 500}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: `{\"message\": \"ok\"}`, + }, + Headers: []conf_v1.Header{ + { + Name: "Set-Cookie", + Value: "cookie1=value", + }, + }, + }, + Redirect: nil, + }, + { + Codes: []int{500, 502}, + Return: nil, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 301, + }, + }, + }, + } expectedSplitClient := version2.SplitClient{ Source: "$request_id", @@ -1581,38 +1815,67 @@ func TestGenerateSplits(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "90%", - Value: "@splits_1_split_0", + Value: "/internal_location_splits_1_split_0", }, { Weight: "10%", - Value: "@splits_1_split_1", + Value: "/internal_location_splits_1_split_1", }, }, } expectedLocations := []version2.Location{ { - Path: "@splits_1_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_splits_1_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + ProxyInterceptErrors: true, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, }, { - Path: "@splits_1_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "/internal_location_splits_1_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + ProxyInterceptErrors: true, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, }, } - resultSplitClient, resultLocations := generateSplits(splits, upstreamNamer, crUpstreams, variableNamer, scIndex, &cfgParams) + resultSplitClient, resultLocations := generateSplits(splits, upstreamNamer, crUpstreams, variableNamer, scIndex, &cfgParams, errorPages, 0) if !reflect.DeepEqual(resultSplitClient, expectedSplitClient) { - t.Errorf("generateSplits() returned %v but expected %v", resultSplitClient, expectedSplitClient) + t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultSplitClient, expectedSplitClient) } if !reflect.DeepEqual(resultLocations, expectedLocations) { - t.Errorf("generateSplits() returned %v but expected %v", resultLocations, expectedLocations) + t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultLocations, expectedLocations) } + } func TestGenerateDefaultSplitsConfig(t *testing.T) { @@ -1651,29 +1914,31 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "90%", - Value: "@splits_1_split_0", + Value: "/internal_location_splits_1_split_0", }, { Weight: "10%", - Value: "@splits_1_split_1", + Value: "/internal_location_splits_1_split_1", }, }, }, }, Locations: []version2.Location{ { - Path: "@splits_1_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_splits_1_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, { - Path: "@splits_1_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "/internal_location_splits_1_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -1684,9 +1949,9 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { cfgParams := ConfigParams{} - result := generateDefaultSplitsConfig(route, upstreamNamer, map[string]conf_v1.Upstream{}, variableNamer, index, &cfgParams) + result := generateDefaultSplitsConfig(route, upstreamNamer, map[string]conf_v1.Upstream{}, variableNamer, index, &cfgParams, route.ErrorPages, 0) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateDefaultSplitsConfig() returned %v but expected %v", result, expected) + t.Errorf("generateDefaultSplitsConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -1762,6 +2027,35 @@ func TestGenerateMatchesConfig(t *testing.T) { Namespace: "default", }, } + errorPages := []conf_v1.ErrorPage{ + { + Codes: []int{400, 500}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: `{\"message\": \"ok\"}`, + }, + Headers: []conf_v1.Header{ + { + Name: "Set-Cookie", + Value: "cookie1=value", + }, + }, + }, + Redirect: nil, + }, + { + Codes: []int{500, 502}, + Return: nil, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 301, + }, + }, + }, + } upstreamNamer := newUpstreamNamerForVirtualServer(&virtualServer) variableNamer := newVariableNamer(&virtualServer) index := 1 @@ -1887,7 +2181,7 @@ func TestGenerateMatchesConfig(t *testing.T) { Parameters: []version2.Parameter{ { Value: "~^1", - Result: "@matches_1_match_0", + Result: "/internal_location_matches_1_match_0", }, { Value: "~^01", @@ -1895,39 +2189,95 @@ func TestGenerateMatchesConfig(t *testing.T) { }, { Value: "default", - Result: "@matches_1_default", + Result: "/internal_location_matches_1_default", }, }, }, }, Locations: []version2.Location{ { - Path: "@matches_1_match_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_matches_1_match_0", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + ProxyInterceptErrors: true, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_2_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, }, { - Path: "@splits_2_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_splits_2_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + ProxyInterceptErrors: true, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_2_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, }, { - Path: "@splits_2_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "/internal_location_splits_2_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + ProxyInterceptErrors: true, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_2_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, }, { - Path: "@matches_1_default", - ProxyPass: "http://vs_default_cafe_tea", + Path: "/internal_location_matches_1_default", + ProxyPass: "http://vs_default_cafe_tea$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + ProxyInterceptErrors: true, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_2_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -1941,11 +2291,11 @@ func TestGenerateMatchesConfig(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "90%", - Value: "@splits_2_split_0", + Value: "/internal_location_splits_2_split_0", }, { Weight: "10%", - Value: "@splits_2_split_1", + Value: "/internal_location_splits_2_split_1", }, }, }, @@ -1954,9 +2304,9 @@ func TestGenerateMatchesConfig(t *testing.T) { cfgParams := ConfigParams{} - result := generateMatchesConfig(route, upstreamNamer, map[string]conf_v1.Upstream{}, variableNamer, index, scIndex, &cfgParams) + result := generateMatchesConfig(route, upstreamNamer, map[string]conf_v1.Upstream{}, variableNamer, index, scIndex, &cfgParams, errorPages, 2) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateMatchesConfig() returned \n%v but expected \n%v", result, expected) + t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -2034,6 +2384,35 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { variableNamer := newVariableNamer(&virtualServer) index := 1 scIndex := 2 + errorPages := []conf_v1.ErrorPage{ + { + Codes: []int{400, 500}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: `{\"message\": \"ok\"}`, + }, + Headers: []conf_v1.Header{ + { + Name: "Set-Cookie", + Value: "cookie1=value", + }, + }, + }, + Redirect: nil, + }, + { + Codes: []int{500, 502}, + Return: nil, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 301, + }, + }, + }, + } expected := routingCfg{ Maps: []version2.Map{ @@ -2086,46 +2465,130 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { }, Locations: []version2.Location{ { - Path: "@splits_2_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_splits_2_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, + ProxyInterceptErrors: true, }, { - Path: "@splits_2_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "/internal_location_splits_2_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, + ProxyInterceptErrors: true, }, { - Path: "@splits_3_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "/internal_location_splits_3_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, + ProxyInterceptErrors: true, }, { - Path: "@splits_3_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_splits_3_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, + ProxyInterceptErrors: true, }, { - Path: "@splits_4_split_0", - ProxyPass: "http://vs_default_cafe_coffee-v1", + Path: "/internal_location_splits_4_split_0", + ProxyPass: "http://vs_default_cafe_coffee-v1$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, + ProxyInterceptErrors: true, }, { - Path: "@splits_4_split_1", - ProxyPass: "http://vs_default_cafe_coffee-v2", + Path: "/internal_location_splits_4_split_1", + ProxyPass: "http://vs_default_cafe_coffee-v2$request_uri", ProxyNextUpstream: "error timeout", ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, + Internal: true, + ErrorPages: []version2.ErrorPage{ + { + Name: "@error_page_0_0", + Codes: "400 500", + ResponseCode: 200, + }, + { + Name: "http://nginx.com", + Codes: "500 502", + ResponseCode: 301, + }, + }, + ProxyInterceptErrors: true, }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -2139,11 +2602,11 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "30%", - Value: "@splits_2_split_0", + Value: "/internal_location_splits_2_split_0", }, { Weight: "70%", - Value: "@splits_2_split_1", + Value: "/internal_location_splits_2_split_1", }, }, }, @@ -2153,11 +2616,11 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "90%", - Value: "@splits_3_split_0", + Value: "/internal_location_splits_3_split_0", }, { Weight: "10%", - Value: "@splits_3_split_1", + Value: "/internal_location_splits_3_split_1", }, }, }, @@ -2167,11 +2630,11 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { Distributions: []version2.Distribution{ { Weight: "99%", - Value: "@splits_4_split_0", + Value: "/internal_location_splits_4_split_0", }, { Weight: "1%", - Value: "@splits_4_split_1", + Value: "/internal_location_splits_4_split_1", }, }, }, @@ -2180,9 +2643,9 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { cfgParams := ConfigParams{} - result := generateMatchesConfig(route, upstreamNamer, map[string]conf_v1.Upstream{}, variableNamer, index, scIndex, &cfgParams) + result := generateMatchesConfig(route, upstreamNamer, map[string]conf_v1.Upstream{}, variableNamer, index, scIndex, &cfgParams, errorPages, 0) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateMatchesConfig() returned \n%v but expected \n%v", result, expected) + t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -2977,3 +3440,189 @@ func TestGeneratePath(t *testing.T) { } } } + +func TestGenerateErrorPageName(t *testing.T) { + tests := []struct { + routeIndex int + index int + expected string + }{ + { + 0, + 0, + "@error_page_0_0", + }, + { + 0, + 1, + "@error_page_0_1", + }, + { + 1, + 0, + "@error_page_1_0", + }, + } + + for _, test := range tests { + result := generateErrorPageName(test.routeIndex, test.index) + if result != test.expected { + t.Errorf("generateErrorPageName(%v, %v) returned %v but expected %v", test.routeIndex, test.index, result, test.expected) + } + } +} + +func TestGenerateErrorPageCodes(t *testing.T) { + tests := []struct { + codes []int + expected string + }{ + { + codes: []int{400}, + expected: "400", + }, + { + codes: []int{404, 405, 502}, + expected: "404 405 502", + }, + } + + for _, test := range tests { + result := generateErrorPageCodes(test.codes) + if result != test.expected { + t.Errorf("generateErrorPageCodes(%v) returned %v but expected %v", test.codes, result, test.expected) + } + } +} + +func TestGenerateErrorPages(t *testing.T) { + tests := []struct { + upstreamName string + errorPages []conf_v1.ErrorPage + expected []version2.ErrorPage + }{ + {}, // empty errorPages + { + "vs_test_test", + []conf_v1.ErrorPage{ + { + Codes: []int{404, 405, 500, 502}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + }, + Headers: nil, + }, + Redirect: nil, + }, + }, + []version2.ErrorPage{ + { + Name: "@error_page_1_0", + Codes: "404 405 500 502", + ResponseCode: 200, + }, + }, + }, + { + "vs_test_test", + []conf_v1.ErrorPage{ + { + Codes: []int{404, 405, 500, 502}, + Return: nil, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.org", + Code: 302, + }, + }, + }, + }, + []version2.ErrorPage{ + { + Name: "http://nginx.org", + Codes: "404 405 500 502", + ResponseCode: 302, + }, + }, + }, + } + + for i, test := range tests { + result := generateErrorPages(i, test.errorPages) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("generateErrorPages(%v, %v) returned %v but expected %v", test.upstreamName, test.errorPages, result, test.expected) + } + } +} + +func TestGenerateErrorPageLocations(t *testing.T) { + tests := []struct { + upstreamName string + errorPages []conf_v1.ErrorPage + expected []version2.ErrorPageLocation + }{ + {}, + { + "vs_test_test", + []conf_v1.ErrorPage{ + { + Codes: []int{404, 405, 500, 502}, + Return: nil, + Redirect: &conf_v1.ErrorPageRedirect{ + ActionRedirect: conf_v1.ActionRedirect{ + URL: "http://nginx.org", + Code: 302, + }, + }, + }, + }, + nil, + }, + { + "vs_test_test", + []conf_v1.ErrorPage{ + { + Codes: []int{404, 405, 500, 502}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: "Hello World", + }, + Headers: []conf_v1.Header{ + { + Name: "HeaderName", + Value: "HeaderValue", + }, + }, + }, + Redirect: nil, + }, + }, + []version2.ErrorPageLocation{ + { + Name: "@error_page_2_0", + DefaultType: "application/json", + Return: &version2.Return{ + Code: 0, + Text: "Hello World", + }, + Headers: []version2.Header{ + { + Name: "HeaderName", + Value: "HeaderValue", + }, + }, + }, + }, + }, + } + + for i, test := range tests { + result := generateErrorPageLocations(i, test.errorPages) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("generateErrorPageLocations(%v, %v) returned %v but expected %v", test.upstreamName, test.errorPages, result, test.expected) + } + } +} diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 70901f5c2d..85f419dd15 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -98,11 +98,12 @@ 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"` + Path string `json:"path"` + Route string `json:"route"` + Action *Action `json:"action"` + Splits []Split `json:"splits"` + Matches []Match `json:"matches"` + ErrorPages []ErrorPage `json:"errorPages"` } // Action defines an action. @@ -147,6 +148,24 @@ type Match struct { Splits []Split `json:"splits"` } +// ErrorPage defines an ErrorPage in a Route. +type ErrorPage struct { + Codes []int `json:"codes"` + Return *ErrorPageReturn `json:"return"` + Redirect *ErrorPageRedirect `json:"redirect"` +} + +// ErrorPageReturn defines a return for an ErrorPage. +type ErrorPageReturn struct { + ActionReturn + Headers []Header `json:"headers"` +} + +// ErrorPageRedirect defines a redirect for an ErrorPage. +type ErrorPageRedirect struct { + ActionRedirect +} + // TLS defines TLS configuration for a VirtualServer. type TLS struct { Secret string `json:"secret"` diff --git a/pkg/apis/configuration/v1/zz_generated.deepcopy.go b/pkg/apis/configuration/v1/zz_generated.deepcopy.go index 62566f44c2..449d133bd2 100644 --- a/pkg/apis/configuration/v1/zz_generated.deepcopy.go +++ b/pkg/apis/configuration/v1/zz_generated.deepcopy.go @@ -82,6 +82,76 @@ func (in *Condition) DeepCopy() *Condition { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ErrorPage) DeepCopyInto(out *ErrorPage) { + *out = *in + if in.Codes != nil { + in, out := &in.Codes, &out.Codes + *out = make([]int, len(*in)) + copy(*out, *in) + } + if in.Return != nil { + in, out := &in.Return, &out.Return + *out = new(ErrorPageReturn) + (*in).DeepCopyInto(*out) + } + if in.Redirect != nil { + in, out := &in.Redirect, &out.Redirect + *out = new(ErrorPageRedirect) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ErrorPage. +func (in *ErrorPage) DeepCopy() *ErrorPage { + if in == nil { + return nil + } + out := new(ErrorPage) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ErrorPageRedirect) DeepCopyInto(out *ErrorPageRedirect) { + *out = *in + out.ActionRedirect = in.ActionRedirect + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ErrorPageRedirect. +func (in *ErrorPageRedirect) DeepCopy() *ErrorPageRedirect { + if in == nil { + return nil + } + out := new(ErrorPageRedirect) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ErrorPageReturn) DeepCopyInto(out *ErrorPageReturn) { + *out = *in + out.ActionReturn = in.ActionReturn + if in.Headers != nil { + in, out := &in.Headers, &out.Headers + *out = make([]Header, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ErrorPageReturn. +func (in *ErrorPageReturn) DeepCopy() *ErrorPageReturn { + if in == nil { + return nil + } + out := new(ErrorPageReturn) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Header) DeepCopyInto(out *Header) { *out = *in @@ -179,6 +249,13 @@ func (in *Route) DeepCopyInto(out *Route) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.ErrorPages != nil { + in, out := &in.ErrorPages, &out.ErrorPages + *out = make([]ErrorPage, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } return } diff --git a/pkg/apis/configuration/validation/validation.go b/pkg/apis/configuration/validation/validation.go index b63a6fa2da..69690e18df 100644 --- a/pkg/apis/configuration/validation/validation.go +++ b/pkg/apis/configuration/validation/validation.go @@ -578,6 +578,10 @@ func validateRoute(route v1.Route, fieldPath *field.Path, upstreamNames sets.Str } } + for i, e := range route.ErrorPages { + allErrs = append(allErrs, validateErrorPage(e, fieldPath.Child("errorPages").Index(i))...) + } + if route.Route != "" { if isRouteFieldForbidden { allErrs = append(allErrs, field.Forbidden(fieldPath.Child("route"), "is not allowed")) @@ -599,6 +603,95 @@ func validateRoute(route v1.Route, fieldPath *field.Path, upstreamNames sets.Str return allErrs } +func errorPageHasRequiredFields(errorPage v1.ErrorPage) bool { + var count int + + if errorPage.Return != nil { + count++ + } + + if errorPage.Redirect != nil { + count++ + } + + return count == 1 +} + +func validateErrorPage(errorPage v1.ErrorPage, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if !errorPageHasRequiredFields(errorPage) { + return append(allErrs, field.Required(fieldPath, "must specify exactly one of `redirect` or `return`")) + } + + if len(errorPage.Codes) == 0 { + return append(allErrs, field.Required(fieldPath.Child("codes"), "must include at least 1 status code in `codes`")) + } + + for i, c := range errorPage.Codes { + for _, msg := range validation.IsInRange(c, 300, 599) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("codes").Index(i), c, msg)) + } + } + + if errorPage.Return != nil { + allErrs = append(allErrs, validateErrorPageReturn(errorPage.Return, fieldPath.Child("return"))...) + } + + if errorPage.Redirect != nil { + allErrs = append(allErrs, validateErrorPageRedirect(errorPage.Redirect, fieldPath.Child("redirect"))...) + } + + return allErrs +} + +var errorPageReturnBodyVariable = map[string]bool{"status": true} + +func validateErrorPageReturn(r *v1.ErrorPageReturn, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + allErrs = append(allErrs, validateActionReturn(&r.ActionReturn, fieldPath, nil, errorPageReturnBodyVariable)...) + + for i, header := range r.Headers { + allErrs = append(allErrs, validateErrorPageHeader(header, fieldPath.Child("headers").Index(i))...) + } + + return allErrs +} + +var errorPageHeaderValueVariables = map[string]bool{"status": true} + +func validateErrorPageHeader(h v1.Header, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + if h.Name == "" { + allErrs = append(allErrs, field.Required(fieldPath.Child("name"), "")) + } + + for _, msg := range validation.IsHTTPHeaderName(h.Name) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("name"), h.Name, msg)) + } + + if !escapedStringsFmtRegexp.MatchString(h.Value) { + msg := validation.RegexError(escapedStringsErrMsg, escapedStringsFmt, "value", `\"${status}\"`) + allErrs = append(allErrs, field.Invalid(fieldPath.Child("value"), h.Value, msg)) + } + + allErrs = append(allErrs, validateStringWithVariables(h.Value, fieldPath.Child("value"), nil, errorPageHeaderValueVariables)...) + + return allErrs +} + +var validErrorPageRedirectVariables = map[string]bool{"scheme": true, "status": true, "http_x_forwarded_proto": true} + +func validateErrorPageRedirect(r *v1.ErrorPageRedirect, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + allErrs = append(allErrs, validateActionRedirect(&r.ActionRedirect, fieldPath, validErrorPageRedirectVariables)...) + + return allErrs +} + func countActions(action *v1.Action) int { var count int if action.Pass != "" { @@ -616,6 +709,43 @@ func countActions(action *v1.Action) int { return count } +// returnBodyVariables includes NGINX variables allowed to be used in a return body. +var returnBodyVariables = map[string]bool{ + "request_uri": true, + "request_method": true, + "request_body": true, + "scheme": true, + "args": true, + "host": true, + "request_time": true, + "request_length": true, + "nginx_version": true, + "pid": true, + "connection": true, + "remote_addr": true, + "remote_port": true, + "time_iso8601": true, + "time_local": true, + "server_addr": true, + "server_port": true, + "server_name": true, + "server_protocol": true, + "connections_active": true, + "connections_reading": true, + "connections_writing": true, + "connections_waiting": true, +} + +var returnBodySpecialVariables = []string{"arg_", "http_", "cookie_"} + +// validRedirectVariableNames includes NGINX variables allowed to be used in redirects. +var validRedirectVariableNames = map[string]bool{ + "scheme": true, + "http_x_forwarded_proto": true, + "request_uri": true, + "host": true, +} + func validateAction(action *v1.Action, fieldPath *field.Path, upstreamNames sets.String) field.ErrorList { allErrs := field.ErrorList{} @@ -628,20 +758,20 @@ func validateAction(action *v1.Action, fieldPath *field.Path, upstreamNames sets } if action.Redirect != nil { - allErrs = append(allErrs, validateActionRedirect(action.Redirect, fieldPath.Child("redirect"))...) + allErrs = append(allErrs, validateActionRedirect(action.Redirect, fieldPath.Child("redirect"), validRedirectVariableNames)...) } if action.Return != nil { - allErrs = append(allErrs, validateActionReturn(action.Return, fieldPath.Child("return"))...) + allErrs = append(allErrs, validateActionReturn(action.Return, fieldPath.Child("return"), returnBodySpecialVariables, returnBodyVariables)...) } return allErrs } -func validateActionRedirect(redirect *v1.ActionRedirect, fieldPath *field.Path) field.ErrorList { +func validateActionRedirect(redirect *v1.ActionRedirect, fieldPath *field.Path, validVars map[string]bool) field.ErrorList { allErrs := field.ErrorList{} - allErrs = append(allErrs, validateRedirectURL(redirect.URL, fieldPath.Child("url"))...) + allErrs = append(allErrs, validateRedirectURL(redirect.URL, fieldPath.Child("url"), validVars)...) if redirect.Code != 0 { allErrs = append(allErrs, validateRedirectStatusCode(redirect.Code, fieldPath.Child("code"))...) @@ -664,15 +794,7 @@ func captureVariables(s string) []string { return nVars } -// validRedirectVariableNames includes NGINX variables allowed to be used in redirects. -var validRedirectVariableNames = map[string]bool{ - "scheme": true, - "http_x_forwarded_proto": true, - "request_uri": true, - "host": true, -} - -func validateRedirectURL(redirectURL string, fieldPath *field.Path) field.ErrorList { +func validateRedirectURL(redirectURL string, fieldPath *field.Path, validVars map[string]bool) field.ErrorList { allErrs := field.ErrorList{} if redirectURL == "" { @@ -684,12 +806,12 @@ func validateRedirectURL(redirectURL string, fieldPath *field.Path) field.ErrorL return append(allErrs, field.Invalid(fieldPath, redirectURL, msg)) } - allErrs = append(allErrs, validateStringWithVariables(redirectURL, fieldPath, validRedirectVariableNames, nil)...) + allErrs = append(allErrs, validateStringWithVariables(redirectURL, fieldPath, nil, validVars)...) return allErrs } -func validateStringWithVariables(str string, fieldPath *field.Path, validVars map[string]bool, specialVars []string) field.ErrorList { +func validateStringWithVariables(str string, fieldPath *field.Path, specialVars []string, validVars map[string]bool) field.ErrorList { allErrs := field.ErrorList{} if strings.HasSuffix(str, "$") { @@ -782,14 +904,14 @@ func validateActionReturnCode(code int, fieldPath *field.Path) field.ErrorList { return append(allErrs, field.Invalid(fieldPath, code, msg)) } -func validateActionReturn(r *v1.ActionReturn, fieldPath *field.Path) field.ErrorList { +func validateActionReturn(r *v1.ActionReturn, fieldPath *field.Path, specialValidVars []string, validVars map[string]bool) field.ErrorList { allErrs := field.ErrorList{} if r.Body == "" { return append(allErrs, field.Required(fieldPath.Child("body"), "")) } - allErrs = append(allErrs, validateActionReturnBody(r.Body, fieldPath.Child("body"))...) + allErrs = append(allErrs, validateActionReturnBody(r.Body, fieldPath.Child("body"), specialValidVars, validVars)...) if r.Type != "" { allErrs = append(allErrs, validateActionReturnType(r.Type, fieldPath.Child("type"))...) @@ -802,36 +924,7 @@ func validateActionReturn(r *v1.ActionReturn, fieldPath *field.Path) field.Error return allErrs } -// returnBodyVariables includes NGINX variables allowed to be used in a return body. -var returnBodyVariables = map[string]bool{ - "request_uri": true, - "request_method": true, - "request_body": true, - "scheme": true, - "args": true, - "host": true, - "request_time": true, - "request_length": true, - "nginx_version": true, - "pid": true, - "connection": true, - "remote_addr": true, - "remote_port": true, - "time_iso8601": true, - "time_local": true, - "server_addr": true, - "server_port": true, - "server_name": true, - "server_protocol": true, - "connections_active": true, - "connections_reading": true, - "connections_writing": true, - "connections_waiting": true, -} - -var returnBodySpecialVariables = []string{"arg_", "http_", "cookie_"} - -func validateActionReturnBody(body string, fieldPath *field.Path) field.ErrorList { +func validateActionReturnBody(body string, fieldPath *field.Path, specialValidVars []string, validVars map[string]bool) field.ErrorList { allErrs := field.ErrorList{} if !escapedStringsFmtRegexp.MatchString(body) { @@ -839,7 +932,7 @@ func validateActionReturnBody(body string, fieldPath *field.Path) field.ErrorLis allErrs = append(allErrs, field.Invalid(fieldPath, body, msg)) } - allErrs = append(allErrs, validateStringWithVariables(body, fieldPath, returnBodyVariables, returnBodySpecialVariables)...) + allErrs = append(allErrs, validateStringWithVariables(body, fieldPath, specialValidVars, validVars)...) return allErrs } diff --git a/pkg/apis/configuration/validation/validation_test.go b/pkg/apis/configuration/validation/validation_test.go index 3e2551df0f..fe17e4a84b 100644 --- a/pkg/apis/configuration/validation/validation_test.go +++ b/pkg/apis/configuration/validation/validation_test.go @@ -968,7 +968,7 @@ func TestValidateRedirectURL(t *testing.T) { } for _, test := range tests { - allErrs := validateRedirectURL(test.redirectURL, field.NewPath("url")) + allErrs := validateRedirectURL(test.redirectURL, field.NewPath("url"), validRedirectVariableNames) if len(allErrs) > 0 { t.Errorf("validateRedirectURL(%s) returned errors %v for valid input for the case of %s", test.redirectURL, allErrs, test.msg) } @@ -1032,7 +1032,7 @@ func TestValidateRedirectURLFails(t *testing.T) { } for _, test := range tests { - allErrs := validateRedirectURL(test.redirectURL, field.NewPath("action")) + allErrs := validateRedirectURL(test.redirectURL, field.NewPath("action"), validRedirectVariableNames) if len(allErrs) == 0 { t.Errorf("validateRedirectURL(%s) returned no errors for invalid input for the case of %s", test.redirectURL, test.msg) } @@ -2689,7 +2689,7 @@ func TestValidateActionReturnBody(t *testing.T) { } for _, test := range tests { - allErrs := validateActionReturnBody(test.body, field.NewPath("body")) + allErrs := validateActionReturnBody(test.body, field.NewPath("body"), returnBodySpecialVariables, returnBodyVariables) if len(allErrs) != 0 { t.Errorf("validateActionReturnBody(%v) returned errors %v for valid input for the case of: %v", test.body, allErrs, test.msg) } @@ -2716,7 +2716,7 @@ func TestValidateActionReturnBodyFails(t *testing.T) { } for _, test := range tests { - allErrs := validateActionReturnBody(test.body, field.NewPath("body")) + allErrs := validateActionReturnBody(test.body, field.NewPath("body"), returnBodySpecialVariables, returnBodyVariables) if len(allErrs) == 0 { t.Errorf("validateActionReturnBody(%v) returned no errors for invalid input for the case of: %v", test.body, test.msg) } @@ -2790,7 +2790,7 @@ func TestValidateActionReturn(t *testing.T) { } for _, test := range tests { - allErrs := validateActionReturn(test, field.NewPath("return")) + allErrs := validateActionReturn(test, field.NewPath("return"), returnBodySpecialVariables, returnBodyVariables) if len(allErrs) != 0 { t.Errorf("validateActionReturn(%v) returned errors for valid input", test) } @@ -2812,7 +2812,7 @@ func TestValidateActionReturnFails(t *testing.T) { } for _, test := range tests { - allErrs := validateActionReturn(test, field.NewPath("return")) + allErrs := validateActionReturn(test, field.NewPath("return"), returnBodySpecialVariables, returnBodyVariables) if len(allErrs) == 0 { t.Errorf("validateActionReturn(%v) returned no errors for invalid input", test) } @@ -2829,7 +2829,7 @@ func TestValidateStringWithVariables(t *testing.T) { validVars := map[string]bool{"scheme": true, "host": true} for _, test := range testStrings { - allErrs := validateStringWithVariables(test, field.NewPath("string"), validVars, nil) + allErrs := validateStringWithVariables(test, field.NewPath("string"), nil, validVars) if len(allErrs) != 0 { t.Errorf("validateStringWithVariables(%v) returned errors for valid input: %v", test, allErrs) } @@ -2843,7 +2843,7 @@ func TestValidateStringWithVariables(t *testing.T) { } for _, test := range testStringsSpecial { - allErrs := validateStringWithVariables(test, field.NewPath("string"), validVars, specialVars) + allErrs := validateStringWithVariables(test, field.NewPath("string"), specialVars, validVars) if len(allErrs) != 0 { t.Errorf("validateStringWithVariables(%v) returned errors for valid input: %v", test, allErrs) } @@ -2861,7 +2861,7 @@ func TestValidateStringWithVariablesFail(t *testing.T) { validVars := map[string]bool{"scheme": true, "host": true} for _, test := range testStrings { - allErrs := validateStringWithVariables(test, field.NewPath("string"), validVars, nil) + allErrs := validateStringWithVariables(test, field.NewPath("string"), nil, validVars) if len(allErrs) == 0 { t.Errorf("validateStringWithVariables(%v) returned no errors for invalid input", test) } @@ -2875,7 +2875,7 @@ func TestValidateStringWithVariablesFail(t *testing.T) { } for _, test := range testStringsSpecial { - allErrs := validateStringWithVariables(test, field.NewPath("string"), validVars, specialVars) + allErrs := validateStringWithVariables(test, field.NewPath("string"), specialVars, validVars) if len(allErrs) == 0 { t.Errorf("validateStringWithVariables(%v) returned no errors for invalid input", test) } @@ -2921,3 +2921,331 @@ func TestValidateSpecialVariableFails(t *testing.T) { } } } + +func TestErrorPageHasRequiredFields(t *testing.T) { + tests := []struct { + errorPage v1.ErrorPage + expected bool + }{ + { + errorPage: v1.ErrorPage{ + Codes: nil, + Return: nil, + Redirect: nil, + }, + expected: false, + }, + { + errorPage: v1.ErrorPage{ + Codes: nil, + Return: &v1.ErrorPageReturn{}, + Redirect: &v1.ErrorPageRedirect{}, + }, + expected: false, + }, + { + errorPage: v1.ErrorPage{ + Codes: nil, + Return: &v1.ErrorPageReturn{}, + Redirect: nil, + }, + expected: true, + }, + { + errorPage: v1.ErrorPage{ + Codes: nil, + Return: nil, + Redirect: &v1.ErrorPageRedirect{}, + }, + expected: true, + }, + } + + for _, test := range tests { + result := errorPageHasRequiredFields(test.errorPage) + if result != test.expected { + t.Errorf("errorPageHasRequiredFields(%v) returned %v but expected %v", test.errorPage, result, test.expected) + } + } +} + +func TestValidateErrorPage(t *testing.T) { + tests := []v1.ErrorPage{ + { + Codes: []int{400, 404}, + Return: &v1.ErrorPageReturn{ + ActionReturn: v1.ActionReturn{ + Body: "Hello World", + }, + }, + Redirect: nil, + }, + { + Codes: []int{400, 404}, + Return: nil, + Redirect: &v1.ErrorPageRedirect{ + ActionRedirect: v1.ActionRedirect{ + URL: "http://nginx.com", + }, + }, + }, + } + + for _, ep := range tests { + allErrs := validateErrorPage(ep, field.NewPath("errorPage")) + if len(allErrs) != 0 { + t.Errorf("validateErrorPage(%v) returned errors for valid input: %v", ep, allErrs) + } + } +} + +func TestValidateErrorPageFails(t *testing.T) { + tests := []v1.ErrorPage{ + {}, + { + Codes: []int{400, 404}, + Return: &v1.ErrorPageReturn{}, + Redirect: &v1.ErrorPageRedirect{}, + }, + { + Codes: []int{100, 700}, + Return: &v1.ErrorPageReturn{}, + Redirect: nil, + }, + { + Codes: nil, + Return: &v1.ErrorPageReturn{}, + Redirect: nil, + }, + } + + for _, ep := range tests { + allErrs := validateErrorPage(ep, field.NewPath("errorPage")) + if len(allErrs) == 0 { + t.Errorf("validateErrorPage(%v) returned no errors for invalid input", ep) + } + } +} + +func TestValidateErrorPageReturn(t *testing.T) { + tests := []v1.ErrorPageReturn{ + { + ActionReturn: v1.ActionReturn{ + Code: 200, + Type: "", + Body: "Could not process request, try again", + }, + Headers: nil, + }, + { + ActionReturn: v1.ActionReturn{ + Code: 0, + Type: "", + Body: "Could not process request, try again. Status ${status}", + }, + Headers: []v1.Header{ + { + Name: "Set-Cookie", + Value: "mycookie=true", + }, + }, + }, + { + ActionReturn: v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: `{\"message\": \"Could not process request, try again\", \"status\": \"${status}\"}`, + }, + Headers: nil, + }, + } + + for _, epr := range tests { + allErrs := validateErrorPageReturn(&epr, field.NewPath("return")) + if len(allErrs) != 0 { + t.Errorf("validateErrorPageReturn(%v) returned errors for valid input: %v", epr, allErrs) + } + } +} + +func TestValidateErrorPageReturnFails(t *testing.T) { + tests := []struct { + msg string + epr v1.ErrorPageReturn + }{ + { + msg: "empty body", + epr: v1.ErrorPageReturn{ + ActionReturn: v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: "", + }, + }, + }, + { + msg: "unescaped double quotes", + epr: v1.ErrorPageReturn{ + ActionReturn: v1.ActionReturn{ + Code: 200, + Type: "", + Body: ` "Oops, Could not process request"`, + }, + }, + }, + { + msg: "invalid variable", + epr: v1.ErrorPageReturn{ + ActionReturn: v1.ActionReturn{ + Code: 0, + Type: "", + Body: "Could not process request, response with invalid var: ${invalid}", + }, + }, + }, + { + msg: "invalid cookie name", + epr: v1.ErrorPageReturn{ + ActionReturn: v1.ActionReturn{ + Code: 200, + Type: "application/json", + Body: `{\"message\": \"Could not process request, try again\", \"status\": \"${status}\"}`, + }, + Headers: []v1.Header{ + { + Name: "Set-Cookie$_%^$ -", + Value: "mycookie=true", + }, + }, + }, + }, + } + + for _, test := range tests { + allErrs := validateErrorPageReturn(&test.epr, field.NewPath("return")) + if len(allErrs) == 0 { + t.Errorf("validateErrorPageReturn(%v) returned no errors for invalid input for the case of %v", test.epr, test.msg) + } + } +} + +func TestValidateErrorPageRedirect(t *testing.T) { + tests := []v1.ErrorPageRedirect{ + { + ActionRedirect: v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 301, + }, + }, + { + ActionRedirect: v1.ActionRedirect{ + URL: "${scheme}://nginx.com", + Code: 302, + }, + }, + } + + for _, epr := range tests { + allErrs := validateErrorPageRedirect(&epr, field.NewPath("redirect")) + if len(allErrs) != 0 { + t.Errorf("validateErrorPageRedirect(%v) returned errors for valid input: %v", epr, allErrs) + } + } +} + +func TestValidateErrorPageRedirectFails(t *testing.T) { + tests := []v1.ErrorPageRedirect{ + { + ActionRedirect: v1.ActionRedirect{ + URL: "", + Code: 301, + }, + }, + { + ActionRedirect: v1.ActionRedirect{ + URL: `"http://nginx.com"`, + Code: 301, + }, + }, + { + ActionRedirect: v1.ActionRedirect{ + URL: "http://nginx.com", + Code: 100, + }, + }, + { + ActionRedirect: v1.ActionRedirect{ + URL: "$scheme://nginx.com", + Code: 302, + }, + }, + { + ActionRedirect: v1.ActionRedirect{ + URL: "https://${host}.com", + Code: 302, + }, + }, + } + + for _, epr := range tests { + allErrs := validateErrorPageRedirect(&epr, field.NewPath("redirect")) + if len(allErrs) == 0 { + t.Errorf("validateErrorPageRedirect(%v) returned no errors for invalid input", epr) + } + } +} + +func TestValidateErrorPageHeader(t *testing.T) { + tests := []v1.Header{ + { + Name: "Header-Name", + Value: "", + }, + { + Name: "Header-Name", + Value: "Value", + }, + { + Name: "Header-Name", + Value: "${status}", + }, + } + + for _, test := range tests { + allErrs := validateErrorPageHeader(test, field.NewPath("header")) + if len(allErrs) != 0 { + t.Errorf("validateErrorPageHeader(%v) returned errors for valid input", test) + } + } +} + +func TestValidateErrorPageHeaderFails(t *testing.T) { + tests := []v1.Header{ + { + Name: "", + Value: "", + }, + { + Name: "Header-!!#Name", + Value: "", + }, + { + Name: "Header-Name", + Value: "$novar", + }, + { + Name: "Header-Name", + Value: "${invalid_var}", + }, { + Name: "Header-Name", + Value: `unescaped "`, + }, + } + + for _, test := range tests { + allErrs := validateErrorPageHeader(test, field.NewPath("header")) + if len(allErrs) == 0 { + t.Errorf("validateErrorPageHeader(%v) returned no errors for invalid input", test) + } + } +}