From 05c3cf3a72a69cfd01df0d6c3cede90d9dd2bb65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 10 Aug 2018 23:37:22 +0100 Subject: [PATCH 01/18] feat(test): use a metalinter to find more Go code issues on CI --- .golangci.yml | 19 +++++++++++++++++++ .travis.yml | 2 ++ Makefile | 4 ++-- 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..d9021044f --- /dev/null +++ b/.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 diff --git a/.travis.yml b/.travis.yml index 600ef379c..a6062640b 100644 --- a/.travis.yml +++ b/.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 diff --git a/Makefile b/Makefile index 08ac10d31..b3be7737c 100644 --- a/Makefile +++ b/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 From 7b00440915528c65558aa643f3f704a5046846de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:23:18 +0100 Subject: [PATCH 02/18] fix(style): simplify timer code --- timer.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/timer.go b/timer.go index 2eaef6a09..52902e8f9 100644 --- a/timer.go +++ b/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() } } From 0310d9d4bc4068d8747a20dd6ce134db15ab61ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:24:58 +0100 Subject: [PATCH 03/18] fix(style): should merge variable declaration with assignment --- internal/transform/colors.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/transform/colors.go b/internal/transform/colors.go index 9ece97f78..d3fef8b2d 100644 --- a/internal/transform/colors.go +++ b/internal/transform/colors.go @@ -43,8 +43,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 From 7a6538e8ff77672bdbef6b1ea1624112f75a620b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:25:57 +0100 Subject: [PATCH 04/18] fix(style): should omit comparison to bool constant --- internal/transform/colors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/transform/colors.go b/internal/transform/colors.go index d3fef8b2d..5b70bf02c 100644 --- a/internal/transform/colors.go +++ b/internal/transform/colors.go @@ -27,7 +27,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) } From fb17ed7e3101322cf9cd3d590782fed62eab242f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:29:03 +0100 Subject: [PATCH 05/18] fix(style): simplify group appends --- internal/mapper/v04/alerts.go | 4 +--- internal/mapper/v05/alerts.go | 4 +--- internal/mapper/v061/alerts.go | 4 +--- internal/mapper/v062/alerts.go | 4 +--- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/internal/mapper/v04/alerts.go b/internal/mapper/v04/alerts.go index b7dfea649..338295745 100644 --- a/internal/mapper/v04/alerts.go +++ b/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 } diff --git a/internal/mapper/v05/alerts.go b/internal/mapper/v05/alerts.go index 24847faa5..600f47845 100644 --- a/internal/mapper/v05/alerts.go +++ b/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 } diff --git a/internal/mapper/v061/alerts.go b/internal/mapper/v061/alerts.go index ba1ef1ceb..cc8be6c93 100644 --- a/internal/mapper/v061/alerts.go +++ b/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 } diff --git a/internal/mapper/v062/alerts.go b/internal/mapper/v062/alerts.go index c902db5bd..ceae09a2a 100644 --- a/internal/mapper/v062/alerts.go +++ b/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 } From 64f072b7d29c54f87a8d701ebdccb1044260e989 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:31:18 +0100 Subject: [PATCH 06/18] fix(style): simplify assignments --- internal/filters/filter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/filters/filter.go b/internal/filters/filter.go index d9f684239..ecc776627 100644 --- a/internal/filters/filter.go +++ b/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 From cb7a6e71c09f1bf1827a4f1f32a840aab54c5eef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:33:10 +0100 Subject: [PATCH 07/18] fix(style): remove unused variable --- main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/main.go b/main.go index a14353753..87dbae9ea 100644 --- a/main.go +++ b/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 { From 40e820c416d00b4cea5c48bac1091476976d2cc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:38:41 +0100 Subject: [PATCH 08/18] fix(style): remove dead code --- assets.go | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/assets.go b/assets.go index bc48da1a5..0fb520134 100644 --- a/assets.go +++ b/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" ) @@ -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()) -} From 305916a3f277a74f8f2050d2804e35dc29f3cf71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:39:07 +0100 Subject: [PATCH 09/18] fix(style): remove dead code --- internal/mock/mock.py | 50 ------------------------------------------- 1 file changed, 50 deletions(-) delete mode 100644 internal/mock/mock.py diff --git a/internal/mock/mock.py b/internal/mock/mock.py deleted file mode 100644 index 187a5b293..000000000 --- a/internal/mock/mock.py +++ /dev/null @@ -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 = [ - - ] From e7d2bca80671fa924d1f4ae22047e87efc9e31ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 08:41:10 +0100 Subject: [PATCH 10/18] fix(style): check for error return value from setupRouterProxyHandlers --- main.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/main.go b/main.go index 87dbae9ea..8072f6e07 100644 --- a/main.go +++ b/main.go @@ -193,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) From c61e3fe945e58664e5c227f8bcceb0a9a190ca40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 22:40:55 +0100 Subject: [PATCH 11/18] fix(style): check for json.Unmarshal error returns in tests Also drop .tests dir, not needed anymore --- api_test.go | 5 ++++- autocomplete_test.go | 10 ++++++++-- views_test.go | 25 ++++++++++++------------- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/api_test.go b/api_test.go index 2f42c959b..1444e4232 100644 --- a/api_test.go +++ b/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", diff --git a/autocomplete_test.go b/autocomplete_test.go index 8c68ed03c..a11f021c0 100644 --- a/autocomplete_test.go +++ b/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)) diff --git a/views_test.go b/views_test.go index f64cb356e..557afcd7c 100644 --- a/views_test.go +++ b/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)) From 2ec3df93d7fd1c70ce2b46bad37462409c5644d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 22:43:22 +0100 Subject: [PATCH 12/18] fix(style): check for alertmanager.RegisterAlertmanager error return --- internal/alertmanager/dedup_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/alertmanager/dedup_test.go b/internal/alertmanager/dedup_test.go index 1488d892f..9487d7c41 100644 --- a/internal/alertmanager/dedup_test.go +++ b/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)) + } } } From d306807ab4e4ea0cf835920b2e54d48a3b1dd7d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 22:45:12 +0100 Subject: [PATCH 13/18] fix(style): check for setupRouterProxyHandlers error return --- proxy_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/proxy_test.go b/proxy_test.go index c9925447e..fa5e435a0 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -99,7 +99,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 +192,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) { From b2a0f61d4efa3fd4fca08a055299f41947ca1f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 22:46:34 +0100 Subject: [PATCH 14/18] fix(style): simplify append code --- internal/alertmanager/dedup.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/alertmanager/dedup.go b/internal/alertmanager/dedup.go index 404f9aa59..e13692566 100644 --- a/internal/alertmanager/dedup.go +++ b/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 From def3236645d984f14d5a0d7b50d8ecdb63a4c0c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 22:54:45 +0100 Subject: [PATCH 15/18] fix(style): check of WriteString error return --- internal/models/alertgroup.go | 20 +++++++++++++++++--- internal/transform/colors.go | 15 +++++++++++++-- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/internal/models/alertgroup.go b/internal/models/alertgroup.go index c9703dd59..70439d641 100644 --- a/internal/models/alertgroup.go +++ b/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)) } diff --git a/internal/transform/colors.go b/internal/transform/colors.go index 5b70bf02c..c88e997f7 100644 --- a/internal/transform/colors.go +++ b/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) From 78bddce664cb19cf9cc1c95e0ba43aa3ccde0d80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 22:59:10 +0100 Subject: [PATCH 16/18] fix(style): check for bind errors in config --- internal/config/config.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index b7fa312e7..7eac867cd 100644 --- a/internal/config/config.go +++ b/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) } From c6dc362186b91950eb7434a11f45b2c89903409a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 23:03:02 +0100 Subject: [PATCH 17/18] fix(style): error strings should not be capitalized or end with punctuation or a newline --- assets.go | 2 +- internal/alertmanager/models.go | 2 +- internal/alertmanager/upstream.go | 4 ++-- internal/mapper/mapper.go | 4 ++-- internal/mock/mock.go | 2 +- internal/uri/http.go | 4 ++-- internal/uri/uri.go | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) diff --git a/assets.go b/assets.go index 0fb520134..17fb2ecdc 100644 --- a/assets.go +++ b/assets.go @@ -36,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, } diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index 57223b3fe..6d4b9346c 100644 --- a/internal/alertmanager/models.go +++ b/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 } diff --git a/internal/alertmanager/upstream.go b/internal/alertmanager/upstream.go index 343b49285..c09a9cba9 100644 --- a/internal/alertmanager/upstream.go +++ b/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 diff --git a/internal/mapper/mapper.go b/internal/mapper/mapper.go index b5916567b..29039996c 100644 --- a/internal/mapper/mapper.go +++ b/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) } diff --git a/internal/mock/mock.go b/internal/mock/mock.go index dc6ec1484..30a5858ed 100644 --- a/internal/mock/mock.go +++ b/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)) } diff --git a/internal/uri/http.go b/internal/uri/http.go index 6d9e8b2c4..df3135fcf 100644 --- a/internal/uri/http.go +++ b/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 diff --git a/internal/uri/uri.go b/internal/uri/uri.go index 1c0217e6d..6ee41e3e2 100644 --- a/internal/uri/uri.go +++ b/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) } } From e9e19c3ea8e5102e4d76acc4077c9e312d09ccc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 11 Aug 2018 23:06:08 +0100 Subject: [PATCH 18/18] fix(style): remove unused code --- proxy_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/proxy_test.go b/proxy_test.go index fa5e435a0..951cc5360 100644 --- a/proxy_test.go +++ b/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 }