[digitalresistor]

Description

The scripts/deploy.sh script now exits with the failing command's status. It reaches the interactive config builder on the first run, and it treats "no" and "skip" answers as declines.

• The error_handler function read $? after local line_no=$1 reset it, so every failure exited with status 0. The ERR trap now passes $?, and the handler captures the status before it runs anything else.
• A missing config.env file on the first run made load_config fire the ERR trap before the config builder ran. The script now logs a warning and continues to the prerequisites step that writes the file.
• When you decline the prerequisites setup, the script now stops with status 1 if the config.env file is missing or lacks required values. Before this change, a missing or incomplete file also stopped the run, but with status 0.
• Answering "n" to the second bootstrap apply prompt fired the ERR trap. The default project and providers download prompts did the same. These calls now end in || true like the other optional prompts.
• Answering "s" took the yes branch of the old file cleanup prompt. The config.env overwrite and providers.tf generation prompts did the same. These prompts now call handle_prompt_if, which takes the yes branch only for "y".
• Choosing to overwrite config.env skipped every question because the sourced values satisfied each while [[ -z ]] loop. Each question now runs once and shows the current value as a default that Enter keeps.
• The prefix prompt now allows 7 characters. This matches the prefix validation in the fast/stages-aw/0-bootstrap/variables.tf file and the Detailed Deployment Guide in the docs/ddg.md file. The generated terraform.tfvars comment now says 7 instead of 9.
• The scripts/README.md file now lists 3-security instead of 3-project-factory as the last stage.

The first commit moves error_handler, handle_prompt, and handle_prompt_if from the scripts/deploy.sh script into the scripts/common-functions.sh library with unchanged bodies. The move exists only so the new shell unit tests can source the helpers. It stays in its own commit so git bisect separates it from the fixes.

The fix commit also makes these changes to the shared library.

• The prompt_with_default helper serves the config builder.
• The shared error_handler keeps writing LAST_FAILED_OPERATION=deploy by default, and an OPERATION_NAME variable overrides it.
• The handler falls back to the current directory when SCRIPT_DIR is unset and ignores a failed state directory creation.
• The handler calls the cleanup function only when one exists.
• The setup_error_handling function now installs the shared handler and keeps its cleanup function argument. It no longer defines a second error_handler with the same name. No script calls it today.

The scripts/destroy.sh script keeps its own copy of error_handler, and that copy has the same $? defect. This PR leaves that script unchanged to keep the scope on deploy.sh. A follow-up can switch it to the shared handler.

Open PR #299 renames the tests/scripts/test_common_functions.sh suite to tests/test_common_functions.sh. Whichever PR merges second needs to carry the new test cases across that rename. Open PR #286 changes neither the scripts/ directory nor the tests/ directory, so it does not conflict.

No tracking issue exists for these defects.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update

Deployment & Compliance Impact

• Applicable Regimes:
  • US Region Restricted (e.g., Access Policy constraint)
  • FedRAMP Moderate
  • FedRAMP High
  • DoD IL4
  • DoD IL5
  • General / All
• NIST 800-53r5 Controls: None. The change affects only the experimental deployment helper scripts.

Checklist

Code Quality & Reusability

• My code adheres to the Maximize Reusability principle. I have not redefined common elements and have reused existing base configurations and modules where possible.
• I have checked that no existing module or configuration in modules/ or fast/stages-aw/ can be leveraged for this change.
• My code follows the established naming conventions outlined in docs/naming-convention.md.

Documentation

• I have updated the README.md of the modified module or blueprint.
• I have added/updated documentation for inputs (variables) and outputs.

Security

• My change adheres to GCP security best practices and the principle of least privilege.
• I have ensured compliance with the targeted regime (FedRAMP Moderate, FedRAMP High, IL4, IL5, etc.).

Testing

• I have tested my changes locally.
• I have included details of my testing in this PR.

The inputs and outputs item stays unchecked because no Terraform variables or outputs change.

Testing Performed

• bash tools/run_tests.sh passes. The Python unit tests, the shell suites, and the Python compile check all pass.
• bash tests/scripts/run_shell_tests.sh passes all 49 unit tests, including 30 new tests.
  • The error_handler tests check the exit status with and without the explicit $? argument, the state file record, and the cleanup call.
  • The setup_error_handling tests check that it installs the shared handler and runs the named cleanup function.
  • The prompt tests check that "n" answers leave the ERR trap untouched, that "s" answers skip the yes branch of handle_prompt_if, and that the NON_INTERACTIVE defaults stay unchanged.
  • The prompt_with_default tests check the default value, typed input, a final line without a newline, and a closed stdin.
  • Static checks on scripts/deploy.sh assert that no yes branch calls handle_prompt, that every bare handle_prompt call ends in || true, and that the ERR trap passes $?.
• The same suite also passes under the macOS system bash 3.2.
• At the move-only commit, the shell suite from main passes with 19 of 19 tests.
• Mutation checks confirm that the tests catch each defect.
  • The error_handler scenario exits 0 with the handler from main and 7 with the fixed handler.
  • Reverting one handle_prompt_if call site, one || true suffix, or the trap argument in scripts/deploy.sh fails the matching static check.
• Smoke runs of scripts/deploy.sh use stub gcloud and terraform commands on PATH, scripted answers, and placeholder values such as user@example.com.
  • A first run without config.env reaches the builder, rejects an 8-character prefix, accepts a 7-character prefix, and writes the file. The same run on main stops at load_config, reports exit code 0, and writes no file.
  • An overwrite with Enter for every question leaves config.env byte-identical.
  • An overwrite answered with "s" keeps the existing file and skips the builder.
  • A closed stdin at a press-any-key prompt now exits 1 through the ERR trap, while main exits 0 at the same prompt.
  • Declining the prerequisites setup without config.env, or with a config.env that lacks required values, exits 1 before Stage 0.
• bash -n passes for every script in the scripts/ directory.
• shellcheck -S warning reports the same warnings per file as main for the three changed shell files.
• pytest -q passes all 66 tests.
• python3 tools/check_boilerplate.py scripts tests tools reports the missing license header in scripts/deploy.sh and scripts/destroy.sh. Both files lack the header on main too, and this PR adds no new files.
• No end-to-end deployment against a live organization ran for this change.

[aghassemlouei]

Verified that capturing $? before local variable declarations in error_handler preserves the failing exit status, handle_prompt_if prevents skip answers from entering yes branches, prompt_with_default re-prompts existing values when overwriting config.env, and all 49 shell unit tests pass.