feat(scorecard): enforce a hub-wide meshstack provider version floor - #259
feat(scorecard): enforce a hub-wide meshstack provider version floor#259JohannesRudolph wants to merge 2 commits into
Conversation
A `>=` constraint only says "not older than this"; nothing in the repo said how old is still acceptable *anywhere*. `PROVIDER_FLOOR` at the top of scorecard.mjs is now that single knob — crank a value and the new `provider_floor` check turns red on every module still declaring less, which is the work item for the bump. The check covers every tier a module owns (meshstack_integration.tf, backplane/, buildingblock/, e2e/, incl. nested submodules) and fails two ways: a required_providers entry for a floor-managed provider with no `version` at all (unbounded — `tofu init` may resolve anything), or a constraint whose lower bound sits below the floor. It verifies rather than rewrites: HCL `required_providers` `version` must be a string literal, so there is nothing a module could reference. This matters for local runs specifically. The hub commits no .terraform.lock.hcl, and the smoke-test workflow injects a provider built from main through Terraform `dev_overrides`, bypassing both required_providers and any lockfile — so CI never notices a stale constraint, but `task e2e:run` does. pr-scorecard.sh now reports on all modules when tools/scorecard/ itself changed, because a PR that only turns the knob touches no module and would otherwise get an empty scorecard comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Satisfies the new `provider_floor` scorecard check: every meshcloud/meshstack entry in a module now declares at least `>= 0.23.0`, the first release carrying the current `meshstack_building_block` resource. Two kinds of change, 41 .tf files (+ 4 regenerated buildingblock READMEs): - e2e roots (and their nested submodules) declared the provider with no `version` at all, so a local `tofu init` was free to resolve anything. On-disk lockfiles in this checkout had settled as far back as 0.20.5. Anything below 0.23.0 predates the lifecycle-aware building block delete poll, where `DeletionSuccessful` accepted only a 404 and could not see a soft delete: a local `task e2e:run` teardown then returns before the block is gone and the following building-block-*definition* delete fails with 409 "…because there are existing BuildingBlocks referencing it", stranding objects in the smoke-test workspace. - integration/backplane/buildingblock tiers carried floors from `>= 0.7.1` to `>= 0.21.0`, predating the same fix. Five provider-level meshstack_integration.tf files (modules/<provider>/) are bumped for consistency but sit outside the scorecard's module discovery, which only walks modules/<provider>/<service>/ — the check cannot guard them. The four README Requirements rows are hand-applied rather than regenerated: the terraform-docs available here (0.22.0) reformats every table separator in the file, which is unrelated churn. Only the meshstack version cell is changed, which is exactly what the pinned hook would produce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Scorecard Check
📊 meshstack-hub Module Scorecard
📋 Per-Module Category SummaryScore per category per building block.
Core Structure — some checks failingBasic module file structure and documentation — applies to 52 modules
Core Structure — Summary
Integration — some checks failingmeshstack_integration.tf conventions — applies to 24 modules
Integration — Summary
Azure Backplane — some checks failingAzure UAMI-based automation principal conventions — applies to 14 modules
Azure Backplane — Summary
STACKIT Backplane — some checks failingSTACKIT WIF-based automation principal conventions — applies to 5 modules
STACKIT Backplane — Summary
Testing — some checks failingEnd-to-end test coverage — applies to 52 modules
Testing — Summary
📈 Overall SummaryOverall Average Score: 73%Score Distribution
|
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
JohannesRudolph
left a comment
There was a problem hiding this comment.
@grubmeshi one point i wonder:
I noticed this discrepancy in foundation mode tests (ie smoke tests with a deployed BBD)
- smoke tests pinned v0.21
- hub declared >= 0.21
- hub used features of >= 0.23
Nothing broken in smoke-tests (because it uses dev overrides).
I see two solution for this
- run foundation mode tests for all hub modules (difficult to achieve, though we have decent coverage across demo + try meshstacks)
- run smoke-test workflow as a matrix once with and once without the dev override. This means we have a true CI tests for all meshStack components at latest versions and another test of "stable provider version works against latest meshStack"
Does that make sense?
|
|
||
| // ─── THE PROVIDER FLOOR KNOB ──────────────────────────────────────────────── | ||
| // | ||
| // One central place defining the minimum provider version the *entire hub* is |
There was a problem hiding this comment.
f: this is unnecessary wordy. just focus on the fact that we want to make sure the hub is verified only against recent versions of the provider
also old providers may become incompatible with never versions of meshStack (e.g. when GA apis retire, or breaking changes in preview APIs)
I'd prefer option 2 for now, as for the foundation tests, I'd actually aim to "dry run" or "test" upgrade paths once hub artifacts/modules/ref archs get properly versions (aka that repo here will be tagged with github releases are written down). Option 2 is then a good addition to our e2e test harness and can be achieved quite easily I guess? |
Adds one central knob for the minimum
meshcloud/meshstackprovider version the whole hub is tested and used against, enforced by a new scorecard check, and raises every module to>= 0.23.0.Two commits, reviewable separately: the check, then the bump.
Why a floor is needed
The hub's e2e roots declared
meshcloud/meshstackwith no version constraint and commit no lockfile. In CI that is harmless, because the smoke-test workflow builds the provider frommainHEAD and installs it via Terraformdev_overrides, which bypasses bothrequired_providersand.terraform.lock.hcl.Locally it is not harmless. Gitignored on-disk lockfiles in this repo reach back to provider
0.20.5(modules/ske/ske-starterkit/e2e),0.20.9(modules/azure/budget-alert/e2e,modules/azure/storage-account/e2e) and0.20.11(modules/azure/resource-group/e2e,modules/meshstack/noop/e2e). All predate provider commit01cabb2(releasedv0.20.9), which made the building-block delete poll lifecycle-aware — before it,DeletionSuccessfulaccepted only a 404 and could not detect a soft delete.The consequence is a real race on a local
task e2e:runwithout the override provider: the building block delete returns before the block is soft-deleted, and the following building-block-definition delete gets409 "…because there are existing BuildingBlocks referencing it", stranding cloud objects.0.23.0is the release that first contains the currentmeshstack_building_blockresource (provider commit0ee7530bc07).The knob
PROVIDER_FLOORintools/scorecard/scorecard.mjs, keyed by registry source address rather than local alias, since the alias is per-module:Crank the value, run the scorecard, and the red list is the bump work item.
Verify, not generate
The deciding reason is mechanical: HCL
required_providers { version = … }must be a string literal — no interpolation — so there is nothing a module could reference. Generating would mean a new codegen tool plus a--checkdrift mode, two mechanisms instead of none, and it would fight theterraform_docspre-commit hook that regenerates README requirement tables. Verification reuses the scorecard exactly as-is, and the constraint stays a reviewable statement of what each module actually needs.Scope of the check
The closest existing check,
provider_pinned, enforces>=style but says nothing about how old, and only scannedbackplane/+buildingblock/. The new check covers all four tiers a module owns —meshstack_integration.tf,backplane/,buildingblock/,e2e/(including nested submodules) — and treats "declared with noversion" as a failure, which was the actual finding.provider_pinnedwas left on its original tiers. The shared collector was refactored intocollectProviderEntries(mod, tiers);provider_pinnedstays at exactly 42/52 and every other check is byte-identical to the pre-change report.Results
Before the bump: 26 of 28 applicable modules failed, e.g.
e2e/terraform.tf: meshstack has no version (need ">= 0.23.0")andmeshstack_integration.tf: meshstack = ">= 0.21.0" is below 0.23.0.After:
24 modules render
➖— they declare no meshstack provider of their own.The bump touches 41
.tffiles across 26 modules plus 4 README requirement rows. 16 were thee2e/tier gaining aversionfor the first time; the rest were floors at>= 0.7.1/0.14.0/0.21.0. Every diff is 2–3 lines,tofu fmt-clean, and theterraform_docshook passes on all changed files.Knob proven: temporarily setting it to
0.24.4flipped the row to0/28with per-file detail and a workingAGENTS.md#provider-version-floorfix link. Constraint parser tested against a throwaway fixture —>= 0.23/>= 0.23.0, < 1.0.0/~> 0.24pass;>= 0.22.9/~> 0.22/> 0.22.0/< 1.0.0/ no-version fail.One wiring gap fixed along the way
pr-scorecard.shreports only on changed modules, so a PR that merely turns the knob touches no module and would have printed_No module changes detected_— the knob would have been silent in CI, which defeats the point. It now reports hub-wide whentools/scorecard/itself changed. Verified: this branch triggers the tool-changed path.Known limitation
Provider-level
modules/<provider>/meshstack_integration.tffiles (aks, aws, azure, gcp, stackit) are real consumer-facing configurations, butdiscoverModulesonly walksmodules/<provider>/<service>/, so no scorecard check reaches them. They are bumped here for uniformity, but the check cannot enforce them. Widening discovery would change the module universe for all 30-odd checks, so that is left alone deliberately.🤖 Generated with Claude Code