Prove the audit bind guard mid-migration (#960) - #963
Merged
Conversation
The audit bind is declared `create_host_path: false` so Docker refuses to manufacture the bind source when the reserve mount is not there, and the migration window is where that matters most: the store is aside under `.pre-mount`, the reserve is mounted and empty, and a bring-up that created `openbao/` there would leave a bind source on the destination side of a copy that has not run. `bootroot infra up` already refuses that state, but that is a different assertion — it says bootroot declines to start containers, not that Docker itself refuses the bind for whoever else brings the stack up. The migration scenario never attempted a Compose bring-up inside the window, so the guarantee was asserted nowhere against the state the window actually produces. The attempt is driven through the override the refusal pass rendered, which is the surface an operator has on disk at that moment, and the failure is read from the bring-up's own non-zero exit and from the error naming the bind source, so a bring-up that returned success having quietly done nothing cannot pass on the absent source alone. The window is also made faithful at its other end. Rendered step 1 stops both writers, and the scenario drops those two lines because one would stop its whole Compose stack and the other names a unit no CI host has installed; the OpenBao writer is now stopped by service instead, stays down across every pass, and comes back only once the migration has closed. Closes #960
Contributor
Author
|
[Reviewer Round 1] Approved — no findings. The new E2E helper correctly verifies the actual migration window: holding directory exists, reserve is active, and The surrounding lifecycle change also keeps OpenBao stopped from before the aside rename until after the migration closes, avoiding a writer race in the newly exercised window. PR linkage and test-plan hygiene are correct ( |
Contributor
Author
|
[Review Verdict Round 1: APPROVED] |
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The audit bind is declared
create_host_path: falseso Docker refuses to manufacture the bind source when the reserve mount is not there. The migration window is where that guarantee matters most — the store is aside under.pre-mount, the reserve is mounted but still carries noopenbao/, and a bring-up that created that directory would leave a bind source on the destination side of a copy that has not run.bootroot infra upalready refuses that state (src/commands/audit_store.rs), but that is a different assertion: it proves bootroot declines to start containers, not that Docker itself refuses the bind when something else brings the stack up. The migration E2E scenario never attempted a Compose bring-up inside the window, so the guarantee was asserted nowhere against the state the window actually produces.Closes #960
Part of #926
Changes
All of it is in
scripts/impl/run-registrar-internal-init-e2e.sh, inside the existingassert_the_rendered_steps_migrate_an_existing_storelifecycle path. No Rust changed.assert_the_audit_bind_refuses_to_create_its_source_mid_migration— a focused sibling called between pass 2 (the activation) and pass 3 (the copy), which is exactly the window. It restates its preconditions rather than assuming them (.pre-mountpresent, the mount unitactive,<audit_store_dir>/openbaoabsent), checks that the override the refusal pass rendered still names that store and still carriescreate_host_path: false, then runs the ordinarydocker compose up -d --no-deps openbaoover it.openbao/still absent, and the mounted store's entry list identical to the one taken before the attempt.rendered_migration_commandsdrops those two lines — one would stop this scenario's whole Compose stack, the other names a unit no CI host has installed. The OpenBao writer is now stopped by service through the same Compose surface just before the aside rename, stays down across every pass, and is brought back only on the closing path at the end of the section. The registrar writer is asserted not running rather than stopped: stopping a unit this scenario did not install would be a host-changing step it has no way to undo.The fixture starts from an existing audit store, so the aside rename and the full window are exercised rather than simulated. Nothing is created, removed, mounted or unmounted to stand in for Docker's result, and no rendered command list, outcome, or
withholds_activationbehaviour is touched.Test plan
docker compose upis attempted while.pre-mountexists, the reserve mount is active, and<audit_store_dir>/openbaois absent.registrar-internal-initlifecycle harness rather than a new one, starting from an existing audit store and leaving both writers stopped until the normal closing path.infra uprefusal coverage is kept and unchanged —cargo test --bin bootroot commands::audit_store::passes 136 tests, includinginfra_up_refuses_the_bring_up_while_a_migration_is_open.cargo clippy --all-targets -- -D warningspasses.cargo fmt -- --config group_imports=StdExternalCrate --checkpasses../scripts/check-docs.shpasses.shellcheck -xandbash -non the changed script are clean../scripts/validate-e2e-run-scope.shand./scripts/validate-e2e-leftover-check.shpass.Docker E2Eregistrar-internal-initjob passes — it ran green on this commit (job 99074231742), and its log shows the new assertions actually executing inside thereserve_activation_is_possibleguard rather than being skipped:PASS both writers are stopped before the store moves aside,PASS the reserve is mounted for the mid-window bring-up,the mounted store holds [lost+found] before the mid-migration bring-up,PASS the mid-migration bring-up fails on the guarded audit bind source,PASS the refused bring-up added no entry beneath the mounted store, andPASS the writers come back only once the migration has closed. It could not run on the development host — macOS with no passwordless sudo, no systemd, no loop device and nomkfs.ext4— so that CI job is the only arm that exercises it.