fix(grafanactl): require a regex on the dashboard datasource variable#249
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds validation coverage and tightens dashboard validation so missing datasource regex is treated as an error (not just a warning).
Changes:
- Add unit tests asserting missing datasource regex is an error and that a present regex passes.
- Promote “missing datasource variable regex” from warning to error in
validateDashboard.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tools/grafanactl/internal/grafana/validate_test.go | New tests for the updated regex validation severity/behavior. |
| tools/grafanactl/internal/grafana/syncer.go | Changes validation outcome from warning to error and clarifies rationale in comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mmazur
force-pushed
the
fix/require-datasource-regex
branch
from
f347494 to
8a46a79
Compare
Dashboard validation previously only emitted a *warning* when a datasource template variable had no regex. As a result, dashboards could ship with an unconstrained datasource picker that lists every datasource in the org -- including obsolete Managed Prometheus datasources that carry no data -- and still pass `grafanactl verify dashboards` in CI. Promote the missing-regex check from a warning to a hard validation error so CI fails when a dashboard does not constrain its datasource variable. Dashboards that intentionally want any datasource can set a permissive regex (e.g. ^Managed_Prometheus_.*$). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
mmazur
force-pushed
the
fix/require-datasource-regex
branch
from
8a46a79 to
1de2f20
Compare
mmazur
added a commit
to mmazur/ARO-HCP
that referenced
this pull request
Jun 16, 2026
Adds a proper dashboard whose datasource variable has an empty regex, to observe how the observability-verify CI check handles it. With the current grafanactl this is only a warning (CI passes); once Azure/ARO-Tools#249 lands and the grafanactl dependency is bumped, the missing regex becomes a hard error. For demonstration only -- not intended to merge. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
stevekuznetsov
approved these changes
Jun 16, 2026
Contributor
|
/ok-to-test |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Promote the dashboard datasource-variable regex check from a warning to a hard validation error in
grafanactl.Why
validateDashboardpreviously only warned when a datasource template variable had no regex. A dashboard with an unconstrained datasource picker — which lists every datasource in the org, including obsolete Managed Prometheus datasources that carry no data — would still passgrafanactl verify dashboardsin CI.Making it an error lets teams enforce that every dashboard constrains its datasource variable. Dashboards that intentionally allow any datasource can set a permissive regex (e.g.
^Managed_Prometheus_.*$).This complements Azure/ARO-HCP#5653, which gives every ARO-HCP dashboard a datasource regex; with that merged, no ARO-HCP dashboard is left unfiltered, so this check can be enforced without breaking CI.
Changes
internal/grafana/syncer.go: missing-regex issue moved fromwarningstoerrors(one-line change).🤖 Generated with Claude Code