fix(ci): let the RDS unprotect step pass on a state that never created its instance

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

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"