fix: reintroduce strict unmarshalling of rule files (#71)

* fix: strict unmarshalling Signed-off-by: Martin Chodur <m.chodur@seznam.cz> * docs: add comments Signed-off-by: Martin Chodur <m.chodur@seznam.cz> * fix: lint Signed-off-by: Martin Chodur <m.chodur@seznam.cz> --------- Signed-off-by: Martin Chodur <m.chodur@seznam.cz>
FUSAKLA · Jul 12, 2024 · 9af9c56 · 9af9c56
1 parent f9d04c2
commit 9af9c56
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Updated: Prometheus and other dependencies
 - CI: Updated Github actions for golangcilint and goreleaser
+- Fixed: :warning: Unmarshalling of the rule files is strict again, this behavior was unintentionally brought when adding support for yaml comments.
+- Added: support for alert field `keep_firing_for`
+- Added: support for the `query_offset` field in the rule group
 
 ## [2.14.1]
 - Fixed: error message in the `hasSourceTenantsForMetrics` validator

diff --git a/pkg/unmarshaler/helpers.go b/pkg/unmarshaler/helpers.go
@@ -1,6 +1,8 @@
 package unmarshaler
 
 import (
+	"fmt"
+	"slices"
 	"strings"
 
 	"gopkg.in/yaml.v3"
@@ -49,7 +51,20 @@ func disabledValidatorsFromComments(comments []string, commentPrefix string) []s
 	return disabledValidators
 }
 
-func unmarshalToNodeAndStruct(value, dstNode *yaml.Node, dstStruct interface{}) error {
+func unmarshalToNodeAndStruct(value, dstNode *yaml.Node, dstStruct interface{}, knownFields []string) error {
+	// Since yaml/v3 Node.Decode doesn't support setting decode options like KnownFields (see https://github.com/go-yaml/yaml/issues/460)
+	// we need to check the fields manually, thus the function requires a list of known fields.
+	if value.Kind == yaml.MappingNode {
+		m := map[string]any{}
+		if err := value.Decode(m); err != nil {
+			return err
+		}
+		for k := range m {
+			if !slices.Contains(knownFields, k) {
+				return fmt.Errorf("unknown field %q when unmarshalling the %T, only supported fields are: %s", k, dstStruct, strings.Join(knownFields, ","))
+			}
+		}
+	}
 	err := value.Decode(dstNode)
 	if err != nil {
 		return err
@@ -60,3 +75,22 @@ func unmarshalToNodeAndStruct(value, dstNode *yaml.Node, dstStruct interface{})
 	}
 	return nil
 }
+
+// mustListStructYamlFieldNames returns a list of yaml field names for the given struct.
+func mustListStructYamlFieldNames(s interface{}) []string {
+	y, err := yaml.Marshal(s)
+	if err != nil {
+		fmt.Println("failed to marshal", err)
+		panic(err)
+	}
+	m := map[string]any{}
+	if err := yaml.Unmarshal(y, m); err != nil {
+		fmt.Println("failed to marshal", err)
+		panic(err)
+	}
+	names := make([]string, 0, len(m))
+	for k := range m {
+		names = append(names, k)
+	}
+	return names
+}
diff --git a/pkg/unmarshaler/unmarshaler.go b/pkg/unmarshaler/unmarshaler.go
@@ -9,16 +9,21 @@ import (
 	"gopkg.in/yaml.v3"
 )
 
-type fakeTestFile struct {
-	RuleFiles          []yaml.Node `yaml:"rule_files,omitempty"`
-	EvaluationInterval yaml.Node   `yaml:"evaluation_interval,omitempty"`
-	GroupEvalOrder     []yaml.Node `yaml:"group_eval_order,omitempty"`
-	Tests              []yaml.Node `yaml:"tests,omitempty"`
-}
+var (
+	// Struct fields marked as omitempty MUST be set to non-default value so they appear in marshalled yaml.
+	rulesFileKnownFields         = mustListStructYamlFieldNames(RulesFile{})
+	groupsWithCommentKnownFields = mustListStructYamlFieldNames(GroupsWithComment{})
+	ruleGroupKnownFields         = mustListStructYamlFieldNames(RuleGroup{})
+	ruleNodeKnownFields          = mustListStructYamlFieldNames(rulefmt.RuleNode{Record: yaml.Node{Kind: yaml.SequenceNode}, Alert: yaml.Node{Kind: yaml.SequenceNode}, For: model.Duration(1), Labels: map[string]string{"foo": "bar"}, Annotations: map[string]string{"foo": "bar"}, KeepFiringFor: model.Duration(1)})
+)
 
 type RulesFile struct {
-	Groups       GroupsWithComment `yaml:"groups"`
-	fakeTestFile                   // Just so we can unmarshal also PromQL test files but ignore them because it has no Groups
+	Groups GroupsWithComment `yaml:"groups"`
+	// Just so we can unmarshal also PromQL test files but ignore them because it has no Groups
+	RuleFiles          interface{} `yaml:"rule_files"`
+	EvaluationInterval interface{} `yaml:"evaluation_interval"`
+	GroupEvalOrder     interface{} `yaml:"group_eval_order"`
+	Tests              interface{} `yaml:"tests"`
 }
 
 type RulesFileWithComment struct {
@@ -33,7 +38,7 @@ func (r *RulesFileWithComment) UnmarshalYAML(value *yaml.Node) error {
 			r.groupsComments = strings.Split(field.HeadComment, "\n")
 		}
 	}
-	return unmarshalToNodeAndStruct(value, &r.node, &r.RulesFile)
+	return unmarshalToNodeAndStruct(value, &r.node, &r.RulesFile, rulesFileKnownFields)
 }
 
 func (r *RulesFileWithComment) DisabledValidators(commentPrefix string) []string {
@@ -42,11 +47,11 @@ func (r *RulesFileWithComment) DisabledValidators(commentPrefix string) []string
 
 type GroupsWithComment struct {
 	node   yaml.Node
-	Groups []RuleGroupWithComment
+	Groups []RuleGroupWithComment `yaml:"groups"`
 }
 
 func (g *GroupsWithComment) UnmarshalYAML(value *yaml.Node) error {
-	return unmarshalToNodeAndStruct(value, &g.node, &g.Groups)
+	return unmarshalToNodeAndStruct(value, &g.node, &g.Groups, groupsWithCommentKnownFields)
 }
 
 func (g *GroupsWithComment) DisabledValidators(commentPrefix string) []string {
@@ -55,11 +60,12 @@ func (g *GroupsWithComment) DisabledValidators(commentPrefix string) []string {
 
 type RuleGroup struct {
 	Name                    string            `yaml:"name"`
-	Interval                model.Duration    `yaml:"interval,omitempty"`
-	PartialResponseStrategy string            `yaml:"partial_response_strategy,omitempty"`
-	SourceTenants           []string          `yaml:"source_tenants,omitempty"`
+	Interval                model.Duration    `yaml:"interval"`
+	QueryOffset             model.Duration    `yaml:"query_offset"`
+	PartialResponseStrategy string            `yaml:"partial_response_strategy"` // Thanos only
+	SourceTenants           []string          `yaml:"source_tenants"`            // Cortex/Mimir only
 	Rules                   []RuleWithComment `yaml:"rules"`
-	Limit                   int               `yaml:"limit,omitempty"`
+	Limit                   int               `yaml:"limit"`
 }
 
 type RuleGroupWithComment struct {
@@ -68,7 +74,7 @@ type RuleGroupWithComment struct {
 }
 
 func (r *RuleGroupWithComment) UnmarshalYAML(value *yaml.Node) error {
-	return unmarshalToNodeAndStruct(value, &r.node, &r.RuleGroup)
+	return unmarshalToNodeAndStruct(value, &r.node, &r.RuleGroup, ruleGroupKnownFields)
 }
 
 func (r *RuleGroupWithComment) DisabledValidators(commentPrefix string) []string {
@@ -82,17 +88,18 @@ type RuleWithComment struct {
 
 func (r *RuleWithComment) OriginalRule() rulefmt.Rule {
 	return rulefmt.Rule{
-		Record:      r.rule.Record.Value,
-		Alert:       r.rule.Alert.Value,
-		Expr:        r.rule.Expr.Value,
-		For:         r.rule.For,
-		Labels:      r.rule.Labels,
-		Annotations: r.rule.Annotations,
+		Record:        r.rule.Record.Value,
+		Alert:         r.rule.Alert.Value,
+		Expr:          r.rule.Expr.Value,
+		For:           r.rule.For,
+		Labels:        r.rule.Labels,
+		Annotations:   r.rule.Annotations,
+		KeepFiringFor: r.rule.KeepFiringFor,
 	}
 }
 
 func (r *RuleWithComment) UnmarshalYAML(value *yaml.Node) error {
-	return unmarshalToNodeAndStruct(value, &r.node, &r.rule)
+	return unmarshalToNodeAndStruct(value, &r.node, &r.rule, ruleNodeKnownFields)
 }
 
 func (r *RuleWithComment) DisabledValidators(commentPrefix string) []string {