From 523c01e4d36394e9a3514ac2b7cdc26afbf26a03 Mon Sep 17 00:00:00 2001 From: owine Date: Thu, 9 Jul 2026 17:27:25 -0500 Subject: [PATCH 1/2] fix(deploy): don't wrap down/ps/logs compose calls in op run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Teardown removed stacks" step runs before the git reset, so it reads the previous compose.env. When a commit removes a stack and also drops a now-orphaned var whose 1Password item was already archived, `op run` fails to resolve that dead ref and aborts โ€” before `docker compose down` executes. The removed stack's containers are never stopped (the failure is only a ::warning::, so the job stays green), and CI can't self-heal on a later run because the stack is no longer detected as removed. `docker compose down`/`ps`/`logs` operate on already-created resources by compose project label and need no resolved secrets โ€” only `up` genuinely requires op run to inject env into new containers. Every non-`up` call had inherited the op run wrapper by copy-paste consistency (git history shows no secrets-driven rationale in any of the introducing commits; the adjacent `docker inspect` calls were already unwrapped). Strip `op run --env-file` from all 17 inline down/ps/logs calls, leaving the 4 `up` calls wrapped. Decouples teardown and failure diagnostics from compose.env resolvability so a dead ref can't strand containers or hide diagnostics. --- .github/workflows/deploy.yml | 43 ++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index c46126b..0da9791 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -235,6 +235,15 @@ jobs: if: | steps.skip-gate.outputs.skipped == 'false' && env.HAS_REMOVED == 'true' + # NB: `down` is intentionally NOT wrapped in `op run`. This step runs + # pre-reset, so it reads the *previous* compose.env โ€” which may still + # reference a 1Password item that's since been archived (e.g. the same + # commit that removes a stack also drops its now-orphaned vars). `op run` + # resolves the whole env-file up front and aborts on any dead ref, which + # would skip the teardown entirely and strand the removed stack's + # containers. `down` identifies resources by compose project label and + # needs no resolved secrets, so run it directly. Same rationale applies + # to every `down`/`ps`/`logs` call in this workflow โ€” only `up` needs op run. run: | set -euo pipefail for stack in $(echo "$REMOVED_STACKS" | jq -r '.[]'); do @@ -248,7 +257,7 @@ jobs: done if [[ -n "$compose_file" ]]; then echo "๐Ÿ›‘ Stopping $stack" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose -f "$compose_file" down || echo "::warning::down failed for $stack" + docker compose -f "$compose_file" down || echo "::warning::down failed for $stack" else echo "::warning::compose file missing for removed stack $stack" fi @@ -363,20 +372,20 @@ jobs: local lines="${FAILED_LOG_LINES:-50}" echo "::group::๐Ÿ” Diagnostics: $stack" echo "โ”€โ”€ docker compose ps -a โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a 2>&1 || true + docker compose ps -a 2>&1 || true local rows - rows=$(op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a --format json 2>/dev/null \ + rows=$(docker compose ps -a --format json 2>/dev/null \ | jq -rs '.[] | select(.Health=="unhealthy" or (.State=="exited" and (.ExitCode//0)!=0) or (.Health=="" and .State!="running" and .State!="exited")) | [.Service,.Name,.State,(.Health//""),((.ExitCode//0)|tostring)] | @tsv' 2>/dev/null || true) if [[ -z "$rows" ]]; then echo "โ”€โ”€ (no specific failing container identified; tailing all services) โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose logs --tail "$lines" --no-color 2>&1 || true + docker compose logs --tail "$lines" --no-color 2>&1 || true else while IFS=$'\t' read -r service name state health exit_code; do [[ -z "$service" ]] && continue echo "โ”€โ”€ โŒ $service (container=$name state=$state health=${health:-none} exit=$exit_code) โ”€โ”€" docker inspect --format '{{if .State.Health}}healthcheck history:{{"\n"}}{{range $i,$h := .State.Health.Log}} [#{{$i}}] exit={{$h.ExitCode}} start={{$h.Start}}{{"\n"}} {{$h.Output}}{{"\n"}}{{end}}{{else}}(no healthcheck defined){{end}}' "$name" 2>/dev/null || true echo "โ”€โ”€ container logs: $service (last $lines lines) โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose logs --tail "$lines" --no-color "$service" 2>&1 || true + docker compose logs --tail "$lines" --no-color "$service" 2>&1 || true done <<< "$rows" fi echo "::endgroup::" @@ -444,7 +453,7 @@ jobs: [[ "$stack" =~ ^[a-zA-Z0-9._-]+$ ]] || { echo "::warning::skipping invalid stack: $stack"; continue; } cd "$LIVE_REPO_PATH/$stack" 2>/dev/null || continue echo "๐Ÿงน Tearing down failed existing stack $stack to clear recreate orphans" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose down || true + docker compose down || true done - name: Deploy new stacks @@ -465,20 +474,20 @@ jobs: local lines="${FAILED_LOG_LINES:-50}" echo "::group::๐Ÿ” Diagnostics: $stack" echo "โ”€โ”€ docker compose ps -a โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a 2>&1 || true + docker compose ps -a 2>&1 || true local rows - rows=$(op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a --format json 2>/dev/null \ + rows=$(docker compose ps -a --format json 2>/dev/null \ | jq -rs '.[] | select(.Health=="unhealthy" or (.State=="exited" and (.ExitCode//0)!=0) or (.Health=="" and .State!="running" and .State!="exited")) | [.Service,.Name,.State,(.Health//""),((.ExitCode//0)|tostring)] | @tsv' 2>/dev/null || true) if [[ -z "$rows" ]]; then echo "โ”€โ”€ (no specific failing container identified; tailing all services) โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose logs --tail "$lines" --no-color 2>&1 || true + docker compose logs --tail "$lines" --no-color 2>&1 || true else while IFS=$'\t' read -r service name state health exit_code; do [[ -z "$service" ]] && continue echo "โ”€โ”€ โŒ $service (container=$name state=$state health=${health:-none} exit=$exit_code) โ”€โ”€" docker inspect --format '{{if .State.Health}}healthcheck history:{{"\n"}}{{range $i,$h := .State.Health.Log}} [#{{$i}}] exit={{$h.ExitCode}} start={{$h.Start}}{{"\n"}} {{$h.Output}}{{"\n"}}{{end}}{{else}}(no healthcheck defined){{end}}' "$name" 2>/dev/null || true echo "โ”€โ”€ container logs: $service (last $lines lines) โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose logs --tail "$lines" --no-color "$service" 2>&1 || true + docker compose logs --tail "$lines" --no-color "$service" 2>&1 || true done <<< "$rows" fi echo "::endgroup::" @@ -536,7 +545,7 @@ jobs: for stack in $(echo "$NEW_STACKS" | jq -r '.[]'); do [[ "$stack" =~ ^[a-zA-Z0-9._-]+$ ]] || { echo "::warning::skipping invalid stack: $stack"; continue; } cd "$LIVE_REPO_PATH/$stack" 2>/dev/null || continue - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose down || true + docker compose down || true done - name: Compute deploy summary outputs @@ -593,20 +602,20 @@ jobs: local lines="${FAILED_LOG_LINES:-50}" echo "::group::๐Ÿ” Diagnostics: $stack" echo "โ”€โ”€ docker compose ps -a โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a 2>&1 || true + docker compose ps -a 2>&1 || true local rows - rows=$(op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a --format json 2>/dev/null \ + rows=$(docker compose ps -a --format json 2>/dev/null \ | jq -rs '.[] | select(.Health=="unhealthy" or (.State=="exited" and (.ExitCode//0)!=0) or (.Health=="" and .State!="running" and .State!="exited")) | [.Service,.Name,.State,(.Health//""),((.ExitCode//0)|tostring)] | @tsv' 2>/dev/null || true) if [[ -z "$rows" ]]; then echo "โ”€โ”€ (no specific failing container identified; tailing all services) โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose logs --tail "$lines" --no-color 2>&1 || true + docker compose logs --tail "$lines" --no-color 2>&1 || true else while IFS=$'\t' read -r service name state health exit_code; do [[ -z "$service" ]] && continue echo "โ”€โ”€ โŒ $service (container=$name state=$state health=${health:-none} exit=$exit_code) โ”€โ”€" docker inspect --format '{{if .State.Health}}healthcheck history:{{"\n"}}{{range $i,$h := .State.Health.Log}} [#{{$i}}] exit={{$h.ExitCode}} start={{$h.Start}}{{"\n"}} {{$h.Output}}{{"\n"}}{{end}}{{else}}(no healthcheck defined){{end}}' "$name" 2>/dev/null || true echo "โ”€โ”€ container logs: $service (last $lines lines) โ”€โ”€" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose logs --tail "$lines" --no-color "$service" 2>&1 || true + docker compose logs --tail "$lines" --no-color "$service" 2>&1 || true done <<< "$rows" fi echo "::endgroup::" @@ -615,7 +624,7 @@ jobs: failed=() for stack in $(echo "$CRITICAL_STACKS" | jq -r '.[]'); do cd "$LIVE_REPO_PATH/$stack" - services=$(op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose ps -a --format json | jq -s '.') + services=$(docker compose ps -a --format json | jq -s '.') # Unhealthy if: explicit "unhealthy", non-running with no healthcheck (and not a clean one-shot exit), # or exited with non-zero code. Exited+0 is a one-shot success (e.g. alembic migrations gated via # service_completed_successfully) and must not fail the gate. @@ -699,7 +708,7 @@ jobs: done if [[ -n "$compose_file" ]]; then echo "๐Ÿ›‘ Tearing down new stack $stack before rollback" - op run --no-masking --env-file="$LIVE_REPO_PATH/compose.env" -- docker compose -f "$compose_file" down || echo "::warning::down failed for $stack" + docker compose -f "$compose_file" down || echo "::warning::down failed for $stack" fi done From 1f0637c7cba97bbf5e543db5bcf32a4e150fa3a5 Mon Sep 17 00:00:00 2001 From: owine Date: Thu, 9 Jul 2026 17:31:30 -0500 Subject: [PATCH 2/2] docs(deploy): tighten teardown op-run comment; note project-naming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Sourcery review: condense the rationale comment and document that default compose project naming (stack dir basename) is what down/ps/logs rely on โ€” callers set no COMPOSE_* vars, so dropping op run doesn't change which project these commands target. --- .github/workflows/deploy.yml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 0da9791..962db94 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -235,15 +235,14 @@ jobs: if: | steps.skip-gate.outputs.skipped == 'false' && env.HAS_REMOVED == 'true' - # NB: `down` is intentionally NOT wrapped in `op run`. This step runs - # pre-reset, so it reads the *previous* compose.env โ€” which may still - # reference a 1Password item that's since been archived (e.g. the same - # commit that removes a stack also drops its now-orphaned vars). `op run` - # resolves the whole env-file up front and aborts on any dead ref, which - # would skip the teardown entirely and strand the removed stack's - # containers. `down` identifies resources by compose project label and - # needs no resolved secrets, so run it directly. Same rationale applies - # to every `down`/`ps`/`logs` call in this workflow โ€” only `up` needs op run. + # `down`/`ps`/`logs` here (and throughout this workflow) intentionally + # skip `op run` โ€” only `up` needs resolved secrets. Two reasons: + # 1. This step runs pre-reset, so it reads the *previous* compose.env; + # if that still references a since-archived 1P item (e.g. a stack + # removal that also drops its now-orphaned vars), `op run` aborts on + # the dead ref and the teardown never runs, stranding the containers. + # 2. `down` targets by compose project label (default: the stack dir + # name, same as `up`); callers set no COMPOSE_* vars, so no secrets. run: | set -euo pipefail for stack in $(echo "$REMOVED_STACKS" | jq -r '.[]'); do