DevFW-CICD/ingress-nginx-helm
16
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix golang-lint findings

Browse source
This commit is contained in:
Ricardo Katz 2024-11-13 09:55:45 -07:00
parent bc232c355e
commit a2b5bea372
13 changed files with 46 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.golangci.yml
View file

@ -24,7 +24,6 @@ linters:
- errname - errname
- ginkgolinter - ginkgolinter
- gocheckcompilerdirectives - gocheckcompilerdirectives
- goconst
- gocritic - gocritic
- gocyclo - gocyclo
- godox - godox

2
internal/ingress/controller/controller_test.go
View file

@ -157,7 +157,7 @@ func (ntc testNginxTestCommand) Test(cfg string) ([]byte, error) {
type fakeTemplate struct{} type fakeTemplate struct{}
func (fakeTemplate) Validate(filename string) error { func (fakeTemplate) Validate(_ string) error {
return nil return nil
} }

3
internal/ingress/controller/template/crossplane/authlocation.go
View file

@ -86,7 +86,8 @@ func buildExternalAuth(cfg any) *externalAuth {
} }
func (c *Template) buildAuthLocation(server *ingress.Server, 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{ locationDirectives := ngx_crossplane.Directives{
buildDirective("internal"), buildDirective("internal"),
} }

5
internal/ingress/controller/template/crossplane/cors.go
View file

@ -23,11 +23,10 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/cors" "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) directives := make(ngx_crossplane.Directives, 0)
if len(locationcors.CorsAllowOrigin) > 0 { if len(locationcors.CorsAllowOrigin) > 0 {
directives = append(directives, buildCorsOriginRegex(locationcors.CorsAllowOrigin)...) directives = append(directives, buildCorsOriginRegex(locationcors.CorsAllowOrigin)...)
} }
directives = append(directives, directives = append(directives,
buildBlockDirective("if", buildBlockDirective("if",
@ -43,7 +42,7 @@ func buildCorsDirectives(locationcors cors.Config) ngx_crossplane.Directives {
} }
// commonCorsDirective builds the common cors directives for a location // 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" corsDir := "true"
if options { if options {
corsDir = "trueoptions" corsDir = "trueoptions"

2
internal/ingress/controller/template/crossplane/extramodules/analyze.go
View file

@ -70,6 +70,8 @@ const (
// helpful directive location alias describing "any" context // helpful directive location alias describing "any" context
// doesn't include ngxHTTPSifConf, ngxHTTPLifConf, ngxHTTPLmtConf, or ngxMgmtMainConf. // doesn't include ngxHTTPSifConf, ngxHTTPLifConf, ngxHTTPLmtConf, or ngxMgmtMainConf.
//
//nolint:unused // This file is generated
const ngxAnyConf = ngxMainConf | ngxEventConf | ngxMailMainConf | ngxMailSrvConf | const ngxAnyConf = ngxMainConf | ngxEventConf | ngxMailMainConf | ngxMailSrvConf |
ngxStreamMainConf | ngxStreamSrvConf | ngxStreamUpsConf | ngxStreamMainConf | ngxStreamSrvConf | ngxStreamUpsConf |
ngxHTTPMainConf | ngxHTTPSrvConf | ngxHTTPLocConf | ngxHTTPUpsConf | ngxHTTPMainConf | ngxHTTPSrvConf | ngxHTTPLocConf | ngxHTTPUpsConf |

30
internal/ingress/controller/template/crossplane/http.go
View file

@ -85,7 +85,7 @@ func (c *Template) initHTTPDirectives() ngx_crossplane.Directives {
return httpBlock return httpBlock
} }
//nolint:gocyclo //nolint:gocyclo // Function is what it is
func (c *Template) buildHTTP() { func (c *Template) buildHTTP() {
cfg := c.tplConfig.Cfg cfg := c.tplConfig.Cfg
httpBlock := c.initHTTPDirectives() httpBlock := c.initHTTPDirectives()
@ -140,10 +140,10 @@ func (c *Template) buildHTTP() {
} }
if cfg.EnableBrotli { if cfg.EnableBrotli {
httpBlock = append(httpBlock, buildDirective("brotli", "on")) httpBlock = append(httpBlock, buildDirective("brotli", "on"),
httpBlock = append(httpBlock, buildDirective("brotli_comp_level", cfg.BrotliLevel)) buildDirective("brotli_comp_level", cfg.BrotliLevel),
httpBlock = append(httpBlock, buildDirective("brotli_min_length", cfg.BrotliMinLength)) buildDirective("brotli_min_length", cfg.BrotliMinLength),
httpBlock = append(httpBlock, buildDirective("brotli_types", cfg.BrotliTypes)) buildDirective("brotli_types", cfg.BrotliTypes))
} }
if (c.tplConfig.Cfg.EnableOpentelemetry || shouldLoadOpentelemetryModule(c.tplConfig.Servers)) && 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)) httpBlock = append(httpBlock, buildBlockDirective("upstream", []string{"upstream_balancer"}, blockUpstreamDirectives))
// Adding Rate limit // Adding Rate limit
for _, rl := range filterRateLimits(c.tplConfig.Servers) { rl := filterRateLimits(c.tplConfig.Servers)
id := fmt.Sprintf("$allowlist_%s", rl.ID) for i := range rl {
httpBlock = append(httpBlock, buildDirective("#", "Ratelimit", rl.Name)) id := fmt.Sprintf("$allowlist_%s", rl[i].ID)
httpBlock = append(httpBlock, buildDirective("#", "Ratelimit", rl[i].Name))
rlDirectives := ngx_crossplane.Directives{ rlDirectives := ngx_crossplane.Directives{
buildDirective("default", 0), buildDirective("default", 0),
} }
for _, ip := range rl.Allowlist { for _, ip := range rl[i].Allowlist {
rlDirectives = append(rlDirectives, buildDirective(ip, "1")) 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("0", cfg.LimitConnZoneVariable),
buildDirective("1", ""), buildDirective("1", ""),
}) })
@ -343,10 +344,11 @@ func (c *Template) buildHTTP() {
if redirectServers, ok := c.tplConfig.RedirectServers.([]*utilingress.Redirect); ok { if redirectServers, ok := c.tplConfig.RedirectServers.([]*utilingress.Redirect); ok {
for _, server := range redirectServers { for _, server := range redirectServers {
httpBlock = append(httpBlock, buildStartServer(server.From)) httpBlock = append(httpBlock,
serverBlock := c.buildRedirectServer(server) buildStartServer(server.From),
httpBlock = append(httpBlock, serverBlock) c.buildRedirectServer(server),
httpBlock = append(httpBlock, buildEndServer(server.From)) buildEndServer(server.From),
)
} }
} }

35
internal/ingress/controller/template/crossplane/location.go
View file

@ -107,7 +107,6 @@ func buildCustomErrorLocationsPerServer(server *ingress.Server, enableMetrics bo
errorLocationsDirectives = append(errorLocationsDirectives, buildCustomErrorLocation(errorLocations[i].UpstreamName, errorLocations[i].Codes, enableMetrics)...) errorLocationsDirectives = append(errorLocationsDirectives, buildCustomErrorLocation(errorLocations[i].UpstreamName, errorLocations[i].Codes, enableMetrics)...)
} }
return errorLocationsDirectives return errorLocationsDirectives
} }
func buildCustomErrorLocation(upstreamName string, errorCodes []int, enableMetrics bool) ngx_crossplane.Directives { 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"), buildDirective("add_header", "Set-Cookie", "$auth_cookie"),
} }
if location.CorsConfig.CorsEnabled { if location.CorsConfig.CorsEnabled {
directives = append(directives, buildCorsDirectives(location.CorsConfig)...) directives = append(directives, buildCorsDirectives(&location.CorsConfig)...)
} }
directives = append(directives, directives = append(directives,
buildDirective("return", buildDirective("return",
@ -208,17 +207,15 @@ func (c *Template) buildServerLocations(server *ingress.Server, locations []*ing
serverLocations = append(serverLocations, buildBlockDirective("location", serverLocations = append(serverLocations, buildBlockDirective("location",
[]string{buildAuthSignURLLocation(location.Path, locationConfig.externalAuth.SigninURL)}, directives)) []string{buildAuthSignURLLocation(location.Path, locationConfig.externalAuth.SigninURL)}, directives))
} }
serverLocations = append(serverLocations, c.buildLocation(server, location, locationConfig)) serverLocations = append(serverLocations, c.buildLocation(server, location, locationConfig))
} }
return serverLocations return serverLocations
} }
func (c *Template) buildLocation(server *ingress.Server, 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) ing := getIngressInformation(location.Ingress, server.Hostname, location.IngressPath)
cfg := c.tplConfig cfg := c.tplConfig
locationDirectives := ngx_crossplane.Directives{ locationDirectives := ngx_crossplane.Directives{
@ -294,7 +291,7 @@ func (c *Template) buildAllowedLocation(server *ingress.Server, location *ingres
} }
if location.CorsConfig.CorsEnabled { if location.CorsConfig.CorsEnabled {
dir = append(dir, buildCorsDirectives(location.CorsConfig)...) dir = append(dir, buildCorsDirectives(&location.CorsConfig)...)
} }
if !isLocationInLocationList(location, c.tplConfig.Cfg.NoAuthLocations) { if !isLocationInLocationList(location, c.tplConfig.Cfg.NoAuthLocations) {
@ -686,8 +683,8 @@ func buildAuthLocationConfig(location *ingress.Location, locationConfig location
directives := make(ngx_crossplane.Directives, 0) directives := make(ngx_crossplane.Directives, 0)
if locationConfig.authPath != "" { if locationConfig.authPath != "" {
if locationConfig.applyAuthUpstream && !locationConfig.applyGlobalAuth { if locationConfig.applyAuthUpstream && !locationConfig.applyGlobalAuth {
directives = append(directives, buildDirective("set", "$auth_cookie", "")) directives = append(directives, buildDirective("set", "$auth_cookie", ""),
directives = append(directives, buildDirective("add_header", "Set-Cookie", "$auth_cookie")) buildDirective("add_header", "Set-Cookie", "$auth_cookie"))
directives = append(directives, buildAuthResponseHeaders(locationConfig.proxySetHeader, locationConfig.externalAuth.ResponseHeaders, true)...) directives = append(directives, buildAuthResponseHeaders(locationConfig.proxySetHeader, locationConfig.externalAuth.ResponseHeaders, true)...)
if len(locationConfig.externalAuth.ResponseHeaders) > 0 { if len(locationConfig.externalAuth.ResponseHeaders) > 0 {
directives = append(directives, buildDirective("set", "$auth_response_headers", strings.Join(locationConfig.externalAuth.ResponseHeaders, ","))) 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 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)
}
*/
} }

6
internal/ingress/controller/template/crossplane/server.go
View file

@ -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"), 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()...) serverBlock = append(serverBlock, c.buildBlockers()...)
if server.Hostname == "_" { if server.Hostname == "_" {
@ -62,7 +62,6 @@ func (c *Template) buildServerDirective(server *ingress.Server) *ngx_crossplane.
// The other locations should come here! // The other locations should come here!
serverBlock = append(serverBlock, c.buildServerLocations(server, server.Locations)...) serverBlock = append(serverBlock, c.buildServerLocations(server, server.Locations)...)
} }
// "/healthz" location // "/healthz" location
@ -101,7 +100,6 @@ func (c *Template) buildServerDirective(server *ingress.Server) *ngx_crossplane.
// End of "nginx_status" location // End of "nginx_status" location
serverBlock = append(serverBlock, buildBlockDirective("location", []string{"/nginx_status"}, statusLocationDirs)) serverBlock = append(serverBlock, buildBlockDirective("location", []string{"/nginx_status"}, statusLocationDirs))
} }
// DO NOT MOVE! THIS IS THE END DIRECTIVE OF SERVERS // 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("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), 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, c.buildBlockers()...)
serverBlock = append(serverBlock, buildDirective("return", c.tplConfig.Cfg.HTTPRedirectCode, "$redirect_to")) serverBlock = append(serverBlock, buildDirective("return", c.tplConfig.Cfg.HTTPRedirectCode, "$redirect_to"))

27
internal/ingress/controller/template/crossplane/utils.go
View file

@ -17,7 +17,7 @@ limitations under the License.
package crossplane package crossplane
import ( import (
"crypto/sha1" "crypto/sha1" //nolint:gosec // We cannot move away from sha1
"encoding/base64" "encoding/base64"
"encoding/hex" "encoding/hex"
"fmt" "fmt"
@ -59,10 +59,12 @@ var (
defaultGlobalAuthRedirectParam = "rd" defaultGlobalAuthRedirectParam = "rd"
) )
type seconds int type (
type minutes int 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 := buildDirective(directive, args...)
dir.Comment = ptr.To(comment) dir.Comment = ptr.To(comment)
return dir return dir
@ -213,25 +215,25 @@ func buildServerName(hostname string) string {
return `~^(?<subdomain>[\w-]+)\.` + strings.Join(parts, "\\.") + `$` return `~^(?<subdomain>[\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) listenDirectives := make(ngx_crossplane.Directives, 0)
co := commonListenOptions(&tc, hostname) co := commonListenOptions(tc, hostname)
addrV4 := []string{""} addrV4 := []string{""}
if len(tc.Cfg.BindAddressIpv4) > 0 { if len(tc.Cfg.BindAddressIpv4) > 0 {
addrV4 = tc.Cfg.BindAddressIpv4 addrV4 = tc.Cfg.BindAddressIpv4
} }
listenDirectives = append(listenDirectives, httpListener(addrV4, co, &tc, false)...) listenDirectives = append(listenDirectives, httpListener(addrV4, co, tc, false)...)
listenDirectives = append(listenDirectives, httpListener(addrV4, co, &tc, true)...) listenDirectives = append(listenDirectives, httpListener(addrV4, co, tc, true)...)
if tc.IsIPV6Enabled { if tc.IsIPV6Enabled {
addrV6 := []string{"[::]"} addrV6 := []string{"[::]"}
if len(tc.Cfg.BindAddressIpv6) > 0 { if len(tc.Cfg.BindAddressIpv6) > 0 {
addrV6 = tc.Cfg.BindAddressIpv6 addrV6 = tc.Cfg.BindAddressIpv6
} }
listenDirectives = append(listenDirectives, httpListener(addrV6, co, &tc, false)...) listenDirectives = append(listenDirectives, httpListener(addrV6, co, tc, false)...)
listenDirectives = append(listenDirectives, httpListener(addrV6, co, &tc, true)...) listenDirectives = append(listenDirectives, httpListener(addrV6, co, tc, true)...)
} }
return listenDirectives return listenDirectives
@ -258,7 +260,7 @@ func commonListenOptions(template *config.TemplateConfig, hostname string) []str
return out 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) listeners := make(ngx_crossplane.Directives, 0)
port := tc.ListenPorts.HTTP port := tc.ListenPorts.HTTP
isTLSProxy := tc.IsSSLPassthroughEnabled isTLSProxy := tc.IsSSLPassthroughEnabled
@ -400,7 +402,7 @@ func changeHostPort(newURL, value string) string {
} }
func buildAuthSignURLLocation(location, authSignURL 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(location))
hasher.Write([]byte(authSignURL)) hasher.Write([]byte(authSignURL))
return "@" + hex.EncodeToString(hasher.Sum(nil)) 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 { func buildGeoIPDirectives(reloadTime int, files []string) ngx_crossplane.Directives {
directives := make(ngx_crossplane.Directives, 0) directives := make(ngx_crossplane.Directives, 0)
buildGeoIPBlock := func(file string, directives ngx_crossplane.Directives) *ngx_crossplane.Directive { buildGeoIPBlock := func(file string, directives ngx_crossplane.Directives) *ngx_crossplane.Directive {
if reloadTime > 0 && file != "GeoIP2-Connection-Type.mmdb" { if reloadTime > 0 && file != "GeoIP2-Connection-Type.mmdb" {

2
internal/ingress/controller/template/template.go
View file

@ -234,7 +234,7 @@ type LuaListenPorts struct {
} }
// Validate is no-op at go-template // Validate is no-op at go-template
func (t *Template) Validate(filename string) error { func (t *Template) Validate(_ string) error {
return nil return nil
} }

1
test/e2e/annotations/canary.go
View file

@ -1106,7 +1106,6 @@ var _ = framework.DescribeAnnotation("canary-*", func() {
!strings.Contains(server, `set $proxy_upstream_name "pstream-default-backend;`) && !strings.Contains(server, `set $proxy_upstream_name "pstream-default-backend;`) &&
!strings.Contains(server, canaryUpstreamNameCrossplane) && !strings.Contains(server, canaryUpstreamNameCrossplane) &&
strings.Contains(server, upstreamNameCrossplane)) strings.Contains(server, upstreamNameCrossplane))
}) })
}) })

1
test/e2e/annotations/cors.go
View file

@ -86,7 +86,6 @@ var _ = framework.DescribeAnnotation("cors-*", func() {
func(server string) bool { func(server string) bool {
return strings.Contains(server, "more_set_headers 'Access-Control-Allow-Methods: POST, GET';") || 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";`) strings.Contains(server, `more_set_headers "Access-Control-Allow-Methods: POST, GET";`)
}) })
}) })

2
test/e2e/settings/proxy_host.go
View file

@ -34,7 +34,6 @@ var _ = framework.IngressNginxDescribe("Dynamic $proxy_host", func() {
}) })
ginkgo.It("should exist a proxy_host", func() { ginkgo.It("should exist a proxy_host", func() {
h := make(map[string]string) h := make(map[string]string)
h["Custom-Header"] = "$proxy_host" h["Custom-Header"] = "$proxy_host"
cfgMap := "add-headers-configmap" 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() { ginkgo.It("should exist a proxy_host using the upstream-vhost annotation value", func() {
h := make(map[string]string) h := make(map[string]string)
h["Custom-Header"] = "$proxy_host" h["Custom-Header"] = "$proxy_host"
cfgMap := "add-headers-configmap" cfgMap := "add-headers-configmap"