Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ real CI job container that doesn't explicitly join a custom Docker network.
already in use"), bounded the `host.docker.internal` DNS lookup with a timeout, and tightened
the corresponding test's timing assertion against the actual configured constant instead of a
loose bound.
- Re-verified 2026-09-07: `git merge-base --is-ancestor` confirms both this PR (#2960) and its
prerequisite (#2942) are ancestors of `HEAD` and of the published `v1.228.0` tag, and no commit
since has touched the network-join/endpoint-selection code. Reran the exact `bugs.md` item 8
reproduction against the published `ghcr.io/cloudposse/atmos:1.228.0` image (a socket-mounted
container, no `--network` flag, against this repo's own `examples/terraform-tests` `fixtures`
stack): `atmos emulator up aws -s fixtures --ephemeral` reported
`emulator aws is up at http://fixtures-aws:4566`, and `curl http://fixtures-aws:4566/` from
inside that same container returned `HTTP 200`. No loopback/gateway fallback, no regression.
(The `--ephemeral` flag was needed only to skip an unrelated host-bind-mount requirement for
persistence when running Atmos itself inside a container with a mounted Docker socket -- not a
factor in the endpoint-selection logic this doc covers.) The original caveat still stands: this
confirms Docker Desktop's local VM networking, not an actual GitHub-hosted Actions job
container's networking.

## Follow-ups

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Fix: `backfillMissingTestJSONRuns` no longer appends unbounded synthetic runs

**Date:** 2026-09-08

## Summary

`pkg/ci/plugins/terraform/parser.go`'s `backfillMissingTestJSONRuns` synthesizes placeholder
`data.Runs` entries when the authoritative `test_summary` counts from a `terraform|tofu test
-json` stream exceed the runs the parser actually captured. The per-status loop bound came
straight from the untrusted `passed`/`failed`/`errored`/`skipped` ints in that stream with no
upper limit, so a single oversized count (e.g. `passed: 1000000000`) drove an unbounded `append`
loop that could exhaust memory or hang the `atmos terraform test --ci` command before it reported
anything. The loop is now capped per status at `maxBackfillRunsPerStatus` (10,000), and the
parser marks the result as incomplete when a count is truncated instead of silently
under-representing it.

## Context

Flagged by CodeRabbit's review of PR #3082 (thread `PRRT_kwDOEW4XoM6gUagt`, 🟠 Major) against
`backfillMissingTestJSONRuns`, which had been added and retained across two earlier commits on
this branch (`3ead9012f7`, `e57b5b8eb0`) as a last-resort guard for a schema gap in the parser.
The guard itself was never bounded, so it traded one failure mode (a schema gap silently dropping
runs) for another (an oversized summary count silently exhausting memory). This fix hardens the
guard added by `docs/fixes/2026-09-08-ci-test-json-opentofu-runs-dropped.md` without changing its
purpose.

## Changes

- `pkg/ci/plugins/terraform/parser.go`:
- New `maxBackfillRunsPerStatus` constant (10,000) bounding synthetic rows per status.
- `backfillMissingTestJSONRuns` clamps the per-status append count to the cap; when a count is
truncated it escalates from `log.Warn` to `log.Error` and sets the new
`data.BackfillTruncated` flag (never touches real captured rows).
- `testJSONHasErrors` includes `BackfillTruncated` so a truncated backfill always marks
`result.HasErrors`.
- `renderTestSummaryLine` (the plain-text fallback renderer) treats `BackfillTruncated` as a
failure condition and appends an explicit "parser output incomplete" line.
- `pkg/ci/internal/plugin/types.go`: new `TerraformTestOutputData.BackfillTruncated bool` field.
- `pkg/ci/plugins/terraform/handlers.go`: `buildTerraformTestStatusDescription` appends a
`"parser output incomplete"` part when `BackfillTruncated` is set, alongside the existing
`CleanupFailures` part, so the step-summary/status description surfaces the truncation.
- `pkg/ci/plugins/terraform/test_json_test.go`:
- `TestBackfillMissingTestJSONRuns` table gained a case asserting untouched, non-truncated
backfills leave `BackfillTruncated` false.
- New `TestBackfillMissingTestJSONRuns_CapsOversizedCount`: a `Pass: 1_000_000_000` summary
count is capped at `maxBackfillRunsPerStatus` synthesized runs, with `BackfillTruncated` and
`Total` asserted.
- New `TestParseTestJSON_OversizedSummaryCountIsBounded`: the same scenario through the public
`ParseTestJSON` entry point, asserting `result.HasErrors` and a bounded `data.Runs`.

## Validation

- `go build ./...` -- clean.
- `go test ./pkg/ci/plugins/terraform/... -run 'TestBackfillMissingTestJSONRuns|TestParseTestJSON' -v`
-- all pass, including the two new oversized-count tests, in under 2s (proving the cap actually
bounds the work rather than just bounding the assertion).
- `go test ./pkg/ci/...` -- full package, all pass, no regressions in JUnit/handlers rendering.
- Confirmed the new tests are load-bearing: with the fix removed (`git stash` of the three
non-test files), the package fails to *compile* because the tests reference
`maxBackfillRunsPerStatus` and `BackfillTruncated`, which only exist after the fix. Actually
reverting just the loop's bound (to reproduce a literal billion-iteration append) was not run,
since doing so would intentionally trigger the exact memory-exhaustion/hang this fix prevents.
- `atmos fix lint` (patch-scoped, `--new-from-rev=origin/main`) -- the only findings are 3
pre-existing issues in unrelated files (`pkg/store/providers/azure_keyvault_store.go`,
`cmd/terraform/utils.go`, `pkg/component/helm/client.go`); none in the files touched by this
fix.

## Follow-ups

None.
103 changes: 103 additions & 0 deletions docs/fixes/2026-09-08-ci-test-json-opentofu-runs-dropped.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# Fix: `terraform test --ci` dropped every OpenTofu run and late assertion diagnostics from the summary/JUnit

**Date:** 2026-09-08

## Summary

In CI mode, `atmos terraform test` always runs `terraform|tofu test -json` and builds the step
summary, the JUnit report, and inline annotations from that event stream. The parser only accepted
a `test_run`/`test_file` event as final when it carried `progress: "complete"`. OpenTofu never
emits a `progress` field at all -- it emits exactly one event per run/file carrying only the final
`status` -- so under OpenTofu every run and file event was discarded. The `test_summary` event has
the same shape in both tools, so the badge counts still came out right while the results table was
empty and `<component>.junit.xml` reported `tests="0"` on a passing run. Separately, both tools
emit an assertion-failure `diagnostic` *after* the run's final event, but the parser only attached
diagnostics that arrived *before* it, so failing runs lost their message and `file:line` (and
therefore the `::error` annotation and the Details column) under Terraform as well.

## Context

Reported from an application repository whose toolchain pins `tofu`: a passing run produced
`TESTS-1`/`PASSED-1` badges, no results table, `app.junit.xml` with `tests="0"`, and a run log
ending `Success! 1 passed, 0 failed, 0 skipped.` That three-field summary line is Atmos's own
`RenderTestText` format (Terraform's native message is `Success! 1 passed, 0 failed.`), which
placed the failure squarely on the JSON path.

The bug could not be reproduced with this repository's own `examples/terraform-tests` fixture:
eleven consecutive `atmos terraform test app -s fixtures --ci` runs against the Floci emulator all
produced `tests="4"` with real run names. That fixture is Terraform-only (its `.tftest.hcl` files
use `variable` blocks, which OpenTofu rejects in favour of `variables`), so it never exercised the
OpenTofu event shape. Capturing raw `-json` streams from both tools on a minimal provider-free
module made the difference obvious:

- Terraform: `{"path":…,"run":"plan_case","progress":"complete","status":"pass"}` (preceded by a
`progress: "starting"` event for the same run).
- OpenTofu: `{"path":…,"run":"plan_case","status":"pass"}` -- no `progress`, one event per run.

`completedTestRun`/`completedTestFile` gated on `Progress != "complete"`, so the OpenTofu stream
yielded an empty `data.Runs`/`data.Files`; `applyTestJSONSummary` then backfilled `Total`/`Pass`
from the summary event, which is exactly the reported badge/table/JUnit disagreement. The same
captures showed the assertion diagnostic arriving after the run event in both tools (Terraform
1.15.8, OpenTofu 1.12.5); the existing `sampleTestJSON` fixture had been hand-written with the
diagnostic first, which is why the diagnostic-attachment tests passed.

This is a sibling of the two earlier fixes on the plain-text fallback path
(`docs/fixes/2026-08-14-ci-summary-test-table-fallback-dropped.md`,
`docs/fixes/2026-08-19-ci-test-summary-fallback-recovers-error-detail.md`); neither touched the
JSON path.

## Changes

- `pkg/ci/plugins/terraform/parser.go`:
- New `testEventComplete(progress, status)`: an event is final when `progress == "complete"`
(Terraform) or when `progress` is absent and a `status` is present (OpenTofu). Used by both
`completedTestRun` and `completedTestFile`. Terraform's intermediate `starting`/`running`/
`teardown` events still carry a `progress` value and remain excluded.
- Diagnostic attachment factored into `attachPendingDiag`; new `attachLateDiagnostics` runs
first in `finalizeTestJSON` (which now receives `diagByRun`) and attaches any diagnostic keyed
by the run's file+name to a recorded run that has no error yet -- covering the
diagnostic-after-run ordering without changing the diagnostic-before-run path.
- `backfillMissingTestJSONRuns` (added earlier on this branch as a stop-gap) is retained purely
as a last-resort guard against a future unrecognised event shape, and now emits a
`log.Warn` whenever it fires so a schema gap can never again be silently absorbed into a
placeholder row.
- `pkg/ci/plugins/terraform/test_json_test.go`:
- `sampleOpenTofuPassJSON` / `sampleOpenTofuFailJSON`: verbatim `tofu test -json` streams
(OpenTofu 1.12.5).
- `TestParseTestJSON_OpenTofu_AllPass`, `TestParseTestJSON_OpenTofu_Failure`,
`TestToJUnit_OpenTofu`, `TestRenderTestText_OpenTofu`: real run names, files, counts,
message, and `file:line` all captured from the OpenTofu shape.
- `TestParseTestJSON_DiagnosticAfterCompleteEvent`: Terraform-shaped stream with the diagnostic
after the `complete` event.
- The earlier `TestParseTestJSON_SummaryExceedsRuns`, `TestBackfillMissingTestJSONRuns`, and
`TestToJUnit_BackfillsMissingRuns` remain, covering the guard.

No changes were needed in `junit.go`, `templates/test.md`, or the annotation emitter -- all of
them already key off `data.Runs`/`data.Files`.

## Validation

- Before the parser change, the new tests failed with the exact reported symptom: every run came
back as `run detail unavailable (pass)` with empty `File`, `Line`, and `Error`, and
`data.Files` was empty.
- `go test ./pkg/ci/plugins/terraform/...` and `go test ./pkg/ci/...` -- all pass, including the
pre-existing diagnostic-before-run fixture.
- `go build ./...`, `gofumpt -l`, `atmos lint --changed` -- clean for the touched files (three
pre-existing findings elsewhere on the branch, untouched).
- End-to-end under OpenTofu: a throwaway Atmos project (`components.terraform.command: tofu`)
around a provider-free module with one passing and one failing run, executed with the rebuilt
binary as `GITHUB_ACTIONS=true … atmos terraform test min -s fx --ci`. Result:
`min.junit.xml` reports `tests="2" failures="1"` with `<testcase name="passing_case">` and
`<testcase name="failing_case" … line="12"><failure message="Test assertion failed: name should
be b">`; the step summary lists both runs by name with `tests/min.tftest.hcl:12` in the Details
column plus the per-file breakdown; and the log carries
`::error file=tests/min.tftest.hcl,line=12,title=terraform test: failing_case::…`.
- Terraform path unchanged: the repository's own `examples/terraform-tests` fixture still yields
`tests="4"` with all four real run names.

## Follow-ups

- `examples/terraform-tests` cannot run under OpenTofu (`variable` vs `variables` in
`.tftest.hcl`), so there is no OpenTofu end-to-end fixture in this repository; the verbatim
OpenTofu streams in `test_json_test.go` are the regression coverage for that shape. Adding a
tool-agnostic fixture would let `atmos test --full` exercise both tools.
5 changes: 5 additions & 0 deletions pkg/ci/internal/plugin/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ type TerraformTestOutputData struct {

// CleanupFailures contains resources Terraform could not destroy after tests.
CleanupFailures []TerraformTestCleanupFailure

// BackfillTruncated indicates a test_summary count for some status exceeded the parser's
// synthetic-placeholder-row cap, so the backfilled Runs (and therefore Total/JUnit/the
// results table) are known to be incomplete for that status.
BackfillTruncated bool
}

// TerraformTestFile represents the result of a single `.tftest.hcl` file.
Expand Down
3 changes: 3 additions & 0 deletions pkg/ci/plugins/terraform/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,9 @@ func buildTerraformTestStatusDescription(testData *plugin.TerraformTestOutputDat
if len(testData.CleanupFailures) > 0 {
parts = append(parts, fmt.Sprintf("%d cleanup failed", len(testData.CleanupFailures)))
}
if testData.BackfillTruncated {
parts = append(parts, "parser output incomplete")
}
return strings.Join(parts, ", ")
}

Expand Down
Loading
Loading