diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index 864799cf54..705a5cb0fb 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -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 diff --git a/cmd/promtool/main.go b/cmd/promtool/main.go index 6b897ba1ae..fbea5eda6a 100644 --- a/cmd/promtool/main.go +++ b/cmd/promtool/main.go @@ -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 { diff --git a/cmd/promtool/main_test.go b/cmd/promtool/main_test.go index bbc52355a9..20e3cf4e7b 100644 --- a/cmd/promtool/main_test.go +++ b/cmd/promtool/main_test.go @@ -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() { diff --git a/model/rulefmt/rulefmt.go b/model/rulefmt/rulefmt.go index d284a14c40..68d34aec2f 100644 --- a/model/rulefmt/rulefmt.go +++ b/model/rulefmt/rulefmt.go @@ -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]) } diff --git a/model/rulefmt/rulefmt_test.go b/model/rulefmt/rulefmt_test.go index 071711319c..33c9ac94ad 100644 --- a/model/rulefmt/rulefmt_test.go +++ b/model/rulefmt/rulefmt_test.go @@ -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") +} diff --git a/rules/manager.go b/rules/manager.go index 5548359ce6..2ac62d7e49 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -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...)) } diff --git a/rules/manager_test.go b/rules/manager_test.go index f790dd1c85..f0dfcf26f4 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -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") }) }