Improve atmos describe affected and atmos describe stacks commands#1590
Improve atmos describe affected and atmos describe stacks commands#1590
atmos describe affected and atmos describe stacks commands#1590Conversation
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
aknysh
added
the
no-release
|
@aknysh update PR description and title |
aknysh
changed the title
atmos describe affected and atmos describe stacks commands
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1590 +/- ##
==========================================
+ Coverage 60.82% 60.92% +0.10%
==========================================
Files 290 290
Lines 32848 32858 +10
==========================================
+ Hits 19979 20019 +40
+ Misses 10988 10957 -31
- Partials 1881 1882 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughSets ProcessTemplates and ProcessYamlFunctions true by default for describe stacks; adds an onlyInStack filter to addDependentsToAffected and propagates it to call sites/tests; treat components with metadata.enabled: false as disabled when computing dependents; bumps Atmos and an AWS SDK dependency; plus comment/import cosmetic edits and test/fixture updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (describe stacks)
participant Args as Arg Parser
participant Exec as Exec Layer
User->>CLI: atmos describe stacks ...
CLI->>Args: set defaults
Note right of Args #BBE3FF: ProcessTemplates = true<br/>ProcessYamlFunctions = true
Args->>Exec: Execute with args
Exec-->>User: Output
sequenceDiagram
autonumber
actor User
participant Exec as Describe Affected
participant Dep as addDependentsToAffected
participant DD as Describe Dependents
User->>Exec: Execute(includeDependents, onlyInStack?)
Exec->>Dep: addDependentsToAffected(affected, onlyInStack)
loop for each affected item
alt onlyInStack set AND item.Stack != onlyInStack
Dep-->>Dep: continue (skip)
else
Dep->>DD: ExecuteDescribeDependents(...)
Note right of DD #F3F7D9: Skip abstract & disabled components
DD-->>Dep: dependents
end
end
Dep-->>Exec: augmented affected
Exec-->>User: Result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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 (5)**/*_test.go📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
**/*.go📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
Files:
internal/exec/**/*_test.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{cmd,internal,pkg}/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
{internal/exec,pkg}/**/*.go📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2024-11-13T21:37:07.852ZApplied to files:
📚 Learning: 2024-10-20T00:57:53.500ZApplied to files:
📚 Learning: 2025-08-16T23:33:07.477ZApplied to files:
🧬 Code graph analysis (1)internal/exec/describe_affected_test.go (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (5)
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}
- Add test case with IncludeDependents flag - Add test case with IncludeDependents and Stack filter - Increases Execute function coverage from 75.0% to 87.5% - Covers the addDependentsToAffected call with onlyInStack parameter
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/exec/describe_affected_test.go (1)
239-239: Add periods to inline comments.All Go comments must end with periods per the coding guidelines (enforced by golangci-lint's godot).
As per coding guidelines.
Apply this diff to add periods to the inline comments:
- Dependents: nil, // must be nil to match actual + Dependents: nil, // Must be nil to match actual.Apply similar changes to all inline comments at lines 239, 252, 273, 287, 331, 345, 366, 380, 423, 437, 941, 955, 1007, 1028, 1049, 1063, and 1064.
Also applies to: 252-252, 273-273, 287-287, 331-331, 345-345, 366-366, 380-380, 423-423, 437-437, 941-941, 955-955, 1007-1007, 1028-1028, 1049-1049, 1063-1064
📜 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)
internal/exec/describe_affected_test.go(14 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for multiple scenarios
**/*_test.go: Use table-driven tests for unit tests.
Always use t.Skipf(...) with a clear reason when skipping tests; do not use t.Skip().
TestMain must call os.Exit(m.Run()) to propagate the test exit code for CLI tests.
Co-locate test files with implementation (use _test.go alongside .go files) and maintain naming symmetry with implementation files.
Files:
internal/exec/describe_affected_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: All code must pass golangci-lint checks
Follow Go error handling idioms and use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider custom error types for domain-specific errors
Follow standard Go coding style; run gofmt and goimports
Use snake_case for environment variables
Document complex logic with inline comments
**/*.go: All Go comments must end with periods. Enforced by golangci-lint's godot.
Organize imports into three groups (stdlib, third-party, Atmos) separated by blank lines; sort alphabetically within each group; maintain existing aliases.
Add defer perf.Track(...) to all public functions and critical private functions, with a blank line after the call and package-prefixed name (e.g., "exec.ProcessComponent"). Use atmosConfig if available, otherwise nil.
Wrap all errors using static errors defined in errors/errors.go; never return dynamic errors directly.
Use the error wrapping pattern: fmt.Errorf("%w: details", errUtils.ErrStaticError, ...) with %w to preserve error chains and add context after the static error.
Bind environment variables with viper.BindEnv() and provide ATMOS_ alternatives for every env var (e.g., viper.BindEnv("ATMOS_GITHUB_TOKEN", "GITHUB_TOKEN")).
Ensure cross-platform compatibility: prefer SDKs over invoking binaries, use filepath.Join, os.PathSeparator/PathListSeparator, and runtime.GOOS when necessary.
Files:
internal/exec/describe_affected_test.go
internal/exec/**/*_test.go
📄 CodeRabbit inference engine (CLAUDE.md)
Add comprehensive tests for new template functions in internal/exec/.
Files:
internal/exec/describe_affected_test.go
{cmd,internal,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
{cmd,internal,pkg}/**/*.go: Do not use logging for UI output. Send prompts/status/progress and actionable errors to stderr; send data/results to stdout; keep logging for system/debug with structured fields.
Most text UI must go to stderr; only data/results go to stdout. Prefer utils.PrintfMessageToTUI for UI messages.
For non-standard execution paths, capture telemetry using telemetry.CaptureCmd(...) or telemetry.CaptureCmdString(...); never capture user data.
Files:
internal/exec/describe_affected_test.go
{internal/exec,pkg}/**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
All new configs must support Go templating using FuncMap() from internal/exec/template_funcs.go.
Files:
internal/exec/describe_affected_test.go
🧠 Learnings (2)
📚 Learning: 2024-10-20T00:57:53.500Z
Learnt from: haitham911
PR: cloudposse/atmos#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/describe_affected_test.go
📚 Learning: 2025-08-16T23:33:07.477Z
Learnt from: aknysh
PR: cloudposse/atmos#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:
internal/exec/describe_affected_test.go
🧬 Code graph analysis (1)
internal/exec/describe_affected_test.go (4)
pkg/schema/schema.go (5)
AtmosConfiguration(27-63)Affected(608-627)ConfigAndStacksInfo(457-534)Dependent(658-673)Settings(679-683)internal/exec/describe_affected.go (1)
DescribeAffectedCmdArgs(28-49)pkg/config/config.go (1)
InitCliConfig(25-62)internal/exec/describe_affected_helpers.go (1)
ExecuteDescribeAffectedWithTargetRepoPath(352-412)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Run pre-commit hooks
- GitHub Check: website-deploy-preview
- GitHub Check: Summary
🔇 Additional comments (2)
internal/exec/describe_affected_test.go (2)
158-220: LGTM!The new
setupDescribeAffectedTesthelper effectively eliminates duplication across test functions and properly usest.Helper()andt.Cleanup.
222-1091: LGTM!The new and updated test functions provide comprehensive coverage of the describe affected functionality:
- Template and function processing (on/off)
- Locked component exclusion
- Dependent processing with stack filtering
- Disabled dependent exclusion
The nil vs empty slice semantics for the
Dependentsfield is correctly implemented per the PR objectives.
|
These changes were released in v1.194.0. |
2 participants
what
atmos describe stackscommandatmos describe affected --include-dependentsresultsatmos describe affected --stack <stack> --include-dependentswhy
1.
atmos describe stacks- Enable Template/Function Processing by DefaultThe documentation states that
atmos describe stacksprocesses templates and YAML functions by default, but the code was doing the opposite. This change aligns the implementation with the documentation and provides consistency with otheratmos describecommands.Users can still disable processing with:
--process-templates=false--process-functions=false2.
atmos describe affected --include-dependents- Honormetadata.enabled: falseWhen using
--include-dependents, disabled dependent components (withmetadata.enabled: false) should not be included in the dependents list for each affected component. This prevents showing components that are intentionally disabled from being part of the dependency chain.3.
atmos describe affected --stack <stack> --include-dependents- Filter Dependents by StackWhen using both
--stack <stack>and--include-dependentsflags together, the provided stack filter should apply to:This ensures that when filtering by a specific stack, you only see the dependency relationships within that stack, making it easier to understand stack-specific impacts.
Example:
# Show affected components in ue1-network and their dependents (also in ue1-network only) atmos describe affected --stack ue1-network --include-dependentsTest Coverage
Added comprehensive test coverage for all describe affected scenarios:
Summary by CodeRabbit