fix(grafana): correct curated-pack revision/hash pins, add maintainer verification script (#350) - #365
Open
giorgi-imerlishvili-elastic wants to merge 1 commit into
Conversation
… verification script (elastic#350) All 6 dashboard_sha256 pins in curated_packs/registry.yaml were wrong (didn't match a canonical-JSON hash of the pinned grafana.com revision), and gnet_id 11835 pinned gnet_revision 4, which has never existed on grafana.com (that dashboard only has revision 1) -- silently defeating the provenance check these fields exist for. - Correct dashboard_sha256 for the 5 packs whose gnet_revision resolves to a real download; re-pin 11835 to revision 1 with an explicit callout that this can't be independently confirmed to match the pack authors' original source, since revision 4 never existed. - Add scripts/verify_curated_pack_pins.py, a maintainer/CI-only command (network required, not part of `make test`) that re-downloads each pinned revision and re-checks its canonical hash, mirroring the existing fetch_community_corpus.py pattern. Hardened against a malformed download response aborting the whole run, and against an empty/unparseable registry silently reporting a false "0/0 verified" pass. - Document these fields as maintainer-verified provenance pins, not a migration-time gate: a pristine grafana.com download structurally differs from any real Grafana-instance export (mutated id/uid/version), so comparing against operator dashboards would mismatch on every real migration. Rewrote docs/design/curated-dashboard-packs.md's "drift detection" section, which described a revision-comparison mechanism and two CLI commands that were never actually implemented, to match this. - Fix a stale duplicate gnet_revision in the 11835 pack's own fidelity_manifest.yaml left behind by the re-pin, and add a test guarding every pack's manifest against the registry to prevent this drift from recurring silently. Depends on elastic#362 (dropped-source-metric disclosure) for the new docs/sources/grafana.md wording to describe behavior that actually exists on main; should land at or after that PR.
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
1 participant
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.
Summary
Issue #350: all 6
dashboard_sha256provenance pins incurated_packs/registry.yamlwere wrong (didn't match a canonical-JSON hash of the pinned grafana.com revision), andgnet_id: 11835pinnedgnet_revision: 4, which has never existed on grafana.com — that dashboard has only ever had revision 1. Together these silently defeated the provenance check these fields exist for.dashboard_sha256for the 5 packs whosegnet_revisionresolves to a real grafana.com download; re-pinned11835to revision 1 (the only real option), with an explicit registry comment noting this can't be independently confirmed to match the pack authors' original source since revision 4 never existed.scripts/verify_curated_pack_pins.py, a maintainer/CI-only command (requires network, intentionally not part ofmake test) that re-downloads each pinned revision from grafana.com and re-checks its canonical hash — mirrors the existingscripts/fetch_community_corpus.pypattern. Ran it live during development: 6/6 pins verify against grafana.com right now.id/uid/version/etc.), so that comparison would mismatch on every real migration. The actual risk (a pack silently missing dashboard content) is instead caught per-panel by the dropped-source-metric detection landing in fix(grafana): disclose source metrics silently dropped from migrated queries #362.registry.yaml's header,docs/sources/grafana.md, anddocs/contributing/dev-commands.md.Code-review fixes (found during a second-model review pass)
docs/design/curated-dashboard-packs.md(linked fromcurated_packs/__init__.pyas the canonical design spec) described a migration-time revision-drift-detection mechanism and two CLI commands (obs-migrate curated-pack check/diff) that were never actually implemented anywhere in the codebase — verifiedresolve_pack_for_dashboarddoes zero revision comparison. Rewrote that section to describe the real current behavior.11835pack's ownfidelity_manifest.yamlstill had the stalegnet_revision: 4left over from before the re-pin (a duplicate, non-runtime-read copy of the same field) — corrected it and added a test that cross-checks every pack's manifest against the registry so this can't drift silently again.verify_curated_pack_pins.py: a malformed/non-JSON download response previously raised an uncaught exception that aborted the entire run instead of failing just that one pack; an empty/unparseable registry previously reported a false "0/0 pins verified" success. Both are now fail-closed, with regression tests.Dependency note
The new
docs/sources/grafana.mdwording describes the per-panel dropped-source-metric detection mechanism, which currently only exists on #362 (not yet merged tomain). This PR should land at or after #362 so that description matches what's actually onmain.Test plan
make test— 5751 passed, 3 skippedmake lint— cleanmake typecheck— cleantests/test_verify_curated_pack_pins.py(new) — offline hashing/parsing/error-handling coverage, no real network callstests/test_curated_packs.py— new registry shape/uniqueness checks plus a manifest-vs-registry consistency guardpython scripts/verify_curated_pack_pins.pyagainst grafana.com — 6/6 pins verified