diff --git a/scripts/disable-owned-rds-deletion-protection.sh b/scripts/disable-owned-rds-deletion-protection.sh index c5463ba0..e37ea7f7 100755 --- a/scripts/disable-owned-rds-deletion-protection.sh +++ b/scripts/disable-owned-rds-deletion-protection.sh @@ -48,10 +48,18 @@ # -------------------+---------------+-----------+---------------+------------ # no state file | {} | 0 | exit 1 | skip, exit 0 # state, no outputs | {} | 0 | exit 1 | skip, exit 0 +# key absent, no | {...} w/o key | >=1 | exit 1 | skip, exit 0 +# aws_db_instance | | | | # key absent | {...} w/o key | >=1 | exit 1 | exit 1, remedy # key present, null | {"value":null}| 1 | exit 1 | exit 1, remedy # key present, "" | {"value":""} | 1 | exit 0, "" | exit 1, remedy # +# "Key absent" is split by `terraform state list`, not by the outputs: an apply +# that failed on aws_db_instance.main (e.g. InsufficientDBInstanceCapacity) +# stores the outputs of what it did create but not this one, which depends on +# the instance. No aws_db_instance in the state means nothing to unprotect; one +# present means a pre-#1821 state that owns an instance, and stays loud (#485). +# # The last row is the trap: an empty string is neither null nor false, so `jq # -er` accepts it and hands the caller a valid-looking empty identifier. It is # caught by its own check rather than left to the selector, for two reasons. The @@ -72,11 +80,13 @@ # never runs. # # Exit codes: -# 0 completed, including the "state already destroyed" and "instance already -# gone" cases, which are normal outcomes and not errors +# 0 completed, including the "state already destroyed", "state owns no RDS +# instance" and "instance already gone" cases, which are normal outcomes +# and not errors # 1 the owned identifier cannot be resolved from a state that has outputs: -# the key is absent or null (state predates the output), or it is present -# and empty (state or module defect). Distinct messages, distinct remedies. +# the key is absent (with an aws_db_instance in the state) or null (state +# predates the output), or it is present and empty (state or module +# defect). Distinct messages, distinct remedies. # 2 usage error (wrong arity, or a state directory that does not exist) # * anything the AWS CLI, terraform, jq or the selector fails with, unmasked @@ -114,6 +124,21 @@ fi # rather than a fallback: the only fallback available is the identifier prefix # this script exists to remove. if ! OWNED_INSTANCE="$(jq -er '.database_instance_identifier.value' <<<"$OUTPUTS_JSON")"; then + # A partially applied state that never created the instance publishes no + # identifier either; tell it apart by the resources the state holds. The + # address must END in an aws_db_instance resource (any module path), so a + # type that merely starts with the name, such as + # aws_db_instance_automated_backups_replication, does not count. A failing + # `state list` stops here under `set -e` rather than reading as "no instance". + HAS_IDENTIFIER_KEY="$(jq -r 'has("database_instance_identifier")' <<<"$OUTPUTS_JSON")" + if [[ "$HAS_IDENTIFIER_KEY" == "false" ]]; then + STATE_RESOURCES="$(terraform -chdir="$STATE_DIR" state list)" + DB_INSTANCE_COUNT="$(awk '/(^|\.)aws_db_instance\.[A-Za-z_][A-Za-z0-9_-]*(\[[^]]*\])?$/ { n++ } END { print n + 0 }' <<<"$STATE_RESOURCES")" + if [[ "$DB_INSTANCE_COUNT" -eq 0 ]]; then + echo "State has outputs but no aws_db_instance resource; this state owns no RDS instance to unprotect." + exit 0 + fi + fi echo "error: state '${STATE_DIR}' has outputs, but 'database_instance_identifier' is" >&2 echo " absent or null, so the instance this state owns cannot be identified." >&2 echo " This state was last applied before that output existed (#1821)." >&2 diff --git a/scripts/test-rds-deletion-protection-scope.sh b/scripts/test-rds-deletion-protection-scope.sh index 9b10de76..b7f76635 100755 --- a/scripts/test-rds-deletion-protection-scope.sh +++ b/scripts/test-rds-deletion-protection-scope.sh @@ -417,9 +417,19 @@ STUB_STATE="$(mktemp -d)" STUB_CALLS="$(mktemp)" trap 'rm -rf "$STUB_DIR" "$STUB_STATE" "$STUB_CALLS"' EXIT +# `state list` reads TF_STATE_LIST with `?`, so a case that reaches it without +# setting one fails loudly instead of passing on an empty (instance-less) state. cat >"${STUB_DIR}/terraform" <<'EOF' #!/usr/bin/env bash -printf '%s' "$TF_OUTPUT_JSON" +case "$2" in + output) printf '%s' "$TF_OUTPUT_JSON" ;; + state) + [[ "${STATE_LIST_FAILS:-0}" == "1" ]] && { echo "state list failed" >&2; exit 1; } + printf '%s\n' "${TF_STATE_LIST?TF_STATE_LIST not set by this case}" + ;; + *) echo "unexpected terraform call: $*" >&2; exit 99 ;; +esac +exit 0 EOF cat >"${STUB_DIR}/aws" <<'EOF' @@ -479,9 +489,10 @@ assert_behaviour "behaviour: a state with no outputs exits 0 without calling aws "$([[ "$STUB_EXIT" -eq 0 && ! -s "$STUB_CALLS" ]] && echo 0 || echo 1)" \ "exit ${STUB_EXIT}, calls: $(cat "$STUB_CALLS")" -# A state that predates the output. Distinct from the empty case below, because -# the remedies differ: this one wants an apply. +# A state that predates the output but owns an instance. Distinct from the +# empty case below, because the remedies differ: this one wants an apply. export TF_OUTPUT_JSON='{"ecr_repository_name":{"value":"cudly-dev-1a2b3c4d"}}' +export TF_STATE_LIST=$'aws_ecr_repository.main\nmodule.database.aws_db_instance.main\nmodule.networking.aws_vpc.main' run_script "$STUB_STATE" assert_behaviour "behaviour: a state missing the output exits 1 without calling aws" \ "$([[ "$STUB_EXIT" -eq 1 && ! -s "$STUB_CALLS" ]] && echo 0 || echo 1)" \ @@ -490,6 +501,38 @@ assert_behaviour "behaviour: the missing-output error tells the operator to re-a "$([[ "$STUB_ERR" == *"absent or null"* && "$STUB_ERR" == *"re-apply this state"* ]] && echo 0 || echo 1)" \ "stderr: ${STUB_ERR}" +# #485: an apply that failed on the instance itself (InsufficientDBInstanceCapacity) +# stores the outputs of what it did create but no identifier, and owns no +# instance. Nothing to unprotect, so the destroy must be allowed to proceed. +export TF_STATE_LIST=$'aws_ecr_repository.main\nmodule.networking.aws_vpc.main\nmodule.secrets.aws_secretsmanager_secret.db' +run_script "$STUB_STATE" +assert_behaviour "behaviour: outputs without the identifier and no aws_db_instance in state exit 0 without calling aws" \ + "$([[ "$STUB_EXIT" -eq 0 && ! -s "$STUB_CALLS" ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, stderr: ${STUB_ERR}, calls: $(cat "$STUB_CALLS")" + +# Names that contain the type but are not an aws_db_instance resource must not +# count as one, or the #485 state above would still exit 1. +export TF_STATE_LIST=$'aws_db_instance_automated_backups_replication.x\nmodule.database.aws_db_instance_role_association.main\nmodule.aws_db_instance_tools.aws_s3_bucket.main\naws_ssm_parameter.aws_db_instance' +run_script "$STUB_STATE" +assert_behaviour "behaviour: resources that merely contain 'aws_db_instance' do not count as an instance" \ + "$([[ "$STUB_EXIT" -eq 0 && ! -s "$STUB_CALLS" ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, stderr: ${STUB_ERR}, calls: $(cat "$STUB_CALLS")" + +# An indexed instance at the root still counts. +export TF_STATE_LIST=$'aws_ecr_repository.main\naws_db_instance.main[0]' +run_script "$STUB_STATE" +assert_behaviour "behaviour: an indexed root aws_db_instance with no identifier output exits 1" \ + "$([[ "$STUB_EXIT" -eq 1 && ! -s "$STUB_CALLS" ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, calls: $(cat "$STUB_CALLS")" + +# A failed `state list` is not an empty state. +export STATE_LIST_FAILS=1 +run_script "$STUB_STATE" +assert_behaviour "behaviour: a failed 'terraform state list' fails the step without calling aws" \ + "$([[ "$STUB_EXIT" -ne 0 && ! -s "$STUB_CALLS" ]] && echo 0 || echo 1)" \ + "exit ${STUB_EXIT}, calls: $(cat "$STUB_CALLS")" +unset STATE_LIST_FAILS TF_STATE_LIST + # `jq -er` accepts an empty string, so without its own check this reaches AWS. export TF_OUTPUT_JSON='{"database_instance_identifier":{"value":""}}' run_script "$STUB_STATE"