feat: surface terraform plan/init/validate warnings on PRs by Laffs2k5 · Pull Request #44 · dsb-norge/github-actions-terraform

Laffs2k5 · 2026-05-25T16:04:49Z

Motivation

Today, terraform Warning: diagnostics (deprecation notices, soft state drift, provider-level advisories) appear only in the raw job log. Reviewers reading the PR conversation never see them, and nothing draws the eye in the GitHub UI — deprecations harden into errors months later, when the provider does a major release, but the moment to act was when the warning first fired.

This branch makes warnings first-class on the PR: counted in the validation summary table, listed in a collapsible block under the plan extract, and emitted as ::warning annotations so they appear inline in the Files-changed view next to the offending source line.

See docs/Plan-warnings.md for the authoritative spec (data flow, warning-block grammar, annotation format, 65k budgeting algorithm, ARG_MAX caveat).

Summary of changes

New action parse-terraform-warnings/ — awk state machine that scans a terraform console-output file for Warning: blocks, emits one ::warning file=…,line=…::message per block (with file/line fallback when source context is absent), and renders a markdown body file ready for embedding in the PR plan-tag comment.
terraform-init/ and terraform-validate/ converted to modern step_*.sh layout (per docs/Action-implementation-guide.md) — both now tee their terraform invocations to a console-output file so parse-terraform-warnings has something to scan. Adds environment-name input to keep file names unique across matrix jobs.
create-validation-summary/ gains a ⚠️ Warnings row in the per-env head and a sibling <details>⚠️ N warnings</summary>…</details> collapser appended after the plan-block. Budgeting is warnings-first against a 65000-byte hard limit (warnings capped at 60k; plan-extract gets whatever remains). Replaces the latent tail -c 65000 cut with a line-anchored variant — fixes a UTF-8 mid-codepoint bug that predates this feature.
aggregate-validation-summaries/ gains a ⚠️ Warnings row in the grouped table, summing per-env warning counts across the three parse-*-warnings step ids. Em-dash — cell convention for "clean envs" / "step didn't run" mirrors the existing Plan time row.
.github/workflows/terraform-ci-cd-default.yml wires three parse-terraform-warnings invocations after init/validate/plan (per-step extraction so each gets its own 10-annotation GHA budget — a noisy init can't crowd out plan-warning annotations), plus an inline concat-warnings-md shell step that aggregates the three markdown files and sums the per-step counts. The arithmetic lives in shell because GHA workflow expressions don't support + between fromJSON() values.

Follow-up refinements (in this same PR)

aggregate-validation-summaries/: omit Warnings + Plan-details rows when empty — the grouped table now suppresses the ⚠️ Warnings row entirely when no env in the group has warnings, and the 📊 Plan details row when no env has plan data. Matches the per-env head, which already gates both. When a row IS shown, clean / data-less envs render — / N/A. ⚠️ reads as a signal, not a permanent fixture.
create-validation-summary/: sync per-env head with grouped head — col-1 step icons gain <span title="…"> hover tooltips (new _render_step_icon_cell helper mirroring the grouped action's), Plan Details → Plan details (sentence-case), Plan-time default switches from `N/A` to em-dash —, and the Plan-details badge stack is wrapped in <div align="left">. The two heads now render identically except status cells (text in per-env's wide Result column vs emoji in the grouped head's narrow per-env columns — documented as an intentional column-width adaptation in format-status and both render-function headers).

Test plan

All five touched per-action test suites pass locally:
- terraform-init/run_all_tests.sh — 19/19
- terraform-validate/run_all_tests.sh — 9/9
- parse-terraform-warnings/run_all_tests.sh — 44/44 (9 fixtures, including 'with module …' context-line regression and UTF-8)
- create-validation-summary/run_all_tests.sh — 70/70 (13 new — warnings row, collapser, 65k budgeting, UTF-8 boundary; goldens updated for the per-env↔grouped sync)
- aggregate-validation-summaries/run_all_tests.sh — 45/45 (7 new — warnings row sum across three step ids, conditional row suppression, Plan-details row gating)
pr-comment and pr-comments-reconcile suites verified unaffected (marker-based, agnostic to body content).
All touched YAML files parse cleanly (python3 -c "import yaml; yaml.safe_load(...)").
Calling-repo end-to-end — verified on dsb-infra/azure-terraform-ikt-app-platform-environments-config#721 via a temporary @dev-plan-warnings tag (now removed). All three matrix envs (dev/prod/test) rendered the grouped Warnings row with ⚠️ 5 each, the per-env plan-tag comments carried the <details>⚠️ 5 warnings</summary>…</details> collapser with correct title / source / aggregator note / blockquoted body, and the check-runs API showed one ::warning annotation per env with file=, line=7, and title=terraform plan warning: Argument is deprecated. Count of 5 vs 1 annotation is correct: terraform's (and 4 more similar warnings elsewhere) aggregator collapses categories, so the parser counts total occurrences but only emits one annotation per shown block (the only one with file/line context).

Notes

This PR no longer carries a dev-tag swap commit; @v0 references are intact throughout the workflow files, so the branch is ready to merge once reviewed.
The dev-tag-swap pattern from docs/Development-and-release.md was used to exercise the feature end-to-end; both the temporary dev-plan-warnings tag and its swap commit have been removed from this branch.

🤖 Generated with Claude Code

Authoritative reference for the upcoming three-action chain (terraform-init/validate/plan console capture → parse-terraform-warnings → create-validation-summary + aggregate-validation-summaries). Documents the warning-block grammar, the ::warning annotation format, the 65k char budget where warnings have priority over plan extract, and the ARG_MAX caveat for step outputs. Cross-linked from Workflow-pr-comments.md §5.1/§5.2/§5.3 so the comment-shape doc still owns the rendered shapes while delegating data-flow and budgeting details to the new spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…nsole Moves the embedded init logic from action.yml into step_init.sh, mirroring the layout established by terraform-plan/ (helpers.sh + step_*.sh + run_local_*.sh + run_tests_step_*.sh + run_all_tests.sh). action.yml becomes a thin shim that passes inputs as env vars and sources step_init.sh. Adds tee of every 'terraform init' invocation (project init + additional dirs) to a single console-output file (tf-init-console-output-<env>.txt, exposed as the action's new 'console-output-file' output) so a downstream warning-extraction action can scan it. New 'environment-name' input keeps file names unique across matrix jobs. 19 tests cover: happy path, additional dirs (both invocations land in same file), failure, mixed success/failure across dirs, missing additional dir, empty additional-dirs JSON, and env-name in file path. Per CLAUDE.md guidance: when touching a legacy action for non-trivial work, convert to the modern layout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…e console Same conversion as the preceding terraform-init commit. action.yml becomes a thin shim; step_validate.sh holds the actual 'terraform validate' invocation with stdout/stderr teed to tf-validate-console-output-<env>.txt (exposed as 'console-output-file'). New 'environment-name' input keeps file names unique across matrix jobs. 9 tests cover: happy path, failure path, env name appearing in file path. Smaller than terraform-init's suite because the action's behaviour is correspondingly smaller — single terraform invocation, no multi-dir loop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…on PRs Scans a terraform console-output file for 'Warning:' diagnostic blocks (e.g. deprecation notices), emits one '::warning file=<f>,line=<n>,title=<t>::<msg>' workflow command per block (with file/line attributes when terraform printed source context, plain fallback otherwise), and renders a markdown body file suitable for embedding in the PR plan-tag comment via create-validation-summary. State-machine parser (awk) handles: multi-line context blocks (the 'with module …' line that precedes 'on <file> line <N>'); the '(and N more similar warnings elsewhere)' aggregator suffix (sums into warning-count without producing extra annotations, since terraform only prints file/line for the shown example); Warning: → Error: → Warning: transitions where the Error: body must not bleed into the warning; non-ASCII message bodies preserved through annotation escaping (%, \r, \n) and attribute escaping (: , in addition). Step outputs are limited to small integers and file paths — the rendered markdown content is never exposed as a step output to avoid the ARG_MAX bomb documented in capture-matrix-job-meta/action.yml. 44 tests across 9 input fixtures cover: no warnings, single warning with/without file context, aggregator suffix, multiple blocks, Warning followed by Error, non-ASCII body, empty file, init provider deprecation, missing/non-existent input file. Tests assert annotation count, warning-count, and that the 'with module' context line does NOT leak into the message body (regression for a bug observed during the initial implementation). See docs/Plan-warnings.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… UTF-8 cut New inputs: 'warning-count' and 'warnings-markdown-file'. In render_head_summary: a '| ⚠️ | Warnings | … |' row sits between the Plan row and Plan Details row whenever warning-count is numeric and > 0 (suppressed when 0 / unset / '?', matching the 'absent when uninteresting' convention used for Plan Details and the Links row). In render_plan_extract: the existing five plan-block shapes are unchanged; a sibling '<details><summary>⚠️ N warnings</summary>…</details>' collapser is appended AFTER the plan-block when warnings-markdown-file is non-empty. Budgeting is warnings-first against a 65000-byte hard limit (warnings capped at 60k as a sanity ceiling; plan-extract gets whatever remains after OVERHEAD=500) — so a noisy run never silently loses warnings to plan output. Replaces 'tail -c 65000' with a line-anchored variant (tail -c <budget> | sed '1d') that drops the partial first line of the cut. UTF-8 safe: newlines are single ASCII bytes, so cutting on line boundaries can never split a codepoint. Fixes a latent bug — predates this feature — where a multi-byte char straddling the 65000-byte boundary corrupted the rendered comment. Legacy 'summary' output (kept for callers still on the pre-overhaul commenting flow) carries the warnings collapser too — additive, no consumer breaks. 13 new tests (70 total) cover: row presence/absence across warning-count values (positive / 0 / unset / '?' / grouped mode); collapser appended after plan-block; collapser kept in grouped mode (per-env plan-tag still posted); huge warnings (>60k) trigger truncation footer; budget-constrained plan extract trimmed first; UTF-8 preserved at boundary; legacy summary includes warnings. See docs/Plan-warnings.md §5–§6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…table New '| ⚠️ | Warnings | … |' row in render_group_body, positioned between the step rows and the Plan details row (top-to-bottom scan: step status → warnings → planned changes). Per-env cell shows '⚠️ N' when N>0, em-dash '—' when zero or missing — mirroring _render_plan_time_cell's 'not measured' convention rather than _render_plan_details_cell's 'always render' convention. _extract_warning_count sums warning-count outputs from three step ids (parse-init-warnings + parse-validate-warnings + parse-plan-warnings) on each matrix-job-meta-*.json file. Missing step entries count as 0, so meta files captured before this feature shipped still render correctly — the row shows '—' across the board. No new inputs on capture-matrix-job-meta: its toJSON(steps)-based future-proof capture means the new parse-*-warnings step outputs flow through automatically. 6 new tests (44 total) cover: sum across all three steps, em-dash when all zero, em-dash when steps missing, sum when only some steps emitted output, mixed-env rows (some with warnings, some without), and that the row sits between Plan and Plan details. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Three parse-terraform-warnings invocations per env (after init / validate / plan), each consuming the corresponding step's console-output-file. Per-step extraction rather than one consolidated scan so each step gets its own 10-annotation GitHub Actions budget — a noisy init can no longer crowd out plan-warning annotations. terraform-init and terraform-validate now receive environment-name so their console files don't collide between matrix jobs. A new inline 'concat-warnings-md' shell step does two jobs: concatenates the three per-step markdown files (init → validate → plan order, empty steps skipped) and sums the per-step warning-counts. The arithmetic has to live in shell because GitHub Actions workflow expressions don't support '+' between fromJSON() values — the expression parser rejects it. Both invocations of create-validation-summary (the initial call and the post-tag re-render with the Links row) consume the aggregated outputs, so the Warnings row + collapser show up in both the per-env head and the per-env plan tag. End-to-end verification: dev-tag swap per docs/Development-and-release.md, then exercise from a calling repo with a module that produces a known deprecation warning. See docs/Plan-warnings.md §2 (data flow). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

github-actions · 2026-05-25T16:05:36Z

🧪 Action test results

Total: 401 tests across 14 suites — 401 passed, 0 failed

Tested (14)

Action	Result	Tests	Details
aggregate-validation-summaries	✅ Pass	45 / 45	job log
auto-merge-pr	✅ Pass	24 / 24	job log
capture-matrix-job-meta	✅ Pass	18 / 18	job log
create-validation-summary	✅ Pass	70 / 70	job log
evaluate-automerge-eligibility	✅ Pass	50 / 50	job log
parse-terraform-plan	✅ Pass	7 / 7	job log
parse-terraform-warnings	✅ Pass	44 / 44	job log
pr-comment	✅ Pass	17 / 17	job log
pr-comments-reconcile	✅ Pass	15 / 15	job log
setup-tflint	✅ Pass	19 / 19	job log
terraform-init	✅ Pass	19 / 19	job log
terraform-plan	✅ Pass	45 / 45	job log
terraform-validate	✅ Pass	9 / 9	job log
verify-terraform-lock	✅ Pass	19 / 19	job log

Not tested yet (10) — modernization candidates

Show list

create-test-report
create-tf-vars-matrix
create-tftest-matrix
export-env-vars
lint-with-tflint
setup-terraform-plugin-cache
terraform-apply
terraform-docs
terraform-fmt
terraform-test

Run: workflow run · Commit: 0baa170

The action shim captured `github.event` JSON via heredoc and then explicitly `export`ed it into envp before sourcing the step. github.event for PR events can be 30-100+ KB (long descriptions, large reviewer lists, labels) — single- string MAX_ARG_STRLEN on Ubuntu is 128 KB, so a sufficiently large PR description would push every subsequent gh / jq fork inside step_auto_merge_pr.sh over the limit and trip exit 126 "Argument list too long". step_auto_merge_pr.sh is sourced from the same shell and reads \${input_github_event_context_json} as a shell-local already — the export was unnecessary as well as unsafe. Drops the line and replaces the comment with a why-not so the trap stays documented. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two related additions: - CLAUDE.md: new "Watch for ARG_MAX in step scripts" subsection enumerating the four in-tree reference patterns (tail -c for file tails, mktemp + redirect for gh api responses, jq --slurpfile for JSON merges, heredoc + body=@file for gh CLI inputs). - docs/Action-implementation-guide.md: replaces the unsafe JSON-input shim example (which previously showed `export input_json_data` — the exact bug) with the safe shell-local pattern. Adds a new "Anti-pattern: exporting heredoc-captured JSON" section explaining the failure mode (ARG_MAX / MAX_ARG_STRLEN at next fork+execve), why the pattern keeps being re-introduced (sourcing makes export feel correct but it isn't), and the small set of cases where exporting is legitimately needed. The guide's previous example was load-bearing — it was the template contributors copy/pasted, which is how the bug spread to four actions. Fixing the example is the prevention; documenting the rationale is the backstop when someone reads the diff and wonders why. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… when empty Grouped table now suppresses the Warnings row when no env in the group has warnings, and the Plan details row when no env has plan data — matching the per-env head, which already gates both (warning-count > 0 and include-plan-details). ⚠️ reads as a signal, not a permanent fixture, and the table stays quiet when there's nothing to show. When a row IS shown, data-less / clean envs still render '—' / 'N/A' so reviewers can see which envs the row applies to. Implemented by accumulating group_warning_total and group_has_plan_data in the existing per-env loops and assembling the optional rows into a mid_rows string — an empty printf arg would otherwise emit a blank table line, so the row must be dropped from the string, not blanked. Tests: flip the two "row renders em-dash at zero" cases to assert the row is absent; add a Plan-details-absent case (empty parse-plan outputs); the row-order golden now seeds a warning + plan data so it still locks the full canonical order. See docs/Plan-warnings.md §6. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The per-env head and the grouped head should render identically except where their column shapes force a difference. This brings the per-env head into line: col-1 step icons gain <span title="…"> hover tooltips (via a new _render_step_icon_cell helper mirroring the grouped action's), "Plan Details" → "Plan details" (sentence-case, like "Plan time"/"Lock file"), the Plan-time default becomes the em-dash '—' instead of `N/A`, and the Plan-details badge stack is wrapped in <div align="left"> — all matching the grouped head byte-for-byte. Step-status cells stay text (`success` / <kbd>failure</kbd>) vs the grouped head's emoji — an intentional, documented divergence: the per-env Result column is wide enough for text, the grouped head's per-env columns are narrow and benefit from emoji. Cross-reference comments in both render functions and a note on format-status spell this out so it isn't "fixed" later. Tests: byte-exact goldens updated to the span-wrapped col-1 / "Plan details" / em-dash / <div> shape; quote-sensitive assertions switched to single quotes so the span markup's inner double-quotes don't break them. See docs/Workflow-pr-comments.md §5.1/§5.3. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

GitHub renders Mermaid natively, so the data-flow / lifecycle / conversation- order diagrams in Plan-warnings.md (§2), Workflow-pr-comments.md (§3, §7), and Testing-in-ci.md (§2) now display as actual node-and-arrow graphs in the browser instead of monospace boxes. The Mermaid sources are also easier to edit without manually maintaining ASCII alignment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Laffs2k5 and others added 7 commits May 25, 2026 18:00

Laffs2k5 and others added 2 commits May 26, 2026 09:41

Laffs2k5 force-pushed the feat/plan-warnings-on-pr branch from 4ce6e58 to 33325eb Compare May 26, 2026 07:42

Laffs2k5 and others added 2 commits May 27, 2026 09:39

Laffs2k5 force-pushed the feat/plan-warnings-on-pr branch from 33325eb to 6a17100 Compare May 27, 2026 07:40

Laffs2k5 force-pushed the feat/plan-warnings-on-pr branch from 6a17100 to 4ec6354 Compare June 1, 2026 10:08

Laffs2k5 marked this pull request as ready for review June 1, 2026 11:04

Laffs2k5 merged commit 8da468f into main Jun 1, 2026
17 checks passed

Laffs2k5 deleted the feat/plan-warnings-on-pr branch June 1, 2026 11:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: surface terraform plan/init/validate warnings on PRs#44

feat: surface terraform plan/init/validate warnings on PRs#44
Laffs2k5 merged 12 commits into
mainfrom
feat/plan-warnings-on-pr

Laffs2k5 commented May 25, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 25, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Conversation

Laffs2k5 commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Summary of changes

Follow-up refinements (in this same PR)

Test plan

Notes

Uh oh!

github-actions Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🧪 Action test results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Laffs2k5 commented May 25, 2026 •

edited

Loading

github-actions Bot commented May 25, 2026 •

edited

Loading