From a2b5bea372aeaa869d2a0361d08e4c5ab54d77b1 Mon Sep 17 00:00:00 2001 From: Ricardo Katz Date: Wed, 13 Nov 2024 09:55:45 -0700 Subject: [PATCH] Fix golang-lint findings --- .golangci.yml | 1 - .../ingress/controller/controller_test.go | 2 +- .../template/crossplane/authlocation.go | 3 +- .../controller/template/crossplane/cors.go | 5 ++- .../crossplane/extramodules/analyze.go | 2 ++ .../controller/template/crossplane/http.go | 30 ++++++++-------- .../template/crossplane/location.go | 35 ++++--------------- .../controller/template/crossplane/server.go | 6 ++-- .../controller/template/crossplane/utils.go | 27 +++++++------- .../ingress/controller/template/template.go | 2 +- test/e2e/annotations/canary.go | 1 - test/e2e/annotations/cors.go | 1 - test/e2e/settings/proxy_host.go | 2 -- 13 files changed, 46 insertions(+), 71 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2d73e14e7..feaf26b28 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -24,7 +24,6 @@ linters: - errname - ginkgolinter - gocheckcompilerdirectives - - goconst - gocritic - gocyclo - godox diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index a830a5729..022f23e8e 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -157,7 +157,7 @@ func (ntc testNginxTestCommand) Test(cfg string) ([]byte, error) { type fakeTemplate struct{} -func (fakeTemplate) Validate(filename string) error { +func (fakeTemplate) Validate(_ string) error { return nil } diff --git a/internal/ingress/controller/template/crossplane/authlocation.go b/internal/ingress/controller/template/crossplane/authlocation.go index ef0dd8e17..113f28878 100644 --- a/internal/ingress/controller/template/crossplane/authlocation.go +++ b/internal/ingress/controller/template/crossplane/authlocation.go @@ -86,7 +86,8 @@ func buildExternalAuth(cfg any) *externalAuth { } func (c *Template) buildAuthLocation(server *ingress.Server, - location *ingress.Location, locationConfig locationCfg) *ngx_crossplane.Directive { + location *ingress.Location, locationConfig locationCfg, +) *ngx_crossplane.Directive { locationDirectives := ngx_crossplane.Directives{ buildDirective("internal"), } diff --git a/internal/ingress/controller/template/crossplane/cors.go b/internal/ingress/controller/template/crossplane/cors.go index 932f25dbb..252fa1702 100644 --- a/internal/ingress/controller/template/crossplane/cors.go +++ b/internal/ingress/controller/template/crossplane/cors.go @@ -23,11 +23,10 @@ import ( "k8s.io/ingress-nginx/internal/ingress/annotations/cors" ) -func buildCorsDirectives(locationcors cors.Config) ngx_crossplane.Directives { +func buildCorsDirectives(locationcors *cors.Config) ngx_crossplane.Directives { directives := make(ngx_crossplane.Directives, 0) if len(locationcors.CorsAllowOrigin) > 0 { directives = append(directives, buildCorsOriginRegex(locationcors.CorsAllowOrigin)...) - } directives = append(directives, buildBlockDirective("if", @@ -43,7 +42,7 @@ func buildCorsDirectives(locationcors cors.Config) ngx_crossplane.Directives { } // commonCorsDirective builds the common cors directives for a location -func commonCorsDirective(cfg cors.Config, options bool) *ngx_crossplane.Directive { +func commonCorsDirective(cfg *cors.Config, options bool) *ngx_crossplane.Directive { corsDir := "true" if options { corsDir = "trueoptions" diff --git a/internal/ingress/controller/template/crossplane/extramodules/analyze.go b/internal/ingress/controller/template/crossplane/extramodules/analyze.go index 22d227d30..0b6d2f335 100644 --- a/internal/ingress/controller/template/crossplane/extramodules/analyze.go +++ b/internal/ingress/controller/template/crossplane/extramodules/analyze.go @@ -70,6 +70,8 @@ const ( // helpful directive location alias describing "any" context // doesn't include ngxHTTPSifConf, ngxHTTPLifConf, ngxHTTPLmtConf, or ngxMgmtMainConf. +// +//nolint:unused // This file is generated const ngxAnyConf = ngxMainConf | ngxEventConf | ngxMailMainConf | ngxMailSrvConf | ngxStreamMainConf | ngxStreamSrvConf | ngxStreamUpsConf | ngxHTTPMainConf | ngxHTTPSrvConf | ngxHTTPLocConf | ngxHTTPUpsConf | diff --git a/internal/ingress/controller/template/crossplane/http.go b/internal/ingress/controller/template/crossplane/http.go index 381ef67a4..02ff72c9a 100644 --- a/internal/ingress/controller/template/crossplane/http.go +++ b/internal/ingress/controller/template/crossplane/http.go @@ -85,7 +85,7 @@ func (c *Template) initHTTPDirectives() ngx_crossplane.Directives { return httpBlock } -//nolint:gocyclo +//nolint:gocyclo // Function is what it is func (c *Template) buildHTTP() { cfg := c.tplConfig.Cfg httpBlock := c.initHTTPDirectives() @@ -140,10 +140,10 @@ func (c *Template) buildHTTP() { } if cfg.EnableBrotli { - httpBlock = append(httpBlock, buildDirective("brotli", "on")) - httpBlock = append(httpBlock, buildDirective("brotli_comp_level", cfg.BrotliLevel)) - httpBlock = append(httpBlock, buildDirective("brotli_min_length", cfg.BrotliMinLength)) - httpBlock = append(httpBlock, buildDirective("brotli_types", cfg.BrotliTypes)) + httpBlock = append(httpBlock, buildDirective("brotli", "on"), + buildDirective("brotli_comp_level", cfg.BrotliLevel), + buildDirective("brotli_min_length", cfg.BrotliMinLength), + buildDirective("brotli_types", cfg.BrotliTypes)) } if (c.tplConfig.Cfg.EnableOpentelemetry || shouldLoadOpentelemetryModule(c.tplConfig.Servers)) && @@ -293,16 +293,17 @@ func (c *Template) buildHTTP() { httpBlock = append(httpBlock, buildBlockDirective("upstream", []string{"upstream_balancer"}, blockUpstreamDirectives)) // Adding Rate limit - for _, rl := range filterRateLimits(c.tplConfig.Servers) { - id := fmt.Sprintf("$allowlist_%s", rl.ID) - httpBlock = append(httpBlock, buildDirective("#", "Ratelimit", rl.Name)) + rl := filterRateLimits(c.tplConfig.Servers) + for i := range rl { + id := fmt.Sprintf("$allowlist_%s", rl[i].ID) + httpBlock = append(httpBlock, buildDirective("#", "Ratelimit", rl[i].Name)) rlDirectives := ngx_crossplane.Directives{ buildDirective("default", 0), } - for _, ip := range rl.Allowlist { + for _, ip := range rl[i].Allowlist { rlDirectives = append(rlDirectives, buildDirective(ip, "1")) } - mapRateLimitDirective := buildMapDirective(id, fmt.Sprintf("$limit_%s", rl.ID), ngx_crossplane.Directives{ + mapRateLimitDirective := buildMapDirective(id, fmt.Sprintf("$limit_%s", rl[i].ID), ngx_crossplane.Directives{ buildDirective("0", cfg.LimitConnZoneVariable), buildDirective("1", ""), }) @@ -343,10 +344,11 @@ func (c *Template) buildHTTP() { if redirectServers, ok := c.tplConfig.RedirectServers.([]*utilingress.Redirect); ok { for _, server := range redirectServers { - httpBlock = append(httpBlock, buildStartServer(server.From)) - serverBlock := c.buildRedirectServer(server) - httpBlock = append(httpBlock, serverBlock) - httpBlock = append(httpBlock, buildEndServer(server.From)) + httpBlock = append(httpBlock, + buildStartServer(server.From), + c.buildRedirectServer(server), + buildEndServer(server.From), + ) } } diff --git a/internal/ingress/controller/template/crossplane/location.go b/internal/ingress/controller/template/crossplane/location.go index ddb28a7b1..bc7c5685e 100644 --- a/internal/ingress/controller/template/crossplane/location.go +++ b/internal/ingress/controller/template/crossplane/location.go @@ -107,7 +107,6 @@ func buildCustomErrorLocationsPerServer(server *ingress.Server, enableMetrics bo errorLocationsDirectives = append(errorLocationsDirectives, buildCustomErrorLocation(errorLocations[i].UpstreamName, errorLocations[i].Codes, enableMetrics)...) } return errorLocationsDirectives - } func buildCustomErrorLocation(upstreamName string, errorCodes []int, enableMetrics bool) ngx_crossplane.Directives { @@ -199,7 +198,7 @@ func (c *Template) buildServerLocations(server *ingress.Server, locations []*ing buildDirective("add_header", "Set-Cookie", "$auth_cookie"), } if location.CorsConfig.CorsEnabled { - directives = append(directives, buildCorsDirectives(location.CorsConfig)...) + directives = append(directives, buildCorsDirectives(&location.CorsConfig)...) } directives = append(directives, buildDirective("return", @@ -208,17 +207,15 @@ func (c *Template) buildServerLocations(server *ingress.Server, locations []*ing serverLocations = append(serverLocations, buildBlockDirective("location", []string{buildAuthSignURLLocation(location.Path, locationConfig.externalAuth.SigninURL)}, directives)) - } serverLocations = append(serverLocations, c.buildLocation(server, location, locationConfig)) - } - return serverLocations } func (c *Template) buildLocation(server *ingress.Server, - location *ingress.Location, locationConfig locationCfg) *ngx_crossplane.Directive { + location *ingress.Location, locationConfig locationCfg, +) *ngx_crossplane.Directive { ing := getIngressInformation(location.Ingress, server.Hostname, location.IngressPath) cfg := c.tplConfig locationDirectives := ngx_crossplane.Directives{ @@ -294,7 +291,7 @@ func (c *Template) buildAllowedLocation(server *ingress.Server, location *ingres } if location.CorsConfig.CorsEnabled { - dir = append(dir, buildCorsDirectives(location.CorsConfig)...) + dir = append(dir, buildCorsDirectives(&location.CorsConfig)...) } if !isLocationInLocationList(location, c.tplConfig.Cfg.NoAuthLocations) { @@ -686,8 +683,8 @@ func buildAuthLocationConfig(location *ingress.Location, locationConfig location directives := make(ngx_crossplane.Directives, 0) if locationConfig.authPath != "" { if locationConfig.applyAuthUpstream && !locationConfig.applyGlobalAuth { - directives = append(directives, buildDirective("set", "$auth_cookie", "")) - directives = append(directives, buildDirective("add_header", "Set-Cookie", "$auth_cookie")) + directives = append(directives, buildDirective("set", "$auth_cookie", ""), + buildDirective("add_header", "Set-Cookie", "$auth_cookie")) directives = append(directives, buildAuthResponseHeaders(locationConfig.proxySetHeader, locationConfig.externalAuth.ResponseHeaders, true)...) if len(locationConfig.externalAuth.ResponseHeaders) > 0 { directives = append(directives, buildDirective("set", "$auth_response_headers", strings.Join(locationConfig.externalAuth.ResponseHeaders, ","))) @@ -733,24 +730,4 @@ func buildAuthLocationConfig(location *ingress.Location, locationConfig location } return directives - /* - Missing this Lua script - # `auth_request` module does not support HTTP keepalives in upstream block: - # https://trac.nginx.org/nginx/ticket/1579 - access_by_lua_block { - local res = ngx.location.capture('{{ $authPath }}', { method = ngx.HTTP_GET, body = '', share_all_vars = {{ $externalAuth.KeepaliveShareVars }} }) - if res.status == ngx.HTTP_OK then - ngx.var.auth_cookie = res.header['Set-Cookie'] - {{- range $line := buildAuthUpstreamLuaHeaders $externalAuth.ResponseHeaders }} # IF 4 - {{ $line }} - {{- end }} # END IF 4 - return - end - if res.status == ngx.HTTP_UNAUTHORIZED or res.status == ngx.HTTP_FORBIDDEN then - ngx.exit(res.status) - end - ngx.exit(ngx.HTTP_INTERNAL_SERVER_ERROR) - } - - */ } diff --git a/internal/ingress/controller/template/crossplane/server.go b/internal/ingress/controller/template/crossplane/server.go index c81003b0c..a78a5a351 100644 --- a/internal/ingress/controller/template/crossplane/server.go +++ b/internal/ingress/controller/template/crossplane/server.go @@ -37,7 +37,7 @@ func (c *Template) buildServerDirective(server *ingress.Server) *ngx_crossplane. buildDirective("ssl_certificate_by_lua_file", "/etc/nginx/lua/nginx/ngx_conf_certificate.lua"), } - serverBlock = append(serverBlock, buildListener(*c.tplConfig, server.Hostname)...) + serverBlock = append(serverBlock, buildListener(c.tplConfig, server.Hostname)...) serverBlock = append(serverBlock, c.buildBlockers()...) if server.Hostname == "_" { @@ -62,7 +62,6 @@ func (c *Template) buildServerDirective(server *ingress.Server) *ngx_crossplane. // The other locations should come here! serverBlock = append(serverBlock, c.buildServerLocations(server, server.Locations)...) - } // "/healthz" location @@ -101,7 +100,6 @@ func (c *Template) buildServerDirective(server *ingress.Server) *ngx_crossplane. // End of "nginx_status" location serverBlock = append(serverBlock, buildBlockDirective("location", []string{"/nginx_status"}, statusLocationDirs)) - } // DO NOT MOVE! THIS IS THE END DIRECTIVE OF SERVERS @@ -167,7 +165,7 @@ func (c *Template) buildRedirectServer(server *utilingress.Redirect) *ngx_crossp buildDirective("ssl_certificate_by_lua_file", "/etc/nginx/lua/nginx/ngx_conf_certificate.lua"), buildDirective("set_by_lua_file", "$redirect_to", "/etc/nginx/lua/nginx/ngx_srv_redirect.lua", server.To), } - serverBlock = append(serverBlock, buildListener(*c.tplConfig, server.From)...) + serverBlock = append(serverBlock, buildListener(c.tplConfig, server.From)...) serverBlock = append(serverBlock, c.buildBlockers()...) serverBlock = append(serverBlock, buildDirective("return", c.tplConfig.Cfg.HTTPRedirectCode, "$redirect_to")) diff --git a/internal/ingress/controller/template/crossplane/utils.go b/internal/ingress/controller/template/crossplane/utils.go index 74b09e6f0..eef9b87ca 100644 --- a/internal/ingress/controller/template/crossplane/utils.go +++ b/internal/ingress/controller/template/crossplane/utils.go @@ -17,7 +17,7 @@ limitations under the License. package crossplane import ( - "crypto/sha1" + "crypto/sha1" //nolint:gosec // We cannot move away from sha1 "encoding/base64" "encoding/hex" "fmt" @@ -59,10 +59,12 @@ var ( defaultGlobalAuthRedirectParam = "rd" ) -type seconds int -type minutes int +type ( + seconds int + minutes int +) -func buildDirectiveWithComment(directive string, comment string, args ...any) *ngx_crossplane.Directive { +func buildDirectiveWithComment(directive, comment string, args ...any) *ngx_crossplane.Directive { dir := buildDirective(directive, args...) dir.Comment = ptr.To(comment) return dir @@ -213,25 +215,25 @@ func buildServerName(hostname string) string { return `~^(?[\w-]+)\.` + strings.Join(parts, "\\.") + `$` } -func buildListener(tc config.TemplateConfig, hostname string) ngx_crossplane.Directives { +func buildListener(tc *config.TemplateConfig, hostname string) ngx_crossplane.Directives { listenDirectives := make(ngx_crossplane.Directives, 0) - co := commonListenOptions(&tc, hostname) + co := commonListenOptions(tc, hostname) addrV4 := []string{""} if len(tc.Cfg.BindAddressIpv4) > 0 { addrV4 = tc.Cfg.BindAddressIpv4 } - listenDirectives = append(listenDirectives, httpListener(addrV4, co, &tc, false)...) - listenDirectives = append(listenDirectives, httpListener(addrV4, co, &tc, true)...) + listenDirectives = append(listenDirectives, httpListener(addrV4, co, tc, false)...) + listenDirectives = append(listenDirectives, httpListener(addrV4, co, tc, true)...) if tc.IsIPV6Enabled { addrV6 := []string{"[::]"} if len(tc.Cfg.BindAddressIpv6) > 0 { addrV6 = tc.Cfg.BindAddressIpv6 } - listenDirectives = append(listenDirectives, httpListener(addrV6, co, &tc, false)...) - listenDirectives = append(listenDirectives, httpListener(addrV6, co, &tc, true)...) + listenDirectives = append(listenDirectives, httpListener(addrV6, co, tc, false)...) + listenDirectives = append(listenDirectives, httpListener(addrV6, co, tc, true)...) } return listenDirectives @@ -258,7 +260,7 @@ func commonListenOptions(template *config.TemplateConfig, hostname string) []str return out } -func httpListener(addresses []string, co []string, tc *config.TemplateConfig, ssl bool) ngx_crossplane.Directives { +func httpListener(addresses, co []string, tc *config.TemplateConfig, ssl bool) ngx_crossplane.Directives { listeners := make(ngx_crossplane.Directives, 0) port := tc.ListenPorts.HTTP isTLSProxy := tc.IsSSLPassthroughEnabled @@ -400,7 +402,7 @@ func changeHostPort(newURL, value string) string { } func buildAuthSignURLLocation(location, authSignURL string) string { - hasher := sha1.New() // #nosec + hasher := sha1.New() //nolint:gosec // We cannot move away from sha1 hasher.Write([]byte(location)) hasher.Write([]byte(authSignURL)) return "@" + hex.EncodeToString(hasher.Sum(nil)) @@ -558,7 +560,6 @@ func buildProxyPass(backends []*ingress.Backend, location *ingress.Location) ngx } func buildGeoIPDirectives(reloadTime int, files []string) ngx_crossplane.Directives { - directives := make(ngx_crossplane.Directives, 0) buildGeoIPBlock := func(file string, directives ngx_crossplane.Directives) *ngx_crossplane.Directive { if reloadTime > 0 && file != "GeoIP2-Connection-Type.mmdb" { diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index e420b9bcf..b976ec142 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -234,7 +234,7 @@ type LuaListenPorts struct { } // Validate is no-op at go-template -func (t *Template) Validate(filename string) error { +func (t *Template) Validate(_ string) error { return nil } diff --git a/test/e2e/annotations/canary.go b/test/e2e/annotations/canary.go index 443a3a486..3632ed696 100644 --- a/test/e2e/annotations/canary.go +++ b/test/e2e/annotations/canary.go @@ -1106,7 +1106,6 @@ var _ = framework.DescribeAnnotation("canary-*", func() { !strings.Contains(server, `set $proxy_upstream_name "pstream-default-backend;`) && !strings.Contains(server, canaryUpstreamNameCrossplane) && strings.Contains(server, upstreamNameCrossplane)) - }) }) diff --git a/test/e2e/annotations/cors.go b/test/e2e/annotations/cors.go index 1db7d5374..57206b2e6 100644 --- a/test/e2e/annotations/cors.go +++ b/test/e2e/annotations/cors.go @@ -86,7 +86,6 @@ var _ = framework.DescribeAnnotation("cors-*", func() { func(server string) bool { return strings.Contains(server, "more_set_headers 'Access-Control-Allow-Methods: POST, GET';") || strings.Contains(server, `more_set_headers "Access-Control-Allow-Methods: POST, GET";`) - }) }) diff --git a/test/e2e/settings/proxy_host.go b/test/e2e/settings/proxy_host.go index 1fcda11a1..afb37c372 100644 --- a/test/e2e/settings/proxy_host.go +++ b/test/e2e/settings/proxy_host.go @@ -34,7 +34,6 @@ var _ = framework.IngressNginxDescribe("Dynamic $proxy_host", func() { }) ginkgo.It("should exist a proxy_host", func() { - h := make(map[string]string) h["Custom-Header"] = "$proxy_host" cfgMap := "add-headers-configmap" @@ -60,7 +59,6 @@ var _ = framework.IngressNginxDescribe("Dynamic $proxy_host", func() { }) ginkgo.It("should exist a proxy_host using the upstream-vhost annotation value", func() { - h := make(map[string]string) h["Custom-Header"] = "$proxy_host" cfgMap := "add-headers-configmap"