Merge pull request #38 from prymitive/go-metalinter

feat(test): use a metalinter to find more Go code issues on CI

.golangci.yml

@@ -0,0 +1,19 @@
+run:
+  deadline: 5m
+  skip-files:
+    - bindata_assetfs.go
+
+linters:
+  enable:
+    - golint
+    - dupl
+    - goconst
+    - gocyclo
+
+linters-settings:
+  govet:
+    check-shadowing: true
+  golint:
+    min-confidence: 0
+  dupl:
+    threshold: 200

.travis.yml

@@ -39,6 +39,8 @@ jobs:
 
     - stage: Lint Go code
       <<: *DEFAULTS_GO
+      before_script:
+        - make mock-assets
       script: make lint-go
 
     - stage: Lint JavaScript code

Makefile

@@ -31,7 +31,7 @@ endif
 
 .build/deps-lint-go.ok:
 	@mkdir -p .build
-	go get -u github.com/golang/lint/golint
+	go get -u github.com/golangci/golangci-lint/cmd/golangci-lint
 	touch $@
 
 .build/deps-build-node.ok: ui/package.json ui/package-lock.json
@@ -103,7 +103,7 @@ run-docker: docker-image
 
 .PHONY: lint-go
 lint-go: .build/deps-lint-go.ok
-	golint ./... | (egrep -v "^vendor/|^bindata_assetfs.go" || true)
+	golangci-lint run
 
 .PHONY: lint-js
 lint-js: .build/deps-build-node.ok

api_test.go

@@ -844,7 +844,10 @@ func TestVerifyAllGroups(t *testing.T) {
 		}
 
 		ur := models.AlertsResponse{}
