diff --git a/controllers/nginx/controller.go b/controllers/nginx/controller.go index 81f7f4ea3..060e8cafe 100644 --- a/controllers/nginx/controller.go +++ b/controllers/nginx/controller.go @@ -558,7 +558,7 @@ func (lbc *loadBalancerController) getDefaultUpstream() *nginx.Upstream { return upstream } -func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.NginxConfiguration, data []interface{}) ([]*nginx.Upstream, []*nginx.Server) { +func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.Configuration, data []interface{}) ([]*nginx.Upstream, []*nginx.Server) { upstreams := lbc.createUpstreams(ngxCfg, data) upstreams[defUpstreamName] = lbc.getDefaultUpstream() @@ -689,7 +689,7 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.NginxConfigur // createUpstreams creates the NGINX upstreams for each service referenced in // Ingress rules. The servers inside the upstream are endpoints. -func (lbc *loadBalancerController) createUpstreams(ngxCfg nginx.NginxConfiguration, data []interface{}) map[string]*nginx.Upstream { +func (lbc *loadBalancerController) createUpstreams(ngxCfg nginx.Configuration, data []interface{}) map[string]*nginx.Upstream { upstreams := make(map[string]*nginx.Upstream) for _, ingIf := range data { diff --git a/controllers/nginx/main.go b/controllers/nginx/main.go index 8188e8814..ed2c9f67f 100644 --- a/controllers/nginx/main.go +++ b/controllers/nginx/main.go @@ -159,7 +159,7 @@ func registerHandlers(lbc *loadBalancerController) { http.HandleFunc("/build", func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) - fmt.Fprint(w, "build: %v - %v", gitRepo, version) + fmt.Fprintf(w, "build: %v - %v", gitRepo, version) }) http.HandleFunc("/stop", func(w http.ResponseWriter, r *http.Request) { diff --git a/controllers/nginx/nginx.tmpl b/controllers/nginx/nginx.tmpl index 8ede81dd1..55b5f84d8 100644 --- a/controllers/nginx/nginx.tmpl +++ b/controllers/nginx/nginx.tmpl @@ -158,7 +158,7 @@ http { {{/* build all the required rate limit zones. Each annotation requires a dedicated zone */}} {{/* 1MB -> 16 thousand 64-byte states or about 8 thousand 128-byte states */}} - {{ $zone := range (buildRateLimitZones .servers) }} + {{ range $zone := (buildRateLimitZones .servers) }} {{ $zone }} {{ end }} diff --git a/controllers/nginx/nginx/auth/main.go b/controllers/nginx/nginx/auth/main.go index fd957f4e4..30a1f64f1 100644 --- a/controllers/nginx/nginx/auth/main.go +++ b/controllers/nginx/nginx/auth/main.go @@ -158,10 +158,5 @@ func dumpSecret(filename string, secret *api.Secret) error { } // TODO: check permissions required - err := ioutil.WriteFile(filename, val, 0777) - if err != nil { - return err - } - - return nil + return ioutil.WriteFile(filename, val, 0777) } diff --git a/controllers/nginx/nginx/auth/main_test.go b/controllers/nginx/nginx/auth/main_test.go index eed975e94..b1cf33e1c 100644 --- a/controllers/nginx/nginx/auth/main_test.go +++ b/controllers/nginx/nginx/auth/main_test.go @@ -164,7 +164,7 @@ func TestIngressAuth(t *testing.T) { if nginxAuth.Realm != "-realm-" { t.Errorf("Expected -realm- as realm but returned %s", nginxAuth.Realm) } - if nginxAuth.Secured != true { + if !nginxAuth.Secured { t.Errorf("Expected true as secured but returned %v", nginxAuth.Secured) } } diff --git a/controllers/nginx/nginx/command.go b/controllers/nginx/nginx/command.go index c4a388e0f..b8da85a42 100644 --- a/controllers/nginx/nginx/command.go +++ b/controllers/nginx/nginx/command.go @@ -54,7 +54,7 @@ func (ngx *Manager) Start() { // shut down, stop accepting new connections and continue to service current requests // until all such requests are serviced. After that, the old worker processes exit. // http://nginx.org/en/docs/beginners_guide.html#control -func (ngx *Manager) CheckAndReload(cfg NginxConfiguration, ingressCfg IngressConfig) { +func (ngx *Manager) CheckAndReload(cfg Configuration, ingressCfg IngressConfig) { ngx.reloadRateLimiter.Accept() ngx.reloadLock.Lock() diff --git a/controllers/nginx/nginx/healthcheck/main.go b/controllers/nginx/nginx/healthcheck/main.go index 2d19a0c2c..dfde50a27 100644 --- a/controllers/nginx/nginx/healthcheck/main.go +++ b/controllers/nginx/nginx/healthcheck/main.go @@ -82,7 +82,7 @@ func (a ingAnnotations) failTimeout() (int, error) { // ParseAnnotations parses the annotations contained in the ingress // rule used to configure upstream check parameters -func ParseAnnotations(cfg nginx.NginxConfiguration, ing *extensions.Ingress) *Upstream { +func ParseAnnotations(cfg nginx.Configuration, ing *extensions.Ingress) *Upstream { if ing.GetAnnotations() == nil { return &Upstream{cfg.UpstreamMaxFails, cfg.UpstreamFailTimeout} } diff --git a/controllers/nginx/nginx/healthcheck/main_test.go b/controllers/nginx/nginx/healthcheck/main_test.go index 80bb0125a..0aeb0a9fa 100644 --- a/controllers/nginx/nginx/healthcheck/main_test.go +++ b/controllers/nginx/nginx/healthcheck/main_test.go @@ -80,11 +80,17 @@ func TestAnnotations(t *testing.T) { ing.SetAnnotations(data) mf, err := ingAnnotations(ing.GetAnnotations()).maxFails() + if err != nil { + t.Errorf("Unexpected error: %v", err) + } if mf != 1 { t.Errorf("Expected 1 but returned %s", mf) } ft, err := ingAnnotations(ing.GetAnnotations()).failTimeout() + if err != nil { + t.Errorf("Unexpected error: %v", err) + } if ft != 1 { t.Errorf("Expected 1 but returned %s", ft) } @@ -97,7 +103,7 @@ func TestIngressHealthCheck(t *testing.T) { data[upsMaxFails] = "2" ing.SetAnnotations(data) - cfg := nginx.NginxConfiguration{} + cfg := nginx.Configuration{} cfg.UpstreamFailTimeout = 1 nginxHz := ParseAnnotations(cfg, ing) diff --git a/controllers/nginx/nginx/main.go b/controllers/nginx/nginx/main.go index bccccfc62..783da6c9c 100644 --- a/controllers/nginx/nginx/main.go +++ b/controllers/nginx/nginx/main.go @@ -84,8 +84,8 @@ var ( sslDirectory = "/etc/nginx-ssl" ) -// NginxConfiguration ... -type NginxConfiguration struct { +// Configuration represents the content of nginx.conf file +type Configuration struct { // http://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size // Sets the maximum allowed size of the client request body BodySize string `structs:"body-size,omitempty"` @@ -251,7 +251,7 @@ type NginxConfiguration struct { type Manager struct { ConfigFile string - defCfg NginxConfiguration + defCfg Configuration defResolver string @@ -267,8 +267,8 @@ type Manager struct { // defaultConfiguration returns the default configuration contained // in the file default-conf.json -func newDefaultNginxCfg() NginxConfiguration { - cfg := NginxConfiguration{ +func newDefaultNginxCfg() Configuration { + cfg := Configuration{ BodySize: bodySize, ErrorLogLevel: errorLevel, HSTS: true, diff --git a/controllers/nginx/nginx/ratelimit/main.go b/controllers/nginx/nginx/ratelimit/main.go index 097d3b10f..66dc3c876 100644 --- a/controllers/nginx/nginx/ratelimit/main.go +++ b/controllers/nginx/nginx/ratelimit/main.go @@ -25,8 +25,8 @@ import ( ) const ( - limitIp = "ingress.kubernetes.io/limit-connections" - limitRps = "ingress.kubernetes.io/limit-rps" + limitIP = "ingress.kubernetes.io/limit-connections" + limitRPS = "ingress.kubernetes.io/limit-rps" // allow 5 times the specified limit as burst defBurst = 5 @@ -68,8 +68,8 @@ type Zone struct { type ingAnnotations map[string]string -func (a ingAnnotations) limitIp() int { - val, ok := a[limitIp] +func (a ingAnnotations) limitIP() int { + val, ok := a[limitIP] if ok { if i, err := strconv.Atoi(val); err == nil { return i @@ -79,8 +79,8 @@ func (a ingAnnotations) limitIp() int { return 0 } -func (a ingAnnotations) limitRps() int { - val, ok := a[limitRps] +func (a ingAnnotations) limitRPS() int { + val, ok := a[limitRPS] if ok { if i, err := strconv.Atoi(val); err == nil { return i @@ -97,8 +97,8 @@ func ParseAnnotations(ing *extensions.Ingress) (*RateLimit, error) { return &RateLimit{}, ErrMissingAnnotations } - rps := ingAnnotations(ing.GetAnnotations()).limitRps() - conn := ingAnnotations(ing.GetAnnotations()).limitIp() + rps := ingAnnotations(ing.GetAnnotations()).limitRPS() + conn := ingAnnotations(ing.GetAnnotations()).limitIP() if rps == 0 && conn == 0 { return &RateLimit{ diff --git a/controllers/nginx/nginx/ratelimit/main_test.go b/controllers/nginx/nginx/ratelimit/main_test.go index 5242f57e9..100001710 100644 --- a/controllers/nginx/nginx/ratelimit/main_test.go +++ b/controllers/nginx/nginx/ratelimit/main_test.go @@ -62,29 +62,29 @@ func buildIngress() *extensions.Ingress { func TestAnnotations(t *testing.T) { ing := buildIngress() - lip := ingAnnotations(ing.GetAnnotations()).limitIp() + lip := ingAnnotations(ing.GetAnnotations()).limitIP() if lip != 0 { - t.Error("Expected 0 in limit by ip but %v was returned", lip) + t.Errorf("Expected 0 in limit by ip but %v was returned", lip) } - lrps := ingAnnotations(ing.GetAnnotations()).limitRps() + lrps := ingAnnotations(ing.GetAnnotations()).limitRPS() if lrps != 0 { - t.Error("Expected 0 in limit by rps but %v was returend", lrps) + t.Errorf("Expected 0 in limit by rps but %v was returend", lrps) } data := map[string]string{} - data[limitIp] = "5" - data[limitRps] = "100" + data[limitIP] = "5" + data[limitRPS] = "100" ing.SetAnnotations(data) - lip = ingAnnotations(ing.GetAnnotations()).limitIp() + lip = ingAnnotations(ing.GetAnnotations()).limitIP() if lip != 5 { - t.Error("Expected %v in limit by ip but %v was returend", lip) + t.Errorf("Expected 5 in limit by ip but %v was returend", lip) } - lrps = ingAnnotations(ing.GetAnnotations()).limitRps() + lrps = ingAnnotations(ing.GetAnnotations()).limitRPS() if lrps != 100 { - t.Error("Expected 100 in limit by rps but %v was returend", lrps) + t.Errorf("Expected 100 in limit by rps but %v was returend", lrps) } } @@ -100,8 +100,8 @@ func TestBadRateLimiting(t *testing.T) { ing := buildIngress() data := map[string]string{} - data[limitIp] = "0" - data[limitRps] = "0" + data[limitIP] = "0" + data[limitRPS] = "0" ing.SetAnnotations(data) _, err := ParseAnnotations(ing) @@ -110,8 +110,8 @@ func TestBadRateLimiting(t *testing.T) { } data = map[string]string{} - data[limitIp] = "5" - data[limitRps] = "100" + data[limitIP] = "5" + data[limitRPS] = "100" ing.SetAnnotations(data) rateLimit, err := ParseAnnotations(ing) @@ -120,10 +120,10 @@ func TestBadRateLimiting(t *testing.T) { } if rateLimit.Connections.Limit != 5 { - t.Error("Expected 5 in limit by ip but %v was returend", rateLimit.Connections) + t.Errorf("Expected 5 in limit by ip but %v was returend", rateLimit.Connections) } if rateLimit.RPS.Limit != 100 { - t.Error("Expected 100 in limit by rps but %v was returend", rateLimit.RPS) + t.Errorf("Expected 100 in limit by rps but %v was returend", rateLimit.RPS) } } diff --git a/controllers/nginx/nginx/rewrite/main_test.go b/controllers/nginx/nginx/rewrite/main_test.go index cd8cf843e..57db686af 100644 --- a/controllers/nginx/nginx/rewrite/main_test.go +++ b/controllers/nginx/nginx/rewrite/main_test.go @@ -72,8 +72,8 @@ func TestAnnotations(t *testing.T) { } f := ingAnnotations(ing.GetAnnotations()).addBaseURL() - if f != false { - t.Error("Expected false in add-base-url but %v was returend", f) + if f { + t.Errorf("Expected false in add-base-url but %v was returend", f) } data := map[string]string{} @@ -83,12 +83,12 @@ func TestAnnotations(t *testing.T) { r = ingAnnotations(ing.GetAnnotations()).rewriteTo() if r != defRoute { - t.Error("Expected %v in rewrite but %v was returend", defRoute, r) + t.Errorf("Expected %v in rewrite but %v was returend", defRoute, r) } f = ingAnnotations(ing.GetAnnotations()).addBaseURL() - if f != true { - t.Error("Expected true in add-base-url but %v was returend", f) + if !f { + t.Errorf("Expected true in add-base-url but %v was returend", f) } } diff --git a/controllers/nginx/nginx/template.go b/controllers/nginx/nginx/template.go index 32e3a3350..a272eaae1 100644 --- a/controllers/nginx/nginx/template.go +++ b/controllers/nginx/nginx/template.go @@ -57,7 +57,7 @@ func (ngx *Manager) loadTemplate() { ngx.template = tmpl } -func (ngx *Manager) writeCfg(cfg NginxConfiguration, ingressCfg IngressConfig) (bool, error) { +func (ngx *Manager) writeCfg(cfg Configuration, ingressCfg IngressConfig) (bool, error) { conf := make(map[string]interface{}) conf["upstreams"] = ingressCfg.Upstreams conf["servers"] = ingressCfg.Servers diff --git a/controllers/nginx/nginx/template_test.go b/controllers/nginx/nginx/template_test.go index 4d82c40ae..006f0dda5 100644 --- a/controllers/nginx/nginx/template_test.go +++ b/controllers/nginx/nginx/template_test.go @@ -72,7 +72,7 @@ func TestBuildLocation(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &Location{ Path: tc.Path, - Redirect: rewrite.Redirect{tc.Target, tc.AddBaseURL}, + Redirect: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, } newLoc := buildLocation(loc) @@ -86,7 +86,7 @@ func TestBuildProxyPass(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &Location{ Path: tc.Path, - Redirect: rewrite.Redirect{tc.Target, tc.AddBaseURL}, + Redirect: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, Upstream: Upstream{Name: "upstream-name"}, } diff --git a/controllers/nginx/nginx/utils.go b/controllers/nginx/nginx/utils.go index 3a4fd6877..dc7ba62e0 100644 --- a/controllers/nginx/nginx/utils.go +++ b/controllers/nginx/nginx/utils.go @@ -67,7 +67,7 @@ func getDNSServers() []string { // getConfigKeyToStructKeyMap returns a map with the ConfigMapKey as key and the StructName as value. func getConfigKeyToStructKeyMap() map[string]string { keyMap := map[string]string{} - n := &NginxConfiguration{} + n := &Configuration{} val := reflect.Indirect(reflect.ValueOf(n)) for i := 0; i < val.Type().NumField(); i++ { fieldSt := val.Type().Field(i) @@ -79,12 +79,12 @@ func getConfigKeyToStructKeyMap() map[string]string { } // ReadConfig obtains the configuration defined by the user merged with the defaults. -func (ngx *Manager) ReadConfig(config *api.ConfigMap) NginxConfiguration { +func (ngx *Manager) ReadConfig(config *api.ConfigMap) Configuration { if len(config.Data) == 0 { return newDefaultNginxCfg() } - cfgCM := NginxConfiguration{} + cfgCM := Configuration{} cfgDefault := newDefaultNginxCfg() metadata := &mapstructure.Metadata{} @@ -96,7 +96,7 @@ func (ngx *Manager) ReadConfig(config *api.ConfigMap) NginxConfiguration { Metadata: metadata, }) - cErrors := make([]int, 0) + var cErrors []int if val, ok := config.Data[customHTTPErrors]; ok { delete(config.Data, customHTTPErrors) for _, i := range strings.Split(val, ",") { @@ -138,7 +138,7 @@ func (ngx *Manager) ReadConfig(config *api.ConfigMap) NginxConfiguration { } func (ngx *Manager) filterErrors(errCodes []int) []int { - fa := make([]int, 0) + var fa []int for _, errCode := range errCodes { if errCode > 299 && errCode < 600 { fa = append(fa, errCode) diff --git a/controllers/nginx/nginx/utils_test.go b/controllers/nginx/nginx/utils_test.go index e93a3f2af..b5b7057b3 100644 --- a/controllers/nginx/nginx/utils_test.go +++ b/controllers/nginx/nginx/utils_test.go @@ -22,7 +22,7 @@ import ( "k8s.io/kubernetes/pkg/api" ) -func getConfigNginxBool(data map[string]string) NginxConfiguration { +func getConfigNginxBool(data map[string]string) Configuration { manager := &Manager{} configMap := &api.ConfigMap{ Data: data, @@ -74,7 +74,7 @@ func TestManagerReadConfigStringSet(t *testing.T) { }) exp := "TLSv1.2" if configNginx.SSLProtocols != exp { - t.Error("Failed to set string value true actual='%s' expected='%s'", configNginx.SSLProtocols, exp) + t.Errorf("Failed to set string value true actual='%s' expected='%s'", configNginx.SSLProtocols, exp) } } @@ -84,6 +84,6 @@ func TestManagerReadConfigStringNothing(t *testing.T) { }) exp := "10m" if configNginx.SSLSessionTimeout != exp { - t.Error("Failed to set string value true actual='%s' expected='%s'", configNginx.SSLSessionTimeout, exp) + t.Errorf("Failed to set string value true actual='%s' expected='%s'", configNginx.SSLSessionTimeout, exp) } }