Fus fix interval params validation (#51)

* fix: validation of the hasAllowedSourceTenants params Signed-off-by: Martin Chodur <[email protected]> * fix: change hasAllowedEvaluationInterval params from time.Duration to Prometheus model.Duration Signed-off-by: Martin Chodur <[email protected]> * chore: changelog Signed-off-by: Martin Chodur <[email protected]> --------- Signed-off-by: Martin Chodur <[email protected]>
FUSAKLA · Feb 29, 2024 · 6854f89 · 6854f89
1 parent 5164ab4
commit 6854f89
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Fixed param validation of the `hasAllowedEvaluationInterval` validator, if the `maximum` was not set.
 
 ## [v2.8.0] - 2024-02-29
 - Added new param `ignoreTemplatedValues` to the `labelHasAllowedValue` validator to ignore templated values in the label.

diff --git a/examples/human_readable.html b/examples/human_readable.html
@@ -48,7 +48,7 @@ <h2><a href="#check-metric-name">check-metric-name</a></h2>
   <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>
-		<li>Group evaluation interval is between <code>20s</code> and <code>10m0s</code> if set</li>
+		<li>Group evaluation interval is between <code>20s</code> and <code>106751d23h47m16s854ms</code> if set</li>
 		<li>Group has valid partial_response_strategy (one of <code>warn</code> or <code>abort</code>) if set</li>
 	  </ul>
 

diff --git a/examples/human_readable.md b/examples/human_readable.md
@@ -33,7 +33,7 @@ Validation rules:
 
   check-groups
     - Group does not have other `source_tenants` than: `tenant1`, `tenant2`, `k8s`
-    - Group evaluation interval is between `20s` and `10m0s` if set
+    - Group evaluation interval is between `20s` and `106751d23h47m16s854ms` if set
     - Group has valid partial_response_strategy (one of `warn` or `abort`) if set
 
   another-checks

diff --git a/examples/validation.yaml b/examples/validation.yaml
@@ -93,6 +93,5 @@ validationRules:
       - type: hasAllowedEvaluationInterval
         params:
           minimum: "20s"
-          maximum: "10m"
           intervalMustBeSet: false
       - type: hasValidPartialStrategy
diff --git a/pkg/validator/group.go b/pkg/validator/group.go
@@ -3,10 +3,10 @@ package validator
 import (
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/fusakla/promruval/v2/pkg/prometheus"
 	"github.com/fusakla/promruval/v2/pkg/unmarshaler"
+	"github.com/prometheus/common/model"
 	"github.com/prometheus/prometheus/model/rulefmt"
 	"golang.org/x/exp/slices"
 	"gopkg.in/yaml.v3"
@@ -45,29 +45,28 @@ func (h hasAllowedSourceTenants) Validate(group unmarshaler.RuleGroup, _ rulefmt
 
 func newHasAllowedEvaluationInterval(paramsConfig yaml.Node) (Validator, error) {
 	params := struct {
-		MinimumEvaluationInterval time.Duration `yaml:"minimum"`
-		MaximumEvaluationInterval time.Duration `yaml:"maximum"`
-		MustBeSet                 bool          `yaml:"intervalMustBeSet"`
+		Minimum   model.Duration `yaml:"minimum"`
+		Maximum   model.Duration `yaml:"maximum"`
+		MustBeSet bool           `yaml:"intervalMustBeSet"`
 	}{}
 	if err := paramsConfig.Decode(&params); err != nil {
 		return nil, err
 	}
-	if params.MinimumEvaluationInterval > params.MaximumEvaluationInterval {
+	if params.Maximum == 0 {
+		params.Maximum = model.Duration(1<<63 - 1)
+	}
+	if params.Minimum > params.Maximum {
 		return nil, fmt.Errorf("minimum is greater than maximum")
 	}
-	if params.MaximumEvaluationInterval == 0 && params.MinimumEvaluationInterval == 0 {
+	if params.Maximum == 0 && params.Minimum == 0 {
 		return nil, fmt.Errorf("at least one of the `minimum` or `maximum` must be set")
 	}
-	if params.MaximumEvaluationInterval == 0 {
-		params.MaximumEvaluationInterval = time.Duration(1<<63 - 1)
-
-	}
-	return &hasAllowedEvaluationInterval{minimum: params.MinimumEvaluationInterval, maximum: params.MaximumEvaluationInterval, mustBeSet: params.MustBeSet}, nil
+	return &hasAllowedEvaluationInterval{minimum: params.Minimum, maximum: params.Maximum, mustBeSet: params.MustBeSet}, nil
 }
 
 type hasAllowedEvaluationInterval struct {
-	minimum   time.Duration
-	maximum   time.Duration
+	minimum   model.Duration
+	maximum   model.Duration
 	mustBeSet bool
 }
 
@@ -88,10 +87,10 @@ func (h hasAllowedEvaluationInterval) Validate(group unmarshaler.RuleGroup, _ ru
 		}
 		return []error{}
 	}
-	if h.minimum != 0 && time.Duration(group.Interval) < h.minimum {
+	if h.minimum != 0 && group.Interval < h.minimum {
 		return []error{fmt.Errorf("evaluation interval %s is less than `%s`", group.Interval, h.minimum)}
 	}
-	if h.maximum != 0 && time.Duration(group.Interval) > h.maximum {
+	if h.maximum != 0 && group.Interval > h.maximum {
 		return []error{fmt.Errorf("evaluation interval %s is greater than `%s`", group.Interval, h.maximum)}
 	}
 	return []error{}

diff --git a/pkg/validator/validator_test.go b/pkg/validator/validator_test.go
@@ -211,11 +211,11 @@ var testCases = []struct {
 	{name: "allowedSourceTenantsAndGroupSourceTenantsWithMultipleTenantsAndOneIsMissing", validator: hasAllowedSourceTenants{allowedSourceTenants: []string{"tenant1", "tenant2"}}, group: unmarshaler.RuleGroup{SourceTenants: []string{"tenant1", "tenant3"}}, rule: rulefmt.Rule{Expr: `up{foo="bar"}`}, expectedErrors: 1},
 
 	// hasAllowedEvaluationInterval
-	{name: "validInterval", validator: hasAllowedEvaluationInterval{minimum: time.Second, maximum: time.Minute, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Minute)}, expectedErrors: 0},
-	{name: "unsetRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: time.Second, maximum: time.Minute, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 1},
-	{name: "unsetNotRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: time.Second, maximum: time.Minute, mustBeSet: false}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 0},
-	{name: "tooShortInterval", validator: hasAllowedEvaluationInterval{minimum: time.Minute, maximum: time.Hour, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Second)}, expectedErrors: 1},
-	{name: "tooHighInterval", validator: hasAllowedEvaluationInterval{minimum: time.Minute, maximum: time.Hour, mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Hour * 2)}, expectedErrors: 1},
+	{name: "validInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Second), maximum: model.Duration(time.Minute), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Minute)}, expectedErrors: 0},
+	{name: "unsetRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Second), maximum: model.Duration(time.Minute), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 1},
+	{name: "unsetNotRequiredInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Second), maximum: model.Duration(time.Minute), mustBeSet: false}, group: unmarshaler.RuleGroup{Interval: 0}, expectedErrors: 0},
+	{name: "tooShortInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Minute), maximum: model.Duration(time.Hour), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Second)}, expectedErrors: 1},
+	{name: "tooHighInterval", validator: hasAllowedEvaluationInterval{minimum: model.Duration(time.Minute), maximum: model.Duration(time.Hour), mustBeSet: true}, group: unmarshaler.RuleGroup{Interval: model.Duration(time.Hour * 2)}, expectedErrors: 1},
 
 	// hasValidPartialStrategy
 	{name: "validPartialStrategy", validator: hasValidPartialStrategy{}, group: unmarshaler.RuleGroup{PartialResponseStrategy: "warn"}, expectedErrors: 0},