chore: make lint more strict and fix all the issues and upgrade githu…

…b actions (#64)

Signed-off-by: Martin Chodur <[email protected]>

.github/workflows/go.yaml

@@ -14,15 +14,15 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version: "1.22"
 
       - name: Golangci-lint
-        uses: golangci/golangci-lint-action@v2.5.2
+        uses: golangci/golangci-lint-action@v4
 
       - name: Build
         run: make build

.github/workflows/release.yaml

@@ -13,21 +13,21 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
         with:
           fetch-depth: 0
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version: "1.21"
+          go-version: "1.22"
 
       - name: Login to Docker Hub
-        uses: docker/login-action@v1
+        uses: docker/login-action@v3
         with:
           username: ${{ secrets.DOCKERHUB_USERNAME }}
           password: ${{ secrets.DOCKERHUB_TOKEN }}
       - name: Run GoReleaser
-        uses: goreleaser/goreleaser-action@v2
+        uses: goreleaser/goreleaser-action@v5
         with:
           distribution: goreleaser
           version: latest

.golangci.yaml

@@ -1,14 +1,68 @@
 linters:
   enable:
+    - contextcheck
+    - durationcheck
     - errcheck
+    - errname
+    - errorlint
+    - gocritic
+    - godot
+    - gofmt
+    - gofumpt
+    - goimports
     - gosimple
     - govet
     - ineffassign
+    - misspell
+    - nakedret
+    - nilerr
+    - nilnil
+    - prealloc
+    - predeclared
+    - promlinter
+    - revive
     - staticcheck
+    - stylecheck
     - typecheck
+    - unconvert
+    - unparam
     - unused
-    - goimports
-    - revive
-    - promlinter
-    - gofmt
-    - errorlint
+    - usestdlibvars
+
+linters-settings:
+  # I'm biased and I'm enabling more than 100 checks
+  # Might be too much for you. See https://go-critic.com/overview.html
+  gocritic:
+    enabled-tags:
+      - diagnostic
+      - experimental
+      - opinionated
+      - performance
+      - style
+    disabled-checks:
+      # These 3 will detect many cases, but they do sense
+      # if it's performance oriented code
+      - hugeParam
+      - rangeExprCopy
+      - rangeValCopy
+
+  errcheck:
+    # Report `a := b.(MyStruct)` when `a, ok := ...` should be.
+    check-type-assertions: true # Default: false
+
+    # Report skipped checks:`num, _ := strconv.Atoi(numStr)`.
+    check-blank: true # Default: false
+
+    # Function to skip.
+    exclude-functions:
+      - io/ioutil.ReadFile
+      - io.Copy(*bytes.Buffer)
+      - io.Copy(os.Stdout)
+
+  govet:
+    disable:
+      - fieldalignment # I'm ok to waste some bytes
+
+  nakedret:
+    # No naked returns, ever.
+    max-func-lines: 1 # Default: 30

CHANGELOG.md

@@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added new `Alert` validator `validateLabelTemplates` to check if the alert labels are valid Go templates.
 - Added new `Alert` validator `keepFiringForIsNotLongerThan` to check if the alert does not have `keep_firing_for` longer then limit.
 - Updated all deps to the latest versions.
+- Upgraded go to 1.22
+- Update all Github actions in CI
+- Made lint more strict and fixed all the issues
 
 ### :warning: CHANGED validator scopes
   From now on each validator has restricted scopes it can be used with since they may not make sense in some contexts.

README.md

@@ -12,15 +12,15 @@ _Prometheus Rule Validator_
 Promtool allows users to verify syntactic correctness and test PromQL expressions.
 Promruval aims to validate the rules' metadata and expression properties
 to match requirements and constraints of the particular Prometheus cluster setup.
-User defines his validation rules in simple yaml configuration and passes them to
+User defines his validation rules in a simple yaml configuration and passes them to
 the promruval which validates specified files with Prometheus rules same way promtool does.
 Usually it would be used in the CI pipeline.
 You can read a blog post about the motivation and
 usage [here](https://fusakla.medium.com/promruval-validating-prometheus-rules-9a29f5dc24d2)
 or [watch a lightning talk about it from PromCon](https://www.youtube.com/watch?v=YYSJ--KhlIo&list=PLj6h78yzYM2PZb0QuIkm6ZY-xTuNA5zRO&index=16)
 .
 
-### Examples of usage
+### Example use-cases
 
 - Make sure the playbook, linked by an alert, is a valid URL and really exists.
 - Ensure the range selectors in the `expr` are not lower than three
@@ -35,11 +35,14 @@ or [watch a lightning talk about it from PromCon](https://www.youtube.com/watch?
 - Forbid usage of some labels or annotations if it got deprecated.
 - and many more...
 
+> As a good starting point you can use the [`docs/default_validation.yaml`](docs/default_validation.yaml) which contains
+some basic validations that are useful for most of the users.
+
 Validations are quite variable, so you can use them as you fit.
 
 **👉 Full list of supported validations can be found [here](docs/validations.md).**
 
-In case of any missing, please create a feature request!
+In case you would like to add some, please create a feature request!
 
 ### Installation
 

docs/default_validation.yaml

@@ -0,0 +1,39 @@
+validationRules:
+  - name: check-groups
+    scope: Group
+    validations:
+      - type: hasAllowedEvaluationInterval
+        params:
+          minimum: "20s"
+      - type: hasValidPartialResponseStrategy
+      - type: maxRulesPerGroup
+        params:
+          limit: 20
+  - name: check-alerts
+    scope: Alert
+    validations:
+      - type: validateLabelTemplates
+      - type: validateAnnotationTemplates
+      - type: forIsNotLongerThan
+        params:
+          limit: "1d"
+      - type: keepFiringForIsNotLongerThan
+        params:
+          limit: "1d"
+  - name: check-all-rules
+    scope: All rules
+    validations:
+      - type: nonEmptyLabels
+      - type: expressionDoesNotUseIrate
+      - type: validFunctionsOnCounters
+      - type: rateBeforeAggregation
+      - type: expressionWithNoMetricName
+      - type: expressionIsWellFormatted
+        params:
+          showExpectedForm: true
+      - type: expressionDoesNotUseRangeShorterThan
+        params:
+          limit: "1m"
+      - type: expressionDoesNotUseOlderDataThan
+        params:
+          limit: "4w"

docs/validations.md

@@ -1,6 +1,8 @@
 # Supported validations by scopes
 All the supported validations are listed here. The validations are grouped by the scope they can be used with.
 
+> If you want some sane default validations, you can look at the [default_validation.yaml](./default_validation.yaml). Those should be a good starting point for your own configuration and applicable to most of the use cases.
+
 - [Supported validations by scopes](#supported-validations-by-scopes)
   - [Groups](#groups)
     - [`hasValidSourceTenants`](#hasvalidsourcetenants)

go.mod

@@ -1,6 +1,6 @@
 module github.com/fusakla/promruval/v2
 
-go 1.21
+go 1.22
 
 require (
 	github.com/alecthomas/kingpin/v2 v2.4.0

main.go

@@ -55,10 +55,10 @@ func loadConfigFile(configFilePath string) (*config.Config, error) {
 	return validationConfig, nil
 }
 
-func validationRulesFromConfig(config *config.Config) ([]*validationrule.ValidationRule, error) {
+func validationRulesFromConfig(validationConfig *config.Config) ([]*validationrule.ValidationRule, error) {
 	var validationRules []*validationrule.ValidationRule
 rulesIteration:
-	for _, validationRule := range config.ValidationRules {
+	for _, validationRule := range validationConfig.ValidationRules {
 		for _, disabledRule := range *disabledRules {
 			if disabledRule == validationRule.Name {
 				continue rulesIteration
@@ -94,7 +94,6 @@ func exitWithError(err error) {
 }
 
 func main() {
-
 	currentCommand := kingpin.MustParse(app.Parse(os.Args[1:]))
 
 	if currentCommand == versionCmd.FullCommand() {
@@ -180,14 +179,19 @@ func main() {
 			prometheusClient.DumpCache()
 		}
 
+		var output string
 		switch *validationOutputFormat {
 		case "text":
-			fmt.Println(validationReport.AsText(2, *color))
+			output, err = validationReport.AsText(2, *color)
 		case "json":
-			fmt.Println(validationReport.AsJSON())
+			output, err = validationReport.AsJSON()
 		case "yaml":
-			fmt.Println(validationReport.AsYaml())
+			output, err = validationReport.AsYaml()
 		}
+		if err != nil {
+			exitWithError(err)
+		}
+		fmt.Println(output)
 		if validationReport.Failed {
 			os.Exit(1)
 		}

pkg/prometheus/cache.go

@@ -63,5 +63,5 @@ func (c *cache) Dump() {
 		log.Warnf("failed to write cache data: %s", err)
 		return
 	}
-	log.Infof("successfully dump cache to file %s\n", c.file)
+	log.Infof("successfully dumped cache to file %s", c.file)
 }

pkg/prometheus/mock.go

@@ -80,10 +80,10 @@ func (m mockV1Client) RoundTrip(_ *http.Request) (*http.Response, error) {
 	return m.newResponseMock(), nil
 }
 
-func NewClientMock(data interface{}, duration time.Duration, warning bool, error bool) *Client {
+func NewClientMock(data interface{}, duration time.Duration, warning, returnError bool) *Client {
 	cli, err := NewClientWithRoundTripper(config.PrometheusConfig{}, mockV1Client{
 		warning:  warning,
-		error:    error,
+		error:    returnError,
 		data:     data,
 		duration: duration,
 	})

pkg/prometheus/prometheus.go

@@ -14,13 +14,13 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-func NewClient(config config.PrometheusConfig) (*Client, error) {
-	return NewClientWithRoundTripper(config, &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipTLSVerify}})
+func NewClient(promConfig config.PrometheusConfig) (*Client, error) {
+	return NewClientWithRoundTripper(promConfig, &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: promConfig.InsecureSkipTLSVerify}})
 }
 
-func NewClientWithRoundTripper(config config.PrometheusConfig, tripper http.RoundTripper) (*Client, error) {
+func NewClientWithRoundTripper(promConfig config.PrometheusConfig, tripper http.RoundTripper) (*Client, error) {
 	cli, err := api.NewClient(api.Config{
-		Address:      config.URL,
+		Address:      promConfig.URL,
 		RoundTripper: tripper,
 	})
 	if err != nil {
@@ -29,9 +29,9 @@ func NewClientWithRoundTripper(config config.PrometheusConfig, tripper http.Roun
 	v1cli := v1.NewAPI(cli)
 	promClient := Client{
 		apiClient: v1cli,
-		url:       config.URL,
-		timeout:   config.Timeout,
-		cache:     newCache(config.CacheFile, config.MaxCacheAge),
+		url:       promConfig.URL,
+		timeout:   promConfig.Timeout,
+		cache:     newCache(promConfig.CacheFile, promConfig.MaxCacheAge),
 	}
 	return &promClient, nil
 }
@@ -110,7 +110,11 @@ func (s *Client) Query(query string) ([]*model.Sample, int, time.Duration, error
 		}
 		switch result.Type() {
 		case model.ValVector:
-			s.cache.Queries[query] = result.(model.Vector)
+			vectorResult, ok := result.(model.Vector)
+			if !ok {
+				return nil, 0, 0, fmt.Errorf("failed to convert result to model.Vector")
+			}
+			s.cache.Queries[query] = vectorResult
 		default:
 			return nil, 0, 0, fmt.Errorf("unknown prometheus response type: %s", result)
 		}

pkg/report/indentedOutput.go → pkg/report/indented_output.go

@@ -64,7 +64,7 @@ func (o *IndentedOutput) AddSuccessLine(line string) {
 	}
 }
 
-func (o *IndentedOutput) addColoredLine(line string, color string) {
+func (o *IndentedOutput) addColoredLine(line, color string) {
 	o.output += color + o.currentIndentation() + line + colorReset + "\n"
 }
 

pkg/report/report.go

@@ -155,7 +155,7 @@ func (r *RuleReport) AsText(output *IndentedOutput) {
 	output.WriteErrors(r.Errors)
 }
 
-func (r *ValidationReport) AsText(indentationStep int, color bool) string {
+func (r *ValidationReport) AsText(indentationStep int, color bool) (string, error) {
 	output := NewIndentedOutput(indentationStep, color)
 	output.AddLine("Validation rules used:")
 	output.IncreaseIndentation()
@@ -191,21 +191,27 @@ func (r *ValidationReport) AsText(indentationStep int, color bool) string {
 	output.AddLine(renderStatistic("Files", r.FilesCount, r.FilesExcludedCount))
 	output.AddLine(renderStatistic("Groups", r.GroupsCount, r.GroupsExcludedCount))
 	output.AddLine(renderStatistic("Rules", r.RulesCount, r.RulesExcludedCount))
-	return output.Text()
+	return output.Text(), nil
 }
 
-func renderStatistic(objectType string, total int, excluded int) string {
+func renderStatistic(objectType string, total, excluded int) string {
 	return fmt.Sprintf("%s: %d and %d of them excluded", objectType, total, excluded)
 }
 
-func (r *ValidationReport) AsJSON() string {
-	b, _ := json.MarshalIndent(r, "", "  ")
+func (r *ValidationReport) AsJSON() (string, error) {
+	b, err := json.MarshalIndent(r, "", "  ")
+	if err != nil {
+		return "", err
+	}
 	buffer := bytes.NewBuffer(b)
-	return buffer.String()
+	return buffer.String(), nil
 }
 
-func (r *ValidationReport) AsYaml() string {
-	b, _ := yaml.Marshal(r)
+func (r *ValidationReport) AsYaml() (string, error) {
+	b, err := yaml.Marshal(r)
+	if err != nil {
+		return "", err
+	}
 	buffer := bytes.NewBuffer(b)
-	return buffer.String()
+	return buffer.String(), nil
 }

pkg/report/validation.go

@@ -19,6 +19,7 @@ var htmlTemplate = `
 	  </ul>
 {{- end }}
 `
+
 var markdownTemplate = `
 # Validation rules
 {{- range .Rules }}