-		json.Unmarshal(resp.Body.Bytes(), &ur)
+		err := json.Unmarshal(resp.Body.Bytes(), &ur)
+		if err != nil {
+			t.Errorf("Failed to unmarshal response: %s", err)
+		}
 
 		if len(ur.AlertGroups) != len(groupTests) {
 			t.Errorf("[%s] Got %d alert(s) in response, expected %d",

assets.go

@@ -1,14 +1,11 @@
 package main
 
 import (
-	"bytes"
 	"errors"
 	"html/template"
 	"net/http"
 	"strings"
 
-	"github.com/gin-gonic/gin"
-
 	assetfs "github.com/elazarl/go-bindata-assetfs"
 	log "github.com/sirupsen/logrus"
 )
@@ -39,7 +36,7 @@ func newBinaryFileSystem(root string) *binaryFileSystem {
 		Asset: Asset,
 		// Don't render directory index, return 404 for /static/ requests)
 		AssetDir: func(path string) ([]string, error) {
-			return nil, errors.New("Not found")
+			return nil, errors.New("not found")
 		},
 		Prefix: root,
 	}
@@ -75,23 +72,3 @@ func loadTemplate(t *template.Template, path string) *template.Template {
 
 	return t
 }
-
-func responseFromStaticFile(c *gin.Context, filepath string, contentType string) {
-	if !staticFileSystem.Exists("/", filepath) {
-		c.String(404, "Not found")
-		return
-	}
-
-	file, err := staticFileSystem.Open(filepath)
-	if err != nil {
-		c.AbortWithError(500, err)
-		return
-	}
-	buf := bytes.NewBuffer(nil)
-	_, err = buf.ReadFrom(file)
-	if err != nil {
-		c.AbortWithError(500, err)
-		return
-	}
-	c.Data(200, contentType, buf.Bytes())
-}

autocomplete_test.go

@@ -59,7 +59,10 @@ func TestKnownLabelNames(t *testing.T) {
 			}
 
 			ur := []string{}
-			json.Unmarshal(resp.Body.Bytes(), &ur)
+			err := json.Unmarshal(resp.Body.Bytes(), &ur)
+			if err != nil {
+				t.Errorf("Failed to unmarshal response: %s", err)
+			}
 
 			if len(ur) != len(testCase.Results) {
 				t.Errorf("Invalid number of label names for %s, got %d, expected %d", url, len(ur), len(testCase.Results))
@@ -114,7 +117,10 @@ func TestKnownLabelValues(t *testing.T) {
 			}
 
 			ur := []string{}
-			json.Unmarshal(resp.Body.Bytes(), &ur)
+			err := json.Unmarshal(resp.Body.Bytes(), &ur)
+			if err != nil {
+				t.Errorf("Failed to unmarshal response: %s", err)
+			}
 
 			if len(ur) != len(testCase.Results) {
 				t.Errorf("Invalid number of label values for %s, got %d, expected %d", url, len(ur), len(testCase.Results))

internal/alertmanager/dedup.go

@@ -43,9 +43,7 @@ func DedupAlerts() []models.AlertGroup {
 					// alertmanager instances to it, this way we end up with all instances
 					// for each unique alert merged into a single alert with all
 					// alertmanager instances attached to it
-					for _, am := range alert.Alertmanager {
-						a.Alertmanager = append(a.Alertmanager, am)
-					}
+					a.Alertmanager = append(a.Alertmanager, alert.Alertmanager...)
 					// set startsAt to the earliest value we have
 					if alert.StartsAt.Before(a.StartsAt) {
 						a.StartsAt = alert.StartsAt

internal/alertmanager/dedup_test.go

@@ -21,7 +21,10 @@ func init() {
 		if err != nil {
 			log.Fatal(err)
 		}
-		alertmanager.RegisterAlertmanager(am)
+		err = alertmanager.RegisterAlertmanager(am)
+		if err != nil {
+			panic(fmt.Sprintf("Failed to register Alertmanager instance %s: %s", am.Name, err))
+		}
 	}
 }
 

internal/alertmanager/models.go

@@ -360,7 +360,7 @@ func (am *Alertmanager) SilenceByID(id string) (models.Silence, error) {
 
 	s, found := am.silences[id]
 	if !found {
-		return models.Silence{}, fmt.Errorf("Silence '%s' not found", id)
+		return models.Silence{}, fmt.Errorf("silence '%s' not found", id)
 	}
 	return s, nil
 }

internal/alertmanager/upstream.go

@@ -59,12 +59,12 @@ func NewAlertmanager(name, upstreamURI string, opts ...Option) (*Alertmanager, e
 // instances used when pulling alerts from upstreams
 func RegisterAlertmanager(am *Alertmanager) error {
 	if _, found := upstreams[am.Name]; found {
-		return fmt.Errorf("Alertmanager upstream '%s' already exist", am.Name)
+		return fmt.Errorf("alertmanager upstream '%s' already exist", am.Name)
 	}
 
 	for _, existingAM := range upstreams {
 		if existingAM.URI == am.URI {
-			return fmt.Errorf("Alertmanager upstream '%s' already collects from '%s'", existingAM.Name, existingAM.URI)
+			return fmt.Errorf("alertmanager upstream '%s' already collects from '%s'", existingAM.Name, existingAM.URI)
 		}
 	}
 	upstreams[am.Name] = am

internal/config/config.go

@@ -82,17 +82,32 @@ func (config *configSchema) Read() {
 
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
 	pflag.Parse()
-	v.BindPFlags(pflag.CommandLine)
+
+	err := v.BindPFlags(pflag.CommandLine)
+	if err != nil {
+		log.Errorf("Failed to bind flag set to the configuration: %s", err)
+	}
 
 	v.AutomaticEnv()
 	v.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
 
 	// special envs
 	// HOST and PORT is used by gin
-	v.BindEnv("listen.address", "HOST")
-	v.BindEnv("listen.port", "PORT")
+	err = v.BindEnv("listen.address", "HOST")
+	if err != nil {
+		log.Errorf("Failed to bind listen.address config key to the HOST env variable", err)
+	}
+
+	err = v.BindEnv("listen.port", "PORT")
+	if err != nil {
+		log.Errorf("Failed to bind listen.port config key to the PORT env variable", err)
+	}
+
 	// raven-go expects this
-	v.BindEnv("sentry.private", "SENTRY_DSN")
+	err = v.BindEnv("sentry.private", "SENTRY_DSN")
+	if err != nil {
+		log.Errorf("Failed to bind sentry.private config key to the SENTRY_DSN env variable", err)
+	}
 
 	configFile := v.GetString("config.file")
 	configDir := v.GetString("config.dir")
@@ -100,7 +115,7 @@ func (config *configSchema) Read() {
 	v.SetConfigName(configFile)
 	v.AddConfigPath(configDir)
 	log.Infof("Reading configuration file %s.yaml", path.Join(configDir, configFile))
-	err := v.ReadInConfig()
+	err = v.ReadInConfig()
 	if v.ConfigFileUsed() != "" && err != nil {
 		log.Fatal(err)
 	}

internal/filters/filter.go

@@ -86,9 +86,9 @@ func NewFilter(expression string) FilterT {
 		}
 	}
 
-	matched, _ := result["matched"]
-	operator, _ := result["operator"]
-	value, _ := result["value"]
+	matched := result["matched"]
+	operator := result["operator"]
+	value := result["value"]
 
 	if matched == "" && operator == "" && value == "" {
 		// no "filter=" part, just the value, use fuzzy filter

internal/mapper/mapper.go

@@ -44,7 +44,7 @@ func GetAlertMapper(version string) (AlertMapper, error) {
 			return m, nil
 		}
 	}
-	return nil, fmt.Errorf("Can't find alert mapper for Alertmanager %s", version)
+	return nil, fmt.Errorf("can't find alert mapper for Alertmanager %s", version)
 }
 
 // RegisterSilenceMapper allows to register mapper implementing silence data
@@ -60,5 +60,5 @@ func GetSilenceMapper(version string) (SilenceMapper, error) {
 			return m, nil
 		}
 	}
-	return nil, fmt.Errorf("Can't find silence mapper for Alertmanager %s", version)
+	return nil, fmt.Errorf("can't find silence mapper for Alertmanager %s", version)
 }

internal/mapper/v04/alerts.go

@@ -134,9 +134,7 @@ func (m AlertMapper) Decode(source io.ReadCloser) ([]models.AlertGroup, error) {
 		}
 	}
 	for _, rcv := range receivers {
-		for _, ag := range rcv.Groups {
-			groups = append(groups, ag)
-		}
+		groups = append(groups, rcv.Groups...)
 	}
 	return groups, nil
 }

internal/mapper/v05/alerts.go

@@ -133,9 +133,7 @@ func (m AlertMapper) Decode(source io.ReadCloser) ([]models.AlertGroup, error) {
 		}
 	}
 	for _, rcv := range receivers {
-		for _, ag := range rcv.Groups {
-			groups = append(groups, ag)
-		}
+		groups = append(groups, rcv.Groups...)
 	}
 	return groups, nil
 }

internal/mapper/v061/alerts.go

@@ -132,9 +132,7 @@ func (m AlertMapper) Decode(source io.ReadCloser) ([]models.AlertGroup, error) {
 		}
 	}
 	for _, rcv := range receivers {
-		for _, ag := range rcv.Groups {
-			groups = append(groups, ag)
-		}
+		groups = append(groups, rcv.Groups...)
 	}
 	return groups, nil
 }

