feat(cloudformation): migration guide, inline templates, and field-test fixes [EXPERIMENTAL] - #3002
Conversation
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. |
f0df545 to
e882672
Compare
5542812 to
1c5a72c
Compare
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
e882672 to
ef7704b
Compare
1c5a72c to
03e1964
Compare
Resource Changes Found for
|
1bc62d1 to
d2fe2dd
Compare
3ab568d to
39f7b60
Compare
39f7b60 to
1e61d70
Compare
d2fe2dd to
09b0c42
Compare
…ooks/S3-auth fixes Fixes the full set of findings from this session's field-test pass on aws/cloudformation (bulk-selection infinite recursion shared by every bulk-capable component type, logs stdout routing, error-sentinel misuse, missing confirmation gates, diff/changeset leaks, real backend auto-provisioning, small doc drift), adds CFN inline-template support (`template:` as inline body, new `path:` for file references, plumbed through two component-config allowlists that were silently dropping the new key), and fixes a missing EndpointURL override in the shared pkg/ci/artifact/s3 store that broke template uploads against emulated AWS endpoints. Live-verified hooks fire only for diff/apply/delete, matching the code. Each change is documented in its own docs/fixes/ entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e-templates fix-log atmos-validate-editorconfig requires indentation in multiples of 2; the numbered-list continuation lines used 3-space indentation (aligned to "1. "/"2. "'s width) instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ainer/workflow/terraform/vendor/list
--labels was WithStringFlag (comma-separated only) while --tags was already WithStringSliceFlag
(repeatable and/or comma-separated) — an inconsistency found while fixing the same issue for
aws/cloudformation. The flag is independently duplicated (not shared) across cloudformation,
kubernetes, helm, container, workflow, terraform, vendor (x4), and list (x6) commands, so this
converts every one of them to match --tags' registration and parsing.
pkg/tags.ParseLabelsFlag now takes []string instead of string; every call site converts from
GetString("labels") to GetStringSlice("labels"), and every flag registration from
WithStringFlag("labels", ...) to WithStringSliceFlag("labels", ...). Docs updated to show the
repeatable form.
See docs/fixes/2026-08-25-labels-flag-repeatable.md for the full file list and validation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the changelog post covering the whole feature (all 4 phases plus this session's field-test fixes, inline templates, and migration guide), and links it into the extensibility initiative's roadmap milestone. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…plate golangci-lint's add-constant rule flagged a 4th literal "%w: %w" after the gh-stack rebase merged phase1's wrapFmt-introducing commit ahead of this one; swap in the existing package-level constant instead of a duplicate literal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…uild failure The docs build's cast-validation step failed because every aws/cloudformation doc page beyond Phase 1's 8 verbs (backend, changeset, drift, fmt, get, list, logs, source, stackset, tree, watch) referenced a screengrab cast that was never recorded — demo/casts/atmos.d/screengrabs/cli.yaml's command manifest still only listed Phase 1's verbs. Added the missing 32 commands to both the generate and validate command lists and recorded all 41 aws/cloudformation casts via `atmos --chdir=demo/casts casts generate screengrabs cli --filter=cloudformation`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… stacks followLogs wrote each stack's newly-fetched events immediately after polling it, per poll iteration — printing events in poll order rather than event-time order whenever an earlier-polled stack's events were actually newer than a later-polled stack's. Now collects the iteration's events across every stack, sorts by timestamp, then writes, matching the non-follow path's existing sort. Also tightened the aws/cloudformation JSON schema: 'template'/'path' now reject empty values, and the object schema enforces they're mutually exclusive (previously only checked in Go's validateComponentConfig, so an IDE/editor validating against the schema wouldn't catch either mistake). Found via CodeRabbit review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ws " from every usage line
cmd/aws/cloudformation/cloudformation.go's init() called both
awsCmd.AddCommand(CloudFormationCmd) (correct, in cmd/aws/aws.go) AND its own
internal.Register(&CommandProvider{}), which re-parents CloudFormationCmd
directly onto RootCmd via the top-level command registry. Cobra's AddCommand
mutates the child's parent pointer in place, so whichever call ran last won
— the registry-based one, since it runs from cmd/root.go's init() after every
imported package's own init() has already run. Two live, confirmed effects:
- Every `atmos aws cloudformation ...` --help usage line rendered
"atmos cloudformation ..." (via cmd.CommandPath()/UseLine()), missing "aws".
- `atmos cloudformation ...` (without the aws namespace) worked as an
undocumented, unintended duplicate top-level command, alongside the real
`atmos aws cloudformation ...` path.
No other `aws/<service>` subcommand (eks, security, compliance, ecr) does
this self-registration — cloudformation was the only one. Removed the rogue
internal.Register call and the now-dead CommandProvider type; it was never
needed since aws.go already wires the command in correctly.
Regenerated all 41 aws/cloudformation screengrab casts to reflect the
corrected usage lines (`atmos --chdir=demo/casts casts generate screengrabs
cli --filter=cloudformation`), which also incidentally resolved the
`changeset delete --help` cast advertising a --retain-resources flag it
doesn't have (that cast was stale, predating other fixes).
Also: documented that `backend update` creates the bucket first when it
doesn't exist yet, matching `backend create`'s shared code path (the doc
previously only described it as an existing-bucket update); and updated the
backend auto-provisioning fix-log's stale Follow-ups section — the S3
identity/EndpointURL bug it flagged as unresolved was fixed the same day in
2026-08-25-artifact-s3-store-endpoint-override.md.
Found via CodeRabbit review (phase4 --dir website/--dir docs passes). --dir
internal came back clean (0 findings). The Rain --config Parameters-map
wrapping finding on from-rain.mdx and the source.uri single-file
template-inference finding on aws-cloudformation.mdx were not fixed: the
former needs verifying Rain's actual external config-file format (can't
confirm from this repo alone), and the latter's claimed behavior doesn't
exist anywhere in pkg/component/aws/cloudformation — no code derives a
template path from source.uri's basename.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…/list renderBackendStatuses fell through to the table renderer for any unrecognized format value (e.g. a typo'd --format=jsonn), the same silently-swallowed-bad-format bug class already fixed for `output`'s renderOutputsSummary. Now errors with ErrInvalidFlagValue, matching the convention pkg/toolchain/list.go already uses for the same kind of check. Found via CodeRabbit review (phase4 --dir cmd pass). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ypes list, fix quoting claim - apply.mdx's Flags list was missing --base, despite the --affected example right above it using --base origin/main. - The shared /cli/configuration/components overview page never listed AWS CloudFormation in its "Supported Component Types" dl, even though the YAML example below it already includes aws/cloudformation. - The CFN configuration page claimed the "aws/cloudformation" YAML key "must be quoted... because it contains a /" — verified against a real YAML parser: a bare '/' does not require quoting in YAML. Reworded to describe the quoting as a documentation convention, not a requirement. Found via CodeRabbit review (phase1 pass). A pkg/datafetcher/schema/stacks/ stack-config/1.0.json finding (add secrets to the CFN component manifest) was not fixed: every component type in that schema file is missing secrets (it's a stale, test-only-referenced duplicate of the actively-maintained pkg/datafetcher/schema/atmos/manifest/1.0.json, which already has it) — fixing only CFN there would be a new inconsistency, not a fix. The "union of sections" affected-detection finding (removed sections aren't flagged as affected) was not fixed either: it's the deliberate, tested behavior "section absent locally is skipped" shared by every component type's equivalent helper (see TestAddHelmSectionAffected_NoFalsePositives and TestAddKubernetesSectionAffected's identical subtests) — diverging only CFN from that convention would be new inconsistency, not a fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The --labels flag was migrated from a single comma-separated string to a repeatable pflag StringSlice (fix(cli) 5587d16), but the CLI --help golden snapshots that render its flag description were never regenerated, so every Acceptance Tests shard exercising `--help` for terraform/config commands fails on a stdout mismatch. Regenerated via `-regenerate-snapshots` and hand-verified against the diff CI itself produced; unrelated macOS-local artifacts (trailing blank-line drift, toolchain-info column width) from the same batch regeneration run were discarded rather than committed.
…ndependent TestPrintStackEvent_FailedStatusWithReason and TestValidateTemplate assert raw substring containment against captured stderr from ui.Error/ui.Success, which route through toastMarkdown's styled renderer. At narrower terminal widths (as GitHub Actions' non-TTY runners render, vs. this local environment), toastMarkdown soft-wraps the text and re-emits ANSI style codes per wrapped segment, splitting literal substrings like "CREATE_FAILED" and "template is valid" across escape-code (and sometimes newline+indent) boundaries — breaking the naive assert.Contains checks in CI while passing locally. Verified against a real repro: sweeping COLUMNS from 15 to 120 reproduced both failure shapes (escape-code-only splits and real newline+indent wraps). Fixed by stripping ANSI (github.com/charmbracelet/x/ansi, already vendored) and collapsing whitespace before asserting, via a shared normalizeUIOutput test helper — confirmed passing at every width in that sweep, not just the one CI happened to hit.
The dupl linter no longer flags this component-processing block as a duplicate after rebasing onto main's newer Helm lifecycle changes, leaving the suppression directive itself flagged by nolintlint as unused. Removed the directive; kept the explanatory comment.
The documented validation command mixed a single-file argument (graph_test.go) with a package-path argument (./pkg/component/...), which go test rejects outright ("named files must be .go files"). Confirmed by running it. Replaced with the package path plus a -run filter for the actual new test, confirmed passing.
Found by coderabbit review --agent --committed --base osterman/cfn-phase3-stacksets-observability --dir docs
…ction never auto-disables streamStackEvents now animates a live spinner while a stack apply is in progress: in-progress resources update the spinner's live line, completed/failed resources print permanently above it colored by outcome (pkg/ui/spinner gains a Println for this). Non-TTY output is unchanged (plain event lines via printStackEvent). applyTerminationProtection is now a no-op unless the component opts in via termination_protection: true, instead of unconditionally reconciling on every apply. That let apply implicitly disable protection by omission, defeating the point of the setting, and broke targets with no UpdateTerminationProtection support (e.g. an AWS emulator) for components that never asked for the feature. Disabling protection now only ever happens via the explicit --disable-termination-protection delete flag. Updated delete's hint text, the CLI/stack-config docs, and the PRD to match. Regenerated the cloudformation lifecycle example cast and updated its validation script for the new spinner-based output shape.
…erroring
atmos aws cloudformation apply/deploy/delete/diff/plan/render/validate/
output/changeset-*/drift-*/get-*/fmt with a component but no --stack
always failed with "stack is required" instead of prompting, unlike
terraform and CFN's own backend subcommands.
Three compounding gaps in pkg/flags, not just missing wiring:
- newOperationCommand's per-command parser never registered the stack
flag itself (it only existed via cobra's inherited persistent flag from
the top-level CloudFormationCmd parser); WithCompletionPrompt only sees
flags registered on its own parser instance.
- promptForSingleMissingFlag resolved the prompted value only into the
parser's returned ParsedConfig, never back onto the underlying
pflag.Flag -- so even with the prompt wired up, cmd.Flag("stack") (what
CFN's applySelectionFlags actually reads) would still see it as empty.
- WithCompletionPrompt had no conditional gate (unlike
WithConditionalPositionalArgPrompt), so wiring it unconditionally would
have broken --all/--affected/--tags/--labels bulk operations by
prompting for a single stack that doesn't apply to them.
pkg/flags/options.go: add WithConditionalCompletionPrompt, mirroring
WithConditionalPositionalArgPrompt; WithCompletionPrompt now delegates to
it with a nil gate, so existing callers are unaffected.
pkg/flags/standard.go: promptForSingleMissingFlag now checks the
ShouldPrompt gate and writes the resolved value back onto the pflag.Flag
(cobraFlag.Value.Set + Changed = true), matching
cmd/terraform/shared.PromptForStack's existing pattern.
cmd/aws/cloudformation/cloudformation.go: operationFlagOptions now
registers the stack flag locally; newOperationCommand wires
WithConditionalCompletionPrompt("stack", ...), gated by the same
hasSelectionFlags check already used for the component prompt.
… chapter reorg Three examples (hooks-tflint, hooks-tfmigrate, hooks-tfmigrate-advanced) shipped with zero README front matter, so they showed raw directory names and never appeared under any category filter except "All" -- tags: were simply empty. terraform-component-mocks and task-runner-dependencies had the same gap (the latter had no README at all). All five now have title/tags front matter; task-runner-dependencies gets a new README describing its custom-command/workflow dependency-DAG fixtures. The "All" view's per-chapter grouping used only each example's primary (first) tag, so a tag that's never anyone's first tag -- e.g. backend-provisioning's [Emulators, Terraform] -- rendered a working filter chip but a permanently empty (and thus dropped) section, even though the tag genuinely had a member. IndexPage.tsx now falls back to matching any of an example's tags when nothing claims the tag as primary. Recategorized several examples whose primary tag didn't match their actual subject: cloudformation and backend-provisioning off Emulators (onto Components and Terraform respectively, keeping Emulators as secondary), local-gitops onto Kubernetes, generate-files/ native-terraform/terraform-tests/caching onto Terraform, demo-ansible/ packer-docker onto Components. Added Terraform to the curated DEFAULT_TAG_ORDER (it now has enough primary-tagged members to warrant a real chapter position instead of an alphabetical-tail afterthought). website/plugins/file-browser/index.js's TAGS_MAP is now ~60 entries lighter: every example it covered has since migrated to front matter (which always wins), so those entries were dead weight risking future edits to code nothing reads. Left only scaffolding/scaffolding-matrix, which deliberately can't use front matter -- atmos scaffold generate copies their README.md verbatim into every generated project, so Docusaurus front matter there would leak into user output; gave them proper titles via the (still-live) TITLES_MAP instead. .claude/agents/example-creator.md described a legacy TAGS_MAP/DOCS_MAP mechanism that no longer matches reality -- exactly how the frontmatter gaps above went unnoticed. Rewritten to document the actual, current front-matter convention and the scaffolding exception.
…E diagram The ASCII-art loop diagram used 5 leading spaces on two lines, not a multiple of 2, failing the atmos-validate-editorconfig pre-commit hook (which scans the whole tree, not just the diff, so this surfaced on an unrelated commit that only touched this file's front matter). Shifted both lines to 6 spaces, aligning the loop-back arrow under "render".
…te expects deleteStack respected termination_protection: true from local config but had no way to detect that a stack is still protected in AWS after local config was (correctly) edited to termination_protection: false - apply only ever turns protection on, never off, so local config and live AWS state can legitimately and permanently diverge. In that drifted state, deleteStack skipped its protection gate entirely and let AWS's raw, generic "cannot be deleted while TerminationProtection is enabled" error through instead of the actionable --disable-termination-protection hint. checkTerminationProtectionGate now verifies the stack's live EnableTerminationProtection via DescribeStacks whenever local config says false, short-circuits with zero extra API calls when local config already says true, and skips the live lookup entirely for --disable-termination-protection (which unconditionally disables protection regardless of which signal reported it). The DescribeStacks result is reused by the --retain-resources gate when both apply, so a delete never issues more than one extra API call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…required S3 targets and renamed error wrapping Three tests broke silently during the phase3->phase4 rebase merge, all pre-existing coverage that just needed updating to match already-landed behavior (not new regressions this rebase introduces): - TestFindS3BackendTargets never set `region` on its S3 target fixtures; s3ConfigFromTarget now requires it (TemplateURL construction needs it, and there's no reliable way to recover it after the S3 upload). - TestResolveSpecAndTemplate_OutputSkipsProvisioning used the `template:` key to mean "path to load from disk", but that key now means an inline template body (phase4's inline-template support); switched it to `path:`. - TestRunApply_RenderOutputsError and the renderOutputsSummary tests asserted a stale "format CloudFormation outputs" error string; the merged renderOutputsSummary now wraps with errUtils.ErrInvalidFlag and "failed to format outputs" (combining a phase1 fix that stopped swallowing Write() errors with phase4's fix that stopped swallowing invalid --format errors). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tion-length lint finding Rebasing phase4 onto the current phase3 base pushed deliverApply to 62 lines (limit 60) after picking up the merged imports/wiring. Extracted the packaging block into its own packageIfNeeded helper -- no behavior change, pure extraction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
atmos lint --changed flagged 16 missing t.Parallel() calls across the cloudformation-related test files and one cyclop finding in componentTypeHasExplicitBasePath (13 > 10). Split the base-path switch into configuredBasePathForComponentType to bring complexity back under the limit, added t.Parallel() where safe, and marked the three tests that mutate the shared global component registry (via registerFakeComponentTypes) and two tests that intentionally mirror sibling component-type tests with explanatory //nolint directives matching existing repo precedent.
|
CodeRabbit (@coderabbitai) full review |
|
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
|
Superseding this PR — its 220-file diff (code + docs combined) exceeded CodeRabbit's 150-file-per-review cap on this repo's free-OSS plan, so it's been split into two PRs off the same fixed base:
No content from this PR was dropped; it's all carried forward into #3136 + #3137 (verified via diff-of-diff checks against the original diff before closing). Closing without merging — not deleting the |
what
aws/cloudformationcomponent type: backend management(
backend create/describe/update/delete/list) for the S3 artifact bucket CloudFormationpackaging uses, and a migration guide for Rain/raw-CloudFormation users.
infinite recursion (shared by every bulk-capable component type),
logsstdout routing, missingconfirmation gates, changeset/diff leaks, error-sentinel misuse, and small doc drift.
provision.backend.enabled: truenow actually provisions,instead of requiring a manual
backend createfirst).template:is now the inline template body (string orstructured map, flowing through Atmos's own
{{ }}templating pipeline), and a newpath:keytakes over the file-reference role
template:used to have.EndpointURLoverride in the sharedpkg/ci/artifact/s3store thatbroke template uploads against emulated AWS endpoints (e.g. Floci).
aws cfn/helm/kubernetesduplicating the entire global CLI flag set as localpersistent flags (found via this session's
--helpoutput investigation).--labelsrepeatable across every command family that has it (kubernetes, helm,container, workflow, terraform, vendor, list, cloudformation), matching
--tags' existingrepeatable-flag behavior.
why
management, a documented migration path off Rain, and every rough edge found during a real,
adversarial DX test pass fixed before the feature is announced.
discovered during this pass, not originally scoped work.
references
docs/fixes/.website/docs/migration/from-rain.mdxcloudformation-component-prd.