upstream/prometheus
1
0
Fork 0
mirror of https://github.com/prometheus/prometheus.git synced 2026-04-21 22:28:51 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

rulefmt: warn when multi-document rule files are detected (#18114)

Browse source 
Signed-off-by: Divyansh Mishra <divyanshmishra@Divyanshs-MacBook-Air-3.local>
Co-authored-by: Divyansh Mishra <mishraa-G@users.noreply.github.com>
Co-authored-by: Divyansh Mishra <divyanshmishra@Divyanshs-MacBook-Air-3.local>
This commit is contained in:
mishraa-G 2026-04-09 18:10:18 +05:30 committed by GitHub
parent 06e0b5af5b
commit a6ffcaa352
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 122 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
cmd/prometheus/main.go
View file

@ -712,7 +712,7 @@ func main() {
}
// Parse rule files to verify they exist and contain valid rules.
if err := rules.ParseFiles(cfgFile.RuleFiles, cfgFile.GlobalConfig.MetricNameValidationScheme, promqlParser); err != nil {
if err := rules.ParseFiles(cfgFile.RuleFiles, cfgFile.GlobalConfig.MetricNameValidationScheme, promqlParser, logger); err != nil {
absPath, pathErr := filepath.Abs(cfg.configFile)
if pathErr != nil {
absPath = cfg.configFile

20
cmd/promtool/main.go
View file

@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"math"
"net/http"
"net/url"
@ -64,6 +65,7 @@ import (
var (
promqlEnableDelayedNameRemoval = false
promtoolParserOpts parser.Options
logger = promslog.New(&promslog.Config{})
)
func init() {
@ -620,7 +622,7 @@ func CheckConfig(agentMode, checkSyntaxOnly bool, lintSettings configLintConfig,
if !checkSyntaxOnly {
scrapeConfigsFailed := lintScrapeConfigs(scrapeConfigs, lintSettings)
failed = failed || scrapeConfigsFailed
rulesFailed, rulesHaveErrors := checkRules(ruleFiles, lintSettings.rulesLintConfig, p)
rulesFailed, rulesHaveErrors := checkRules(ruleFiles, lintSettings.rulesLintConfig, p, logger)
failed = failed || rulesFailed
hasErrors = hasErrors || rulesHaveErrors
}
@ -664,7 +666,7 @@ func checkFileExists(fn string) error {
func checkConfig(agentMode bool, filename string, checkSyntaxOnly bool) ([]string, []*config.ScrapeConfig, error) {
fmt.Println("Checking", filename)
cfg, err := config.LoadFile(filename, agentMode, promslog.NewNopLogger())
cfg, err := config.LoadFile(filename, agentMode, logger)
if err != nil {
return nil, nil, err
}
@ -851,9 +853,9 @@ func CheckRules(ls rulesLintConfig, p parser.Parser, files ...string) int {
failed := false
hasErrors := false
if len(files) == 0 {
failed, hasErrors = checkRulesFromStdin(ls, p)
failed, hasErrors = checkRulesFromStdin(ls, p, logger)
} else {
failed, hasErrors = checkRules(files, ls, p)
failed, hasErrors = checkRules(files, ls, p, logger)
}
if failed && hasErrors {
@ -867,7 +869,7 @@ func CheckRules(ls rulesLintConfig, p parser.Parser, files ...string) int {
}
// checkRulesFromStdin validates rule from stdin.
func checkRulesFromStdin(ls rulesLintConfig, p parser.Parser) (bool, bool) {
func checkRulesFromStdin(ls rulesLintConfig, p parser.Parser, logger *slog.Logger) (bool, bool) {
failed := false
hasErrors := false
fmt.Println("Checking standard input")
@ -876,7 +878,7 @@ func checkRulesFromStdin(ls rulesLintConfig, p parser.Parser) (bool, bool) {
fmt.Fprintln(os.Stderr, " FAILED:", err)
return true, true
}
rgs, errs := rulefmt.Parse(data, ls.ignoreUnknownFields, ls.nameValidationScheme, p)
rgs, errs := rulefmt.Parse(data, ls.ignoreUnknownFields, ls.nameValidationScheme, p, logger)
if errs != nil {
failed = true
fmt.Fprintln(os.Stderr, " FAILED:")
@ -905,12 +907,12 @@ func checkRulesFromStdin(ls rulesLintConfig, p parser.Parser) (bool, bool) {
}
// checkRules validates rule files.
func checkRules(files []string, ls rulesLintConfig, p parser.Parser) (bool, bool) {
func checkRules(files []string, ls rulesLintConfig, p parser.Parser, logger *slog.Logger) (bool, bool) {
failed := false
hasErrors := false
for _, f := range files {
fmt.Println("Checking", f)
rgs, errs := rulefmt.ParseFile(f, ls.ignoreUnknownFields, ls.nameValidationScheme, p)
rgs, errs := rulefmt.ParseFile(f, ls.ignoreUnknownFields, ls.nameValidationScheme, p, logger)
if errs != nil {
failed = true
fmt.Fprintln(os.Stderr, " FAILED:")
@ -1307,7 +1309,7 @@ func importRules(url *url.URL, roundTripper http.RoundTripper, start, end, outpu
return fmt.Errorf("new api client error: %w", err)
}
ruleImporter := newRuleImporter(promslog.New(&promslog.Config{}), cfg, api)
ruleImporter := newRuleImporter(logger, cfg, api)
errs := ruleImporter.loadGroups(ctx, files)
for _, err := range errs {
if err != nil {

5
cmd/promtool/main_test.go
View file

@ -33,6 +33,7 @@ import (
"time"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/require"
"github.com/prometheus/prometheus/model/labels"
@ -204,7 +205,7 @@ func TestCheckDuplicates(t *testing.T) {
c := test
t.Run(c.name, func(t *testing.T) {
t.Parallel()
rgs, err := rulefmt.ParseFile(c.ruleFile, false, model.UTF8Validation, parser.NewParser(parser.Options{}))
rgs, err := rulefmt.ParseFile(c.ruleFile, false, model.UTF8Validation, parser.NewParser(parser.Options{}), promslog.NewNopLogger())
require.Empty(t, err)
dups := checkDuplicates(rgs.Groups)
require.Equal(t, c.expectedDups, dups)
@ -213,7 +214,7 @@ func TestCheckDuplicates(t *testing.T) {
}
func BenchmarkCheckDuplicates(b *testing.B) {
rgs, err := rulefmt.ParseFile("./testdata/rules_large.yml", false, model.UTF8Validation, parser.NewParser(parser.Options{}))
rgs, err := rulefmt.ParseFile("./testdata/rules_large.yml", false, model.UTF8Validation, parser.NewParser(parser.Options{}), promslog.NewNopLogger())
require.Empty(b, err)
for b.Loop() {

26
model/rulefmt/rulefmt.go
View file

@ -19,6 +19,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"os"
"strings"
"time"
@ -339,7 +340,13 @@ func testTemplateParsing(rl *Rule) (errs []error) {
}
// Parse parses and validates a set of rules.
func Parse(content []byte, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme, p parser.Parser) (*RuleGroups, []error) {
func Parse(
content []byte,
ignoreUnknownFields bool,
nameValidationScheme model.ValidationScheme,
p parser.Parser,
logger *slog.Logger,
) (*RuleGroups, []error) {
var (
groups RuleGroups
node ruleGroups
@ -355,6 +362,13 @@ func Parse(content []byte, ignoreUnknownFields bool, nameValidationScheme model.
if err != nil && !errors.Is(err, io.EOF) {
errs = append(errs, err)
}
// Check for a second document.
var secondDoc any
err = decoder.Decode(&secondDoc)
if !errors.Is(err, io.EOF) {
logger.Warn("Multiple document yaml rules files are not supported, only the first document is processed")
}
err = yaml.Unmarshal(content, &node)
if err != nil {
errs = append(errs, err)
@ -368,12 +382,18 @@ func Parse(content []byte, ignoreUnknownFields bool, nameValidationScheme model.
}
// ParseFile reads and parses rules from a file.
func ParseFile(file string, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme, p parser.Parser) (*RuleGroups, []error) {
func ParseFile(
file string,
ignoreUnknownFields bool,
nameValidationScheme model.ValidationScheme,
p parser.Parser,
logger *slog.Logger,
) (*RuleGroups, []error) {
b, err := os.ReadFile(file)
if err != nil {
return nil, []error{fmt.Errorf("%s: %w", file, err)}
}
rgs, errs := Parse(b, ignoreUnknownFields, nameValidationScheme, p)
rgs, errs := Parse(b, ignoreUnknownFields, nameValidationScheme, p, logger)
for i := range errs {
errs[i] = fmt.Errorf("%s: %w", file, errs[i])
}

80
model/rulefmt/rulefmt_test.go
View file

@ -14,29 +14,35 @@
package rulefmt
import (
"bytes"
"errors"
"io"
"log/slog"
"path/filepath"
"testing"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/require"
"go.yaml.in/yaml/v3"
"github.com/prometheus/prometheus/promql/parser"
)
var testParser = parser.NewParser(parser.Options{})
var (
testParser = parser.NewParser(parser.Options{})
testLogger = promslog.NewNopLogger()
)
func TestParseFileSuccess(t *testing.T) {
_, errs := ParseFile("testdata/test.yaml", false, model.UTF8Validation, testParser)
_, errs := ParseFile("testdata/test.yaml", false, model.UTF8Validation, testParser, testLogger)
require.Empty(t, errs, "unexpected errors parsing file")
_, errs = ParseFile("testdata/utf-8_lname.good.yaml", false, model.UTF8Validation, testParser)
_, errs = ParseFile("testdata/utf-8_lname.good.yaml", false, model.UTF8Validation, testParser, testLogger)
require.Empty(t, errs, "unexpected errors parsing file")
_, errs = ParseFile("testdata/utf-8_annotation.good.yaml", false, model.UTF8Validation, testParser)
_, errs = ParseFile("testdata/utf-8_annotation.good.yaml", false, model.UTF8Validation, testParser, testLogger)
require.Empty(t, errs, "unexpected errors parsing file")
_, errs = ParseFile("testdata/legacy_validation_annotation.good.yaml", false, model.LegacyValidation, testParser)
_, errs = ParseFile("testdata/legacy_validation_annotation.good.yaml", false, model.LegacyValidation, testParser, testLogger)
require.Empty(t, errs, "unexpected errors parsing file")
}
@ -45,7 +51,7 @@ func TestParseFileSuccessWithAliases(t *testing.T) {
/
sum without(instance) (rate(requests_total[5m]))
`
rgs, errs := ParseFile("testdata/test_aliases.yaml", false, model.UTF8Validation, testParser)
rgs, errs := ParseFile("testdata/test_aliases.yaml", false, model.UTF8Validation, testParser, testLogger)
require.Empty(t, errs, "unexpected errors parsing file")
for _, rg := range rgs.Groups {
require.Equal(t, "HighAlert", rg.Rules[0].Alert)
@ -123,7 +129,7 @@ func TestParseFileFailure(t *testing.T) {
if c.nameValidationScheme == model.UnsetValidation {
c.nameValidationScheme = model.UTF8Validation
}
_, errs := ParseFile(filepath.Join("testdata", c.filename), false, c.nameValidationScheme, testParser)
_, errs := ParseFile(filepath.Join("testdata", c.filename), false, c.nameValidationScheme, testParser, testLogger)
require.NotEmpty(t, errs, "Expected error parsing %s but got none", c.filename)
require.ErrorContainsf(t, errs[0], c.errMsg, "Expected error for %s.", c.filename)
})
@ -219,7 +225,7 @@ groups:
}
for _, tst := range tests {
rgs, errs := Parse([]byte(tst.ruleString), false, model.UTF8Validation, testParser)
rgs, errs := Parse([]byte(tst.ruleString), false, model.UTF8Validation, testParser, testLogger)
require.NotNil(t, rgs, "Rule parsing, rule=\n"+tst.ruleString)
passed := (tst.shouldPass && len(errs) == 0) || (!tst.shouldPass && len(errs) > 0)
require.True(t, passed, "Rule validation failed, rule=\n"+tst.ruleString)
@ -246,7 +252,7 @@ groups:
annotations:
summary: "Instance {{ $labels.instance }} up"
`
_, errs := Parse([]byte(group), false, model.UTF8Validation, testParser)
_, errs := Parse([]byte(group), false, model.UTF8Validation, testParser, testLogger)
require.Len(t, errs, 2, "Expected two errors")
var err00 *Error
require.ErrorAs(t, errs[0], &err00)
@ -391,3 +397,59 @@ func TestErrorUnwrap(t *testing.T) {
})
}
}
func TestMultiDocParse(t *testing.T) {
const (
valid = `
groups:
- name: example
rules:
- alert: InstanceDown
expr: up == 0
`
multi = `
groups:
- name: example
rules:
- alert: InstanceDown
expr: up == 0
---
groups:
- name: example2
rules:
- alert: InstanceDown2
expr: up == 0
`
multiEmpty = `
groups:
- name: example
rules:
- alert: InstanceDown
expr: up == 0
---
`
)
var buf bytes.Buffer
bufLogger := slog.New(slog.NewTextHandler(&buf, nil))
rgs, errs := Parse([]byte(valid), false, model.UTF8Validation, testParser, bufLogger)
require.Empty(t, errs)
require.NotNil(t, rgs)
require.Len(t, rgs.Groups, 1)
require.NotContains(t, buf.String(), "Multiple document yaml rules files are not supported")
buf.Reset()
rgs, errs = Parse([]byte(multi), false, model.UTF8Validation, testParser, bufLogger)
require.Empty(t, errs)
require.NotNil(t, rgs)
require.Len(t, rgs.Groups, 1)
require.Contains(t, buf.String(), "Multiple document yaml rules files are not supported")
buf.Reset()
rgs, errs = Parse([]byte(multiEmpty), false, model.UTF8Validation, testParser, bufLogger)
require.Empty(t, errs)
require.NotNil(t, rgs)
require.Len(t, rgs.Groups, 1)
require.Contains(t, buf.String(), "Multiple document yaml rules files are not supported")
}

14
rules/manager.go
View file

@ -166,7 +166,7 @@ func NewManager(o *ManagerOptions) *Manager {
}
if o.GroupLoader == nil {
o.GroupLoader = FileLoader{parser: o.Parser}
o.GroupLoader = FileLoader{parser: o.Parser, logger: o.Logger}
}
if o.RuleConcurrencyController == nil {
@ -330,10 +330,11 @@ type GroupLoader interface {
// for loading and uses the configured Parser for expression parsing.
type FileLoader struct {
parser parser.Parser
logger *slog.Logger
}
func (fl FileLoader) Load(identifier string, ignoreUnknownFields bool, nameValidationScheme model.ValidationScheme) (*rulefmt.RuleGroups, []error) {
return rulefmt.ParseFile(identifier, ignoreUnknownFields, nameValidationScheme, fl.parser)
return rulefmt.ParseFile(identifier, ignoreUnknownFields, nameValidationScheme, fl.parser, fl.logger)
}
func (fl FileLoader) Parse(query string) (parser.Expr, error) {
@ -617,7 +618,12 @@ func FromMaps(maps ...map[string]string) labels.Labels {
}
// ParseFiles parses the rule files corresponding to glob patterns.
func ParseFiles(patterns []string, nameValidationScheme model.ValidationScheme, p parser.Parser) error {
func ParseFiles(
patterns []string,
nameValidationScheme model.ValidationScheme,
p parser.Parser,
logger *slog.Logger,
) error {
files := map[string]string{}
for _, pat := range patterns {
fns, err := filepath.Glob(pat)
@ -637,7 +643,7 @@ func ParseFiles(patterns []string, nameValidationScheme model.ValidationScheme,
}
}
for fn, pat := range files {
_, errs := rulefmt.ParseFile(fn, false, nameValidationScheme, p)
_, errs := rulefmt.ParseFile(fn, false, nameValidationScheme, p, logger)
if len(errs) > 0 {
return fmt.Errorf("parse rules from file %q (pattern: %q): %w", fn, pat, errors.Join(errs...))
}

6
rules/manager_test.go
View file

@ -809,7 +809,7 @@ func TestUpdate(t *testing.T) {
}
// Groups will be recreated if updated.
rgs, errs := rulefmt.ParseFile("fixtures/rules.yaml", false, model.UTF8Validation, testParser)
rgs, errs := rulefmt.ParseFile("fixtures/rules.yaml", false, model.UTF8Validation, testParser, promslog.NewNopLogger())
require.Empty(t, errs, "file parsing failures")
tmpFile, err := os.CreateTemp("", "rules.test.*.yaml")
@ -2594,11 +2594,11 @@ func TestLabels_FromMaps(t *testing.T) {
func TestParseFiles(t *testing.T) {
t.Run("good files", func(t *testing.T) {
err := ParseFiles([]string{filepath.Join("fixtures", "rules.y*ml")}, model.UTF8Validation, testParser)
err := ParseFiles([]string{filepath.Join("fixtures", "rules.y*ml")}, model.UTF8Validation, testParser, promslog.NewNopLogger())
require.NoError(t, err)
})
t.Run("bad files", func(t *testing.T) {
err := ParseFiles([]string{filepath.Join("fixtures", "invalid_rules.y*ml")}, model.UTF8Validation, testParser)
err := ParseFiles([]string{filepath.Join("fixtures", "invalid_rules.y*ml")}, model.UTF8Validation, testParser, promslog.NewNopLogger())
require.ErrorContains(t, err, "field unexpected_field not found in type rulefmt.Rule")
})
}