From ef8124ab8188f29b192f00d079b52684582854dc Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Sun, 24 May 2026 21:56:32 +0300 Subject: [PATCH 01/15] linter to check rules and dashboards for source=deckhouse label --- internal/manager/manager.go | 55 ++++ internal/module/module.go | 12 + pkg/config.go | 8 + pkg/config/global/global.go | 1 + pkg/config/linters_settings.go | 7 + pkg/linters/templates/rules/source_label.go | 299 ++++++++++++++++++ .../templates/rules/source_label_test.go | 127 ++++++++ pkg/linters/templates/templates.go | 3 + 8 files changed, 512 insertions(+) create mode 100644 pkg/linters/templates/rules/source_label.go create mode 100644 pkg/linters/templates/rules/source_label_test.go diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 62121769..4427b3e4 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -153,7 +153,10 @@ func (m *Manager) Run() { wg := new(sync.WaitGroup) processingCh := make(chan struct{}, flags.LintersLimit) + recordNames := collectRecordingRuleNames(m.Modules) + for _, module := range m.Modules { + module.SetRecordingRuleNames(recordNames) processingCh <- struct{}{} wg.Add(1) @@ -195,6 +198,58 @@ func getLintersForModule(cfg *pkg.LintersSettings, errList *errors.LintRuleError } } +func collectRecordingRuleNames(modules []*module.Module) map[string]struct{} { + names := make(map[string]struct{}) + for _, m := range modules { + for _, obj := range m.GetStorage() { + if obj.Unstructured.GetKind() != "PrometheusRule" { + continue + } + ispec, ok := obj.Unstructured.Object["spec"] + if !ok { + continue + } + spec, ok := ispec.(map[string]any) + if !ok { + continue + } + igroups, ok := spec["groups"] + if !ok { + continue + } + groups, ok := igroups.([]any) + if !ok { + continue + } + for _, ig := range groups { + group, ok := ig.(map[string]any) + if !ok { + continue + } + irules, ok := group["rules"] + if !ok { + continue + } + rules, ok := irules.([]any) + if !ok { + continue + } + for _, ir := range rules { + rule, ok := ir.(map[string]any) + if !ok { + continue + } + record, ok := rule["record"].(string) + if ok && record != "" { + names[record] = struct{}{} + } + } + } + } + } + return names +} + func (m *Manager) PrintResult() { errs := m.errors.GetErrors() diff --git a/internal/module/module.go b/internal/module/module.go index 188dd86e..d8a93571 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -146,6 +146,15 @@ func (m *Module) GetModuleConfig() *pkg.LintersSettings { return m.linterConfig } +func (m *Module) SetRecordingRuleNames(names map[string]struct{}) { + if m == nil || m.linterConfig == nil { + return + } + + m.linterConfig.Templates.SourceLabelSettings.RecordingRuleNames = names +} + + // remapLinterSettings converts configuration settings from the config package format // to the pkg package format, mapping both rule-level configurations and exclusion rules // across all linter domains (Container, Image, NoCyrillic, OpenAPI, Templates, RBAC, Hooks, Module). @@ -342,6 +351,7 @@ func mapTemplatesRules(linterSettings *pkg.LintersSettings, configSettings *conf rules.ClusterDomainRule.SetLevel(globalRules.ClusterDomainRule.Impact, fallbackImpact) rules.RegistryRule.SetLevel(globalRules.RegistryRule.Impact, fallbackImpact) rules.EnabledModulesRule.SetLevel(globalRules.EnabledModulesRule.Impact, fallbackImpact) + rules.SourceLabelRule.SetLevel(globalRules.SourceLabelRule.Impact, fallbackImpact) } // mapOpenAPIRules configures OpenAPI linter rules @@ -459,6 +469,8 @@ func mapTemplatesExclusionsAndSettings(linterSettings *pkg.LintersSettings, conf // Additional settings linterSettings.Templates.PrometheusRuleSettings.Disable = configSettings.Templates.PrometheusRules.Disable linterSettings.Templates.GrafanaDashboardsSettings.Disable = configSettings.Templates.GrafanaDashboards.Disable + linterSettings.Templates.SourceLabelSettings.Disable = configSettings.Templates.SourceLabel.Disable + linterSettings.Templates.SourceLabelSettings.AllowedMetrics = configSettings.Templates.SourceLabel.AllowedMetrics } // mapRBACExclusions maps RBAC linter exclusion rules diff --git a/pkg/config.go b/pkg/config.go index ef89490d..50aa42f1 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -129,6 +129,13 @@ type TemplatesLinterConfig struct { ExcludeRules TemplatesExcludeRules PrometheusRuleSettings PrometheusRuleSettings GrafanaDashboardsSettings GrafanaDashboardsSettings + SourceLabelSettings SourceLabelSettings +} + +type SourceLabelSettings struct { + Disable bool + AllowedMetrics []string + RecordingRuleNames map[string]struct{} } type TemplatesLinterRules struct { VPARule RuleConfig @@ -141,6 +148,7 @@ type TemplatesLinterRules struct { ClusterDomainRule RuleConfig RegistryRule RuleConfig EnabledModulesRule RuleConfig + SourceLabelRule RuleConfig } type PrometheusRuleSettings struct { diff --git a/pkg/config/global/global.go b/pkg/config/global/global.go index d6c969a9..a9fda869 100644 --- a/pkg/config/global/global.go +++ b/pkg/config/global/global.go @@ -139,6 +139,7 @@ type TemplatesLinterRules struct { ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` RegistryRule RuleConfig `mapstructure:"registry"` EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + SourceLabelRule RuleConfig `mapstructure:"source-label"` } func (c LinterConfig) IsWarn() bool { diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index bd60f17d..a06f2fe6 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -189,11 +189,17 @@ type TemplatesSettings struct { ExcludeRules TemplatesExcludeRules `mapstructure:"exclude-rules"` GrafanaDashboards GrafanaDashboardsExcludeList `mapstructure:"grafana-dashboards"` PrometheusRules PrometheusRulesExcludeList `mapstructure:"prometheus-rules"` + SourceLabel SourceLabelSettings `mapstructure:"source-label"` Rules TemplatesLinterRules `mapstructure:"rules"` Impact string `mapstructure:"impact"` } +type SourceLabelSettings struct { + Disable bool `mapstructure:"disable"` + AllowedMetrics []string `mapstructure:"allowed-metrics"` +} + type TemplatesLinterRules struct { VPARule RuleConfig `mapstructure:"vpa"` PDBRule RuleConfig `mapstructure:"pdb"` @@ -205,6 +211,7 @@ type TemplatesLinterRules struct { ClusterDomainRule RuleConfig `mapstructure:"cluster-domain"` RegistryRule RuleConfig `mapstructure:"registry"` EnabledModulesRule RuleConfig `mapstructure:"enabled-modules"` + SourceLabelRule RuleConfig `mapstructure:"source-label"` } type TemplatesExcludeRules struct { diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go new file mode 100644 index 00000000..8ba8f946 --- /dev/null +++ b/pkg/linters/templates/rules/source_label.go @@ -0,0 +1,299 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/promql/parser" + "github.com/tidwall/gjson" + "k8s.io/utils/ptr" + "sigs.k8s.io/yaml" + + "github.com/deckhouse/dmt/internal/fsutils" + "github.com/deckhouse/dmt/internal/storage" + "github.com/deckhouse/dmt/pkg" + "github.com/deckhouse/dmt/pkg/errors" +) + +const ( + SourceLabelRuleName = "source-label" +) + +type SourceLabelRule struct { + pkg.RuleMeta + pkg.BoolRule + recordingRuleNames map[string]struct{} + allowedMetrics map[string]struct{} +} + +func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { + var exclude bool + allowedMetrics := make(map[string]struct{}) + recordNames := make(map[string]struct{}) + if cfg != nil { + exclude = cfg.SourceLabelSettings.Disable + for _, m := range cfg.SourceLabelSettings.AllowedMetrics { + allowedMetrics[m] = struct{}{} + } + if cfg.SourceLabelSettings.RecordingRuleNames != nil { + recordNames = cfg.SourceLabelSettings.RecordingRuleNames + } + } + + return &SourceLabelRule{ + RuleMeta: pkg.RuleMeta{ + Name: SourceLabelRuleName, + }, + BoolRule: pkg.BoolRule{ + Exclude: exclude, + }, + recordingRuleNames: recordNames, + allowedMetrics: allowedMetrics, + } +} + +func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObject, errorList *errors.LintRuleErrorsList) { + if !r.Enabled() { + errorList = errorList.WithMaxLevel(ptr.To(pkg.Ignored)) + } + + errorList = errorList.WithFilePath(m.GetPath()).WithRule(r.GetName()) + + if object.Unstructured.GetKind() != "PrometheusRule" { + return + } + + ispec, ok := object.Unstructured.Object["spec"] + if !ok { + return + } + spec, ok := ispec.(map[string]any) + if !ok { + return + } + + specBytes, err := yaml.Marshal(spec) + if err != nil { + return + } + + type rule struct { + Record string `yaml:"record,omitempty"` + Alert string `yaml:"alert,omitempty"` + Expr string `yaml:"expr"` + } + type ruleGroup struct { + Name string `yaml:"name"` + Rules []rule `yaml:"rules"` + } + type ruleGroups struct { + Groups []ruleGroup `yaml:"groups"` + } + + var groups ruleGroups + if err := yaml.Unmarshal(specBytes, &groups); err != nil { + return + } + + for _, group := range groups.Groups { + for _, rl := range group.Rules { + if rl.Expr == "" { + continue + } + ruleName := rl.Alert + if ruleName == "" { + ruleName = rl.Record + } + r.checkExpr(rl.Expr, ruleName, group.Name, object.GetPath(), errorList) + } + } +} + +func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, errorList *errors.LintRuleErrorsList) { + ast, err := parser.ParseExpr(expr) + if err != nil { + return + } + + parser.Inspect(ast, func(node parser.Node, _ []parser.Node) error { + vs, ok := node.(*parser.VectorSelector) + if !ok { + return nil + } + + metricName := vs.Name + if metricName == "" { + for _, m := range vs.LabelMatchers { + if m.Name == labels.MetricName && m.Type == labels.MatchEqual { + metricName = m.Value + break + } + } + } + + if metricName == "" { + return nil + } + + if _, ok := r.recordingRuleNames[metricName]; ok { + return nil + } + if _, ok := r.allowedMetrics[metricName]; ok { + return nil + } + + hasSourceLabel := false + for _, m := range vs.LabelMatchers { + if m.Name == "source" && m.Type == labels.MatchEqual && m.Value == "deckhouse" { + hasSourceLabel = true + break + } + } + + if !hasSourceLabel { + errorList.WithFilePath(filePath). + Errorf("metric '%s' in rule '%s' (group '%s') must have source=\"deckhouse\" selector", + metricName, ruleName, groupName) + } + + return nil + }) +} + +var ( + grafanaBuiltinVarRe = regexp.MustCompile(`\$__\w+`) + grafanaVarBracesRe = regexp.MustCompile(`\$\{(\w+)(?::[^}]*)?\}`) + grafanaVarSimpleRe = regexp.MustCompile(`\$([a-zA-Z_]\w*)`) +) + +func sanitizeGrafanaExpr(expr string) string { + result := grafanaBuiltinVarRe.ReplaceAllString(expr, "5m") + result = grafanaVarBracesRe.ReplaceAllString(result, "__placeholder__") + result = grafanaVarSimpleRe.ReplaceAllStringFunc(result, func(match string) string { + name := match[1:] + if name == "source" { + return match + } + return "__placeholder__" + }) + return result +} + +func (r *SourceLabelRule) SourceLabelCheckDashboards(m pkg.Module, errorList *errors.LintRuleErrorsList) { + if !r.Enabled() { + errorList = errorList.WithMaxLevel(ptr.To(pkg.Ignored)) + } + + errorList = errorList.WithRule(r.GetName()) + + searchPath := filepath.Join(m.GetPath(), "monitoring", "grafana-dashboards") + entries := fsutils.GetFiles(searchPath, true, fsutils.FilterFileByExtensions(".json", ".tpl")) + + for _, entry := range entries { + r.checkDashboardFile(entry, errorList) + } +} + +func (r *SourceLabelRule) checkDashboardFile(filePath string, errorList *errors.LintRuleErrorsList) { + content, err := os.ReadFile(filePath) + if err != nil { + errorList.WithFilePath(filePath).Errorf("failed to read dashboard file: %s", err) + return + } + + dashboard := gjson.ParseBytes(content) + if !dashboard.IsObject() { + return + } + + panels := r.extractDashboardPanels(&dashboard) + for _, panel := range panels { + r.checkPanel(&panel, filePath, errorList) + } + + r.checkTemplateVariables(&dashboard, filePath, errorList) +} + +func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, errorList *errors.LintRuleErrorsList) { + panelTitle := panel.Get("title").String() + if panelTitle == "" { + panelTitle = "unnamed" + } + + targets := panel.Get("targets").Array() + for _, target := range targets { + expr := target.Get("expr").String() + if expr == "" { + continue + } + sanitized := sanitizeGrafanaExpr(expr) + r.checkExpr(sanitized, fmt.Sprintf("panel '%s'", panelTitle), "dashboard", filePath, errorList) + } +} + +func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePath string, errorList *errors.LintRuleErrorsList) { + templating := dashboard.Get("templating.list") + if !templating.Exists() || !templating.IsArray() { + return + } + + for _, tmpl := range templating.Array() { + if tmpl.Get("type").String() != "query" { + continue + } + + query := tmpl.Get("definition").String() + if query == "" { + query = tmpl.Get("query").String() + } + if query == "" { + continue + } + + sanitized := sanitizeGrafanaExpr(query) + tmplName := tmpl.Get("name").String() + r.checkExpr(sanitized, fmt.Sprintf("template variable '%s'", tmplName), "dashboard", filePath, errorList) + } +} + +func (r *SourceLabelRule) extractDashboardPanels(dashboard *gjson.Result) []gjson.Result { + panels := make([]gjson.Result, 0) + + rows := dashboard.Get("rows").Array() + for _, row := range rows { + rowPanels := row.Get("panels").Array() + panels = append(panels, rowPanels...) + } + + directPanels := dashboard.Get("panels").Array() + for _, panel := range directPanels { + panelType := panel.Get("type").String() + if panelType == "row" { + rowPanels := panel.Get("panels").Array() + panels = append(panels, rowPanels...) + } else { + panels = append(panels, panel) + } + } + + return panels +} diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go new file mode 100644 index 00000000..d971f997 --- /dev/null +++ b/pkg/linters/templates/rules/source_label_test.go @@ -0,0 +1,127 @@ +/* +Copyright 2025 Flant JSC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rules + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/deckhouse/dmt/pkg/errors" +) + +func TestCheckExprWithSourceLabel(t *testing.T) { + tests := []struct { + name string + expr string + recordNames map[string]struct{} + allowedMetrics map[string]struct{} + expectedErrors int + }{ + { + name: "metric with source=deckhouse has no errors", + expr: `kube_pod_info{source="deckhouse"}`, + expectedErrors: 0, + }, + { + name: "metric without source selector produces error", + expr: `kube_pod_info`, + expectedErrors: 1, + }, + { + name: "recording rule name is allowed without source", + expr: `my_recording_rule`, + recordNames: map[string]struct{}{"my_recording_rule": {}}, + expectedErrors: 0, + }, + { + name: "binary expr - one metric with source, one without", + expr: `a{source="deckhouse"} * on() b`, + expectedErrors: 1, + }, + { + name: "allowed metric does not produce error", + expr: `allowed_metric`, + allowedMetrics: map[string]struct{}{"allowed_metric": {}}, + expectedErrors: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + recordNames := tt.recordNames + if recordNames == nil { + recordNames = make(map[string]struct{}) + } + allowedMetrics := tt.allowedMetrics + if allowedMetrics == nil { + allowedMetrics = make(map[string]struct{}) + } + + rule := &SourceLabelRule{ + recordingRuleNames: recordNames, + allowedMetrics: allowedMetrics, + } + + errorList := errors.NewLintRuleErrorsList() + rule.checkExpr(tt.expr, "test-rule", "test-group", "/test/file.yaml", errorList) + + assert.Len(t, errorList.GetErrors(), tt.expectedErrors) + }) + } +} + +func TestSanitizeGrafanaExpr(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "replaces $__rate_interval with 5m", + input: `rate(m{source="deckhouse"}[$__rate_interval])`, + expected: `rate(m{source="deckhouse"}[5m])`, + }, + { + name: "replaces $namespace with __placeholder__", + input: `m{namespace="$namespace"}`, + expected: `m{namespace="__placeholder__"}`, + }, + { + name: "replaces ${var:regex} with __placeholder__", + input: `m{var="${var:regex}"}`, + expected: `m{var="__placeholder__"}`, + }, + { + name: "does not replace $source", + input: `m{source="$source"}`, + expected: `m{source="$source"}`, + }, + { + name: "replaces $__range with 5m", + input: `increase(m[$__range])`, + expected: `increase(m[5m])`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := sanitizeGrafanaExpr(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/linters/templates/templates.go b/pkg/linters/templates/templates.go index 16c2b4e2..6d96c842 100644 --- a/pkg/linters/templates/templates.go +++ b/pkg/linters/templates/templates.go @@ -63,10 +63,12 @@ func (l *Templates) Run(m *module.Module) { // monitoring prometheusRule := rules.NewPrometheusRule(l.cfg) grafanaRule := rules.NewGrafanaRule(l.cfg) + sourceLabelRule := rules.NewSourceLabelRule(l.cfg) if err := dirExists(m.GetPath(), "monitoring"); err == nil { grafanaRule.ValidateGrafanaDashboards(m, errorList.WithMaxLevel(l.cfg.Rules.GrafanaRule.GetLevel())) prometheusRule.ValidatePrometheusRules(m, errorList.WithMaxLevel(l.cfg.Rules.PrometheusRule.GetLevel())) + sourceLabelRule.SourceLabelCheckDashboards(m, errorList.WithMaxLevel(l.cfg.Rules.SourceLabelRule.GetLevel())) } else if !os.IsNotExist(err) { errorList.Errorf("reading the 'monitoring' folder failed: %s", err) } @@ -80,6 +82,7 @@ func (l *Templates) Run(m *module.Module) { servicePortRule.ObjectServiceTargetPort(object, errorList.WithMaxLevel(l.cfg.Rules.ServicePortRule.GetLevel())) prometheusRule.PromtoolRuleCheck(m, object, errorList.WithMaxLevel(l.cfg.Rules.PrometheusRule.GetLevel())) ingressRule.CheckSnippetsRule(object, errorList.WithMaxLevel(l.cfg.Rules.IngressRule.GetLevel())) + sourceLabelRule.SourceLabelCheck(m, object, errorList.WithMaxLevel(l.cfg.Rules.SourceLabelRule.GetLevel())) } // Cluster domain rule From 4a0ee5a08e2fb6b1ccc5449a2fe1e34e0d5ed183 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Sun, 24 May 2026 22:07:10 +0300 Subject: [PATCH 02/15] linter fix --- internal/manager/manager.go | 12 ++++++++++++ internal/module/module.go | 1 - pkg/linters/templates/rules/source_label.go | 14 ++++++++++++++ pkg/linters/templates/rules/source_label_test.go | 7 ++++--- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/internal/manager/manager.go b/internal/manager/manager.go index 4427b3e4..ccb27af5 100644 --- a/internal/manager/manager.go +++ b/internal/manager/manager.go @@ -157,6 +157,7 @@ func (m *Manager) Run() { for _, module := range m.Modules { module.SetRecordingRuleNames(recordNames) + processingCh <- struct{}{} wg.Add(1) @@ -200,45 +201,55 @@ func getLintersForModule(cfg *pkg.LintersSettings, errList *errors.LintRuleError func collectRecordingRuleNames(modules []*module.Module) map[string]struct{} { names := make(map[string]struct{}) + for _, m := range modules { for _, obj := range m.GetStorage() { if obj.Unstructured.GetKind() != "PrometheusRule" { continue } + ispec, ok := obj.Unstructured.Object["spec"] if !ok { continue } + spec, ok := ispec.(map[string]any) if !ok { continue } + igroups, ok := spec["groups"] if !ok { continue } + groups, ok := igroups.([]any) if !ok { continue } + for _, ig := range groups { group, ok := ig.(map[string]any) if !ok { continue } + irules, ok := group["rules"] if !ok { continue } + rules, ok := irules.([]any) if !ok { continue } + for _, ir := range rules { rule, ok := ir.(map[string]any) if !ok { continue } + record, ok := rule["record"].(string) if ok && record != "" { names[record] = struct{}{} @@ -247,6 +258,7 @@ func collectRecordingRuleNames(modules []*module.Module) map[string]struct{} { } } } + return names } diff --git a/internal/module/module.go b/internal/module/module.go index d8a93571..42776328 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -154,7 +154,6 @@ func (m *Module) SetRecordingRuleNames(names map[string]struct{}) { m.linterConfig.Templates.SourceLabelSettings.RecordingRuleNames = names } - // remapLinterSettings converts configuration settings from the config package format // to the pkg package format, mapping both rule-level configurations and exclusion rules // across all linter domains (Container, Image, NoCyrillic, OpenAPI, Templates, RBAC, Hooks, Module). diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 8ba8f946..4195f639 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -47,13 +47,16 @@ type SourceLabelRule struct { func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { var exclude bool + allowedMetrics := make(map[string]struct{}) recordNames := make(map[string]struct{}) + if cfg != nil { exclude = cfg.SourceLabelSettings.Disable for _, m := range cfg.SourceLabelSettings.AllowedMetrics { allowedMetrics[m] = struct{}{} } + if cfg.SourceLabelSettings.RecordingRuleNames != nil { recordNames = cfg.SourceLabelSettings.RecordingRuleNames } @@ -86,6 +89,7 @@ func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObj if !ok { return } + spec, ok := ispec.(map[string]any) if !ok { return @@ -101,10 +105,12 @@ func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObj Alert string `yaml:"alert,omitempty"` Expr string `yaml:"expr"` } + type ruleGroup struct { Name string `yaml:"name"` Rules []rule `yaml:"rules"` } + type ruleGroups struct { Groups []ruleGroup `yaml:"groups"` } @@ -119,10 +125,12 @@ func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObj if rl.Expr == "" { continue } + ruleName := rl.Alert if ruleName == "" { ruleName = rl.Record } + r.checkExpr(rl.Expr, ruleName, group.Name, object.GetPath(), errorList) } } @@ -157,11 +165,13 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, if _, ok := r.recordingRuleNames[metricName]; ok { return nil } + if _, ok := r.allowedMetrics[metricName]; ok { return nil } hasSourceLabel := false + for _, m := range vs.LabelMatchers { if m.Name == "source" && m.Type == labels.MatchEqual && m.Value == "deckhouse" { hasSourceLabel = true @@ -193,8 +203,10 @@ func sanitizeGrafanaExpr(expr string) string { if name == "source" { return match } + return "__placeholder__" }) + return result } @@ -245,6 +257,7 @@ func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, error if expr == "" { continue } + sanitized := sanitizeGrafanaExpr(expr) r.checkExpr(sanitized, fmt.Sprintf("panel '%s'", panelTitle), "dashboard", filePath, errorList) } @@ -265,6 +278,7 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa if query == "" { query = tmpl.Get("query").String() } + if query == "" { continue } diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index d971f997..7ed7a02b 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -43,9 +43,9 @@ func TestCheckExprWithSourceLabel(t *testing.T) { expectedErrors: 1, }, { - name: "recording rule name is allowed without source", - expr: `my_recording_rule`, - recordNames: map[string]struct{}{"my_recording_rule": {}}, + name: "recording rule name is allowed without source", + expr: `my_recording_rule`, + recordNames: map[string]struct{}{"my_recording_rule": {}}, expectedErrors: 0, }, { @@ -67,6 +67,7 @@ func TestCheckExprWithSourceLabel(t *testing.T) { if recordNames == nil { recordNames = make(map[string]struct{}) } + allowedMetrics := tt.allowedMetrics if allowedMetrics == nil { allowedMetrics = make(map[string]struct{}) From 34b14136f671615a7950acded0c4eca4ebbc686f Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Mon, 25 May 2026 12:32:09 +0300 Subject: [PATCH 03/15] fix prometheus metrics --- pkg/linters/templates/rules/source_label.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 4195f639..68792d66 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -38,6 +38,16 @@ const ( SourceLabelRuleName = "source-label" ) +// prometheusSyntheticMetrics contains Prometheus built-in metrics that are generated +// internally by the engine and never receive scrape-time labels like source="deckhouse". +// This set is intentionally separate from allowedMetrics (per-module config) and +// recordingRuleNames (computed at runtime) because these metrics are universally +// synthetic regardless of module or deployment. +var prometheusSyntheticMetrics = map[string]struct{}{ + "ALERTS": {}, + "ALERTS_FOR_STATE": {}, +} + type SourceLabelRule struct { pkg.RuleMeta pkg.BoolRule @@ -170,6 +180,10 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, return nil } + if _, ok := prometheusSyntheticMetrics[metricName]; ok { + return nil + } + hasSourceLabel := false for _, m := range vs.LabelMatchers { From 45e1d998021afd154f6c8dc7e74f342fbc251d2e Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Mon, 25 May 2026 13:28:04 +0300 Subject: [PATCH 04/15] add regex support --- pkg/linters/templates/rules/source_label.go | 45 ++++++- .../templates/rules/source_label_test.go | 110 ++++++++++++++++-- 2 files changed, 144 insertions(+), 11 deletions(-) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 68792d66..248a3308 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -21,6 +21,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/promql/parser" @@ -52,19 +53,45 @@ type SourceLabelRule struct { pkg.RuleMeta pkg.BoolRule recordingRuleNames map[string]struct{} - allowedMetrics map[string]struct{} + allowedMetrics []*regexp.Regexp +} + +// globToRegexp converts a simple glob pattern (supporting * and ?) to a regexp. +// Plain strings without wildcards are compiled as ^exact_name$, behaving like exact match. +func globToRegexp(pattern string) (*regexp.Regexp, error) { + var b strings.Builder + b.WriteString("^") + + for _, ch := range pattern { + switch ch { + case '*': + b.WriteString(".*") + case '?': + b.WriteString(".") + default: + b.WriteString(regexp.QuoteMeta(string(ch))) + } + } + + b.WriteString("$") + + return regexp.Compile(b.String()) } func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { var exclude bool - allowedMetrics := make(map[string]struct{}) + var allowedMetrics []*regexp.Regexp + recordNames := make(map[string]struct{}) if cfg != nil { exclude = cfg.SourceLabelSettings.Disable + for _, m := range cfg.SourceLabelSettings.AllowedMetrics { - allowedMetrics[m] = struct{}{} + if re, err := globToRegexp(m); err == nil { + allowedMetrics = append(allowedMetrics, re) + } } if cfg.SourceLabelSettings.RecordingRuleNames != nil { @@ -176,7 +203,7 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, return nil } - if _, ok := r.allowedMetrics[metricName]; ok { + if r.isAllowedMetric(metricName) { return nil } @@ -203,6 +230,16 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, }) } +func (r *SourceLabelRule) isAllowedMetric(metricName string) bool { + for _, re := range r.allowedMetrics { + if re.MatchString(metricName) { + return true + } + } + + return false +} + var ( grafanaBuiltinVarRe = regexp.MustCompile(`\$__\w+`) grafanaVarBracesRe = regexp.MustCompile(`\$\{(\w+)(?::[^}]*)?\}`) diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index 7ed7a02b..6c6c9f7d 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -17,6 +17,7 @@ limitations under the License. package rules import ( + "regexp" "testing" "github.com/stretchr/testify/assert" @@ -29,7 +30,7 @@ func TestCheckExprWithSourceLabel(t *testing.T) { name string expr string recordNames map[string]struct{} - allowedMetrics map[string]struct{} + allowedMetrics []string expectedErrors int }{ { @@ -54,11 +55,55 @@ func TestCheckExprWithSourceLabel(t *testing.T) { expectedErrors: 1, }, { - name: "allowed metric does not produce error", + name: "exact allowed metric does not produce error", expr: `allowed_metric`, - allowedMetrics: map[string]struct{}{"allowed_metric": {}}, + allowedMetrics: []string{"allowed_metric"}, expectedErrors: 0, }, + { + name: "glob pattern matches metric prefix", + expr: `coredns_panics_total + coredns_dns_requests_total`, + allowedMetrics: []string{"coredns_*"}, + expectedErrors: 0, + }, + { + name: "glob pattern does not match unrelated metric", + expr: `kube_pod_info`, + allowedMetrics: []string{"coredns_*"}, + expectedErrors: 1, + }, + { + name: "glob pattern with question mark", + expr: `metric_v1_total + metric_v2_total`, + allowedMetrics: []string{"metric_v?_total"}, + expectedErrors: 0, + }, + { + name: "mixed exact and glob", + expr: `exact_metric + coredns_cache_hits_total + unknown_metric`, + allowedMetrics: []string{"exact_metric", "coredns_*"}, + expectedErrors: 1, + }, + { + name: "ALERTS synthetic metric is allowed without source", + expr: `ALERTS{alertname="SomeAlert"}`, + expectedErrors: 0, + }, + { + name: "ALERTS_FOR_STATE synthetic metric is allowed without source", + expr: `ALERTS_FOR_STATE{alertname="SomeAlert"}`, + expectedErrors: 0, + }, + { + name: "ALERTS in complex expr does not produce error", + expr: `ALERTS{alertname="KubeQuotaExceeded"} == 1 and on(namespace) kube_pod_info{source="deckhouse"}`, + expectedErrors: 0, + }, + { + name: "ALERTS without source but regular metric also without source", + expr: `ALERTS{alertname="X"} * on() some_metric`, + expectedErrors: 1, + }, } for _, tt := range tests { @@ -68,14 +113,16 @@ func TestCheckExprWithSourceLabel(t *testing.T) { recordNames = make(map[string]struct{}) } - allowedMetrics := tt.allowedMetrics - if allowedMetrics == nil { - allowedMetrics = make(map[string]struct{}) + var compiled []*regexp.Regexp + for _, m := range tt.allowedMetrics { + re, err := globToRegexp(m) + assert.NoError(t, err) + compiled = append(compiled, re) } rule := &SourceLabelRule{ recordingRuleNames: recordNames, - allowedMetrics: allowedMetrics, + allowedMetrics: compiled, } errorList := errors.NewLintRuleErrorsList() @@ -86,6 +133,55 @@ func TestCheckExprWithSourceLabel(t *testing.T) { } } +func TestGlobToRegexp(t *testing.T) { + tests := []struct { + pattern string + match []string + noMatch []string + }{ + { + pattern: "coredns_*", + match: []string{"coredns_panics_total", "coredns_dns_requests_total", "coredns_"}, + noMatch: []string{"xcoredns_foo", "kube_pod_info", "coredns"}, + }, + { + pattern: "metric_v?_total", + match: []string{"metric_v1_total", "metric_vX_total"}, + noMatch: []string{"metric_v12_total", "metric_v_total"}, + }, + { + pattern: "exact_metric", + match: []string{"exact_metric"}, + noMatch: []string{"exact_metric_extra", "xexact_metric", "exact_metricx"}, + }, + { + pattern: "ingress_nginx_*_responses_*", + match: []string{"ingress_nginx_detail_responses_total", "ingress_nginx_overall_responses_5xx"}, + noMatch: []string{"ingress_nginx_connections", "nginx_responses_total"}, + }, + { + pattern: "*.total", + match: []string{"foo.total", "bar_baz.total"}, + noMatch: []string{"foo_total", "total"}, + }, + } + + for _, tt := range tests { + t.Run(tt.pattern, func(t *testing.T) { + re, err := globToRegexp(tt.pattern) + assert.NoError(t, err) + + for _, s := range tt.match { + assert.True(t, re.MatchString(s), "expected %q to match pattern %q", s, tt.pattern) + } + + for _, s := range tt.noMatch { + assert.False(t, re.MatchString(s), "expected %q NOT to match pattern %q", s, tt.pattern) + } + }) + } +} + func TestSanitizeGrafanaExpr(t *testing.T) { tests := []struct { name string From df0eb0d3d3d54ef374676645ee620582d4a07831 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Mon, 25 May 2026 13:33:39 +0300 Subject: [PATCH 05/15] linter fix --- pkg/linters/templates/rules/source_label_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index 6c6c9f7d..4e7919f6 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -113,10 +113,12 @@ func TestCheckExprWithSourceLabel(t *testing.T) { recordNames = make(map[string]struct{}) } - var compiled []*regexp.Regexp + compiled := make([]*regexp.Regexp, 0, len(tt.allowedMetrics)) + for _, m := range tt.allowedMetrics { re, err := globToRegexp(m) assert.NoError(t, err) + compiled = append(compiled, re) } From b91fe9f07df705c65799b41e7c22ccf7be279564 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Mon, 25 May 2026 14:21:47 +0300 Subject: [PATCH 06/15] fix --- pkg/linters/templates/rules/source_label.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 248a3308..c8dd55ec 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -296,7 +296,16 @@ func (r *SourceLabelRule) checkDashboardFile(filePath string, errorList *errors. r.checkTemplateVariables(&dashboard, filePath, errorList) } +func isPrometheusDatasource(obj *gjson.Result) bool { + dsType := obj.Get("datasource.type").String() + return dsType == "" || dsType == "prometheus" +} + func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, errorList *errors.LintRuleErrorsList) { + if !isPrometheusDatasource(panel) { + return + } + panelTitle := panel.Get("title").String() if panelTitle == "" { panelTitle = "unnamed" @@ -304,6 +313,10 @@ func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, error targets := panel.Get("targets").Array() for _, target := range targets { + if !isPrometheusDatasource(&target) { + continue + } + expr := target.Get("expr").String() if expr == "" { continue @@ -325,6 +338,10 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa continue } + if !isPrometheusDatasource(&tmpl) { + continue + } + query := tmpl.Get("definition").String() if query == "" { query = tmpl.Get("query").String() From 9193d5962be583edca675617f23f5cd2c92fdf43 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Tue, 26 May 2026 17:37:24 +0300 Subject: [PATCH 07/15] fix --- internal/values/global-openapi/values.yaml | 4 +- pkg/linters/templates/rules/source_label.go | 57 +++++-- .../templates/rules/source_label_test.go | 142 ++++++++++++++++++ 3 files changed, 185 insertions(+), 18 deletions(-) diff --git a/internal/values/global-openapi/values.yaml b/internal/values/global-openapi/values.yaml index c3e810c3..68f7e430 100644 --- a/internal/values/global-openapi/values.yaml +++ b/internal/values/global-openapi/values.yaml @@ -276,10 +276,10 @@ properties: items: type: string x-examples: - - ["cert-manager", "vertical-pod-autoscaler", "vertical-pod-autoscaler-crd", "prometheus", "priority-class", "prometheus-crd", "operator-prometheus", "operator-prometheus"] + - ["cert-manager", "vertical-pod-autoscaler", "vertical-pod-autoscaler-crd", "prometheus", "priority-class", "prometheus-crd", "operator-prometheus", "operator-prometheus-crd"] - ["cert-manager", "prometheus", "priority-class"] x-dmt-default: - ["cert-manager", "vertical-pod-autoscaler", "vertical-pod-autoscaler-crd", "prometheus", "priority-class", "prometheus-crd", "operator-prometheus", "operator-prometheus"] + ["cert-manager", "vertical-pod-autoscaler", "vertical-pod-autoscaler-crd", "prometheus", "priority-class", "prometheus-crd", "operator-prometheus", "operator-prometheus-crd"] discovery: additionalProperties: true type: object diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index c8dd55ec..b2356837 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -195,7 +195,7 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, } } - if metricName == "" { + if metricName == "" || metricName == "__placeholder__" { return nil } @@ -214,7 +214,8 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, hasSourceLabel := false for _, m := range vs.LabelMatchers { - if m.Name == "source" && m.Type == labels.MatchEqual && m.Value == "deckhouse" { + if m.Name == "source" && m.Type == labels.MatchEqual && + (m.Value == "deckhouse" || strings.HasPrefix(m.Value, "$")) { hasSourceLabel = true break } @@ -248,7 +249,14 @@ var ( func sanitizeGrafanaExpr(expr string) string { result := grafanaBuiltinVarRe.ReplaceAllString(expr, "5m") - result = grafanaVarBracesRe.ReplaceAllString(result, "__placeholder__") + result = grafanaVarBracesRe.ReplaceAllStringFunc(result, func(match string) string { + sub := grafanaVarBracesRe.FindStringSubmatch(match) + if len(sub) > 1 && sub[1] == "source" { + return match + } + + return "__placeholder__" + }) result = grafanaVarSimpleRe.ReplaceAllStringFunc(result, func(match string) string { name := match[1:] if name == "source" { @@ -296,13 +304,23 @@ func (r *SourceLabelRule) checkDashboardFile(filePath string, errorList *errors. r.checkTemplateVariables(&dashboard, filePath, errorList) } -func isPrometheusDatasource(obj *gjson.Result) bool { - dsType := obj.Get("datasource.type").String() +func isPrometheusDataSource(obj *gjson.Result) bool { + ds := obj.Get("datasource") + if !ds.Exists() { + return true + } + + if ds.Type == gjson.String { + return false + } + + dsType := ds.Get("type").String() + return dsType == "" || dsType == "prometheus" } func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, errorList *errors.LintRuleErrorsList) { - if !isPrometheusDatasource(panel) { + if !isPrometheusDataSource(panel) { return } @@ -313,7 +331,7 @@ func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, error targets := panel.Get("targets").Array() for _, target := range targets { - if !isPrometheusDatasource(&target) { + if !isPrometheusDataSource(&target) { continue } @@ -338,7 +356,7 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa continue } - if !isPrometheusDatasource(&tmpl) { + if !isPrometheusDataSource(&tmpl) { continue } @@ -358,7 +376,7 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa } func (r *SourceLabelRule) extractDashboardPanels(dashboard *gjson.Result) []gjson.Result { - panels := make([]gjson.Result, 0) + var panels []gjson.Result rows := dashboard.Get("rows").Array() for _, row := range rows { @@ -367,15 +385,22 @@ func (r *SourceLabelRule) extractDashboardPanels(dashboard *gjson.Result) []gjso } directPanels := dashboard.Get("panels").Array() - for _, panel := range directPanels { - panelType := panel.Get("type").String() - if panelType == "row" { - rowPanels := panel.Get("panels").Array() - panels = append(panels, rowPanels...) + panels = append(panels, collectPanelsRecursive(directPanels)...) + + return panels +} + +func collectPanelsRecursive(items []gjson.Result) []gjson.Result { + var result []gjson.Result + + for _, item := range items { + if item.Get("type").String() == "row" { + nested := item.Get("panels").Array() + result = append(result, collectPanelsRecursive(nested)...) } else { - panels = append(panels, panel) + result = append(result, item) } } - return panels + return result } diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index 4e7919f6..f1bad0d0 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/tidwall/gjson" "github.com/deckhouse/dmt/pkg/errors" ) @@ -104,6 +105,26 @@ func TestCheckExprWithSourceLabel(t *testing.T) { expr: `ALERTS{alertname="X"} * on() some_metric`, expectedErrors: 1, }, + { + name: "placeholder metric name is skipped", + expr: `__placeholder__{some="label"}`, + expectedErrors: 0, + }, + { + name: "invalid PromQL is silently skipped", + expr: `label_values(kube_pod_info, namespace)`, + expectedErrors: 0, + }, + { + name: "expression with $source variable is accepted", + expr: `m{source="$source"}`, + expectedErrors: 0, + }, + { + name: "expression with ${source} variable is accepted", + expr: `m{source="${source}"}`, + expectedErrors: 0, + }, } for _, tt := range tests { @@ -184,6 +205,112 @@ func TestGlobToRegexp(t *testing.T) { } } +func TestIsPrometheusDataSource(t *testing.T) { + tests := []struct { + name string + json string + expected bool + }{ + { + name: "no datasource field defaults to prometheus", + json: `{"title": "panel"}`, + expected: true, + }, + { + name: "datasource type prometheus", + json: `{"datasource": {"type": "prometheus", "uid": "$ds"}}`, + expected: true, + }, + { + name: "datasource type empty defaults to prometheus", + json: `{"datasource": {"uid": "$ds"}}`, + expected: true, + }, + { + name: "datasource is plain string", + json: `{"datasource": "Graphite"}`, + expected: false, + }, + { + name: "datasource type loki", + json: `{"datasource": {"type": "loki", "uid": "$ds_loki"}}`, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := gjson.Parse(tt.json) + assert.Equal(t, tt.expected, isPrometheusDataSource(&result)) + }) + } +} + +func TestCollectPanelsRecursive(t *testing.T) { + tests := []struct { + name string + json string + expectedTitles []string + }{ + { + name: "flat panels", + json: `[ + {"type": "graph", "title": "A"}, + {"type": "stat", "title": "B"} + ]`, + expectedTitles: []string{"A", "B"}, + }, + { + name: "one level of row nesting", + json: `[ + {"type": "row", "panels": [ + {"type": "graph", "title": "A"} + ]}, + {"type": "stat", "title": "B"} + ]`, + expectedTitles: []string{"A", "B"}, + }, + { + name: "two levels of row nesting", + json: `[ + {"type": "row", "panels": [ + {"type": "row", "panels": [ + {"type": "graph", "title": "deep"} + ]} + ]} + ]`, + expectedTitles: []string{"deep"}, + }, + { + name: "three levels of row nesting", + json: `[ + {"type": "row", "panels": [ + {"type": "row", "panels": [ + {"type": "row", "panels": [ + {"type": "graph", "title": "very-deep"} + ]} + ]} + ]} + ]`, + expectedTitles: []string{"very-deep"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + items := gjson.Parse(tt.json).Array() + panels := collectPanelsRecursive(items) + + titles := make([]string, 0, len(panels)) + for _, p := range panels { + titles = append(titles, p.Get("title").String()) + } + + assert.Equal(t, tt.expectedTitles, titles) + }) + } +} + func TestSanitizeGrafanaExpr(t *testing.T) { tests := []struct { name string @@ -215,6 +342,21 @@ func TestSanitizeGrafanaExpr(t *testing.T) { input: `increase(m[$__range])`, expected: `increase(m[5m])`, }, + { + name: "does not replace ${source}", + input: `m{source="${source}"}`, + expected: `m{source="${source}"}`, + }, + { + name: "does not replace ${source:json}", + input: `m{source="${source:json}"}`, + expected: `m{source="${source:json}"}`, + }, + { + name: "replaces ${other_var} with __placeholder__", + input: `m{ns="${namespace}"}`, + expected: `m{ns="__placeholder__"}`, + }, } for _, tt := range tests { From 3986fd47b33adac0647494b71d6ae8c1e056c563 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Tue, 26 May 2026 17:48:55 +0300 Subject: [PATCH 08/15] fix --- pkg/linters/templates/rules/source_label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index b2356837..41f3d42a 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -376,7 +376,7 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa } func (r *SourceLabelRule) extractDashboardPanels(dashboard *gjson.Result) []gjson.Result { - var panels []gjson.Result + panels := make([]gjson.Result, 0) rows := dashboard.Get("rows").Array() for _, row := range rows { From 01e52057e8726ca226b37d5976b8d2c0bbfbdb0a Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Tue, 26 May 2026 17:56:29 +0300 Subject: [PATCH 09/15] fix --- pkg/linters/templates/rules/source_label.go | 2 +- pkg/linters/templates/rules/source_label_test.go | 12 +++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 41f3d42a..5257f4af 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -311,7 +311,7 @@ func isPrometheusDataSource(obj *gjson.Result) bool { } if ds.Type == gjson.String { - return false + return strings.Contains(strings.ToLower(ds.String()), "prometheus") } dsType := ds.Get("type").String() diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index f1bad0d0..d70dd6f1 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -227,10 +227,20 @@ func TestIsPrometheusDataSource(t *testing.T) { expected: true, }, { - name: "datasource is plain string", + name: "datasource is plain string Graphite", json: `{"datasource": "Graphite"}`, expected: false, }, + { + name: "datasource is plain string with prometheus in name", + json: `{"datasource": "$ds_prometheus"}`, + expected: true, + }, + { + name: "datasource is plain string Prometheus capitalized", + json: `{"datasource": "Prometheus"}`, + expected: true, + }, { name: "datasource type loki", json: `{"datasource": {"type": "loki", "uid": "$ds_loki"}}`, From fe868bd9d606bd4085c210eb2280dc4c49f48d69 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Tue, 26 May 2026 17:59:56 +0300 Subject: [PATCH 10/15] fix linter --- pkg/linters/templates/rules/source_label.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 5257f4af..1ed79941 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -376,22 +376,22 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa } func (r *SourceLabelRule) extractDashboardPanels(dashboard *gjson.Result) []gjson.Result { - panels := make([]gjson.Result, 0) - rows := dashboard.Get("rows").Array() + directPanels := dashboard.Get("panels").Array() + panels := make([]gjson.Result, 0, len(rows)+len(directPanels)) + for _, row := range rows { rowPanels := row.Get("panels").Array() panels = append(panels, rowPanels...) } - directPanels := dashboard.Get("panels").Array() panels = append(panels, collectPanelsRecursive(directPanels)...) return panels } func collectPanelsRecursive(items []gjson.Result) []gjson.Result { - var result []gjson.Result + result := make([]gjson.Result, 0, len(items)) for _, item := range items { if item.Get("type").String() == "row" { From 3645ef7e11a5f8dd8fc8e05ecc1efa4f2584a28a Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Tue, 26 May 2026 18:14:37 +0300 Subject: [PATCH 11/15] fix --- pkg/linters/templates/rules/source_label.go | 2 +- pkg/linters/templates/rules/source_label_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 1ed79941..88e5949c 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -195,7 +195,7 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, } } - if metricName == "" || metricName == "__placeholder__" { + if metricName == "" || strings.Contains(metricName, "__placeholder__") { return nil } diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index d70dd6f1..a379e0ed 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -110,6 +110,11 @@ func TestCheckExprWithSourceLabel(t *testing.T) { expr: `__placeholder__{some="label"}`, expectedErrors: 0, }, + { + name: "placeholder embedded in metric name is skipped", + expr: `__placeholder___requests_total{some="label"}`, + expectedErrors: 0, + }, { name: "invalid PromQL is silently skipped", expr: `label_values(kube_pod_info, namespace)`, From 8688ce575a471404871460b8062d299d800e0f9a Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Tue, 26 May 2026 18:54:16 +0300 Subject: [PATCH 12/15] add tests --- .../templates/rules/source_label_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index a379e0ed..9ab38e51 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -115,6 +115,26 @@ func TestCheckExprWithSourceLabel(t *testing.T) { expr: `__placeholder___requests_total{some="label"}`, expectedErrors: 0, }, + { + name: "selector without metric name is skipped", + expr: `{job="foo"}`, + expectedErrors: 0, + }, + { + name: "exact __name__ matcher requires source label", + expr: `{__name__="kube_pod_info"}`, + expectedErrors: 1, + }, + { + name: "exact __name__ matcher with source is ok", + expr: `{__name__="kube_pod_info", source="deckhouse"}`, + expectedErrors: 0, + }, + { + name: "regex __name__ matcher is skipped", + expr: `{__name__=~"kube_.*"}`, + expectedErrors: 0, + }, { name: "invalid PromQL is silently skipped", expr: `label_values(kube_pod_info, namespace)`, From 48ee91878195e529170f334319e3b7f13431b1fd Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Thu, 11 Jun 2026 12:16:28 +0300 Subject: [PATCH 13/15] fix --- internal/module/module.go | 1 - pkg/config.go | 1 - pkg/config/linters_settings.go | 1 - pkg/linters/templates/rules/source_label.go | 17 ----------------- 4 files changed, 20 deletions(-) diff --git a/internal/module/module.go b/internal/module/module.go index 42776328..83aabc08 100644 --- a/internal/module/module.go +++ b/internal/module/module.go @@ -468,7 +468,6 @@ func mapTemplatesExclusionsAndSettings(linterSettings *pkg.LintersSettings, conf // Additional settings linterSettings.Templates.PrometheusRuleSettings.Disable = configSettings.Templates.PrometheusRules.Disable linterSettings.Templates.GrafanaDashboardsSettings.Disable = configSettings.Templates.GrafanaDashboards.Disable - linterSettings.Templates.SourceLabelSettings.Disable = configSettings.Templates.SourceLabel.Disable linterSettings.Templates.SourceLabelSettings.AllowedMetrics = configSettings.Templates.SourceLabel.AllowedMetrics } diff --git a/pkg/config.go b/pkg/config.go index 50aa42f1..b59d6389 100644 --- a/pkg/config.go +++ b/pkg/config.go @@ -133,7 +133,6 @@ type TemplatesLinterConfig struct { } type SourceLabelSettings struct { - Disable bool AllowedMetrics []string RecordingRuleNames map[string]struct{} } diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index a06f2fe6..dfa1792f 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -196,7 +196,6 @@ type TemplatesSettings struct { } type SourceLabelSettings struct { - Disable bool `mapstructure:"disable"` AllowedMetrics []string `mapstructure:"allowed-metrics"` } diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 88e5949c..eee43f10 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -26,7 +26,6 @@ import ( "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/promql/parser" "github.com/tidwall/gjson" - "k8s.io/utils/ptr" "sigs.k8s.io/yaml" "github.com/deckhouse/dmt/internal/fsutils" @@ -51,7 +50,6 @@ var prometheusSyntheticMetrics = map[string]struct{}{ type SourceLabelRule struct { pkg.RuleMeta - pkg.BoolRule recordingRuleNames map[string]struct{} allowedMetrics []*regexp.Regexp } @@ -79,15 +77,11 @@ func globToRegexp(pattern string) (*regexp.Regexp, error) { } func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { - var exclude bool - var allowedMetrics []*regexp.Regexp recordNames := make(map[string]struct{}) if cfg != nil { - exclude = cfg.SourceLabelSettings.Disable - for _, m := range cfg.SourceLabelSettings.AllowedMetrics { if re, err := globToRegexp(m); err == nil { allowedMetrics = append(allowedMetrics, re) @@ -103,19 +97,12 @@ func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { RuleMeta: pkg.RuleMeta{ Name: SourceLabelRuleName, }, - BoolRule: pkg.BoolRule{ - Exclude: exclude, - }, recordingRuleNames: recordNames, allowedMetrics: allowedMetrics, } } func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObject, errorList *errors.LintRuleErrorsList) { - if !r.Enabled() { - errorList = errorList.WithMaxLevel(ptr.To(pkg.Ignored)) - } - errorList = errorList.WithFilePath(m.GetPath()).WithRule(r.GetName()) if object.Unstructured.GetKind() != "PrometheusRule" { @@ -270,10 +257,6 @@ func sanitizeGrafanaExpr(expr string) string { } func (r *SourceLabelRule) SourceLabelCheckDashboards(m pkg.Module, errorList *errors.LintRuleErrorsList) { - if !r.Enabled() { - errorList = errorList.WithMaxLevel(ptr.To(pkg.Ignored)) - } - errorList = errorList.WithRule(r.GetName()) searchPath := filepath.Join(m.GetPath(), "monitoring", "grafana-dashboards") From 2f3e30cdf12a6ab653e4ddb705b0e2f591208747 Mon Sep 17 00:00:00 2001 From: Ekaterina Ryzhkova Date: Thu, 11 Jun 2026 12:51:47 +0300 Subject: [PATCH 14/15] e2e --- pkg/linters/templates/rules/source_label.go | 12 +++++----- .../templates/rules/source_label_test.go | 2 +- .../source-label-allowed/expected.yaml | 5 ++++ .../source-label-allowed/module/.dmtlint.yaml | 5 ++++ .../source-label-allowed/module/module.yaml | 2 ++ .../module/openapi/config-values.yaml | 2 ++ .../module/openapi/values.yaml | 4 ++++ .../module/templates/prometheus-rule.yaml | 14 +++++++++++ .../expected.yaml | 5 ++++ .../module/.dmtlint.yaml | 5 ++++ .../module/module.yaml | 2 ++ .../grafana-dashboards/test-dashboard.json | 24 +++++++++++++++++++ .../module/openapi/config-values.yaml | 2 ++ .../module/openapi/values.yaml | 4 ++++ .../expected.yaml | 9 +++++++ .../module/module.yaml | 2 ++ .../grafana-dashboards/test-dashboard.json | 24 +++++++++++++++++++ .../module/openapi/config-values.yaml | 2 ++ .../module/openapi/values.yaml | 4 ++++ .../source-label-missing/expected.yaml | 9 +++++++ .../source-label-missing/module/module.yaml | 2 ++ .../module/openapi/config-values.yaml | 2 ++ .../module/openapi/values.yaml | 4 ++++ .../module/templates/prometheus-rule.yaml | 14 +++++++++++ 24 files changed, 153 insertions(+), 7 deletions(-) create mode 100644 test/e2e/testdata/templates/source-label-allowed/expected.yaml create mode 100644 test/e2e/testdata/templates/source-label-allowed/module/.dmtlint.yaml create mode 100644 test/e2e/testdata/templates/source-label-allowed/module/module.yaml create mode 100644 test/e2e/testdata/templates/source-label-allowed/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/source-label-allowed/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/source-label-allowed/module/templates/prometheus-rule.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-allowed/expected.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-allowed/module/.dmtlint.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-allowed/module/module.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-allowed/module/monitoring/grafana-dashboards/test-dashboard.json create mode 100644 test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-missing/expected.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-missing/module/module.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-missing/module/monitoring/grafana-dashboards/test-dashboard.json create mode 100644 test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/source-label-missing/expected.yaml create mode 100644 test/e2e/testdata/templates/source-label-missing/module/module.yaml create mode 100644 test/e2e/testdata/templates/source-label-missing/module/openapi/config-values.yaml create mode 100644 test/e2e/testdata/templates/source-label-missing/module/openapi/values.yaml create mode 100644 test/e2e/testdata/templates/source-label-missing/module/templates/prometheus-rule.yaml diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index eee43f10..748b4797 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -155,12 +155,12 @@ func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObj ruleName = rl.Record } - r.checkExpr(rl.Expr, ruleName, group.Name, object.GetPath(), errorList) + r.checkExpr(rl.Expr, fmt.Sprintf("rule '%s' (group '%s')", ruleName, group.Name), object.GetPath(), errorList) } } } -func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, errorList *errors.LintRuleErrorsList) { +func (r *SourceLabelRule) checkExpr(expr, context, filePath string, errorList *errors.LintRuleErrorsList) { ast, err := parser.ParseExpr(expr) if err != nil { return @@ -210,8 +210,8 @@ func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, if !hasSourceLabel { errorList.WithFilePath(filePath). - Errorf("metric '%s' in rule '%s' (group '%s') must have source=\"deckhouse\" selector", - metricName, ruleName, groupName) + Errorf("metric '%s' in %s must have source=\"deckhouse\" selector", + metricName, context) } return nil @@ -324,7 +324,7 @@ func (r *SourceLabelRule) checkPanel(panel *gjson.Result, filePath string, error } sanitized := sanitizeGrafanaExpr(expr) - r.checkExpr(sanitized, fmt.Sprintf("panel '%s'", panelTitle), "dashboard", filePath, errorList) + r.checkExpr(sanitized, fmt.Sprintf("panel '%s'", panelTitle), filePath, errorList) } } @@ -354,7 +354,7 @@ func (r *SourceLabelRule) checkTemplateVariables(dashboard *gjson.Result, filePa sanitized := sanitizeGrafanaExpr(query) tmplName := tmpl.Get("name").String() - r.checkExpr(sanitized, fmt.Sprintf("template variable '%s'", tmplName), "dashboard", filePath, errorList) + r.checkExpr(sanitized, fmt.Sprintf("template variable '%s'", tmplName), filePath, errorList) } } diff --git a/pkg/linters/templates/rules/source_label_test.go b/pkg/linters/templates/rules/source_label_test.go index 9ab38e51..7cbcf166 100644 --- a/pkg/linters/templates/rules/source_label_test.go +++ b/pkg/linters/templates/rules/source_label_test.go @@ -174,7 +174,7 @@ func TestCheckExprWithSourceLabel(t *testing.T) { } errorList := errors.NewLintRuleErrorsList() - rule.checkExpr(tt.expr, "test-rule", "test-group", "/test/file.yaml", errorList) + rule.checkExpr(tt.expr, "rule 'test-rule' (group 'test-group')", "/test/file.yaml", errorList) assert.Len(t, errorList.GetErrors(), tt.expectedErrors) }) diff --git a/test/e2e/testdata/templates/source-label-allowed/expected.yaml b/test/e2e/testdata/templates/source-label-allowed/expected.yaml new file mode 100644 index 00000000..17268ce0 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-allowed/expected.yaml @@ -0,0 +1,5 @@ +description: > + A PrometheusRule with a metric listed in allowed-metrics should not produce + source-label errors. +module: module +expect: [] diff --git a/test/e2e/testdata/templates/source-label-allowed/module/.dmtlint.yaml b/test/e2e/testdata/templates/source-label-allowed/module/.dmtlint.yaml new file mode 100644 index 00000000..f83b2aaf --- /dev/null +++ b/test/e2e/testdata/templates/source-label-allowed/module/.dmtlint.yaml @@ -0,0 +1,5 @@ +linters-settings: + templates: + source-label: + allowed-metrics: + - "rabbitmq_*" diff --git a/test/e2e/testdata/templates/source-label-allowed/module/module.yaml b/test/e2e/testdata/templates/source-label-allowed/module/module.yaml new file mode 100644 index 00000000..f6eed6c2 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-allowed/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-source-allowed +namespace: e2e-source-allowed diff --git a/test/e2e/testdata/templates/source-label-allowed/module/openapi/config-values.yaml b/test/e2e/testdata/templates/source-label-allowed/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/source-label-allowed/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-allowed/module/openapi/values.yaml b/test/e2e/testdata/templates/source-label-allowed/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-allowed/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-allowed/module/templates/prometheus-rule.yaml b/test/e2e/testdata/templates/source-label-allowed/module/templates/prometheus-rule.yaml new file mode 100644 index 00000000..4852064a --- /dev/null +++ b/test/e2e/testdata/templates/source-label-allowed/module/templates/prometheus-rule.yaml @@ -0,0 +1,14 @@ +apiVersion: monitoring.coreos.com/v1 +kind: PrometheusRule +metadata: + name: e2e-source-allowed-rules + namespace: e2e-source-allowed +spec: + groups: + - name: e2e.source-allowed + rules: + - alert: TestAllowedMetric + expr: rabbitmq_node_mem_used{job="rabbitmq"} > 1000000 + for: 5m + labels: + severity_level: "5" diff --git a/test/e2e/testdata/templates/source-label-dashboard-allowed/expected.yaml b/test/e2e/testdata/templates/source-label-dashboard-allowed/expected.yaml new file mode 100644 index 00000000..b0ca21a3 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-allowed/expected.yaml @@ -0,0 +1,5 @@ +description: > + A Grafana dashboard with a metric listed in allowed-metrics should not produce + source-label errors. +module: module +expect: [] diff --git a/test/e2e/testdata/templates/source-label-dashboard-allowed/module/.dmtlint.yaml b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/.dmtlint.yaml new file mode 100644 index 00000000..f83b2aaf --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/.dmtlint.yaml @@ -0,0 +1,5 @@ +linters-settings: + templates: + source-label: + allowed-metrics: + - "rabbitmq_*" diff --git a/test/e2e/testdata/templates/source-label-dashboard-allowed/module/module.yaml b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/module.yaml new file mode 100644 index 00000000..adec94e2 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-source-dash-ok +namespace: e2e-source-dash-ok diff --git a/test/e2e/testdata/templates/source-label-dashboard-allowed/module/monitoring/grafana-dashboards/test-dashboard.json b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/monitoring/grafana-dashboards/test-dashboard.json new file mode 100644 index 00000000..2343fb11 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/monitoring/grafana-dashboards/test-dashboard.json @@ -0,0 +1,24 @@ +{ + "title": "E2E Allowed Dashboard", + "panels": [ + { + "type": "graph", + "title": "RabbitMQ Memory", + "targets": [ + { + "expr": "rabbitmq_node_mem_used{job=\"rabbitmq\"}" + } + ], + "datasource": {"type": "prometheus", "uid": "${ds_prometheus}"} + } + ], + "templating": { + "list": [ + { + "name": "ds_prometheus", + "type": "datasource", + "query": "prometheus" + } + ] + } +} diff --git a/test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/config-values.yaml b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/values.yaml b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-allowed/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-dashboard-missing/expected.yaml b/test/e2e/testdata/templates/source-label-dashboard-missing/expected.yaml new file mode 100644 index 00000000..4b173cb5 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-missing/expected.yaml @@ -0,0 +1,9 @@ +description: > + A Grafana dashboard with a metric missing source="deckhouse" selector must be + flagged by the source-label rule. +module: module +expect: + - linter: templates + rule: source-label + level: error + textContains: "metric 'container_cpu_usage_seconds_total' in panel 'CPU Usage' must have source=\"deckhouse\" selector" diff --git a/test/e2e/testdata/templates/source-label-dashboard-missing/module/module.yaml b/test/e2e/testdata/templates/source-label-dashboard-missing/module/module.yaml new file mode 100644 index 00000000..7aaabb0f --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-missing/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-source-dashboard +namespace: e2e-source-dashboard diff --git a/test/e2e/testdata/templates/source-label-dashboard-missing/module/monitoring/grafana-dashboards/test-dashboard.json b/test/e2e/testdata/templates/source-label-dashboard-missing/module/monitoring/grafana-dashboards/test-dashboard.json new file mode 100644 index 00000000..170de4a9 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-missing/module/monitoring/grafana-dashboards/test-dashboard.json @@ -0,0 +1,24 @@ +{ + "title": "E2E Source Label Dashboard", + "panels": [ + { + "type": "graph", + "title": "CPU Usage", + "targets": [ + { + "expr": "rate(container_cpu_usage_seconds_total{namespace=\"default\"}[5m])" + } + ], + "datasource": {"type": "prometheus", "uid": "${ds_prometheus}"} + } + ], + "templating": { + "list": [ + { + "name": "ds_prometheus", + "type": "datasource", + "query": "prometheus" + } + ] + } +} diff --git a/test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/config-values.yaml b/test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/values.yaml b/test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-dashboard-missing/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-missing/expected.yaml b/test/e2e/testdata/templates/source-label-missing/expected.yaml new file mode 100644 index 00000000..9078b95c --- /dev/null +++ b/test/e2e/testdata/templates/source-label-missing/expected.yaml @@ -0,0 +1,9 @@ +description: > + A PrometheusRule with a metric missing source="deckhouse" selector must be + flagged by the source-label rule. +module: module +expect: + - linter: templates + rule: source-label + level: error + textContains: "metric 'up' in rule 'TestAlertMissingSource' (group 'e2e.source-label') must have source=\"deckhouse\" selector" diff --git a/test/e2e/testdata/templates/source-label-missing/module/module.yaml b/test/e2e/testdata/templates/source-label-missing/module/module.yaml new file mode 100644 index 00000000..3045202d --- /dev/null +++ b/test/e2e/testdata/templates/source-label-missing/module/module.yaml @@ -0,0 +1,2 @@ +name: e2e-source-label +namespace: e2e-source-label diff --git a/test/e2e/testdata/templates/source-label-missing/module/openapi/config-values.yaml b/test/e2e/testdata/templates/source-label-missing/module/openapi/config-values.yaml new file mode 100644 index 00000000..03b0d8bf --- /dev/null +++ b/test/e2e/testdata/templates/source-label-missing/module/openapi/config-values.yaml @@ -0,0 +1,2 @@ +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-missing/module/openapi/values.yaml b/test/e2e/testdata/templates/source-label-missing/module/openapi/values.yaml new file mode 100644 index 00000000..47180da5 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-missing/module/openapi/values.yaml @@ -0,0 +1,4 @@ +x-extend: + schema: config-values.yaml +type: object +properties: {} diff --git a/test/e2e/testdata/templates/source-label-missing/module/templates/prometheus-rule.yaml b/test/e2e/testdata/templates/source-label-missing/module/templates/prometheus-rule.yaml new file mode 100644 index 00000000..671581e7 --- /dev/null +++ b/test/e2e/testdata/templates/source-label-missing/module/templates/prometheus-rule.yaml @@ -0,0 +1,14 @@ +apiVersion: monitoring.coreos.com/v1 +kind: PrometheusRule +metadata: + name: e2e-source-label-rules + namespace: e2e-source-label +spec: + groups: + - name: e2e.source-label + rules: + - alert: TestAlertMissingSource + expr: up{job="node-exporter"} == 0 + for: 5m + labels: + severity_level: "5" From e04fd4249192f73248f1ff4cf93ee87a57b78afb Mon Sep 17 00:00:00 2001 From: Pavel Okhlopkov Date: Mon, 15 Jun 2026 15:46:38 +0300 Subject: [PATCH 15/15] add docs Signed-off-by: Pavel Okhlopkov --- pkg/linters/templates/README.md | 87 +++++++++++++++++++++ pkg/linters/templates/rules/source_label.go | 33 ++++++++ 2 files changed, 120 insertions(+) diff --git a/pkg/linters/templates/README.md b/pkg/linters/templates/README.md index f1e4c27e..9be6049e 100644 --- a/pkg/linters/templates/README.md +++ b/pkg/linters/templates/README.md @@ -17,6 +17,7 @@ Proper template validation prevents runtime issues, ensures applications are pro | [ingress-rules](#ingress-rules) | Validates Ingress configuration snippets | ✅ | enabled | | [prometheus-rules](#prometheus-rules) | Validates Prometheus rules with promtool and proper templates | ✅ | enabled | | [grafana-dashboards](#grafana-dashboards) | Validates Grafana dashboard templates | ✅ | enabled | +| [source-label](#source-label) | Requires `source="deckhouse"` selector on metrics in Prometheus rules and Grafana dashboards | ✅ | enabled | | [cluster-domain](#cluster-domain) | Validates cluster domain configuration is dynamic | ❌ | enabled | | [registry](#registry) | Validates registry secret configuration | ❌ | enabled | | [werf](#werf) | Validates image names in `werf.yaml` do not contain underscores | ❌ | enabled | @@ -1284,6 +1285,85 @@ linters-settings: --- +### source-label + +**Purpose:** Ensures every Deckhouse-owned metric referenced in PromQL expressions (in PrometheusRule objects and Grafana dashboards) is selected with an explicit `source="deckhouse"` label matcher, isolating our metrics from foreign metrics that share the same name. + +**Description:** + +The rule parses the PromQL expressions of alerting/recording rules and dashboard panel/template queries, and for every vector selector over a Deckhouse metric it requires a `source="deckhouse"` matcher (or a `source=$...` Grafana/templating variable that resolves to it). + +**Why it matters:** + +User can ran their own exporter that exposed a metric with the **same name** as one of ours. Because both time series shared the metric name, the metric was effectively duplicated, the rule expression returned ambiguous data, and it failed to evaluate. Pinning `source="deckhouse"` separates our metrics from foreign ones with colliding names, so a query only matches our own time series and evaluates reliably. + +**What it checks:** + +1. `PrometheusRule` objects — the `expr` of every alerting and recording rule +2. Grafana dashboards under `monitoring/grafana-dashboards` — `expr` of panel targets and `query`/`definition` of query template variables (only Prometheus datasources) +3. Each Deckhouse metric selector contains `source="deckhouse"` (or `source=$`) + +**Exemptions (no source selector required):** + +- Metrics produced by recording rules within the module (collected automatically at runtime) +- Metrics matching the per-module `allowed-metrics` globs (for intentionally foreign metrics, e.g. third-party exporters) +- Prometheus synthetic metrics: `ALERTS`, `ALERTS_FOR_STATE` + +**Examples:** + +❌ **Incorrect** - Metric without source selector: + +```yaml +# templates/monitoring.yaml -> PrometheusRule +- alert: TestAlertMissingSource + expr: up{job="node-exporter"} == 0 + for: 5m + labels: + severity_level: "5" +``` + +**Error:** +``` +Error: metric 'up' in rule 'TestAlertMissingSource' (group 'e2e.source-label') must have source="deckhouse" selector +``` + +✅ **Correct** - Metric with source selector: + +```yaml +- alert: TestAlertWithSource + expr: up{job="node-exporter", source="deckhouse"} == 0 + for: 5m + labels: + severity_level: "5" +``` + +✅ **Correct** - Grafana panel query using a source variable: + +```json +{ + "title": "Requests", + "targets": [ + { "expr": "rate(my_module_requests_total{source=\"deckhouse\"}[$__rate_interval])" } + ] +} +``` + +**Configuration:** + +The rule does not use `exclude-rules`. Instead, use `allowed-metrics` to list metric names (glob patterns with `*` and `?` are supported) that are allowed to appear without a source selector: + +```yaml +# .dmt.yaml +linters-settings: + templates: + source-label: + allowed-metrics: + - "rabbitmq_*" # All metrics from a third-party exporter + - "node_exporter_build_info" +``` + +--- + ### cluster-domain **Purpose:** Prevents hardcoding of the cluster domain (`cluster.local`) in templates. Ensures cluster domain is configurable to support custom cluster configurations and multi-cluster deployments. @@ -1663,6 +1743,11 @@ linters-settings: prometheus-rules: disable: true + + # Allow specific (foreign) metrics without a source="deckhouse" selector + source-label: + allowed-metrics: + - "rabbitmq_*" ``` ### Per-Rule Impact Levels @@ -1694,6 +1779,8 @@ linters-settings: impact: error enabled-modules: impact: warning + source-label: + impact: error ``` ### Rule-Level Exclusions diff --git a/pkg/linters/templates/rules/source_label.go b/pkg/linters/templates/rules/source_label.go index 748b4797..237709f7 100644 --- a/pkg/linters/templates/rules/source_label.go +++ b/pkg/linters/templates/rules/source_label.go @@ -48,6 +48,17 @@ var prometheusSyntheticMetrics = map[string]struct{}{ "ALERTS_FOR_STATE": {}, } +// SourceLabelRule enforces that every Deckhouse-owned metric referenced in +// PromQL expressions (PrometheusRule objects and Grafana dashboards) is selected +// with an explicit source="deckhouse" label matcher. +// +// The following metrics are exempt from the check: +// - metrics produced by recording rules within the module (recordingRuleNames), +// since those are computed by us and already scoped; +// - metrics matching the per-module allowed-metrics globs (allowedMetrics), +// used for intentionally foreign metrics such as third-party exporters; +// - Prometheus synthetic metrics (ALERTS, ALERTS_FOR_STATE) that never carry +// scrape-time labels. type SourceLabelRule struct { pkg.RuleMeta recordingRuleNames map[string]struct{} @@ -76,6 +87,10 @@ func globToRegexp(pattern string) (*regexp.Regexp, error) { return regexp.Compile(b.String()) } +// NewSourceLabelRule builds the rule from the templates linter config. The +// allowed-metrics patterns (globs supporting * and ?) are compiled into regexps, +// and the runtime-collected recording rule names are stored so that metrics they +// produce are not required to carry a source selector. func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { var allowedMetrics []*regexp.Regexp @@ -102,6 +117,10 @@ func NewSourceLabelRule(cfg *pkg.TemplatesLinterConfig) *SourceLabelRule { } } +// SourceLabelCheck inspects a single PrometheusRule object and verifies that the +// PromQL expressions of all its alerting and recording rules select Deckhouse +// metrics with a source="deckhouse" matcher. Non-PrometheusRule objects are +// ignored. func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObject, errorList *errors.LintRuleErrorsList) { errorList = errorList.WithFilePath(m.GetPath()).WithRule(r.GetName()) @@ -160,6 +179,10 @@ func (r *SourceLabelRule) SourceLabelCheck(m pkg.Module, object storage.StoreObj } } +// checkExpr parses a PromQL expression and reports every vector selector over a +// Deckhouse metric that lacks a source="deckhouse" matcher. Exempt metrics +// (recording rule outputs, allowed-metrics, synthetic metrics and placeholder +// names produced by Grafana variable sanitization) are skipped. func (r *SourceLabelRule) checkExpr(expr, context, filePath string, errorList *errors.LintRuleErrorsList) { ast, err := parser.ParseExpr(expr) if err != nil { @@ -234,6 +257,12 @@ var ( grafanaVarSimpleRe = regexp.MustCompile(`\$([a-zA-Z_]\w*)`) ) +// sanitizeGrafanaExpr makes a Grafana panel/template expression parseable as +// plain PromQL by replacing Grafana variables with neutral placeholders: +// built-in variables (e.g. $__rate_interval) become a dummy duration, while +// other variables become "__placeholder__" so they are ignored by checkExpr. +// The "source" variable is preserved so a source=$source matcher still counts +// as an explicit source selector. func sanitizeGrafanaExpr(expr string) string { result := grafanaBuiltinVarRe.ReplaceAllString(expr, "5m") result = grafanaVarBracesRe.ReplaceAllStringFunc(result, func(match string) string { @@ -256,6 +285,10 @@ func sanitizeGrafanaExpr(expr string) string { return result } +// SourceLabelCheckDashboards walks every Grafana dashboard file under +// monitoring/grafana-dashboards and verifies that the PromQL queries in their +// panels and template variables select Deckhouse metrics with a +// source="deckhouse" matcher. func (r *SourceLabelRule) SourceLabelCheckDashboards(m pkg.Module, errorList *errors.LintRuleErrorsList) { errorList = errorList.WithRule(r.GetName())