Add explicit stack name override via manifest 'name' field#1835
Add explicit stack name override via manifest 'name' field#1835
Conversation
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1835 +/- ##
==========================================
+ Coverage 73.10% 73.13% +0.02%
==========================================
Files 550 550
Lines 53176 53208 +32
==========================================
+ Hits 38876 38913 +37
+ Misses 11441 11440 -1
+ Partials 2859 2855 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughIntroduces explicit stack-level name override via a "name" field in stack manifests, with precedence: manifest name > name_template > name_pattern > filename. The feature spans core stack processing, schema updates, configuration constants, test fixtures, and comprehensive documentation across multiple files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.go📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
pkg/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2024-10-20T00:41:57.135ZApplied to files:
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📝 WalkthroughWalkthroughThis PR introduces per-stack manifest-level name overrides. It adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
website/blog/2025-12-03-stack-manifest-name-override.mdx (1)
135-143: Minor wording suggestion.The static analysis flagged word repetition on line 143 ("needs" used twice). Consider rewording for variety.
-Add a `name` field to any stack manifest that needs an explicit name. This feature is fully backward compatible - stacks without a `name` field continue to use `name_template`, `name_pattern`, or the filename as before. +Add a `name` field to any stack manifest requiring an explicit override. This feature is fully backward compatible—stacks without a `name` field continue to use `name_template`, `name_pattern`, or the filename as before.internal/exec/stack_manifest_name_test.go (2)
15-148: Extract repeated test setup to reduce duplication.All three test functions have identical setup code (changing directory, initializing config). Consider extracting this into a helper function and defining the test directory as a constant.
Apply this pattern:
+const testFixtureDir = "../../tests/fixtures/scenarios/stack-manifest-name" + +// setupTest initializes the test environment and returns the atmos config. +func setupTest(t *testing.T) schema.AtmosConfiguration { + t.Helper() + t.Chdir(testFixtureDir) + + configAndStacksInfo := schema.ConfigAndStacksInfo{} + atmosConfig, err := cfg.InitCliConfig(configAndStacksInfo, true) + require.NoError(t, err) + + return atmosConfig +} + func TestStackManifestNameInStacksMap(t *testing.T) { - // Change to the test fixture directory. - testDir := "../../tests/fixtures/scenarios/stack-manifest-name" - t.Chdir(testDir) - - // Initialize the CLI config. - configAndStacksInfo := schema.ConfigAndStacksInfo{} - atmosConfig, err := cfg.InitCliConfig(configAndStacksInfo, true) - require.NoError(t, err) + atmosConfig := setupTest(t) // Call FindStacksMap to get the processed stacks. stacksMap, _, err := FindStacksMap(&atmosConfig, false)
59-98: Consider adding error case coverage.The tests validate happy paths well, but consider adding test cases for error scenarios to improve coverage:
- Invalid or empty name values in manifests
- Missing fixture files
- Malformed stack configurations
This would help catch edge cases and improve overall test reliability.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
CLAUDE.md(1 hunks)internal/exec/describe_stacks.go(7 hunks)internal/exec/stack_manifest_name_test.go(1 hunks)internal/exec/stack_processor_process_stacks.go(2 hunks)internal/exec/stack_utils.go(1 hunks)pkg/config/const.go(1 hunks)pkg/datafetcher/schema/stacks/stack-config/1.0.json(2 hunks)pkg/schema/schema.go(1 hunks)tests/fixtures/scenarios/stack-manifest-name/atmos.yaml(1 hunks)tests/fixtures/scenarios/stack-manifest-name/components/terraform/mock/main.tf(1 hunks)tests/fixtures/scenarios/stack-manifest-name/stacks/catalog/base.yaml(1 hunks)tests/fixtures/scenarios/stack-manifest-name/stacks/legacy-prod.yaml(1 hunks)tests/fixtures/scenarios/stack-manifest-name/stacks/no-name-prod.yaml(1 hunks)website/blog/2025-12-03-stack-manifest-name-override.mdx(1 hunks)website/docs/core-concepts/stacks/stacks.mdx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
pkg/schema/schema.gopkg/config/const.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_utils.gointernal/exec/stack_manifest_name_test.gointernal/exec/describe_stacks.go
website/blog/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, after intro, tags (feature/enhancement/bugfix/contributors), and author (committer's GitHub username).
Files:
website/blog/2025-12-03-stack-manifest-name-override.mdx
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/blog/2025-12-03-stack-manifest-name-override.mdxwebsite/docs/core-concepts/stacks/stacks.mdx
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Files:
internal/exec/stack_manifest_name_test.go
pkg/datafetcher/schema/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
Update all schemas in pkg/datafetcher/schema/ when adding config options.
Files:
pkg/datafetcher/schema/stacks/stack-config/1.0.json
🧠 Learnings (34)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
📚 Learning: 2024-10-20T00:41:57.135Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
Applied to files:
pkg/schema/schema.gointernal/exec/stack_utils.gointernal/exec/stack_manifest_name_test.gointernal/exec/describe_stacks.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
pkg/config/const.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/stack_processor_process_stacks.gointernal/exec/stack_utils.gointernal/exec/stack_manifest_name_test.gointernal/exec/describe_stacks.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/stack_processor_process_stacks.gointernal/exec/stack_manifest_name_test.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/stack_processor_process_stacks.gointernal/exec/stack_utils.gotests/fixtures/scenarios/stack-manifest-name/atmos.yamlinternal/exec/stack_manifest_name_test.gointernal/exec/describe_stacks.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
internal/exec/stack_processor_process_stacks.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
internal/exec/stack_processor_process_stacks.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/stack_utils.gointernal/exec/stack_manifest_name_test.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/stack_utils.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/stack_utils.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
internal/exec/stack_utils.go
📚 Learning: 2024-12-03T04:01:16.446Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx:0-0
Timestamp: 2024-12-03T04:01:16.446Z
Learning: In the `terraform.output.mdx` documentation file (`website/docs/core-concepts/stacks/yaml-functions/terraform.output.mdx`), the cache invalidation and cache scope behavior for the `!terraform.output` function are already described.
Applied to files:
website/blog/2025-12-03-stack-manifest-name-override.mdx
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yamlwebsite/docs/core-concepts/stacks/stacks.mdxpkg/datafetcher/schema/stacks/stack-config/1.0.json
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-01-25T15:21:40.413Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-09-13T16:39:20.007Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: cmd/markdown/atmos_toolchain_aliases.md:2-4
Timestamp: 2025-09-13T16:39:20.007Z
Learning: In the cloudposse/atmos repository, CLI documentation files in cmd/markdown/ follow a specific format that uses " $ atmos command" (with leading space and dollar sign prompt) in code blocks. This is the established project convention and should not be changed to comply with standard markdownlint rules MD040 and MD014.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to website/blog/**/*.mdx : PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, <!--truncate--> after intro, tags (feature/enhancement/bugfix/contributors), and author (committer's GitHub username).
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-29T17:35:01.177Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1819
File: website/blog/2025-10-13-introducing-atmos-auth.md:5-6
Timestamp: 2025-11-29T17:35:01.177Z
Learning: In the Atmos blog (website/blog/), blog post authors should be set to the actual content author (the person who wrote the feature/content), not generic organization names like "cloudposse" or "atmos". If the actual author differs from the PR opener, use the actual author. Authors must exist in website/blog/authors.yml.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Update website documentation in the `website/` directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to CHANGELOG.md : Follow semantic versioning, update CHANGELOG.md with each release, and create GitHub releases with detailed release notes
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Applied to files:
CLAUDE.md
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/components/terraform/mock/main.tftests/fixtures/scenarios/stack-manifest-name/stacks/no-name-prod.yamlinternal/exec/stack_manifest_name_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to internal/exec/template_funcs.go : New configs support Go templating with FuncMap() from internal/exec/template_funcs.go. Implement template functions in internal/exec/template_funcs.go, register them, add tests, and update documentation.
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2024-10-31T19:23:45.538Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:65-66
Timestamp: 2024-10-31T19:23:45.538Z
Learning: The variable `shouldCheckStack` in `ExecuteTerraform` controls whether validation is performed.
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to pkg/datafetcher/schema/**/*.json : Update all schemas in pkg/datafetcher/schema/ when adding config options.
Applied to files:
pkg/datafetcher/schema/stacks/stack-config/1.0.json
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/describe_stacks.go
🪛 LanguageTool
website/blog/2025-12-03-stack-manifest-name-override.mdx
[typographical] ~24-~24: To join two clauses or introduce examples, consider using an em dash.
Context: ...nt}-{environment}-{stage}` - Default - The basename of the stack file (zero-con...
(DASH_RULE)
[typographical] ~57-~57: To join two clauses or introduce examples, consider using an em dash.
Context: ... in this order: 1. name (highest) - Explicit name from stack manifest 2. **`...
(DASH_RULE)
[typographical] ~59-~59: To join two clauses or introduce examples, consider using an em dash.
Context: ... from atmos.yaml 3. name_pattern - Token pattern from atmos.yaml 4. **Def...
(DASH_RULE)
[typographical] ~60-~60: To join two clauses or introduce examples, consider using an em dash.
Context: ...pattern from atmos.yaml 4. Default - Basename of the stack file This means y...
(DASH_RULE)
[style] ~143-~143: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...name field to any stack manifest that needs an explicit name. This feature is fully...
(EN_REPEATEDWORDS_NEED)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (27)
tests/fixtures/scenarios/stack-manifest-name/components/terraform/mock/main.tf (1)
1-17: LGTM!Clean test fixture with sensible defaults for exercising the stack manifest name feature.
tests/fixtures/scenarios/stack-manifest-name/stacks/catalog/base.yaml (1)
1-10: LGTM!Well-structured base catalog that provides shared configuration for the test scenario.
tests/fixtures/scenarios/stack-manifest-name/stacks/no-name-prod.yaml (1)
1-13: LGTM!Excellent test fixture for validating the fallback behavior when no explicit
nameis provided. The comment clearly documents the intent.pkg/schema/schema.go (1)
625-625: LGTM!Clean addition of the
StackManifestNamefield with clear documentation explaining its role in the naming precedence hierarchy.internal/exec/stack_utils.go (1)
29-45: LGTM!The precedence logic is cleanly implemented. The manifest name takes priority as intended, with appropriate fallbacks to
name_template,name_pattern, and finally the filename. The comment clearly documents the precedence order.tests/fixtures/scenarios/stack-manifest-name/atmos.yaml (1)
1-12: LGTM!Standard test configuration that properly sets up the environment for validating the stack manifest name feature.
website/docs/core-concepts/stacks/stacks.mdx (1)
105-186: LGTM!Excellent documentation that clearly explains the new
nameoverride feature, its precedence, and provides a practical example. The use case justification helps users understand when to apply this feature.pkg/config/const.go (1)
65-65: LGTM!Clean addition of the
NameSectionNameconstant, consistently named and appropriately placed alongside other section constants.tests/fixtures/scenarios/stack-manifest-name/stacks/legacy-prod.yaml (1)
1-15: LGTM!Clean fixture that demonstrates the new
namefield override. The structure is correct and the comment clearly indicates the purpose.internal/exec/stack_processor_process_stacks.go (2)
51-57: LGTM!The extraction logic correctly reads the optional
namefield with proper type assertion and follows the existing pattern used for other global sections.
522-525: LGTM!Correctly injects the stack manifest name into the result map only when set, maintaining backward compatibility.
website/blog/2025-12-03-stack-manifest-name-override.mdx (2)
1-9: LGTM!Front matter is properly structured with required fields. The
featuretag andostermanauthor align with the PR guidelines.
53-62: Documentation aligns with implementation.The precedence order documented here matches the code implementation across
describe_stacks.goandstack_utils.go. This consistency is important.pkg/datafetcher/schema/stacks/stack-config/1.0.json (2)
7-10: LGTM!The
nameproperty is correctly defined with proper type and a clear description that explains the precedence behavior.
111-116: LGTM!Adding
nameto theanyOfalternatives allows a minimal manifest with just a name override to be valid. This follows the existing pattern for other optional top-level fields.CLAUDE.md (2)
420-421: LGTM!Good addition. Requiring explicit reads of
tags.ymlandauthors.ymlprevents invalid metadata in blog posts.
436-438: LGTM!The expanded tag taxonomy and the
coretag for internal changes provide better categorization options.internal/exec/describe_stacks.go (8)
149-149: LGTM!Variable declaration follows the existing pattern of declaring section variables at function scope.
157-163: LGTM!Correctly extracts the stack-level name override at the start of stack processing. The pattern matches other extraction logic in this file.
264-290: LGTM!The
StackManifestNamefield is correctly populated inConfigAndStacksInfofor Terraform components. This ensures the manifest name flows through to workspace derivation.
304-322: Precedence logic is correct.The stack name resolution follows the documented order: manifest name > name_template > name_pattern > filename. The explicit else clause for filename fallback is a good addition for clarity.
Based on retrieved learnings, the duplication across component types is intentional.
512-538: LGTM!
StackManifestNameis consistently populated for Helmfile components, matching the Terraform implementation.
552-570: LGTM!Helmfile precedence logic is consistent with Terraform. The duplication is acceptable per project conventions.
737-763: LGTM!
StackManifestNameis consistently populated for Packer components, completing the coverage across all component types.
777-795: LGTM!Packer precedence logic completes the consistent implementation across all component types. Well done maintaining parity.
internal/exec/stack_manifest_name_test.go (2)
3-11: Import organization looks solid.The three-group structure with proper aliases follows the guidelines perfectly.
130-147: Deep nesting is explicit but verbose.The nested type assertions provide good error messages at each level, which is helpful for debugging. Just be aware that this makes the test somewhat brittle to structure changes. The current approach is fine for verifying this specific behavior.
osterman
force-pushed
the
osterman/stack-manifest-name
branch
from
6241bf4 to
d53cf4b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exec/stack_processor_process_stacks.go (1)
51-57: Stack manifestnameextraction/propagation looks good, with one optional tightening.The new logic cleanly:
- Reads a string
namefrom the raw config without affecting existing processing.- Adds it back into the processed
resultmap so downstream consumers can respect the manifest override.One thing you might consider aligning with other top-level sections: if
config["name"]is present but not a string, you currently ignore it. Other sections (e.g.,vars,terraform) return a typed error on mismatch. If you want consistent validation, treating a non-stringnameas an error instead of silently dropping it could make misconfigurations more obvious.Also applies to: 522-525
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
CLAUDE.md(1 hunks)internal/exec/describe_stacks.go(7 hunks)internal/exec/stack_manifest_name_test.go(1 hunks)internal/exec/stack_processor_process_stacks.go(2 hunks)internal/exec/stack_utils.go(1 hunks)pkg/config/const.go(1 hunks)pkg/datafetcher/schema/stacks/stack-config/1.0.json(2 hunks)pkg/schema/schema.go(1 hunks)tests/fixtures/scenarios/stack-manifest-name/atmos.yaml(1 hunks)tests/fixtures/scenarios/stack-manifest-name/components/terraform/mock/main.tf(1 hunks)tests/fixtures/scenarios/stack-manifest-name/stacks/catalog/base.yaml(1 hunks)tests/fixtures/scenarios/stack-manifest-name/stacks/legacy-prod.yaml(1 hunks)tests/fixtures/scenarios/stack-manifest-name/stacks/no-name-prod.yaml(1 hunks)website/blog/2025-12-03-stack-manifest-name-override.mdx(1 hunks)website/docs/learn/stacks/stacks.mdx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Use functional options pattern to avoid functions with many parameters. Define Option functions that modify a Config struct and pass variadic options to New functions.
Use context.Context for cancellation signals, deadlines/timeouts, and request-scoped values (trace IDs). Context should be first parameter in functions that accept it. DO NOT use context for configuration, dependencies, or avoiding proper function parameters.
All comments must end with periods. This is enforced by the godot linter.
NEVER delete existing comments without a very strong reason. Preserve helpful comments, update them to match code changes, refactor for clarity, and add context when modifying code. Only remove factually incorrect, duplicated, or outdated comments.
Organize imports in three groups separated by blank lines (Go stdlib, 3rd-party excluding cloudposse/atmos, Atmos packages), sorted alphabetically. Maintain aliases: cfg, log, u, errUtils.
Add defer perf.Track(atmosConfig, "pkg.FuncName")() + blank line to all public functions. Use nil if no atmosConfig parameter.
All errors MUST be wrapped using static errors defined in errors/errors.go. Use errors.Join for combining multiple errors, fmt.Errorf with %w for adding string context, error builder for complex errors, and errors.Is() for error checking. NEVER use dynamic errors directly.
Use go.uber.org/mock/mockgen with //go:generate directives for mock generation. Never create manual mocks.
Use viper.BindEnv with ATMOS_ prefix for environment variables.
Use colors from pkg/ui/theme/colors.go for theme consistency.
Ensure Linux/macOS/Windows compatibility. Use SDKs over binaries. Use filepath.Join(), not hardcoded path separators.
Small focused files (<600 lines). One cmd/impl per file. Co-locate tests. Never use //revive:disable:file-length-limit.
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider us...
Files:
pkg/config/const.gopkg/schema/schema.gointernal/exec/stack_utils.gointernal/exec/describe_stacks.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_manifest_name_test.go
website/blog/**/*.mdx
📄 CodeRabbit inference engine (CLAUDE.md)
PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, after intro, tags (feature/enhancement/bugfix/contributors), and author (committer's GitHub username).
Files:
website/blog/2025-12-03-stack-manifest-name-override.mdx
website/**
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
website/**: Update website documentation in thewebsite/directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Keep website code in thewebsite/directory, follow the existing website architecture and style, and test website changes locally before committing
Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Files:
website/blog/2025-12-03-stack-manifest-name-override.mdxwebsite/docs/learn/stacks/stacks.mdx
**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*_test.go: Prefer unit tests with mocks over integration tests. Use interfaces + dependency injection for testability. Generate mocks with go.uber.org/mock/mockgen. Use table-driven tests for comprehensive coverage. Target >80% code coverage.
Test behavior, not implementation. Never test stub functions. Avoid tautological tests. Make code testable with DI to avoid os.Exit and external systems. Use assert.ErrorIs(err, ErrSentinel) for stdlib/our errors.
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Files:
internal/exec/stack_manifest_name_test.go
pkg/datafetcher/schema/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
Update all schemas in pkg/datafetcher/schema/ when adding config options.
Files:
pkg/datafetcher/schema/stacks/stack-config/1.0.json
🧠 Learnings (32)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
pkg/config/const.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to website/blog/**/*.mdx : PRs labeled minor/major MUST include blog post at website/blog/YYYY-MM-DD-feature-name.mdx with YAML front matter, <!--truncate--> after intro, tags (feature/enhancement/bugfix/contributors), and author (committer's GitHub username).
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Update website documentation in the `website/` directory when adding new features, ensure consistency between CLI help text and website documentation, and follow the website's documentation structure and style
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to website/** : Keep CLI documentation and website documentation in sync and document new features on the website with examples and use cases
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-29T17:35:01.177Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1819
File: website/blog/2025-10-13-introducing-atmos-auth.md:5-6
Timestamp: 2025-11-29T17:35:01.177Z
Learning: In the Atmos blog (website/blog/), blog post authors should be set to the actual content author (the person who wrote the feature/content), not generic organization names like "cloudposse" or "atmos". If the actual author differs from the PR opener, use the actual author. Authors must exist in website/blog/authors.yml.
Applied to files:
CLAUDE.md
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to CHANGELOG.md : Follow semantic versioning, update CHANGELOG.md with each release, and create GitHub releases with detailed release notes
Applied to files:
CLAUDE.md
📚 Learning: 2024-10-20T00:41:57.135Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:93-98
Timestamp: 2024-10-20T00:41:57.135Z
Learning: When downloading schema files in `internal/exec/validate_stacks.go`, use a consistent temporary file name to overwrite the file each time and avoid creating multiple temporary files.
Applied to files:
pkg/schema/schema.gointernal/exec/describe_stacks.gointernal/exec/stack_manifest_name_test.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
internal/exec/stack_utils.gointernal/exec/describe_stacks.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_manifest_name_test.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/stack_utils.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/stack_utils.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
internal/exec/stack_utils.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
internal/exec/stack_utils.gointernal/exec/describe_stacks.gointernal/exec/stack_processor_process_stacks.gointernal/exec/stack_manifest_name_test.go
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/stacks/no-name-prod.yamltests/fixtures/scenarios/stack-manifest-name/atmos.yamlinternal/exec/stack_manifest_name_test.gotests/fixtures/scenarios/stack-manifest-name/components/terraform/mock/main.tf
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
internal/exec/describe_stacks.go
📚 Learning: 2025-09-24T20:45:40.401Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: tests/fixtures/scenarios/atmos-auth/stacks/deploy/nonprod.yaml:3-4
Timestamp: 2025-09-24T20:45:40.401Z
Learning: In Atmos stack files, the correct syntax for importing other stack files is `import:` (singular), not `imports:` (plural). All stack files in the Atmos codebase consistently use `import:` followed by a list of paths to import.
Applied to files:
website/docs/learn/stacks/stacks.mdxtests/fixtures/scenarios/stack-manifest-name/atmos.yamlpkg/datafetcher/schema/stacks/stack-config/1.0.json
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-09-10T21:17:55.273Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/http_client_test.go:3-10
Timestamp: 2025-09-10T21:17:55.273Z
Learning: In the cloudposse/atmos repository, imports should never be changed as per samtholiya's coding guidelines.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-01-25T03:51:57.689Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-01-08T19:01:32.938Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 899
File: examples/tests/test-vendor/test-components/main.tf:1-7
Timestamp: 2025-01-08T19:01:32.938Z
Learning: In the examples/tests directory of the atmos project, code examples are intentionally kept minimal and simple to facilitate understanding. Avoid suggesting additional complexity or validations that might make the examples harder to follow.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-11-01T20:24:29.557Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1714
File: NOTICE:0-0
Timestamp: 2025-11-01T20:24:29.557Z
Learning: In the cloudposse/atmos repository, the NOTICE file is programmatically generated and should not be manually edited. Issues with dependency license URLs in NOTICE will be resolved when upstream package metadata is corrected.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2025-09-08T01:25:44.958Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1466
File: website/docs/cli/commands/toolchain/usage.mdx:117-121
Timestamp: 2025-09-08T01:25:44.958Z
Learning: The atmos toolchain has been updated to follow XDG Base Directory Specification with helper functions GetXDGCacheDir() and GetXDGTempCacheDir() in toolchain/xdg_cache.go, using XDG_CACHE_HOME when set and falling back to ~/.cache/atmos-toolchain, making it consistent with atmos core's XDG compliance.
Applied to files:
tests/fixtures/scenarios/stack-manifest-name/atmos.yaml
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/stack_processor_process_stacks.go
📚 Learning: 2025-10-13T18:13:54.020Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1622
File: pkg/perf/perf.go:140-184
Timestamp: 2025-10-13T18:13:54.020Z
Learning: In pkg/perf/perf.go, the `trackWithSimpleStack` function intentionally skips ownership checks at call stack depth > 1 to avoid expensive `getGoroutineID()` calls on every nested function. This is a performance optimization for the common single-goroutine execution case (most Atmos commands), accepting the rare edge case of potential metric corruption if multi-goroutine execution occurs at depth > 1. The ~19× performance improvement justifies this trade-off.
Applied to files:
internal/exec/stack_processor_process_stacks.go
📚 Learning: 2024-12-08T14:26:16.972Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 828
File: pkg/schema/schema.go:98-100
Timestamp: 2024-12-08T14:26:16.972Z
Learning: The `ListConfig` columns array in the `Components` struct can be empty.
Applied to files:
internal/exec/stack_processor_process_stacks.go
📚 Learning: 2025-11-24T17:35:20.297Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:35:20.297Z
Learning: Applies to internal/exec/template_funcs.go : New configs support Go templating with FuncMap() from internal/exec/template_funcs.go. Implement template functions in internal/exec/template_funcs.go, register them, add tests, and update documentation.
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 731
File: internal/exec/validate_stacks.go:0-0
Timestamp: 2024-10-20T00:57:53.500Z
Learning: In `internal/exec/validate_stacks.go`, when downloading the Atmos JSON Schema file to the temp directory, the temporary file is overwritten each time, so explicit removal is not necessary.
Applied to files:
internal/exec/stack_manifest_name_test.go
📚 Learning: 2024-10-31T19:23:45.538Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform.go:65-66
Timestamp: 2024-10-31T19:23:45.538Z
Learning: The variable `shouldCheckStack` in `ExecuteTerraform` controls whether validation is performed.
Applied to files:
internal/exec/stack_manifest_name_test.go
🧬 Code graph analysis (4)
internal/exec/stack_utils.go (1)
pkg/schema/schema.go (1)
Stacks(439-445)
internal/exec/describe_stacks.go (5)
pkg/schema/schema.go (2)
Context(480-495)Stacks(439-445)pkg/config/const.go (1)
NameSectionName(65-65)internal/exec/template_utils.go (1)
ProcessTmpl(53-97)internal/exec/stack_utils.go (1)
GetStackNamePattern(205-207)pkg/config/utils.go (2)
GetContextFromVars(640-668)GetContextPrefix(671-741)
internal/exec/stack_processor_process_stacks.go (1)
pkg/config/const.go (1)
NameSectionName(65-65)
internal/exec/stack_manifest_name_test.go (3)
pkg/schema/schema.go (1)
ConfigAndStacksInfo(621-716)internal/exec/utils.go (1)
FindStacksMap(262-312)internal/exec/describe_stacks.go (1)
ExecuteDescribeStacks(116-959)
🪛 LanguageTool
website/blog/2025-12-03-stack-manifest-name-override.mdx
[typographical] ~24-~24: To join two clauses or introduce examples, consider using an em dash.
Context: ...nt}-{environment}-{stage}` - Default - The basename of the stack file (zero-con...
(DASH_RULE)
[typographical] ~57-~57: To join two clauses or introduce examples, consider using an em dash.
Context: ... in this order: 1. name (highest) - Explicit name from stack manifest 2. **`...
(DASH_RULE)
[typographical] ~59-~59: To join two clauses or introduce examples, consider using an em dash.
Context: ... from atmos.yaml 3. name_pattern - Token pattern from atmos.yaml 4. **Def...
(DASH_RULE)
[typographical] ~60-~60: To join two clauses or introduce examples, consider using an em dash.
Context: ...pattern from atmos.yaml 4. Default - Basename of the stack file This means y...
(DASH_RULE)
[style] ~143-~143: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...name field to any stack manifest that needs an explicit name. This feature is fully...
(EN_REPEATEDWORDS_NEED)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (linux)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
🔇 Additional comments (23)
tests/fixtures/scenarios/stack-manifest-name/components/terraform/mock/main.tf (1)
1-17: Well-formed test fixture.Clean and minimal Terraform configuration that's appropriately scoped for testing the stack-manifest-name feature. Variables are properly typed with sensible defaults, and outputs correctly reference them.
pkg/datafetcher/schema/stacks/stack-config/1.0.json (1)
7-10: Stack manifestnamefield schema reads cleanly.New
nameproperty and the additionalrequired: ["name"]branch are consistent with existing schema patterns and safely extend validation for the override use case.Also applies to: 111-115
pkg/config/const.go (1)
64-66: Centralizing thenamesection key is spot on.Adding
NameSectionNamealongside other section-name constants is consistent and makes downstream usage safer.tests/fixtures/scenarios/stack-manifest-name/stacks/catalog/base.yaml (1)
1-10: Base catalog fixture looks consistent.Straightforward vars and
terraform.vpccomponent definition; fits existing fixture conventions and is enough to support the scenario.tests/fixtures/scenarios/stack-manifest-name/atmos.yaml (1)
1-12: Scenarioatmos.yamlis wired sensibly.Stack and component base paths plus include/exclude patterns align with other test configs and suit the stack-manifest-name scenario.
website/docs/learn/stacks/stacks.mdx (1)
106-108:nameoverride docs are clear and aligned with behavior.The added comment in the
stack.yamlexample and the Stack Attributes entry describe the precedence (name>name_template>name_pattern> filename) and impact on Terraform workspaces in a way that matches the new fixtures and intended semantics.Also applies to: 156-187
tests/fixtures/scenarios/stack-manifest-name/stacks/legacy-prod.yaml (1)
1-15: Legacy stack fixture cleanly exercises the override path.Custom
name, import, vars, and component config are all minimal and focused, ideal for testing the manifest-name precedence.tests/fixtures/scenarios/stack-manifest-name/stacks/no-name-prod.yaml (1)
1-13: Filename-fallback fixture nicely complements the override case.This stack omits
namewhile sharing the same catalog base, which is exactly what you need to validate the default slug behavior.pkg/schema/schema.go (1)
625-625: LGTM!The new
StackManifestNamefield is well-placed and documented. The inline comment clearly states its purpose and precedence, following the struct's existing patterns.internal/exec/stack_utils.go (1)
29-45: Clean implementation of the naming precedence.The logic correctly prioritizes
StackManifestNameover template and pattern-based naming. The comment clearly documents the precedence order, and the fallback chain remains intact.website/blog/2025-12-03-stack-manifest-name-override.mdx (1)
1-143: Well-structured blog post documenting the feature.The post covers the problem, solution, use cases, and precedence clearly. The YAML examples are practical for the migration scenarios described.
One quick check: verify that
featureexists inwebsite/blog/tags.ymlandostermanexists inwebsite/blog/authors.ymlbefore merging, per the updated CLAUDE.md guidelines.CLAUDE.md (1)
420-438: Good documentation tightening.Mandating that blog posts use predefined tags and authors from the YAML files reduces inconsistency. The categorization between user-facing and internal tags is clear.
internal/exec/stack_manifest_name_test.go (3)
15-55: Solid test coverage for the name field extraction.The test correctly validates that the
namefield is present and has the expected value. The helper function is clean for logging purposes.
57-98: Good precedence verification.This test validates the core behavior: manifest name takes precedence over filename, and stacks without a custom name use the filename. The assertions cover both positive and negative cases.
100-148: Workspace derivation test is thorough. Verifies that the Terraform workspace correctly uses the manifest-provided name by testing against the actual fixture with custom name override (my-legacy-prod-stack), which is the key integration point for this feature. The fixture directory and all supporting files are properly configured.internal/exec/describe_stacks.go (8)
149-149: Variable declaration is appropriately scoped.
157-163: Clean extraction of the manifest-level name.Using the
cfg.NameSectionNameconstant keeps the key consistent across the codebase. The reset to empty string at the start of each iteration prevents cross-contamination between stacks.
264-290: StackManifestName correctly propagated to configAndStacksInfo.This ensures downstream processing (including workspace derivation) has access to the manifest override.
304-322: Precedence logic is correct for Terraform components.The explicit else clause defaulting to
stackFileNamemakes the logic complete. The comment clearly documents the precedence order.Based on learnings, the similar logic in Helmfile and Packer sections is intentionally not extracted into a helper function.
512-538: Helmfile section mirrors Terraform correctly.StackManifestName is propagated and precedence follows the same pattern.
552-570: Helmfile precedence logic matches Terraform.
737-763: Packer section correctly propagates StackManifestName.
777-795: Packer precedence logic completes the trio.All three component types now respect the same stack name precedence, ensuring consistent behavior across Terraform, Helmfile, and Packer.
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Introduces a top-level `name` field in stack manifests that allows explicit control over logical stack names. This takes precedence over `name_template` and `name_pattern` from atmos.yaml, providing an escape hatch for legacy infrastructure or migrations from other tools. Stack name precedence: 1. `name` (highest) - Explicit name from stack manifest 2. `name_template` - From atmos.yaml, Go template 3. `name_pattern` - From atmos.yaml, token replacement 4. Default - Basename of the stack file (zero-config) - Add NameSectionName constant to pkg/config/const.go - Add StackManifestName field to ConfigAndStacksInfo struct - Update describe_stacks.go with name precedence logic for Terraform, Helmfile, Packer - Update BuildTerraformWorkspace in stack_utils.go to use manifest name - Extract and propagate name field in stack processing - Update JSON schema for stack manifests - Add comprehensive tests with fixtures - Document feature in stacks.mdx - Update CLAUDE.md with blog post tag guidance 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…guide - Create website/docs/stacks/name.mdx documenting the `name` stack manifest field - Present choice between `name_template` (declarative) vs `name` (imperative) - Add "Stack Naming for Migrations" section to Terragrunt migration guide - Update stacks overview to include `name` in configuration sections table - Update learn/stacks/stacks.mdx with precedence order and links - Fix broken documentation links in blog post 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Lead with practical reason first - every Atmos command requires a stack name. Workspace convention is secondary consideration. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
osterman
force-pushed
the
osterman/stack-manifest-name
branch
from
f5977d9 to
370fc5b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pkg/terminal/pty/pty_test.go (1)
148-151: Mirrored printf/sleep changes and enriched assertion are solid; same optional robustness note as above.The mirrored
printf+ 100ms sleep in the masking‑disabled path keeps the two tests consistent, and the updated assertion that dumps both the string and its byte slice will be very helpful when debugging CI oddities.Same optional nit as above: if this ever moves away from a fixed constant, consider passing
secretKeyas a process arg instead of embedding it in the shell command string. Otherwise this looks good to ship.Also applies to: 164-171
🧹 Nitpick comments (1)
pkg/terminal/pty/pty_test.go (1)
99-103: PTY‑friendly printf and flush delay look good; consider slightly safer command construction.The switch to
printf '%s\n'and the smalltime.Sleepto let PTY buffers settle are reasonable pragmatic fixes for CI flakiness.If you ever change
secretKeyto be dynamic or less constrained, it’d be a bit safer to avoid shell string concatenation and pass it as an argument, e.g.:- cmd := exec.Command("sh", "-c", "printf '%s\\n' '"+secretKey+"'") + cmd := exec.Command("printf", "%s\n", secretKey)Since the secret here is a fixed test constant, this is purely an optional hardening/clarity tweak, not a blocker.
Also applies to: 116-118
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
pkg/terminal/pty/pty_test.go(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Import organization: three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use//revive:disable:file-length-limit
Use sentinel errors fromerrors/errors.gowith ErrorBuilder for all user-facing errors. Useerrors.Is()for checking errors, never string comparison.WithCause()preserves underlying error messages,WithExplanation()adds context,WithHint()provides guidance,WithContext()adds key-value pairs
Usecontext.Contextas first parameter for: cancellation signals (prop...
Files:
pkg/terminal/pty/pty_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Useassert.ErrorIs(t, err, ErrSentinel)for checking our own errors and stdlib errors. Only use string matching for third-party errors
NEVER useassert.Contains(err.Error(), ...)for error checking. ALWAYS useassert.ErrorIs()instead. NEVER use string comparison:err.Error() == "..."orstrings.Contains(err.Error(), ...)
Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoidos.Exit,CheckErrorPrintAndExit, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenarios
Usego.uber.org/mock/mockgenwith//go:generatedirectives for mock generation. Never create manual mocks. Pattern://go:generate go run go.uber.org/mock/mockgen@latest -source=filename.go -destination=mock_filename_test.go
Use table-driven tests for comprehensive coverage. Tests must call actual production code, never duplicate logic. Target >80% test coverage (enforced by CodeCov)
Files:
pkg/terminal/pty/pty_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/**/*.go: Business logic packages must follow the Service-Oriented Architecture pattern with: one Service struct per domain, Provider interfaces for classes of problems, default implementations, mock implementations, and dependency injection for testability
Use interface-driven design: define interfaces for all major functionality, use dependency injection for testability, generate mocks withgo.uber.org/mock/mockgenwith//go:generatedirectives, avoid integration tests by mocking external dependencies
Create focused purpose-built packages inpkg/for new functionality, each with clear responsibility. DO NOT add new functions topkg/utils/orinternal/exec/. Each package should have focused business logic with tests
Implement registry pattern for extensibility and plugin-like architecture. Define interfaces for provider implementations, register implementations in registry, generate mocks with mockgen. Examples: Command Registry incmd/internal/registry.go, Store Registry inpkg/store/registry.go
Files:
pkg/terminal/pty/pty_test.go
🧠 Learnings (8)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to tests/**/*_test.go : NEVER use pipe redirection (`2>&1`, `| head`, `| tail`) when running tests. Piping breaks TTY detection → ASCII fallback → wrong snapshots. NEVER manually edit golden snapshot files - always use `-regenerate-snapshots` flag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace)
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenarios
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Use dependency injection and interface-driven design to make code testable - avoid `os.Exit`, `CheckErrorPrintAndExit`, and direct external system calls in testable functions
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-02-20T18:33:16.567Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1077
File: pkg/downloader/custom_github_detector.go:47-68
Timestamp: 2025-02-20T18:33:16.567Z
Learning: Go doesn't have a built-in secure string type. For sensitive data like tokens, using environment variables (which are protected by the OS) is a common and secure practice.
Applied to files:
pkg/terminal/pty/pty_test.go
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Acceptance Tests (macos)
- GitHub Check: Acceptance Tests (windows)
- GitHub Check: Summary
PTY tests fail intermittently on Linux CI due to a well-documented Go PTY race condition (go.dev/issue/57141): when a subprocess exits, EIO may be delivered to the master before all buffered output is read. Solution: Have subprocess sleep briefly (100ms) after writing output. This keeps the subprocess alive while the parent reads all buffered data, then exits cleanly without racing against the output read. Also fixes pre-existing lint errors in cmd/list and internal/exec packages: - Use %w for error wrapping instead of %v - Extract string constants for repeated flag names - Remove unused nolint directive - Add period to comment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
pkg/terminal/pty/pty_test.go (1)
147-152: Same command construction pattern as the masking test.This has the same string concatenation concern as lines 99-104. Consider applying the same environment variable approach here for consistency.
🧹 Nitpick comments (3)
cmd/list/values.go (1)
179-186: Consider extracting "vars" flag name to a constant for consistency.The new constants improve consistency, but the "vars" flag on lines 180 and 184 remains hardcoded. For full consistency with the established pattern, consider adding
flagVars = "vars"to the constant block and using it here.Apply this diff to complete the refactoring:
const ( flagAbstract = "abstract" + flagVars = "vars" flagProcessTemplates = "process-templates" flagProcessFunctions = "process-functions" )Then update lines 180 and 184:
valuesParser = newCommonListParser( flags.WithBoolFlag(flagAbstract, "", false, "Include abstract components"), - flags.WithBoolFlag("vars", "", false, "Show only vars (equivalent to --query .vars)"), + flags.WithBoolFlag(flagVars, "", false, "Show only vars (equivalent to --query .vars)"), flags.WithBoolFlag(flagProcessTemplates, "", true, "Enable/disable Go template processing in Atmos stack manifests when executing the command"), flags.WithBoolFlag(flagProcessFunctions, "", true, "Enable/disable YAML functions processing in Atmos stack manifests when executing the command"), flags.WithEnvVars(flagAbstract, "ATMOS_LIST_ABSTRACT"), - flags.WithEnvVars("vars", "ATMOS_LIST_VARS"), + flags.WithEnvVars(flagVars, "ATMOS_LIST_VARS"), flags.WithEnvVars(flagProcessTemplates, "ATMOS_PROCESS_TEMPLATES"), flags.WithEnvVars(flagProcessFunctions, "ATMOS_PROCESS_FUNCTIONS"), )pkg/terminal/pty/pty_test.go (1)
99-104: Race condition fix looks solid, but consider a more robust command construction.The sleep-based mitigation for the PTY race condition is reasonable. However, building the shell command via string concatenation is fragile. If the test secret ever contains shell metacharacters, this breaks.
Consider using an environment variable or a here-doc to avoid concatenation:
- // Subprocess writes output then sleeps briefly before exiting. - // The sleep ensures the PTY has time to read all buffered output before - // the subprocess exits and triggers EIO. This avoids the race condition - // described in https://go.dev/issue/57141 - cmd := exec.Command("sh", "-c", "printf '%s\\n' '"+secretKey+"'; sleep 0.1") + // Subprocess writes output then sleeps briefly before exiting. + // The sleep ensures the PTY has time to read all buffered output before + // the subprocess exits and triggers EIO. This avoids the race condition + // described in https://go.dev/issue/57141 + cmd := exec.Command("sh", "-c", "printf '%s\\n' \"$SECRET\"; sleep 0.1") + cmd.Env = append(cmd.Env, "SECRET="+secretKey)internal/exec/stack_processor_utils.go (1)
653-653: Consider matching the wrapping pattern from lines 649-651.The format string
"%w: stack manifest '%s'\n%w%s"is valid but could be clearer. For consistency with lines 649-651, consider:-return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("%w: stack manifest '%s'\n%w%s", errUtils.ErrInvalidStackManifest, relativeFilePath, tmplErr, stackManifestTemplatesErrorMessage) +wrappedErr := fmt.Errorf("%w: %w", errUtils.ErrInvalidStackManifest, tmplErr) +return nil, nil, nil, nil, nil, nil, nil, fmt.Errorf("%w: stack manifest '%s'%s", wrappedErr, relativeFilePath, stackManifestTemplatesErrorMessage)This matches the pattern used when
mergeContextis not nil and makes the error wrapping more explicit.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
cmd/list/components.go(3 hunks)cmd/list/components_test.go(0 hunks)cmd/list/stacks.go(3 hunks)cmd/list/values.go(3 hunks)internal/exec/stack_processor_utils.go(2 hunks)pkg/terminal/pty/pty_test.go(3 hunks)
💤 Files with no reviewable changes (1)
- cmd/list/components_test.go
🧰 Additional context used
📓 Path-based instructions (4)
cmd/**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/**/*.go: Use Cobra's recommended command structure with a root command and subcommands, implementing each command in a separate file undercmd/directory
Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
cmd/**/*.go: CLI command files must be thin wrappers that only handle CLI concerns (flags, arguments, command registration). All business logic must be delegated topkg/packages
Useflags.NewStandardParser()for command-specific flag parsing. NEVER callviper.BindEnv()orviper.BindPFlag()directly - use the unifiedpkg/flags/infrastructure instead. Register flags with Cobra command and bind to Viper for precedence handling
All CLI commands MUST implement the CommandProvider interface and register via command registry pattern incmd/internal/registry.go
Embed examples fromcmd/markdown/*_usage.mdusing//go:embed. Render withutils.PrintfMarkdown()
Files:
cmd/list/components.gocmd/list/values.gocmd/list/stacks.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags in CLI commands
Use interfaces for external dependencies to facilitate mocking and consider using testify/mock for creating mock implementations
All code must pass golangci-lint checks
Follow Go's error handling idioms: use meaningful error messages, wrap errors with context usingfmt.Errorf("context: %w", err), and consider using custom error types for domain-specific errors
Follow standard Go coding style: usegofmtandgoimportsto format code, prefer short descriptive variable names, use kebab-case for command-line flags, and snake_case for environment variables
Document all exported functions, types, and methods following Go's documentation conventions
Document complex logic with inline comments in Go code
Support configuration via files, environment variables, and flags following the precedence order: flags > environment variables > config file > defaults
Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
**/*.go: All comments must end with periods (enforced bygodotlinter)
Import organization: three groups separated by blank lines, sorted alphabetically: 1) Go stdlib, 2) 3rd-party (NOT cloudposse/atmos), 3) Atmos packages. Maintain aliases:cfg,log,u,errUtils
Adddefer perf.Track(atmosConfig, "pkg.FuncName")()+ blank line to all public functions. Usenilif no atmosConfig param
Keep files small and focused (<600 lines). One cmd/impl per file. Co-locate tests. Never use//revive:disable:file-length-limit
Use sentinel errors fromerrors/errors.gowith ErrorBuilder for all user-facing errors. Useerrors.Is()for checking errors, never string comparison.WithCause()preserves underlying error messages,WithExplanation()adds context,WithHint()provides guidance,WithContext()adds key-value pairs
Usecontext.Contextas first parameter for: cancellation signals (prop...
Files:
cmd/list/components.gointernal/exec/stack_processor_utils.gocmd/list/values.gocmd/list/stacks.gopkg/terminal/pty/pty_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages
Use table-driven tests for testing multiple scenarios in Go
Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
**/*_test.go: Useassert.ErrorIs(t, err, ErrSentinel)for checking our own errors and stdlib errors. Only use string matching for third-party errors
NEVER useassert.Contains(err.Error(), ...)for error checking. ALWAYS useassert.ErrorIs()instead. NEVER use string comparison:err.Error() == "..."orstrings.Contains(err.Error(), ...)
Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoidos.Exit,CheckErrorPrintAndExit, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenarios
Usego.uber.org/mock/mockgenwith//go:generatedirectives for mock generation. Never create manual mocks. Pattern://go:generate go run go.uber.org/mock/mockgen@latest -source=filename.go -destination=mock_filename_test.go
Use table-driven tests for comprehensive coverage. Tests must call actual production code, never duplicate logic. Target >80% test coverage (enforced by CodeCov)
Files:
pkg/terminal/pty/pty_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
pkg/**/*.go: Business logic packages must follow the Service-Oriented Architecture pattern with: one Service struct per domain, Provider interfaces for classes of problems, default implementations, mock implementations, and dependency injection for testability
Use interface-driven design: define interfaces for all major functionality, use dependency injection for testability, generate mocks withgo.uber.org/mock/mockgenwith//go:generatedirectives, avoid integration tests by mocking external dependencies
Create focused purpose-built packages inpkg/for new functionality, each with clear responsibility. DO NOT add new functions topkg/utils/orinternal/exec/. Each package should have focused business logic with tests
Implement registry pattern for extensibility and plugin-like architecture. Define interfaces for provider implementations, register implementations in registry, generate mocks with mockgen. Examples: Command Registry incmd/internal/registry.go, Store Registry inpkg/store/registry.go
Files:
pkg/terminal/pty/pty_test.go
🧠 Learnings (51)
📓 Common learnings
Learnt from: Listener430
Repo: cloudposse/atmos PR: 934
File: tests/fixtures/scenarios/docs-generate/README.md.gotmpl:99-118
Timestamp: 2025-01-25T03:51:57.689Z
Learning: For the cloudposse/atmos repository, changes to template contents should be handled in dedicated PRs and are typically considered out of scope for PRs focused on other objectives.
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/list/components.gointernal/exec/stack_processor_utils.gocmd/list/values.gocmd/list/stacks.go
📚 Learning: 2025-09-29T02:20:11.636Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1540
File: internal/exec/validate_component.go:117-118
Timestamp: 2025-09-29T02:20:11.636Z
Learning: The ValidateComponent function in internal/exec/validate_component.go had its componentSection parameter type refined from `any` to `map[string]any` without adding new parameters. This is a type safety improvement, not a signature change requiring call site updates.
Applied to files:
cmd/list/components.gointernal/exec/stack_processor_utils.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide meaningful feedback to users and include progress indicators for long-running operations in CLI commands
Applied to files:
cmd/list/components.go
📚 Learning: 2025-09-13T18:06:07.674Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1466
File: toolchain/list.go:39-42
Timestamp: 2025-09-13T18:06:07.674Z
Learning: In the cloudposse/atmos repository, for UI messages in the toolchain package, use utils.PrintfMessageToTUI instead of log.Error or fmt.Fprintln(os.Stderr, ...). Import pkg/utils with alias "u" to follow the established pattern.
Applied to files:
cmd/list/components.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
cmd/list/components.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Follow Go's error handling idioms: use meaningful error messages, wrap errors with context using `fmt.Errorf("context: %w", err)`, and consider using custom error types for domain-specific errors
Applied to files:
cmd/list/components.gointernal/exec/stack_processor_utils.go
📚 Learning: 2025-01-09T22:27:25.538Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
Applied to files:
cmd/list/components.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to cmd/**/*.go : CLI command files must be thin wrappers that only handle CLI concerns (flags, arguments, command registration). All business logic must be delegated to `pkg/` packages
Applied to files:
cmd/list/components.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Provide clear error messages to users, include troubleshooting hints when appropriate, and log detailed errors for debugging
Applied to files:
cmd/list/components.go
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2025-08-16T23:32:40.412Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:455-456
Timestamp: 2025-08-16T23:32:40.412Z
Learning: In the cloudposse/atmos Go codebase, `InitCliConfig` returns a `schema.AtmosConfiguration` value (not a pointer), while `ExecuteDescribeDependents` expects a `*schema.AtmosConfiguration` pointer parameter. Therefore, when passing the result of `InitCliConfig` to `ExecuteDescribeDependents`, use `&atmosConfig` to pass the address of the value.
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2025-11-08T19:56:18.660Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1697
File: internal/exec/oci_utils.go:0-0
Timestamp: 2025-11-08T19:56:18.660Z
Learning: In the Atmos codebase, when a function receives an `*schema.AtmosConfiguration` parameter, it should read configuration values from `atmosConfig.Settings` fields rather than using direct `os.Getenv()` or `viper.GetString()` calls. The Atmos pattern is: viper.BindEnv in cmd/root.go binds environment variables → Viper unmarshals into atmosConfig.Settings via mapstructure → business logic reads from the Settings struct. This provides centralized config management, respects precedence, and enables testability. Example: `atmosConfig.Settings.AtmosGithubToken` instead of `os.Getenv("ATMOS_GITHUB_TOKEN")` in functions like `getGHCRAuth` in internal/exec/oci_utils.go.
Applied to files:
cmd/list/components.go
📚 Learning: 2024-12-11T18:46:02.483Z
Learnt from: Listener430
Repo: cloudposse/atmos PR: 844
File: cmd/terraform.go:39-39
Timestamp: 2024-12-11T18:46:02.483Z
Learning: `cliConfig` is initialized in `cmd/root.go` and can be used across the `cmd` package.
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1405
File: internal/exec/describe_dependents_test.go:651-652
Timestamp: 2025-08-16T23:33:07.477Z
Learning: In the cloudposse/atmos Go codebase, ExecuteDescribeDependents expects a pointer to AtmosConfiguration (*schema.AtmosConfiguration), so when calling it with a value returned by cfg.InitCliConfig (which returns schema.AtmosConfiguration), the address-of operator (&) is necessary: ExecuteDescribeDependents(&atmosConfig, ...).
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2024-11-13T21:37:07.852Z
Learnt from: Cerebrovinny
Repo: cloudposse/atmos PR: 764
File: internal/exec/describe_stacks.go:289-295
Timestamp: 2024-11-13T21:37:07.852Z
Learning: In the `internal/exec/describe_stacks.go` file of the `atmos` project written in Go, avoid extracting the stack name handling logic into a helper function within the `ExecuteDescribeStacks` method, even if the logic appears duplicated.
Applied to files:
cmd/list/components.gocmd/list/stacks.go
📚 Learning: 2024-11-19T23:00:45.899Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 795
File: internal/exec/stack_processor_utils.go:378-386
Timestamp: 2024-11-19T23:00:45.899Z
Learning: In the `ProcessYAMLConfigFile` function within `internal/exec/stack_processor_utils.go`, directory traversal in stack imports is acceptable and should not be restricted.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning specific error variables (like `ErrWorkflowNoSteps`, `ErrInvalidFromStep`, `ErrInvalidWorkflowStepType`, `ErrWorkflowStepFailed`) instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions while maintaining excellent user-facing error formatting.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-10-22T14:55:44.014Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1695
File: pkg/auth/manager.go:169-171
Timestamp: 2025-10-22T14:55:44.014Z
Learning: Go 1.20+ supports multiple %w verbs in fmt.Errorf, which returns an error implementing Unwrap() []error. This is valid and does not panic. Atmos uses Go 1.24.8 and configures errorlint with errorf-multi: true to validate this pattern.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to **/*.go : New configs support Go templating with `FuncMap()` from `internal/exec/template_funcs.go`. Implement template functions in `internal/exec/template_funcs.go`, register them, add tests, and update documentation
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-06-02T14:12:02.710Z
Learnt from: milldr
Repo: cloudposse/atmos PR: 1229
File: internal/exec/workflow_test.go:0-0
Timestamp: 2025-06-02T14:12:02.710Z
Learning: In the atmos codebase, workflow error handling was refactored to use `PrintErrorMarkdown` followed by returning the error instead of `PrintErrorMarkdownAndExit`. This pattern allows proper error testing without the function terminating the process with `os.Exit`, enabling unit tests to assert on error conditions.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-09-10T22:38:42.212Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1475
File: pkg/auth/identities/aws/user.go:141-145
Timestamp: 2025-09-10T22:38:42.212Z
Learning: ErrWrappingFormat is correctly defined as "%w: %w" in the errors package and is used throughout the codebase to wrap two error types together. The usage fmt.Errorf(errUtils.ErrWrappingFormat, errUtils.ErrAuthAwsFileManagerFailed, err) is the correct pattern when both arguments are error types.
Applied to files:
internal/exec/stack_processor_utils.go
📚 Learning: 2025-06-23T02:14:30.937Z
Learnt from: aknysh
Repo: cloudposse/atmos PR: 1327
File: cmd/terraform.go:111-117
Timestamp: 2025-06-23T02:14:30.937Z
Learning: In cmd/terraform.go, flags for the DescribeAffected function are added dynamically at runtime when info.Affected is true. This is intentional to avoid exposing internal flags like "file", "format", "verbose", "include-spacelift-admin-stacks", "include-settings", and "upload" in the terraform command interface, while still providing them for the shared DescribeAffected function used by both `atmos describe affected` and `atmos terraform apply --affected`.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to cmd/**/*.go : Use `flags.NewStandardParser()` for command-specific flag parsing. NEVER call `viper.BindEnv()` or `viper.BindPFlag()` directly - use the unified `pkg/flags/` infrastructure instead. Register flags with Cobra command and bind to Viper for precedence handling
Applied to files:
cmd/list/values.go
📚 Learning: 2025-10-10T23:51:36.597Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1599
File: internal/exec/terraform.go:394-402
Timestamp: 2025-10-10T23:51:36.597Z
Learning: In Atmos (internal/exec/terraform.go), when adding OpenTofu-specific flags like `--var-file` for `init`, do not gate them based on command name (e.g., checking if `info.Command == "tofu"` or `info.Command == "opentofu"`) because command names don't reliably indicate the actual binary being executed (symlinks, aliases). Instead, document the OpenTofu requirement in code comments and documentation, trusting users who enable the feature (e.g., `PassVars`) to ensure their terraform command points to an OpenTofu binary.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*.go : Use Viper for managing configuration, environment variables, and flags in CLI commands
Applied to files:
cmd/list/values.go
📚 Learning: 2025-11-10T03:03:31.505Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 0
File: :0-0
Timestamp: 2025-11-10T03:03:31.505Z
Learning: In the Atmos codebase, commands using the `StandardParser` flag pattern (from pkg/flags) do NOT need explicit `viper.BindPFlag()` calls in their code. The StandardParser encapsulates flag binding internally: flags are registered via `parser.RegisterFlags(cmd)` in init(), and bound via `parser.BindFlagsToViper(cmd, v)` in RunE, which internally calls viper.BindPFlag for each flag. This pattern is used throughout Atmos (e.g., cmd/toolchain/get.go, cmd/toolchain/info.go, cmd/toolchain/install.go, cmd/toolchain/path.go). Do not flag missing viper.BindPFlag calls when StandardParser is used.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-06-07T19:28:21.289Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.
Applied to files:
cmd/list/values.go
📚 Learning: 2024-10-31T19:25:41.298Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:233-235
Timestamp: 2024-10-31T19:25:41.298Z
Learning: When specifying color values in functions like `confirmDeleteTerraformLocal` in `internal/exec/terraform_clean.go`, avoid hardcoding color values. Instead, use predefined color constants or allow customization through configuration settings to improve accessibility and user experience across different terminals and themes.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to cmd/**/*.go : Provide comprehensive help text for all commands and flags, include examples in command help, and follow Go's documentation conventions in Cobra command definitions
Applied to files:
cmd/list/values.go
📚 Learning: 2025-05-22T19:58:32.988Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-09-05T14:57:37.360Z
Learnt from: RoseSecurity
Repo: cloudposse/atmos PR: 1448
File: cmd/ansible.go:26-28
Timestamp: 2025-09-05T14:57:37.360Z
Learning: The Atmos codebase uses a consistent pattern for commands that delegate to external tools: `PersistentFlags().Bool("", false, doubleDashHint)` where doubleDashHint provides help text about using double dashes to separate Atmos options from native command arguments. This pattern is used across terraform, packer, helmfile, atlantis, aws, and ansible commands.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-02-18T13:18:53.146Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1068
File: cmd/vendor_pull.go:31-31
Timestamp: 2025-02-18T13:18:53.146Z
Learning: Error checking is not required for cobra.Command.RegisterFlagCompletionFunc calls as these are static configurations done at init time.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to **/*.go : Use `viper.BindEnv("ATMOS_VAR", "ATMOS_VAR", "FALLBACK")` for environment variables - ATMOS_ prefix required. Configuration loading precedence: CLI flags → ENV vars → config files → defaults
Applied to files:
cmd/list/values.go
📚 Learning: 2025-10-07T00:25:16.333Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 1498
File: website/src/components/Screengrabs/atmos-terraform-metadata--help.html:25-55
Timestamp: 2025-10-07T00:25:16.333Z
Learning: In Atmos CLI, subcommands inherit flags from their parent commands via Cobra's command inheritance. For example, `atmos terraform metadata --help` shows `--affected` and related flags inherited from the parent `terraform` command (defined in cmd/terraform.go), even though the metadata subcommand doesn't explicitly define these flags. This is expected Cobra behavior and auto-generated help screengrabs accurately reflect this inheritance.
Applied to files:
cmd/list/values.go
📚 Learning: 2025-02-09T14:38:53.443Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 992
File: cmd/cmd_utils.go:0-0
Timestamp: 2025-02-09T14:38:53.443Z
Learning: Error handling for RegisterFlagCompletionFunc in AddStackCompletion is not required as the errors are non-critical for tab completion functionality.
Applied to files:
cmd/list/stacks.go
📚 Learning: 2024-10-30T13:25:45.965Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:405-412
Timestamp: 2024-10-30T13:25:45.965Z
Learning: In `internal/exec/terraform_clean.go`, when appending `stackFolders` to `folders` in the `handleCleanSubCommand` function, it's unnecessary to check if `stackFolders` is nil before appending, because in Go, appending a nil slice is safe and does not cause a panic.
Applied to files:
cmd/list/stacks.go
📚 Learning: 2024-10-27T04:34:08.011Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 727
File: internal/exec/terraform_clean.go:183-187
Timestamp: 2024-10-27T04:34:08.011Z
Learning: In the `getStackTerraformStateFolder` function, it's acceptable and not an error if no Terraform state folders are found for a stack.
Applied to files:
cmd/list/stacks.go
📚 Learning: 2025-02-03T05:57:18.407Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Applied to files:
cmd/list/stacks.go
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
Repo: cloudposse/atmos PR: 736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
cmd/list/stacks.go
📚 Learning: 2025-09-07T18:07:00.549Z
Learnt from: Benbentwo
Repo: cloudposse/atmos PR: 1452
File: cmd/auth_login.go:43-44
Timestamp: 2025-09-07T18:07:00.549Z
Learning: In the atmos project, the identity flag is defined as a persistent flag on the auth root command (cmd/auth.go), making it available to all auth subcommands without needing to be redefined in each individual subcommand.
Applied to files:
cmd/list/stacks.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to tests/**/*_test.go : NEVER use pipe redirection (`2>&1`, `| head`, `| tail`) when running tests. Piping breaks TTY detection → ASCII fallback → wrong snapshots. NEVER manually edit golden snapshot files - always use `-regenerate-snapshots` flag. Snapshots capture exact output including invisible formatting (lipgloss padding, ANSI codes, trailing whitespace)
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Applies to **/*_test.go : Test behavior, not implementation. Never test stub functions - implement or remove. Avoid tautological tests. Make code testable using dependency injection to avoid `os.Exit`, `CheckErrorPrintAndExit`, external systems. Remove always-skipped tests - fix or delete. Table-driven tests must have real scenarios
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-11-24T17:35:37.209Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-11-24T17:35:37.209Z
Learning: Applies to **/*_test.go : Include integration tests for command flows and test CLI end-to-end when possible with test fixtures
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2024-11-10T18:37:10.032Z
Learnt from: osterman
Repo: cloudposse/atmos PR: 768
File: internal/exec/vendor_component_utils.go:354-360
Timestamp: 2024-11-10T18:37:10.032Z
Learning: In the vendoring process, a TTY can exist without being interactive. If the process does not prompt the user, we should not require interactive mode to display the TUI. The `CheckTTYSupport` function should check TTY support on stdout rather than stdin.
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-12-06T19:24:49.343Z
Learnt from: CR
Repo: cloudposse/atmos PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-06T19:24:49.343Z
Learning: Use dependency injection and interface-driven design to make code testable - avoid `os.Exit`, `CheckErrorPrintAndExit`, and direct external system calls in testable functions
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2025-02-20T18:33:16.567Z
Learnt from: samtholiya
Repo: cloudposse/atmos PR: 1077
File: pkg/downloader/custom_github_detector.go:47-68
Timestamp: 2025-02-20T18:33:16.567Z
Learning: Go doesn't have a built-in secure string type. For sensitive data like tokens, using environment variables (which are protected by the OS) is a common and secure practice.
Applied to files:
pkg/terminal/pty/pty_test.go
📚 Learning: 2024-11-07T20:16:15.381Z
Learnt from: pkbhowmick
Repo: cloudposse/atmos PR: 766
File: cmd/cmd_utils.go:177-178
Timestamp: 2024-11-07T20:16:15.381Z
Learning: In 'cmd/cmd_utils.go', using `os.Exit(1)` directly is acceptable in this context, and wrapping it with deferred cleanup is considered unnecessary.
Applied to files:
pkg/terminal/pty/pty_test.go
🧬 Code graph analysis (3)
internal/exec/stack_processor_utils.go (1)
errors/errors.go (1)
ErrInvalidStackManifest(252-252)
cmd/list/values.go (1)
pkg/flags/options.go (2)
WithBoolFlag(51-62)WithEnvVars(221-244)
pkg/terminal/pty/pty_test.go (1)
pkg/terminal/terminal.go (1)
Stdin(83-83)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
🔇 Additional comments (7)
cmd/list/components.go (1)
58-58: LGTM – Solid error handling improvements.The changes improve error handling by:
- Using
%wfor proper error wrapping (lines 87, 98), enablingerrors.Is()anderrors.Unwrap()- Explicitly discarding the
ui.Inforeturn value with blank identifier (line 58)These align well with Go's error handling idioms and the coding guidelines.
Also applies to: 87-87, 98-98
cmd/list/stacks.go (1)
58-58: LGTM – Consistent error handling improvements.Same solid improvements as in
components.go:
- Proper error wrapping with
%w(lines 88, 99)- Explicit handling of
ui.Inforeturn value (line 58)Nice consistency across both list commands.
Also applies to: 88-88, 99-99
cmd/list/values.go (2)
39-44: Good refactoring with flag name constants.Extracting flag names to constants eliminates magic strings and improves maintainability. The unexported constants are appropriately scoped for internal package use.
207-212: Consistent application of flag constants.The constants are used correctly throughout the varsParser initialization. The absence of the "vars" flag is appropriate since the vars command inherently sets the query to ".vars".
pkg/terminal/pty/pty_test.go (1)
170-170: Nice improvement to test diagnostics.The enhanced error message with quoted string format and byte representation will make debugging PTY output issues much easier.
internal/exec/stack_processor_utils.go (2)
649-649: LGTM!Properly wraps both the sentinel error and template error using
%w, enabling correct error unwrapping witherrors.Is().
1397-1397: LGTM!Comment now properly ends with a period per coding guidelines.
Resolved conflicts by accepting main's refactored list commands while preserving the PTY test fix (subprocess sleep approach to avoid race condition). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
These changes were released in v1.203.0-test.1. |
2 participants
what
namefield in stack manifests for explicit control over logical stack namesname_templateandname_patternfrom atmos.yamlwhy
Stack naming conventions work well for greenfield projects but struggle with:
This feature allows selective stack name overrides without requiring all stacks to follow a pattern.
references
metadata.namefield for component renamingSummary by CodeRabbit
New Features
Documentation
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.