Merge pull request #99 from cloudflare/dups

Don't enable multiple instances of the same check
cloudflare · Dec 23, 2021 · 0b3fc40 · 0b3fc40
2 parents dd530dd + cbb4e9b
commit 0b3fc40
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,15 @@
 # Changelog
 
+## v0.5.2
+
+### Added
+
+- `--no-color` flag for disabling output colouring.
+
+### Fixed
+
+- Fixed duplicated warnings when multiple `rule {...}` blocks where configured.
+
 ## v0.5.1
 
 ### Fixed

diff --git a/cmd/pint/ci.go b/cmd/pint/ci.go
@@ -19,7 +19,7 @@ import (
 )
 
 func actionCI(c *cli.Context) (err error) {
-	err = initLogger(c.String(logLevelFlag))
+	err = initLogger(c.String(logLevelFlag), c.Bool(noColorFlag))
 	if err != nil {
 		return fmt.Errorf("failed to set log level: %w", err)
 	}

diff --git a/cmd/pint/config.go b/cmd/pint/config.go
@@ -10,7 +10,7 @@ import (
 )
 
 func actionConfig(c *cli.Context) (err error) {
-	err = initLogger(c.String(logLevelFlag))
+	err = initLogger(c.String(logLevelFlag), c.Bool(noColorFlag))
 	if err != nil {
 		return fmt.Errorf("failed to set log level: %w", err)
 	}

diff --git a/cmd/pint/lint.go b/cmd/pint/lint.go
@@ -14,7 +14,7 @@ import (
 )
 
 func actionLint(c *cli.Context) (err error) {
-	err = initLogger(c.String(logLevelFlag))
+	err = initLogger(c.String(logLevelFlag), c.Bool(noColorFlag))
 	if err != nil {
 		return fmt.Errorf("failed to set log level: %w", err)
 	}

diff --git a/cmd/pint/logger.go b/cmd/pint/logger.go
@@ -18,10 +18,10 @@ func lvlFormatter(level interface{}) string {
 	return fmt.Sprintf("level=%s", level)
 }
 
-func initLogger(level string) error {
+func initLogger(level string, noColor bool) error {
 	log.Logger = log.Logger.Output(zerolog.ConsoleWriter{
 		Out:           os.Stderr,
-		NoColor:       false,
+		NoColor:       noColor,
 		FormatLevel:   lvlFormatter,
 		FormatMessage: msgFormatter,
 		FormatTimestamp: func(interface{}) string {

diff --git a/cmd/pint/main.go b/cmd/pint/main.go
@@ -16,6 +16,7 @@ const (
 	logLevelFlag = "log-level"
 	disabledFlag = "disabled"
 	offlineFlag  = "offline"
+	noColorFlag  = "no-color"
 )
 
 var (
@@ -39,6 +40,12 @@ func newApp() *cli.App {
 				Value:   zerolog.InfoLevel.String(),
 				Usage:   "Log level",
 			},
+			&cli.BoolFlag{
+				Name:    noColorFlag,
+				Aliases: []string{"n"},
+				Value:   false,
+				Usage:   "Disable output colouring",
+			},
 		},
 		Commands: []*cli.Command{
 			{
@@ -108,7 +115,7 @@ func main() {
 }
 
 func actionVersion(c *cli.Context) (err error) {
-	err = initLogger(c.String(logLevelFlag))
+	err = initLogger(c.String(logLevelFlag), c.Bool(noColorFlag))
 	if err != nil {
 		return fmt.Errorf("failed to set log level: %w", err)
 	}

diff --git a/cmd/pint/parse.go b/cmd/pint/parse.go
@@ -126,7 +126,7 @@ func parseQuery(query string) error {
 }
 
 func actionParse(c *cli.Context) (err error) {
-	err = initLogger(c.String(logLevelFlag))
+	err = initLogger(c.String(logLevelFlag), c.Bool(noColorFlag))
 	if err != nil {
 		return fmt.Errorf("failed to set log level: %w", err)
 	}

diff --git a/cmd/pint/tests/0023_enabled_checks.txt b/cmd/pint/tests/0023_enabled_checks.txt
@@ -0,0 +1,27 @@
+pint.error -l debug --no-color lint rules
+! stdout .
+stderr 'level=debug msg="Configured checks for rule" enabled=\["promql/syntax","alerts/for","promql/comparison","alerts/template","promql/rate\(prom\)","query/series\(prom\)","promql/vector_matching"\] path=rules/1.yaml rule=one'
+stderr 'level=debug msg="Configured checks for rule" enabled=\["promql/syntax","alerts/for","promql/comparison","alerts/template","promql/rate\(prom\)","query/series\(prom\)","promql/vector_matching"\] path=rules/1.yaml rule=two'
+stderr 'level=debug msg="Configured checks for rule" enabled=\["promql/syntax","alerts/for","promql/comparison","alerts/template","promql/rate\(prom\)","query/series\(prom\)","promql/vector_matching"\] path=rules/2.yaml rule=one'
+stderr 'level=debug msg="Configured checks for rule" enabled=\["promql/syntax","alerts/for","promql/comparison","alerts/template","promql/rate\(prom\)","query/series\(prom\)","promql/vector_matching"\] path=rules/2.yaml rule=two'
+
+-- rules/1.yaml --
+- record: one
+  expr: up == 0
+- record: two
+  expr: up == 0
+-- rules/2.yaml --
+- record: one
+  expr: up == 0
+- record: two
+  expr: up == 0
+
+-- .pint.hcl --
+prometheus "prom" {
+  uri     = "https://"
+  timeout = "2m"
+}
+
+rule{}
+rule{}
+rule{}
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -81,17 +81,8 @@ func (cfg Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChec
 			continue
 		}
 		proms = append(proms, prom)
-		timeout, _ := parseDuration(prom.Timeout)
-		if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.RateCheckName, r) {
-			enabled = append(enabled, checks.NewRateCheck(prom.Name, prom.URI, timeout))
-		}
-		if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.SeriesCheckName, r) {
-			enabled = append(enabled, checks.NewSeriesCheck(prom.Name, prom.URI, timeout))
-		}
-		if isEnabled(cfg.Checks.Enabled, cfg.Checks.Disabled, checks.VectorMatchingCheckName, r) {
-			enabled = append(enabled, checks.NewVectorMatchingCheck(prom.Name, prom.URI, timeout))
-		}
 	}
+
 	for _, rule := range cfg.Rules {
 		for _, c := range rule.resolveChecks(path, r, cfg.Checks.Enabled, cfg.Checks.Disabled, proms) {
 			if r.HasComment(fmt.Sprintf("disable %s", removeRedundantSpaces(c.String()))) {
@@ -101,7 +92,17 @@ func (cfg Config) GetChecksForRule(path string, r parser.Rule) []checks.RuleChec
 					Msg("Check disabled by comment")
 				continue
 			}
-			enabled = append(enabled, c)
+			// check if rule was already enabled
+			var v bool
+			for _, er := range enabled {
+				if er.String() == c.String() {
+					v = true
+					break
+				}
+			}
+			if !v {
+				enabled = append(enabled, c)
+			}
 		}
 	}
 

diff --git a/internal/config/rule.go b/internal/config/rule.go
@@ -199,6 +199,20 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 		}
 	}
 
+	if isEnabled(enabledChecks, disabledChecks, checks.SeriesCheckName, r) {
+		for _, prom := range proms {
+			timeout, _ := parseDuration(prom.Timeout)
+			enabled = append(enabled, checks.NewSeriesCheck(prom.Name, prom.URI, timeout))
+		}
+	}
+
+	if isEnabled(enabledChecks, disabledChecks, checks.VectorMatchingCheckName, r) {
+		for _, prom := range proms {
+			timeout, _ := parseDuration(prom.Timeout)
+			enabled = append(enabled, checks.NewVectorMatchingCheck(prom.Name, prom.URI, timeout))
+		}
+	}
+
 	if rule.Cost != nil && isEnabled(enabledChecks, disabledChecks, checks.CostCheckName, r) {
 		severity := rule.Cost.getSeverity(checks.Bug)
 		for _, prom := range proms {
@@ -228,13 +242,6 @@ func (rule Rule) resolveChecks(path string, r parser.Rule, enabledChecks, disabl
 		}
 	}
 
-	if isEnabled(enabledChecks, disabledChecks, checks.SeriesCheckName, r) {
-		for _, prom := range proms {
-			timeout, _ := parseDuration(prom.Timeout)
-			enabled = append(enabled, checks.NewSeriesCheck(prom.Name, prom.URI, timeout))
-		}
-	}
-
 	if rule.Alerts != nil && isEnabled(enabledChecks, disabledChecks, checks.AlertsCheckName, r) {
 		qRange := time.Hour * 24
 		if rule.Alerts.Range != "" {