From 36bedb66d4cfa3c2ec427b14310fd01288600500 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Mon, 16 Nov 2020 11:11:27 +0000 Subject: [PATCH 01/16] Update log format with k8s object reference --- .../configmap-resource.md | 2 +- internal/configs/ingress.go | 8 +- internal/configs/ingress_test.go | 6 + internal/configs/version1/config.go | 3 + .../configs/version1/nginx-plus.ingress.tmpl | 8 ++ internal/configs/version1/nginx-plus.tmpl | 8 +- internal/configs/version1/nginx.ingress.tmpl | 6 +- internal/configs/version1/nginx.tmpl | 5 +- internal/configs/version2/http.go | 2 + .../version2/nginx-plus.virtualserver.tmpl | 8 ++ .../configs/version2/nginx.virtualserver.tmpl | 8 ++ internal/configs/virtualserver.go | 2 + internal/configs/virtualserver_test.go | 126 ++++++++++-------- 13 files changed, 128 insertions(+), 64 deletions(-) diff --git a/docs-web/configuration/global-configuration/configmap-resource.md b/docs-web/configuration/global-configuration/configmap-resource.md index cda394b8af..d54988336c 100644 --- a/docs-web/configuration/global-configuration/configmap-resource.md +++ b/docs-web/configuration/global-configuration/configmap-resource.md @@ -205,7 +205,7 @@ See the doc about [VirtualServer and VirtualServerRoute resources](/nginx-ingres * - ``log-format`` - Sets the custom `log format `_ for HTTP and HTTPS traffic. For convenience, it is possible to define the log format across multiple lines (each line separated by ``\n``). In that case, the Ingress Controller will replace every ``\n`` character with a space character. All ``'`` characters must be escaped. - See the `template file `_ for the access log. - - + - 127.0.0.1 - - [01/Jun/2020:18:54:31 +0000] "GET /coffee HTTP/1.1" 200 156 "-" "curl/7.54.0" "-" resource_namespace=default resource_name=cafe resource_type=ingress service_namespace=default service_name=coffee-svc pod_name=coffee-8c8ff9b4f-r99h9 * - ``log-format-escaping`` - Sets the characters escaping for the variables of the log format. Supported values: ``json`` (JSON escaping), ``default`` (the default escaping) ``none`` (disables escaping). - ``default`` diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index ad6c5105ab..4a08a93483 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -200,7 +200,8 @@ func generateNginxCfg(ingEx *IngressEx, pems map[string]string, apResources map[ ssl := isSSLEnabled(sslServices[path.Backend.ServiceName], cfgParams, staticParams) proxySSLName := generateProxySSLName(path.Backend.ServiceName, ingEx.Ingress.Namespace) loc := createLocation(pathOrDefault(path.Path), upstreams[upsName], &cfgParams, wsServices[path.Backend.ServiceName], rewrites[path.Backend.ServiceName], - ssl, grpcServices[path.Backend.ServiceName], proxySSLName, path.PathType) + ssl, grpcServices[path.Backend.ServiceName], proxySSLName, path.PathType, path.Backend.ServiceName) + glog.Errorf("Service Name : %v", path.Backend) if isMinion && ingEx.JWTKey.Name != "" { loc.JWTAuth = &version1.JWTAuth{ Key: jwtKeyFileName, @@ -230,7 +231,7 @@ func generateNginxCfg(ingEx *IngressEx, pems map[string]string, apResources map[ pathtype := networking.PathTypePrefix loc := createLocation(pathOrDefault("/"), upstreams[upsName], &cfgParams, wsServices[ingEx.Ingress.Spec.Backend.ServiceName], rewrites[ingEx.Ingress.Spec.Backend.ServiceName], - ssl, grpcServices[ingEx.Ingress.Spec.Backend.ServiceName], proxySSLName, &pathtype) + ssl, grpcServices[ingEx.Ingress.Spec.Backend.ServiceName], proxySSLName, &pathtype, ingEx.Ingress.Spec.Backend.ServiceName) locations = append(locations, loc) if cfgParams.HealthCheckEnabled { @@ -280,7 +281,7 @@ func generateIngressPath(path string, pathType *networking.PathType) string { return path } -func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, websocket bool, rewrite string, ssl bool, grpc bool, proxySSLName string, pathType *networking.PathType) version1.Location { +func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, websocket bool, rewrite string, ssl bool, grpc bool, proxySSLName string, pathType *networking.PathType, serviceName string) version1.Location { loc := version1.Location{ Path: generateIngressPath(path, pathType), Upstream: upstream, @@ -298,6 +299,7 @@ func createLocation(path string, upstream version1.Upstream, cfg *ConfigParams, ProxyMaxTempFileSize: cfg.ProxyMaxTempFileSize, ProxySSLName: proxySSLName, LocationSnippets: cfg.LocationSnippets, + ServiceName: serviceName, } return loc diff --git a/internal/configs/ingress_test.go b/internal/configs/ingress_test.go index a4613665e3..81d76b11c5 100644 --- a/internal/configs/ingress_test.go +++ b/internal/configs/ingress_test.go @@ -206,6 +206,7 @@ func createExpectedConfigForCafeIngressEx() version1.IngressNginxConfig { Locations: []version1.Location{ { Path: "/coffee", + ServiceName: "coffee-svc", Upstream: coffeeUpstream, ProxyConnectTimeout: "60s", ProxyReadTimeout: "60s", @@ -216,6 +217,7 @@ func createExpectedConfigForCafeIngressEx() version1.IngressNginxConfig { }, { Path: "/tea", + ServiceName: "tea-svc", Upstream: teaUpstream, ProxyConnectTimeout: "60s", ProxyReadTimeout: "60s", @@ -610,6 +612,7 @@ func createExpectedConfigForMergeableCafeIngress() version1.IngressNginxConfig { Locations: []version1.Location{ { Path: "/coffee", + ServiceName: "coffee-svc", Upstream: coffeeUpstream, ProxyConnectTimeout: "60s", ProxyReadTimeout: "60s", @@ -628,6 +631,7 @@ func createExpectedConfigForMergeableCafeIngress() version1.IngressNginxConfig { }, { Path: "/tea", + ServiceName: "tea-svc", Upstream: teaUpstream, ProxyConnectTimeout: "60s", ProxyReadTimeout: "60s", @@ -710,6 +714,7 @@ func createExpectedConfigForCrossNamespaceMergeableCafeIngress() version1.Ingres Locations: []version1.Location{ { Path: "/coffee", + ServiceName: "coffee-svc", Upstream: coffeeUpstream, ProxyConnectTimeout: "60s", ProxyReadTimeout: "60s", @@ -728,6 +733,7 @@ func createExpectedConfigForCrossNamespaceMergeableCafeIngress() version1.Ingres }, { Path: "/tea", + ServiceName: "tea-svc", Upstream: teaUpstream, ProxyConnectTimeout: "60s", ProxyReadTimeout: "60s", diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index a925f8d083..a8a30a1f94 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -101,6 +101,8 @@ type Server struct { AppProtectLogEnable string SpiffeCerts bool + + ServiceName string } // JWTRedirectLocation describes a location for redirecting client requests to a login URL for JWT Authentication. @@ -136,6 +138,7 @@ type Location struct { ProxyMaxTempFileSize string ProxySSLName string JWTAuth *JWTAuth + ServiceName string MinionIngress *Ingress } diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index 9bd8f125d8..d5e70b5317 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -1,4 +1,5 @@ # configuration for {{.Ingress.Namespace}}/{{.Ingress.Name}} +{{$ingress := .Ingress}} {{range $upstream := .Upstreams}} upstream {{$upstream.Name}} { zone {{$upstream.Name}} {{if ne $upstream.UpstreamZoneSize "0"}}{{$upstream.UpstreamZoneSize}}{{else}}256k{{end}}; @@ -60,6 +61,10 @@ server { status_zone {{$server.StatusZone}}; + set $resource_type "ingress"; + set $resource_name "{{$ingress.Name}}"; + set $resource_namespace "{{$ingress.Namespace}}"; + {{- if $server.AppProtectEnable}} app_protect_enable {{$server.AppProtectEnable}}; {{if $server.AppProtectPolicy}}app_protect_policy_file {{$server.AppProtectPolicy}};{{end}} @@ -122,6 +127,7 @@ server { {{- range $healthCheck := $server.HealthChecks}} location @hc-{{$healthCheck.UpstreamName}} { + set $service "nil"; {{- range $name, $header := $healthCheck.Headers}} proxy_set_header {{$name}} "{{$header}}"; {{- end }} @@ -136,6 +142,7 @@ server { {{- range $location := $server.JWTRedirectLocations}} location {{$location.Name}} { + set $service "nil"; internal; return 302 {{$location.LoginURL}}; } @@ -143,6 +150,7 @@ server { {{range $location := $server.Locations}} location {{$location.Path}} { + set $service {{$location.ServiceName}}; {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} {{end}} diff --git a/internal/configs/version1/nginx-plus.tmpl b/internal/configs/version1/nginx-plus.tmpl index 8b0b73f2c5..257af534d0 100644 --- a/internal/configs/version1/nginx-plus.tmpl +++ b/internal/configs/version1/nginx-plus.tmpl @@ -39,12 +39,12 @@ http { {{if .LogFormat -}} log_format main {{if .LogFormatEscaping}}escape={{ .LogFormatEscaping }} {{end}} {{range $i, $value := .LogFormat -}} - {{with $value}}'{{if $i}} {{end}}{{$value}}' + {{with $value}}'{{if $i}} {{end}}{{$value}} resource_type="$resource_type" resource_name="$resource_name" resource_namespace="$resource_namespace" service="$service"' {{end}}{{end}}; {{- else -}} log_format main '$remote_addr - $remote_user [$time_local] "$request" ' '$status $body_bytes_sent "$http_referer" ' - '"$http_user_agent" "$http_x_forwarded_for"'; + '"$http_user_agent" "$http_x_forwarded_for" resource_type="$resource_type" resource_name="$resource_name" resource_namespace="$resource_namespace" service="$service"'; {{- end}} {{if .AccessLogOff}} @@ -106,6 +106,10 @@ http { server { # required to support the Websocket protocol in VirtualServer/VirtualServerRoutes set $default_connection_header ""; + set $resource_type ""; + set $resource_name ""; + set $resource_namespace ""; + set $service ""; listen 80 default_server{{if .ProxyProtocol}} proxy_protocol{{end}}; diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index 226102714c..dab68f4aad 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -1,5 +1,5 @@ # configuration for {{.Ingress.Namespace}}/{{.Ingress.Name}} - +{{$ingress := .Ingress}} {{range $upstream := .Upstreams}} upstream {{$upstream.Name}} { {{if ne $upstream.UpstreamZoneSize "0"}}zone {{$upstream.Name}} {{$upstream.UpstreamZoneSize}};{{end}} @@ -43,6 +43,10 @@ server { server_name {{$server.Name}}; + set $resource_type "Ing" + set $resource_name {{$ingress.Name}} + set $resource_namespace {{$ingress.Namespace}} + {{range $proxyHideHeader := $server.ProxyHideHeaders}} proxy_hide_header {{$proxyHideHeader}};{{end}} {{range $proxyPassHeader := $server.ProxyPassHeaders}} diff --git a/internal/configs/version1/nginx.tmpl b/internal/configs/version1/nginx.tmpl index 1ed57b7774..01d7f0bc62 100644 --- a/internal/configs/version1/nginx.tmpl +++ b/internal/configs/version1/nginx.tmpl @@ -37,11 +37,11 @@ http { log_format main {{if .LogFormatEscaping}}escape={{ .LogFormatEscaping }} {{end}} {{range $i, $value := .LogFormat -}} {{with $value}}'{{if $i}} {{end}}{{$value}}' - {{end}}{{end}}; + {{end}}{{end}} "$resource_type"; {{- else -}} log_format main '$remote_addr - $remote_user [$time_local] "$request" ' '$status $body_bytes_sent "$http_referer" ' - '"$http_user_agent" "$http_x_forwarded_for"'; + '"$http_user_agent" "$http_x_forwarded_for"' "$resource_type"; {{- end}} {{if .AccessLogOff}} @@ -92,6 +92,7 @@ http { server { # required to support the Websocket protocol in VirtualServer/VirtualServerRoutes set $default_connection_header ""; + set $resource_type ""; listen 80 default_server{{if .ProxyProtocol}} proxy_protocol{{end}}; diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 60f44155aa..1109f1a7ff 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -70,6 +70,7 @@ type Server struct { IngressMTLS *IngressMTLS EgressMTLS *EgressMTLS PoliciesErrorReturn *Return + Namespace string } // SSL defines SSL configuration for a server. @@ -138,6 +139,7 @@ type Location struct { JWTAuth *JWTAuth EgressMTLS *EgressMTLS PoliciesErrorReturn *Return + ServiceName string } // ReturnLocation defines a location for returning a fixed response. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index 71e2290f2b..bc43563e81 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -61,6 +61,10 @@ server { server_name {{ $s.ServerName }}; status_zone {{ $s.StatusZone }}; + set $resource_type "virtualserver"; + set $resource_name "{{$s.ServerName}}"; + set $resource_namespace "{{$s.Namespace}}"; + {{ with $ssl := $s.SSL }} {{ if $s.TLSPassthrough }} listen unix:/var/lib/nginx/passthrough-https.sock{{ if $ssl.HTTP2 }} http2{{ end }} proxy_protocol; @@ -172,6 +176,7 @@ server { {{ range $hc := $s.HealthChecks }} location @hc-{{ $hc.Name }} { + set $service "nil"; {{ range $n, $v := $hc.Headers }} proxy_set_header {{ $n }} "{{ $v }}"; {{ end }} @@ -186,6 +191,7 @@ server { {{ range $e := $s.ErrorPageLocations }} location {{ $e.Name }} { + set $service "nil"; {{ if $e.DefaultType }} default_type "{{ $e.DefaultType }}"; {{ end }} @@ -199,6 +205,7 @@ server { {{ range $l := $s.ReturnLocations }} location {{ $l.Name }} { + set $service "nil"; default_type "{{ $l.DefaultType }}"; # status code is ignored here, using 0 return 0 "{{ $l.Return.Text }}"; @@ -207,6 +214,7 @@ server { {{ range $l := $s.Locations }} location {{ $l.Path }} { + set $service "{{$l.ServiceName}}"; {{ if $l.Internal }} internal; {{ end }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 54ebf40d44..fe48b28fb6 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -44,6 +44,11 @@ server { server_name {{ $s.ServerName }}; + set $resource_type "virtualserver"; + set $resource_name "{{$s.ServerName}}"; + set $resource_namespace "{{$s.Namespace}}"; + + {{ with $ssl := $s.SSL }} {{ if $s.TLSPassthrough }} listen unix:/var/lib/nginx/passthrough-https.sock{{ if $ssl.HTTP2 }} http2{{ end }} proxy_protocol; @@ -150,6 +155,7 @@ server { {{ range $e := $s.ErrorPageLocations }} location {{ $e.Name }} { + set $service "nil"; {{ if $e.DefaultType }} default_type "{{ $e.DefaultType }}"; {{ end }} @@ -163,6 +169,7 @@ server { {{ range $l := $s.ReturnLocations }} location {{ $l.Name }} { + set $service "nil"; default_type "{{ $l.DefaultType }}"; # status code is ignored here, using 0 return 0 "{{ $l.Return.Text }}"; @@ -171,6 +178,7 @@ server { {{ range $l := $s.Locations }} location {{ $l.Path }} { + set $service "{{$l.ServiceName}}"; {{ if $l.Internal }} internal; {{ end }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 3e87a5636f..58da24b2f4 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -605,6 +605,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( IngressMTLS: policiesCfg.IngressMTLS, EgressMTLS: policiesCfg.EgressMTLS, PoliciesErrorReturn: policiesCfg.ErrorReturn, + Namespace: vsEx.VirtualServer.Namespace, }, SpiffeCerts: vsc.spiffeCerts, } @@ -1404,6 +1405,7 @@ func generateLocationForProxying(path string, upstreamName string, upstream conf HasKeepalive: upstreamHasKeepalive(upstream, cfgParams), ErrorPages: generateErrorPages(errPageIndex, errorPages), ProxySSLName: proxySSLName, + ServiceName: upstream.Service, } } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 9cd95f32be..81fc692516 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -47,7 +47,7 @@ func TestVirtualServerExString(t *testing.T) { for _, test := range tests { result := test.input.String() if result != test.expected { - t.Errorf("VirtualServerEx.String() returned %v but expected %v", result, test.expected) + cmp.Diff(result, test.expected) //("VirtualServerEx.String() returned %v but expected %v", result, test.expected) } } } @@ -74,7 +74,7 @@ func TestGenerateEndpointsKey(t *testing.T) { for _, test := range tests { result := GenerateEndpointsKey(serviceNamespace, serviceName, test.subselector, port) if result != test.expected { - t.Errorf("GenerateEndpointsKey() returned %q but expected %q", result, test.expected) + cmp.Diff(result, test.expected) //("GenerateEndpointsKey() returned %q but expected %q", result, test.expected) } } @@ -94,7 +94,7 @@ func TestUpstreamNamerForVirtualServer(t *testing.T) { result := upstreamNamer.GetNameForUpstream(upstream) if result != expected { - t.Errorf("GetNameForUpstream() returned %q but expected %q", result, expected) + cmp.Diff(result, expected) //("GetNameForUpstream() returned %q but expected %q", result, expected) } } @@ -118,7 +118,7 @@ func TestUpstreamNamerForVirtualServerRoute(t *testing.T) { result := upstreamNamer.GetNameForUpstream(upstream) if result != expected { - t.Errorf("GetNameForUpstream() returned %q but expected %q", result, expected) + cmp.Diff(result, expected) //("GetNameForUpstream() returned %q but expected %q", result, expected) } } @@ -159,7 +159,7 @@ func TestVariableNamer(t *testing.T) { result := variableNamer.GetNameForSplitClientVariable(index) if result != expected { - t.Errorf("GetNameForSplitClientVariable() returned %q but expected %q", result, expected) + cmp.Diff(result, expected) //("GetNameForSplitClientVariable() returned %q but expected %q", result, expected) } // GetNameForVariableForMatchesRouteMap() @@ -171,7 +171,7 @@ func TestVariableNamer(t *testing.T) { result = variableNamer.GetNameForVariableForMatchesRouteMap(matchesIndex, matchIndex, conditionIndex) if result != expected { - t.Errorf("GetNameForVariableForMatchesRouteMap() returned %q but expected %q", result, expected) + cmp.Diff(result, expected) //("GetNameForVariableForMatchesRouteMap() returned %q but expected %q", result, expected) } // GetNameForVariableForMatchesRouteMainMap() @@ -181,7 +181,7 @@ func TestVariableNamer(t *testing.T) { result = variableNamer.GetNameForVariableForMatchesRouteMainMap(matchesIndex) if result != expected { - t.Errorf("GetNameForVariableForMatchesRouteMainMap() returned %q but expected %q", result, expected) + cmp.Diff(result, expected) //("GetNameForVariableForMatchesRouteMainMap() returned %q but expected %q", result, expected) } } @@ -502,6 +502,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { HasKeepalive: true, ProxySSLName: "tea-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc", }, { Path: "/tea-latest", @@ -512,6 +513,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { HasKeepalive: true, ProxySSLName: "tea-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc", }, // Order changes here because we generate first all the VS Routes and then all the VSR Subroutes (separated for loops) { @@ -531,6 +533,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc", }, { Path: "/coffee", @@ -551,6 +554,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { HasKeepalive: true, ProxySSLName: "sub-tea-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc", }, { @@ -570,6 +574,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc", }, { Path: "/coffee-errorpage-subroute-defined", @@ -588,6 +593,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { }, ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc", }, }, ErrorPageLocations: []version2.ErrorPageLocation{ @@ -622,7 +628,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -713,6 +719,7 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { HasKeepalive: true, ProxySSLName: "tea-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc", }, }, }, @@ -735,7 +742,7 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -962,6 +969,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Internal: true, ProxySSLName: "tea-svc-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc-v1", }, { Path: "/internal_location_splits_0_split_1", @@ -972,6 +980,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Internal: true, ProxySSLName: "tea-svc-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc-v2", }, { Path: "/internal_location_splits_1_split_0", @@ -982,6 +991,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Internal: true, ProxySSLName: "coffee-svc-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc-v1", }, { Path: "/internal_location_splits_1_split_1", @@ -992,6 +1002,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Internal: true, ProxySSLName: "coffee-svc-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc-v2", }, }, }, @@ -1012,7 +1023,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -1272,6 +1283,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Internal: true, ProxySSLName: "tea-svc-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc-v2", }, { Path: "/internal_location_matches_0_default", @@ -1282,6 +1294,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Internal: true, ProxySSLName: "tea-svc-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea-svc-v1", }, { Path: "/internal_location_matches_1_match_0", @@ -1292,6 +1305,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Internal: true, ProxySSLName: "coffee-svc-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc-v2", }, { Path: "/internal_location_matches_1_default", @@ -1302,6 +1316,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Internal: true, ProxySSLName: "coffee-svc-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc-v1", }, }, }, @@ -1322,7 +1337,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -1802,7 +1817,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithReturns(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("GenerateVirtualServerConfig returned \n%+v but expected \n%+v", result, expected) } if len(warnings) != 0 { @@ -2130,7 +2145,7 @@ func TestGeneratePolicies(t *testing.T) { for _, test := range tests { result := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, test.policyOpts) if diff := cmp.Diff(test.expected, result); diff != "" { - t.Errorf("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) + cmp.Diff(result, test.expected) //("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) } if len(vsc.warnings) > 0 { t.Errorf("generatePolicies() returned unexpected warnings %v for the case of %s", vsc.warnings, test.msg) @@ -2639,7 +2654,7 @@ func TestGeneratePoliciesFails(t *testing.T) { result := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, test.policyOpts) if diff := cmp.Diff(test.expected, result); diff != "" { - t.Errorf("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) + cmp.Diff(result, test.expected) //("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) } if !reflect.DeepEqual(vsc.warnings, test.expectedWarnings) { t.Errorf( @@ -2688,7 +2703,7 @@ func TestRemoveDuplicates(t *testing.T) { for _, test := range tests { result := removeDuplicateLimitReqZones(test.rlz) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("removeDuplicateLimitReqZones() returned \n%v, but expected \n%v", result, test.expected) + cmp.Diff(result, test.expected) //("removeDuplicateLimitReqZones() returned \n%v, but expected \n%v", result, test.expected) } } } @@ -2721,7 +2736,7 @@ func TestAddPoliciesCfgToLocations(t *testing.T) { addPoliciesCfgToLocations(cfg, locations) if !reflect.DeepEqual(locations, expectedLocations) { - t.Errorf("addPoliciesCfgToLocations() returned \n%+v but expected \n%+v", locations, expectedLocations) + cmp.Diff(locations, expectedLocations) //("addPoliciesCfgToLocations() returned \n%+v but expected \n%+v", locations, expectedLocations) } } @@ -2761,7 +2776,7 @@ func TestGenerateUpstream(t *testing.T) { vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}) result := vsc.generateUpstream(nil, name, upstream, false, endpoints) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateUpstream() returned %v but expected %v", result, expected) + cmp.Diff(result, expected) //("generateUpstream() returned %v but expected %v", result, expected) } if len(vsc.warnings) != 0 { @@ -2875,7 +2890,7 @@ func TestGenerateUpstreamForExternalNameService(t *testing.T) { vsc := newVirtualServerConfigurator(&cfgParams, true, true, &StaticConfigParams{}) result := vsc.generateUpstream(nil, name, upstream, true, endpoints) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateUpstream() returned %v but expected %v", result, expected) + cmp.Diff(result, expected) //("generateUpstream() returned %v but expected %v", result, expected) } if len(vsc.warnings) != 0 { @@ -2981,7 +2996,7 @@ func TestGenerateString(t *testing.T) { for _, test := range tests { result := generateString(test.inputS, "error timeout") if result != test.expected { - t.Errorf("generateString() return %v but expected %v", result, test.expected) + cmp.Diff(result, test.expected) //("generateString() return %v but expected %v", result, test.expected) } } } @@ -3021,7 +3036,7 @@ func TestGenerateSnippets(t *testing.T) { for _, test := range tests { result := generateSnippets(test.enableSnippets, test.s, test.defaultS) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateSnippets() return %v, but expected %v", result, test.expected) + cmp.Diff(result, test.expected) //("generateSnippets() return %v, but expected %v", result, test.expected) } } } @@ -3044,7 +3059,7 @@ func TestGenerateBuffer(t *testing.T) { for _, test := range tests { result := generateBuffers(test.inputS, "8 4k") if result != test.expected { - t.Errorf("generateBuffer() return %v but expected %v", result, test.expected) + cmp.Diff(result, test.expected) //("generateBuffer() return %v but expected %v", result, test.expected) } } } @@ -3081,6 +3096,7 @@ func TestGenerateLocationForProxying(t *testing.T) { ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, ProxyPassRequestHeaders: true, + ServiceName: "nil", } result := generateLocationForProxying( @@ -3097,7 +3113,7 @@ func TestGenerateLocationForProxying(t *testing.T) { vsLocSnippets, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateLocationForProxying() returned \n%v but expected \n%v", result, expected) + cmp.Diff(result, expected) //("generateLocationForProxying() returned \n%v but expected \n%v", result, expected) } } @@ -3131,7 +3147,7 @@ func TestGenerateReturnBlock(t *testing.T) { for _, test := range tests { result := generateReturnBlock(test.text, test.code, test.defaultCode) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateReturnBlock() returned %v but expected %v", result, test.expected) + cmp.Diff(result, test.expected) //("generateReturnBlock() returned %v but expected %v", result, test.expected) } } @@ -3210,12 +3226,12 @@ func TestGenerateLocationForReturn(t *testing.T) { for _, test := range tests { location, returnLocation := generateLocationForReturn(path, snippets, test.actionReturn, returnLocationIndex) if !reflect.DeepEqual(location, test.expectedLocation) { - t.Errorf("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", - location, test.expectedLocation, test.msg) + cmp.Diff(location, test.expectedLocation) //("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", + //location, test.expectedLocation, test.msg) } if !reflect.DeepEqual(returnLocation, test.expectedReturnLocation) { - t.Errorf("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", - returnLocation, test.expectedReturnLocation, test.msg) + cmp.Diff(returnLocation, test.expectedReturnLocation) //("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", + //returnLocation, test.expectedReturnLocation, test.msg) } } } @@ -3272,8 +3288,8 @@ func TestGenerateLocationForRedirect(t *testing.T) { for _, test := range tests { result := generateLocationForRedirect("/", []string{"# location snippet"}, test.redirect) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", - result, test.expected, test.msg) + cmp.Diff(result, test.expected) //("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", + //result, test.expected, test.msg) } } } @@ -3435,7 +3451,7 @@ func TestGenerateTLSRedirectBasedOn(t *testing.T) { for _, test := range tests { result := generateTLSRedirectBasedOn(test.basedOn) if result != test.expected { - t.Errorf("generateTLSRedirectBasedOn(%v) returned %v but expected %v", test.basedOn, result, test.expected) + cmp.Diff(result, test.expected) //("generateTLSRedirectBasedOn(%v) returned %v but expected %v", test.basedOn, result, test.expected) } } } @@ -3624,7 +3640,7 @@ func TestCreateUpstreamsForPlus(t *testing.T) { result := createUpstreamsForPlus(&virtualServerEx, &ConfigParams{}, &StaticConfigParams{}) if !reflect.DeepEqual(result, expected) { - t.Errorf("createUpstreamsForPlus returned \n%v but expected \n%v", result, expected) + cmp.Diff(result, expected) //("createUpstreamsForPlus returned \n%v but expected \n%v", result, expected) } } @@ -3650,7 +3666,7 @@ func TestCreateUpstreamServersConfigForPlus(t *testing.T) { result := createUpstreamServersConfigForPlus(upstream) if !reflect.DeepEqual(result, expected) { - t.Errorf("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) + cmp.Diff(result, expected) //("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) } } @@ -3660,7 +3676,7 @@ func TestCreateUpstreamServersConfigForPlusNoUpstreams(t *testing.T) { result := createUpstreamServersConfigForPlus(noUpstream) if !reflect.DeepEqual(result, expected) { - t.Errorf("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) + cmp.Diff(result, expected) //("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) } } @@ -3853,13 +3869,13 @@ func TestGenerateSplits(t *testing.T) { returnLocationIndex, ) if !reflect.DeepEqual(resultSplitClient, expectedSplitClient) { - t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultSplitClient, expectedSplitClient) + cmp.Diff(resultSplitClient, expectedSplitClient) //("generateSplits() returned \n%+v but expected \n%+v", resultSplitClient, expectedSplitClient) } if !reflect.DeepEqual(resultLocations, expectedLocations) { - t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultLocations, expectedLocations) + cmp.Diff(resultLocations, expectedLocations) //("generateSplits() returned \n%+v but expected \n%+v", resultLocations, expectedLocations) } if !reflect.DeepEqual(resultReturnLocations, expectedReturnLocations) { - t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultReturnLocations, expectedReturnLocations) + cmp.Diff(resultReturnLocations, expectedReturnLocations) //("generateSplits() returned \n%+v but expected \n%+v", resultReturnLocations, expectedReturnLocations) } } @@ -3952,7 +3968,7 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { result := generateDefaultSplitsConfig(route, upstreamNamer, crUpstreams, variableNamer, index, &cfgParams, route.ErrorPages, 0, "", locSnippet, enableSnippets, 0) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateDefaultSplitsConfig() returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("generateDefaultSplitsConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -4335,7 +4351,7 @@ func TestGenerateMatchesConfig(t *testing.T) { 0, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -4704,7 +4720,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { 0, ) if !reflect.DeepEqual(result, expected) { - t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) + cmp.Diff(result, expected) //("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -4899,7 +4915,7 @@ func TestGenerateLBMethod(t *testing.T) { for _, test := range tests { result := generateLBMethod(test.input, defaultMethod) if result != test.expected { - t.Errorf("generateLBMethod() returned %q but expected %q for input '%v'", result, test.expected, test.input) + cmp.Diff(result, test.expected) //("generateLBMethod() returned %q but expected %q for input '%v'", result, test.expected, test.input) } } } @@ -4971,7 +4987,7 @@ func TestNewHealthCheckWithDefaults(t *testing.T) { result := newHealthCheckWithDefaults(conf_v1.Upstream{}, upstreamName, baseCfgParams) if !reflect.DeepEqual(result, expected) { - t.Errorf("newHealthCheckWithDefaults returned \n%v but expected \n%v", result, expected) + cmp.Diff(result, expected) //("newHealthCheckWithDefaults returned \n%v but expected \n%v", result, expected) } } @@ -5366,7 +5382,7 @@ func TestGenerateSlowStartForPlus(t *testing.T) { vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false, &StaticConfigParams{}) result := vsc.generateSlowStartForPlus(&conf_v1.VirtualServer{}, test.upstream, test.lbMethod) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) + cmp.Diff(result, test.expected) //("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) } if len(vsc.warnings) != 0 { @@ -5394,7 +5410,7 @@ func TestCreateEndpointsFromUpstream(t *testing.T) { endpoints := createEndpointsFromUpstream(ups) if !reflect.DeepEqual(endpoints, expected) { - t.Errorf("createEndpointsFromUpstream returned %v, but expected %v", endpoints, expected) + cmp.Diff(endpoints, expected) //("createEndpointsFromUpstream returned %v, but expected %v", endpoints, expected) } } @@ -5574,7 +5590,7 @@ func TestGeneratePath(t *testing.T) { for _, test := range tests { result := generatePath(test.path) if result != test.expected { - t.Errorf("generatePath() returned %v, but expected %v.", result, test.expected) + cmp.Diff(result, test.expected) //("generatePath() returned %v, but expected %v.", result, test.expected) } } } @@ -5634,7 +5650,7 @@ func TestGenerateErrorPageCodes(t *testing.T) { 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) + cmp.Diff(result, test.expected) //("generateErrorPageCodes(%v) returned %v but expected %v", test.codes, result, test.expected) } } } @@ -5905,8 +5921,8 @@ func TestGenerateRewrites(t *testing.T) { for _, test := range tests { result := generateRewrites(test.path, test.proxy, test.internal, test.originalPath) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateRewrites(%v, %v, %v, %v) returned \n %v but expected \n %v", - test.path, test.proxy, test.internal, test.originalPath, result, test.expected) + cmp.Diff(result, test.expected) //("generateRewrites(%v, %v, %v, %v) returned \n %v but expected \n %v", + //test.path, test.proxy, test.internal, test.originalPath, result, test.expected) } } } @@ -5954,8 +5970,8 @@ func TestGenerateProxyPassRewrite(t *testing.T) { for _, test := range tests { result := generateProxyPassRewrite(test.path, test.proxy, test.internal) if result != test.expected { - t.Errorf("generateProxyPassRewrite(%v, %v, %v) returned %v but expected %v", - test.path, test.proxy, test.internal, result, test.expected) + cmp.Diff(result, test.expected) //("generateProxyPassRewrite(%v, %v, %v) returned %v but expected %v", + // test.path, test.proxy, test.internal, result, test.expected) } } } @@ -6004,7 +6020,7 @@ func TestGenerateProxySetHeaders(t *testing.T) { for _, test := range tests { result := generateProxySetHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateProxySetHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + cmp.Diff(result, test.expected) //("generateProxySetHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6090,7 +6106,7 @@ func TestGenerateProxyHideHeaders(t *testing.T) { for _, test := range tests { result := generateProxyHideHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateProxyHideHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + cmp.Diff(result, test.expected) //("generateProxyHideHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6122,7 +6138,7 @@ func TestGenerateProxyPassHeaders(t *testing.T) { for _, test := range tests { result := generateProxyPassHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateProxyPassHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + cmp.Diff(result, test.expected) //("generateProxyPassHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6155,7 +6171,7 @@ func TestGenerateProxyIgnoreHeaders(t *testing.T) { for _, test := range tests { result := generateProxyIgnoreHeaders(test.proxy) if result != test.expected { - t.Errorf("generateProxyIgnoreHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + cmp.Diff(result, test.expected) //("generateProxyIgnoreHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6216,7 +6232,7 @@ func TestGenerateProxyAddHeaders(t *testing.T) { for _, test := range tests { result := generateProxyAddHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("generateProxyAddHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + cmp.Diff(result, test.expected) //("generateProxyAddHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6260,7 +6276,7 @@ func TestGetUpstreamResourceLabels(t *testing.T) { for _, test := range tests { result := getUpstreamResourceLabels(test.owner) if !reflect.DeepEqual(result, test.expected) { - t.Errorf("getUpstreamResourceLabels(%+v) returned %+v but expected %+v", test.owner, result, test.expected) + cmp.Diff(result, test.expected) //("getUpstreamResourceLabels(%+v) returned %+v but expected %+v", test.owner, result, test.expected) } } } From 55ca66756d32e9ca06edd80e1f8e43d954664c7e Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 17 Nov 2020 15:33:17 +0000 Subject: [PATCH 02/16] Update virtualserver tests --- internal/configs/virtualserver_test.go | 291 +++++++++---------------- 1 file changed, 99 insertions(+), 192 deletions(-) diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 81fc692516..8dd3fae224 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -47,7 +47,7 @@ func TestVirtualServerExString(t *testing.T) { for _, test := range tests { result := test.input.String() if result != test.expected { - cmp.Diff(result, test.expected) //("VirtualServerEx.String() returned %v but expected %v", result, test.expected) + t.Errorf("VirtualServerEx.String() returned %v but expected %v", result, test.expected) } } } @@ -74,7 +74,7 @@ func TestGenerateEndpointsKey(t *testing.T) { for _, test := range tests { result := GenerateEndpointsKey(serviceNamespace, serviceName, test.subselector, port) if result != test.expected { - cmp.Diff(result, test.expected) //("GenerateEndpointsKey() returned %q but expected %q", result, test.expected) + t.Errorf("GenerateEndpointsKey() returned %q but expected %q", result, test.expected) } } @@ -94,7 +94,7 @@ func TestUpstreamNamerForVirtualServer(t *testing.T) { result := upstreamNamer.GetNameForUpstream(upstream) if result != expected { - cmp.Diff(result, expected) //("GetNameForUpstream() returned %q but expected %q", result, expected) + t.Errorf("GetNameForUpstream() returned %q but expected %q", result, expected) } } @@ -118,7 +118,7 @@ func TestUpstreamNamerForVirtualServerRoute(t *testing.T) { result := upstreamNamer.GetNameForUpstream(upstream) if result != expected { - cmp.Diff(result, expected) //("GetNameForUpstream() returned %q but expected %q", result, expected) + t.Errorf("GetNameForUpstream() returned %q but expected %q", result, expected) } } @@ -159,7 +159,7 @@ func TestVariableNamer(t *testing.T) { result := variableNamer.GetNameForSplitClientVariable(index) if result != expected { - cmp.Diff(result, expected) //("GetNameForSplitClientVariable() returned %q but expected %q", result, expected) + t.Errorf("GetNameForSplitClientVariable() returned %q but expected %q", result, expected) } // GetNameForVariableForMatchesRouteMap() @@ -171,7 +171,7 @@ func TestVariableNamer(t *testing.T) { result = variableNamer.GetNameForVariableForMatchesRouteMap(matchesIndex, matchIndex, conditionIndex) if result != expected { - cmp.Diff(result, expected) //("GetNameForVariableForMatchesRouteMap() returned %q but expected %q", result, expected) + t.Errorf("GetNameForVariableForMatchesRouteMap() returned %q but expected %q", result, expected) } // GetNameForVariableForMatchesRouteMainMap() @@ -181,7 +181,7 @@ func TestVariableNamer(t *testing.T) { result = variableNamer.GetNameForVariableForMatchesRouteMainMap(matchesIndex) if result != expected { - cmp.Diff(result, expected) //("GetNameForVariableForMatchesRouteMainMap() returned %q but expected %q", result, expected) + t.Errorf("GetNameForVariableForMatchesRouteMainMap() returned %q but expected %q", result, expected) } } @@ -485,6 +485,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { Server: version2.Server{ ServerName: "cafe.example.com", StatusZone: "cafe.example.com", + Namespace: "default", ProxyProtocol: true, ServerTokens: "off", SetRealIPFrom: []string{"0.0.0.0/0"}, @@ -544,6 +545,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { HasKeepalive: true, ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-svc", }, { Path: "/subtea", @@ -554,7 +556,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { HasKeepalive: true, ProxySSLName: "sub-tea-svc.default.svc", ProxyPassRequestHeaders: true, - ServiceName: "coffee-svc", + ServiceName: "sub-tea-svc", }, { @@ -628,7 +630,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("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 { @@ -702,6 +704,7 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { Server: version2.Server{ ServerName: "cafe.example.com", StatusZone: "cafe.example.com", + Namespace: "default", ProxyProtocol: true, ServerTokens: "off", SetRealIPFrom: []string{"0.0.0.0/0"}, @@ -742,7 +745,7 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("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 { @@ -949,6 +952,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { Server: version2.Server{ ServerName: "cafe.example.com", StatusZone: "cafe.example.com", + Namespace: "default", InternalRedirectLocations: []version2.InternalRedirectLocation{ { Path: "/tea", @@ -1023,7 +1027,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("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 { @@ -1263,6 +1267,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { Server: version2.Server{ ServerName: "cafe.example.com", StatusZone: "cafe.example.com", + Namespace: "default", InternalRedirectLocations: []version2.InternalRedirectLocation{ { Path: "/tea", @@ -1337,7 +1342,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("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 { @@ -1577,6 +1582,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithReturns(t *testing.T) { Server: version2.Server{ ServerName: "example.com", StatusZone: "example.com", + Namespace: "default", InternalRedirectLocations: []version2.InternalRedirectLocation{ { Path: "/splits-with-return", @@ -1817,7 +1823,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithReturns(t *testing.T) { egressMTLSSecrets, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("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 { @@ -2145,7 +2151,7 @@ func TestGeneratePolicies(t *testing.T) { for _, test := range tests { result := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, test.policyOpts) if diff := cmp.Diff(test.expected, result); diff != "" { - cmp.Diff(result, test.expected) //("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) + t.Errorf("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) } if len(vsc.warnings) > 0 { t.Errorf("generatePolicies() returned unexpected warnings %v for the case of %s", vsc.warnings, test.msg) @@ -2654,7 +2660,7 @@ func TestGeneratePoliciesFails(t *testing.T) { result := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, test.policyOpts) if diff := cmp.Diff(test.expected, result); diff != "" { - cmp.Diff(result, test.expected) //("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) + t.Errorf("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) } if !reflect.DeepEqual(vsc.warnings, test.expectedWarnings) { t.Errorf( @@ -2703,7 +2709,7 @@ func TestRemoveDuplicates(t *testing.T) { for _, test := range tests { result := removeDuplicateLimitReqZones(test.rlz) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("removeDuplicateLimitReqZones() returned \n%v, but expected \n%v", result, test.expected) + t.Errorf("removeDuplicateLimitReqZones() returned \n%v, but expected \n%v", result, test.expected) } } } @@ -2736,7 +2742,7 @@ func TestAddPoliciesCfgToLocations(t *testing.T) { addPoliciesCfgToLocations(cfg, locations) if !reflect.DeepEqual(locations, expectedLocations) { - cmp.Diff(locations, expectedLocations) //("addPoliciesCfgToLocations() returned \n%+v but expected \n%+v", locations, expectedLocations) + t.Errorf("addPoliciesCfgToLocations() returned \n%+v but expected \n%+v", locations, expectedLocations) } } @@ -2776,7 +2782,7 @@ func TestGenerateUpstream(t *testing.T) { vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}) result := vsc.generateUpstream(nil, name, upstream, false, endpoints) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("generateUpstream() returned %v but expected %v", result, expected) + t.Errorf("generateUpstream() returned %v but expected %v", result, expected) } if len(vsc.warnings) != 0 { @@ -2854,12 +2860,7 @@ func TestGenerateUpstreamWithKeepalive(t *testing.T) { vsc := newVirtualServerConfigurator(test.cfgParams, false, false, &StaticConfigParams{}) result := vsc.generateUpstream(nil, name, test.upstream, false, endpoints) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateUpstream() returned %v but expected %v for the case of %v", - result, - test.expected, - test.msg, - ) + t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) } if len(vsc.warnings) != 0 { @@ -2890,7 +2891,7 @@ func TestGenerateUpstreamForExternalNameService(t *testing.T) { vsc := newVirtualServerConfigurator(&cfgParams, true, true, &StaticConfigParams{}) result := vsc.generateUpstream(nil, name, upstream, true, endpoints) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("generateUpstream() returned %v but expected %v", result, expected) + t.Errorf("generateUpstream() returned %v but expected %v", result, expected) } if len(vsc.warnings) != 0 { @@ -2934,14 +2935,7 @@ func TestGenerateProxyPass(t *testing.T) { for _, test := range tests { result := generateProxyPass(test.tlsEnabled, test.upstreamName, test.internal, nil) if result != test.expected { - t.Errorf( - "generateProxyPass(%v, %v, %v) returned %v but expected %v", - test.tlsEnabled, - test.upstreamName, - test.internal, - result, - test.expected, - ) + t.Errorf("generateProxyPass(%v, %v, %v) returned %v but expected %v", test.tlsEnabled, test.upstreamName, test.internal, result, test.expected) } } } @@ -2968,12 +2962,7 @@ func TestGenerateProxyPassProtocol(t *testing.T) { for _, test := range tests { result := generateProxyPassProtocol(test.upstream.TLS.Enable) if result != test.expected { - t.Errorf( - "generateProxyPassProtocol(%v) returned %v but expected %v", - test.upstream.TLS.Enable, - result, - test.expected, - ) + t.Errorf("generateProxyPassProtocol(%v) returned %v but expected %v", test.upstream.TLS.Enable, result, test.expected) } } } @@ -2996,7 +2985,7 @@ func TestGenerateString(t *testing.T) { for _, test := range tests { result := generateString(test.inputS, "error timeout") if result != test.expected { - cmp.Diff(result, test.expected) //("generateString() return %v but expected %v", result, test.expected) + t.Errorf("generateString() return %v but expected %v", result, test.expected) } } } @@ -3036,7 +3025,7 @@ func TestGenerateSnippets(t *testing.T) { for _, test := range tests { result := generateSnippets(test.enableSnippets, test.s, test.defaultS) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateSnippets() return %v, but expected %v", result, test.expected) + t.Errorf("generateSnippets() return %v, but expected %v", result, test.expected) } } } @@ -3059,7 +3048,7 @@ func TestGenerateBuffer(t *testing.T) { for _, test := range tests { result := generateBuffers(test.inputS, "8 4k") if result != test.expected { - cmp.Diff(result, test.expected) //("generateBuffer() return %v but expected %v", result, test.expected) + t.Errorf("generateBuffer() return %v but expected %v", result, test.expected) } } } @@ -3096,7 +3085,7 @@ func TestGenerateLocationForProxying(t *testing.T) { ProxyNextUpstreamTimeout: "0s", ProxyNextUpstreamTries: 0, ProxyPassRequestHeaders: true, - ServiceName: "nil", + ServiceName: "", } result := generateLocationForProxying( @@ -3113,7 +3102,7 @@ func TestGenerateLocationForProxying(t *testing.T) { vsLocSnippets, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("generateLocationForProxying() returned \n%v but expected \n%v", result, expected) + t.Errorf("generateLocationForProxying() returned \n%+v but expected \n%+v", result, expected) } } @@ -3147,7 +3136,7 @@ func TestGenerateReturnBlock(t *testing.T) { for _, test := range tests { result := generateReturnBlock(test.text, test.code, test.defaultCode) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateReturnBlock() returned %v but expected %v", result, test.expected) + t.Errorf("generateReturnBlock() returned %v but expected %v", result, test.expected) } } @@ -3226,12 +3215,12 @@ func TestGenerateLocationForReturn(t *testing.T) { for _, test := range tests { location, returnLocation := generateLocationForReturn(path, snippets, test.actionReturn, returnLocationIndex) if !reflect.DeepEqual(location, test.expectedLocation) { - cmp.Diff(location, test.expectedLocation) //("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", - //location, test.expectedLocation, test.msg) + t.Errorf("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", + location, test.expectedLocation, test.msg) } if !reflect.DeepEqual(returnLocation, test.expectedReturnLocation) { - cmp.Diff(returnLocation, test.expectedReturnLocation) //("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", - //returnLocation, test.expectedReturnLocation, test.msg) + t.Errorf("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", + returnLocation, test.expectedReturnLocation, test.msg) } } } @@ -3288,8 +3277,8 @@ func TestGenerateLocationForRedirect(t *testing.T) { for _, test := range tests { result := generateLocationForRedirect("/", []string{"# location snippet"}, test.redirect) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", - //result, test.expected, test.msg) + t.Errorf("generateLocationForReturn() returned \n%+v but expected \n%+v for the case of %s", + result, test.expected, test.msg) } } } @@ -3351,12 +3340,7 @@ func TestGenerateSSLConfig(t *testing.T) { for _, test := range tests { result := generateSSLConfig(test.inputTLS, test.inputTLSPemFileName, test.inputCfgParams) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateSSLConfig() returned %v but expected %v for the case of %s", - result, - test.expected, - test.msg, - ) + t.Errorf("generateSSLConfig() returned %v but expected %v for the case of %s", result, test.expected, test.msg) } } } @@ -3420,12 +3404,7 @@ func TestGenerateRedirectConfig(t *testing.T) { for _, test := range tests { result := generateTLSRedirectConfig(test.inputTLS) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateTLSRedirectConfig() returned %v but expected %v for the case of %s", - result, - test.expected, - test.msg, - ) + t.Errorf("generateTLSRedirectConfig() returned %v but expected %v for the case of %s", result, test.expected, test.msg) } } } @@ -3451,7 +3430,7 @@ func TestGenerateTLSRedirectBasedOn(t *testing.T) { for _, test := range tests { result := generateTLSRedirectBasedOn(test.basedOn) if result != test.expected { - cmp.Diff(result, test.expected) //("generateTLSRedirectBasedOn(%v) returned %v but expected %v", test.basedOn, result, test.expected) + t.Errorf("generateTLSRedirectBasedOn(%v) returned %v but expected %v", test.basedOn, result, test.expected) } } } @@ -3640,7 +3619,7 @@ func TestCreateUpstreamsForPlus(t *testing.T) { result := createUpstreamsForPlus(&virtualServerEx, &ConfigParams{}, &StaticConfigParams{}) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("createUpstreamsForPlus returned \n%v but expected \n%v", result, expected) + t.Errorf("createUpstreamsForPlus returned \n%v but expected \n%v", result, expected) } } @@ -3666,7 +3645,7 @@ func TestCreateUpstreamServersConfigForPlus(t *testing.T) { result := createUpstreamServersConfigForPlus(upstream) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) + t.Errorf("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) } } @@ -3676,7 +3655,7 @@ func TestCreateUpstreamServersConfigForPlusNoUpstreams(t *testing.T) { result := createUpstreamServersConfigForPlus(noUpstream) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) + t.Errorf("createUpstreamServersConfigForPlus returned %v but expected %v", result, expected) } } @@ -3804,6 +3783,7 @@ func TestGenerateSplits(t *testing.T) { ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, Snippets: []string{locSnippet}, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_1_split_1", @@ -3828,6 +3808,7 @@ func TestGenerateSplits(t *testing.T) { ProxySSLName: "coffee-v2.default.svc", ProxyPassRequestHeaders: true, Snippets: []string{locSnippet}, + ServiceName: "coffee-v2", }, { Path: "/internal_location_splits_1_split_2", @@ -3869,13 +3850,13 @@ func TestGenerateSplits(t *testing.T) { returnLocationIndex, ) if !reflect.DeepEqual(resultSplitClient, expectedSplitClient) { - cmp.Diff(resultSplitClient, expectedSplitClient) //("generateSplits() returned \n%+v but expected \n%+v", resultSplitClient, expectedSplitClient) + t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultSplitClient, expectedSplitClient) } if !reflect.DeepEqual(resultLocations, expectedLocations) { - cmp.Diff(resultLocations, expectedLocations) //("generateSplits() returned \n%+v but expected \n%+v", resultLocations, expectedLocations) + t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultLocations, expectedLocations) } if !reflect.DeepEqual(resultReturnLocations, expectedReturnLocations) { - cmp.Diff(resultReturnLocations, expectedReturnLocations) //("generateSplits() returned \n%+v but expected \n%+v", resultReturnLocations, expectedReturnLocations) + t.Errorf("generateSplits() returned \n%+v but expected \n%+v", resultReturnLocations, expectedReturnLocations) } } @@ -3935,6 +3916,7 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { Internal: true, ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_1_split_1", @@ -3945,6 +3927,7 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { Internal: true, ProxySSLName: "coffee-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v2", }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -3968,7 +3951,7 @@ func TestGenerateDefaultSplitsConfig(t *testing.T) { result := generateDefaultSplitsConfig(route, upstreamNamer, crUpstreams, variableNamer, index, &cfgParams, route.ErrorPages, 0, "", locSnippet, enableSnippets, 0) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("generateDefaultSplitsConfig() returned \n%+v but expected \n%+v", result, expected) + t.Errorf("generateDefaultSplitsConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -4234,6 +4217,7 @@ func TestGenerateMatchesConfig(t *testing.T) { }, ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_2_split_0", @@ -4257,6 +4241,7 @@ func TestGenerateMatchesConfig(t *testing.T) { }, ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_2_split_1", @@ -4280,6 +4265,7 @@ func TestGenerateMatchesConfig(t *testing.T) { }, ProxySSLName: "coffee-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v2", }, { Path: "/internal_location_matches_1_default", @@ -4303,6 +4289,7 @@ func TestGenerateMatchesConfig(t *testing.T) { }, ProxySSLName: "tea.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "tea", }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -4351,7 +4338,7 @@ func TestGenerateMatchesConfig(t *testing.T) { 0, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) + t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -4531,6 +4518,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { ProxyInterceptErrors: true, ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_2_split_1", @@ -4554,6 +4542,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { ProxyInterceptErrors: true, ProxySSLName: "coffee-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v2", }, { Path: "/internal_location_splits_3_split_0", @@ -4577,6 +4566,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { ProxyInterceptErrors: true, ProxySSLName: "coffee-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v2", }, { Path: "/internal_location_splits_3_split_1", @@ -4600,6 +4590,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { ProxyInterceptErrors: true, ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_4_split_0", @@ -4623,6 +4614,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { ProxyInterceptErrors: true, ProxySSLName: "coffee-v1.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v1", }, { Path: "/internal_location_splits_4_split_1", @@ -4646,6 +4638,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { ProxyInterceptErrors: true, ProxySSLName: "coffee-v2.default.svc", ProxyPassRequestHeaders: true, + ServiceName: "coffee-v2", }, }, InternalRedirectLocation: version2.InternalRedirectLocation{ @@ -4720,7 +4713,7 @@ func TestGenerateMatchesConfigWithMultipleSplits(t *testing.T) { 0, ) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) + t.Errorf("generateMatchesConfig() returned \n%+v but expected \n%+v", result, expected) } } @@ -4780,20 +4773,10 @@ func TestGenerateValueForMatchesRouteMap(t *testing.T) { for _, test := range tests { resultValue, resultIsNegative := generateValueForMatchesRouteMap(test.input) if resultValue != test.expectedValue { - t.Errorf( - "generateValueForMatchesRouteMap(%q) returned %q but expected %q as the value", - test.input, - resultValue, - test.expectedValue, - ) + t.Errorf("generateValueForMatchesRouteMap(%q) returned %q but expected %q as the value", test.input, resultValue, test.expectedValue) } if resultIsNegative != test.expectedIsNegative { - t.Errorf( - "generateValueForMatchesRouteMap(%q) returned %v but expected %v as the isNegative", - test.input, - resultIsNegative, - test.expectedIsNegative, - ) + t.Errorf("generateValueForMatchesRouteMap(%q) returned %v but expected %v as the isNegative", test.input, resultIsNegative, test.expectedIsNegative) } } } @@ -4837,13 +4820,7 @@ func TestGenerateParametersForMatchesRouteMap(t *testing.T) { for _, test := range tests { result := generateParametersForMatchesRouteMap(test.inputMatchedValue, test.inputSuccessfulResult) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateParametersForMatchesRouteMap(%q, %q) returned %v but expected %v", - test.inputMatchedValue, - test.inputSuccessfulResult, - result, - test.expected, - ) + t.Errorf("generateParametersForMatchesRouteMap(%q, %q) returned %v but expected %v", test.inputMatchedValue, test.inputSuccessfulResult, result, test.expected) } } } @@ -4882,12 +4859,7 @@ func TestGetNameForSourceForMatchesRouteMapFromCondition(t *testing.T) { for _, test := range tests { result := getNameForSourceForMatchesRouteMapFromCondition(test.input) if result != test.expected { - t.Errorf( - "getNameForSourceForMatchesRouteMapFromCondition() returned %q but expected %q for input %v", - result, - test.expected, - test.input, - ) + t.Errorf("getNameForSourceForMatchesRouteMapFromCondition() returned %q but expected %q for input %v", result, test.expected, test.input) } } } @@ -4915,7 +4887,7 @@ func TestGenerateLBMethod(t *testing.T) { for _, test := range tests { result := generateLBMethod(test.input, defaultMethod) if result != test.expected { - cmp.Diff(result, test.expected) //("generateLBMethod() returned %q but expected %q for input '%v'", result, test.expected, test.input) + t.Errorf("generateLBMethod() returned %q but expected %q for input '%v'", result, test.expected, test.input) } } } @@ -4953,12 +4925,7 @@ func TestUpstreamHasKeepalive(t *testing.T) { for _, test := range tests { result := upstreamHasKeepalive(test.upstream, test.cfgParams) if result != test.expected { - t.Errorf( - "upstreamHasKeepalive() returned %v, but expected %v for the case of %v", - result, - test.expected, - test.msg, - ) + t.Errorf("upstreamHasKeepalive() returned %v, but expected %v for the case of %v", result, test.expected, test.msg) } } } @@ -4987,7 +4954,7 @@ func TestNewHealthCheckWithDefaults(t *testing.T) { result := newHealthCheckWithDefaults(conf_v1.Upstream{}, upstreamName, baseCfgParams) if !reflect.DeepEqual(result, expected) { - cmp.Diff(result, expected) //("newHealthCheckWithDefaults returned \n%v but expected \n%v", result, expected) + t.Errorf("newHealthCheckWithDefaults returned \n%v but expected \n%v", result, expected) } } @@ -5111,12 +5078,7 @@ func TestGenerateHealthCheck(t *testing.T) { for _, test := range tests { result := generateHealthCheck(test.upstream, test.upstreamName, baseCfgParams) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateHealthCheck returned \n%v but expected \n%v \n for case: %v", - result, - test.expected, - test.msg, - ) + t.Errorf("generateHealthCheck returned \n%v but expected \n%v \n for case: %v", result, test.expected, test.msg) } } } @@ -5297,14 +5259,8 @@ func TestGenerateEndpointsForUpstream(t *testing.T) { ) result := vsc.generateEndpointsForUpstream(test.vsEx.VirtualServer, namespace, test.upstream, test.vsEx) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateEndpointsForUpstream(isPlus=%v, isResolverConfigured=%v) returned %v, but expected %v for case: %v", - test.isPlus, - test.isResolverConfigured, - result, - test.expected, - test.msg, - ) + t.Errorf("generateEndpointsForUpstream(isPlus=%v, isResolverConfigured=%v) returned %v, but expected %v for case: %v", + test.isPlus, test.isResolverConfigured, result, test.expected, test.msg) } if len(vsc.warnings) == 0 && test.warningsExpected { @@ -5343,12 +5299,7 @@ func TestGenerateSlowStartForPlusWithInCompatibleLBMethods(t *testing.T) { result := vsc.generateSlowStartForPlus(&conf_v1.VirtualServer{}, upstream, lbMethod) if !reflect.DeepEqual(result, expected) { - t.Errorf( - "generateSlowStartForPlus returned %v, but expected %v for lbMethod %v", - result, - expected, - lbMethod, - ) + t.Errorf("generateSlowStartForPlus returned %v, but expected %v for lbMethod %v", result, expected, lbMethod) } if len(vsc.warnings) == 0 { @@ -5382,7 +5333,7 @@ func TestGenerateSlowStartForPlus(t *testing.T) { vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false, &StaticConfigParams{}) result := vsc.generateSlowStartForPlus(&conf_v1.VirtualServer{}, test.upstream, test.lbMethod) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) + t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) } if len(vsc.warnings) != 0 { @@ -5410,7 +5361,7 @@ func TestCreateEndpointsFromUpstream(t *testing.T) { endpoints := createEndpointsFromUpstream(ups) if !reflect.DeepEqual(endpoints, expected) { - cmp.Diff(endpoints, expected) //("createEndpointsFromUpstream returned %v, but expected %v", endpoints, expected) + t.Errorf("createEndpointsFromUpstream returned %v, but expected %v", endpoints, expected) } } @@ -5481,12 +5432,7 @@ func TestGenerateUpstreamWithQueue(t *testing.T) { vsc := newVirtualServerConfigurator(&ConfigParams{}, test.isPlus, false, &StaticConfigParams{}) result := vsc.generateUpstream(nil, test.name, test.upstream, false, []string{}) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateUpstream() returned %v but expected %v for the case of %v", - result, - test.expected, - test.msg, - ) + t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) } } @@ -5518,12 +5464,7 @@ func TestGenerateQueueForPlus(t *testing.T) { for _, test := range tests { result := generateQueueForPlus(test.upstreamQueue, "60s") if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateQueueForPlus() returned %v but expected %v for the case of %v", - result, - test.expected, - test.msg, - ) + t.Errorf("generateQueueForPlus() returned %v but expected %v for the case of %v", result, test.expected, test.msg) } } @@ -5554,12 +5495,7 @@ func TestGenerateSessionCookie(t *testing.T) { for _, test := range tests { result := generateSessionCookie(test.sc) if !reflect.DeepEqual(result, test.expected) { - t.Errorf( - "generateSessionCookie() returned %v, but expected %v for the case of: %v", - result, - test.expected, - test.msg, - ) + t.Errorf("generateSessionCookie() returned %v, but expected %v for the case of: %v", result, test.expected, test.msg) } } } @@ -5590,7 +5526,7 @@ func TestGeneratePath(t *testing.T) { for _, test := range tests { result := generatePath(test.path) if result != test.expected { - cmp.Diff(result, test.expected) //("generatePath() returned %v, but expected %v.", result, test.expected) + t.Errorf("generatePath() returned %v, but expected %v.", result, test.expected) } } } @@ -5621,13 +5557,7 @@ func TestGenerateErrorPageName(t *testing.T) { 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, - ) + t.Errorf("generateErrorPageName(%v, %v) returned %v but expected %v", test.routeIndex, test.index, result, test.expected) } } } @@ -5650,7 +5580,7 @@ func TestGenerateErrorPageCodes(t *testing.T) { for _, test := range tests { result := generateErrorPageCodes(test.codes) if result != test.expected { - cmp.Diff(result, test.expected) //("generateErrorPageCodes(%v) returned %v but expected %v", test.codes, result, test.expected) + t.Errorf("generateErrorPageCodes(%v) returned %v but expected %v", test.codes, result, test.expected) } } } @@ -5711,13 +5641,7 @@ func TestGenerateErrorPages(t *testing.T) { 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, - ) + t.Errorf("generateErrorPages(%v, %v) returned %v but expected %v", test.upstreamName, test.errorPages, result, test.expected) } } } @@ -5788,13 +5712,7 @@ func TestGenerateErrorPageLocations(t *testing.T) { 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, - ) + t.Errorf("generateErrorPageLocations(%v, %v) returned %v but expected %v", test.upstreamName, test.errorPages, result, test.expected) } } } @@ -5853,13 +5771,7 @@ func TestIsTLSEnabled(t *testing.T) { for _, test := range tests { result := isTLSEnabled(test.upstream, test.spiffeCert) if result != test.expected { - t.Errorf( - "isTLSEnabled(%v, %v) returned %v but expected %v", - test.upstream, - test.spiffeCert, - result, - test.expected, - ) + t.Errorf("isTLSEnabled(%v, %v) returned %v but expected %v", test.upstream, test.spiffeCert, result, test.expected) } } @@ -5921,8 +5833,8 @@ func TestGenerateRewrites(t *testing.T) { for _, test := range tests { result := generateRewrites(test.path, test.proxy, test.internal, test.originalPath) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateRewrites(%v, %v, %v, %v) returned \n %v but expected \n %v", - //test.path, test.proxy, test.internal, test.originalPath, result, test.expected) + t.Errorf("generateRewrites(%v, %v, %v, %v) returned \n %v but expected \n %v", + test.path, test.proxy, test.internal, test.originalPath, result, test.expected) } } } @@ -5970,8 +5882,8 @@ func TestGenerateProxyPassRewrite(t *testing.T) { for _, test := range tests { result := generateProxyPassRewrite(test.path, test.proxy, test.internal) if result != test.expected { - cmp.Diff(result, test.expected) //("generateProxyPassRewrite(%v, %v, %v) returned %v but expected %v", - // test.path, test.proxy, test.internal, result, test.expected) + t.Errorf("generateProxyPassRewrite(%v, %v, %v) returned %v but expected %v", + test.path, test.proxy, test.internal, result, test.expected) } } } @@ -6020,7 +5932,7 @@ func TestGenerateProxySetHeaders(t *testing.T) { for _, test := range tests { result := generateProxySetHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateProxySetHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + t.Errorf("generateProxySetHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6069,12 +5981,7 @@ func TestGenerateProxyPassRequestHeaders(t *testing.T) { for _, test := range tests { result := generateProxyPassRequestHeaders(test.proxy) if result != test.expected { - t.Errorf( - "generateProxyPassRequestHeaders(%v) returned %v but expected %v", - test.proxy, - result, - test.expected, - ) + t.Errorf("generateProxyPassRequestHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6106,7 +6013,7 @@ func TestGenerateProxyHideHeaders(t *testing.T) { for _, test := range tests { result := generateProxyHideHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateProxyHideHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + t.Errorf("generateProxyHideHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6138,7 +6045,7 @@ func TestGenerateProxyPassHeaders(t *testing.T) { for _, test := range tests { result := generateProxyPassHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateProxyPassHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + t.Errorf("generateProxyPassHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6171,7 +6078,7 @@ func TestGenerateProxyIgnoreHeaders(t *testing.T) { for _, test := range tests { result := generateProxyIgnoreHeaders(test.proxy) if result != test.expected { - cmp.Diff(result, test.expected) //("generateProxyIgnoreHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + t.Errorf("generateProxyIgnoreHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6232,7 +6139,7 @@ func TestGenerateProxyAddHeaders(t *testing.T) { for _, test := range tests { result := generateProxyAddHeaders(test.proxy) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("generateProxyAddHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) + t.Errorf("generateProxyAddHeaders(%v) returned %v but expected %v", test.proxy, result, test.expected) } } } @@ -6276,7 +6183,7 @@ func TestGetUpstreamResourceLabels(t *testing.T) { for _, test := range tests { result := getUpstreamResourceLabels(test.owner) if !reflect.DeepEqual(result, test.expected) { - cmp.Diff(result, test.expected) //("getUpstreamResourceLabels(%+v) returned %+v but expected %+v", test.owner, result, test.expected) + t.Errorf("getUpstreamResourceLabels(%+v) returned %+v but expected %+v", test.owner, result, test.expected) } } } From 032290590e27adec93727f4cbc1ce4864ee64c7d Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Tue, 17 Nov 2020 15:36:11 +0000 Subject: [PATCH 03/16] Transport Server --- .../basic-tcp-udp/global-configuration.yaml | 1 - examples/complete-example/cafe-ingress.yaml | 3 ++- internal/configs/transportserver.go | 2 ++ internal/configs/transportserver_test.go | 4 ++++ internal/configs/version2/nginx-plus.transportserver.tmpl | 4 ++++ internal/configs/version2/nginx.transportserver.tmpl | 3 +++ internal/configs/version2/stream.go | 2 ++ 7 files changed, 17 insertions(+), 2 deletions(-) diff --git a/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml b/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml index 2ced7c188c..d6cdb5d670 100644 --- a/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml +++ b/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml @@ -2,7 +2,6 @@ apiVersion: k8s.nginx.org/v1alpha1 kind: GlobalConfiguration metadata: name: nginx-configuration - namespace: nginx-ingress spec: listeners: - name: dns-udp diff --git a/examples/complete-example/cafe-ingress.yaml b/examples/complete-example/cafe-ingress.yaml index 2eca43d223..53c3205e59 100644 --- a/examples/complete-example/cafe-ingress.yaml +++ b/examples/complete-example/cafe-ingress.yaml @@ -2,8 +2,9 @@ apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: name: cafe-ingress + namespace: nginx-ingress spec: - # ingressClassName: nginx # use only with k8s version >= 1.18.0 + ingressClassName: nginx # use only with k8s version >= 1.18.0 tls: - hosts: - cafe.example.com diff --git a/internal/configs/transportserver.go b/internal/configs/transportserver.go index 8cca246c7e..78ac25a507 100644 --- a/internal/configs/transportserver.go +++ b/internal/configs/transportserver.go @@ -49,6 +49,8 @@ func generateTransportServerConfig(transportServerEx *TransportServerEx, listene ProxyRequests: proxyRequests, ProxyResponses: proxyResponses, ProxyPass: upstreamNamer.GetNameForUpstream(transportServerEx.TransportServer.Spec.Action.Pass), + Name: transportServerEx.TransportServer.Name, + Namespace: transportServerEx.TransportServer.Namespace, }, Upstreams: upstreams, } diff --git a/internal/configs/transportserver_test.go b/internal/configs/transportserver_test.go index 1112f158e9..a0e14f1b31 100644 --- a/internal/configs/transportserver_test.go +++ b/internal/configs/transportserver_test.go @@ -110,6 +110,8 @@ func TestGenerateTransportServerConfigForTCP(t *testing.T) { UDP: false, StatusZone: "tcp-listener", ProxyPass: "ts_default_tcp-server_tcp-app", + Name: "tcp-server", + Namespace: "default", }, } @@ -178,6 +180,8 @@ func TestGenerateTransportServerConfigForUDP(t *testing.T) { ProxyRequests: &udpRequests, ProxyResponses: &udpResponses, ProxyPass: "ts_default_udp-server_udp-app", + Name: "udp-server", + Namespace: "default", }, } diff --git a/internal/configs/version2/nginx-plus.transportserver.tmpl b/internal/configs/version2/nginx-plus.transportserver.tmpl index bb92cd0cfe..013d8581ff 100644 --- a/internal/configs/version2/nginx-plus.transportserver.tmpl +++ b/internal/configs/version2/nginx-plus.transportserver.tmpl @@ -12,6 +12,10 @@ upstream {{ $u.Name }} { {{ $s := .Server }} server { + set $resource_type "transportserver"; + set $resource_name "{{$s.Name}}"; + set $resource_namespace "{{$s.Namespace}}"; + {{ if $s.TLSPassthrough }} listen {{ $s.UnixSocket }} proxy_protocol; set_real_ip_from unix:; diff --git a/internal/configs/version2/nginx.transportserver.tmpl b/internal/configs/version2/nginx.transportserver.tmpl index d19bfca69b..60aacb923c 100644 --- a/internal/configs/version2/nginx.transportserver.tmpl +++ b/internal/configs/version2/nginx.transportserver.tmpl @@ -12,6 +12,9 @@ upstream {{ $u.Name }} { {{ $s := .Server }} server { + set $resource_type "transportserver"; + set $resource_name "{{$s.Name}}"; + set $resource_namespace "{{$s.Namespace}}"; {{ if $s.TLSPassthrough }} listen {{ $s.UnixSocket }} proxy_protocol; set_real_ip_from unix:; diff --git a/internal/configs/version2/stream.go b/internal/configs/version2/stream.go index aa8783fe86..ca6da09dc6 100644 --- a/internal/configs/version2/stream.go +++ b/internal/configs/version2/stream.go @@ -27,6 +27,8 @@ type StreamServer struct { ProxyRequests *int ProxyResponses *int ProxyPass string + Name string + Namespace string } // TLSPassthroughHostsConfig defines a mapping between TLS Passthrough hosts and the corresponding unix sockets. From 37a83f67dac4059e595d035b09e1b9189d8d767b Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 19 Nov 2020 11:25:40 +0000 Subject: [PATCH 04/16] Address Feedback --- internal/configs/ingress.go | 1 - internal/configs/version1/config.go | 2 -- internal/configs/version1/nginx-plus.ingress.tmpl | 2 ++ internal/configs/version1/nginx.ingress.tmpl | 10 ++++++---- .../configs/version2/nginx-plus.virtualserver.tmpl | 4 ++-- internal/configs/version2/nginx.virtualserver.tmpl | 4 ++-- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/internal/configs/ingress.go b/internal/configs/ingress.go index 4a08a93483..e6a5ab6006 100644 --- a/internal/configs/ingress.go +++ b/internal/configs/ingress.go @@ -201,7 +201,6 @@ func generateNginxCfg(ingEx *IngressEx, pems map[string]string, apResources map[ proxySSLName := generateProxySSLName(path.Backend.ServiceName, ingEx.Ingress.Namespace) loc := createLocation(pathOrDefault(path.Path), upstreams[upsName], &cfgParams, wsServices[path.Backend.ServiceName], rewrites[path.Backend.ServiceName], ssl, grpcServices[path.Backend.ServiceName], proxySSLName, path.PathType, path.Backend.ServiceName) - glog.Errorf("Service Name : %v", path.Backend) if isMinion && ingEx.JWTKey.Name != "" { loc.JWTAuth = &version1.JWTAuth{ Key: jwtKeyFileName, diff --git a/internal/configs/version1/config.go b/internal/configs/version1/config.go index a8a30a1f94..ac9f160385 100644 --- a/internal/configs/version1/config.go +++ b/internal/configs/version1/config.go @@ -101,8 +101,6 @@ type Server struct { AppProtectLogEnable string SpiffeCerts bool - - ServiceName string } // JWTRedirectLocation describes a location for redirecting client requests to a login URL for JWT Authentication. diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index d5e70b5317..fd5d1da70f 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -153,6 +153,8 @@ server { set $service {{$location.ServiceName}}; {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} + set resource_name "{{$location.MinionIngress.Name}}"; + set resource_namespace "{{$location.MinionIngressNamespace}}"; {{end}} {{if $location.GRPC}} {{if not $server.GRPCOnly}} diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index dab68f4aad..3308dbe3dc 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -1,5 +1,4 @@ # configuration for {{.Ingress.Namespace}}/{{.Ingress.Name}} -{{$ingress := .Ingress}} {{range $upstream := .Upstreams}} upstream {{$upstream.Name}} { {{if ne $upstream.UpstreamZoneSize "0"}}zone {{$upstream.Name}} {{$upstream.UpstreamZoneSize}};{{end}} @@ -43,9 +42,9 @@ server { server_name {{$server.Name}}; - set $resource_type "Ing" - set $resource_name {{$ingress.Name}} - set $resource_namespace {{$ingress.Namespace}} + set $resource_type "ingress"; + set $resource_name "{{$.Ingress.Name}}"; + set $resource_namespace "{{$.Ingress.Namespace}}"; {{range $proxyHideHeader := $server.ProxyHideHeaders}} proxy_hide_header {{$proxyHideHeader}};{{end}} @@ -89,8 +88,11 @@ server { {{range $location := $server.Locations}} location {{$location.Path}} { + set $service "{{$location.ServiceName}}"; {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} + set resource_name "{{$location.MinionIngress.Name}}"; + set resource_namespace "{{$location.MinionIngressNamespace}}"; {{end}} {{if $location.GRPC}} {{if not $server.GRPCOnly}} diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index bc43563e81..ab146e293f 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -62,8 +62,8 @@ server { status_zone {{ $s.StatusZone }}; set $resource_type "virtualserver"; - set $resource_name "{{$s.ServerName}}"; - set $resource_namespace "{{$s.Namespace}}"; + set $resource_name "{{$s.ServerName}}"; + set $resource_namespace "{{$s.Namespace}}"; {{ with $ssl := $s.SSL }} {{ if $s.TLSPassthrough }} diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index fe48b28fb6..04e5815c48 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -45,8 +45,8 @@ server { server_name {{ $s.ServerName }}; set $resource_type "virtualserver"; - set $resource_name "{{$s.ServerName}}"; - set $resource_namespace "{{$s.Namespace}}"; + set $resource_name "{{$s.ServerName}}"; + set $resource_namespace "{{$s.Namespace}}"; {{ with $ssl := $s.SSL }} From 2606b1216de0a8810fb30cad98418594a7b4f795 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Fri, 20 Nov 2020 12:07:30 +0000 Subject: [PATCH 05/16] Address feedback --- deployments/helm-chart/values.yaml | 4 +- .../configs/version1/nginx-plus.ingress.tmpl | 5 +- internal/configs/version1/nginx.ingress.tmpl | 2 +- internal/configs/version1/nginx.tmpl | 4 +- internal/configs/version2/http.go | 3 + .../version2/nginx-plus.virtualserver.tmpl | 9 ++- internal/configs/virtualserver.go | 56 ++++++++----------- internal/configs/virtualserver_test.go | 17 ++---- 8 files changed, 43 insertions(+), 57 deletions(-) diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml index 611fb2d49c..0db9507d3e 100644 --- a/deployments/helm-chart/values.yaml +++ b/deployments/helm-chart/values.yaml @@ -20,7 +20,7 @@ controller: enable: false ## Enables the Ingress controller pods to use the host's network namespace. - hostNetwork: false + hostNetwork: true ## Enables debugging for NGINX. Uses the nginx-debug binary. Requires error-log-level: debug in the ConfigMap via `controller.config.entries`. nginxDebug: false @@ -168,7 +168,7 @@ controller: ## Add a location based on the value of health-status-uri to the default server. The location responds with the 200 status code for any request. ## Useful for external health-checking of the Ingress controller. - healthStatus: false + healthStatus: true ## Sets the URI of health status location in the default server. Requires contoller.healthStatus. healthStatusURI: "/nginx-health" diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index fd5d1da70f..7a05ade632 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -127,7 +127,6 @@ server { {{- range $healthCheck := $server.HealthChecks}} location @hc-{{$healthCheck.UpstreamName}} { - set $service "nil"; {{- range $name, $header := $healthCheck.Headers}} proxy_set_header {{$name}} "{{$header}}"; {{- end }} @@ -150,11 +149,11 @@ server { {{range $location := $server.Locations}} location {{$location.Path}} { - set $service {{$location.ServiceName}}; + set $service "{{$location.ServiceName}}"; {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} set resource_name "{{$location.MinionIngress.Name}}"; - set resource_namespace "{{$location.MinionIngressNamespace}}"; + set resource_namespace "{{$location.MinionIngress.Namespace}}"; {{end}} {{if $location.GRPC}} {{if not $server.GRPCOnly}} diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index 3308dbe3dc..1d86aefb96 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -92,7 +92,7 @@ server { {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} set resource_name "{{$location.MinionIngress.Name}}"; - set resource_namespace "{{$location.MinionIngressNamespace}}"; + set resource_namespace "{{$location.MinionIngress.Namespace}}"; {{end}} {{if $location.GRPC}} {{if not $server.GRPCOnly}} diff --git a/internal/configs/version1/nginx.tmpl b/internal/configs/version1/nginx.tmpl index 01d7f0bc62..94440b38fc 100644 --- a/internal/configs/version1/nginx.tmpl +++ b/internal/configs/version1/nginx.tmpl @@ -37,11 +37,11 @@ http { log_format main {{if .LogFormatEscaping}}escape={{ .LogFormatEscaping }} {{end}} {{range $i, $value := .LogFormat -}} {{with $value}}'{{if $i}} {{end}}{{$value}}' - {{end}}{{end}} "$resource_type"; + {{end}}{{end}}; {{- else -}} log_format main '$remote_addr - $remote_user [$time_local] "$request" ' '$status $body_bytes_sent "$http_referer" ' - '"$http_user_agent" "$http_x_forwarded_for"' "$resource_type"; + '"$http_user_agent" "$http_x_forwarded_for"'; {{- end}} {{if .AccessLogOff}} diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 1109f1a7ff..9de9b95d0b 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -140,6 +140,9 @@ type Location struct { EgressMTLS *EgressMTLS PoliciesErrorReturn *Return ServiceName string + IsVSR bool + VSRName string + VSRNamespace string } // ReturnLocation defines a location for returning a fixed response. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index ab146e293f..fba5a1b29e 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -176,7 +176,6 @@ server { {{ range $hc := $s.HealthChecks }} location @hc-{{ $hc.Name }} { - set $service "nil"; {{ range $n, $v := $hc.Headers }} proxy_set_header {{ $n }} "{{ $v }}"; {{ end }} @@ -214,7 +213,13 @@ server { {{ range $l := $s.Locations }} location {{ $l.Path }} { - set $service "{{$l.ServiceName}}"; + set $service "{{ $l.ServiceName }}"; + {{ if $l.IsVSR }} + set $resource_type "virtualserverroute"; + set $resource_name "{{ $l.VSRName }}"; + set $resource_namespace "{{ $l.VSRNamespace }}"; + {{ end }} + {{ if $l.Internal }} internal; {{ end }} diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 58da24b2f4..38481472a7 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -359,6 +359,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( var vsrErrorPagesRouteIndex = make(map[string]int) var vsrLocationSnippetsFromVs = make(map[string]string) var vsrPoliciesFromVs = make(map[string][]conf_v1.PolicyReference) + IsVSR := false matchesRoutes := 0 variableNamer := newVariableNamer(vsEx.VirtualServer) @@ -450,7 +451,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( 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)) + errorPageIndex, proxySSLName, r.Path, vsLocSnippets, vsc.enableSnippets, len(returnLocations), IsVSR, "", "") addPoliciesCfgToLocation(routePoliciesCfg, &loc) locations = append(locations, loc) @@ -462,6 +463,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( // generate config for subroutes of each VirtualServerRoute for _, vsr := range vsEx.VirtualServerRoutes { + IsVSR := true upstreamNamer := newUpstreamNamerForVirtualServerRoute(vsEx.VirtualServer, vsr) for _, r := range vsr.Spec.Subroutes { errorPageIndex := len(errorPageLocations) @@ -553,7 +555,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( 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)) + errorPageIndex, proxySSLName, r.Path, locSnippets, vsc.enableSnippets, len(returnLocations), IsVSR, vsr.Name, vsr.Namespace) addPoliciesCfgToLocation(routePoliciesCfg, &loc) locations = append(locations, loc) @@ -1262,21 +1264,9 @@ 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, - errorPages []conf_v1.ErrorPage, - internal bool, - errPageIndex int, - proxySSLName string, - originalPath string, - locSnippets string, - enableSnippets bool, - retLocIndex int, -) (version2.Location, *version2.ReturnLocation) { +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) { locationSnippets := generateSnippets(enableSnippets, locSnippets, cfgParams.LocationSnippets) @@ -1288,19 +1278,8 @@ func generateLocation( return generateLocationForReturn(path, cfgParams.LocationSnippets, action.Return, retLocIndex) } - return generateLocationForProxying( - path, - upstreamName, - upstream, - cfgParams, - errorPages, - internal, - errPageIndex, - proxySSLName, - action.Proxy, - originalPath, - locationSnippets, - ), nil + return generateLocationForProxying(path, upstreamName, upstream, cfgParams, errorPages, internal, + errPageIndex, proxySSLName, action.Proxy, originalPath, locationSnippets, IsVSR, vsrName, vsrNamespace), nil } func generateProxySetHeaders(proxy *conf_v1.ActionProxy) []version2.Header { @@ -1376,7 +1355,7 @@ func generateProxyAddHeaders(proxy *conf_v1.ActionProxy) []version2.AddHeader { func generateLocationForProxying(path string, upstreamName string, upstream conf_v1.Upstream, cfgParams *ConfigParams, errorPages []conf_v1.ErrorPage, internal bool, errPageIndex int, - proxySSLName string, proxy *conf_v1.ActionProxy, originalPath string, locationSnippets []string) version2.Location { + proxySSLName string, proxy *conf_v1.ActionProxy, originalPath string, locationSnippets []string, isVSR bool, vsrName string, vsrNamespace string) version2.Location { return version2.Location{ Path: generatePath(path), Internal: internal, @@ -1406,6 +1385,9 @@ func generateLocationForProxying(path string, upstreamName string, upstream conf ErrorPages: generateErrorPages(errPageIndex, errorPages), ProxySSLName: proxySSLName, ServiceName: upstream.Service, + IsVSR: isVSR, + VSRName: vsrName, + VSRNamespace: vsrNamespace, } } @@ -1497,6 +1479,9 @@ func generateSplits( ) (version2.SplitClient, []version2.Location, []version2.ReturnLocation) { var distributions []version2.Distribution + IsVSR := false + vsrName := "" + vsrNamespace := "" for i, s := range splits { d := version2.Distribution{ @@ -1522,7 +1507,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) + errPageIndex, proxySSLName, originalPath, locSnippets, enableSnippets, newRetLocIndex, IsVSR, vsrName, vsrNamespace) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -1637,6 +1622,9 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr var locations []version2.Location var returnLocations []version2.ReturnLocation var splitClients []version2.SplitClient + IsVSR := false + vsrName := "" + vsrNamespace := "" scLocalIndex = 0 for i, m := range route.Matches { @@ -1667,7 +1655,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) + errPageIndex, proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, IsVSR, vsrName, vsrNamespace) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -1702,7 +1690,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) + errPageIndex, proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, IsVSR, vsrName, vsrNamespace) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 8dd3fae224..f980d734bd 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -3086,21 +3086,12 @@ func TestGenerateLocationForProxying(t *testing.T) { ProxyNextUpstreamTries: 0, ProxyPassRequestHeaders: true, ServiceName: "", + IsVSR: false, + VSRName: "", + VSRNamespace: "", } - result := generateLocationForProxying( - path, - upstreamName, - conf_v1.Upstream{}, - &cfgParams, - nil, - false, - 0, - "", - nil, - "", - vsLocSnippets, - ) + result := generateLocationForProxying(path, upstreamName, conf_v1.Upstream{}, &cfgParams, nil, false, 0, "", nil, "", vsLocSnippets, false, "", "") if !reflect.DeepEqual(result, expected) { t.Errorf("generateLocationForProxying() returned \n%+v but expected \n%+v", result, expected) } From 21efbc2b79bc6c6b3dd2d804e838f3ebe7c0acf2 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Mon, 23 Nov 2020 13:54:20 +0000 Subject: [PATCH 06/16] =?UTF-8?q?Address=20feedback=202=C2=B8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../global-configuration/configmap-resource.md | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/docs-web/configuration/global-configuration/configmap-resource.md b/docs-web/configuration/global-configuration/configmap-resource.md index d54988336c..586bc48826 100644 --- a/docs-web/configuration/global-configuration/configmap-resource.md +++ b/docs-web/configuration/global-configuration/configmap-resource.md @@ -205,7 +205,22 @@ See the doc about [VirtualServer and VirtualServerRoute resources](/nginx-ingres * - ``log-format`` - Sets the custom `log format `_ for HTTP and HTTPS traffic. For convenience, it is possible to define the log format across multiple lines (each line separated by ``\n``). In that case, the Ingress Controller will replace every ``\n`` character with a space character. All ``'`` characters must be escaped. - See the `template file `_ for the access log. - - 127.0.0.1 - - [01/Jun/2020:18:54:31 +0000] "GET /coffee HTTP/1.1" 200 156 "-" "curl/7.54.0" "-" resource_namespace=default resource_name=cafe resource_type=ingress service_namespace=default service_name=coffee-svc pod_name=coffee-8c8ff9b4f-r99h9 + - ```kind: ConfigMap + apiVersion: v1 + metadata: + name: nginx-config + namespace: nginx-ingress + data: + log-format: compression '$remote_addr - $remote_user [$time_local] ' + '"$request" $status $bytes_sent ' + '"$http_referer" "$http_user_agent" "$gzip_ratio"' + 'type="$resource_type" name="$resource_name" service="$service";``` + In addition to built-in NGINX variables, you can also use the variables that the Ingress Controller configures: + $resource_type - The type of the k8s object. + $resource_name - The name of the k8s resource. + $resource_namespace - The namespace the k8s object exists in. + $service - The service for the k8s object. + **note** these are only available for VirtualServer and Ingress resources * - ``log-format-escaping`` - Sets the characters escaping for the variables of the log format. Supported values: ``json`` (JSON escaping), ``default`` (the default escaping) ``none`` (disables escaping). - ``default`` From aebf5fc213dc1e3c4c260585a6e2514bf173175a Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 25 Nov 2020 10:20:33 +0000 Subject: [PATCH 07/16] Feedback --- deployments/helm-chart/values.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/deployments/helm-chart/values.yaml b/deployments/helm-chart/values.yaml index 0db9507d3e..611fb2d49c 100644 --- a/deployments/helm-chart/values.yaml +++ b/deployments/helm-chart/values.yaml @@ -20,7 +20,7 @@ controller: enable: false ## Enables the Ingress controller pods to use the host's network namespace. - hostNetwork: true + hostNetwork: false ## Enables debugging for NGINX. Uses the nginx-debug binary. Requires error-log-level: debug in the ConfigMap via `controller.config.entries`. nginxDebug: false @@ -168,7 +168,7 @@ controller: ## Add a location based on the value of health-status-uri to the default server. The location responds with the 200 status code for any request. ## Useful for external health-checking of the Ingress controller. - healthStatus: true + healthStatus: false ## Sets the URI of health status location in the default server. Requires contoller.healthStatus. healthStatusURI: "/nginx-health" From 06f40575ae8c66955f544cefc5c3d2e5d1778585 Mon Sep 17 00:00:00 2001 From: Lorcan McVeigh Date: Wed, 25 Nov 2020 10:45:39 +0000 Subject: [PATCH 08/16] Update default log format with new variables Co-authored-by: Mike Stephen --- internal/configs/version1/nginx-plus.tmpl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/configs/version1/nginx-plus.tmpl b/internal/configs/version1/nginx-plus.tmpl index 257af534d0..6f00febdb3 100644 --- a/internal/configs/version1/nginx-plus.tmpl +++ b/internal/configs/version1/nginx-plus.tmpl @@ -44,7 +44,11 @@ http { {{- else -}} log_format main '$remote_addr - $remote_user [$time_local] "$request" ' '$status $body_bytes_sent "$http_referer" ' - '"$http_user_agent" "$http_x_forwarded_for" resource_type="$resource_type" resource_name="$resource_name" resource_namespace="$resource_namespace" service="$service"'; + '"$http_user_agent" "$http_x_forwarded_for" ' + 'resource_type="$resource_type" ' + 'resource_name="$resource_name" ' + 'resource_namespace="$resource_namespace" ' + 'service="$service"'; {{- end}} {{if .AccessLogOff}} From 127a7b90cb7b738c215e34732c7d57428c68e918 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 25 Nov 2020 10:53:32 +0000 Subject: [PATCH 09/16] Feedback 2 --- internal/configs/version1/nginx-plus.ingress.tmpl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index 7a05ade632..d1842d42e6 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -1,5 +1,4 @@ # configuration for {{.Ingress.Namespace}}/{{.Ingress.Name}} -{{$ingress := .Ingress}} {{range $upstream := .Upstreams}} upstream {{$upstream.Name}} { zone {{$upstream.Name}} {{if ne $upstream.UpstreamZoneSize "0"}}{{$upstream.UpstreamZoneSize}}{{else}}256k{{end}}; @@ -62,8 +61,8 @@ server { status_zone {{$server.StatusZone}}; set $resource_type "ingress"; - set $resource_name "{{$ingress.Name}}"; - set $resource_namespace "{{$ingress.Namespace}}"; + set $resource_name "{{$.Ingress.Name}}"; + set $resource_namespace "{{$.Ingress.Namespace}}"; {{- if $server.AppProtectEnable}} app_protect_enable {{$server.AppProtectEnable}}; From 2e0a64cd97f9eb1674c5a5986e618b19667580e6 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 25 Nov 2020 11:37:15 +0000 Subject: [PATCH 10/16] Update tests --- internal/configs/version1/nginx-plus.ingress.tmpl | 1 - internal/configs/version1/nginx-plus.tmpl | 2 +- .../configs/version2/nginx-plus.virtualserver.tmpl | 2 -- internal/configs/version2/nginx.virtualserver.tmpl | 2 -- internal/configs/virtualserver_test.go | 12 ++++++++++++ 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index d1842d42e6..5e04111864 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -140,7 +140,6 @@ server { {{- range $location := $server.JWTRedirectLocations}} location {{$location.Name}} { - set $service "nil"; internal; return 302 {{$location.LoginURL}}; } diff --git a/internal/configs/version1/nginx-plus.tmpl b/internal/configs/version1/nginx-plus.tmpl index 6f00febdb3..f12908a384 100644 --- a/internal/configs/version1/nginx-plus.tmpl +++ b/internal/configs/version1/nginx-plus.tmpl @@ -39,7 +39,7 @@ http { {{if .LogFormat -}} log_format main {{if .LogFormatEscaping}}escape={{ .LogFormatEscaping }} {{end}} {{range $i, $value := .LogFormat -}} - {{with $value}}'{{if $i}} {{end}}{{$value}} resource_type="$resource_type" resource_name="$resource_name" resource_namespace="$resource_namespace" service="$service"' + {{with $value}}'{{if $i}} {{end}}{{$value}}' {{end}}{{end}}; {{- else -}} log_format main '$remote_addr - $remote_user [$time_local] "$request" ' diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index fba5a1b29e..7be245803f 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -190,7 +190,6 @@ server { {{ range $e := $s.ErrorPageLocations }} location {{ $e.Name }} { - set $service "nil"; {{ if $e.DefaultType }} default_type "{{ $e.DefaultType }}"; {{ end }} @@ -204,7 +203,6 @@ server { {{ range $l := $s.ReturnLocations }} location {{ $l.Name }} { - set $service "nil"; default_type "{{ $l.DefaultType }}"; # status code is ignored here, using 0 return 0 "{{ $l.Return.Text }}"; diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 04e5815c48..d543bd7b43 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -155,7 +155,6 @@ server { {{ range $e := $s.ErrorPageLocations }} location {{ $e.Name }} { - set $service "nil"; {{ if $e.DefaultType }} default_type "{{ $e.DefaultType }}"; {{ end }} @@ -169,7 +168,6 @@ server { {{ range $l := $s.ReturnLocations }} location {{ $l.Name }} { - set $service "nil"; default_type "{{ $l.DefaultType }}"; # status code is ignored here, using 0 return 0 "{{ $l.Return.Text }}"; diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index f980d734bd..f12718f22c 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -546,6 +546,9 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, ServiceName: "coffee-svc", + IsVSR: true, + VSRName: "coffee", + VSRNamespace: "default", }, { Path: "/subtea", @@ -557,6 +560,9 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ProxySSLName: "sub-tea-svc.default.svc", ProxyPassRequestHeaders: true, ServiceName: "sub-tea-svc", + IsVSR: true, + VSRName: "subtea", + VSRNamespace: "default", }, { @@ -577,6 +583,9 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, ServiceName: "coffee-svc", + IsVSR: true, + VSRName: "subcoffee", + VSRNamespace: "default", }, { Path: "/coffee-errorpage-subroute-defined", @@ -596,6 +605,9 @@ func TestGenerateVirtualServerConfig(t *testing.T) { ProxySSLName: "coffee-svc.default.svc", ProxyPassRequestHeaders: true, ServiceName: "coffee-svc", + IsVSR: true, + VSRName: "subcoffee", + VSRNamespace: "default", }, }, ErrorPageLocations: []version2.ErrorPageLocation{ From 9458ccf97cc89be8a24e412251b891bd16d00233 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 25 Nov 2020 15:22:03 +0000 Subject: [PATCH 11/16] Add example for custom log-format --- .../configmap-resource.md | 17 +------------- .../basic-tcp-udp/global-configuration.yaml | 1 + examples/complete-example/cafe-ingress.yaml | 2 +- examples/custom-log-format/README.md | 22 +++++++++++++++++++ 4 files changed, 25 insertions(+), 17 deletions(-) create mode 100644 examples/custom-log-format/README.md diff --git a/docs-web/configuration/global-configuration/configmap-resource.md b/docs-web/configuration/global-configuration/configmap-resource.md index 586bc48826..3a8b35af40 100644 --- a/docs-web/configuration/global-configuration/configmap-resource.md +++ b/docs-web/configuration/global-configuration/configmap-resource.md @@ -205,22 +205,7 @@ See the doc about [VirtualServer and VirtualServerRoute resources](/nginx-ingres * - ``log-format`` - Sets the custom `log format `_ for HTTP and HTTPS traffic. For convenience, it is possible to define the log format across multiple lines (each line separated by ``\n``). In that case, the Ingress Controller will replace every ``\n`` character with a space character. All ``'`` characters must be escaped. - See the `template file `_ for the access log. - - ```kind: ConfigMap - apiVersion: v1 - metadata: - name: nginx-config - namespace: nginx-ingress - data: - log-format: compression '$remote_addr - $remote_user [$time_local] ' - '"$request" $status $bytes_sent ' - '"$http_referer" "$http_user_agent" "$gzip_ratio"' - 'type="$resource_type" name="$resource_name" service="$service";``` - In addition to built-in NGINX variables, you can also use the variables that the Ingress Controller configures: - $resource_type - The type of the k8s object. - $resource_name - The name of the k8s resource. - $resource_namespace - The namespace the k8s object exists in. - $service - The service for the k8s object. - **note** these are only available for VirtualServer and Ingress resources + - `` * - ``log-format-escaping`` - Sets the characters escaping for the variables of the log format. Supported values: ``json`` (JSON escaping), ``default`` (the default escaping) ``none`` (disables escaping). - ``default`` diff --git a/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml b/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml index d6cdb5d670..2ced7c188c 100644 --- a/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml +++ b/examples-of-custom-resources/basic-tcp-udp/global-configuration.yaml @@ -2,6 +2,7 @@ apiVersion: k8s.nginx.org/v1alpha1 kind: GlobalConfiguration metadata: name: nginx-configuration + namespace: nginx-ingress spec: listeners: - name: dns-udp diff --git a/examples/complete-example/cafe-ingress.yaml b/examples/complete-example/cafe-ingress.yaml index 53c3205e59..d1bf9b2a6a 100644 --- a/examples/complete-example/cafe-ingress.yaml +++ b/examples/complete-example/cafe-ingress.yaml @@ -4,7 +4,7 @@ metadata: name: cafe-ingress namespace: nginx-ingress spec: - ingressClassName: nginx # use only with k8s version >= 1.18.0 + # ingressClassName: nginx # use only with k8s version >= 1.18.0 tls: - hosts: - cafe.example.com diff --git a/examples/custom-log-format/README.md b/examples/custom-log-format/README.md new file mode 100644 index 0000000000..dc2cfee4c9 --- /dev/null +++ b/examples/custom-log-format/README.md @@ -0,0 +1,22 @@ +# Custom NGINX log format + +This example lets you set the log-format for NGINX using the configmap reosurce + +```yaml +kind: ConfigMap +apiVersion: v1 +metadata: + name: nginx-config +data: + log-format: | + compression '$remote_addr - $remote_user [$time_local] ' + '"$request" $status $bytes_sent ' + '"$http_referer" "$http_user_agent" "$gzip_ratio"' +``` + +In addition to the built-in NGINX variables, you can also use the variables that the Ingress Controller configures: + +- $resource_type - The type of k8s resource. +- $resource_name - The name of the k8s resource +- $resource_namespace - The namespace the resource exists in. +- $service - The service that exposes the resource. From a19b394e10dfa2f29242eae2702e972b8493fc9c Mon Sep 17 00:00:00 2001 From: Lorcan McVeigh Date: Wed, 25 Nov 2020 15:37:07 +0000 Subject: [PATCH 12/16] Update log Co-authored-by: Dean Coakley --- .../configuration/global-configuration/configmap-resource.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs-web/configuration/global-configuration/configmap-resource.md b/docs-web/configuration/global-configuration/configmap-resource.md index 3a8b35af40..ca19005e97 100644 --- a/docs-web/configuration/global-configuration/configmap-resource.md +++ b/docs-web/configuration/global-configuration/configmap-resource.md @@ -205,7 +205,7 @@ See the doc about [VirtualServer and VirtualServerRoute resources](/nginx-ingres * - ``log-format`` - Sets the custom `log format `_ for HTTP and HTTPS traffic. For convenience, it is possible to define the log format across multiple lines (each line separated by ``\n``). In that case, the Ingress Controller will replace every ``\n`` character with a space character. All ``'`` characters must be escaped. - See the `template file `_ for the access log. - - `` + - `Custom Log Format `_. * - ``log-format-escaping`` - Sets the characters escaping for the variables of the log format. Supported values: ``json`` (JSON escaping), ``default`` (the default escaping) ``none`` (disables escaping). - ``default`` From a168390fd2e943ee9f1770f0eec35d683eb96d07 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Wed, 25 Nov 2020 16:46:03 +0000 Subject: [PATCH 13/16] Remove namespace from ingress example --- examples/complete-example/cafe-ingress.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/complete-example/cafe-ingress.yaml b/examples/complete-example/cafe-ingress.yaml index d1bf9b2a6a..2eca43d223 100644 --- a/examples/complete-example/cafe-ingress.yaml +++ b/examples/complete-example/cafe-ingress.yaml @@ -2,7 +2,6 @@ apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: name: cafe-ingress - namespace: nginx-ingress spec: # ingressClassName: nginx # use only with k8s version >= 1.18.0 tls: From 877ab00aa572b96799d08392d8e8a003143d8cd3 Mon Sep 17 00:00:00 2001 From: Lorcan McVeigh Date: Thu, 26 Nov 2020 10:00:10 +0000 Subject: [PATCH 14/16] Update variables Co-authored-by: Dean Coakley --- internal/configs/virtualserver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 38481472a7..bf1b5aeaf6 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -359,7 +359,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( var vsrErrorPagesRouteIndex = make(map[string]int) var vsrLocationSnippetsFromVs = make(map[string]string) var vsrPoliciesFromVs = make(map[string][]conf_v1.PolicyReference) - IsVSR := false + isVSR := false matchesRoutes := 0 variableNamer := newVariableNamer(vsEx.VirtualServer) From 8d809e4cc4c75824c6599627f57b29ce2e1078b6 Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Thu, 26 Nov 2020 13:44:16 +0000 Subject: [PATCH 15/16] Update variables name --- internal/configs/virtualserver.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index bf1b5aeaf6..e5e1572ee0 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -451,7 +451,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( 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, "", "") + errorPageIndex, proxySSLName, r.Path, vsLocSnippets, vsc.enableSnippets, len(returnLocations), isVSR, "", "") addPoliciesCfgToLocation(routePoliciesCfg, &loc) locations = append(locations, loc) @@ -463,7 +463,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( // generate config for subroutes of each VirtualServerRoute for _, vsr := range vsEx.VirtualServerRoutes { - IsVSR := true + isVSR := true upstreamNamer := newUpstreamNamerForVirtualServerRoute(vsEx.VirtualServer, vsr) for _, r := range vsr.Spec.Subroutes { errorPageIndex := len(errorPageLocations) @@ -555,7 +555,7 @@ func (vsc *virtualServerConfigurator) GenerateVirtualServerConfig( 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) + errorPageIndex, proxySSLName, r.Path, locSnippets, vsc.enableSnippets, len(returnLocations), isVSR, vsr.Name, vsr.Namespace) addPoliciesCfgToLocation(routePoliciesCfg, &loc) locations = append(locations, loc) @@ -1266,7 +1266,7 @@ func generateReturnBlock(text string, code int, defaultCode int) *version2.Retur 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) { + originalPath string, locSnippets string, enableSnippets bool, retLocIndex int, isVSR bool, vsrName string, vsrNamespace string) (version2.Location, *version2.ReturnLocation) { locationSnippets := generateSnippets(enableSnippets, locSnippets, cfgParams.LocationSnippets) @@ -1279,7 +1279,7 @@ func generateLocation(path string, upstreamName string, upstream conf_v1.Upstrea } return generateLocationForProxying(path, upstreamName, upstream, cfgParams, errorPages, internal, - errPageIndex, proxySSLName, action.Proxy, originalPath, locationSnippets, IsVSR, vsrName, vsrNamespace), nil + errPageIndex, proxySSLName, action.Proxy, originalPath, locationSnippets, isVSR, vsrName, vsrNamespace), nil } func generateProxySetHeaders(proxy *conf_v1.ActionProxy) []version2.Header { @@ -1479,7 +1479,7 @@ func generateSplits( ) (version2.SplitClient, []version2.Location, []version2.ReturnLocation) { var distributions []version2.Distribution - IsVSR := false + isVSR := false vsrName := "" vsrNamespace := "" @@ -1507,7 +1507,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) + errPageIndex, proxySSLName, originalPath, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -1622,7 +1622,7 @@ func generateMatchesConfig(route conf_v1.Route, upstreamNamer *upstreamNamer, cr var locations []version2.Location var returnLocations []version2.ReturnLocation var splitClients []version2.SplitClient - IsVSR := false + isVSR := false vsrName := "" vsrNamespace := "" scLocalIndex = 0 @@ -1655,7 +1655,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) + errPageIndex, proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) @@ -1690,7 +1690,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) + errPageIndex, proxySSLName, route.Path, locSnippets, enableSnippets, newRetLocIndex, isVSR, vsrName, vsrNamespace) locations = append(locations, loc) if returnLoc != nil { returnLocations = append(returnLocations, *returnLoc) From 7ca07ae4ab44a7364462d4241f4f62f0f35423ba Mon Sep 17 00:00:00 2001 From: LorcanMcVeigh Date: Mon, 30 Nov 2020 12:10:46 +0000 Subject: [PATCH 16/16] Fix error in template files --- internal/configs/version1/nginx-plus.ingress.tmpl | 4 ++-- internal/configs/version1/nginx.ingress.tmpl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/configs/version1/nginx-plus.ingress.tmpl b/internal/configs/version1/nginx-plus.ingress.tmpl index 5e04111864..6cb76db497 100644 --- a/internal/configs/version1/nginx-plus.ingress.tmpl +++ b/internal/configs/version1/nginx-plus.ingress.tmpl @@ -150,8 +150,8 @@ server { set $service "{{$location.ServiceName}}"; {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} - set resource_name "{{$location.MinionIngress.Name}}"; - set resource_namespace "{{$location.MinionIngress.Namespace}}"; + set $resource_name "{{$location.MinionIngress.Name}}"; + set $resource_namespace "{{$location.MinionIngress.Namespace}}"; {{end}} {{if $location.GRPC}} {{if not $server.GRPCOnly}} diff --git a/internal/configs/version1/nginx.ingress.tmpl b/internal/configs/version1/nginx.ingress.tmpl index 1d86aefb96..8f3162ca9a 100644 --- a/internal/configs/version1/nginx.ingress.tmpl +++ b/internal/configs/version1/nginx.ingress.tmpl @@ -91,8 +91,8 @@ server { set $service "{{$location.ServiceName}}"; {{with $location.MinionIngress}} # location for minion {{$location.MinionIngress.Namespace}}/{{$location.MinionIngress.Name}} - set resource_name "{{$location.MinionIngress.Name}}"; - set resource_namespace "{{$location.MinionIngress.Namespace}}"; + set $resource_name "{{$location.MinionIngress.Name}}"; + set $resource_namespace "{{$location.MinionIngress.Namespace}}"; {{end}} {{if $location.GRPC}} {{if not $server.GRPCOnly}}