diff --git a/scripts/force-delete-owned-ecr-repo.sh b/scripts/force-delete-owned-ecr-repo.sh index e13e13c0..a733a5db 100755 --- a/scripts/force-delete-owned-ecr-repo.sh +++ b/scripts/force-delete-owned-ecr-repo.sh @@ -16,8 +16,8 @@ # the whole safety property here, so it stays visible at each call site. # # The owned name is read from `terraform output` on the state the destroy is -# about to tear down and compared by exact equality against every repository in -# the account, by scripts/select-owned-name.sh. The callers used to +# about to tear down, looked up by that name alone, and the response compared by +# exact equality against it by scripts/select-owned-name.sh. The callers used to # select by the `cudly-dev*` / `cudly-staging*` prefix, which also matches # `cudly-staging-prod-mirror` and `cudly-staging--backup` and force-deleted # every image in them (#1592, #1820). The prefix also spans both staging states: @@ -40,9 +40,9 @@ # callers used to carry reported success after a failed listing or a failed # delete, and a failed listing is indistinguishable from an empty account, so # the cleanup did nothing and `terraform destroy` then failed on the images -# still in the repository. "Already gone" needs no swallowing: the repository is -# simply absent from the listing, the selector prints nothing and exits 0, and -# the loop body never runs. +# still in the repository. "Already gone" is the one lookup error that is +# recognised, by name (RepositoryNotFoundException), and reported as a normal +# exit; every other error fails the script with the AWS CLI's own exit code. # # Exit codes: # 0 cleanup completed, including the "state already destroyed" and "nothing @@ -82,7 +82,29 @@ fi OWNED_REPO="$(jq -er '.ecr_repository_name.value' <<<"$OUTPUTS_JSON")" echo "This state owns ECR repository '$OWNED_REPO'" -aws ecr describe-repositories --query 'repositories[].repositoryName' --output text \ +# Asks for the owned repository by name rather than listing the account. A +# DescribeRepositories without --repository-names is authorized against +# repository/*, and the deploy role's ECR grant covers only repository/cudly-* +# (terraform/environments/aws/ci-cd-permissions/policy_compute.tf, +# ECRRepositoryScoped), so the listing was denied before the selector ever ran +# (#483). The response still goes through the selector, so "only the owned +# repository is ever deleted" stays a property of this script rather than of +# the API's filtering. +DESCRIBE_ERR="$(mktemp)" +trap 'rm -f "$DESCRIBE_ERR"' EXIT +DESCRIBE_EXIT=0 +LISTING="$(aws ecr describe-repositories --repository-names "$OWNED_REPO" \ + --query 'repositories[].repositoryName' --output text 2>"$DESCRIBE_ERR")" || DESCRIBE_EXIT=$? +if [[ "$DESCRIBE_EXIT" -ne 0 ]]; then + if grep -q 'RepositoryNotFoundException' "$DESCRIBE_ERR"; then + echo "ECR repository '$OWNED_REPO' does not exist; it is already deleted and there is nothing to clean up." + exit 0 + fi + cat "$DESCRIBE_ERR" >&2 + exit "$DESCRIBE_EXIT" +fi + +printf '%s\n' "$LISTING" \ | tr '\t' '\n' \ | "${SCRIPT_DIR}/select-owned-name.sh" "$OWNED_REPO" \ | while IFS= read -r REPO; do diff --git a/scripts/test-ecr-delete-selection.sh b/scripts/test-ecr-delete-selection.sh index 9e552ac7..41bb98a1 100755 --- a/scripts/test-ecr-delete-selection.sh +++ b/scripts/test-ecr-delete-selection.sh @@ -662,6 +662,134 @@ EOF assert_sweep "a finding names the file it came from, not the file swept after it" \ "${FIXTURE_DIR}/misattrib" 'a-unwired.yml: step "Force-delete all staging ECR repos"' +# --- Behaviour: the script run end to end against stubbed terraform and aws --- +# +# The deploy role's ECR grant covers only arn:aws:ecr:*:*:repository/cudly-* +# (terraform/environments/aws/ci-cd-permissions/policy_compute.tf, +# ECRRepositoryScoped). A DescribeRepositories without --repository-names is +# authorized against repository/*, so the role is denied it, which is how every +# real destroy failed before the selector ran (#483). The aws stub reproduces +# that denial, so a script that goes back to listing the account fails here +# rather than on the next live destroy. +STUB_DIR="$(mktemp -d)" +STUB_STATE="$(mktemp -d)" +STUB_CALLS="$(mktemp)" +trap 'rm -rf "$FIXTURE_DIR" "$STUB_DIR" "$STUB_STATE" "$STUB_CALLS"' EXIT + +cat >"${STUB_DIR}/terraform" <<'EOF' +#!/usr/bin/env bash +printf '%s' "$TF_OUTPUT_JSON" +EOF + +cat >"${STUB_DIR}/aws" <<'EOF' +#!/usr/bin/env bash +echo "aws $*" >>"$AWS_CALLS" +case "$2" in + describe-repositories) + requested="" + while [[ $# -gt 0 ]]; do + [[ "$1" == "--repository-names" ]] && requested="$2" + shift + done + if [[ -z "$requested" ]]; then + echo "An error occurred (AccessDeniedException) when calling the DescribeRepositories operation: User: arn:aws:sts::111111111111:assumed-role/cudly-terraform-deploy/GitHubActions is not authorized to perform: ecr:DescribeRepositories on resource: arn:aws:ecr:us-east-1:111111111111:repository/*" >&2 + exit 254 + fi + case "$DESCRIBE_MODE" in + found) printf '%s\n' "$requested" ;; + other) printf '%s\n' "${requested}-backup" ;; + missing) + echo "An error occurred (RepositoryNotFoundException) when calling the DescribeRepositories operation: The repository with name '${requested}' does not exist in the registry with id '111111111111'" >&2 + exit 254 + ;; + error) + echo "An error occurred (ThrottlingException) when calling the DescribeRepositories operation: Rate exceeded" >&2 + exit 254 + ;; + esac + ;; + delete-repository) + [[ "${DELETE_FAILS:-0}" == "1" ]] && { echo "delete failed" >&2; exit 254; } + ;; +esac +exit 0 +EOF +chmod +x "${STUB_DIR}/terraform" "${STUB_DIR}/aws" + +export AWS_CALLS="$STUB_CALLS" +export TF_OUTPUT_JSON='{"ecr_repository_name":{"value":"cudly-dev-1a2b3c4d"}}' + +# run_script -> STUB_EXIT, STUB_OUT, STUB_ERR, and a truncated call log +run_script() { + : >"$STUB_CALLS" + local outfile errfile + outfile="$(mktemp)" + errfile="$(mktemp)" + STUB_EXIT=0 + PATH="${STUB_DIR}:${PATH}" "$CLEANUP_SCRIPT" "$@" >"$outfile" 2>"$errfile" || STUB_EXIT=$? + STUB_OUT="$(cat "$outfile")" + STUB_ERR="$(cat "$errfile")" + rm -f "$outfile" "$errfile" +} + +# assert_behaviour LABEL CONDITION_RESULT DETAIL +assert_behaviour() { + if [[ "$2" == "0" ]]; then + echo "PASS: $1" + ((pass++)) || true + else + echo "FAIL: $1" + echo " $3" + ((fail++)) || true + fi +} + +count_calls() { grep -c -- "$1" "$STUB_CALLS" 2>/dev/null || true; } + +export DESCRIBE_MODE=found +run_script "$STUB_STATE" +assert_behaviour "behaviour: the owned repository is looked up by name, not by listing the account" \ + "$([[ "$STUB_EXIT" -eq 0 ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, stderr: ${STUB_ERR}" +assert_behaviour "behaviour: exactly the owned repository is force-deleted" \ + "$([[ "$(count_calls 'delete-repository')" -eq 1 ]] && grep -q 'aws ecr delete-repository --repository-name cudly-dev-1a2b3c4d --force' "$STUB_CALLS" && echo 0 || echo 1)" \ + "calls: $(cat "$STUB_CALLS")" + +# The response is still passed through the exact-match selector, so a name the +# API returns that is not the owned one is never deleted. +export DESCRIBE_MODE=other +run_script "$STUB_STATE" +assert_behaviour "behaviour: a returned name other than the owned one is not deleted" \ + "$([[ "$STUB_EXIT" -eq 0 && "$(count_calls 'delete-repository')" -eq 0 ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, calls: $(cat "$STUB_CALLS")" + +# Re-running a cleanup after a completed one is normal, not an error. +export DESCRIBE_MODE=missing +run_script "$STUB_STATE" +assert_behaviour "behaviour: RepositoryNotFoundException exits 0 and deletes nothing" \ + "$([[ "$STUB_EXIT" -eq 0 && "$(count_calls 'delete-repository')" -eq 0 ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, stderr: ${STUB_ERR}, calls: $(cat "$STUB_CALLS")" +assert_behaviour "behaviour: the already-deleted case says so" \ + "$([[ "$STUB_OUT" == *"already deleted"* ]] && echo 0 || echo 1)" \ + "stdout: ${STUB_OUT}" + +# Any other lookup error must fail the step and surface the AWS message, not +# read as "already deleted". +export DESCRIBE_MODE=error +run_script "$STUB_STATE" +assert_behaviour "behaviour: any other lookup error fails the step and deletes nothing" \ + "$([[ "$STUB_EXIT" -ne 0 && "$(count_calls 'delete-repository')" -eq 0 ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, calls: $(cat "$STUB_CALLS")" +assert_behaviour "behaviour: the lookup error's AWS message reaches stderr" \ + "$([[ "$STUB_ERR" == *"ThrottlingException"* ]] && echo 0 || echo 1)" \ + "stderr: ${STUB_ERR}" + +export DESCRIBE_MODE=found DELETE_FAILS=1 +run_script "$STUB_STATE" +assert_behaviour "behaviour: a failed delete fails the step" \ + "$([[ "$STUB_EXIT" -ne 0 ]] && echo 0 || echo 1)" "exit ${STUB_EXIT}" +unset DELETE_FAILS + echo echo "passed: ${pass}, failed: ${fail}" [[ "$fail" -eq 0 ]]