fix: extract apply logic to script file for CI log visibility

ldraney commented

2026-06-16 12:37:09 +00:00

Owner

Summary

Extract the apply step's init + apply logic into a script file to fix CI log visibility.

Changes

Extracts init + apply logic into /tmp/apply.sh via single-quoted heredoc (same pattern as existing post-comment.sh)
Redirects tofu init output to a file — only shown on failure, saving ~40 lines of ANSI-heavy provider download output from the log buffer
The actual apply error output (tail -100) now survives Woodpecker's log buffer limit

Every push-to-main pipeline since PR #446 showed logs ending at "Running full apply" with no apply output visible. The Woodpecker trace echo plus tofu init ANSI output consumed the entire log buffer before tail -100 could appear.

Test Plan

PR pipeline passes (validate + plan steps)
After merge, push-to-main pipeline shows === APPLY OUTPUT (last 100 lines) === in logs
If apply fails, the actual error is visible in the tail output

Review Checklist

No functional change to apply logic — same init, same apply, same lock retry
Follows existing post-comment.sh heredoc pattern
Single $ inside <<'APPLYEOF' heredoc (no Woodpecker $$ escaping needed)

Follow-up to PR #447 which added tail -100 but didn't solve the underlying log buffer exhaustion

Closes #448

## Summary Extract the apply step's init + apply logic into a script file to fix CI log visibility. ## Changes - Extracts init + apply logic into `/tmp/apply.sh` via single-quoted heredoc (same pattern as existing `post-comment.sh`) - Redirects `tofu init` output to a file — only shown on failure, saving ~40 lines of ANSI-heavy provider download output from the log buffer - The actual apply error output (`tail -100`) now survives Woodpecker's log buffer limit Every push-to-main pipeline since PR #446 showed logs ending at "Running full apply" with no apply output visible. The Woodpecker trace echo plus `tofu init` ANSI output consumed the entire log buffer before `tail -100` could appear. ## Test Plan - [ ] PR pipeline passes (validate + plan steps) - [ ] After merge, push-to-main pipeline shows `=== APPLY OUTPUT (last 100 lines) ===` in logs - [ ] If apply fails, the actual error is visible in the tail output ## Review Checklist - [x] No functional change to apply logic — same init, same apply, same lock retry - [x] Follows existing `post-comment.sh` heredoc pattern - [x] Single `$` inside `<<'APPLYEOF'` heredoc (no Woodpecker `$$` escaping needed) ## Related Notes - Follow-up to PR #447 which added `tail -100` but didn't solve the underlying log buffer exhaustion Closes #448

ldraney added 1 commit

2026-06-16 12:37:09 +00:00

fix: extract apply logic to script file for CI log visibility

ci/woodpecker/push/terraform Pipeline was successful

Details

ci/woodpecker/pr/terraform Pipeline was successful

Details

ci/woodpecker/pull_request_closed/terraform Pipeline was successful

Details

16923176d0

Woodpecker trace echo + tofu init ANSI output filled the log buffer
before the apply output (tail -100) could appear. Every push-to-main
pipeline since #446 showed "Running full apply" then nothing — the
actual error was invisible.

Fix: write init + apply logic to /tmp/apply.sh via heredoc (same
pattern as the existing post-comment.sh). Init output is redirected
to a file and only shown on failure. The script execution produces
compact output that fits within Woodpecker's log buffer.

Closes #448

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ldraney commented

2026-06-16 12:39:46 +00:00

Author

Owner

PR #448 Review

DOMAIN REVIEW

Tech stack: Woodpecker CI / shell scripting / OpenTofu (Terraform Plugin Framework context)

Heredoc pattern consistency. The new apply.sh heredoc follows the same cat > /tmp/script.sh <<'DELIMITER' pattern already used by post-comment.sh (lines 229-260). Single-quoted delimiter prevents shell expansion during write, so $ is correct inside the body. The $$ to $ conversion across all variable references is consistent and complete -- I count 16 substitutions and all are accounted for.

Init output suppression. Redirecting tofu init to /tmp/init-output.txt and only showing it on failure is a sound approach. The nested retry logic (if ! init; then sleep 3; if ! init; then show output + exit 1) is an improvement over the old || (sleep 3 && init) one-liner because it now captures and displays the failure output. The old pattern silently swallowed init errors.

cd terraform placement. The old code had cd terraform as a separate Woodpecker command step (line 394 in main). The new code moves it inside the script. This is correct -- the script runs in a subshell via /bin/sh /tmp/apply.sh, so the cd needs to be inside that script's scope.

Environment variable inheritance. The post-comment.sh pattern explicitly passes Woodpecker CI variables (FORGEJO_TOKEN, CI_REPO, CI_COMMIT_PULL_REQUEST) on the invocation line because those are $$-escaped CI variables that need Woodpecker expansion at invocation time. The apply.sh script does not reference any CI_* variables -- it only uses file-based state (/tmp/apply-mode.txt, /tmp/changed-modules.txt) and TF_VAR_* environment variables inherited from Woodpecker's environment: block. No explicit passing needed. Correct.

Lock retry path. The lock-retry logic is unchanged in substance. The retry eval $APPLY_CMD on line after tofu force-unlock still runs without output capture -- same as before. This means a second failure during retry won't be captured in /tmp/apply-output.txt. This is a pre-existing concern, not introduced by this PR.

BLOCKERS

