DEV-488: docs: Component migrations in YAML by nitrocode · Pull Request #285 · cloudposse/atmos

RB (nitrocode) · 2022-12-27T20:33:25Z

what

docs: Component migrations in YAML

why

Easier to migrate from the component key to the metadata.inherits key for better inheritance

references

DEV-488

Andriy Knysh (aknysh)

please see comments

Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>

Bump the pnpm.overrides pin for fast-uri to >=3.1.6, resolving: - GHSA (#288) host confusion via skipped IDN canonicalization on scheme-relative references - GHSA (#287) SSRF via malformed IPv6 normalization - GHSA (#286) SSRF via repeated hostname percent-decoding - GHSA (#285) host confusion via percent-encoded scheme normalization qs (#283, #284, patched in 6.16.0) is not fixed here: that release is 4 days old and blocked by this repo's 14-day pnpm minimum-release-age cooldown until ~2026-09-12. browserslist/postcss-selector-parser (#280-282) were already fixed in b59c389, prior to this branch's last few merges from main. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Resolves 4 high-severity Dependabot alerts for fast-uri < 3.1.6. browserslist and postcss-selector-parser alerts are already resolved on disk (4.28.8 and 6.1.4/7.1.5 respectively, both above their patched thresholds) -- likely stale alerts pending a rescan. qs's patched 6.16.0 is blocked by pnpm's minimumReleaseAge cooldown (published within the last 14 days); left unbumped pending the cooldown window per that policy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…loudposse#3022) * ci: run atmos test race on pull requests The race detector has caught five real data races in this codebase during 2026 (see docs/fixes/), but no workflow ever ran `atmos test race`. Add a job that installs libudev-dev (required to compile the cgo half of github.com/bearsh/hid under CGO_ENABLED=1, the same trap govulncheck documents but works around differently) and runs the full suite under the race detector on every PR, merge-queue entry, and push to main/release branches. Also fixes the race command's description, which said "quick tests" despite always running the full ./... suite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): exclude tests/ acceptance suite from atmos test race The new race job timed out: github.com/cloudposse/atmos/tests hung for 11m and tests/testhelpers panicked with "test timed out after 10m0s" inside TestAtmosRunner_buildWithCoverage, stuck waiting on a `go build` subprocess. The `test` job's own matrix comment measures this suite's unsharded runtime at ~90m on Linux (hence its 10-way shard split) -- `atmos test race`'s $(go list ./...) was pulling it in whole. It also shells out to a plain (non -race) `atmos` binary, so racing the driver process caught no races in the binary under test. Exclude ./tests/... from the race run; it's still covered (without race) by the sharded acceptance job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): resolve race job timeouts and a shuffle-order test bug Second real run of the race job surfaced two more issues: - pkg/toolchain timed out at 10m0s. Its tests install real tool binaries from real registries with no mock seam, and the race command runs the whole ./... suite unsharded and unauthenticated, so every package's network calls compete for the same runner and the same IP-wide GitHub rate limit -- unlike the `test` job's acceptance steps, which already set GITHUB_TOKEN for this reason. Set GITHUB_TOKEN on the race job's step and raise the command's -timeout from 10m to 20m for headroom. - pkg/utils's TestClearInternPool asserted stats without first clearing the package-level intern pool, silently depending on running before any other test interned a string. -shuffle=on randomizes order, so once another test ran first the assertion failed (expected 3, got 17). This is the first time -shuffle=on ran against the full unit-test suite in CI. Fixed by clearing the pool at the start, matching the adjacent TestResetInternStats. See docs/fixes/2026-09-01-race-detector-ci-job-timeouts.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: close a real data race in the toolchain's concurrent installer The race job (fixed for timeouts/shuffle-order in the prior commits) caught its first genuine production bug: pkg/toolchain's concurrent batch installer runs pkg/ui/theme.getActiveThemeName() (writes via viper.BindEnv, on every styled render) and pkg/http.GetGitHubTokenFromEnv() (reads via viper.GetString) from separate worker goroutines, both against the process-wide global viper singleton, which has no locking of its own. pkg/config already has a SafeViper mutex-guard for exactly this class of problem (built for the DAG scheduler's concurrent LoadConfig calls), but pkg/http and pkg/ui/theme sit below pkg/config in the import graph and can't reach it without a cycle. Move the guard into a new leaf package, pkg/viperguard, with no Atmos-internal imports; pkg/config.GlobalViper() now delegates to it instead of keeping a second, independent mutex (two separate locks on the same underlying singleton wouldn't exclude each other), and pkg/http/pkg/ui/theme route their global-viper access through it directly. Also fixes a second -shuffle=on test-isolation bug this run surfaced: TestGitHubTokenEnvBinding depended on TestMain's one-time "github-token" env binding surviving every sibling test, but set_test.go's teardownTest() calls viper.Reset(), which discards it. Previously inert (GITHUB_TOKEN was never set in CI before the prior commit); now fixed by re-binding after Reset() and defensively before the assertion that depends on it. See docs/fixes/2026-09-01-race-detector-ci-job-timeouts.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: two more real races and four shuffle-order test bugs; bump race job runner The race job ran the full suite to completion for the first time and surfaced seven independent failures in one run: - pkg/lsp/server: DocumentManager.Update mutated an existing *Document's fields in place under its own lock, but validateDocument reads them through the returned pointer with no lock held right after -- a second, overlapping didChange for the same URI raced with it. Update now stores a new *Document per version instead of mutating the shared one. - pkg/terraform/cache: nativeWindowsTrustInstall's closure read the package-level installWindowsTrustFunc var from inside a background goroutine that a timeout lets keep running after the caller returns; a test's t.Cleanup reassigning that var raced with it. Now snapshots the func into a local var before the goroutine starts. - pkg/terraform/registry: fakeRegistry's dlHits/verHits counters were incremented from concurrent httptest.Server handler goroutines with no synchronization -- and never read anywhere. Removed them. - pkg/runner/step, pkg/provisioner/backend, pkg/scanners/sarif, pkg/ui/theme: four more -shuffle=on test-isolation bugs, all the same shape as round 3's github-token one -- a test resets/overrides package-level global state in cleanup without restoring it (or, for sarif, uses viper.Set where t.Setenv was needed), silently breaking whatever test the shuffled order runs next. Also: once every timeout was fixed, this job became the slowest check in the PR. go test's package-level concurrency scales with cores, so swap ubuntu-latest for the same RunsOn runner family the build job's linux leg already uses for CPU-heavy Go work. See docs/fixes/2026-09-01-race-detector-ci-job-timeouts.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): fix race job runner swap - wrong family, broken apt mirror step The runner-swap commit picked "terraform" without checking its specs: turns out it's an i4i.large (2 cores, 15.7GB RAM) -- fewer cores than ubuntu-latest, a downgrade for this CPU/memory-bound -race workload. Switch to "large" (used by the release job's goreleaser step): an r7a.xlarge, 4 cores, 31GB RAM, double "terraform" on both counts. Separately, the job failed outright before running any tests: its "Install Linux build dependencies" step's `sed -i .../ubuntu.sources` (copied from the floci-go job, which runs on ubuntu-latest) errored with "No such file or directory" -- the RunsOn AMI is Ubuntu 22.04, which has no DEB822 .sources file (a GitHub-hosted 24.04+ image convention). Guard the sed behind a file-existence check so the step works on either runner image. See docs/fixes/2026-09-01-race-detector-ci-job-timeouts.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: close pkg/perf data race and a heatmap-test tracking leak The race job ran to completion for the first time (after fixing its runner) and hit ~20 failing cmd tests plus 24 DATA RACE warnings. 21 of the 24 races traced to one root cause: pkg/perf's "simple stack" fast path (used by the defer perf.Track(...) call at the top of nearly every public function) only verifies goroutine ownership of its shared global stack at call depth 0/1, trusting it at deeper nesting for speed -- a documented "known limitation" where a second goroutine's calls can silently share the stack undetected. Beyond producing wrong metrics (the accepted tradeoff), StackFrame.childTime was a plain time.Duration read/written with no synchronization at all once two goroutines' frames actually interleaved -- a genuine data race. Changed it to atomic.Int64. That doesn't explain why so many otherwise-unrelated cmd tests hit it: perf tracking is off by default (Track is a no-op) unless something enables it. cmd/root_heatmap_test.go's TestDisplayPerformanceHeatmap (both cases) and TestHeatmapNonTTYOutput call perf.EnableTracking(true) directly to exercise the heatmap display but, unlike the well-behaved TestEnableHeatmapIfRequested, never disabled it afterward -- so once any ran under -shuffle=on, tracking stayed on for the rest of the cmd package's test binary, turning every subsequent perf.Track() call live and racy. Added perf.ResetForTesting() (the misleading pre-existing comments already claimed a reset existed) and wired all four heatmap-adjacent tests to enable/reset/disable correctly. One further failure was unrelated: TestUninstallCmd_RunE_MultipleSkills set the force flag via Lookup("force").Value.Set("true"), which updates the value but not pflag's Changed bookkeeping that viper's binding checks for precedence -- unlike every sibling test in the file, which correctly uses Flags().Set(). Fixed to match. See docs/fixes/2026-09-01-race-detector-ci-job-timeouts.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: upstream bubbles race, widespread --help flag leak, two more resets Another clean-ish run (7 total now) surfaced a smaller set of failures once the previous round's fixes landed: - internal/exec: TestExecuteComponentVendorPullBatch_PullsAllComponentsInOneCall hit a real race inside charmbracelet/bubbles@v1.0.0's progress.Model -- SetPercent's returned tea.Cmd reads m.tag/m.id back off the same *Model pointer when its tick fires, on a different goroutine than the one that can call SetPercent again before that tick lands. This is an upstream bug; can't patch a vendored dependency here, so avoid triggering it instead -- nothing renders the animation without a TTY, so only call SetPercent when m.isTTY. - cmd/init, cmd/scaffold: found the same latent bug in three "--help" integration tests. Cobra's execute() checks the "help" pflag's *current* value on every Execute() call, not whether --help was in that specific invocation's args -- so initCmd.SetArgs([]string{"--help"}) (and four scaffold equivalents) left it permanently true, and every later test that called that command's Execute() got nil back having silently printed help, RunE never called, no matter what args it passed. This is why TestExecuteInit_ArgumentParsing (already fixed once, for a different reason) kept reappearing -- it needed the exact same shuffle order to reproduce both bugs together. - cmd/version, cmd/describe_stacks/dependents (carried over from the round 6 investigation), cmd/validate_editorconfig: two more reset-without-restore leaks (a package-level "format" var, a ciFlagsParser Viper binding lost to another test's viper.Reset()). One failure -- cmd/list's TestListStacksWithOptions_CoverageIntegration -- didn't reproduce on retries with the seed that had just produced it, ruling out simple ordering in favor of genuine goroutine-timing nondeterminism. Left open; see docs/fixes Follow-ups. See docs/fixes/2026-09-01-race-detector-ci-job-timeouts.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: process-cache leak in pkg/auth, cobra flag Changed leaks, cmd/list identity leak, CodeRabbit findings Round 9 of the race-detector CI incident, plus CodeRabbit review on PR cloudposse#3022: - pkg/auth: describeWorkflowsCmd's test-added --pager flag panicked ("flag redefined") when a prior test's full Execute() pipeline had already merged RootCmd's persistent --pager flag in; guard with a Lookup check. - pkg/auth/manager_test.go: the process-level credential cache (processCredentialCache) was never reset between Whoami/Authenticate tests that reuse the same provider/identity names, so a passing test's cached credentials leaked into a test asserting authentication failure. Added resetProcessCredentialCache() + t.Cleanup to every affected test. - cmd/list: root-caused and fixed the previously "left open" cmd/list flake (TestListStacksWithOptions_CoverageIntegration and three siblings) -- several tests left a leaked viper "identity" value that a later executor-integration test's identity resolution picked up, tripping an "authentication requires at least one identity configured" error against the intentionally identity-less `complete` fixture. See the fix-log's Round 9 section for the full mechanism. - cmd/terraform/cache/mirror_test.go: TestMirrorCmdRunAll left the --all flag's Changed=true after passing --all, so a later test that never passes --all still observed All=true via viper's flag-precedence binding. - internal/exec/vendor_model.go: closed the remaining edge of the SetPercent data race (flagged by CodeRabbit) by dropping bubbles' animated SetPercent/tea.Tick machinery entirely in favor of a plain percent field rendered with ViewAs -- no shared Model state, no race. - cmd/ai/skill/uninstall_test.go: restore the force flag's original Changed state in cleanup, not just its value (CodeRabbit). - pkg/viperguard: corrected the IsSet doc comment (SetDefault also makes IsSet true), documented View's callback-must-not-call-a-writer restriction, and added test coverage for GetBool/View/viperReaderAdapter (100% package coverage). - .atmos.d/test.yaml: the race command's `go list | grep` pipe could mask a failing go list under mvdan/sh's default (off) pipefail; switched to an explicit set -euo pipefail + variable-assignment form. - .github/workflows/test.yml: bumped the race job's timeout from 45 to 75 minutes -- recent runs finished within a few minutes of the old budget. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: fix-log wording nits from CodeRabbit (executed vs compiled, -count=3 caveat) - "compiled test set" -> "executed test set": -run filters which compiled tests execute, it doesn't change what's compiled into the binary. - Make explicit that the -count=3 run is not part of the validation record: it was interrupted by the 300s timeout before finishing, not a pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: generalize the --help flag leak fix to the whole command tree (Round 11) TestRootHelpFunc_RealTree_UnknownSubcommandErrors reappeared in a full local -race -shuffle=on run of the entire cmd package (461s), this time failing on both "toolchain versions --help" and "terraform bogus-subcommand --help" even though Round 10 already reset both implicated tests' own --help flags. Some other test elsewhere in this large package also drives a real `atmos toolchain --help`/`atmos terraform --help` invocation through RootCmd.ExecuteC(), leaking that flag the same way -- per-test whack-a-mole fixes don't scale to "some other test, somewhere in a 461-second package." Fixed at the root: cmd/testing_helpers_test.go's snapshotRootCmdState/restoreRootCmdState (the mechanism behind every test's NewTestKit(t) call) only ever snapshotted/restored RootCmd's own Flags()/PersistentFlags() -- never any subcommand's, even though a real --help invocation against any subcommand parses onto that command's own FlagSet, a package-level singleton no different from RootCmd's. Added walkCommandTree to recursively visit RootCmd and every reachable command, and used it in both the snapshot and restore paths so every command's flags are captured and restored after each NewTestKit-protected test. cmdStateSnapshot.flags changed from map[string]flagSnapshot to map[*cobra.Command]map[string]flagSnapshot; the three tests in testing_helpers_snapshot_test.go that read the field directly were updated to index by RootCmd explicitly. Also confirmed two separate, pre-existing issues unrelated to this incident (documented in the fix-log's Follow-ups, not fixed here): pkg/provisioner's TestAutoProvisionBackendWritesWarningsToOutputWriter only fails when run in isolation via -run, never as part of the full package (as real CI always runs it); and cmd/root_test.go's two TestApplyCIGitCloneBootstrap_* tests hit a still-unidentified atmosConfig.CI.Enabled leak from some other test in the large cmd package, never seen in any real CI run's failure list across this whole incident. Validation: go build/go vet/golangci-lint clean. go test -race -shuffle=on ./cmd/ -timeout 900s (the entire package, no -run filter, matching the exact shape that exposed the --help leak) completed in 461s with zero failures. A full local run of the entire package set (minus tests/) completed with 390 of 391 packages passing -- the one remaining failure is the separate, already-documented atmosConfig.CI.Enabled leak above, not the --help leak this round fixed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): remediate 3 new Dependabot alerts (grpc, browserslist x2) - google.golang.org/grpc v1.82.1 -> v1.83.1 (GHSA covering heap memory exhaustion via HTTP/2 DATA frame fragmentation, patched in 1.83.1). - browserslist (transitive, both the 4.27.x and 4.28.x lines present in the lockfile) -> 4.28.8 via a pnpm override, covering two advisories: unbounded memory growth from unbounded query-result caching, and a crash/prototype-write via untrusted browserslist-stats.json custom stats. All three are minor/patch bumps, allowed by dependabot.yml's major-only ignore rule. Regenerated NOTICE via the new `mage notice:generate` target (this branch's merge from main replaced scripts/generate-notice.sh with tools/noticegen). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: real data race in pkg/ai/tools/atmos + CodeRabbit findings (Round 12) Genuine data race caught by the latest CI run: pkg/ai/tools/atmos's commandTreeProvider (injected once at MCP server startup, read on every atmos_list_commands/atmos_command_help call) had no synchronization at all. cmd/mcp/server's own tests call SetCommandTreeProvider from more than one goroutine -- one directly, another indirectly through setupMCPServer's config-loading path -- and the concurrent writes raced. This is a real, narrow production bug: concurrent MCP server initializations (or an initialization racing a tool call) could corrupt or drop the provider assignment. Fixed with a sync.RWMutex guarding both the write (SetCommandTreeProvider) and the read (commandTreeRoots); the two same-package test files reading the raw variable directly for save/restore were updated to take the read lock too. Also addresses three CodeRabbit findings from the PR review: - cmd/testing_helpers_test.go: restoreRootCmdCommands only restored RootCmd's direct children, not grandchildren -- a test that adds/removes a subcommand under a non-RootCmd parent could leave that nested registration in the shared Cobra tree. Replaced with restoreCommandChildren, walking the whole snapshotted tree recursively via a new cmdStateSnapshot.childCommands map. Added TestTestKit_GrandchildCommandRestoration using a throwaway parent/child pair (not a real subcommand, to avoid interfering with tests that drive real command dispatch through RootCmd.ExecuteC()). - docs/fixes: removed a duplicate "## Follow-ups" heading (MD024) -- a stale early "None." section from before Round 9-11's items existed. - internal/tui/utils/utils.go: sanitizeForFigurine converted embedded newlines to '?', losing multi-line banner text's line breaks. Added writeStyledFigurine, splitting on '\n' first and rendering each line as its own figlet block joined by a real newline. Added a forced-color regression test for "A\nB". Validation: go build/go vet/golangci-lint clean. go test -race -shuffle=on ./cmd/mcp/server/... passes (previously reproduced the race). Targeted -race reruns of every touched area pass repeatedly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: skip gomonkey-based tests under -race, not just darwin/arm64 gomonkey.ApplyFunc/NewPatches patches a target function's compiled machine code in place. Race-detector instrumentation changes that function's compiled layout, corrupting the patch so the mocked call hangs indefinitely instead of erroring or crashing -- taking the whole package's `go test -race` run down with it once the package timeout fires. This caused internal/exec's race CI job to time out at 20 minutes. Add tests.RaceEnabled (build-tag const) and tests.SkipIfGomonkeyUnsafe(t testing.TB, reason), which checks -race first and falls through to the existing darwin/arm64 SIGBUS check. Replace every inline runtime.GOARCH=="arm64" gomonkey guard (and one package-local helper) across the four files using gomonkey: internal/exec/terraform_affected_test.go, internal/exec/terraform_utils_test.go, cmd/terraform/lint_test.go, pkg/vendoring/install/copy_glob_test.go. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: root-cause the recurring cmd --help flag leak and a pkg/provisioner test order dependency cmd package: two previously-undiscovered bugs behind the recurring TestRootHelpFunc_RealTree_UnknownSubcommandErrors failure, found by instrumenting the actual dispatch instead of reasoning statically: 1. NewTestKit's flag restore skipped any live flag absent from its snapshot map (e.g. Cobra's lazily-created --help on RootCmd, added on first real Execute() call, not at registration). With no "before" state to restore, the flag leaked forward permanently. Now resets to the flag's own DefValue/Changed=false instead of skipping. 2. applyColoredHelpTemplateForTopic calls cmd.SetHelpFunc(...) on every command it renders help for, including real singletons like toolchain/terraform/version -- an unexported per-command function pointer the flag-walk never touched. The first successful --help render for any of those commands permanently overrides its HelpFunc, bypassing rootHelpFunc (and its unknown-subcommand rejection) for the rest of the test binary. Now reset every command's helpFunc in the same tree walk (RootCmd back to rootHelpFunc, everything else to nil/inherit). Fixing (2) exposed a masked pipe deadlock in the test itself: the fully-styled "Unknown command" error can exceed the OS pipe buffer, and the test only drained stderr after ExecuteC() returned. Now drains concurrently via a goroutine. pkg/provisioner: TestAutoProvisionBackendWrapsCreationError and TestAutoProvisionBackendWritesWarningsToOutputWriter only reset the backend registry via t.Cleanup (after running), so whichever runs first under -shuffle=on inherits s3.go's real, network-calling S3BackendExists checker instead of a clean registry -- causing autoProvisionBackend to silently no-op instead of calling the mock create function. Now reset before registering mocks too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: TestSetupColorProfileFromEnv leaks CLICOLOR_FORCE, defeating NO_COLOR in later tests setupColorProfileFromEnvWithArgs has real, permanent side effects when force-color is detected: lipgloss.SetColorProfile(TrueColor) and a raw os.Setenv("CLICOLOR_FORCE", "1") (deliberately process-wide in production, since it backs Boa's help-renderer color detection). TestSetupColorProfileFromEnv exercises this via its "ATMOS_FORCE_COLOR set" and "force-color flag" cases but never wrapped itself in NewTestKit and never restored CLICOLOR_FORCE, so once either subtest ran, CLICOLOR_FORCE=1 stayed set in the process for the rest of the test binary. TestTerraformGenerateVarfileCmdNoColor sets NO_COLOR=1 and expects zero ANSI codes; a later, separate code path (atmosConfig.Settings.Terminal.ForceColor, bound to both ATMOS_FORCE_COLOR and CLICOLOR_FORCE) picked up the leaked env var during real config load and forced the logger back to TrueColor, overriding the test's own explicit color-profile reset. Confirmed as the actual cause of an intermittent CI failure on the race job, not just a local flake. Fixed by wrapping the test in NewTestKit (restores the lipgloss/theme side) and explicitly saving/restoring CLICOLOR_FORCE around each subtest. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): bump fast-uri to 3.1.6 (Dependabot cloudposse#285-288) Resolves 4 high-severity Dependabot alerts for fast-uri < 3.1.6. browserslist and postcss-selector-parser alerts are already resolved on disk (4.28.8 and 6.1.4/7.1.5 respectively, both above their patched thresholds) -- likely stale alerts pending a rescan. qs's patched 6.16.0 is blocked by pnpm's minimumReleaseAge cooldown (published within the last 14 days); left unbumped pending the cooldown window per that policy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: TestWorkflowValidationErrorOwnsDiagnostics fails under colorized rendering assert.Contains checked the raw, ANSI-embedded rendered string. Lipgloss's word-wrap machinery re-applies a separate style/reset escape run per word while reflowing a styled paragraph, even when the words stay on the same visual line -- so "validation" and " failed" become non-contiguous once the embedded escape codes are counted as bytes, and strings.Contains can never reliably find the full phrase whenever color rendering is active, independent of MaxLineLength. Round 10's MaxLineLength: 200 fix addressed a real but different bug (wrapping onto a second visual line) that looked similar; it never touched the actual failure mode, which only shows up when color is on (confirmed CI's actual cause via a temporary revert-and-rerun under ATMOS_FORCE_COLOR=1 CLICOLOR_FORCE=1). Fixed by stripping ANSI codes (atmosansi.Strip, already this codebase's established pattern for this class of assertion) before the Contains checks, so the test verifies rendered text rather than incidental per-word styling boundaries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: [race] job never installed terraform/opentofu/packer/helm/helmfile The race job went straight from installing libudev-dev to running `atmos test race`, unlike the test/build jobs, which explicitly run `atmos toolchain install --default ...` for all five before testing. Without that step, any test needing these binaries depended entirely on Atmos's own lazy/auto-install succeeding in time -- unreliable here since `go test ./...` runs ~400 packages as separate concurrent processes with nothing serializing a shared install across them. This explains why TestRunTerraformMigratePlan_NoMigrationsDirSkipsCleanly never reproduced locally despite matching CI's exact -shuffle seed across multiple rounds: a local single-process run never faces the same install race a ~400-process CI run does. Fixed by adding the same toolchain-install step the test/build jobs already use. Also fixed TestPackerValidateCmd, which failed with "Missing plugins ... Did you run packer init for this project?": it's in the same cmd package as TestPackerInitCmd, which installs the aws/bastion fixture's "amazon" packer plugin as a side effect of running "packer init" against the same template. TestPackerValidateCmd never did this itself -- it only ever passed because TestPackerInitCmd happened to run first. Under -shuffle=on, running before it left the plugin cache empty. Made TestPackerValidateCmd self-sufficient by running "packer init" itself first, mirroring TestPackerInitCmd's own call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: migrate skipped tfmigrate but still spawned tofu; markdown Render raced on os.Stdout; NewTestKit restores atmosConfig TestRunTerraformMigratePlan_NoMigrationsDirSkipsCleanly was never a flake: it failed deterministically whenever `tofu` was absent (9 of the last 10 race-job runs) and only "passed locally" because the dev machine has Homebrew tofu on PATH. executeTfmigrateSingle ran `tofu workspace select` before resolveTfmigrateDefaultConfig decided there was nothing to migrate, so the "skipped entirely, no binary needed" contract was false. Reorder the skip decision ahead of the workspace select, and make the test assert the guarantee with an empty PATH (its sibling's idiom). CustomRenderer.Render reassigned the process-global os.Stdout around every glamour render to hide "Warning: unhandled element"; that is a data race against every concurrent os.Stdout reader (the open TestValidateStacksCmd_Failure trace) and dead code: the only kinds glamour warns on are ast.KindString (already overridden by the linkify extension's higher-priority renderer) and footnotes (never enabled). Remove the swap; add a stdout-silence guard test and a -race regression test. NewTestKit now snapshots/restores the package-level atmosConfig (closes the CI.Enabled leak generically); resetFlagToDefault handles bool flags whose DefValue was blanked (--version) instead of silently failing Set(""); TestPackerValidateCmd verifies the prerequisite init actually installed the plugin. Fix-log Round 19 corrects Round 18's install-race explanation. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix: `.tool-versions` as single source for CI tool pins; race test becomes a mage target Round 18's `[race]` job fix copied an already-duplicated `--default <tool>@${{ env.X_VERSION }}` install pattern present in 9 separate workflow blocks, which had already silently drifted (workflow pinned opentofu 1.12.2, .tool-versions pinned 1.12.5). Consolidate onto .tool-versions everywhere and stop mutating it from CI. Also convert the race command's untested inline shell script to a unit-tested mage target (magefiles/test_race.go), matching this repo's existing Go-backed mage target convention for other custom commands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: clear leaked stringSlice flag state; normalize Windows packer plugin path check resetFlagToDefault's stringSlice/stringArray case was a no-op (only cleared Changed), leaking a lazily-created flag's value (e.g. --skip) into later tests. The reflection-based clearing it deferred to (restoreStringSliceFlag) was itself silently broken: pflag's stringSliceValue holds an unexported *[]string plus a private "changed" bool, and Go's reflect read-only flag on unexported fields persists even through pointer indirection, so the old code's Value.Set never actually took effect. Both now go through a shared clearFlagSliceValue helper using the unsafe.Pointer + reflect.NewAt pattern already used by getGlamourGoldmark. TestResetFlagToDefault now asserts the cleared value, not just Changed. Also: TestCustomRendererRenderDoesNotRaceStdoutReaders now stops its stdout reader goroutine via t.Cleanup so it isn't skipped by an early require failure, and a stale docs/fixes Follow-ups pointer is corrected to name the round that actually resolved it (CodeRabbit review on cloudposse#3022). Separately, TestPackerValidateCmd was failing on Windows CI: its plugin prerequisite check compares packer's OS-native, backslash-separated output path against a forward-slash plugin id. Normalize with filepath.ToSlash before the Contains check. * fix: TestResetFlagToDefault uses NewTestKit per cmd test harness convention CodeRabbit follow-up on PR cloudposse#3022: this test builds its own local pflag.FlagSet rather than touching RootCmd, but cmd package tests must still call NewTestKit(t) per repo convention, for consistency and to guard against future edits that do touch global state. * fix(security): bump golang.org/x/crypto to v0.56.0 (CodeQL/govulncheck #5980, #5981) Resolves two govulncheck-flagged DoS vulnerabilities in x/crypto/ssh (CVE-2026-56855, CVE-2026-78662): a malicious peer could deadlock the connection via crafted channel messages on an established or undecided channel. Pulled in transitively through go-git's ssh transport (internal/exec). Verified with govulncheck that both advisories no longer apply after the bump. * [autocommit] formatting fixes * fix(test): close temp file before SaveToolVersions reopens it by path CodeRabbit review on PR cloudposse#3022: TestSetToolVersion_UpdatesExistingAliasKeyInPlace left the os.CreateTemp file handle open while immediately reopening the same path via SaveToolVersions -- an unclosed handle can block a second open on Windows even though it's harmless on Linux/macOS. * fix(oci): retry transient connection failures on the initial registry Get Third occurrence in CI: TestVendorPullFullWorkflow's myapp1 OCI pull failed on Windows shard 3/10 with "dial tcp ...: connectex: An attempt was made to access a socket in a way forbidden by its access permissions" pulling ghcr.io -- Harden-Runner block-mode's WFP allow rule for ghcr.io was installed 3.6ms *after* the connection attempt started, per that job's own network-event log. Two earlier Windows shards hit the same myapp1/ghcr.io failure (one as a ~400s stall before timing out). pullImage's remote.Get call for the manifest/auth ping had no retry at all for connection-level failures -- only the layer-download path (processLayerWithRetry) already retried transient network errors. Add the same bounded retry (3 attempts, exponential backoff, matching ociLayerRetry*) around the initial Get and the anonymous-auth-fallback Get, gated on a new isRetryableOCIConnectError predicate that only matches network-layer errors (dial/DNS/reset), not registry-level auth/status rejections -- those still go through the existing, separate anonymous-auth fallback unchanged. * fix(test): stop pkg/io context_test.go leaking viper "mask" state across tests [race] job failure: TestNewOutputMasksAllSinks failed under go test -shuffle=on -- output was never masked at all. Root cause: TestNewContextWithMaskingDisabled (and four sibling tests in context_test.go) call viper.Set("mask", ...)/viper.Reset() to control NewContext()'s DisableMasking without ever restoring viper afterward, unlike reconcile_masking_test.go's tests, which already t.Cleanup both Reset() and viper.Reset(). Under a shuffle order where the masking-disabled test runs before something that clears globalContext, whichever test Initialize()s next inherits mask=false process-wide. Add t.Cleanup(viper.Reset) to every context_test.go test that mutates viper, and make TestNewOutputMasksAllSinks itself defensively call the existing resetGlobals() helper so it doesn't depend on run order at all. Verified with go test ./pkg/io/... -race -shuffle=on -count=20: no failures (reliably reproduced the original failure before the fix by running TestNewContextWithMaskingDisabled followed by TestNewOutputMasksAllSinks in that specific order). * fix(test): TestTerraformGenerateVarfileCmdNoColor raced a leaked goroutine's stderr write [race] job failure: the test's captured stderr sometimes carried colorized content from an already-completed, unrelated test at the very start of the buffer, tripping the "no ANSI codes" assertion even though NO_COLOR was correctly respected for this test's own command output. Traced the leaked writer to mvdan.cc/sh/v3's DefaultExecHandler (used by the "shell" step type): on context cancellation it sends SIGINT, then spawns a detached goroutine that sleeps up to killTimeout (2s default) before SIGKILL -- that goroutine's own doc comment admits as much ("TODO: don't sleep in this goroutine if the program stops itself with the interrupt above"). cmd.Wait() returns as soon as the process dies (SIGINT is enough for a plain Go test-binary target), so callers like TestCustomCommandIntegration_ShellStepCancelledByContext return well before that timeout goroutine's window closes, running concurrently with whatever the next test does to the shared os.Stderr. Since this is third-party library behavior (not something to patch here) and the race is inherent to any test that reassigns the global os.Stderr, scope the ANSI check to only the portion of captured output after this invocation's own first log line (root.go's unconditional "Set logs-level=..." debug line) instead of the whole buffer, so leaked content from a preceding test can't trip the assertion. Verified with go test -race -count=1. * fix(ci): remove stale opentofu duplicate, bump .tool-versions pins to latest tofu v1.11.6 was a dead duplicate of opentofu/opentofu, superseded in August but never deleted; packer/helm/helmfile/tflint were still pinned to whatever Round 20 copied in, already behind upstream. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): restore the tofu alias in .tool-versions; it wasn't stale The atmos.ci autofix job's "Format HCL" step runs `atmos toolchain exec -- tofu fmt ...` inside ghcr.io/cloudposse/atmos:1.215.0, an older pinned binary that requires a literal `tofu` key in .tool-versions to resolve a version - unlike current HEAD, it can't auto-resolve the bare short-name alias against the registry. Restored it pinned to the same version as opentofu/opentofu (1.12.6, not the stale v1.11.6 that was there before), with a comment explaining why. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(toolchain): add tofu -> opentofu/opentofu BuiltinAlias atmos toolchain exec -- tofu ... only worked by luck of the aqua-registry short-name search (Resolve step 3) matching opentofu's binary name -- a fuzzy, index-dependent path that had already failed once in this exact spot (the atmos.ci "Format HCL" job against an older pinned container image with no short-name search at all, worked around by duplicating a `tofu` entry in .tool-versions). Register it as an explicit BuiltinAlias instead, matching the existing "atmos" -> "cloudposse/atmos" precedent, so resolution is deterministic and doesn't depend on the registry index being populated. The .tool-versions duplicate stays for now (that CI job's container image still pins an atmos release without this alias) with an updated comment explaining when it can be removed. * docs(fixes): move tofu-alias incident narrative out of .tool-versions .tool-versions is a config file, not a place for incident writeups. Trim the comment to a one-line factual note and move the full root-cause narrative (verified: 9323b8e, which introduced aqua short-name search, is not an ancestor of v1.215.0) to docs/fixes, matching this repo's convention. * test: cover the builtin tofu alias's Resolve() branch, not just the map entry TestDefaultToolResolver_AliasResolution's "tofu" case supplies the same opentofu/opentofu mapping through AtmosConfig.Toolchain.Aliases (the user-alias branch), so it wouldn't catch a regression in the builtin-alias fallback that Resolve() actually falls through to when no user config is set. Addresses CodeRabbit review comment on PR cloudposse#3022. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address CodeRabbit findings on the race-detector PR - Rename the "[race]" job to "non-acceptance test suite": it excludes ./tests/..., so "full test suite" was misleading. - cmd/root_heatmap_test.go: reset the perf registry in cleanup too, not just disable tracking, so this test's metrics can't leak into whatever -shuffle=on runs next. - pkg/utils/string_utils_test.go: TestClearInternPool leaves an interned string behind on success (every sibling test in this file clears on both entry and exit); register the missing cleanup. - pkg/auth/identities/aws/credentials_loader.go: loadAWSCredentialsFromEnvironment mutates process-wide AWS_* env vars with no synchronization, so two concurrent identity resolutions can load each other's profile/region. The AWS SDK has no direct equivalent for this code's AWS_REGION-masking behavior (there's a prior incident behind why that's needed), so keep the env-var approach but serialize the whole setup/load/restore transaction with a mutex instead of rewriting the SDK call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): cover the missing-import.yaml error shape in the invalid-stacks test TestExecuteTerraform_TerraformPlanWithInvalidTemplates asserts the invalid-stacks fixture's error mentions one of several known shapes, since filesystem walk order (which invalid file gets hit first) isn't deterministic across OSes. missing-import.yaml's error -- errors.ErrStackImportNotFound wrapping errors.ErrFailedToFindImport ("stack import not found: ... failed to find import") -- wasn't in that list, so CI failed when the walk hit it before any of the covered files (job 100836458008, PR cloudposse#3022's [race] job). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(fixes): document Round 22 (missing-import.yaml error-shape gap) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): allowlist Go vanity-import and toolchain domains in codeql.yml Confirmed via the StepSecurity MCP (list_blocked_domain_calls) as currently blocked, unresolved detections on this workflow's analyze and govulncheck jobs: gocloud.dev, filippo.io, and sigs.k8s.io are all real transitive dependencies (present in go.sum) whose vanity import paths need to resolve during `go build`/`go mod download`; go.dev and pkg.go.dev are standard Go toolchain hosts. Also dropped a duplicate storage.googleapis.com line in the analyze job's allowlist. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): clear ambient AWS static-key env vars before profile-based SDK load setupAWSEnv masked AWS_SHARED_CREDENTIALS_FILE/AWS_CONFIG_FILE/AWS_PROFILE/ AWS_REGION but not AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY/AWS_SESSION_TOKEN. The AWS SDK's default credential chain checks the static-key env provider before the shared-file/profile provider this loader exists to drive, so any ambient static keys in the process environment would silently outrank the selected profile. Clear and restore them using the same save/restore mechanism already used for the other vars. Addresses a CodeRabbit finding on PR cloudposse#3022. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): dedupe govulncheck's harden-runner allowlist after merge The merge of origin/main duplicated go.dev:443 and pkg.go.dev:443 in the govulncheck job's allowed-endpoints: both branches had independently added the same domains, and the auto-merge concatenated rather than deduped them. Harmless but sloppy; removed the duplicates. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(security): clear remaining AWS ambient credential/region selectors; fix test cleanup ordering Follow-up to the earlier static-key masking fix, addressing 4 CodeRabbit findings on PR cloudposse#3022: - setupAWSEnv now also clears AWS_DEFAULT_REGION (the SDK's fallback when AWS_REGION isn't set), plus the legacy AWS_ACCESS_KEY/AWS_SECRET_KEY aliases and the web-identity-token selectors (AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME) -- all checked by the AWS SDK's default credential/region resolution before the shared-file/profile provider this loader drives (verified against the pinned aws-sdk-go-v2/config@v1.32.18's env_config.go). - Fixed a real bug in TestSetupAWSEnv's own cleanup ordering: the custom savedEnv-restore was registered via t.Cleanup AFTER the t.Setenv calls. Since cleanups run LIFO, the custom restore ran first and each t.Setenv's own cleanup then ran after it, re-clobbering those keys back to "unset" -- silently dropping any ambient credentials that existed before the test. Moved the registration before the t.Setenv loop. - Added test coverage for AWS_DEFAULT_REGION and the newly-masked credential/identity selectors, and godot periods on the struct field comments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ci): TestGetTerminalWidthPrecedence left the ui formatter permanently nil Its outer t.Cleanup called ui.Reset() (nils globalFormatter) with nothing to re-initialize it afterward. TestMain's ui.InitFormatter runs once at binary startup, so any test that ran after this one in the same shuffled run saw ui.ErrUIFormatterNotInitialized -- caught TestSupportCommand_RunE in CI (job 101009660640, PR cloudposse#3022's [race] job). Reinitialize instead of leaving the formatter torn down. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(fixes): document Round 23 (ui formatter left nil across shuffled tests) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: atmos-pro[bot] <173522224+atmos-pro[bot]@users.noreply.github.com>

Create atmos-component-migrations-in-yaml.md

579aee3

RB (nitrocode) requested review from a team as code owners December 27, 2022 20:33

RB (nitrocode) requested review from PePe Amengual (jamengual) and Jeff (woz5999) December 27, 2022 20:33

RB (nitrocode) temporarily deployed to preview December 27, 2022 20:33 — with GitHub Actions Inactive

RB (nitrocode) changed the title ~~docs: Component migrations in YAML~~ DEV-488: docs: Component migrations in YAML Dec 27, 2022