fix(deploy-hook): --build-staging must build from validated worktree, recreate+health 8501
All checks were successful
CI / test (push) Successful in 17s
All checks were successful
CI / test (push) Successful in 17s
Closes reviewer P0/P1 (ORCH-058 attempt 3): the committed --build-staging hook
recomputed GIT_SHA=$(git rev-parse HEAD) in $REPO (prod clone on `main`) and built
`docker build ... "$REPO"`, ignoring the caller-supplied BUILD_CONTEXT/GIT_SHA. On
the deploy-staging -> deploy edge the PR is not yet merged, so `main` HEAD != the
validated SHA -> the staging image got the wrong revision label and Strategy-B's
guard fail-closed on EVERY valid self-deploy (AC-6 deadlock). It also only did
`docker build` + exit 0 — never recreating 8501 nor health-checking — so
rebuild_staging_image's rc=0 ("rebuilt and healthy") was a lie (AC-4 unmet).
- Hook --build-staging now honours caller BUILD_CONTEXT (validated worktree) and
GIT_SHA, recreates orchestrator-staging on the fresh image and runs the 10x6s
health-check; build/health failure -> exit 1 (FAILED contract preserved).
- image_freshness.rebuild_staging_image: document why COMPOSE_PROFILE/TARGET_SERVICE/
TARGET_PORT are intentionally omitted (hook STAGING defaults -> 8501 only, P2).
- tests: assert the caller<->hook contract (builds from $BUILD_CONTEXT, no
`git rev-parse HEAD` recompute, recreates + health-checks 8501) so the P0
regression can't pass green again (P1).
Refs: ORCH-058
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -17,6 +17,17 @@ _HOOK = _ROOT / "scripts" / "orchestrator-deploy-hook.sh"
|
||||
_DOCKERFILE = _ROOT / "Dockerfile"
|
||||
|
||||
|
||||
def _build_staging_block() -> str:
|
||||
"""Return only the body of the hook's ``--build-staging`` branch, so the
|
||||
contract assertions below cannot be satisfied by lookalike strings elsewhere
|
||||
in the script (e.g. the NORMAL DEPLOY recreate). The block runs from the
|
||||
``--build-staging`` guard up to the NORMAL DEPLOY section header."""
|
||||
text = _HOOK.read_text(encoding="utf-8")
|
||||
start = text.index('"${1:-}" == "--build-staging"')
|
||||
end = text.index("NORMAL DEPLOY mode", start)
|
||||
return text[start:end]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TC-07: hook fail-closed provenance guard + --build-staging rebuild mode
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -49,6 +60,41 @@ def test_tc07_build_staging_mode_stamps_git_sha():
|
||||
assert 'docker build --build-arg GIT_SHA="$GIT_SHA"' in text
|
||||
|
||||
|
||||
def test_tc07_build_staging_builds_from_caller_context_not_repo():
|
||||
"""Contract (caller <-> hook): --build-staging must build from the
|
||||
caller-supplied BUILD_CONTEXT (the validated worktree), NOT the prod clone.
|
||||
|
||||
Regression guard for the P0 deadlock: the block must honour the caller's
|
||||
GIT_SHA (BUILD_CONTEXT/GIT_SHA defaulting) and must NOT recompute the SHA
|
||||
from the host clone's HEAD (`git rev-parse HEAD`) — on the
|
||||
deploy-staging -> deploy edge `main` HEAD != validated SHA, which would
|
||||
stamp the wrong revision label and deadlock the Strategy-B guard.
|
||||
"""
|
||||
block = _build_staging_block()
|
||||
# Build context is the caller-supplied worktree, defaulting to $REPO.
|
||||
assert 'BUILD_CONTEXT="${BUILD_CONTEXT:-$REPO}"' in block
|
||||
assert 'docker build --build-arg GIT_SHA="$GIT_SHA" -t "$TARGET_IMAGE" "$BUILD_CONTEXT"' in block
|
||||
# Honour the caller's GIT_SHA; never hard-build against the prod clone.
|
||||
assert 'GIT_SHA="${GIT_SHA:-}"' in block
|
||||
assert 'docker build --build-arg GIT_SHA="$GIT_SHA" -t "$TARGET_IMAGE" "$REPO"' not in block
|
||||
# Must NOT recompute the validated SHA from the host clone's HEAD.
|
||||
assert "git rev-parse HEAD" not in block
|
||||
|
||||
|
||||
def test_tc07_build_staging_recreates_and_health_checks_8501():
|
||||
"""AC-4: --build-staging must recreate the staging container on the fresh
|
||||
image and validate it (health-check), so rebuild_staging_image's rc=0 truly
|
||||
means "rebuilt AND healthy". A bare `docker build` + exit 0 would make the
|
||||
freshness verdict a lie."""
|
||||
block = _build_staging_block()
|
||||
# Recreate the staging service on the freshly built image.
|
||||
assert 'docker compose --profile "$COMPOSE_PROFILE" up -d --no-build "$TARGET_SERVICE"' in block
|
||||
# Validate the fresh container before reporting success.
|
||||
assert 'health_check 10 6 "build-staging-health"' in block
|
||||
# Health failure surfaces as a non-zero exit (FAILED contract preserved).
|
||||
assert "exit 1" in block
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TC-08: Dockerfile stamps the OCI revision label from a build-arg
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user