From 974015c235d39aa0b89b4ba894088942cb063332 Mon Sep 17 00:00:00 2001 From: Ciara Stacke Date: Thu, 2 Dec 2021 20:28:31 +0000 Subject: [PATCH] Chore: generate error page warnings for gRPC upstreams --- .../version2/nginx-plus.virtualserver.tmpl | 44 +- .../configs/version2/nginx.virtualserver.tmpl | 44 +- internal/configs/virtualserver.go | 123 ++++-- internal/configs/virtualserver_test.go | 407 +++++++++++++++++- .../virtual-server-error-page.yaml | 29 ++ tests/suite/test_virtual_server_grpc.py | 19 +- 6 files changed, 578 insertions(+), 88 deletions(-) create mode 100644 tests/data/virtual-server-grpc/virtual-server-error-page.yaml diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 79b50dd057..50d3730e35 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -340,6 +340,28 @@ server { {{ end }} {{ end }} + {{ if $l.GRPCPass }} + error_page 400 = @grpc_internal; + error_page 401 = @grpc_unauthenticated; + error_page 403 = @grpc_permission_denied; + error_page 404 = @grpc_unimplemented; + error_page 429 = @grpc_unavailable; + error_page 502 = @grpc_unavailable; + error_page 503 = @grpc_unavailable; + error_page 504 = @grpc_unavailable; + error_page 405 = @grpc_internal; + error_page 408 = @grpc_deadline_exceeded; + error_page 413 = @grpc_resource_exhausted; + error_page 414 = @grpc_resource_exhausted; + error_page 415 = @grpc_internal; + error_page 426 = @grpc_internal; + error_page 495 = @grpc_unauthenticated; + error_page 496 = @grpc_unauthenticated; + error_page 497 = @grpc_internal; + error_page 500 = @grpc_internal; + error_page 501 = @grpc_internal; + {{ end }} + {{ range $e := $l.ErrorPages }} error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}"; {{ end }} @@ -372,28 +394,6 @@ server { {{ if $l.ProxyBufferSize }} {{ $proxyOrGRPC }}_buffer_size {{ $l.ProxyBufferSize }}; {{ end }} - - {{ if $l.GRPCPass }} - error_page 400 = @grpc_internal; - error_page 401 = @grpc_unauthenticated; - error_page 403 = @grpc_permission_denied; - error_page 404 = @grpc_unimplemented; - error_page 429 = @grpc_unavailable; - error_page 502 = @grpc_unavailable; - error_page 503 = @grpc_unavailable; - error_page 504 = @grpc_unavailable; - error_page 405 = @grpc_internal; - error_page 408 = @grpc_deadline_exceeded; - error_page 413 = @grpc_resource_exhausted; - error_page 414 = @grpc_resource_exhausted; - error_page 415 = @grpc_internal; - error_page 426 = @grpc_internal; - error_page 495 = @grpc_unauthenticated; - error_page 496 = @grpc_unauthenticated; - error_page 497 = @grpc_internal; - error_page 500 = @grpc_internal; - error_page 501 = @grpc_internal; - {{ end }} {{ if not $l.GRPCPass }} proxy_http_version 1.1; diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 58fc4d0c9c..96dd4d3408 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -244,6 +244,28 @@ server { {{ $proxyOrGRPC }}_ssl_name {{ .SSLName }}; {{ end }} + {{ if $l.GRPCPass }} + error_page 400 = @grpc_internal; + error_page 401 = @grpc_unauthenticated; + error_page 403 = @grpc_permission_denied; + error_page 404 = @grpc_unimplemented; + error_page 429 = @grpc_unavailable; + error_page 502 = @grpc_unavailable; + error_page 503 = @grpc_unavailable; + error_page 504 = @grpc_unavailable; + error_page 405 = @grpc_internal; + error_page 408 = @grpc_deadline_exceeded; + error_page 413 = @grpc_resource_exhausted; + error_page 414 = @grpc_resource_exhausted; + error_page 415 = @grpc_internal; + error_page 426 = @grpc_internal; + error_page 495 = @grpc_unauthenticated; + error_page 496 = @grpc_unauthenticated; + error_page 497 = @grpc_internal; + error_page 500 = @grpc_internal; + error_page 501 = @grpc_internal; + {{ end }} + {{ range $e := $l.ErrorPages }} error_page {{ $e.Codes }} {{ if ne 0 $e.ResponseCode }}={{ $e.ResponseCode }}{{ end }} "{{ $e.Name }}"; {{ end }} @@ -276,28 +298,6 @@ server { {{ if $l.ProxyBufferSize }} {{ $proxyOrGRPC }}_buffer_size {{ $l.ProxyBufferSize }}; {{ end }} - - {{ if $l.GRPCPass }} - error_page 400 = @grpc_internal; - error_page 401 = @grpc_unauthenticated; - error_page 403 = @grpc_permission_denied; - error_page 404 = @grpc_unimplemented; - error_page 429 = @grpc_unavailable; - error_page 502 = @grpc_unavailable; - error_page 503 = @grpc_unavailable; - error_page 504 = @grpc_unavailable; - error_page 405 = @grpc_internal; - error_page 408 = @grpc_deadline_exceeded; - error_page 413 = @grpc_resource_exhausted; - error_page 414 = @grpc_resource_exhausted; - error_page 415 = @grpc_internal; - error_page 426 = @grpc_internal; - error_page 495 = @grpc_unauthenticated; - error_page 496 = @grpc_unauthenticated; - error_page 497 = @grpc_internal; - error_page 500 = @grpc_internal; - error_page 501 = @grpc_internal; - {{ end }} {{ if not $l.GRPCPass }} proxy_http_version 1.1; diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index b688901af8..1f64647c9a 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -27,6 +27,28 @@ const ( subRouteContext = "subroute" ) +var grpcConflictingErrors = map[int]bool{ + 400: true, + 401: true, + 403: true, + 404: true, + 405: true, + 408: true, + 413: true, + 414: true, + 415: true, + 426: true, + 429: true, + 495: true, + 496: true, + 497: true, + 500: true, + 501: true, + 502: true, + 503: true, + 504: true, +} + var incompatibleLBMethodsForSlowStart = map[string]bool{ "random": true, "ip_hash": true, @@ -375,8 +397,12 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS // generates config for VirtualServer routes for _, r := range vsEx.VirtualServer.Spec.Routes { - errorPageIndex := len(errorPageLocations) - errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPageIndex, r.ErrorPages)...) + errorPages := errorPageDetails{ + pages: r.ErrorPages, + index: len(errorPageLocations), + owner: vsEx.VirtualServer, + } + errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPages.index, errorPages.pages)...) // ignore routes that reference VirtualServerRoute if r.Route != "" { @@ -392,8 +418,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS // store route error pages and route index for the referenced VirtualServerRoute in case they don't define their own if len(r.ErrorPages) > 0 { - vsrErrorPagesFromVs[name] = r.ErrorPages - vsrErrorPagesRouteIndex[name] = errorPageIndex + vsrErrorPagesFromVs[name] = errorPages.pages + vsrErrorPagesRouteIndex[name] = errorPages.index } // store route policies for the referenced VirtualServerRoute in case they don't define their own @@ -426,13 +452,13 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS matchesRoutes, len(splitClients), vsc.cfgParams, - r.ErrorPages, - errorPageIndex, + errorPages, vsLocSnippets, vsc.enableSnippets, len(returnLocations), isVSR, "", "", + vsc.warnings, ) addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations) @@ -444,9 +470,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS matchesRoutes++ } 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), isVSR, "", "") + vsc.cfgParams, errorPages, r.Path, vsLocSnippets, vsc.enableSnippets, len(returnLocations), isVSR, "", "", vsc.warnings) addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations) - splitClients = append(splitClients, cfg.SplitClients...) locations = append(locations, cfg.Locations...) internalRedirectLocations = append(internalRedirectLocations, cfg.InternalRedirectLocation) @@ -457,8 +482,8 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS proxySSLName := generateProxySSLName(upstream.Service, vsEx.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), isVSR, "", "") + loc, returnLoc := generateLocation(r.Path, upstreamName, upstream, r.Action, vsc.cfgParams, errorPages, false, + proxySSLName, r.Path, vsLocSnippets, vsc.enableSnippets, len(returnLocations), isVSR, "", "", vsc.warnings) addPoliciesCfgToLocation(routePoliciesCfg, &loc) locations = append(locations, loc) @@ -473,15 +498,18 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS isVSR := true upstreamNamer := newUpstreamNamerForVirtualServerRoute(vsEx.VirtualServer, vsr) for _, r := range vsr.Spec.Subroutes { - errorPageIndex := len(errorPageLocations) - errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPageIndex, r.ErrorPages)...) - errorPages := r.ErrorPages + errorPages := errorPageDetails{ + pages: r.ErrorPages, + index: len(errorPageLocations), + owner: vsr, + } + errorPageLocations = append(errorPageLocations, generateErrorPageLocations(errorPages.index, errorPages.pages)...) vsrNamespaceName := fmt.Sprintf("%v/%v", vsr.Namespace, vsr.Name) // use the VirtualServer error pages if the route does not define any if r.ErrorPages == nil { if vsErrorPages, ok := vsrErrorPagesFromVs[vsrNamespaceName]; ok { - errorPages = vsErrorPages - errorPageIndex = vsrErrorPagesRouteIndex[vsrNamespaceName] + errorPages.pages = vsErrorPages + errorPages.index = vsrErrorPagesRouteIndex[vsrNamespaceName] } } @@ -529,13 +557,13 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS len(splitClients), vsc.cfgParams, errorPages, - errorPageIndex, locSnippets, vsc.enableSnippets, len(returnLocations), isVSR, vsr.Name, vsr.Namespace, + vsc.warnings, ) addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations) @@ -547,7 +575,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS matchesRoutes++ } 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), isVSR, vsr.Name, vsr.Namespace) + errorPages, r.Path, locSnippets, vsc.enableSnippets, len(returnLocations), isVSR, vsr.Name, vsr.Namespace, vsc.warnings) addPoliciesCfgToLocations(routePoliciesCfg, cfg.Locations) splitClients = append(splitClients, cfg.SplitClients...) @@ -560,7 +588,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig(vsEx *VirtualS 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), isVSR, vsr.Name, vsr.Namespace) + proxySSLName, r.Path, locSnippets, vsc.enableSnippets, len(returnLocations), isVSR, vsr.Name, vsr.Namespace, vsc.warnings) addPoliciesCfgToLocation(routePoliciesCfg, &loc) locations = append(locations, loc) @@ -1495,9 +1523,16 @@ func generateReturnBlock(text string, code int, defaultCode int) *version2.Retur return returnBlock } +type errorPageDetails struct { + pages []conf_v1.ErrorPage + index int + owner runtime.Object +} + func generateLocation(path string, upstreamName string, upstream conf_v1.Upstream, action *conf_v1.Action, - cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, internal bool, errPageIndex int, proxySSLName string, - originalPath string, locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, vsrNamespace string) (version2.Location, *version2.ReturnLocation) { + cfgParams *ConfigParams, errorPages errorPageDetails, internal bool, proxySSLName string, + originalPath string, locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, + vsrNamespace string, vscWarnings Warnings) (version2.Location, *version2.ReturnLocation) { locationSnippets := generateSnippets(enableSnippets, locSnippets, cfgParams.LocationSnippets) if action.Redirect != nil { @@ -1508,8 +1543,10 @@ func generateLocation(path string, upstreamName string, upstream conf_v1.Upstrea return generateLocationForReturn(path, cfgParams.LocationSnippets, action.Return, retLocIndex) } - return generateLocationForProxying(path, upstreamName, upstream, cfgParams, errorPages, internal, - errPageIndex, proxySSLName, action.Proxy, originalPath, locationSnippets, isVSR, vsrName, vsrNamespace), nil + checkGrpcErrorPageCodes(errorPages, isGRPC(upstream.Type), upstream.Name, &vscWarnings) + + return generateLocationForProxying(path, upstreamName, upstream, cfgParams, errorPages.pages, internal, + errorPages.index, proxySSLName, action.Proxy, originalPath, locationSnippets, isVSR, vsrName, vsrNamespace), nil } func generateProxySetHeaders(proxy *conf_v1.ActionProxy) []version2.Header { @@ -1710,8 +1747,7 @@ func generateSplits( variableNamer *variableNamer, scIndex int, cfgParams *ConfigParams, - errorPages []conf_v1.ErrorPage, - errPageIndex int, + errorPages errorPageDetails, originalPath string, locSnippets string, enableSnippets bool, @@ -1719,6 +1755,7 @@ func generateSplits( isVSR bool, vsrName string, vsrNamespace string, + vscWarnings Warnings, ) (version2.SplitClient, []version2.Location, []version2.ReturnLocation) { var distributions []version2.Distribution @@ -1746,7 +1783,7 @@ func generateSplits( proxySSLName := generateProxySSLName(upstream.Service, upstreamNamer.namespace) newRetLocIndex := retLocIndex + len(returnLocations) loc, returnLoc := generateLocation(path, upstreamName, upstream, s.Action, cfgParams, errorPages, true, - errPageIndex, proxySSLName, originalPath, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace) + proxySSLName, originalPath, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace, vscWarnings) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -1763,8 +1800,7 @@ func generateDefaultSplitsConfig( variableNamer *variableNamer, scIndex int, cfgParams *ConfigParams, - errorPages []conf_v1.ErrorPage, - errPageIndex int, + errorPages errorPageDetails, originalPath string, locSnippets string, enableSnippets bool, @@ -1772,9 +1808,10 @@ func generateDefaultSplitsConfig( isVSR bool, vsrName string, vsrNamespace string, + vscWarnings Warnings, ) routingCfg { sc, locs, returnLocs := generateSplits(route.Splits, upstreamNamer, crUpstreams, variableNamer, scIndex, cfgParams, - errorPages, errPageIndex, originalPath, locSnippets, enableSnippets, retLocIndex, isVSR, vsrName, vsrNamespace) + errorPages, originalPath, locSnippets, enableSnippets, retLocIndex, isVSR, vsrName, vsrNamespace, vscWarnings) splitClientVarName := variableNamer.GetNameForSplitClientVariable(scIndex) @@ -1792,8 +1829,8 @@ func generateDefaultSplitsConfig( } func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, crUpstreams map[string]conf_v1.Upstream, - variableNamer *variableNamer, index int, scIndex int, cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, - errPageIndex int, locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, vsrNamespace string) routingCfg { + variableNamer *variableNamer, index int, scIndex int, cfgParams *ConfigParams, errorPages errorPageDetails, + locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, vsrNamespace string, vscWarnings Warnings) routingCfg { // Generate maps var maps []version2.Map @@ -1876,7 +1913,6 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr scIndex+scLocalIndex, cfgParams, errorPages, - errPageIndex, route.Path, locSnippets, enableSnippets, @@ -1884,6 +1920,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr isVSR, vsrName, vsrNamespace, + vscWarnings, ) scLocalIndex++ splitClients = append(splitClients, sc) @@ -1896,7 +1933,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr proxySSLName := generateProxySSLName(upstream.Service, upstreamNamer.namespace) newRetLocIndex := retLocIndex + len(returnLocations) loc, returnLoc := generateLocation(path, upstreamName, upstream, m.Action, cfgParams, errorPages, true, - errPageIndex, proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace) + proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace, vscWarnings) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -1915,7 +1952,6 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr scIndex+scLocalIndex, cfgParams, errorPages, - errPageIndex, route.Path, locSnippets, enableSnippets, @@ -1923,6 +1959,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr isVSR, vsrName, vsrNamespace, + vscWarnings, ) splitClients = append(splitClients, sc) locations = append(locations, locs...) @@ -1934,7 +1971,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr proxySSLName := generateProxySSLName(upstream.Service, upstreamNamer.namespace) newRetLocIndex := retLocIndex + len(returnLocations) loc, returnLoc := generateLocation(path, upstreamName, upstream, route.Action, cfgParams, errorPages, true, - errPageIndex, proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace) + proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace, vscWarnings) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -2179,6 +2216,24 @@ func generateErrorPageName(errPageIndex int, index int) string { return fmt.Sprintf("@error_page_%v_%v", errPageIndex, index) } +func checkGrpcErrorPageCodes(errorPages errorPageDetails, isGRPC bool, uName string, vscWarnings *Warnings) { + if errorPages.pages == nil || !isGRPC { + return + } + + var c []int + for _, e := range errorPages.pages { + for _, code := range e.Codes { + if grpcConflictingErrors[code] { + c = append(c, code) + } + } + } + if len(c) > 0 { + vscWarnings.AddWarningf(errorPages.owner, "The error page configuration for the upstream %s is ignored for status code(s) %v, which cannot be used for GRPC upstreams.", uName, c) + } +} + func generateErrorPageCodes(codes []int) string { var c []string for _, code := range codes { diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 6c40b631b9..e1c0c9b968 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -657,6 +657,371 @@ func TestGenerateVirtualServerConfig(t *testing.T) { } } +func TestGenerateVirtualServerConfigGrpcErrorPageWarning(t *testing.T) { + virtualServerEx := VirtualServerEx{ + VirtualServer: &conf_v1.VirtualServer{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "cafe", + Namespace: "default", + }, + Spec: conf_v1.VirtualServerSpec{ + Host: "cafe.example.com", + TLS: &conf_v1.TLS{ + Secret: "", + }, + Upstreams: []conf_v1.Upstream{ + { + Name: "grpc-app-1", + Service: "grpc-svc", + Port: 50051, + Type: "grpc", + TLS: conf_v1.UpstreamTLS{ + Enable: true, + }, + }, + { + Name: "grpc-app-2", + Service: "grpc-svc2", + Port: 50052, + Type: "grpc", + TLS: conf_v1.UpstreamTLS{ + Enable: true, + }, + }, + { + Name: "tea", + Service: "tea-svc", + Port: 80, + }, + }, + Routes: []conf_v1.Route{ + { + Path: "/grpc-errorpage", + Action: &conf_v1.Action{ + Pass: "grpc-app-1", + }, + ErrorPages: []conf_v1.ErrorPage{ + { + Codes: []int{404, 405}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "text/plain", + Body: "All Good", + }, + }, + }, + }, + }, + { + Path: "/grpc-matches", + Matches: []conf_v1.Match{ + { + Conditions: []conf_v1.Condition{ + { + Variable: "$request_method", + Value: "POST", + }, + }, + Action: &conf_v1.Action{ + Pass: "grpc-app-2", + }, + }, + }, + Action: &conf_v1.Action{ + Pass: "tea", + }, + ErrorPages: []conf_v1.ErrorPage{ + { + Codes: []int{404}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "text/plain", + Body: "Original resource not found, but success!", + }, + }, + }, + }, + }, + { + Path: "/grpc-splits", + Splits: []conf_v1.Split{ + { + Weight: 90, + Action: &conf_v1.Action{ + Pass: "grpc-app-1", + }, + }, + { + Weight: 10, + Action: &conf_v1.Action{ + Pass: "grpc-app-2", + }, + }, + }, + ErrorPages: []conf_v1.ErrorPage{ + { + Codes: []int{404, 405}, + Return: &conf_v1.ErrorPageReturn{ + ActionReturn: conf_v1.ActionReturn{ + Code: 200, + Type: "text/plain", + Body: "All Good", + }, + }, + }, + }, + }, + }, + }, + }, + Endpoints: map[string][]string{ + "default/grpc-svc:50051": { + "10.0.0.20:80", + }, + }, + } + + baseCfgParams := ConfigParams{ + HTTP2: true, + } + + expected := version2.VirtualServerConfig{ + Upstreams: []version2.Upstream{ + { + UpstreamLabels: version2.UpstreamLabels{ + Service: "grpc-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Name: "vs_default_cafe_grpc-app-1", + Servers: []version2.UpstreamServer{ + { + Address: "10.0.0.20:80", + }, + }, + }, + { + Name: "vs_default_cafe_grpc-app-2", + UpstreamLabels: version2.UpstreamLabels{ + Service: "grpc-svc2", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Servers: []version2.UpstreamServer{ + { + Address: "unix:/var/lib/nginx/nginx-502-server.sock", + }, + }, + }, + { + Name: "vs_default_cafe_tea", + UpstreamLabels: version2.UpstreamLabels{ + Service: "tea-svc", + ResourceType: "virtualserver", + ResourceName: "cafe", + ResourceNamespace: "default", + }, + Servers: []version2.UpstreamServer{ + { + Address: "unix:/var/lib/nginx/nginx-502-server.sock", + }, + }, + }, + }, + HTTPSnippets: []string{}, + LimitReqZones: []version2.LimitReqZone{}, + Maps: []version2.Map{ + { + Source: "$request_method", + Variable: "$vs_default_cafe_matches_0_match_0_cond_0", + Parameters: []version2.Parameter{ + { + Value: `"POST"`, + Result: "1", + }, + { + Value: "default", + Result: "0", + }, + }, + }, + { + Source: "$vs_default_cafe_matches_0_match_0_cond_0", + Variable: "$vs_default_cafe_matches_0", + Parameters: []version2.Parameter{ + { + Value: "~^1", + Result: "/internal_location_matches_0_match_0", + }, + { + Value: "default", + Result: "/internal_location_matches_0_default", + }, + }, + }, + }, + Server: version2.Server{ + ServerName: "cafe.example.com", + StatusZone: "cafe.example.com", + VSNamespace: "default", + VSName: "cafe", + SSL: &version2.SSL{ + HTTP2: true, + Certificate: "/etc/nginx/secrets/wildcard", + CertificateKey: "/etc/nginx/secrets/wildcard", + }, + InternalRedirectLocations: []version2.InternalRedirectLocation{ + { + Path: "/grpc-matches", + Destination: "$vs_default_cafe_matches_0", + }, + { + Path: "/grpc-splits", + Destination: "$vs_default_cafe_splits_0", + }, + }, + Locations: []version2.Location{ + { + Path: "/grpc-errorpage", + ProxyPass: "https://vs_default_cafe_grpc-app-1", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ErrorPages: []version2.ErrorPage{{Name: "@error_page_0_0", Codes: "404 405", ResponseCode: 200}}, + ProxyInterceptErrors: true, + ProxySSLName: "grpc-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "grpc-svc", + GRPCPass: "grpcs://vs_default_cafe_grpc-app-1", + }, + { + Path: "/internal_location_matches_0_match_0", + Internal: true, + ProxyPass: "https://vs_default_cafe_grpc-app-2$request_uri", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + Rewrites: []string{"^ $request_uri break"}, + ErrorPages: []version2.ErrorPage{{Name: "@error_page_1_0", Codes: "404", ResponseCode: 200}}, + ProxyInterceptErrors: true, + ProxySSLName: "grpc-svc2.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "grpc-svc2", + GRPCPass: "grpcs://vs_default_cafe_grpc-app-2", + }, + { + Path: "/internal_location_matches_0_default", + Internal: true, + ProxyPass: "http://vs_default_cafe_tea$request_uri", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + ErrorPages: []version2.ErrorPage{{Name: "@error_page_1_0", Codes: "404", ResponseCode: 200}}, + ProxyInterceptErrors: true, + ProxySSLName: "tea-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "tea-svc", + }, + { + Path: "/internal_location_splits_0_split_0", + Internal: true, + ProxyPass: "https://vs_default_cafe_grpc-app-1$request_uri", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: false, + ErrorPages: []version2.ErrorPage{{Name: "@error_page_2_0", Codes: "404 405", ResponseCode: 200}}, + ProxyInterceptErrors: true, + Rewrites: []string{"^ $request_uri break"}, + ProxySSLName: "grpc-svc.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "grpc-svc", + GRPCPass: "grpcs://vs_default_cafe_grpc-app-1", + }, + { + Path: "/internal_location_splits_0_split_1", + Internal: true, + ProxyPass: "https://vs_default_cafe_grpc-app-2$request_uri", + ProxyNextUpstream: "error timeout", + ProxyNextUpstreamTimeout: "0s", + ProxyNextUpstreamTries: 0, + HasKeepalive: false, + ErrorPages: []version2.ErrorPage{{Name: "@error_page_2_0", Codes: "404 405", ResponseCode: 200}}, + ProxyInterceptErrors: true, + Rewrites: []string{"^ $request_uri break"}, + ProxySSLName: "grpc-svc2.default.svc", + ProxyPassRequestHeaders: true, + ProxySetHeaders: []version2.Header{{Name: "Host", Value: "$host"}}, + ServiceName: "grpc-svc2", + GRPCPass: "grpcs://vs_default_cafe_grpc-app-2", + }, + }, + ErrorPageLocations: []version2.ErrorPageLocation{ + { + Name: "@error_page_0_0", + DefaultType: "text/plain", + Return: &version2.Return{Text: "All Good"}, + }, + { + Name: "@error_page_1_0", + DefaultType: "text/plain", + Return: &version2.Return{Text: "Original resource not found, but success!"}, + }, + { + Name: "@error_page_2_0", + DefaultType: "text/plain", + Return: &version2.Return{Text: "All Good"}, + }, + }, + }, + SplitClients: []version2.SplitClient{ + { + Source: "$request_id", + Variable: "$vs_default_cafe_splits_0", + Distributions: []version2.Distribution{ + { + Weight: "90%", + Value: "/internal_location_splits_0_split_0", + }, + { + Weight: "10%", + Value: "/internal_location_splits_0_split_1", + }, + }, + }, + }, + } + expectedWarnings := Warnings{ + virtualServerEx.VirtualServer: { + `The error page configuration for the upstream grpc-app-1 is ignored for status code(s) [404 405], which cannot be used for GRPC upstreams.`, + `The error page configuration for the upstream grpc-app-2 is ignored for status code(s) [404], which cannot be used for GRPC upstreams.`, + `The error page configuration for the upstream grpc-app-1 is ignored for status code(s) [404 405], which cannot be used for GRPC upstreams.`, + `The error page configuration for the upstream grpc-app-2 is ignored for status code(s) [404 405], which cannot be used for GRPC upstreams.`, + }, + } + isPlus := false + isResolverConfigured := false + isWildcardEnabled := true + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled) + + result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil) + if diff := cmp.Diff(expected, result); diff != "" { + t.Errorf("TestGenerateVirtualServerConfigGrpcErrorPageWarning() mismatch (-want +got):\n%s", diff) + } + + if !reflect.DeepEqual(vsc.warnings, expectedWarnings) { + t.Errorf("GenerateVirtualServerConfig() returned warnings of \n%v but expected \n%v", warnings, expectedWarnings) + } +} + func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { virtualServerEx := VirtualServerEx{ VirtualServer: &conf_v1.VirtualServer{ @@ -4652,7 +5017,6 @@ func TestGenerateSplits(t *testing.T) { }, }, } - expectedSplitClient := version2.SplitClient{ Source: "$request_id", Variable: "$vs_default_cafe_splits_1", @@ -4759,6 +5123,14 @@ func TestGenerateSplits(t *testing.T) { } returnLocationIndex := 1 + errorPageDetails := errorPageDetails{ + pages: errorPages, + index: 0, + owner: nil, + } + + vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}, false) + resultSplitClient, resultLocations, resultReturnLocations := generateSplits( splits, upstreamNamer, @@ -4766,8 +5138,7 @@ func TestGenerateSplits(t *testing.T) { variableNamer, scIndex, &cfgParams, - errorPages, - 0, + errorPageDetails, originalPath, locSnippet, enableSnippets, @@ -4775,6 +5146,7 @@ func TestGenerateSplits(t *testing.T) { true, "coffee", "default", + vsc.warnings, ) if !reflect.DeepEqual(resultSplitClient, expectedSplitClient) { t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultSplitClient, expectedSplitClient) @@ -4882,8 +5254,14 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { }, } + errorPageDetails := errorPageDetails{ + pages: route.ErrorPages, + index: 0, + owner: nil, + } + result := generateDefaultSplitsConfig(route, upstreamNamer, crUpstreams, variableNamer, index, &cfgParams, - route.ErrorPages, 0, "", locSnippet, enableSnippets, 0, true, "coffee", "default") + errorPageDetails, "", locSnippet, enableSnippets, 0, true, "coffee", "default", Warnings{}) if !reflect.DeepEqual(result, expected) { t.Errorf("generateDefaultSplitsConfig() returned \n%+v but expected \n%+v", result, expected) } @@ -5273,6 +5651,12 @@ func TestGenerateMatchesConfig(t *testing.T) { "vs_default_cafe_tea": {Service: "tea"}, } + errorPageDetails := errorPageDetails{ + pages: errorPages, + index: 2, + owner: nil, + } + result := generateMatchesConfig( route, upstreamNamer, @@ -5281,14 +5665,14 @@ func TestGenerateMatchesConfig(t *testing.T) { index, scIndex, &cfgParams, - errorPages, - 2, + errorPageDetails, locSnippets, enableSnippets, 0, false, "", "", + Warnings{}, ) if !reflect.DeepEqual(result, expected) { t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) @@ -5675,6 +6059,13 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { "vs_default_cafe_coffee-v1": {Service: "coffee-v1"}, "vs_default_cafe_coffee-v2": {Service: "coffee-v2"}, } + + errorPageDetails := errorPageDetails{ + pages: errorPages, + index: 0, + owner: nil, + } + result := generateMatchesConfig( route, upstreamNamer, @@ -5683,14 +6074,14 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { index, scIndex, &cfgParams, - errorPages, - 0, + errorPageDetails, locSnippets, enableSnippets, 0, true, "coffee", "default", + Warnings{}, ) if !reflect.DeepEqual(result, expected) { t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) diff --git a/tests/data/virtual-server-grpc/virtual-server-error-page.yaml b/tests/data/virtual-server-grpc/virtual-server-error-page.yaml new file mode 100644 index 0000000000..6d13fb6011 --- /dev/null +++ b/tests/data/virtual-server-grpc/virtual-server-error-page.yaml @@ -0,0 +1,29 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server +spec: + host: virtual-server.example.com + tls: + secret: virtual-server-tls-grpc-secret + upstreams: + - name: grpc1 + service: grpc1-svc + port: 50051 + type: grpc + - name: grpc2 + service: grpc2-svc + port: 50051 + type: grpc + routes: + - path: "/helloworld.Greeter" + action: + pass: grpc1 + errorPages: + - codes: [404] + return: + code: 200 + body: "Original resource not found, but success!" + - path: "/notimplemented" + action: + pass: grpc2 diff --git a/tests/suite/test_virtual_server_grpc.py b/tests/suite/test_virtual_server_grpc.py index 2bc1290c76..5cbe800e51 100644 --- a/tests/suite/test_virtual_server_grpc.py +++ b/tests/suite/test_virtual_server_grpc.py @@ -1,11 +1,11 @@ import grpc import pytest import time -from kubernetes.client.rest import ApiException from settings import TEST_DATA, DEPLOYMENTS from suite.custom_assertions import assert_event_starts_with_text_and_contains_errors, \ - assert_grpc_entries_exist, assert_proxy_entries_do_not_exist, assert_vs_conf_not_exists + assert_grpc_entries_exist, assert_proxy_entries_do_not_exist, \ + assert_vs_conf_not_exists, assert_event from suite.grpc.helloworld_pb2 import HelloRequest from suite.grpc.helloworld_pb2_grpc import GreeterStub from suite.resources_utils import create_example_app, wait_until_all_pods_are_ready, \ @@ -186,6 +186,21 @@ def test_config_after_enable_tls(self, kube_apis, ingress_controller_prerequisit ingress_controller_prerequisites.namespace) assert 'grpc_pass grpcs://' in config + @pytest.mark.ciara + @pytest.mark.parametrize("backend_setup", [{"app_type": "grpc-vs"}], indirect=True) + def test_config_error_page_warning(self, kube_apis, ingress_controller_prerequisites, crd_ingress_controller, + backend_setup, virtual_server_setup): + text = f"{virtual_server_setup.namespace}/{virtual_server_setup.vs_name}" + vs_event_warning_text = f"Configuration for {text} was added or updated ; with warning(s): The error page" + patch_virtual_server_from_yaml(kube_apis.custom_objects, + virtual_server_setup.vs_name, + f"{TEST_DATA}/virtual-server-grpc/virtual-server-error-page.yaml", + virtual_server_setup.namespace) + wait_before_test(30) + + events = get_events(kube_apis.v1, virtual_server_setup.namespace) + assert_event(vs_event_warning_text, events) + @pytest.mark.vs @pytest.mark.smoke