[dmt] linter for checking rules and dashboards for source=deckhouse label#371
[dmt] linter for checking rules and dashboards for source=deckhouse label#371rvekaterina wants to merge 6 commits into
Conversation
|
|
||
| var ( | ||
| grafanaBuiltinVarRe = regexp.MustCompile(`\$__\w+`) | ||
| grafanaVarBracesRe = regexp.MustCompile(`\$\{(\w+)(?::[^}]*)?\}`) |
There was a problem hiding this comment.
${source} and ${source:json} get replaced with placeholder because this regex runs first and blindly replaces everything, no check on the variable name. the simple $source form is fine below (lines 253-255) - it checks name == "source" and preserves it. but the braced form has no such check.
to reproduce: dashboard with m{source="${source}"}, after sanitization you get m{source="placeholder"}, checkExpr doesn't see source="deckhouse" and complains. TestSanitizeGrafanaExpr only tests
|
|
||
| func (r *SourceLabelRule) checkExpr(expr, ruleName, groupName, filePath string, errorList *errors.LintRuleErrorsList) { | ||
| ast, err := parser.ParseExpr(expr) | ||
| if err != nil { |
There was a problem hiding this comment.
promql parse errors are just eaten. if a rule has a broken expr or the sanitized grafana expression is garbage - the linter silently moves on. no error, no warning.
| } | ||
|
|
||
| func (r *SourceLabelRule) extractDashboardPanels(dashboard *gjson.Result) []gjson.Result { | ||
| panels := make([]gjson.Result, 0) |
There was a problem hiding this comment.
only one level of row -> panels is unwrapped. if a dashboard has a row inside another row, checkPanel never reaches the panels at the second level.
example: panels[0] (type=row) -> inside it panels[0].panels[0] (type=row) -> inside that panels[0].panels[0].panels[0] (the actual panel with targets). the linter skips it.
| } | ||
|
|
||
| func isPrometheusDatasource(obj *gjson.Result) bool { | ||
| dsType := obj.Get("datasource.type").String() |
There was a problem hiding this comment.
when datasource is a plain string, not an object (e.g. datasource: "Graphite"), datasource.type is missing, dsType is empty, the function returns true - claims it's prometheus. in practice ParseExpr silently fails on foreign syntax and produces no errors, so no real harm, but we're burning cpu parsing non-promql stuff.
| grafanaVarSimpleRe = regexp.MustCompile(`\$([a-zA-Z_]\w*)`) | ||
| ) | ||
|
|
||
| func sanitizeGrafanaExpr(expr string) string { |
There was a problem hiding this comment.
if a grafana variable sits where a metric name should be (not a label value), it gets replaced with placeholder, checkExpr parses it as a metric name, doesn't find source="deckhouse", and fires an error.
example: panel expression $my_metric{some="label"} -> placeholder{some="label"} -> false positive.
| r.checkTemplateVariables(&dashboard, filePath, errorList) | ||
| } | ||
|
|
||
| func isPrometheusDatasource(obj *gjson.Result) bool { |
There was a problem hiding this comment.
casing: isPrometheusDatasource -> should be isPrometheusDataSource (lowercase s in camelCase, not uppercase S).
diyliv
added
enhancement
2 participants
No description provided.