internal/mapper/v062/alerts.go

@@ -136,9 +136,7 @@ func (m AlertMapper) Decode(source io.ReadCloser) ([]models.AlertGroup, error) {
 		}
 	}
 	for _, rcv := range receivers {
-		for _, ag := range rcv.Groups {
-			groups = append(groups, ag)
-		}
+		groups = append(groups, rcv.Groups...)
 	}
 	return groups, nil
 }

internal/mock/mock.go

@@ -26,7 +26,7 @@ func RegisterURL(url string, version string, filename string) {
 		panic(err)
 	}
 	if len(mockJSON) == 0 {
-		panic(fmt.Errorf("Empty mock file '%s'", fullPath))
+		panic(fmt.Errorf("empty mock file '%s'", fullPath))
 	}
 	httpmock.RegisterResponder("GET", url, httpmock.NewBytesResponder(200, mockJSON))
 }

internal/mock/mock.py

@@ -1,50 +0,0 @@
-#!/usr/bin/env python
-
-
-import random
-import time
-
-
-def send_alert(annotations, labels):
-    data = {
-        "annotations": annotations,
-        "labels": data,
-        "generatorURL": "http://localhost:9093"
-    }
-    req = urllib2.Request('http://localhost:9093/api/v1/alerts')
-    req.add_header('Content-Type', 'application/json')
-    response = urllib2.urlopen(req, json.dumps(data))
-    print(response)
-
-
-class FlappingAlert(object):
-
-    annotations = {}
-    labels = {}
-
-    def __init__(self, active_for, idle_for, splay):
-        self._active_for = active_for
-        self._idle_for = idle_for
-        self._max_splay = splay
-
-        self.active = True
-        self.last_change = time.time()
-        self.splay = self.random_splay()
-
-    def random_splay(self):
-        return random.randrange(0, self._max_splay)
-
-    def tick():
-        if time.time() >= self.last_change + self.splay:
-            self.active = !self.active
-            self.last_change = time.time()
-            self.splay = self.random_splay()
-
-        if self.active:
-            send_alert(self.annotations, self.labels)
-
-
-def main():
-    alerts = [
-
-    ]