None.

NITS

Missing set -e in apply.sh. The post-comment.sh script includes set -e (fail-fast on error). The apply.sh script omits it. This is likely intentional since the script manually checks exit codes (echo $? > /tmp/apply-exit), but it is worth noting the inconsistency. The set -e would actually be incompatible with the eval $APPLY_CMD > /tmp/apply-output.txt 2>&1 ; echo $? > /tmp/apply-exit pattern since set -e would abort on the non-zero eval before capturing the exit code. So the omission is correct. No action needed.
Retry apply output not captured. The lock-retry path runs eval $APPLY_CMD without redirecting to a file or tailing output. If the retry also fails, its output goes to stdout without the "last 100 lines" formatting. This is pre-existing behavior not introduced by this PR, but could be a follow-up improvement.

SOP COMPLIANCE

PR body has Summary, Changes, Test Plan, Related sections
No secrets or credentials committed
Scope is tight -- single file, single concern (log visibility)
Commit message is descriptive ("fix: extract apply logic to script file for CI log visibility")
Test plan is appropriate for CI infrastructure changes (pipeline validation)

PROCESS OBSERVATIONS

This is a targeted fix for a real CI observability problem (apply output invisible since PR #446). Fast follow-up to PR #447 which partially addressed it.
Change failure risk is low: no functional change to apply logic, only restructuring how the shell executes. The tofu init change is actually an improvement (error output now visible on failure).
The heredoc-extraction pattern is now established for two scripts in this pipeline (post-comment.sh and apply.sh), creating a clear convention for future CI script additions.

VERDICT: APPROVED

## PR #448 Review ### DOMAIN REVIEW **Tech stack:** Woodpecker CI / shell scripting / OpenTofu (Terraform Plugin Framework context) **Heredoc pattern consistency.** The new `apply.sh` heredoc follows the same `cat > /tmp/script.sh <<'DELIMITER'` pattern already used by `post-comment.sh` (lines 229-260). Single-quoted delimiter prevents shell expansion during write, so `$` is correct inside the body. The `$$` to `$` conversion across all variable references is consistent and complete -- I count 16 substitutions and all are accounted for. **Init output suppression.** Redirecting `tofu init` to `/tmp/init-output.txt` and only showing it on failure is a sound approach. The nested retry logic (`if ! init; then sleep 3; if ! init; then show output + exit 1`) is an improvement over the old `|| (sleep 3 && init)` one-liner because it now captures and displays the failure output. The old pattern silently swallowed init errors. **cd terraform placement.** The old code had `cd terraform` as a separate Woodpecker command step (line 394 in main). The new code moves it inside the script. This is correct -- the script runs in a subshell via `/bin/sh /tmp/apply.sh`, so the `cd` needs to be inside that script's scope. **Environment variable inheritance.** The `post-comment.sh` pattern explicitly passes Woodpecker CI variables (`FORGEJO_TOKEN`, `CI_REPO`, `CI_COMMIT_PULL_REQUEST`) on the invocation line because those are `$$`-escaped CI variables that need Woodpecker expansion at invocation time. The `apply.sh` script does not reference any `CI_*` variables -- it only uses file-based state (`/tmp/apply-mode.txt`, `/tmp/changed-modules.txt`) and `TF_VAR_*` environment variables inherited from Woodpecker's `environment:` block. No explicit passing needed. Correct. **Lock retry path.** The lock-retry logic is unchanged in substance. The retry `eval $APPLY_CMD` on line after `tofu force-unlock` still runs without output capture -- same as before. This means a second failure during retry won't be captured in `/tmp/apply-output.txt`. This is a pre-existing concern, not introduced by this PR. ### BLOCKERS None. ### NITS 1. **Missing `set -e` in apply.sh.** The `post-comment.sh` script includes `set -e` (fail-fast on error). The `apply.sh` script omits it. This is likely intentional since the script manually checks exit codes (`echo $? > /tmp/apply-exit`), but it is worth noting the inconsistency. The `set -e` would actually be incompatible with the `eval $APPLY_CMD > /tmp/apply-output.txt 2>&1 ; echo $? > /tmp/apply-exit` pattern since `set -e` would abort on the non-zero eval before capturing the exit code. So the omission is correct. No action needed. 2. **Retry apply output not captured.** The lock-retry path runs `eval $APPLY_CMD` without redirecting to a file or tailing output. If the retry also fails, its output goes to stdout without the "last 100 lines" formatting. This is pre-existing behavior not introduced by this PR, but could be a follow-up improvement. ### SOP COMPLIANCE - [x] PR body has Summary, Changes, Test Plan, Related sections - [x] No secrets or credentials committed - [x] Scope is tight -- single file, single concern (log visibility) - [x] Commit message is descriptive ("fix: extract apply logic to script file for CI log visibility") - [x] Test plan is appropriate for CI infrastructure changes (pipeline validation) ### PROCESS OBSERVATIONS - This is a targeted fix for a real CI observability problem (apply output invisible since PR #446). Fast follow-up to PR #447 which partially addressed it. - Change failure risk is low: no functional change to apply logic, only restructuring how the shell executes. The `tofu init` change is actually an improvement (error output now visible on failure). - The heredoc-extraction pattern is now established for two scripts in this pipeline (`post-comment.sh` and `apply.sh`), creating a clear convention for future CI script additions. ### VERDICT: APPROVED