Repository navigation
Estate Consolidation P0 — cli installs the CLI the repository pins, and the estates' infra-diff becomes one reusable workflow - #11
Closed
igorlamos wants to merge 2 commits into
Conversation
… (BE-213) A CLI release must never change what a repository's CI does without a commit in that repository, and `cli` defaulted to `latest`. Now, with no `BE_CLI_VERSION`, it installs the version pinned in the nearest beplus.estate.json → cli.version by running `be auto` — the same command a laptop, a Docker build stage or a buildspec runs, so the pin is resolved in one place — and then fails unless `beplus --version` is exactly that pin. An inexact or unreadable pin fails the run; it never falls back to `latest`. An explicit `BE_CLI_VERSION` still wins (with a warning when it differs from the pin). With no pin the action keeps today's behaviour, `be latest`, and says with a `::warning::` that the repository floats. `BE_WORKING_DIRECTORY` says where the nearest manifest is looked up from (default: the workspace). `cdk-package` installs the CLI through `cli`; its `cli-version` default goes from `latest` to empty so it honours the pin too (its note about `latest` being v1 is stale: latest is 2.11.0). The four reusable workflows stop passing an explicit `latest`, which would override a pin; `vars.BE_CLI_VERSION` still wins when it is set. `test-cli.yml` runs this branch's action against fixtures in test/cli: a pin installs exactly it (2.10.0 — not the newest release, so a run that floated cannot pass), also from a package directory below the manifest; BE_CLI_VERSION wins; no pin and no manifest float on latest; an inexact pin (`2`) fails the run. The pinned legs go green once `be auto` reads beplus.estate.json on beplus/be@main. Co-Authored-By: Igor Lamos <igor@be.plus>
… workflow (BE-218) bepluscloud/ai, ironmountain-cz/monorepo and mrkithq/monorepo each carried a 186-line infra-diff.yml. Their jobs differed in three values, and all three are derived now, so the workflow has no inputs: - the role github-actions-<owner>-<repo>-<env>-cdk-diff, from the calling repository (github.repository_owner + github.event.repository.name), as each estate's GitHubOIDCStack names it; the account is still vars.BE_AWS_ACCOUNT_ID of the environment; - the region, from beplus.estate.json defaultRegion (vars.AWS_REGION still wins); - the sticky-comment marker, from naming.product lower-cased: be / mountaineer / mrkit, exactly the markers the copies used, so the switch edits a PR's existing comment. A plan step reads the last two from the manifest, validates them, and hands the marker to the comment script through env, never pasted into it. Every other step body is byte-identical to the copies, bar the CLI version: `vars.BE_CLI_VERSION || '2'` floated on the newest 2.x; empty now installs the repository's pin through `be auto` (BE-213). It cannot deploy: the composite always runs `mode: diff` as the read-only …-cdk-diff role, both literals, and with no input declared a caller that passes `mode` is refused by GitHub. The trigger paths, the permissions and the concurrency group stay in each repository's ~30-line caller. scripts/verify-callers.mjs gains `infra-diff`: it compares the shared jobs, resolved for an estate, with the file its caller replaced — every job key, every step, every run/script byte — allowing only declared differences (three derivations, the BE-213 pin, the added manifest step/outputs/env), and checks the shared workflow can only diff. The three callers against bepluscloud origin/dev, Mountaineer origin/dev and MRKIT dev: 30 steps, 18 identical, 3 identical once derived, 6 the intended pin, 3 added, 0 divergent. Nine mutations (deploy mode, deploy role, a mode input, an edited or dropped step, a caller passing mode, a lost path, an extra job, the wrong repository) all go red. The R2-1 library mode is unchanged. Co-Authored-By: Igor Lamos <igor@be.plus>
Contributor
|
Closing so the agent opens this PR under its own App identity (bea), as agreed. Same branch, title and body — the replacement links back here. |
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.
This PR does two things. First, the
cliaction now installs the beplus CLI version a repository pins inbeplus.estate.json(viabe auto) instead of floating onlatest, so a CLI release can no longer change what a repository's CI does without a commit in that repository. Second, the 186-lineinfra-diff.ymlthat each of the three estates carries becomes one reusable workflow here, with no inputs. It cannot be made to deploy, and a script proves step by step that it reproduces each copy it replaces.Merge order: this PR comes after the beplus/be
be autoPR (BE-213). It must be merged, and thev2tag moved to the merge commit, before any estate merges its BE-218 infra-diff caller. The full list is under "Waiting on Igor" at the end.BE-213 — Pin the beplus CLI per repository (Linear)
A CLI release must never change what a repository's CI does unless that repository has a commit that says so. Until now,
clidefaulted tolatest.What changed
cli/action.ymlBE_CLI_VERSIONnow defaults to empty instead oflatest.beplus.estate.jsonat or aboveBE_WORKING_DIRECTORY, walking up the same waybe autodoes. It readscli.versiononly to decide whether a pin exists. Resolving and validating the pin staysbe auto's job (PLAN principle 1).be auto. A post-check then fails the run unlessbeplus --versionis exactly the pin. An inexact pin, or a bug inbe auto, can never fall back to latest.BE_CLI_VERSION: the action runsbe <value>. The explicit value wins, with a::warning::when it differs from the pin.be latest, as today, and emits a::warning::that the repository floats. The warning shows thecli.versionsnippet to paste in.beargument now reaches the script through env and is quoted, not interpolated.BE_WORKING_DIRECTORY(default: the workspace) says where the manifest search starts.cdk-package/action.ymlinstalls the CLI throughcli@v2. Itscli-versiondefault changes fromlatestto empty, so it honours the pin too. I also removed its stale note that latest is v1; latest is 2.11.0.docs-pages,library-ci,library-publish(two places) andprovisioningno longer passvars.BE_CLI_VERSION || 'latest'. An explicitlatestwould override a pin. They now passvars.BE_CLI_VERSION, which still wins when it is set..github/workflows/test-cli.ymlruns this branch's./cliin a six-leg matrix against fixtures intest/cli/{pinned,unpinned,inexact}and asserts whatbeplus --versionreports.cli/README.mdgains a "Pinning the CLI" section, the bump procedure (one commit per repository, CI proves it, merged on its own) and troubleshooting entries for the two new messages. The rootREADME.mdexamples no longer passBE_CLI_VERSION: latest.Gates
node harness.cjs <worktree>: runscli/action.yml's choose → install → verify steps andtest-cli.yml's assert step locally, for every matrix leg, with a fakebethat implementsbe autoas BE-213 specifies (FAKE_BE_MODE=spec)BE_CLI_VERSION=2.9.0wins, with the override warning. No pin and no manifest both float on latest, with the float warning. The inexact pin"2"fails the action.FAKE_BE_MODE=lax(a buggybe autothat resolves"2"to 2.11.0)::error … pins beplus CLI 2, but be auto installed 2.11.0FAKE_BE_MODE=old(today's beplus/be@main, whosebe autodoes not readbeplus.estate.jsonyet)be autofrom the beplus/be branch (4aa9ea2)shellcheckon everyrun:block ofcli/action.ymlandtest-cli.yml(extracted, with${{ }}substituted)parseDocumentoncli/action.yml,cdk-package/action.yml,.github/workflows/*.ymlinputs.Xdeclared, everysteps.choose.outputs.Xwritten, every in-repo caller ofclipasses only declared inputs"cli"as a string, a version with whitespace,version: null::errornaming the file.nullcounts as no pin and floats with the warning.node scripts/verify-callers.mjs ~/Git/github.com/beplus(the repo's old R2-1 library gate), at base and on the branchTypeError … reading 'map'on the same expression. It was already broken atmain, because the HEAD~1 originals are gone, so it is unrelated to this change.Deviations
latestwould also report 2.11.0 and the test would prove nothing. 2.10.0 is a real release (it is in the S3 release index), so a run that floats cannot pass.BE_WORKING_DIRECTORY. The test needs it to point the action at fixture manifests, and monorepos may want a manifest that is not at the root. The default keeps behaviour unchanged for every existing caller.cliandcdk-package: the four reusable workflows passed|| 'latest', which would override any pin. They now pass onlyvars.BE_CLI_VERSION. For repositories without a pin, the only visible change is the new float warning.cli.versionitself, but only to detect a pin and to checkbeplus --versionafter the install. That post-check is deliberate defence in depth.be/beplusbinaries were downloaded or installed on the agent machine. The logic was proven with a fakebe, plus the review re-run against the realbe autoscript. The live proof is this PR's "Test cli" run.provisioning/README.md:106still showsBE_CLI_VERSION: latestin an example.cli.versionset to empty, null or false, or"cli": {}, floats to latest with a warning instead of failing.BE_CLI_VERSION: autoskips the post-check.BE-218 —
infra-diff.ymlas a reusable workflow (Linear)bepluscloud/ai, ironmountain-cz/monorepo and mrkithq/monorepo each carried a 186-line
infra-diff.yml. Their jobs differ in three values only, and all three are now derived, so the shared workflow has no inputs.What changed
.github/workflows/infra-diff.yml(on: workflow_call). I built it from the three estate copies: bepluscloudorigin/dev3065b04, Mountaineerorigin/dev3fd8e9893 and MRKITdev13220b751. The three derived values:arn:aws:iam::${{ vars.BE_AWS_ACCOUNT_ID }}:role/github-actions-${{ github.repository_owner }}-${{ github.event.repository.name }}-${{ matrix.env }}-cdk-diff.GitHubOIDCStack.tsbuildgithub-actions-${org}-${repo}-${env}-${name}fromGITHUB_REPOSITORY.aud,repository_owner_idandsub(pull_request/environment:<env>), with noworkflow_refcondition, so the roles keep trusting a run that comes through a reusable workflow.vars.BE_AWS_ACCOUNT_ID.vars.AWS_REGION || needs.plan.outputs.region, whereregioncomes frombeplus.estate.json→defaultRegion. That gives us-east-1, eu-central-1 and us-east-1, each matching its copy's literal.naming.productlower-cased (be/mountaineer/mrkit), exactly what each copy used, so open PRs get their existing comment edited instead of a second one.planjob (id: estate) reads and validates the region and the marker. The marker reachesgithub-scriptonly through env, never pasted into the script.vars.BE_CLI_VERSION || '2'becomesvars.BE_CLI_VERSION, so the repository's pin is installed viabe auto(BE-213).mode: diffand the-cdk-diffsuffix are literals.with:a caller passes.…-cdk-diffrole can only reach the CDK lookup role, and…-cdk-deploydoes not trustpull_request.scripts/verify-callers.mjsgains aninfra-diffsubcommand.run/scriptbyte.README.mddocuments the workflow and the ~30-line caller. The trigger paths, the permissions and the concurrency group stay in each repository.Gates
node scripts/verify-callers.mjs infra-diff --repo ~/Git/github.com/bepluscloud/ai --ref origin/dev --caller <bepluscloud caller> --repo ~/Git/github.com/ironmountain-cz/monorepo --ref origin/dev --caller <mountaineer caller> --repo ~/Git/github.com/mrkithq/monorepo --ref dev --caller <mrkit caller>(Node 22.14)on, permissions and concurrency are identical to the original, with one job calling@v2andsecrets: inherit. Per estate, 10 steps: 6 identical, 1 identical once derived, 2 intended (BE-213 pin), 1 added, 0 divergent. Total: 18 identical, 3 derived, 6 intended, 3 added, 0 divergent.3065b04, Mountaineerorigin/devnowd8d997d3c, MRKIT1c28cd1d1)infra-diff.ymlon that estate's P0 branch.verify-callers infra-diffagainst nine mutations on scratch copies: sharedmode: deploy; shared-cdk-deployrole; sharedmodeinput wired to cdk-package; shared step body edited; shared step dropped; caller passeswith: mode: deploy; caller loses a trigger path; caller adds a cdk-package deploy job; wrong--repositorybeplus infra lint --path <scratch estate>(CLI v2.11.0 from cli_v2 source; scratch =.github+beplus.estate.jsonof each estate)mode: deployfails (exit 2): the PR path would run the composite in deploy mode. The issue's acceptance check, that lint still fails a branch adding a deploy mode, holds.eab15d0: lint now also checks a callerinfra linton each estate passes with the original and with the caller. It exits 2 with exactly one named failure for each of: amodeunderwith:, arole-arnunderwith:, another repository's workflow, and a missingid-token: write. Run on all 3 estates. plugin-infra 175/175, core/cli 601/601, golden-diff 0 divergent.Deviations
github.repositoryplus the manifest. It turned out to come entirely from the calling repository, the same wayGitHubOIDCStacknames it.vars.BE_CLI_VERSION || '2'becomesvars.BE_CLI_VERSION, the BE-213 behaviour.cli.version(P3), it installslatestwith a float warning.be 2resolves to.infra-diff.ymlwith the caller.Companion commits (same P0 branch name in each repository)
fde4997, ironmountain-cz/monorepof974307d5, mrkithq/monorepoccbd2672d:ci(infra-diff): a pull request's cdk diff comes from setup-beplus's shared workflow (BE-218). In each,infra-diff.ymlbecomes the caller.eab15d0(BE-218: infra lint checks callers) and5032316(BE-213: manifestcli.version, plus a lint warning when it is missing).4aa9ea2(BE-213:be autoreadsbeplus.estate.json→cli.version).Waiting on Igor (in order)
be autoPR first (BE-213,4aa9ea2).bin/bump minoron its branch, thenscripts/release/verify-version.sh pr origin/main feature/estate-consolidation-p0-foundations. Expected: "Merging this PR releases @beplus/be@0.10.0".Test clipinned legs stay red, becausecliinstallsbefrom beplus/be@main..github/workflowschanges.pinned, andpinnedfrom a package directory: "beplus CLI 2.10.0, as expected".v2tag:git tag -f v2 <merge-sha> && git push -f origin v2.cli@v2,cdk-package@v2and.github/workflows/infra-diff.yml@v2exist and honour the pin.clisection yet, so they only gain the float warning."cli": {"version": "2.11.0"}tobeplus.estate.jsonand drops explicitBE_CLI_VERSION: ${{ vars.BE_CLI_VERSION || '2' }}ininfra.yml. Until then, today's explicit value wins over a pin, with a warning.