internal/models/alertgroup.go

@@ -6,6 +6,8 @@ import (
 	"io"
 
 	"github.com/cnf/structhash"
+
+	log "github.com/sirupsen/logrus"
 )
 
 // AlertList is flat list of UnseeAlert objects
@@ -46,8 +48,17 @@ type AlertGroup struct {
 // it should be unique for each AlertGroup
 func (ag AlertGroup) LabelsFingerprint() string {
 	agIDHasher := sha1.New()
-	io.WriteString(agIDHasher, ag.Receiver)
-	io.WriteString(agIDHasher, fmt.Sprintf("%x", structhash.Sha1(ag.Labels, 1)))
+
+	_, err := io.WriteString(agIDHasher, ag.Receiver)
+	if err != nil {
+		log.Errorf("Failed to write receiver value to alertgroup '%s' fingerprint: %s", ag.ID, err)
+	}
+
+	_, err = io.WriteString(agIDHasher, fmt.Sprintf("%x", structhash.Sha1(ag.Labels, 1)))
+	if err != nil {
+		log.Errorf("Failed to write labels sha1 value to alertgroup '%s' fingerprint: %s", ag.ID, err)
+	}
+
 	return fmt.Sprintf("%x", agIDHasher.Sum(nil))
 }
 
@@ -55,7 +66,10 @@ func (ag AlertGroup) LabelsFingerprint() string {
 func (ag AlertGroup) ContentFingerprint() string {
 	h := sha1.New()
 	for _, alert := range ag.Alerts {
-		io.WriteString(h, alert.ContentFingerprint())
+		_, err := io.WriteString(h, alert.ContentFingerprint())
+		if err != nil {
+			log.Errorf("Failed to write alert fingerprint value to alertgroup '%s' fingerprint: %s", ag.ID, err)
+		}
 	}
 	return fmt.Sprintf("%x", h.Sum(nil))
 }

internal/transform/colors.go

@@ -10,12 +10,23 @@ import (
 	"github.com/prymitive/unsee/internal/slices"
 
 	"github.com/hansrodtang/randomcolor"
+
+	log "github.com/sirupsen/logrus"
 )
 
 func labelToSeed(key string, val string) int64 {
 	h := sha1.New()
-	io.WriteString(h, key)
-	io.WriteString(h, val)
+
+	_, err := io.WriteString(h, key)
+	if err != nil {
+		log.Errorf("Failed to write label key '%s' to the seed sha1: %s", key, err)
+	}
+
+	_, err = io.WriteString(h, val)
+	if err != nil {
+		log.Errorf("Failed to write label value '%s' to the seed sha1: %s", val, err)
+	}
+
 	var seed int64
 	for _, i := range h.Sum(nil) {
 		seed += int64(i)
@@ -27,7 +38,7 @@ func labelToSeed(key string, val string) int64 {
 // from label key and value passed here
 // It's used to generate unique colors for configured labels
 func ColorLabel(colorStore models.LabelsColorMap, key string, val string) {
-	if slices.StringInSlice(config.Config.Labels.Color.Unique, key) == true {
+	if slices.StringInSlice(config.Config.Labels.Color.Unique, key) {
 		if _, found := colorStore[key]; !found {
 			colorStore[key] = make(map[string]models.LabelColors)
 		}
@@ -43,8 +54,7 @@ func ColorLabel(colorStore models.LabelsColorMap, key string, val string) {
 			}
 			// check if color is bright or dark and pick the right background
 			// uses https://www.w3.org/WAI/ER/WD-AERT/#color-contrast method
-			var brightness int32
-			brightness = ((int32(bc.Red) * 299) + (int32(bc.Green) * 587) + (int32(bc.Blue) * 114)) / 1000
+			brightness := ((int32(bc.Red) * 299) + (int32(bc.Green) * 587) + (int32(bc.Blue) * 114)) / 1000
 			var fc models.Color
 			if brightness <= 125 {
 				// background color is dark, use white font

internal/uri/http.go

@@ -29,7 +29,7 @@ func (r *HTTPURIReader) Read(uri string) (io.ReadCloser, error) {
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("Request to %s failed with %s", SanitizeURI(uri), resp.Status)
+		return nil, fmt.Errorf("request to %s failed with %s", SanitizeURI(uri), resp.Status)
 	}
 
 	var reader io.ReadCloser
@@ -37,7 +37,7 @@ func (r *HTTPURIReader) Read(uri string) (io.ReadCloser, error) {
 	case "gzip":
 		reader, err = gzip.NewReader(resp.Body)
 		if err != nil {
-			return nil, fmt.Errorf("Failed to decode gzipped content: %s", err.Error())
+			return nil, fmt.Errorf("failed to decode gzipped content: %s", err.Error())
 		}
 	default:
 		reader = resp.Body

internal/uri/uri.go

@@ -31,6 +31,6 @@ func NewReader(uri string, timeout time.Duration, clientTransport http.RoundTrip
 	case "file":
 		return &FileURIReader{}, nil
 	default:
-		return nil, fmt.Errorf("Unsupported URI scheme '%s' in '%s'", u.Scheme, u)
+		return nil, fmt.Errorf("unsupported URI scheme '%s' in '%s'", u.Scheme, u)
 	}
 }

main.go

@@ -41,7 +41,6 @@ var (
 
 	// used by static file view handlers
 	staticFileSystem = newBinaryFileSystem("ui/build")
-	staticFileServer = http.FileServer(staticFileSystem)
 )
 
 func getViewURL(sub string) string {
@@ -194,7 +193,10 @@ func main() {
 
 	setupRouter(router)
 	for _, am := range alertmanager.GetAlertmanagers() {
-		setupRouterProxyHandlers(router, am)
+		err := setupRouterProxyHandlers(router, am)
+		if err != nil {
+			log.Fatalf("Failed to setup proxy handlers for Alertmanager '%s': %s", am.Name, err)
+		}
 	}
 	listen := fmt.Sprintf("%s:%d", config.Config.Listen.Address, config.Config.Listen.Port)
 	log.Infof("Listening on %s", listen)

proxy_test.go

@@ -24,10 +24,6 @@ func newCloseNotifyingRecorder() *closeNotifyingRecorder {
 	}
 }
 
-func (c *closeNotifyingRecorder) close() {
-	c.closed <- true
-}
-
 func (c *closeNotifyingRecorder) CloseNotify() <-chan bool {
 	return c.closed
 }
@@ -99,7 +95,10 @@ func TestProxy(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	setupRouterProxyHandlers(r, am)
+	err = setupRouterProxyHandlers(r, am)
+	if err != nil {
+		t.Errorf("Failed to setup proxy for Alertmanager %s: %s", am.Name, err)
+	}
 
 	httpmock.Activate()
 	defer httpmock.DeactivateAndReset()
@@ -189,7 +188,10 @@ func TestProxyHeaders(t *testing.T) {
 		if err != nil {
 			t.Error(err)
 		}
-		setupRouterProxyHandlers(r, am)
+		err = setupRouterProxyHandlers(r, am)
+		if err != nil {
+			t.Errorf("Failed to setup proxy for Alertmanager %s: %s", am.Name, err)
+		}
 
 		httpmock.Reset()
 		httpmock.RegisterResponder(testCase.method, testCase.upstreamURI, func(req *http.Request) (*http.Response, error) {

timer.go

@@ -38,10 +38,7 @@ func pullFromAlertmanager() {
 
 // Tick is the background timer used to call PullFromAlertmanager
 func Tick() {
-	for {
-		select {
-		case <-ticker.C:
-			pullFromAlertmanager()
-		}
+	for range ticker.C {
+		pullFromAlertmanager()
 	}
 }

views_test.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"html/template"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -99,7 +98,10 @@ func TestAlerts(t *testing.T) {
 		}
 
 		ur := models.AlertsResponse{}
-		json.Unmarshal(resp.Body.Bytes(), &ur)
+		err := json.Unmarshal(resp.Body.Bytes(), &ur)
+		if err != nil {
+			t.Errorf("Failed to unmarshal response: %s", err)
+		}
 		if len(ur.Filters) != 3 {
 			t.Errorf("[%s] Got %d filter(s) in response, expected %d", version, len(ur.Filters), 3)
 		}
@@ -166,7 +168,10 @@ func TestValidateAllAlerts(t *testing.T) {
 		}
 		ur := models.AlertsResponse{}
 		body := resp.Body.Bytes()
-		json.Unmarshal(body, &ur)
+		err := json.Unmarshal(body, &ur)
+		if err != nil {
+			t.Errorf("Failed to unmarshal response: %s", err)
+		}
 		for _, ag := range ur.AlertGroups {
 			for _, a := range ag.Alerts {
 				if !slices.StringInSlice(models.AlertStateList, a.State) {
@@ -177,15 +182,6 @@ func TestValidateAllAlerts(t *testing.T) {
 				}
 			}
 		}
-		// write JSON response to a file, it will be used by (optional) JS tests
-		// those require actual JSON responses and shouldn't be mocked
-		if _, err := os.Stat(".tests"); os.IsNotExist(err) {
-			os.Mkdir(".tests", 0755)
-		}
-		err := ioutil.WriteFile(".tests/alerts.json", body, 0644)
-		if err != nil {
-			t.Logf("Failed to write .tests/alerts.json: %s", err)
-		}
 	}
 }
 
@@ -349,7 +345,10 @@ func TestAutocomplete(t *testing.T) {
 			}
 
 			ur := []string{}
-			json.Unmarshal(resp.Body.Bytes(), &ur)
+			err := json.Unmarshal(resp.Body.Bytes(), &ur)
+			if err != nil {
+				t.Errorf("Failed to unmarshal response: %s", err)
+			}
 
 			if len(ur) != len(acTest.Results) {
 				t.Errorf("Invalid number of autocomplete hints for %s, got %d, expected %d", url, len(ur), len(acTest.Results))