diff --git a/.github/path-filters/aws-ci.paths b/.github/path-filters/aws-ci.paths new file mode 100644 index 00000000..b095da77 --- /dev/null +++ b/.github/path-filters/aws-ci.paths @@ -0,0 +1,18 @@ +# The build paths for .github/workflows/aws-ci.yml. Moved out of that +# workflow's trigger blocks so the gate job is reported on every pull +# request - see scripts/changed_paths.py and issue #632. + +deploy/aws/template.yaml +deploy/aws/lightsail-template.yaml +docs/index.html +docs/vendor/** +scripts/ws_smoke.py +scripts/check_lightsail_userdata.py +scripts/check_docs_vendor.py +bin/agentbox +tests/test_agentbox.py +tests/native/** +.github/path-filters/aws-ci.paths +scripts/changed_paths.py +.github/workflows/aws-ci.yml +.github/workflows/deploy-test.yml diff --git a/.github/path-filters/azure-ci.paths b/.github/path-filters/azure-ci.paths new file mode 100644 index 00000000..fe9b79dc --- /dev/null +++ b/.github/path-filters/azure-ci.paths @@ -0,0 +1,9 @@ +# The build paths for .github/workflows/azure-ci.yml. Moved out of that +# workflow's trigger blocks so the gate job is reported on every pull +# request - see scripts/changed_paths.py and issue #632. + +deploy/azure/** +scripts/check_azure_template.py +.github/path-filters/azure-ci.paths +scripts/changed_paths.py +.github/workflows/azure-ci.yml diff --git a/.github/path-filters/ci.paths b/.github/path-filters/ci.paths new file mode 100644 index 00000000..925821d0 --- /dev/null +++ b/.github/path-filters/ci.paths @@ -0,0 +1,69 @@ +# The build paths for .github/workflows/ci.yml. +# +# This list used to be the workflow's own `on: push/pull_request: paths:` +# block. It moved here because a trigger-level filter reports NO check run +# on a pull request it does not match, which makes the check unrequireable +# (issue #632) - see scripts/changed_paths.py for the whole reasoning. The +# workflow's `changes` job reads this file; its `gate` job reports either +# way. +# +# Blank lines and `#` comments are ignored. The dialect is GitHub's own +# `paths` glob, so an entry moved here from a trigger block needs no +# rewriting. + +**.nix + +# Sources of the generated modules/agent-box.nix (issue #140): a change +# here can reassemble to the same bytes (a pure refactor), so it would +# not touch a *.nix file - match them explicitly so the module-generated- +# up-to-date guard (and the rest of CI) still runs. +modules/agent-box.nix.in +modules/src/** +bin/assemble-module.py +tests/test-assemble-module.py + +# The vendored third-party assets under modules/src/vendor are matched +# by 'modules/src/**' above; this is the checker that verifies them. +scripts/check_vendor.py + +# Every workflow, not just this one: CI validates no other workflow's +# YAML, so a file that cannot parse is simply never run and nothing +# says so. PR #483 shipped exactly that - vendor-updates.yml had a +# heredoc dedented out of its `run: |` block scalar, CI stayed green, +# and the weekly job would have silently never fired. +.github/workflows/** + +# The path filters themselves, and the matcher that reads them. Editing +# one of these decides which jobs run at all, so it has to run the jobs +# (issue #632). +.github/path-filters/** +scripts/changed_paths.py +tests/test-changed-paths.py + +tests/test-envstore.py + +# The native-render fixture (issue #154 Phase 4): the renderer under +# test, the test itself, and its committed expected/ snapshot must all +# re-run agentbox-render when any one changes, or a stale fixture only +# surfaces via a manual dispatch (as it did for PR #381). +bin/agentbox +tests/test_agentbox.py +tests/native/** + +# The mascot mark (issue #185) is embedded into the settings daemon at +# assemble time, so it is a module source as much as a website asset. +docs/potato.svg + +# The golden behavior snapshot (issue #154): the fixture and its +# renderer must re-run the golden-snapshot check when either changes. +bin/golden-snapshot.py +tests/golden/** + +# The release manifest (issue #632) is what promotion records and what +# publish-template.yml consumes instead of re-resolving anything, so its +# builder and its tests are build paths like any other checked script. +scripts/release_manifest.py +tests/test-release-manifest.py + +flake.lock +.github/workflows/ci.yml diff --git a/.github/workflows/aws-ci.yml b/.github/workflows/aws-ci.yml index 07ce77c9..793e402f 100644 --- a/.github/workflows/aws-ci.yml +++ b/.github/workflows/aws-ci.yml @@ -1,23 +1,17 @@ name: AWS template CI on: + # No `paths:` filter here, on purpose (issue #632): a + # trigger-level filter means the workflow never STARTS on a + # change it does not match, and a workflow that never starts + # reports no check run at all - so the check cannot be + # required without leaving unrelated pull requests pending + # forever. The filter lives in .github/path-filters/aws-ci.paths, + # read by the `changes` job; the `gate` job reports either + # way and is the check to require. push: branches: [ master ] - paths: &aws-ci-paths - - 'deploy/aws/template.yaml' - - 'deploy/aws/lightsail-template.yaml' - - 'docs/index.html' - - 'docs/vendor/**' - - 'scripts/ws_smoke.py' - - 'scripts/check_lightsail_userdata.py' - - 'scripts/check_docs_vendor.py' - - 'bin/agentbox' - - 'tests/test_agentbox.py' - - 'tests/native/**' - - '.github/workflows/aws-ci.yml' - - '.github/workflows/deploy-test.yml' pull_request: - paths: *aws-ci-paths workflow_dispatch: concurrency: @@ -28,8 +22,41 @@ permissions: contents: read jobs: + # Cheap, always runs, and decides for the expensive job below (issue + # #632). Its own failure is a gate failure: "we could not work out + # whether the checks were needed" must never read as "they passed". + changes: + name: Decide whether AWS template CI's build paths changed + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + build: ${{ steps.filter.outputs.build }} + steps: + - uses: actions/checkout@v5 + with: + # Both ends of the range have to be IN the clone; the default + # depth-1 checkout has neither a pull request's base nor a + # push's `before`. + fetch-depth: 0 + + - name: Match the changed paths against the filter + id: filter + env: + # HEAD for both events: on a pull request actions/checkout leaves + # the MERGE commit checked out, whose merge base with base.sha is + # base.sha itself, so `base...HEAD` is exactly the pull request's + # own diff. + BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} + run: | + answer=$(python3 scripts/changed_paths.py \ + .github/path-filters/aws-ci.paths --base "$BASE" --head HEAD) + echo "build=$answer" >> "$GITHUB_OUTPUT" + echo "::notice::AWS template CI build paths changed: $answer" + validate: name: Validate AWS template and smoke helper + needs: changes + if: needs.changes.outputs.build == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 @@ -66,3 +93,42 @@ jobs: # loads would otherwise ship to the public site with nothing to catch it. - name: Check docs/vendor pins run: python3 scripts/check_docs_vendor.py + + # THE check to require in the branch ruleset (issue #632): reported on + # every push and every pull request, whatever paths they touch, so it + # can never be the required-but-never-reported check that leaves a pull + # request permanently unmergeable. + gate: + name: AWS template gate + needs: [ changes, validate ] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Report the aggregate result + env: + CHANGES: ${{ needs.changes.result }} + VALIDATE: ${{ needs.validate.result }} + BUILD: ${{ needs.changes.outputs.build }} + run: | + echo "changes=$CHANGES build=$BUILD validate=$VALIDATE" + if [ "$CHANGES" != "success" ]; then + echo "::error::the changes job did not succeed ($CHANGES), so nothing here knows whether the checks were needed." + exit 1 + fi + case "$VALIDATE" in + success) + echo "AWS template CI passed." + ;; + skipped) + if [ "$BUILD" = "true" ]; then + echo "::error::the build paths changed but validate was skipped - the guard expression on that job is wrong." + exit 1 + fi + echo "No relevant path changed; AWS template CI had nothing to run." + ;; + *) + echo "::error::validate did not pass ($VALIDATE)." + exit 1 + ;; + esac diff --git a/.github/workflows/azure-ci.yml b/.github/workflows/azure-ci.yml index f57681c2..80b2eeab 100644 --- a/.github/workflows/azure-ci.yml +++ b/.github/workflows/azure-ci.yml @@ -1,14 +1,17 @@ name: Azure template CI on: + # No `paths:` filter here, on purpose (issue #632): a + # trigger-level filter means the workflow never STARTS on a + # change it does not match, and a workflow that never starts + # reports no check run at all - so the check cannot be + # required without leaving unrelated pull requests pending + # forever. The filter lives in .github/path-filters/azure-ci.paths, + # read by the `changes` job; the `gate` job reports either + # way and is the check to require. push: branches: [ master ] - paths: &azure-ci-paths - - 'deploy/azure/**' - - 'scripts/check_azure_template.py' - - '.github/workflows/azure-ci.yml' pull_request: - paths: *azure-ci-paths workflow_dispatch: concurrency: @@ -19,8 +22,41 @@ permissions: contents: read jobs: + # Cheap, always runs, and decides for the expensive job below (issue + # #632). Its own failure is a gate failure: "we could not work out + # whether the checks were needed" must never read as "they passed". + changes: + name: Decide whether Azure template CI's build paths changed + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + build: ${{ steps.filter.outputs.build }} + steps: + - uses: actions/checkout@v5 + with: + # Both ends of the range have to be IN the clone; the default + # depth-1 checkout has neither a pull request's base nor a + # push's `before`. + fetch-depth: 0 + + - name: Match the changed paths against the filter + id: filter + env: + # HEAD for both events: on a pull request actions/checkout leaves + # the MERGE commit checked out, whose merge base with base.sha is + # base.sha itself, so `base...HEAD` is exactly the pull request's + # own diff. + BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} + run: | + answer=$(python3 scripts/changed_paths.py \ + .github/path-filters/azure-ci.paths --base "$BASE" --head HEAD) + echo "build=$answer" >> "$GITHUB_OUTPUT" + echo "::notice::Azure template CI build paths changed: $answer" + validate: name: Validate the Azure Bicep template + needs: changes + if: needs.changes.outputs.build == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 @@ -48,3 +84,42 @@ jobs: # leaves the extension's protectedSettings. - name: Check the compiled template and the bootstrap it carries run: python3 scripts/check_azure_template.py + + # THE check to require in the branch ruleset (issue #632): reported on + # every push and every pull request, whatever paths they touch, so it + # can never be the required-but-never-reported check that leaves a pull + # request permanently unmergeable. + gate: + name: Azure template gate + needs: [ changes, validate ] + if: always() + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - name: Report the aggregate result + env: + CHANGES: ${{ needs.changes.result }} + VALIDATE: ${{ needs.validate.result }} + BUILD: ${{ needs.changes.outputs.build }} + run: | + echo "changes=$CHANGES build=$BUILD validate=$VALIDATE" + if [ "$CHANGES" != "success" ]; then + echo "::error::the changes job did not succeed ($CHANGES), so nothing here knows whether the checks were needed." + exit 1 + fi + case "$VALIDATE" in + success) + echo "Azure template CI passed." + ;; + skipped) + if [ "$BUILD" = "true" ]; then + echo "::error::the build paths changed but validate was skipped - the guard expression on that job is wrong." + exit 1 + fi + echo "No relevant path changed; Azure template CI had nothing to run." + ;; + *) + echo "::error::validate did not pass ($VALIDATE)." + exit 1 + ;; + esac diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d871c61..93ad233e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,41 +1,20 @@ name: CI on: + # No `paths:` filter here, on purpose (issue #632). A trigger-level + # filter means the workflow never STARTS on a change it does not match, + # and a workflow that never starts reports no check run at all - so + # requiring green CI in the branch ruleset would have left every + # docs-only pull request pending forever on a check nothing would ever + # report. The filter moved down a level, to + # .github/path-filters/ci.paths, which the `changes` job below reads: + # the workflow always starts, the expensive `native`/`vm` jobs are + # skipped when nothing relevant changed, and the `gate` job at the + # bottom reports success or failure either way. `gate` is the check to + # require. push: branches: [ master ] - paths: &build-paths - - '**.nix' - # Sources of the generated modules/agent-box.nix (issue #140): a change - # here can reassemble to the same bytes (a pure refactor), so it would - # not touch a *.nix file — match them explicitly so the module-generated- - # up-to-date guard (and the rest of CI) still runs. - - 'modules/agent-box.nix.in' - - 'modules/src/**' - - 'bin/assemble-module.py' - # Native test/validator edits must trigger the discovered checks too. - - 'tests/**' - - 'scripts/**' - # Every workflow, not just this one: CI validates no other workflow's - # YAML, so a file that cannot parse is simply never run and nothing - # says so. PR #483 shipped exactly that — vendor-updates.yml had a - # heredoc dedented out of its `run: |` block scalar, CI stayed green, - # and the weekly job would have silently never fired. - - '.github/workflows/**' - # The native-render fixture (issue #154 Phase 4): the renderer under - # test, the test itself, and its committed expected/ snapshot must all - # re-run agentbox-render when any one changes, or a stale fixture only - # surfaces via a manual dispatch (as it did for PR #381). - - 'bin/agentbox' - # The mascot mark (issue #185) is embedded into the settings daemon at - # assemble time, so it is a module source as much as a website asset. - - 'docs/potato.svg' - # The golden behavior snapshot (issue #154): the fixture and its - # renderer must re-run the golden-snapshot check when either changes. - - 'bin/golden-snapshot.py' - - 'flake.lock' - - '.github/workflows/ci.yml' pull_request: - paths: *build-paths workflow_dispatch: concurrency: @@ -49,8 +28,43 @@ permissions: contents: read jobs: + # Cheap, always runs, and decides for the expensive jobs below. Its own + # failure is a gate failure: "we could not work out whether the checks + # were needed" must never read as "the checks passed". + changes: + name: Decide whether CI's build paths changed + runs-on: ubuntu-latest + timeout-minutes: 5 + outputs: + build: ${{ steps.filter.outputs.build }} + steps: + - uses: actions/checkout@v5 + with: + # Both ends of the range have to be IN the clone. The default + # depth-1 checkout has neither a pull request's base nor a + # push's `before`, and changed_paths.py answers "true" rather + # than guessing when the range is unreadable - correct, but it + # would run the whole VM suite on every docs commit. + fetch-depth: 0 + + - name: Match the changed paths against the filter + id: filter + env: + # HEAD for both events: on a pull request actions/checkout leaves + # the MERGE commit checked out, whose merge base with base.sha is + # base.sha itself, so `base...HEAD` is exactly the pull request's + # own diff without needing the head commit to be present. + BASE: ${{ github.event_name == 'pull_request' && github.event.pull_request.base.sha || github.event.before }} + run: | + answer=$(python3 scripts/changed_paths.py \ + .github/path-filters/ci.paths --base "$BASE" --head HEAD) + echo "build=$answer" >> "$GITHUB_OUTPUT" + echo "::notice::CI build paths changed: $answer" + native: name: Native checks + needs: changes + if: needs.changes.outputs.build == 'true' runs-on: ubuntu-latest timeout-minutes: 15 steps: @@ -69,6 +83,8 @@ jobs: vm: name: VM (${{ matrix.lane }}) + needs: changes + if: needs.changes.outputs.build == 'true' runs-on: ubuntu-latest timeout-minutes: 25 strategy: @@ -109,20 +125,58 @@ jobs: timeout-minutes: 12 run: bash scripts/ci-vm-tests.sh ci-drivers ${{ matrix.jobs }} - validate: - # Preserve the existing status name and fail closed on failure, skip or - # cancellation in ANY dependency. Measure the full workflow critical - # path when assessing #519, not this short aggregation job in isolation. - name: Validate module & VM - needs: [native, vm] - if: ${{ always() }} + # THE check to require in the branch ruleset (issue #632): reported on + # every push and every pull request, whatever paths they touch, so it + # can never be the required-but-never-reported check that makes a pull + # request unmergeable. It reports what actually happened rather than + # what ran - including the case a plain "did native/vm pass?" would get + # wrong, where the guard expression skipped the job even though the + # paths did change. + gate: + name: CI gate + needs: [ changes, native, vm ] + if: always() runs-on: ubuntu-latest - timeout-minutes: 2 + timeout-minutes: 5 steps: - - name: Require every validation job to pass + - name: Report the aggregate CI result env: - NATIVE_RESULT: ${{ needs.native.result }} - VM_RESULT: ${{ needs.vm.result }} + CHANGES: ${{ needs.changes.result }} + NATIVE: ${{ needs.native.result }} + VM: ${{ needs.vm.result }} + BUILD: ${{ needs.changes.outputs.build }} run: | - echo "Native checks: $NATIVE_RESULT; VM lanes: $VM_RESULT" - [[ "$NATIVE_RESULT" == success && "$VM_RESULT" == success ]] + echo "changes=$CHANGES build=$BUILD native=$NATIVE vm=$VM" + if [ "$CHANGES" != "success" ]; then + echo "::error::the changes job did not succeed ($CHANGES), so nothing here knows whether CI was needed." + exit 1 + fi + + check_job() { + local name=$1 result=$2 + case "$result" in + success) + return 0 + ;; + skipped) + if [ "$BUILD" = "true" ]; then + echo "::error::the build paths changed but $name was skipped - the guard expression on that job is wrong." + exit 1 + fi + return 0 + ;; + *) + echo "::error::$name did not pass ($result)." + exit 1 + ;; + esac + } + + check_job native "$NATIVE" + check_job vm "$VM" + + if [ "$BUILD" = "true" ]; then + echo "CI passed." + else + echo "No build-relevant path changed; CI had nothing to run." + fi diff --git a/.github/workflows/deploy-test.yml b/.github/workflows/deploy-test.yml index 54c15e45..53b25e13 100644 --- a/.github/workflows/deploy-test.yml +++ b/.github/workflows/deploy-test.yml @@ -8,6 +8,57 @@ on: - 'modules/**' - 'scripts/ws_smoke.py' - '.github/workflows/deploy-test.yml' + # Called by promote.yml, so that promotion tests the EXACT candidate it + # is about to publish (issue #632). Everything the manifest pins arrives + # here as an input instead of being resolved again: `ref` is the + # candidate commit, and the four pin inputs are the identities + # release_manifest.py recorded once. A run with them set boots the same + # box the 1-click templates will create - which the push-triggered runs + # below deliberately do NOT, since a source-template launch is supposed + # to track the default branch and the base channel. + # + # Note which leg decides: ipv6-outputs is `continue-on-error` until + # issue #190 is diagnosed, so a caller waiting on this workflow is + # waiting on the ipv4-full leg. + workflow_call: + inputs: + ref: + description: "Commit to test (default: the caller's sha)" + type: string + default: '' + agent_box_rev: + description: AgentBoxRev from the candidate's release manifest + type: string + default: '' + agent_box_sha256: + description: AgentBoxSha256 from the candidate's release manifest + type: string + default: '' + agent_nixpkgs_url: + description: AgentNixpkgsUrl from the candidate's release manifest + type: string + default: '' + agent_nixpkgs_sha256: + description: AgentNixpkgsSha256 from the candidate's release manifest + type: string + default: '' + region: + type: string + default: us-west-2 + instance_type: + type: string + default: t4g.small (2 vCPU / 2 GiB) + destroy: + type: boolean + default: true + use_spot: + type: boolean + default: false + web_password: + # Declared so the shared steps below can read it on either path; + # promotion never sets it. + type: string + default: '' workflow_dispatch: inputs: region: @@ -112,6 +163,11 @@ jobs: fi - uses: actions/checkout@v5 + with: + # Empty means "whatever triggered this run", which is what the + # push and dispatch paths want. promote.yml passes the candidate + # commit, so the tree tested is the tree promoted. + ref: ${{ inputs.ref }} - name: Configure AWS credentials via OIDC uses: aws-actions/configure-aws-credentials@v4 @@ -121,6 +177,13 @@ jobs: - name: Generate WebPassword id: pw + # inputs.web_password reaches this step as an env var, not + # interpolated into the script text: a dispatch caller controls + # that value, and splicing it directly into `run:` would let a + # single quote break out of the assignment and run arbitrary shell + # on a runner that just assumed the AWS OIDC role above. + env: + WEB_PASSWORD_OVERRIDE: ${{ inputs.web_password }} run: | # 48 hex chars by default. Dispatch runs may override the plaintext # to reproduce shape-specific auth bugs; the override is @@ -131,8 +194,14 @@ jobs: # plaintext itself for its later login smoke tests, so both outputs # are kept. Caddy's own image computes the hash so this step needs # no new toolchain beyond the Docker the runner already has. - if [ -n '${{ inputs.web_password }}' ]; then - pw='${{ inputs.web_password }}' + if [ -n "$WEB_PASSWORD_OVERRIDE" ]; then + case "$WEB_PASSWORD_OVERRIDE" in + *$'\n'*|*$'\r'*) + echo "::error::web_password must be a single line." + exit 1 + ;; + esac + pw="$WEB_PASSWORD_OVERRIDE" echo "::notice::Using dispatch-provided WebPassword override." else pw="$(openssl rand -hex 24)" @@ -142,19 +211,58 @@ jobs: | docker run --rm -i caddy:2 caddy hash-password --algorithm argon2id)" echo "hash=$hash" >> "$GITHUB_OUTPUT" - - name: Compute module pin (AgentBoxRev + AgentBoxSha256) + - name: Resolve the pins this run tests id: pin - # The source template intentionally has no Default: for these — see - # publish-template.yml for the same computation on the S3 path. Here - # we pin to the commit that triggered this run so deploy-test always - # exercises the module content that just landed. + # Two callers, two meanings, and the difference is the point of + # issue #632. + # + # A push or a dispatch has no manifest: pin AgentBoxRev/Sha256 to + # the commit that triggered the run and leave the nixpkgs pair + # EMPTY, so the box tracks the base channel exactly as a + # source-template launch does. That is the early-warning run. + # + # promote.yml passes the candidate's recorded identities. Then all + # four are pinned, and the box this test boots is pinned to the + # same source AND the same dependency set the published templates + # will carry - which is what "run the deployment tests against + # THAT candidate" has to mean. Before this, deploy-test left the + # channel unpinned while publish-template re-resolved it, so the + # tested box and the 1-click box were never the same artifact. + env: + IN_REV: ${{ inputs.agent_box_rev }} + IN_SHA: ${{ inputs.agent_box_sha256 }} + IN_NIXPKGS_URL: ${{ inputs.agent_nixpkgs_url }} + IN_NIXPKGS_SHA: ${{ inputs.agent_nixpkgs_sha256 }} run: | - rev="${{ github.sha }}" - sha="sha256-$(curl -sSfL "https://raw.githubusercontent.com/${{ github.repository }}/${rev}/modules/agent-box.nix" \ - | openssl dgst -sha256 -binary | base64)" - echo "rev=$rev" >> "$GITHUB_OUTPUT" - echo "sha=$sha" >> "$GITHUB_OUTPUT" - echo "::notice::Pinned AgentBoxRev=$rev AgentBoxSha256=$sha" + if [ -n "$IN_REV" ]; then + if [ -z "$IN_SHA" ]; then + echo "::error::agent_box_rev was given without agent_box_sha256 - an unpinned pair would silently fetch a different module." + exit 1 + fi + if { [ -n "$IN_NIXPKGS_URL" ] && [ -z "$IN_NIXPKGS_SHA" ]; } \ + || { [ -z "$IN_NIXPKGS_URL" ] && [ -n "$IN_NIXPKGS_SHA" ]; }; then + echo "::error::agent_nixpkgs_url and agent_nixpkgs_sha256 must be given together - the template requires the pair or neither." + exit 1 + fi + rev="$IN_REV" + sha="$IN_SHA" + nixpkgs_url="$IN_NIXPKGS_URL" + nixpkgs_sha="$IN_NIXPKGS_SHA" + echo "::notice::Testing the promotion candidate's recorded pins." + else + rev="${{ github.sha }}" + sha="sha256-$(curl -sSfL "https://raw.githubusercontent.com/${{ github.repository }}/${rev}/modules/agent-box.nix" \ + | openssl dgst -sha256 -binary | base64)" + nixpkgs_url="" + nixpkgs_sha="" + fi + { + echo "rev=$rev" + echo "sha=$sha" + echo "nixpkgs_url=$nixpkgs_url" + echo "nixpkgs_sha=$nixpkgs_sha" + } >> "$GITHUB_OUTPUT" + echo "::notice::Pinned AgentBoxRev=$rev AgentBoxSha256=$sha AgentNixpkgsUrl=${nixpkgs_url:-(base channel)}" - name: Create stack id: create @@ -201,6 +309,8 @@ jobs: "ParameterKey=EnableSsm,ParameterValue=true" \ "ParameterKey=AgentBoxRev,ParameterValue=${{ steps.pin.outputs.rev }}" \ "ParameterKey=AgentBoxSha256,ParameterValue=${{ steps.pin.outputs.sha }}" \ + "ParameterKey=AgentNixpkgsUrl,ParameterValue=${{ steps.pin.outputs.nixpkgs_url }}" \ + "ParameterKey=AgentNixpkgsSha256,ParameterValue=${{ steps.pin.outputs.nixpkgs_sha }}" \ --on-failure DO_NOTHING \ --capabilities CAPABILITY_IAM \ --tags "Key=Purpose,Value=agent-box-e2e" "Key=RunId,Value=${{ github.run_id }}" diff --git a/.github/workflows/promote.yml b/.github/workflows/promote.yml new file mode 100644 index 00000000..9359d80e --- /dev/null +++ b/.github/workflows/promote.yml @@ -0,0 +1,448 @@ +name: Promote a release candidate + +# Explicit promotion (issue #632). Before this, "release" was whatever had +# most recently been pushed to master: publish-template.yml ran on every +# push, independently of CI and of the fresh-boot deployment test, and it +# re-resolved the nixos-unstable channel at publish time - so the box the +# public 1-click templates create was never the box anything had booted. +# +# This workflow is the promotion, and it is the only path to the public +# defaults. In order, and stopping at the first failure: +# +# 1. resolve the candidate to a full commit sha; +# 2. refuse it unless every aggregate CI gate is green FOR THAT SHA; +# 3. build its release manifest ONCE (scripts/release_manifest.py); +# 4. boot it in deploy-test with the manifest's own pins; +# 5. publish the same manifest's pins into the S3 templates; +# 6. record the promotion: a release-* tag, a GitHub Release carrying the +# manifest, and the `release` branch pointer boxes can follow. +# +# A failure anywhere before 5 leaves the public defaults exactly as they +# were, which is the acceptance criterion this exists for. +# +# ROLLBACK: dispatch this again with `sha` set to an earlier promoted +# commit and `allow_rollback` on. That candidate is already tested and +# already tagged, so nothing new is tagged; its manifest is rebuilt from +# its own immutable rev, republished, and the `release` pointer is moved +# back. `skip_deploy_test` saves the 40 minutes when the point is to undo a +# bad release quickly - only ever use it for a commit that was promoted +# before. + +on: + workflow_dispatch: + inputs: + sha: + description: >- + Candidate commit (full or short sha, tag or branch; empty = the + tip of the ref this run was dispatched on) + type: string + default: '' + skip_deploy_test: + description: >- + Skip the fresh-boot deployment test. ONLY for re-promoting a + commit that already passed it - a rollback. + type: boolean + default: false + allow_rollback: + description: >- + Allow the `release` pointer to move to a commit that is not a + descendant of where it is now. + type: boolean + default: false + reason: + description: Why this candidate is being promoted (goes in the release notes) + type: string + default: '' + +concurrency: + # One promotion at a time, and never cancelled halfway: the phases move + # public state. + group: promote + cancel-in-progress: false + +permissions: + contents: read + +jobs: + candidate: + name: Resolve and gate the candidate + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + checks: read + outputs: + rev: ${{ steps.rev.outputs.rev }} + module_sha256: ${{ steps.pins.outputs.module_sha256 }} + agent_nixpkgs_url: ${{ steps.pins.outputs.agent_nixpkgs_url }} + agent_nixpkgs_sha256: ${{ steps.pins.outputs.agent_nixpkgs_sha256 }} + steps: + - uses: actions/checkout@v5 + with: + # All of it: the candidate may be older than the tip, and the + # record job needs the tags to tell a first promotion from a + # rollback. + fetch-depth: 0 + + - name: Resolve the candidate to a full commit sha + id: rev + env: + WANT: ${{ inputs.sha }} + run: | + if [ -n "$WANT" ]; then + rev=$(git rev-parse --verify "$WANT^{commit}") || { + echo "::error::$WANT does not name a commit in this repository." + exit 1 + } + else + rev=$(git rev-parse HEAD) + fi + echo "rev=$rev" >> "$GITHUB_OUTPUT" + echo "::notice::Candidate is $rev" + + - name: Refuse an unauthorized rollback before any public write + # Preflight, not just the final push guard in the `record` job: that + # guard only fires AFTER publish has already replaced the public S3 + # templates and the Release has been written, which can roll public + # installs backward and then fail with the release branch already + # moved past them. Checking ancestry here, before deploy-test or + # publish ever run, means an unauthorized rollback never reaches a + # public write in the first place. The later push is kept as the + # concurrency-safe final check - two promotions racing each other is + # what THAT guards against, not this. + env: + REV: ${{ steps.rev.outputs.rev }} + ALLOW_ROLLBACK: ${{ inputs.allow_rollback }} + run: | + git fetch origin release:refs/remotes/origin/release 2>/dev/null || true + if ! git rev-parse --verify --quiet origin/release >/dev/null; then + echo "::notice::No release branch yet - nothing to fast-forward from." + exit 0 + fi + if [ "$ALLOW_ROLLBACK" = "true" ]; then + echo "::notice::allow_rollback is set - skipping the fast-forward check." + exit 0 + fi + if git merge-base --is-ancestor origin/release "$REV"; then + echo "::notice::$REV is a fast-forward of the release branch." + else + echo "::error::$REV is not a fast-forward of the release branch - refusing before any public write. Dispatch again with allow_rollback for a deliberate rollback." + exit 1 + fi + + - name: Require every aggregate CI gate to be green for that sha + env: + GH_TOKEN: ${{ github.token }} + REV: ${{ steps.rev.outputs.rev }} + # The gates are the always-reported terminal jobs of ci.yml, + # aws-ci.yml and azure-ci.yml. Requiring them HERE and not only in + # the branch ruleset matters: a ruleset governs the merge, and an + # administrator can bypass one, while nothing reaches the public + # install default without passing through this step. + # + # "Absent" is a failure, not a pass. That is the whole reason the + # gates exist as separate always-running jobs: a path-filtered + # check that never ran reports nothing at all, and treating silence + # as success is how an untested candidate would be promoted. + run: | + gh api --paginate --slurp \ + "repos/${GITHUB_REPOSITORY}/commits/${REV}/check-runs?per_page=100" \ + | jq '[.[].check_runs[]]' > runs.json + echo "Check runs reported for ${REV}:" + jq -r 'sort_by(.name)[] | " \(.name): \(.status)/\(.conclusion // "none")"' runs.json + failed=0 + for want in "CI gate" "AWS template gate" "Azure template gate"; do + verdict=$(jq -r --arg n "$want" ' + map(select(.name == $n)) | sort_by(.started_at) | last + | if . == null then "absent" + else "\(.status)/\(.conclusion // "none")" end' runs.json) + if [ "$verdict" = "completed/success" ]; then + echo "ok: $want" + else + echo "::error::$want is $verdict for ${REV} - refusing to promote." + failed=1 + fi + done + [ "$failed" -eq 0 ] + + - uses: cachix/install-nix-action@v31 + + - name: Check out the candidate into its own directory + # The top-level checkout above is fetch-depth 0 so REV can be + # resolved from anywhere in history, but its WORKING TREE still + # sits at whatever ref this run was dispatched on - not at REV + # unless the two happen to coincide. Hashing "." here would hash + # the dispatch ref's files while claiming they were REV's - and + # release_manifest.py's own remote check catches the mismatch and + # fails loudly rather than promoting the wrong tree, but a + # candidate other than the tip could then never be promoted at + # all. A separate worktree gives an on-disk copy of exactly REV, + # while scripts/release_manifest.py itself keeps running from the + # trusted ref this workflow was defined on. + env: + REV: ${{ steps.rev.outputs.rev }} + run: git worktree add --detach _candidate "$REV" + + - name: Decide whether this is a fresh candidate or a re-promotion + id: rollback + # skip_deploy_test is only ever valid for a commit that already + # passed the fresh-boot test - i.e. one that was promoted before + # and carries a release-* tag. Refusing it here (rather than + # discovering it later, or worse, silently rebuilding a manifest + # for a commit that was never boot-tested) is what closes the gap + # a manual dispatch of publish-template.yml shares: neither path + # may publish an untested candidate. + env: + REV: ${{ steps.rev.outputs.rev }} + SKIP_DEPLOY_TEST: ${{ inputs.skip_deploy_test }} + run: | + existing=$(git tag --points-at "$REV" --list 'release-*' | head -n 1) + if [ "$SKIP_DEPLOY_TEST" = "true" ] && [ -z "$existing" ]; then + echo "::error::skip_deploy_test was requested for $REV, but it carries no release-* tag - it was never promoted before, so there is no prior fresh-boot result to reuse. Only dispatch skip_deploy_test for a commit that already passed deploy-test (a rollback)." + exit 1 + fi + echo "existing_tag=$existing" >> "$GITHUB_OUTPUT" + + - name: Reuse the original manifest for a rollback + # A rollback must not re-derive anything: build() would resolve + # the nixos-unstable channel again, and the same source sha can + # acquire dependency pins that were never tested (a rebuild of one + # sha, hours apart, was proven to change agent_nixpkgs). The + # manifest this commit was ORIGINALLY promoted with is the + # immutable record of what was tested, so a rollback fetches it + # back from its Release and verifies it still describes REV rather + # than building a new one. + if: steps.rollback.outputs.existing_tag != '' + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.rollback.outputs.existing_tag }} + REV: ${{ steps.rev.outputs.rev }} + run: | + gh release download "$TAG" --repo "$GITHUB_REPOSITORY" \ + --pattern release-manifest.json --clobber + python3 scripts/release_manifest.py verify release-manifest.json \ + --rev "$REV" --source-dir _candidate + + - name: Build the candidate's release manifest, once + # Once, from one commit, and every phase after this reads it + # instead of resolving anything again. --source-dir with the remote + # check on: the hashes recorded are the checked-out candidate's, and + # the tree is proven to be byte-identical to what GitHub serves at + # that rev, which is where a launching box fetches the module from. + if: steps.rollback.outputs.existing_tag == '' + env: + REV: ${{ steps.rev.outputs.rev }} + run: | + python3 scripts/release_manifest.py build \ + --repo "$GITHUB_REPOSITORY" --rev "$REV" \ + --source-dir _candidate \ + --created-by "promote.yml run ${GITHUB_RUN_ID}" \ + --out release-manifest.json + + - name: Read the pins the deployment test has to use + id: pins + run: | + m() { python3 scripts/release_manifest.py field release-manifest.json "$1"; } + # Assigned first, and deliberately not `echo "k=$(m ...)"`. Under + # `bash -e` the status of that echo is echo's own, so a failing + # `field` would be discarded and an EMPTY pin written to + # GITHUB_OUTPUT - which deploy-test then reads as "base channel" + # and boots an unpinned box, the exact divergence this workflow + # exists to prevent. An assignment carries the substitution's + # status, so it fails the step. + module_sha256=$(m module_sha256) + agent_nixpkgs_url=$(m agent_nixpkgs.url) + agent_nixpkgs_sha256=$(m agent_nixpkgs.sha256) + { + echo "module_sha256=$module_sha256" + echo "agent_nixpkgs_url=$agent_nixpkgs_url" + echo "agent_nixpkgs_sha256=$agent_nixpkgs_sha256" + } >> "$GITHUB_OUTPUT" + python3 scripts/release_manifest.py show release-manifest.json \ + | tee -a "$GITHUB_STEP_SUMMARY" + + - name: Hand the manifest to the phases that follow + uses: actions/upload-artifact@v4 + with: + name: release-manifest + path: release-manifest.json + if-no-files-found: error + + # Phase 4: the same candidate, booted from nothing but its manifest's + # identities. The reusable workflow's DEFINITION comes from the ref this + # run was dispatched on (a local `uses:` cannot be pinned to another + # ref); the tree it deploys is the candidate, via `ref`. + deploy-test: + name: Fresh-boot the candidate + needs: candidate + if: ${{ !inputs.skip_deploy_test }} + uses: ./.github/workflows/deploy-test.yml + secrets: inherit + permissions: + id-token: write + contents: read + with: + ref: ${{ needs.candidate.outputs.rev }} + agent_box_rev: ${{ needs.candidate.outputs.rev }} + agent_box_sha256: ${{ needs.candidate.outputs.module_sha256 }} + agent_nixpkgs_url: ${{ needs.candidate.outputs.agent_nixpkgs_url }} + agent_nixpkgs_sha256: ${{ needs.candidate.outputs.agent_nixpkgs_sha256 }} + + # Phase 5: the first step that changes anything the public can see. + publish: + name: Publish the promoted templates + needs: [ candidate, deploy-test ] + # `always()` is needed to run at all when deploy-test was skipped by + # input - and then every acceptable state has to be spelled out, or + # `always()` would also run this after a FAILED deployment test, which + # is precisely the thing that must leave the public defaults alone. + if: >- + always() + && needs.candidate.result == 'success' + && (needs.deploy-test.result == 'success' + || (needs.deploy-test.result == 'skipped' && inputs.skip_deploy_test)) + uses: ./.github/workflows/publish-template.yml + secrets: inherit + permissions: + id-token: write + contents: read + with: + ref: ${{ needs.candidate.outputs.rev }} + manifest_artifact: release-manifest + + record: + name: Record the promotion + needs: [ candidate, publish ] + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + # The tag, the Release, and the `release` branch pointer. + contents: write + steps: + - uses: actions/checkout@v5 + with: + ref: ${{ needs.candidate.outputs.rev }} + fetch-depth: 0 + + - uses: actions/download-artifact@v4 + with: + name: release-manifest + + - name: Verify the manifest still describes this commit + # Cheap, and it closes the window: the manifest was built in the + # first job and the tree has been checked out again since. Anything + # that does not match means the artifact and the tag would disagree + # about what was promoted. + # + # AGENT_BOX_SKIP_PREFETCH because this job installs no Nix, so + # `nix-prefetch-url` is not here to re-hash the channel tarball - + # and the publish job, which does have it, already ran the full + # verification including that hash before anything was uploaded. + # Every other identity is still recomputed here. + env: + AGENT_BOX_SKIP_PREFETCH: "1" + run: | + python3 scripts/release_manifest.py verify release-manifest.json \ + --rev "${{ needs.candidate.outputs.rev }}" --source-dir . + + - name: Tag the release, or recognise a rollback + id: tag + env: + GH_TOKEN: ${{ github.token }} + REV: ${{ needs.candidate.outputs.rev }} + REASON: ${{ inputs.reason }} + run: | + existing=$(git tag --points-at "$REV" --list 'release-*' | head -n 1) + if [ -n "$existing" ]; then + # A commit is promoted at most once. Re-promoting one is a + # rollback, and it keeps the tag it already has - a second tag + # on the same commit would make the history of what was public + # unreadable. + echo "::notice::$REV is already tagged $existing - re-promoting it." + echo "tag=$existing" >> "$GITHUB_OUTPUT" + echo "created=false" >> "$GITHUB_OUTPUT" + exit 0 + fi + tag="release-$(date -u +%Y%m%d-%H%M%S)-$(git rev-parse --short=7 "$REV")" + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git tag -a "$tag" "$REV" -m "agent-box $tag${REASON:+ - }${REASON}" + git push origin "refs/tags/$tag" + echo "tag=$tag" >> "$GITHUB_OUTPUT" + echo "created=true" >> "$GITHUB_OUTPUT" + echo "::notice::Tagged $REV as $tag" + + - name: Publish the Release object carrying the manifest + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.tag.outputs.tag }} + REV: ${{ needs.candidate.outputs.rev }} + REASON: ${{ inputs.reason }} + run: | + { + echo "Promoted \`$REV\`." + echo + if [ -n "$REASON" ]; then + echo "$REASON" + echo + fi + echo "The attached \`release-manifest.json\` is the identity of" + echo "this release: the source rev, the module hash the EC2" + echo "template fetches, the flake ref the Lightsail template" + echo "installs, the flake.lock hash, and the resolved nixpkgs" + echo "channel snapshot. The published 1-click templates carry" + echo "exactly these pins, and the deployment test booted them." + echo + echo "Verify any of it with:" + echo '```' + echo "python3 scripts/release_manifest.py verify release-manifest.json --rev $REV" + echo '```' + } > notes.md + if gh release view "$TAG" >/dev/null 2>&1; then + gh release edit "$TAG" --notes-file notes.md + gh release upload "$TAG" release-manifest.json --clobber + else + gh release create "$TAG" release-manifest.json \ + --title "agent-box $TAG" --notes-file notes.md + fi + + - name: Move the `release` pointer + env: + REV: ${{ needs.candidate.outputs.rev }} + ALLOW_ROLLBACK: ${{ inputs.allow_rollback }} + # The tag and the Release are the record; this branch is the thing a + # box can FOLLOW. `agentbox update --branch release` and the NixOS + # module's selfUpdate.branch both take a branch name, and + # agent-box-source's fast-forward guard is written around one - so a + # box tracking `release` moves from tested release to tested + # release and never through an untested tip. + # + # Fast-forward by default. A rollback is a real backwards move and + # has to be asked for by name, because it is the one push here that + # can lose a commit from the branch. + run: | + if [ "$ALLOW_ROLLBACK" = "true" ]; then + git push --force origin "$REV:refs/heads/release" + echo "::warning::\`release\` was FORCE-moved to $REV (rollback)." + elif ! git push origin "$REV:refs/heads/release"; then + echo "::error::$REV is not a fast-forward of \`release\`. If this is a deliberate rollback, dispatch again with allow_rollback." + exit 1 + fi + + - name: Summarise what is now public + env: + TAG: ${{ steps.tag.outputs.tag }} + REV: ${{ needs.candidate.outputs.rev }} + run: | + { + echo "## Promoted" + echo + echo "| | |" + echo "|---|---|" + echo "| tag | \`$TAG\` |" + echo "| rev | \`$REV\` |" + echo "| deployment test | ${{ inputs.skip_deploy_test && 'SKIPPED (re-promotion)' || 'passed' }} |" + echo "| \`release\` branch | \`$REV\` |" + echo + python3 scripts/release_manifest.py show release-manifest.json + } >> "$GITHUB_STEP_SUMMARY" diff --git a/.github/workflows/publish-template.yml b/.github/workflows/publish-template.yml index 6365b16f..a439e03b 100644 --- a/.github/workflows/publish-template.yml +++ b/.github/workflows/publish-template.yml @@ -1,19 +1,52 @@ name: Publish CFN template to S3 on: - # No `paths:` filter. The published templates embed AgentBoxRev/ - # AgentBoxSha256 pinned to the commit that ran this workflow (see the - # "Inject pinned defaults" step below), so ANY push to master makes that - # pin stale, not just a push touching the template files themselves. A - # `paths:` filter here previously let master drift 30+ commits ahead of - # the S3-published templates before anything touched the two YAML files - # again — a freshly-launched box came up already 30 commits behind - # (issue #408). deploy-test never caught this: it always computes its - # own fresh pin from the triggering commit instead of reading the - # published template. - push: - branches: [ master ] + # No `push:` trigger any more (issue #632). + # + # It used to publish on EVERY push to master, deliberately without a + # `paths:` filter, because the published copies embed AgentBoxRev/ + # AgentBoxSha256 pinned to the commit that ran the workflow - so any push + # made an older pin stale, and a filter once let master drift 30+ commits + # ahead of S3 (issue #408). + # + # That fixed the staleness and left the real problem: publishing ran + # independently of CI and of the fresh-boot deployment test, so whatever + # landed on master became the public install default within minutes, + # tested or not. And it re-resolved the nixos-unstable channel HERE, at + # publish time, which meant the dependency set in the published template + # had never been booted by anything. + # + # So publishing is no longer a consequence of pushing. It is the last + # phase of promote.yml, which builds a candidate's release manifest once, + # proves the aggregate CI gates are green for that exact commit, boots it + # in deploy-test with THOSE pins, and only then calls this workflow with + # the same manifest. Nothing here resolves anything; it injects what the + # manifest recorded. + # + # Issue #408's failure cannot come back, because the pin and the template + # are now always the same candidate's. What CAN happen is master running + # ahead of the last promoted release - and that is the intent: a 1-click + # launch gets the release that was tested, not the tip that was not. + workflow_call: + inputs: + ref: + description: The candidate commit to publish + type: string + required: true + manifest_artifact: + description: Run artifact holding the candidate's release-manifest.json + type: string + default: release-manifest workflow_dispatch: + inputs: + ref: + description: >- + Commit, tag or branch to publish (empty = the ref this run was + dispatched on). A manual dispatch builds the manifest here rather + than receiving one, so use it to re-publish an existing release + tag; for a new release use promote.yml, which tests first. + type: string + default: '' concurrency: group: publish-template @@ -40,6 +73,9 @@ jobs: # step below. TEMPLATES: lightsail-template.yaml template.yaml ROLE_ARN: ${{ vars.AWS_ROLE_ARN }} + # Published beside them so anyone can read what the public default + # actually pins, without guessing from a Default: line. + MANIFEST: release-manifest.json steps: - name: Sanity-check required variables run: | @@ -52,20 +88,95 @@ jobs: fi - uses: actions/checkout@v5 + with: + ref: ${{ inputs.ref }} + # 0 so a manual dispatch (below) can see whether this commit + # already carries a release-* tag - a shallow clone has no tags + # to check. + fetch-depth: 0 - uses: cachix/install-nix-action@v31 + - name: Resolve the commit being published + id: rev + run: | + rev=$(git rev-parse HEAD) + echo "rev=$rev" >> "$GITHUB_OUTPUT" + echo "::notice::Publishing $rev" + + # A manual dispatch has no upstream candidate: nothing here runs CI + # or boots a box, so the ONLY thing that makes publishing it safe is + # that promote.yml already did those things for this exact commit. + # That is what a release-* tag records. Without this check, a manual + # dispatch on master (or any ref) reaches the S3 upload having + # checked nothing - the comment on the input used to say "for + # re-publishing" while the code let it publish anything. + - name: Refuse a manual publish of a commit that was never promoted + if: inputs.manifest_artifact == '' + env: + REV: ${{ steps.rev.outputs.rev }} + run: | + existing=$(git tag --points-at "$REV" --list 'release-*' | head -n 1) + if [ -z "$existing" ]; then + echo "::error::$REV carries no release-* tag - it was never promoted through promote.yml, which is the only path that runs the CI gates and the fresh-boot deployment test. A manual dispatch here only re-publishes an already-promoted release; to promote something new, run promote.yml instead." + exit 1 + fi + echo "::notice::$REV is tagged $existing - re-publishing an already-promoted release." + + # promote.yml already built this, from one commit, before the + # deployment test booted it. Downloading it here is what makes the + # published template the SAME artifact that was tested rather than a + # freshly re-resolved lookalike. + - name: Fetch the candidate's release manifest + # Keyed on the input, NOT on github.event_name: inside a called + # workflow that name is the CALLER's event, so a promote.yml run + # (itself a workflow_dispatch) would take the manual branch below + # and quietly rebuild the manifest it was handed. + if: inputs.manifest_artifact != '' + uses: actions/download-artifact@v4 + with: + name: ${{ inputs.manifest_artifact }} + + # A manual dispatch has no upstream candidate, so it builds one. That + # is fine for re-publishing an existing release tag (the identities + # are recomputed from that immutable rev) and is NOT a way to promote + # something new: nothing here runs CI or boots a box. + - name: Build a manifest for a manual publish + if: inputs.manifest_artifact == '' + run: | + python3 scripts/release_manifest.py build \ + --repo "${{ github.repository }}" \ + --rev "${{ steps.rev.outputs.rev }}" \ + --source-dir . \ + --created-by "publish-template.yml (manual dispatch)" \ + --out "$MANIFEST" + + # Refuses a manifest that describes another commit, or one whose + # recorded hashes no longer match the tree being published. Without + # this the download step above would happily inject one candidate's + # pins into another candidate's templates. + - name: Verify the manifest describes exactly this commit + run: | + python3 scripts/release_manifest.py verify "$MANIFEST" \ + --rev "${{ steps.rev.outputs.rev }}" --source-dir . + - name: Validate templates locally run: | pip install --quiet cfn-lint cfn-lint deploy/aws/template.yaml deploy/aws/lightsail-template.yaml - - name: Inject pinned defaults for the 1-click copies + - name: Inject the manifest's pins into the 1-click copies # The source templates deliberately ship without pinned values (see # the params' Descriptions): a source-template launch like - # deploy-test then tracks the default branch / the base channel. The - # 1-click Launch buttons want a reproducible box, so pinned values go - # into the S3 COPIES only. + # deploy-test's push runs then tracks the default branch / the base + # channel. The 1-click Launch buttons want a reproducible box, so + # pinned values go into the S3 COPIES only. + # + # Every value below comes out of the manifest. Nothing is computed + # here, which is the whole change: a curl of the module and a fresh + # channel redirect used to happen at this point, so the published + # template could differ from the tested one in the one input that + # was never immutable. # # The two templates pin agent-box by different means, so they get one # awk each: @@ -75,13 +186,15 @@ jobs: # lightsail-template.yaml installs the runtime profile from a # flake ref -> one Default. run: | - rev="${{ github.sha }}" - sha="sha256-$(curl -sSfL "https://raw.githubusercontent.com/${{ github.repository }}/${rev}/modules/agent-box.nix" \ - | openssl dgst -sha256 -binary | base64)" - agentRelease="$(curl -fsSLo /dev/null -w '%{url_effective}' https://channels.nixos.org/nixos-unstable)" - agentUrl="${agentRelease%/}/nixexprs.tar.xz" - agentSha="$(nix-prefetch-url --unpack "$agentUrl")" - flakeRef="github:${{ github.repository }}/${rev}" + # One value per call, and `field` exits non-zero on a missing or + # empty one - so an incomplete manifest fails here rather than + # injecting a blank `Default:` into a public template. + m() { python3 scripts/release_manifest.py field "$MANIFEST" "$1"; } + rev=$(m rev) + sha=$(m module_sha256) + agentUrl=$(m agent_nixpkgs.url) + agentSha=$(m agent_nixpkgs.sha256) + flakeRef=$(m flake_ref) echo "Rev: $rev" echo "Sha: $sha" echo "AgentUrl: $agentUrl" @@ -154,12 +267,13 @@ jobs: --public-access-block-configuration \ 'BlockPublicAcls=true,IgnorePublicAcls=true,BlockPublicPolicy=false,RestrictPublicBuckets=false' - # Public-read on the two template objects only — CloudFormation's - # Launch Stack UI requires an S3 URL for templateURL. The module - # itself is fetched by the box direct from raw.githubusercontent.com - # (dual-stack), so this bucket carries no other public assets. One - # policy covers both objects (a single PutBucketPolicy replaces the - # whole document, so the two templates must not fight over it). + # Public-read on the two template objects and the manifest that + # says what they pin — CloudFormation's Launch Stack UI requires an + # S3 URL for templateURL. The module itself is fetched by the box + # direct from raw.githubusercontent.com (dual-stack), so this + # bucket carries no other public assets. One policy covers all + # three objects (a single PutBucketPolicy replaces the whole + # document, so they must not fight over it). aws s3api put-bucket-policy --bucket "$BUCKET" --policy "$(cat <.checkout-bootstrap` runs `tests/test-checkout-bootstrap.sh` against `modules/src/checkout-cli.sh`, the script that puts this repo ON a deployed box (issue #242). Its assertions are mostly REFUSALS, because that is where the damage would be: it runs unattended at every supervisor start, in a tree sibling sessions are working in, so a realign that moved somebody's branch pointer would destroy work at boot on a box nobody is watching. `origin` is a local repository and `gh` is a shim, so there is no network and it runs natively on every architecture. Runnable without Nix too: `bash tests/test-checkout-bootstrap.sh modules/src/checkout-cli.sh`. - `nix build -L .#checks..source-tree` runs `tests/test-source-tree.sh` against `modules/src/source-tree.sh`, the tree the box is BUILT from and the whole of what an update moves (issue #242). Weighted at the refusals for the same reason: it runs as root, unattended, and the tree it leaves behind is what the next rebuild builds — so a rewritten history, a downgrade, and a baseline the tree has never heard of each get an assertion, as does the realign that makes the fast-forward guard measure ancestry from the rev the box is RUNNING. It also pins the two locks the trust boundary rests on: `check` answers from `git ls-remote` and so creates no tree and fetches into none, and git runs with `core.hooksPath` pointed at nothing, so a `post-checkout`/`post-merge` hook in the tree cannot run as root (that assertion has a negative control — remove the lock and it fails). `origin` is a local repository, so there is no network and it runs natively on every architecture. Runnable without Nix too: `bash tests/test-source-tree.sh modules/src/source-tree.sh`. - `nix build -L .#checks..checkout-options` is the eval regression for `selfUpdate`'s three path assertions (issue #242). It reads `config.assertions` rather than forcing `toplevel`, so a failure names WHICH assertion fired instead of only reporting that something did — and it asserts the accepting cases too, so an assertion that rejects everything fails it as loudly as one that rejects nothing. `selfUpdate.srcDir` is the one that matters most, because root BUILDS the box from that tree: it is confined to a normalized path under `/var/lib`, since owning the directory is not enough — a writable ancestor (`/home/agent/src`, `/tmp/src`) lets an agent swap the whole tree and choose what root builds. For `selfUpdate.checkout.path` the inputs that matter are the ones a first pass at "must be relative" lets through: `../agent-box` escapes the home, `.` and `""` collapse to `/home/` itself — which the agent unit's `ProtectSystem=strict` would refuse as EROFS inside a background job's journal — and `a//b`, which resolves to a perfectly ordinary child path and is refused for a different reason: every empty component is, because one is how the collapsing cases are spelled. -- `nix build -L .#checks..webhook-defer` runs `tests/test-webhook-defer.sh`: what `modules/src/webhook-spawn.sh` answers when shared session admission refuses a start, and what the pinned `webhook.py` does with that answer (issues #170, #301). A refusal is `exit 75` (`EX_TEMPFAIL`), the one code the dispatcher reads as "declined for now" rather than "this spawner is broken" -- every other code drops the batch, and a standing watch is for events NO session owns, so nothing else is holding them. The second half runs the REAL wrapper as the REAL Dispatcher's spawn command, fills the cap, frees a slot and asserts the declined batch starts by itself, because the bug was the two programs disagreeing about what a non-zero exit meant. Runnable without Nix too: `bash tests/test-webhook-defer.sh modules/src/webhook-spawn.sh /path/to/webhook.py` (the dispatcher half is skipped, and says so, when no `webhook.py` is given). +- `nix build -L .#checks..changed-paths` runs `tests/test-changed-paths.py`: the GitHub `paths` glob dialect as `scripts/changed_paths.py` reimplements it, the committed `.github/path-filters/*.paths` against the concrete paths whose bug histories put them in the list, and - the part nothing else can see - that no gated workflow has grown a trigger-level `paths:` again. That filter shape is what made green CI unrequireable (issue #632): a workflow that never starts reports no check run, so requiring it would block every docs-only PR forever. Runnable without Nix too: `python3 tests/test-changed-paths.py` (one case needs a git checkout and says so when it skips). +- `nix build -L .#checks..release-manifest` runs `tests/test-release-manifest.py` against `scripts/release_manifest.py`, the record of what a promoted candidate actually IS (issue #632). Weighted at the refusals: a manifest that verifies when it should not is how an untested artifact becomes the public install default, and it looks exactly like a pass - so a changed module, a changed template, a changed `flake.lock`, another rev, an unpinned channel and a `flake_ref` naming a different commit each get an assertion. Hermetic - no network, and `nix-prefetch-url` is a stub the test writes itself - so it runs natively on every architecture: `python3 tests/test-release-manifest.py`. +- `nix build -L .#checks..webhook-defer` runs `tests/test-webhook-defer.sh`: what `modules/src/webhook-spawn.sh` answers at the hook-session ceiling, and what the pinned `webhook.py` does with that answer (issues #170, #301). A refusal is `exit 75` (`EX_TEMPFAIL`), the one code the dispatcher reads as "declined for now" rather than "this spawner is broken" -- every other code drops the batch, and a standing watch is for events NO session owns, so nothing else is holding them. The second half runs the REAL wrapper as the REAL Dispatcher's spawn command, fills the cap, frees a slot and asserts the declined batch starts by itself, because the bug was the two programs disagreeing about what a non-zero exit meant. Runnable without Nix too: `bash tests/test-webhook-defer.sh modules/src/webhook-spawn.sh /path/to/webhook.py` (the dispatcher half is skipped, and says so, when no `webhook.py` is given). - `nix flake metadata` validates flake inputs and basic evaluation. - `nix build .#packages.x86_64-linux.vm` builds the bootable qcow2 image under `result/`. - `nix build -L .#checks..multi-user` runs the quick module/configuration assertion. @@ -451,6 +453,69 @@ one here has already cost at least one. correct it. Any time a run becomes a claim to somebody else, capture the status without a pipe first, then look at the output. +## Releases: promote a tested candidate, never the branch tip + +The public install default is not master. `publish-template.yml` used to run +on every push to master, independently of CI and of the fresh-boot deployment +test, and it re-resolved the nixos-unstable channel at publish time - so the +box a 1-click Launch button creates had never been booted by anything, and its +one mutable dependency was resolved after the last thing that could have +tested it (issue #632). + +Promotion is now explicit and is the only path to those defaults. +`promote.yml` (Actions > Promote a release candidate > Run workflow) does, in +order, stopping at the first failure: + +1. resolves the candidate to a full commit sha; +2. refuses it unless all three aggregate gates are `success` FOR THAT SHA - + and treats an ABSENT gate as a failure, which is the whole reason they are + separate always-running jobs; +3. builds `release-manifest.json` ONCE with `scripts/release_manifest.py`: + the rev, the SRI hash of `modules/agent-box.nix` that `template.yaml` + fetches, the flake ref `lightsail-template.yaml` installs, the `flake.lock` + hash, the resolved nixpkgs channel SNAPSHOT url and its hash, and a hash + per deployment template; +4. calls `deploy-test.yml` with those pins, so the box it boots is pinned to + the same source AND the same dependency set the published templates will + carry. Before this the two were never the same artifact: deploy-test left + `AgentNixpkgsUrl` empty and publish injected a pair it resolved itself; +5. calls `publish-template.yml` with the SAME manifest - which now computes + nothing and injects only what the manifest recorded, and uploads the + manifest to S3 beside the templates; +6. records it: a `release-*` tag, a GitHub Release carrying the manifest, and + the `release` branch pointer. + +So a failure anywhere before step 5 leaves the public defaults exactly as they +were. What master running ahead of the last promoted release now means is "the +tip has not been promoted yet", not issue #408's bug where the pin and the +template disagreed - those always come from one candidate now. + +**Nothing publishes on its own any more.** If the S3 templates look stale, the +answer is that nobody has promoted, and the fix is one workflow dispatch. + +**Boxes.** A launch of the published templates starts AT the promoted release. +A box's own self-update still follows the tracked branch, which is master +unless it is told otherwise - `agentbox update --branch release`, or +`selfUpdate.branch = "release"` on NixOS. Making `release` the shipped default +is deliberately not done here: it changes what every existing box updates to, +and it needs a promotion to exist first. + +**Rollback.** Dispatch `promote.yml` again with `sha` set to an earlier +promoted commit, `allow_rollback` on, and `skip_deploy_test` on (that +candidate already passed it). It keeps the tag it already has - a commit is +tagged at most once - rebuilds its manifest from its own immutable rev, +republishes it, and force-moves the `release` pointer back. For one box: +`agentbox update --rev --force` (the `--force` is required +because `agent-box-source`'s fast-forward guard refuses a backwards move by +construction). + +**Verifying a box against a release.** `python3 scripts/release_manifest.py +verify release-manifest.json --rev ` recomputes every recorded +identity from the rev the manifest names and refuses any difference. It +re-hashes the channel URL the manifest RECORDED rather than re-resolving the +channel - re-resolving would compare an old release against wherever unstable +has since moved, which would make every rollback unverifiable. + ## Filing Issues, and when to skip straight to the PR When you hit something wrong - a bug, a design gap, a stale doc, a flaky check, surprising behavior you had to work around - do not let it die in a session transcript: the next agent starts with none of your context. But an issue is not automatically the right container for it. This is a repo we control, so **when you know the fix and can push it, open the PR instead**; an issue that already carries the diagnosis and the patch is churn, costing a read for every future triager and giving nothing the PR does not carry. Name the symptom in the PR body, so the work is still findable by what went wrong. @@ -467,14 +532,24 @@ Attach those screenshots with `agent-box-upload FILE --repo defangdevs/agent-box ### Landing a PR -**The branch ruleset requires no status check.** It gates on resolved -conversations and zero approvals, and nothing else - so -`gh pr merge NNN --squash --auto` is not a promise to wait for green here. It -is an immediate merge with extra steps: PR #456 merged on the spot with its CI -run still `IN_PROGRESS`. To land on green, poll the run and merge once it -passes, or add the check to the ruleset first. Read `rules/branches` before -assuming any gate exists; the classic branch-protection API does not describe -this repo. This is also how stale page-copy assertions reach master at all. +**There is a gate to require now, and requiring it is a repo-settings +change nobody can make from a PR.** Every gated workflow ends in an +always-reported terminal job - `CI gate`, `AWS template gate`, +`Azure template gate` - added for issue #632. Those three are safe to +require, because they are reported on EVERY pull request whatever it +touches: the path filters moved off the workflow triggers into +`.github/path-filters/*.paths`, so a docs-only change now gets a gate +saying "nothing to run" instead of getting no check run at all. Requiring +a path-filtered job itself would have blocked such a PR forever. + +**Until an admin adds them, the branch ruleset still requires no status +check.** It gates on resolved conversations and zero approvals, and nothing +else - so `gh pr merge NNN --squash --auto` is not a promise to wait for +green here. It is an immediate merge with extra steps: PR #456 merged on +the spot with its CI run still `IN_PROGRESS`. To land on green, poll the +run and merge once it passes. Read `rules/branches` before assuming any +gate exists; the classic branch-protection API does not describe this repo. +This is also how stale page-copy assertions reach master at all. **A CONFLICTING PR reports "no checks reported", not a failure.** The workflows trigger on `pull_request`, which builds the MERGE commit, and a conflicted PR diff --git a/deploy/aws/README.md b/deploy/aws/README.md index 39b3cc7c..ff92b47f 100644 --- a/deploy/aws/README.md +++ b/deploy/aws/README.md @@ -578,14 +578,34 @@ commit if anything changed. CloudFormation's `templateURL` accepts only S3 URLs, so the templates live at `s3://defang-agent-box/lightsail-template.yaml` (the default Launch buttons) and `s3://defang-agent-box/template.yaml` (the EC2 alternative). -`.github/workflows/publish-template.yml` uploads both on every push to -`master` via GitHub OIDC (no static AWS keys). +`.github/workflows/publish-template.yml` uploads both, plus the +`release-manifest.json` that says what they pin, via GitHub OIDC (no static +AWS keys). + +**It no longer runs on a push to `master`** (issue #632). Publishing was +independent of CI and of the fresh-boot deploy test, and it re-resolved the +nixos-unstable channel at publish time - so the box a Launch button creates +had never been booted by anything, and its one mutable dependency was +resolved after the last thing that could have tested it. Publishing is now +the last phase of `.github/workflows/promote.yml`, which requires the three +aggregate CI gates green for one exact commit, builds that candidate's +release manifest ONCE, boots it in `deploy-test` with those pins, and only +then calls this workflow with the same manifest. So a failed deployment test +leaves the public defaults unchanged, and the templates always pin what was +actually tested. See "Releases" in the repo's `AGENTS.md`, including how to +roll back. + +To publish, dispatch `Promote a release candidate`. A `workflow_dispatch` of +`Publish CFN template to S3` still exists for re-publishing an existing +release tag; it rebuilds the manifest from that immutable rev and runs no +tests of its own, so it is not a way to promote something new. ### Prerequisites (forking this repo) The workflow is self-bootstrapping - it upserts the bucket, its public-access configuration, and an `s3:GetObject` policy scoped to the two -template objects (the only objects in the bucket) every run. The module itself +template objects plus `release-manifest.json` (the only objects in the +bucket) every run. The module itself is fetched by the box direct from `raw.githubusercontent.com` at first boot; that host is dual-stack, so an IPv6-only box needs no NAT64. It reads all deploy config from **repo-level Actions variables** (Settings > Secrets and @@ -618,13 +638,17 @@ Verify with a `workflow_dispatch` run of `Publish CFN template to S3`, then: ```bash curl -I "https://${AGENT_BOX_BUCKET}.s3.amazonaws.com/lightsail-template.yaml" curl -I "https://${AGENT_BOX_BUCKET}.s3.amazonaws.com/template.yaml" +# what those two currently pin, and what a box can be checked against +curl -s "https://${AGENT_BOX_BUCKET}.s3.amazonaws.com/release-manifest.json" ``` ## Pull request validation -`.github/workflows/aws-ci.yml` runs on pull requests that touch the AWS -templates, launch page, browser-terminal smoke helper, or related workflows. It -does not create AWS resources; it runs +`.github/workflows/aws-ci.yml` starts on every pull request. Its `AWS template +gate` check is always reported, so it is the one to require; the `validate` +job itself only runs when a change touches the AWS templates, launch page, +browser-terminal smoke helper, or related workflows (issue #632). It does not +create AWS resources; when it runs, it runs `cfn-lint deploy/aws/template.yaml deploy/aws/lightsail-template.yaml` and compiles `scripts/ws_smoke.py` so template/auth-helper changes get fast PR feedback. diff --git a/flake.nix b/flake.nix index e8ed4baf..0fe68a80 100644 --- a/flake.nix +++ b/flake.nix @@ -1615,6 +1615,67 @@ open(sys.argv[3], "w").write(header + yaml.safe_dump(data, sort_keys=True))' \ cp log "$out" ''; + # The path filters and the always-reporting CI gate (issue #632). + # The gate is a REQUIRED status check, so both of its failure + # directions are silent: a filter that matches too little skips + # the checks and reports green over the change that needed them, + # and one that matches too much runs the whole VM suite on a + # docs commit. The `Wiring` cases are the ones no other check + # can see - that a gated workflow has not grown a trigger-level + # `paths:` again, which is exactly what made green CI + # unrequireable before. + changed-paths = + pkgs.runCommand "agent-box-changed-paths" + { + nativeBuildInputs = [ pkgs.python3 ]; + matcher = ./scripts/changed_paths.py; + tests = ./tests/test-changed-paths.py; + filters = ./.github/path-filters; + workflows = ./.github/workflows; + } '' + install -d repo/scripts repo/tests repo/.github + cp "$matcher" repo/scripts/changed_paths.py + cp "$tests" repo/tests/test-changed-paths.py + cp -r "$filters" repo/.github/path-filters + cp -r "$workflows" repo/.github/workflows + # Not piped into tee: the log has to reach the build output + # whether the tests pass or fail, and the exit status has to + # be python's own. + python3 repo/tests/test-changed-paths.py > log 2>&1 || { + cat log + exit 1 + } + cat log + cp log "$out" + ''; + + # The release manifest promotion records (issue #632): the + # identities the deployment test ran against and the identities + # the published templates carry. Weighted at the refusals, since + # a manifest that verifies when it should not is how an untested + # artifact becomes the public install default - and it looks + # exactly like a pass. Hermetic: no network, and + # `nix-prefetch-url` is a stub the test writes itself. + release-manifest = + pkgs.runCommand "agent-box-release-manifest" + { + nativeBuildInputs = [ pkgs.python3 ]; + # Not `builder`, which is a derivation's own + # reserved attribute. + manifester = ./scripts/release_manifest.py; + tests = ./tests/test-release-manifest.py; + } '' + install -d repo/scripts repo/tests + cp "$manifester" repo/scripts/release_manifest.py + cp "$tests" repo/tests/test-release-manifest.py + python3 repo/tests/test-release-manifest.py > log 2>&1 || { + cat log + exit 1 + } + cat log + cp log "$out" + ''; + # Unit test for agent-box-upload (issue #368). The three things it # gets right were shipped WRONG first, as a curl recipe in the guide # (PR #367 review): the token in argv, where every other Linux user diff --git a/scripts/changed_paths.py b/scripts/changed_paths.py new file mode 100755 index 00000000..10335bb4 --- /dev/null +++ b/scripts/changed_paths.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +"""Decide whether a commit range touched any of a workflow's build paths. + +Why this exists (issue #632). CI's expensive jobs are path-filtered, and +until now the filter lived on the workflow TRIGGER (`on: pull_request: +paths:`). A trigger-level filter means the whole workflow never starts on a +PR that misses it -- and a workflow that never starts reports NO check run +at all. That is fine while nothing depends on the check, and fatal the +moment a branch ruleset requires it: a required check that is never +reported leaves the PR pending forever, so requiring green CI would have +blocked every docs-only change permanently. + +So the filter moved DOWN a level. The workflow now always starts, a cheap +`changes` job runs this script, the expensive job is `if:`-guarded on its +answer, and a terminal `gate` job reports success/failure unconditionally. +That gate is the check a ruleset can require: it is reported on every pull +request and every push, and it says "skipped, correctly" rather than +saying nothing at all. + +The pattern dialect is GitHub's own (`on..paths`), because these +pattern files ARE the lists that used to sit in those trigger blocks: + + * zero or more characters, but never `/` + ** zero or more characters, `/` included + ? exactly one character, but never `/` + +Anything else is literal. Blank lines and `#` comments are ignored, which +is the reason the lists are plain text and not JSON: every entry in them +carries a comment saying which bug put it there, and those comments are +the most valuable part of the file. + +Usage: + + changed_paths.py FILTER --base SHA --head SHA # asks git + changed_paths.py FILTER --files-from FILE # or a given list + changed_paths.py FILTER --files-from - # ... on stdin + +It prints `true` or `false` and exits 0 either way; a non-zero exit means +the question could not be answered. With no usable base (a force-push, a +brand-new branch, a manual dispatch) it prints `true`: the fail-safe +direction for a gate is to RUN the checks, never to skip them. +""" + +import argparse +import re +import subprocess +import sys + + +def load_patterns(path): + """The non-comment, non-blank lines of a filter file, in order.""" + patterns = [] + with open(path, encoding="utf-8") as fh: + for line in fh: + line = line.strip() + if line and not line.startswith("#"): + patterns.append(line) + if not patterns: + raise SystemExit(f"{path}: no patterns -- an empty filter would " + "silently skip every build") + return patterns + + +def to_regex(pattern): + """GitHub's `paths` glob, as an anchored regex. + + `**` has to be consumed before `*`, or `**.nix` compiles to + `[^/]*[^/]*\\.nix` and stops matching `modules/foo.nix` -- which is the + whole point of that entry. + """ + out, i = [], 0 + while i < len(pattern): + char = pattern[i] + if char == "*": + if pattern[i:i + 2] == "**": + out.append(".*") + i += 2 + continue + out.append("[^/]*") + elif char == "?": + out.append("[^/]") + else: + out.append(re.escape(char)) + i += 1 + return re.compile("".join(out) + r"\Z") + + +def matches(patterns, files): + """Every (file, pattern) pair that fired, so the log can say why.""" + compiled = [(p, to_regex(p)) for p in patterns] + hits = [] + for name in files: + for pattern, rx in compiled: + if rx.match(name): + hits.append((name, pattern)) + break + return hits + + +def changed_files(base, head): + """The paths git reports between two revs, or None if it cannot.""" + if not base or not head: + return None + # An all-zero base is how GitHub spells "there was no previous commit" + # in a push payload (a new branch, or the first push to one). + if set(base) == {"0"}: + return None + proc = subprocess.run( + ["git", "diff", "--name-only", "--no-renames", f"{base}...{head}"], + capture_output=True, text=True) + if proc.returncode != 0: + # A shallow clone, or a base the force-push took away. Either way + # the range is not answerable here. + sys.stderr.write(proc.stderr) + return None + return [line for line in proc.stdout.splitlines() if line] + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + ap.add_argument("filter", help="a .paths file") + ap.add_argument("--base", default="", help="the rev to diff from") + ap.add_argument("--head", default="", help="the rev to diff to") + ap.add_argument("--files-from", default=None, + help="read the changed paths from a file, or - for stdin") + args = ap.parse_args(argv) + + patterns = load_patterns(args.filter) + + if args.files_from: + source = sys.stdin if args.files_from == "-" else \ + open(args.files_from, encoding="utf-8") + with source: + files = [line.strip() for line in source if line.strip()] + else: + files = changed_files(args.base, args.head) + if files is None: + print("::notice::no usable commit range " + f"({args.base or '(none)'}...{args.head or '(none)'}) -- " + "running the checks", file=sys.stderr) + print("true") + return 0 + + hits = matches(patterns, files) + for name, pattern in hits[:20]: + print(f"{name} <- {pattern}", file=sys.stderr) + if len(hits) > 20: + print(f"... and {len(hits) - 20} more", file=sys.stderr) + print(f"{len(files)} changed path(s), {len(hits)} matched " + f"{args.filter}", file=sys.stderr) + print("true" if hits else "false") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/release_manifest.py b/scripts/release_manifest.py new file mode 100755 index 00000000..b1f6c042 --- /dev/null +++ b/scripts/release_manifest.py @@ -0,0 +1,425 @@ +#!/usr/bin/env python3 +"""Build, show and verify a release candidate's manifest (issue #632). + +Until this existed, "a release" was a commit sha and nothing else, and the +two things that install agent-box for the public disagreed about what they +were shipping: + + * deploy-test.yml pinned AgentBoxRev/AgentBoxSha256 to the commit that + triggered it and left AgentNixpkgsUrl/AgentNixpkgsSha256 EMPTY, so the + box it booted tracked whatever the nixos-unstable channel was at boot; + * publish-template.yml re-resolved that channel at publish time and + injected the pair it happened to get into the S3 templates. + +So the box that passed the fresh-boot test and the box a 1-click launch +creates were never pinned to the same dependency set, and neither identity +was written down anywhere afterwards. An immutable source commit is not a +complete release manifest. + +This is that manifest. It is built ONCE per candidate, from one commit, +and everything downstream - the deployment test, the published templates, +an operator asking what a box is running - reads it instead of resolving +anything again: + + rev the commit; the source identity + module_sha256 SRI hash of modules/agent-box.nix at that rev, + which is how template.yaml fetches it (issue #51) + flake_ref github:OWNER/REPO/, which is how + lightsail-template.yaml installs the runtime + flake_lock_sha256 the flake's own pinned input set at that rev + agent_nixpkgs the ONE mutable external input: the resolved + channel SNAPSHOT url plus its unpacked hash + templates hash per deployment template at that rev + +Three verbs: + + build resolve every identity and write the manifest + show print it the way a workflow log and a PR body want it + verify recompute every identity from the recorded rev and refuse any + difference - the reproducibility proof, and what says whether + an installed box is running the candidate that was tested + +Runnable without Nix for everything but the channel hash, which is +`nix-prefetch-url`'s answer and nothing else's. +""" + +import argparse +import base64 +import datetime +import hashlib +import json +import os +import shutil +import subprocess +import sys +import urllib.request + +MANIFEST_VERSION = 1 + +# A channel tarball is tens of megabytes, so this is generous for the +# download - but it has to stay BELOW the 15-minute timeout of promote.yml's +# `candidate` job, which spends time on checkout, rev resolution and the +# CI-gate check before ever reaching this call. At 900s (== the job's own +# timeout) a stalled prefetch would be killed by the JOB timeout first, +# which GitHub reports as `cancelled` - indistinguishable from a supersede, +# and invisible to a standing webhook watch that only spawns on +# `failure`/`timed_out` (see AGENTS.md, "Give a long job a STEP-level +# timeout-minutes"). 600s leaves the preceding steps headroom and still +# fails as a reported ManifestError rather than a silent cancellation. +PREFETCH_TIMEOUT = 600 + +# The channel the templates' AgentNixpkgsUrl pair pins. Kept here rather +# than in the workflow because `verify` has to resolve the same one. +DEFAULT_CHANNEL = "https://channels.nixos.org/nixos-unstable" + +# Hashed into the manifest because the published template IS these files +# with defaults injected: a template edited after the deployment test is a +# different artifact, whatever the rev says. +TEMPLATES = ( + "deploy/aws/template.yaml", + "deploy/aws/lightsail-template.yaml", +) + +# The module a box fetches as a single file (issue #51). +MODULE = "modules/agent-box.nix" + +RAW = "https://raw.githubusercontent.com/{repo}/{rev}/{path}" + +# Fields that describe when the manifest was made rather than what it +# describes. `verify` ignores them; everything else must match exactly. +PROVENANCE = ("created", "created_by", "manifest_version") + + +class ManifestError(Exception): + """A candidate whose identities could not be resolved or did not match.""" + + +def sri(data): + """`sha256-`, the form Nix's fetchurl and the templates want.""" + return "sha256-" + base64.b64encode(hashlib.sha256(data).digest()).decode() + + +def hexsum(data): + return hashlib.sha256(data).hexdigest() + + +def read_remote(repo, rev, path, timeout=60): + """The bytes GitHub serves for one path at one commit. + + By rev, never by branch: this is the same immutable URL a launching box + fetches the module from, so what is hashed here is what a box gets. + """ + url = RAW.format(repo=repo, rev=rev, path=path) + try: + with urllib.request.urlopen(url, timeout=timeout) as fh: + return fh.read() + except Exception as exc: # noqa: BLE001 - reported + raise ManifestError(f"cannot read {url}: {exc}") from exc + + +def read_source(repo, rev, path, source_dir=None, check_remote=True): + """One candidate file, and the assurance the rev really serves it. + + A local checkout is the cheap and obvious source, and it is also the + one that can lie: a workflow with an uncommitted edit, or a checkout + at another rev, would hash bytes no launching box will ever see. So + the remote copy at that exact rev is fetched and compared unless the + caller says not to. + """ + if source_dir is None: + return read_remote(repo, rev, path) + local = os.path.join(source_dir, path) + try: + with open(local, "rb") as fh: + data = fh.read() + except OSError as exc: + raise ManifestError(f"cannot read {local}: {exc}") from exc + if check_remote: + remote = read_remote(repo, rev, path) + if remote != data: + raise ManifestError( + f"{path} in {source_dir} differs from {repo}@{rev[:12]} - " + "the tree is not the candidate it claims to be") + return data + + +def resolve_channel(channel=DEFAULT_CHANNEL, timeout=60): + """The channel's current SNAPSHOT url - immutable once resolved. + + channels.nixos.org/nixos-unstable is a redirect to a dated release + directory. The redirect target is a fixed artifact; the redirector is + not. Recording the target is what turns "we built against unstable" + into a dependency identity. + """ + override = os.environ.get("AGENT_BOX_CHANNEL_URL") + if override: + return override + req = urllib.request.Request(channel, method="HEAD") + try: + with urllib.request.urlopen(req, timeout=timeout) as fh: + resolved = fh.geturl() + except Exception as exc: # noqa: BLE001 - reported + raise ManifestError(f"cannot resolve {channel}: {exc}") from exc + return resolved.rstrip("/") + "/nixexprs.tar.xz" + + +def prefetch(url): + """`nix-prefetch-url --unpack`, which is the only source of this hash.""" + tool = shutil.which("nix-prefetch-url") + if not tool: + raise ManifestError( + "nix-prefetch-url is not on PATH, so the channel hash cannot be " + "computed - a manifest without it is not a release manifest") + try: + # Bounded, because an unbounded stall here would run the job out of + # its own timeout - and a job that exceeds its timeout is reported + # `cancelled`, which this repo has already learned is + # indistinguishable from a routine supersede and so reaches nobody. + proc = subprocess.run([tool, "--unpack", url], capture_output=True, + text=True, timeout=PREFETCH_TIMEOUT) + except subprocess.TimeoutExpired as exc: + raise ManifestError( + f"nix-prefetch-url {url} did not finish within " + f"{PREFETCH_TIMEOUT}s") from exc + if proc.returncode != 0: + raise ManifestError(f"nix-prefetch-url {url} failed: {proc.stderr}") + out = proc.stdout.strip().splitlines() + if not out: + raise ManifestError(f"nix-prefetch-url {url} printed nothing") + return out[-1].strip() + + +def build(repo, rev, source_dir=None, check_remote=True, + channel=DEFAULT_CHANNEL, created_by=None): + """Every identity of one candidate, resolved exactly once.""" + if len(rev) != 40 or any(c not in "0123456789abcdef" for c in rev): + raise ManifestError( + f"rev must be a full 40-character commit sha, got {rev!r} - a " + "branch or short sha is not an immutable identity") + + module = read_source(repo, rev, MODULE, source_dir, check_remote) + lock = read_source(repo, rev, "flake.lock", source_dir, check_remote) + templates = { + path: hexsum(read_source(repo, rev, path, source_dir, check_remote)) + for path in TEMPLATES + } + url = resolve_channel(channel) + return { + "manifest_version": MANIFEST_VERSION, + "created": datetime.datetime.now(datetime.timezone.utc) + .replace(microsecond=0).isoformat(), + "created_by": created_by or "release_manifest.py", + "repo": repo, + "rev": rev, + "module_path": MODULE, + "module_sha256": sri(module), + "flake_ref": f"github:{repo}/{rev}", + "flake_lock_sha256": hexsum(lock), + "agent_nixpkgs": { + "channel": channel, + "url": url, + "sha256": prefetch(url), + }, + "templates": templates, + } + + +def verify(manifest, source_dir=None, check_remote=True, expect_rev=None): + """Recompute the manifest from its own rev and report every difference. + + The channel entry is re-hashed from the URL the manifest RECORDED, not + from the channel: re-resolving the redirect would compare the candidate + against whatever unstable moved to since, which is the exact confusion + this file exists to end. + """ + problems = [] + repo, rev = manifest.get("repo"), manifest.get("rev") + if not repo or not rev: + raise ManifestError("manifest has no repo/rev - nothing to verify") + if expect_rev and expect_rev != rev: + problems.append( + f"rev: manifest describes {rev}, expected {expect_rev}") + + try: + module = read_source(repo, rev, MODULE, source_dir, check_remote) + if sri(module) != manifest.get("module_sha256"): + problems.append( + f"module_sha256: {MODULE} at {rev[:12]} hashes to " + f"{sri(module)}, manifest says {manifest.get('module_sha256')}") + except ManifestError as exc: + problems.append(str(exc)) + + try: + lock = read_source(repo, rev, "flake.lock", source_dir, check_remote) + if hexsum(lock) != manifest.get("flake_lock_sha256"): + problems.append( + f"flake_lock_sha256: flake.lock at {rev[:12]} hashes to " + f"{hexsum(lock)}, manifest says " + f"{manifest.get('flake_lock_sha256')}") + except ManifestError as exc: + problems.append(str(exc)) + + recorded = manifest.get("templates") or {} + if set(recorded) != set(TEMPLATES): + problems.append( + f"templates: manifest records {sorted(recorded)}, this release " + f"ships {sorted(TEMPLATES)}") + for path in sorted(set(recorded) & set(TEMPLATES)): + try: + got = hexsum(read_source(repo, rev, path, source_dir, + check_remote)) + except ManifestError as exc: + problems.append(str(exc)) + continue + if got != recorded[path]: + problems.append( + f"templates[{path}]: hashes to {got}, manifest says " + f"{recorded[path]}") + + pins = manifest.get("agent_nixpkgs") or {} + if not pins.get("url") or not pins.get("sha256"): + problems.append( + "agent_nixpkgs: no url/sha256 pair - this is the mutable " + "external input, so a manifest without it promotes something " + "that was never pinned") + elif not os.environ.get("AGENT_BOX_SKIP_PREFETCH"): + try: + got = prefetch(pins["url"]) + except ManifestError as exc: + problems.append(str(exc)) + else: + if got != pins["sha256"]: + problems.append( + f"agent_nixpkgs.sha256: {pins['url']} now hashes to " + f"{got}, manifest says {pins['sha256']}") + + expected = {"repo", "rev", "module_path", "module_sha256", "flake_ref", + "flake_lock_sha256", "agent_nixpkgs", "templates"} + missing = expected - set(manifest) + if missing: + problems.append(f"missing field(s): {', '.join(sorted(missing))}") + if manifest.get("flake_ref") != f"github:{repo}/{rev}": + problems.append( + f"flake_ref: {manifest.get('flake_ref')} does not name " + f"{repo}@{rev}") + return problems + + +def summary(manifest): + pins = manifest.get("agent_nixpkgs") or {} + lines = [ + f"repo {manifest.get('repo')}", + f"rev {manifest.get('rev')}", + f"flake_ref {manifest.get('flake_ref')}", + f"module_sha256 {manifest.get('module_sha256')}", + f"flake_lock_sha256 {manifest.get('flake_lock_sha256')}", + f"agent_nixpkgs url {pins.get('url')}", + f"agent_nixpkgs sha {pins.get('sha256')}", + ] + for path, digest in sorted((manifest.get("templates") or {}).items()): + lines.append(f"template {digest} {path}") + lines.append(f"built {manifest.get('created')} by " + f"{manifest.get('created_by')}") + return "\n".join(lines) + + +def load(path): + with open(path, encoding="utf-8") as fh: + return json.load(fh) + + +def dump(manifest, path): + text = json.dumps(manifest, indent=2, sort_keys=True) + "\n" + if path == "-": + sys.stdout.write(text) + else: + with open(path, "w", encoding="utf-8") as fh: + fh.write(text) + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + sub = ap.add_subparsers(dest="verb", required=True) + + def shared(p): + p.add_argument("--source-dir", default=None, metavar="DIR", + help="read the candidate's files from a local " + "checkout instead of fetching them") + p.add_argument("--no-remote-check", action="store_true", + help="trust --source-dir without comparing it " + "against the rev GitHub serves") + return p + + b = shared(sub.add_parser("build", help="resolve and write a manifest")) + b.add_argument("--repo", required=True, metavar="OWNER/REPO") + b.add_argument("--rev", required=True, metavar="SHA", + help="the candidate commit, in full") + b.add_argument("--channel", default=DEFAULT_CHANNEL) + b.add_argument("--created-by", default=None) + b.add_argument("--out", default="-", metavar="FILE") + + v = shared(sub.add_parser("verify", help="recompute and compare")) + v.add_argument("manifest") + v.add_argument("--rev", default=None, metavar="SHA", + help="also require the manifest to describe this rev - " + "how a box's reported rev is checked against the " + "candidate that was tested") + + s = sub.add_parser("show", help="print a manifest readably") + s.add_argument("manifest") + + f = sub.add_parser("field", help="print one recorded value") + f.add_argument("manifest") + f.add_argument("path", metavar="a.dotted.path", + help="e.g. rev, module_sha256, agent_nixpkgs.url") + + args = ap.parse_args(argv) + try: + if args.verb == "build": + manifest = build(args.repo, args.rev, + source_dir=args.source_dir, + check_remote=not args.no_remote_check, + channel=args.channel, + created_by=args.created_by) + dump(manifest, args.out) + if args.out != "-": + print(summary(manifest)) + return 0 + if args.verb == "show": + print(summary(load(args.manifest))) + return 0 + if args.verb == "field": + # For the workflows, which need single values on stdout and + # must not silently inject an empty Default: when a field is + # missing. + value = load(args.manifest) + for part in args.path.split("."): + if not isinstance(value, dict) or part not in value: + raise ManifestError( + f"{args.manifest} has no {args.path}") + value = value[part] + if value in (None, "", {}, []): + raise ManifestError( + f"{args.manifest}: {args.path} is empty") + print(value) + return 0 + problems = verify(load(args.manifest), + source_dir=args.source_dir, + check_remote=not args.no_remote_check, + expect_rev=args.rev) + if problems: + for problem in problems: + print(f"::error::{problem}", file=sys.stderr) + print(f"{args.manifest}: {len(problems)} difference(s) - this is " + "not the candidate it claims to be", file=sys.stderr) + return 1 + print(f"{args.manifest}: every recorded identity still matches") + print(summary(load(args.manifest))) + return 0 + except ManifestError as exc: + print(f"::error::{exc}", file=sys.stderr) + return 2 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/test-changed-paths.py b/tests/test-changed-paths.py new file mode 100644 index 00000000..5855176f --- /dev/null +++ b/tests/test-changed-paths.py @@ -0,0 +1,226 @@ +#!/usr/bin/env python3 +"""Unit tests for scripts/changed_paths.py and the committed filter files. + +The gate this feeds is a REQUIRED status check (issue #632), so both of its +failure directions are expensive and neither is visible in review: + + * a pattern that matches too little skips the checks on a change that + needed them, and the gate reports green over it; + * a pattern that matches too much runs the whole VM suite on a + docs-only pull request. + +So the dialect gets unit tests, and the real committed .paths files get +assertions against the concrete paths whose bug histories put them in the +list. Runnable directly: `python3 tests/test-changed-paths.py`. +""" + +import pathlib +import re +import shutil +import subprocess +import sys +import tempfile +import unittest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT / "scripts")) + +import changed_paths # noqa: E402 + +FILTERS = ROOT / ".github" / "path-filters" +WORKFLOWS = ROOT / ".github" / "workflows" + +# Every gated workflow, and the filter its `changes` job must read. +GATED = { + "ci.yml": "ci.paths", + "aws-ci.yml": "aws-ci.paths", + "azure-ci.yml": "azure-ci.paths", +} + + +def decide(filter_name, files): + """What the gate would answer for this set of changed paths.""" + patterns = changed_paths.load_patterns(str(FILTERS / filter_name)) + return bool(changed_paths.matches(patterns, files)) + + +class Dialect(unittest.TestCase): + def test_double_star_crosses_slashes(self): + rx = changed_paths.to_regex("**.nix") + self.assertTrue(rx.match("flake.nix")) + self.assertTrue(rx.match("modules/agent-box.nix")) + self.assertTrue(rx.match("a/b/c/d.nix")) + self.assertFalse(rx.match("modules/agent-box.nix.in")) + + def test_single_star_stops_at_a_slash(self): + rx = changed_paths.to_regex("tests/*.nix") + self.assertTrue(rx.match("tests/webhook.nix")) + self.assertFalse(rx.match("tests/e2e/a.nix")) + + def test_trailing_double_star_is_a_prefix(self): + rx = changed_paths.to_regex("modules/src/**") + self.assertTrue(rx.match("modules/src/settings.js")) + self.assertTrue(rx.match("modules/src/vendor/idiomorph.js")) + self.assertFalse(rx.match("modules/agent-box.nix")) + + def test_question_mark_is_one_non_slash_character(self): + rx = changed_paths.to_regex("a?c") + self.assertTrue(rx.match("abc")) + self.assertFalse(rx.match("a/c")) + self.assertFalse(rx.match("abbc")) + + def test_patterns_are_anchored_at_both_ends(self): + rx = changed_paths.to_regex("bin/agentbox") + self.assertTrue(rx.match("bin/agentbox")) + self.assertFalse(rx.match("bin/agentbox.bak")) + self.assertFalse(rx.match("x/bin/agentbox")) + + def test_dots_are_literal(self): + # The bug this forbids: an unescaped `.` in `flake.lock` also + # matching `flakeXlock`, which is harmless, and in `**.nix` + # matching `anix`, which is not. + rx = changed_paths.to_regex("flake.lock") + self.assertTrue(rx.match("flake.lock")) + self.assertFalse(rx.match("flakeXlock")) + + def test_an_empty_filter_is_refused(self): + # An empty list matches nothing, so it would skip every build and + # the gate would report green. Louder than that: a hard failure. + with tempfile.TemporaryDirectory() as tmp: + empty = pathlib.Path(tmp) / "empty.paths" + empty.write_text("# nothing but a comment\n", encoding="utf-8") + with self.assertRaises(SystemExit): + changed_paths.load_patterns(str(empty)) + + +class CiFilter(unittest.TestCase): + def test_module_sources_run_ci(self): + for name in ("flake.nix", + "modules/agent-box.nix", + "modules/agent-box.nix.in", + "modules/src/settings.js", + "modules/src/vendor/idiomorph.js", + "bin/assemble-module.py", + "bin/agentbox", + "bin/golden-snapshot.py", + "tests/golden/web/etc/agent-box/x", + "tests/native/config.json", + "docs/potato.svg", + "flake.lock", + "scripts/check_vendor.py", + ".github/workflows/vendor-updates.yml"): + with self.subTest(name): + self.assertTrue(decide("ci.paths", [name])) + + def test_the_gate_machinery_runs_ci(self): + # A change to what decides which jobs run has to run them. + for name in (".github/path-filters/ci.paths", + ".github/path-filters/azure-ci.paths", + "scripts/changed_paths.py", + "tests/test-changed-paths.py", + "scripts/release_manifest.py", + "tests/test-release-manifest.py"): + with self.subTest(name): + self.assertTrue(decide("ci.paths", [name])) + + def test_documentation_only_changes_skip_ci(self): + # The case the whole restructure exists for: this must answer + # false AND still get a reported gate. + self.assertFalse(decide("ci.paths", ["README.md", + "AGENTS.md", + "docs/index.html", + "deploy/aws/README.md"])) + + def test_one_matching_path_in_a_large_change_is_enough(self): + self.assertTrue(decide("ci.paths", ["README.md"] * 50 + ["flake.nix"])) + + +class DeployFilters(unittest.TestCase): + def test_aws(self): + self.assertTrue(decide("aws-ci.paths", ["deploy/aws/template.yaml"])) + self.assertTrue(decide("aws-ci.paths", ["docs/index.html"])) + self.assertTrue(decide("aws-ci.paths", ["tests/native/config.json"])) + self.assertFalse(decide("aws-ci.paths", ["deploy/azure/agent-box.bicep"])) + self.assertFalse(decide("aws-ci.paths", ["README.md"])) + + def test_azure(self): + self.assertTrue(decide("azure-ci.paths", ["deploy/azure/agent-box.bicep"])) + self.assertTrue(decide("azure-ci.paths", ["deploy/azure/README.md"])) + self.assertFalse(decide("azure-ci.paths", ["deploy/aws/template.yaml"])) + self.assertFalse(decide("azure-ci.paths", ["README.md"])) + + +class Wiring(unittest.TestCase): + """The workflow side, which no other check looks at. + + A filter file nothing reads, or a gated workflow that quietly grew a + trigger-level `paths:` again, both put the repo back where issue #632 + found it - with a required check that is never reported. + """ + + def test_every_filter_is_read_by_its_workflow(self): + for workflow, filt in GATED.items(): + text = (WORKFLOWS / workflow).read_text(encoding="utf-8") + with self.subTest(workflow): + self.assertIn(f".github/path-filters/{filt}", text) + self.assertIn("scripts/changed_paths.py", text) + + def test_no_gated_workflow_filters_on_its_trigger(self): + # `paths:`/`paths-ignore:` inside `on:` is exactly the shape that + # makes a workflow report nothing. The gated ones must not have it. + for workflow in GATED: + text = (WORKFLOWS / workflow).read_text(encoding="utf-8") + body = text.split("\njobs:", 1)[0] + with self.subTest(workflow): + self.assertIsNone( + re.search(r"^\s*paths(-ignore)?:", body, re.M), + f"{workflow}: the trigger filters on paths again; the " + "filter belongs in .github/path-filters (issue #632)") + + def test_every_gated_workflow_has_an_always_reporting_gate(self): + for workflow in GATED: + text = (WORKFLOWS / workflow).read_text(encoding="utf-8") + with self.subTest(workflow): + self.assertIn("if: always()", text) + self.assertIn("gate:", text) + + def test_no_orphan_filter_files(self): + on_disk = {p.name for p in FILTERS.glob("*.paths")} + self.assertEqual(on_disk, set(GATED.values())) + + +class Cli(unittest.TestCase): + def test_files_from_stdin(self): + proc = subprocess.run( + [sys.executable, str(ROOT / "scripts" / "changed_paths.py"), + str(FILTERS / "ci.paths"), "--files-from", "-"], + input="README.md\nflake.nix\n", capture_output=True, text=True) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual(proc.stdout.strip(), "true") + + def test_no_usable_range_runs_the_checks(self): + # A force-push, a new branch, a manual dispatch. Fail SAFE: run. + for base in ("", "0" * 40): + with self.subTest(base=base): + self.assertEqual( + self._run(["--base", base, "--head", "HEAD"]), "true") + + @unittest.skipUnless( + (ROOT / ".git").exists() and shutil.which("git"), + "no git checkout here (the flake check copies files, not a repo)") + def test_a_real_commit_range_is_read_from_git(self): + # An empty range (a rev against itself) touches nothing. + self.assertEqual( + self._run(["--base", "HEAD", "--head", "HEAD"]), "false") + + def _run(self, extra): + proc = subprocess.run( + [sys.executable, str(ROOT / "scripts" / "changed_paths.py"), + str(FILTERS / "ci.paths")] + extra, + capture_output=True, text=True, cwd=str(ROOT)) + self.assertEqual(proc.returncode, 0, proc.stderr) + return proc.stdout.strip() + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/tests/test-ci-scheduling.py b/tests/test-ci-scheduling.py index 1d3e6952..1d0ae7b3 100644 --- a/tests/test-ci-scheduling.py +++ b/tests/test-ci-scheduling.py @@ -1,5 +1,4 @@ """Check the workflow's failure gates, lane budget and VM build invocation.""" -import fnmatch import json import os from pathlib import Path @@ -64,14 +63,18 @@ def test_empty_inventory_and_invalid_budget_do_not_execute(self): self.assertNotEqual(result.returncode, 0) self.assertIsNone(call) - def test_native_test_and_validator_edits_trigger_ci(self): - # PyYAML's YAML 1.1 loader reads the unquoted `on` key as True. - events = WORKFLOW.get("on", WORKFLOW.get(True)) - for event in ["push", "pull_request"]: - for path in ["tests/test-new-native.py", "tests/native/expected/etc/example", - "scripts/check_new_native.py"]: - self.assertTrue(any(fnmatch.fnmatch(path, pattern) - for pattern in events[event]["paths"]), (event, path)) + # Whether a native-test or validator edit actually triggers CI is now a + # question about .github/path-filters/ci.paths, not this workflow's + # trigger (issue #632 removed the trigger-level `paths:` entirely) - + # see tests/test-changed-paths.py's CiFilter and Wiring cases instead. + + def test_native_and_vm_are_gated_on_changes(self): + for job in ("native", "vm"): + with self.subTest(job=job): + self.assertEqual(WORKFLOW["jobs"][job]["needs"], "changes") + self.assertEqual( + WORKFLOW["jobs"][job]["if"], + "needs.changes.outputs.build == 'true'") def test_matrix_matches_nix_lanes_and_keeps_concurrency_budget(self): strategy = WORKFLOW["jobs"]["vm"]["strategy"] @@ -83,23 +86,49 @@ def test_matrix_matches_nix_lanes_and_keeps_concurrency_budget(self): self.assertEqual(sum(row["jobs"] for row in matrix), 4) def test_gate_rejects_failure_cancellation_and_skipped_jobs(self): - gate = WORKFLOW["jobs"]["validate"] - self.assertEqual(gate["name"], "Validate module & VM") - self.assertEqual(sorted(gate["needs"]), ["native", "vm"]) - self.assertEqual(gate["if"], "${{ always() }}") + # The `changes` job deciding whether CI's build paths changed sits + # in front of `native`/`vm` (issue #632); the `gate` job at the + # bottom is what the branch ruleset requires, and it has to report + # correctly whether or not those two jobs even ran. + gate = WORKFLOW["jobs"]["gate"] + self.assertEqual(gate["name"], "CI gate") + self.assertEqual(sorted(gate["needs"]), ["changes", "native", "vm"]) + self.assertEqual(gate["if"], "always()") step, = gate["steps"] self.assertEqual(step["env"], { - "NATIVE_RESULT": "${{ needs.native.result }}", - "VM_RESULT": "${{ needs.vm.result }}", + "CHANGES": "${{ needs.changes.result }}", + "NATIVE": "${{ needs.native.result }}", + "VM": "${{ needs.vm.result }}", + "BUILD": "${{ needs.changes.outputs.build }}", }) - for native in ["success", "failure", "cancelled", "skipped"]: - for vm in ["success", "failure", "cancelled", "skipped"]: - result = subprocess.run( - ["bash", "-e", "-c", step["run"]], capture_output=True, - env={**os.environ, "NATIVE_RESULT": native, "VM_RESULT": vm}, - timeout=10, - ) - self.assertEqual(result.returncode == 0, native == vm == "success") + for changes in ["success", "failure"]: + for build in ["true", "false"]: + for native in ["success", "failure", "cancelled", "skipped"]: + for vm in ["success", "failure", "cancelled", "skipped"]: + with self.subTest(changes=changes, build=build, + native=native, vm=vm): + result = subprocess.run( + ["bash", "-e", "-c", step["run"]], + capture_output=True, + env={**os.environ, "CHANGES": changes, + "BUILD": build, "NATIVE": native, + "VM": vm}, + timeout=10, + ) + # `changes` must succeed, and each of native/vm + # must either succeed outright, or be skipped + # while the build paths did NOT change (a skip + # while they DID change means the job's own + # guard expression is broken). + def ok(job_result): + return (job_result == "success" + or (job_result == "skipped" + and build != "true")) + expected = (changes == "success" + and ok(native) and ok(vm)) + self.assertEqual( + result.returncode == 0, expected, + result.stderr) if __name__ == "__main__": diff --git a/tests/test-release-manifest.py b/tests/test-release-manifest.py new file mode 100644 index 00000000..359905d6 --- /dev/null +++ b/tests/test-release-manifest.py @@ -0,0 +1,289 @@ +#!/usr/bin/env python3 +"""Unit tests for scripts/release_manifest.py (issue #632). + +The manifest is what promotion promises: these are the identities the +deployment test ran against and the identities the published templates +carry. So the tests are weighted at the REFUSALS - a manifest that +verifies when it should not is the failure that lets an untested artifact +become the public install default, and it looks exactly like a pass. + +No network and no Nix: the candidate is a directory, the channel URL comes +from AGENT_BOX_CHANNEL_URL, and `nix-prefetch-url` is a stub on PATH. +Runnable directly: `python3 tests/test-release-manifest.py`. +""" + +import copy +import json +import os +import pathlib +import shutil +import subprocess +import sys +import tempfile +import textwrap +import unittest + +ROOT = pathlib.Path(__file__).resolve().parent.parent +sys.path.insert(0, str(ROOT / "scripts")) + +import release_manifest as rm # noqa: E402 + +REV = "a" * 40 +REPO = "defangdevs/agent-box" +CHANNEL_URL = "https://releases.example/nixos/unstable/nixos-25.11pre1/nixexprs.tar.xz" +CHANNEL_HASH = "0000000000000000000000000000000000000000000000000000" + + +class Rig(unittest.TestCase): + """A candidate tree, a stubbed prefetch, and no network at all.""" + + def setUp(self): + self.tmp = pathlib.Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + self.src = self.tmp / "src" + (self.src / "modules").mkdir(parents=True) + (self.src / "deploy" / "aws").mkdir(parents=True) + (self.src / rm.MODULE).write_text("{ ... }: { }\n", encoding="utf-8") + (self.src / "flake.lock").write_text('{"nodes":{}}\n', + encoding="utf-8") + for path in rm.TEMPLATES: + (self.src / path).write_text(f"# {path}\n", encoding="utf-8") + + bindir = self.tmp / "bin" + bindir.mkdir() + stub = bindir / "nix-prefetch-url" + stub.write_text(textwrap.dedent(f"""\ + #!{sys.executable} + import sys + print({CHANNEL_HASH!r}) + """), encoding="utf-8") + stub.chmod(0o755) + self._patch_env("PATH", f"{bindir}{os.pathsep}{os.environ['PATH']}") + self._patch_env("AGENT_BOX_CHANNEL_URL", CHANNEL_URL) + + def _patch_env(self, key, value): + old = os.environ.get(key) + os.environ[key] = value + + def restore(): + if old is None: + os.environ.pop(key, None) + else: + os.environ[key] = old + self.addCleanup(restore) + + def build(self, **kw): + kw.setdefault("source_dir", str(self.src)) + kw.setdefault("check_remote", False) + return rm.build(REPO, REV, **kw) + + def verify(self, manifest, **kw): + kw.setdefault("source_dir", str(self.src)) + kw.setdefault("check_remote", False) + return rm.verify(manifest, **kw) + + +class Build(Rig): + def test_records_every_identity(self): + m = self.build() + self.assertEqual(m["repo"], REPO) + self.assertEqual(m["rev"], REV) + self.assertEqual(m["flake_ref"], f"github:{REPO}/{REV}") + self.assertTrue(m["module_sha256"].startswith("sha256-")) + self.assertEqual(len(m["flake_lock_sha256"]), 64) + self.assertEqual(m["agent_nixpkgs"]["url"], CHANNEL_URL) + self.assertEqual(m["agent_nixpkgs"]["sha256"], CHANNEL_HASH) + self.assertEqual(set(m["templates"]), set(rm.TEMPLATES)) + + def test_the_module_hash_is_the_form_the_template_wants(self): + # template.yaml passes AgentBoxSha256 to Nix's fetchurl, which + # takes SRI. A hex digest there fails on the box, at first boot, + # where nobody is watching. + m = self.build() + self.assertEqual( + m["module_sha256"], + rm.sri((self.src / rm.MODULE).read_bytes())) + + def test_a_short_sha_is_refused(self): + for bad in ("abc1234", "master", "release-2026-09-10", "A" * 40, + "g" * 40, ""): + with self.subTest(bad): + with self.assertRaises(rm.ManifestError): + rm.build(REPO, bad, source_dir=str(self.src), + check_remote=False) + + def test_a_local_tree_that_is_not_the_rev_is_refused(self): + # The reason --no-remote-check is opt-in: a workflow with an + # uncommitted edit would otherwise record hashes of bytes no + # launching box will ever be served. + original = rm.read_remote + self.addCleanup(setattr, rm, "read_remote", original) + rm.read_remote = lambda repo, rev, path, timeout=60: b"something else" + with self.assertRaises(rm.ManifestError) as cm: + self.build(check_remote=True) + self.assertIn("not the candidate it claims to be", str(cm.exception)) + + +class Verify(Rig): + def setUp(self): + super().setUp() + self.manifest = self.build() + + def test_a_fresh_manifest_verifies(self): + self.assertEqual(self.verify(self.manifest), []) + + def test_provenance_is_not_part_of_the_identity(self): + # Two builds of the same candidate differ in `created`, and that + # must not read as a different release. + m = copy.deepcopy(self.manifest) + for field in rm.PROVENANCE: + m[field] = "changed" + self.assertEqual(self.verify(m), []) + + def test_a_changed_module_is_caught(self): + (self.src / rm.MODULE).write_text("{ ... }: { evil = true; }\n", + encoding="utf-8") + problems = self.verify(self.manifest) + self.assertTrue(any("module_sha256" in p for p in problems), problems) + + def test_a_changed_template_is_caught(self): + # The case a rev alone cannot see: promotion hashes the templates + # because the published artifact IS those files. + (self.src / rm.TEMPLATES[0]).write_text("# edited\n", + encoding="utf-8") + problems = self.verify(self.manifest) + self.assertTrue(any(rm.TEMPLATES[0] in p for p in problems), problems) + + def test_a_changed_flake_lock_is_caught(self): + (self.src / "flake.lock").write_text('{"nodes":{"x":1}}\n', + encoding="utf-8") + problems = self.verify(self.manifest) + self.assertTrue(any("flake_lock_sha256" in p for p in problems), + problems) + + def test_a_different_rev_is_caught(self): + problems = self.verify(self.manifest, expect_rev="b" * 40) + self.assertTrue(any(p.startswith("rev:") for p in problems), problems) + + def test_the_candidate_rev_is_accepted(self): + self.assertEqual(self.verify(self.manifest, expect_rev=REV), []) + + def test_an_unpinned_channel_is_refused(self): + for pins in ({}, {"url": CHANNEL_URL}, {"sha256": CHANNEL_HASH}, + {"url": "", "sha256": ""}): + with self.subTest(pins=pins): + m = copy.deepcopy(self.manifest) + m["agent_nixpkgs"] = pins + problems = self.verify(m) + self.assertTrue( + any("agent_nixpkgs" in p for p in problems), problems) + + def test_the_channel_hash_is_rechecked_against_the_RECORDED_url(self): + # The whole point of the manifest: verify must not re-resolve the + # channel. If it did, it would compare the candidate against + # whatever unstable moved to since, and every older release would + # fail to verify - which is how a rollback becomes impossible. + self._patch_env("AGENT_BOX_CHANNEL_URL", + "https://releases.example/nixos/unstable/moved-on/" + "nixexprs.tar.xz") + self.assertEqual(self.verify(self.manifest), []) + + def test_a_wrong_channel_hash_is_caught(self): + m = copy.deepcopy(self.manifest) + m["agent_nixpkgs"]["sha256"] = "1" * 52 + problems = self.verify(m) + self.assertTrue(any("agent_nixpkgs.sha256" in p for p in problems), + problems) + + def test_a_flake_ref_that_names_another_rev_is_caught(self): + m = copy.deepcopy(self.manifest) + m["flake_ref"] = f"github:{REPO}/master" + problems = self.verify(m) + self.assertTrue(any("flake_ref" in p for p in problems), problems) + + def test_a_missing_field_is_caught(self): + for field in ("module_sha256", "templates", "agent_nixpkgs", + "flake_lock_sha256", "flake_ref"): + with self.subTest(field): + m = copy.deepcopy(self.manifest) + del m[field] + self.assertTrue(self.verify(m)) + + def test_a_manifest_missing_a_template_is_caught(self): + m = copy.deepcopy(self.manifest) + m["templates"].pop(rm.TEMPLATES[1]) + problems = self.verify(m) + self.assertTrue(any("templates:" in p for p in problems), problems) + + def test_a_manifest_with_no_rev_cannot_be_verified(self): + m = copy.deepcopy(self.manifest) + del m["rev"] + with self.assertRaises(rm.ManifestError): + self.verify(m) + + +class Cli(Rig): + def run_cli(self, *argv): + return subprocess.run( + [sys.executable, str(ROOT / "scripts" / "release_manifest.py"), + *argv], capture_output=True, text=True) + + def test_build_show_verify_round_trip(self): + out = self.tmp / "release-manifest.json" + proc = self.run_cli("build", "--repo", REPO, "--rev", REV, + "--source-dir", str(self.src), + "--no-remote-check", "--out", str(out)) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn(REV, proc.stdout) + manifest = json.loads(out.read_text(encoding="utf-8")) + self.assertEqual(manifest["rev"], REV) + + proc = self.run_cli("show", str(out)) + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertIn("agent_nixpkgs url", proc.stdout) + + proc = self.run_cli("verify", str(out), "--source-dir", str(self.src), + "--no-remote-check", "--rev", REV) + self.assertEqual(proc.returncode, 0, proc.stderr) + + def test_field_prints_one_value_and_refuses_a_missing_one(self): + # `field` is the verb both workflows inject public template + # `Default:` values from, and a blank Default still lints and + # still publishes - so "missing or empty exits non-zero" is the + # contract keeping an unpinned template off S3. + out = self.tmp / "release-manifest.json" + self.run_cli("build", "--repo", REPO, "--rev", REV, "--source-dir", + str(self.src), "--no-remote-check", "--out", str(out)) + + proc = self.run_cli("field", str(out), "agent_nixpkgs.url") + self.assertEqual(proc.returncode, 0, proc.stderr) + self.assertEqual(proc.stdout.strip(), CHANNEL_URL) + self.assertEqual( + self.run_cli("field", str(out), "rev").stdout.strip(), REV) + + for path in ("nope", "agent_nixpkgs.nope", "rev.nope"): + with self.subTest(missing=path): + self.assertNotEqual( + self.run_cli("field", str(out), path).returncode, 0) + + blank = json.loads(out.read_text(encoding="utf-8")) + blank["agent_nixpkgs"]["url"] = "" + out.write_text(json.dumps(blank), encoding="utf-8") + self.assertNotEqual( + self.run_cli("field", str(out), "agent_nixpkgs.url").returncode, 0) + + def test_verify_exits_non_zero_on_a_difference(self): + out = self.tmp / "release-manifest.json" + self.run_cli("build", "--repo", REPO, "--rev", REV, "--source-dir", + str(self.src), "--no-remote-check", "--out", str(out)) + (self.src / rm.MODULE).write_text("{ ... }: { evil = true; }\n", + encoding="utf-8") + proc = self.run_cli("verify", str(out), "--source-dir", + str(self.src), "--no-remote-check") + self.assertEqual(proc.returncode, 1) + self.assertIn("::error::", proc.stderr) + + +if __name__ == "__main__": + unittest.main(verbosity=2)