Add support for validation of rule groups themselves (#46)

* feat: add a new Group validation rule scope and make the allowedSourceTenants validator only work for this scope

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

* fix: lint

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

---------

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

CHANGELOG.md

@@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- Added new validation rule scope `Group` to validate the rule group itself (not the rules in it).
+- CHANGED: The validator `allowedSourceTenants` is now allowed only in the `Group` scope validation rules.
+
 ## [v2.7.1] - 2024-02-01
 - Upgrade all dependencies
 - Fix: `promruval version` now works without specifying `--config-file`

README.md

@@ -145,7 +145,7 @@ prometheus:
 validationRules:
   # Name of the validation rule.
   - name: example-validation
-    # What Prometheus rules to validate, possible values are: 'Alert', 'Recording rule', 'All rules'.
+    # What Prometheus rules to validate, possible values are: 'Group', 'Alert', 'Recording rule', 'All rules'.
     scope: All rules
     # List of validations to be used.
     validations:

docs/validations.md

@@ -30,10 +30,11 @@
     - [`expressionUsesExistingLabels`](#expressionusesexistinglabels)
     - [`expressionSelectorsMatchesAnything`](#expressionselectorsmatchesanything)
     - [`expressionWithNoMetricName`](#expressionwithnometricname)
-  - [Alert](#alert)
+  - [Alerts](#alerts)
     - [`forIsNotLongerThan`](#forisnotlongerthan)
   - [Others](#others)
     - [`hasSourceTenantsForMetrics`](#hassourcetenantsformetrics)
+  - [Groups](#groups)
     - [`hasValidSourceTenants`](#hasvalidsourcetenants)
 
 ## Labels
@@ -281,7 +282,7 @@ instance.
 
 Fails if an expression doesn't use an explicit metric name (also if used as `__name__` label) in all its selectors(eg `up{foo="bar"}`).
 
-## Alert
+## Alerts
 
 ### `forIsNotLongerThan`
 
@@ -305,6 +306,9 @@ params:
     # k8s: "kube_.*|container_.*"
 ```
 
+## Groups
+Validators that are designed to validate the group itself. Not the rules within it.
+
 ### `hasValidSourceTenants`
 
 Fails if the rule group has other than than the configured source tenants.

examples/human_readable.html

@@ -38,14 +38,18 @@ <h2><a href="#check-prometheus-limitations">check-prometheus-limitations</a></h2
   <h2><a href="#check-source-tenants">check-source-tenants</a></h2>
 	  <ul>
 		<li>All rules rule group, the rule belongs to, has the required <code>source_tenants</code> configured, according to the mapping of metric names to tenants: <code>k8s</code>:<code>^container_.*|kube_.*$</code></li>
-		<li>All rules rule group, the rule belongs to, does not have other <code>source_tenants</code> than: <code>tenant1</code>, <code>tenant2</code>, <code>k8s</code></li>
 	  </ul>
 
   <h2><a href="#check-metric-name">check-metric-name</a></h2>
 	  <ul>
 		<li>Alert expression uses metric name in selectors</li>
 	  </ul>
 
+  <h2><a href="#check-groups">check-groups</a></h2>
+	  <ul>
+		<li>Group does not have other <code>source_tenants</code> than: <code>tenant1</code>, <code>tenant2</code>, <code>k8s</code></li>
+	  </ul>
+
   <h2><a href="#another-checks">another-checks</a></h2>
 	  <ul>
 		<li>All rules labels does not have empty values</li>

examples/human_readable.md

@@ -27,11 +27,13 @@ Validation rules:
 
   check-source-tenants
     - All rules rule group, the rule belongs to, has the required `source_tenants` configured, according to the mapping of metric names to tenants: `k8s`:`^container_.*|kube_.*$`
-    - All rules rule group, the rule belongs to, does not have other `source_tenants` than: `tenant1`, `tenant2`, `k8s`
 
   check-metric-name
     - Alert expression uses metric name in selectors
 
+  check-groups
+    - Group does not have other `source_tenants` than: `tenant1`, `tenant2`, `k8s`
+
   another-checks
     - All rules labels does not have empty values
 

examples/validation.yaml

@@ -77,11 +77,15 @@ validationRules:
         params:
           sourceTenants:
             k8s: "container_.*|kube_.*"
-      - type: hasAllowedSourceTenants
-        params:
-          allowedSourceTenants: ["tenant1", "tenant2", "k8s"]
 
   - name: check-metric-name
     scope: Alert
     validations:
       - type: expressionWithNoMetricName
+
+  - name: check-groups
+    scope: Group
+    validations:
+      - type: hasAllowedSourceTenants
+        params:
+          allowedSourceTenants: ["tenant1", "tenant2", "k8s"]

main.go

@@ -72,7 +72,10 @@ rulesIteration:
 		}
 		newRule := validationrule.New(validationRule.Name, validationRule.Scope)
 		for _, validatorConfig := range validationRule.Validations {
-			newValidator, err := validator.NewFromConfig(validatorConfig)
+			if err := validator.KnownValidators(validationRule.Scope, []string{validatorConfig.ValidatorType}); err != nil {
+				return nil, err
+			}
+			newValidator, err := validator.NewFromConfig(validationRule.Scope, validatorConfig)
 			if err != nil {
 				return nil, fmt.Errorf("loading validator config: %w", err)
 			}

pkg/config/config.go

@@ -13,9 +13,10 @@ const (
 	AlertScope         ValidationScope = "Alert"
 	RecordingRuleScope ValidationScope = "Recording rule"
 	AllRulesScope      ValidationScope = "All rules"
+	Group              ValidationScope = "Group"
 )
 
-var ValidationScopes = []ValidationScope{AlertScope, RecordingRuleScope, AllRulesScope}
+var ValidationScopes = []ValidationScope{Group, AlertScope, RecordingRuleScope, AllRulesScope}
 
 type Config struct {
 	CustomExcludeAnnotation string           `yaml:"customExcludeAnnotation"`

pkg/report/report.go

@@ -68,6 +68,7 @@ func (r *FileReport) NewGroupReport(name string) *GroupReport {
 		Name:        name,
 		Valid:       true,
 		RuleReports: []*RuleReport{},
+		Errors:      []error{},
 	}
 	r.GroupReports = append(r.GroupReports, &newReport)
 	return &newReport
@@ -92,6 +93,7 @@ type GroupReport struct {
 	Name        string
 	Excluded    bool
 	RuleReports []*RuleReport
+	Errors      []error
 }
 
 func (r *GroupReport) NewRuleReport(name string, ruleType config.ValidationScope) *RuleReport {
@@ -116,6 +118,12 @@ func (r *GroupReport) AsText(output *IndentedOutput) {
 		output.AddLine("Skipped")
 		return
 	}
+	if r.Errors != nil {
+		output.AddLine("Group level errors:")
+		output.IncreaseIndentation()
+		output.WriteErrors(r.Errors)
+		output.DecreaseIndentation()
+	}
 	if len(r.RuleReports) == 0 {
 		output.AddLine("No rules")
 		return

pkg/validate/validate.go

@@ -13,6 +13,7 @@ import (
 	"github.com/fusakla/promruval/v2/pkg/unmarshaler"
 	"github.com/fusakla/promruval/v2/pkg/validationrule"
 	"github.com/fusakla/promruval/v2/pkg/validator"
+	"github.com/prometheus/prometheus/model/rulefmt"
 	log "github.com/sirupsen/logrus"
 	"gopkg.in/yaml.v3"
 )
@@ -47,6 +48,22 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 		for _, group := range rf.Groups {
 			validationReport.GroupsCount++
 			groupReport := fileReport.NewGroupReport(group.Name)
+			for _, rule := range validationRules {
+				if rule.Scope() != config.Group {
+					continue
+				}
+				for _, v := range rule.Validators() {
+					validatorName := reflect.TypeOf(v).Elem().Name()
+					for _, err := range v.Validate(group, rulefmt.Rule{}, prometheusClient) {
+						groupReport.Errors = append(groupReport.Errors, fmt.Errorf("%s: %w", validatorName, err))
+					}
+				}
+			}
+			if len(groupReport.Errors) > 0 {
+				validationReport.Failed = true
+				fileReport.Valid = false
+				groupReport.Valid = false
+			}
 			for _, ruleNode := range group.Rules {
 				originalRule := ruleNode.OriginalRule()
 				var ruleReport *report.RuleReport
@@ -61,10 +78,13 @@ func Files(fileNames []string, validationRules []*validationrule.ValidationRule,
 					excludedRules = strings.Split(excludedRulesText, ",")
 				}
 				disabledValidators := ruleNode.DisabledValidators(disableValidationsComment)
-				if err := validator.KnownValidators(disabledValidators); err != nil {
+				if err := validator.KnownValidators(config.AllRulesScope, disabledValidators); err != nil {
 					ruleReport.Errors = append(ruleReport.Errors, err)
 				}
 				for _, rule := range validationRules {
+					if rule.Scope() == config.Group {
+						continue
+					}
 					skipRule := false
 					if (rule.Scope() != ruleReport.RuleType) && (rule.Scope() != config.AllRulesScope) {
 						skipRule = true

pkg/validator/annotations.go

@@ -307,7 +307,7 @@ func (h validateAnnotationTemplates) Validate(_ unmarshaler.RuleGroup, rule rule
 		"{{$value := .Value}}",
 	}
 	for k, v := range rule.Annotations {
-		t := template.NewTemplateExpander(context.Background(), strings.Join(append(defs, v), ""), k, data, model.Now(), func(ctx context.Context, s string, time time.Time) (promql.Vector, error) { return nil, nil }, &url.URL{}, []string{})
+		t := template.NewTemplateExpander(context.Background(), strings.Join(append(defs, v), ""), k, data, model.Now(), func(_ context.Context, _ string, _ time.Time) (promql.Vector, error) { return nil, nil }, &url.URL{}, []string{})
 		if _, err := t.Expand(); err != nil && !strings.Contains(err.Error(), "error executing template") {
 			errs = append(errs, fmt.Errorf("invalid template of annotation %s: %w", k, err))
 		}

pkg/validator/config.go

@@ -2,14 +2,15 @@ package validator
 
 import (
 	"fmt"
+	"maps"
 
 	"github.com/fusakla/promruval/v2/pkg/config"
 	"gopkg.in/yaml.v3"
 )
 
 type validatorCreator func(params yaml.Node) (Validator, error)
 
-var registeredValidators = map[string]validatorCreator{
+var registeredRuleValidators = map[string]validatorCreator{
 	"hasLabels":                            newHasLabels,
 	"hasAnnotations":                       newHasAnnotations,
 	"doesNotHaveLabels":                    newDoesNotHaveLabels,
@@ -38,28 +39,40 @@ var registeredValidators = map[string]validatorCreator{
 	"expressionSelectorsMatchesAnything":   newExpressionSelectorsMatchesAnything,
 	"expressionWithNoMetricName":           newExpressionWithNoMetricName,
 	"hasSourceTenantsForMetrics":           newHasSourceTenantsForMetrics,
-	"hasAllowedSourceTenants":              newHasAllowedSourceTenants,
 }
 
-func NewFromConfig(config config.ValidatorConfig) (Validator, error) {
-	validatorFactory, ok := registeredValidators[config.ValidatorType]
+var registeredGroupValidators = map[string]validatorCreator{
+	"hasAllowedSourceTenants": newHasAllowedSourceTenants,
+}
+
+var registeredValidators = map[string]validatorCreator{}
+
+func init() {
+	maps.Copy(registeredValidators, registeredRuleValidators)
+	maps.Copy(registeredValidators, registeredGroupValidators)
+}
+
+func NewFromConfig(scope config.ValidationScope, config config.ValidatorConfig) (Validator, error) {
+	factory, ok := creator(scope, config.ValidatorType)
 	if !ok {
 		return nil, fmt.Errorf("unknown validator type `%s`", config.ValidatorType)
 	}
-	return validatorFactory(config.Params)
+	return factory(config.Params)
 }
 
-func KnownValidatorName(name string) bool {
-	if _, ok := registeredValidators[name]; ok {
-		return true
+func creator(scope config.ValidationScope, name string) (validatorCreator, bool) {
+	validators := registeredRuleValidators
+	if scope == config.Group {
+		validators = registeredGroupValidators
 	}
-	return false
+	creator, ok := validators[name]
+	return creator, ok
 }
 
-func KnownValidators(validatorNames []string) error {
+func KnownValidators(scope config.ValidationScope, validatorNames []string) error {
 	for _, validatorName := range validatorNames {
-		if !KnownValidatorName(validatorName) {
-			return fmt.Errorf("unknown validator `%s`", validatorName)
+		if _, ok := creator(scope, validatorName); !ok {
+			return fmt.Errorf("unknown validator `%s` for given validation rule scope %s", validatorName, scope)
 		}
 	}
 	return nil

pkg/validator/expression.go

@@ -42,7 +42,7 @@ func (h expressionDoesNotUseOlderDataThan) Validate(_ unmarshaler.RuleGroup, rul
 		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 {
+	parser.Inspect(expr, func(n parser.Node, _ []parser.Node) error {
 		// TODO(FUSAKLA) Having range query in subquery should have the time added.
 		switch n := n.(type) {
 		case *parser.MatrixSelector:
@@ -130,7 +130,7 @@ func (h expressionDoesNotUseRangeShorterThan) Validate(_ unmarshaler.RuleGroup,
 		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 {
+	parser.Inspect(expr, func(n parser.Node, _ []parser.Node) error {
 		switch v := n.(type) {
 		case *parser.MatrixSelector:
 			if v.Range < time.Duration(h.limit) {
@@ -162,7 +162,7 @@ func (h expressionDoesNotUseIrate) Validate(_ unmarshaler.RuleGroup, rule rulefm
 		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 {
+	parser.Inspect(expr, func(n parser.Node, _ []parser.Node) error {
 		switch v := n.(type) {
 		case *parser.Call:
 			if v != nil && v.Func != nil && v.Func.Name == "irate" {
@@ -207,7 +207,7 @@ func (h validFunctionsOnCounters) Validate(_ unmarshaler.RuleGroup, rule rulefmt
 	if h.allowHistograms {
 		match = regexp.MustCompile(`(_total|_count|_bucket|_sum)$`)
 	}
-	parser.Inspect(expr, func(n parser.Node, ns []parser.Node) error {
+	parser.Inspect(expr, func(n parser.Node, _ []parser.Node) error {
 		switch v := n.(type) {
 		case *parser.Call:
 			if v == nil || v.Func == nil || (v.Func.Name != "rate" && v.Func.Name != "increase") {

pkg/validator/expression_helpers.go

@@ -14,7 +14,7 @@ func getExpressionUsedLabels(expr string) ([]string, error) {
 		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 {
+	parser.Inspect(promQl, func(n parser.Node, _ []parser.Node) error {
 		switch v := n.(type) {
 		case *parser.AggregateExpr:
 			usedLabels = append(usedLabels, v.Grouping...)
@@ -40,7 +40,7 @@ func getExpressionVectorSelectors(expr string) ([]*parser.VectorSelector, error)
 		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 {
+	parser.Inspect(promQl, func(n parser.Node, _ []parser.Node) error {
 		switch v := n.(type) {
 		case *parser.VectorSelector:
 			selectors = append(selectors, &parser.VectorSelector{Name: v.Name, LabelMatchers: v.LabelMatchers})

pkg/validator/expression_helpers_test.go

@@ -115,7 +115,7 @@ func TestGetVectorSelectorMetricName(t *testing.T) {
 		promQl, err := parser.ParseExpr(test.vectorSelectorString)
 		assert.NoError(t, err)
 		var selectors []*parser.VectorSelector
-		parser.Inspect(promQl, func(n parser.Node, ns []parser.Node) error {
+		parser.Inspect(promQl, func(n parser.Node, _ []parser.Node) error {
 			switch v := n.(type) {
 			case *parser.VectorSelector:
 				selectors = append(selectors, &parser.VectorSelector{Name: v.Name, LabelMatchers: v.LabelMatchers})

pkg/validator/group.go

@@ -0,0 +1,43 @@
+package validator
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/fusakla/promruval/v2/pkg/prometheus"
+	"github.com/fusakla/promruval/v2/pkg/unmarshaler"
+	"github.com/prometheus/prometheus/model/rulefmt"
+	"golang.org/x/exp/slices"
+	"gopkg.in/yaml.v3"
+)
+
+func newHasAllowedSourceTenants(paramsConfig yaml.Node) (Validator, error) {
+	params := struct {
+		AllowedSourceTenants []string `yaml:"allowedSourceTenants"`
+	}{}
+	if err := paramsConfig.Decode(&params); err != nil {
+		return nil, err
+	}
+	return &hasAllowedSourceTenants{allowedSourceTenants: params.AllowedSourceTenants}, nil
+}
+
+type hasAllowedSourceTenants struct {
+	allowedSourceTenants []string
+}
+
+func (h hasAllowedSourceTenants) String() string {
+	return fmt.Sprintf("does not have other `source_tenants` than: `%s`", strings.Join(h.allowedSourceTenants, "`, `"))
+}
+
+func (h hasAllowedSourceTenants) Validate(group unmarshaler.RuleGroup, _ rulefmt.Rule, _ *prometheus.Client) []error {
+	var invalidTenants []string
+	for _, tenant := range group.SourceTenants {
+		if !slices.Contains(h.allowedSourceTenants, tenant) {
+			invalidTenants = append(invalidTenants, tenant)
+		}
+	}
+	if len(invalidTenants) == 0 {
+		return []error{}
+	}
+	return []error{fmt.Errorf("group has invalid source_tenants: `%s`", strings.Join(invalidTenants, "`,`"))}
+}