fix(starters): derive the OpenTofu pin from the range nic enforces - #617
Conversation
The aws starter's opentofu constraint was a hand-copied literal of pkg/tofu.MinVersion. The two have to agree in both directions: a floor below MinVersion lets pixi resolve a binary compatibleVersion then rejects, so nic downloads its own copy anyway - silently, and the pin the workspace exists to provide is gone. Build the constraint from MinVersion and MaxVersionExclusive instead, so a bump to either reaches the starters in the same commit rather than leaving a stale literal behind. MaxVersionExclusive is exported for this: together the two constants are the supported range a packager has to express. Generated output changes only in spelling, <2 becomes <2.0.0, which is the same conda constraint.
afc5c7b to
fad7388
Compare
The MaxVersionExclusive comment said the workspace pin and the enforced range cannot drift apart. The generated textual bounds cannot, but the accepted version sets can still differ on prereleases: compatibleVersion compares by Core(), so 2.0.0rc1 is rejected as 2.0.0, while conda orders it below 2.0.0 and admits it under <2.0.0.
dcmcand
left a comment
There was a problem hiding this comment.
Deriving the constraint from the constants nic already enforces removes the drift class instead of documenting it, and I agree the Sprintf doesn't need a test of its own - pkg/tofu/resolve_test.go already covers the half that can actually be wrong.
Generated at -version 0.14.0 and confirmed the output is identical to the old literal apart from <2 becoming <2.0.0; both starters' pixi.toml still parse.
Two suggestions inline, neither gating. Docs: no drift. docs/operations/packaging.md cites pkg/tofu.MinVersion symbolically, ADR-0016 states the window as [1.11.3, 2.0.0), and starters/templates/README.aws.md says nic pins OpenTofu - nothing there became false.
| // the other end. Deriving it means a bump to either constant reaches the | ||
| // starters in the same commit instead of leaving a stale literal behind. | ||
| func opentofuConstraint() string { | ||
| return fmt.Sprintf(`opentofu = ">=%s,<%s"`, tofu.MinVersion, tofu.MaxVersionExclusive) |
There was a problem hiding this comment.
This makes the generated pixi.toml depend on pkg/tofu, which .github/workflows/starters.yml does not watch - its paths lists are starters/**, cmd/starters/**, the two example configs, and itself. The PR that bumps MinVersion will change published starter bundles without running validate-starters.
The only step that would catch a floor conda-forge cannot satisfy is pixi lock, and it runs solely in publish-starters, behind if: startsWith(github.ref, 'refs/tags/v'). validate-starters checks token substitution, TOML parseability and nic validate behaviour, none of which see a version constraint. So the bump PR goes green and the tag build fails in the credentialed publish job.
Adding pkg/tofu/** to both paths lists is half of it; on its own it only re-runs steps that cannot fail on this. Worth pairing with a pixi lock against dist/starters/aws in validate-starters.
Not gating - a loud release failure is strictly better than the silent fallback this PR removes. It just lands in the wrong PR.
| // pins and the ones this file enforces cannot drift apart. The accepted sets | ||
| // can still differ on prereleases, which compatibleVersion compares by Core() | ||
| // while conda applies its own ordering. | ||
| const MaxVersionExclusive = "2.0.0" |
There was a problem hiding this comment.
docs/adr/0016-opentofu-runtime-version-policy.md is where someone raising the floor will look, and its maintenance policy covers the MinVersion <= Version invariant and the CI lockfile workflow's independence, but not this. Worth a bullet: raising the floor now edits an artifact that gets pushed to quay.io. Nothing in the ADR is false, so this is a gap rather than drift.
Separately, the prerelease caveat this comment adds checks out and is currently unreachable: compatibleVersion("2.0.0-rc1") rejects (it compares Core()) while conda's <2.0.0 would accept 2.0.0rc1, but conda-forge has published 39 opentofu versions and zero prereleases.
Closes
Part of #552. Prerequisite for #618, and the instance that #619 generalises.
Follows the starter generator rewrite in #615.
The problem
The aws starter declared its OpenTofu dependency as a literal:
with a comment explaining that the floor has to clear
pkg/tofu.MinVersion. That comment is the whole guard. Nothing enforces it.The two have to agree in both directions, and a mismatch fails quietly rather than loudly:
compatibleVersionthen rejects, so nic falls through to downloading its own copy. The workspace still solves, the deploy still works, and the pin the starter exists to provide is silently gone.This is live rather than hypothetical: a bump to
pkg/tofu.Version/MinVersionis being tracked separately, and it would leave this literal behind.The fix
Build the constraint from the constants nic already enforces:
maxVersionExclusivebecomesMaxVersionExclusivefor this. The two constants together are the supported range a packager has to express, so exporting the pair is more coherent than exporting half of it.Follow-up from review: the
MaxVersionExclusivecomment claimed the two "cannotdrift apart". The generated textual bounds cannot, but the accepted version sets
can still differ on prereleases, since
compatibleVersioncompares byCore()while conda applies its own ordering. Scoped to release versions in
e83da89.No new test. Deriving the value removes the drift class outright, and a test re-asserting the derivation would only restate the
Sprintf- the behavioural half, thatcompatibleVersionacceptsMinVersionand rejects below it, is already covered inpkg/tofu/resolve_test.go.What this does not fix
A future provider entry can still ship
deps: ""and lose its pin - azure is OpenTofu-backed and is the realistic case when the starter scope widens past local+aws. #615 made that much harder by folding the provider list and its deps into one map, so the omission is at least visible in the struct literal, but nothing fails if someone copies thelocalentry. Left alone rather than guarded speculatively, since the right guard depends on how the scope actually widens.Also unchanged: the Hetzner provider's external binary cannot be pinned this way at all.
hetzner-k3sis not published on conda-forge or prefix.dev, so a hetzner starter would fall through to nic's checksum-verified download. Worth knowing when reading the "one lockfile pins the whole toolchain" claim: it holds for aws and local, not for hetzner.How to Test
Same constraint as before, spelled from the source of truth.
<2becoming<2.0.0is the only output change.make vet,make lintandgo test -short ./...all pass.