chore: make linting more strict and fix all the issues (#38)

Signed-off-by: Martin Chodur <[email protected]>
FUSAKLA · Dec 6, 2023 · 7d2edd1 · 7d2edd1
1 parent 7d11bb1
commit 7d2edd1
Show file tree

Hide file tree

Showing 13 changed files with 49 additions and 29 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -0,0 +1,14 @@
+linters:
+  enable:
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - typecheck
+    - unused
+    - goimports
+    - revive
+    - promlinter
+    - gofmt
+    - errorlint
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Improved the HTML output of human readable validation description.
 - Added examples of the human-readable validation descriptions to the examples dir.
 - Refactored the validation so it can use also group to validate the context of the rule.
+- Improve linting and fix all the linting issues.
 
 ## [v2.6.0] - 2023-12-06
 - Added new validator `expressionWithNoMetricName`, see its [docs](docs/validations.md#expressionwithnometricname). Thanks @tizki !

diff --git a/Makefile b/Makefile
@@ -30,6 +30,9 @@ $(RELEASE_NOTES): $(TMP_DIR)
 lint:
 	golangci-lint run
 
+lint-fix:
+	golangci-lint run --fix
+
 test:
 	go test -race ./...
 

diff --git a/main.go b/main.go
@@ -104,7 +104,7 @@ func main() {
 		if err != nil {
 			exitWithError(err)
 		}
-		if validationConfig.Prometheus.Url != "" {
+		if validationConfig.Prometheus.URL != "" {
 			mainValidationConfig.Prometheus = validationConfig.Prometheus
 		}
 		if validationConfig.CustomExcludeAnnotation != "" {
@@ -150,7 +150,7 @@ func main() {
 		}
 
 		var prometheusClient *prometheus.Client
-		if mainValidationConfig.Prometheus.Url != "" {
+		if mainValidationConfig.Prometheus.URL != "" {
 			prometheusClient, err = prometheus.NewClient(mainValidationConfig.Prometheus)
 			if err != nil {
 				exitWithError(fmt.Errorf("failed to initialize prometheus client: %w", err))
@@ -167,7 +167,7 @@ func main() {
 		}
 		validationReport := validate.Files(filesToBeValidated, validationRules, excludeAnnotation, disableValidatorsComment, prometheusClient)
 
-		if mainValidationConfig.Prometheus.Url != "" {
+		if mainValidationConfig.Prometheus.URL != "" {
 			prometheusClient.DumpCache()
 		}
 

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -25,9 +25,9 @@ type Config struct {
 }
 
 type PrometheusConfig struct {
-	Url                   string        `yaml:"url"`
+	URL                   string        `yaml:"url"`
 	Timeout               time.Duration `yaml:"timeout" default:"30s"`
-	InsecureSkipTlsVerify bool          `yaml:"insecureSkipTlsVerify"`
+	InsecureSkipTLSVerify bool          `yaml:"insecureSkipTlsVerify"`
 	CacheFile             string        `yaml:"cacheFile,omitempty" default:".promruval_cache.json"`
 	MaxCacheAge           time.Duration `yaml:"maxCacheAge,omitempty" default:"1h"`
 }

diff --git a/pkg/prometheus/cache.go b/pkg/prometheus/cache.go
@@ -2,10 +2,11 @@ package prometheus
 
 import (
 	"encoding/json"
-	"github.com/prometheus/common/model"
 	"os"
 	"time"
 
+	"github.com/prometheus/common/model"
+
 	log "github.com/sirupsen/logrus"
 )
 

diff --git a/pkg/prometheus/prometheus.go b/pkg/prometheus/prometheus.go
@@ -15,12 +15,12 @@ import (
 )
 
 func NewClient(config config.PrometheusConfig) (*Client, error) {
-	return NewClientWithRoundTripper(config, &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipTlsVerify}})
+	return NewClientWithRoundTripper(config, &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipTLSVerify}})
 }
 
 func NewClientWithRoundTripper(config config.PrometheusConfig, tripper http.RoundTripper) (*Client, error) {
 	cli, err := api.NewClient(api.Config{
-		Address:      config.Url,
+		Address:      config.URL,
 		RoundTripper: tripper,
 	})
 	if err != nil {
@@ -29,7 +29,7 @@ func NewClientWithRoundTripper(config config.PrometheusConfig, tripper http.Roun
 	v1cli := v1.NewAPI(cli)
 	promClient := Client{
 		apiClient: v1cli,
-		url:       config.Url,
+		url:       config.URL,
 		timeout:   config.Timeout,
 		cache:     newCache(config.CacheFile, config.MaxCacheAge),
 	}

diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go
@@ -32,7 +32,7 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 		if err != nil {
 			validationReport.Failed = true
 			fileReport.Valid = false
-			fileReport.Errors = []error{fmt.Errorf("cannot read file %s: %s", fileName, err)}
+			fileReport.Errors = []error{fmt.Errorf("cannot read file %s: %w", fileName, err)}
 			continue
 		}
 		var rf unmarshaler.RulesFile
@@ -41,7 +41,7 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 		if err != nil {
 			validationReport.Failed = true
 			fileReport.Valid = false
-			fileReport.Errors = []error{fmt.Errorf("invalid file %s: %s", fileName, err)}
+			fileReport.Errors = []error{fmt.Errorf("invalid file %s: %w", fileName, err)}
 			continue
 		}
 		for _, group := range rf.Groups {

diff --git a/pkg/validator/annotations.go b/pkg/validator/annotations.go
@@ -278,7 +278,7 @@ func (h annotationIsValidPromQL) Validate(_ unmarshaler.RuleGroup, rule rulefmt.
 		return []error{}
 	}
 	if _, err := parser.ParseExpr(value); err != nil {
-		return []error{fmt.Errorf("annotation `%s` is not valid PromQL: %s", h.annotation, err)}
+		return []error{fmt.Errorf("annotation `%s` is not valid PromQL: %w", h.annotation, err)}
 	}
 	return []error{}
 }

diff --git a/pkg/validator/expression.go b/pkg/validator/expression.go
@@ -39,7 +39,7 @@ func (h expressionDoesNotUseOlderDataThan) String() string {
 func (h expressionDoesNotUseOlderDataThan) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Rule, _ *prometheus.Client) []error {
 	expr, err := parser.ParseExpr(rule.Expr)
 	if err != nil {
-		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
 	}
 	var errs []error
 	parser.Inspect(expr, func(n parser.Node, ns []parser.Node) error {
@@ -127,7 +127,7 @@ func (h expressionDoesNotUseRangeShorterThan) String() string {
 func (h expressionDoesNotUseRangeShorterThan) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Rule, _ *prometheus.Client) []error {
 	expr, err := parser.ParseExpr(rule.Expr)
 	if err != nil {
-		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
 	}
 	var errs []error
 	parser.Inspect(expr, func(n parser.Node, ns []parser.Node) error {
@@ -159,7 +159,7 @@ func (h expressionDoesNotUseIrate) String() string {
 func (h expressionDoesNotUseIrate) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Rule, _ *prometheus.Client) []error {
 	expr, err := parser.ParseExpr(rule.Expr)
 	if err != nil {
-		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
 	}
 	var errs []error
 	parser.Inspect(expr, func(n parser.Node, ns []parser.Node) error {
@@ -200,7 +200,7 @@ func (h validFunctionsOnCounters) String() string {
 func (h validFunctionsOnCounters) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Rule, _ *prometheus.Client) []error {
 	expr, err := parser.ParseExpr(rule.Expr)
 	if err != nil {
-		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
 	}
 	var errs []error
 	match := regexp.MustCompile(`_total$`)
@@ -241,7 +241,7 @@ func (h rateBeforeAggregation) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Ru
 	var errs []error
 	expr, err := parser.ParseExpr(rule.Expr)
 	if err != nil {
-		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
 	}
 	parser.Inspect(expr, func(n parser.Node, ns []parser.Node) error {
 		switch n := n.(type) {
@@ -451,7 +451,7 @@ func (h expressionDoesNotUseMetrics) String() string {
 func (h expressionDoesNotUseMetrics) Validate(_ unmarshaler.RuleGroup, rule rulefmt.Rule, _ *prometheus.Client) []error {
 	expr, err := parser.ParseExpr(rule.Expr)
 	if err != nil {
-		return []error{fmt.Errorf("failed to parse expression `%s`: %s", rule.Expr, err)}
+		return []error{fmt.Errorf("failed to parse expression `%s`: %w", rule.Expr, err)}
 	}
 	var errs []error
 	usedMetrics, err := getExpressionMetrics(expr.String())

diff --git a/pkg/validator/expression_helpers.go b/pkg/validator/expression_helpers.go
@@ -11,7 +11,7 @@ import (
 func getExpressionUsedLabels(expr string) ([]string, error) {
 	promQl, err := parser.ParseExpr(expr)
 	if err != nil {
-		return []string{}, fmt.Errorf("failed to parse expression `%s`: %s", expr, err)
+		return []string{}, fmt.Errorf("failed to parse expression `%s`: %w", expr, err)
 	}
 	var usedLabels []string
 	parser.Inspect(promQl, func(n parser.Node, ns []parser.Node) error {
@@ -37,7 +37,7 @@ func getExpressionUsedLabels(expr string) ([]string, error) {
 func getExpressionVectorSelectors(expr string) ([]*parser.VectorSelector, error) {
 	promQl, err := parser.ParseExpr(expr)
 	if err != nil {
-		return []*parser.VectorSelector{}, fmt.Errorf("failed to parse expression `%s`: %s", expr, err)
+		return []*parser.VectorSelector{}, fmt.Errorf("failed to parse expression `%s`: %w", expr, err)
 	}
 	var selectors []*parser.VectorSelector
 	parser.Inspect(promQl, func(n parser.Node, ns []parser.Node) error {
@@ -63,8 +63,8 @@ func getVectorSelectorMetricName(selector *parser.VectorSelector) string {
 
 // MetricWithVectorSelector is a struct that contains a metric name and a vector selector where it is used, to give a context, in the error messages.
 type MetricWithVectorSelector struct {
-	VectorSelector     *parser.VectorSelector
-	Name string
+	VectorSelector *parser.VectorSelector
+	Name           string
 }
 
 func getExpressionMetrics(expr string) ([]MetricWithVectorSelector, error) {

diff --git a/pkg/validator/expression_helpers_test.go b/pkg/validator/expression_helpers_test.go
@@ -1,6 +1,7 @@
 package validator
 
 import (
+	"errors"
 	"reflect"
 	"testing"
 
@@ -27,7 +28,7 @@ func TestGetExpressionUsedLabels(t *testing.T) {
 	for _, test := range tests {
 		labels, err := getExpressionUsedLabels(test.expr)
 		assert.ElementsMatch(t, labels, test.expected, "Expected labels %v, but got %v", test.expected, labels)
-		if err != test.expectedErr {
+		if !errors.Is(err, test.expectedErr) {
 			t.Errorf("Expected error %v, but got %v", test.expectedErr, err)
 		}
 	}
@@ -62,7 +63,7 @@ func TestGetExpressionMetrics(t *testing.T) {
 		if !reflect.DeepEqual(results, test.expected) {
 			t.Errorf("Expected metric names %v, but got %v", test.expected, results)
 		}
-		if err != test.expectedErr {
+		if !errors.Is(err, test.expectedErr) {
 			t.Errorf("Expected error %v, but got %v", test.expectedErr, err)
 		}
 	}
@@ -89,7 +90,7 @@ func TestGetExpressionSelectors(t *testing.T) {
 		if !reflect.DeepEqual(selectors, test.expected) {
 			t.Errorf("Expected selectors %v, but got %v", test.expected, selectors)
 		}
-		if err != test.expectedErr {
+		if !errors.Is(err, test.expectedErr) {
 			t.Errorf("Expected error %v, but got %v", test.expectedErr, err)
 		}
 	}

diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go
@@ -195,10 +195,10 @@ var testCases = []struct {
 
 	// hasSourceTenantsForMetrics
 	{name: "emptyMapping", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 0},
-	{name: "usesMetricWithSourceTenantAndGroupHasSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant1"}},rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0},
-	{name: "usesMetricWithSourceTenantAndGroupDoesNotHaveSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2"}},rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1},
-	{name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenants", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant1"}},rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0},
-	{name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenantsAndOneIsMissing", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant3"}},rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1},
+	{name: "usesMetricWithSourceTenantAndGroupHasSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant1"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0},
+	{name: "usesMetricWithSourceTenantAndGroupDoesNotHaveSourceTenant", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1},
+	{name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenants", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant1"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 0},
+	{name: "usesMetricWithSourceTenantAndGroupHasMultipleSourceTenantsAndOneIsMissing", validator: hasSourceTenantsForMetrics{sourceTenants: map[string]*regexp.Regexp{"tenant1": regexp.MustCompile(`^teanant1_metric$`)}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant2", "tenant3"}}, rule: rulefmt.Rule{Expr: `teanant1_metric{foo="bar"}`}, expectedErrors: 1},
 }
 
 func Test(t *testing.T) {