feat: Enhance source list commands with optional filters#1996
Merged
Conversation
…ack support - Make --stack flag optional for both `atmos terraform source list` and `atmos list sources` - when omitted, lists sources across ALL stacks - Add optional [component] positional argument to filter by component name or folder (follows Atmos convention) - Add dynamic Folder column shown only when metadata.component differs from instance name - Add Type column to `atmos list sources` for multi-type visibility - Refactor to reduce cognitive/cyclomatic complexity with helper functions - Add constants for map keys to pass linting - Add comprehensive tests for new functionality - Update PRD documentation with new command variants - Add blog post documenting the feature Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
…tern - Detect "failed to find import" errors and wrap with ErrNoStacksFound - Add clear explanation: "No stack configuration files were found matching the configured import patterns" - Add actionable hint: "Ensure your stacks directory contains valid YAML files and check the 'stacks' settings in atmos.yaml" - Apply to both `atmos terraform source list` and `atmos list sources` Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds a unified Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as Atmos CLI
participant Config as Config Loader
participant Stacks as Stacks Resolver
participant Extractor as Source Extractor
participant Renderer as Output Renderer
User->>CLI: atmos list sources [component] [--stack stack]
CLI->>Config: Initialize Atmos configuration / auth
Config-->>CLI: AtmosConfiguration
CLI->>Stacks: Fetch stack data (single or all) via describe
Stacks-->>CLI: Stack definitions
CLI->>Extractor: Extract sources from stack(s)
Extractor->>Extractor: Traverse Terraform/Helmfile/Packer components
Extractor->>Extractor: Extract URI, Version, Type, Folder
Extractor-->>CLI: Aggregated sources list
CLI->>Extractor: Apply component/stack filters (if provided)
Extractor-->>CLI: Filtered sources
CLI->>Renderer: Determine columns & sort
Renderer->>Renderer: Render in requested format (table/json/yaml/csv)
Renderer-->>User: Display formatted output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
… paths - Add markdown formatting with backticks for code references - Include link to atmos.tools documentation for configuration help - Extract and display searched paths from import errors for debugging - Apply consistent error messaging across both list commands Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change from non-existent /core-concepts/stacks to valid /learn/stacks path. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cmd/list/sources.go`:
- Around line 304-314: The comparator in sortSourcesByTypeComponent uses
unchecked assertions like sources[i][keyType].(string) and
sources[i][keyComponent].(string), which can panic if values aren't strings;
update the comparator to defensively extract values (use the comma-ok form or a
type switch) and fall back to a safe string representation (e.g., empty string
or fmt.Sprint(value)) when the assertion fails, ensuring comparisons use those
safe strings for typeI/typeJ and component values so the sort never panics.
In `@pkg/provisioner/source/cmd/list.go`:
- Around line 361-364: The sort comparator currently uses direct type assertions
like sources[i][keyComponent].(string) which can panic; update the sort.Slice
comparators to perform safe type assertions using the ok idiom (e.g., v, _ :=
sources[i][keyComponent].(string) and v2, _ :=
sources[j][keyComponent].(string)) and fall back to "" when the value is missing
or not a string, and apply the same defensive change to the other comparator
block (the later sort.Slice at lines ~439-448) so both comparators use safe
extraction from the sources map before comparing.
🧹 Nitpick comments (3)
pkg/provisioner/source/cmd/list.go (1)
137-177: Error wrapping is helpful, but string matching is fragile.The user-friendly hints are valuable. Note that matching on error message substrings (lines 142-143, 160-161) could break if upstream error messages change. Consider defining error sentinels upstream if this becomes a maintenance burden.
cmd/list/sources.go (2)
344-350: Component types are hardcoded.The list
terraform,helmfile,packercovers current Atmos component types. If new types are added, this will need updating. Consider extracting to a shared constant if this pattern appears elsewhere.
448-500: Consider extracting shared error wrapping utilities.
wrapConfigErrorandextractSearchedPathinpkg/provisioner/source/cmd/list.goare identical towrapSourcesConfigErrorandextractSourcesSearchedPathincmd/list/sources.go. These generic utilities would benefit from consolidation in a shared error handling package to reduce duplication.
…tern
- Use error builder pattern at source (glob files) with WithContext("paths", ...)
- Remove brittle string parsing from wrapConfigError functions
- All commands that load stacks now benefit from cleaner error messages
- Context is available in verbose mode for debugging
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- WithCause now extracts and preserves context from cause errors - Formatter uses GetAllSafeDetails to traverse entire error chain - DefaultFormatterConfig checks viper for --verbose flag - Context now displayed in verbose mode (--verbose flag) This allows structured error context (like searched paths) to be visible when debugging with --verbose, while keeping normal output clean. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Address CodeRabbit review feedback by converting direct type assertions to the comma-ok idiom in all sort functions. This prevents potential panics if map values are nil or of unexpected types. Changed functions: - sortSourcesByTypeComponent (cmd/list/sources.go) - sortSourcesByStackTypeComponent (cmd/list/sources.go) - sortSourcesByStackComponent (pkg/provisioner/source/cmd/list.go) - extractSourcesFromStack inner sort (pkg/provisioner/source/cmd/list.go) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move version from URI query parameter (?ref=X.X.X) to the dedicated version field in source configuration. This demonstrates the proper way to configure source versions and ensures the Version column displays correctly in source list output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update source provisioner messages to: - Include version when available (e.g., myapp@0.25.0) - Use markdown backticks for code formatting in messages Messages now render with proper inline code styling: - "Vendored `myapp@0.25.0` to `.workdir/terraform/dev-myapp`" - "Vendoring `myapp@0.25.0` from `github.com/...`" Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The spinner progress message was showing raw markdown (literal backticks) instead of rendering them as styled inline code. The completion message rendered correctly because it used FormatSuccess/FormatError which call toastMarkdown(). Changes: - Add inline stylesheet functions to pkg/ui/theme/converter.go - Add renderToastMarkdownFromStylesheet shared helper to reduce duplication - Add FormatInline() for single-line markdown rendering without newlines - Update all three spinner View() functions to use ui.FormatInline() - Fix TestSourceProvisionerList to expect success (command is now implemented) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add comprehensive unit tests for the new inline stylesheet functions introduced in the spinner progress message fix: - TestConvertToGlamourStyleInline: Verifies inline style conversion - TestCreateGlamourStyleInlineFromTheme: Tests style properties - TestGetGlamourStyleForInline: Tests theme retrieval with fallback - TestFormatInline: Tests backtick rendering and no trailing newlines - TestFormatInline_FallbackBehavior: Tests graceful fallback Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmd/list/sources.go`:
- Around line 137-152: The ConfigAndStacksInfo is built only with Stack while
global flags in opts.Flags are ignored; replace the manual creation of
configInfo with a call to buildConfigAndStacksInfo using opts.Flags and
opts.Stack (i.e., call buildConfigAndStacksInfo(opts.Flags, opts.Stack) and pass
its result to initCliConfigForSources) so initCliConfigForSources receives the
global selection flags; update any variable names accordingly (configInfo,
atmosConfig, opts.AtmosConfig remain the same) and keep existing error handling
around initCliConfigForSources.
🧹 Nitpick comments (6)
pkg/ui/theme/converter.go (2)
264-273: Missingperf.Trackcall per coding guidelines.Public functions should include performance tracking. Consider adding:
func GetGlamourStyleForInline(themeName string) ([]byte, error) { defer perf.Track(nil, "theme.GetGlamourStyleForInline")() // ... }Same applies to
ConvertToGlamourStyleInlineat line 277.
407-416: CodeBlock Chroma style may be unnecessary for inline rendering.For inline rendering (single-line output), code blocks with syntax highlighting seem unlikely to be used. The
Chromafield could be set tonilto reduce overhead.That said, this is a minor point and doesn't affect correctness.
pkg/ui/formatter.go (1)
452-464: Clean implementation with proper fallback behavior.
FormatInlinegracefully returns the original text when the formatter isn't initialized or rendering fails. This prevents UI disruptions.Per coding guidelines, consider adding perf tracking:
func FormatInline(text string) string { defer perf.Track(nil, "ui.FormatInline")() // ... }pkg/provisioner/source/cmd/list.go (2)
138-171: String-based error detection is fragile.Matching error messages works but breaks if upstream wording changes. Consider using
errors.Is()with sentinel errors when available. For now, this provides helpful UX, so acceptable.
269-277: Direct interface comparison.
s[keyComponent] != s[keyFolder]comparesanyvalues. Works since both are strings internally, but explicit assertion would be clearer.♻️ Optional: explicit string comparison
func checkFolderDiffers(sources []map[string]any) bool { for _, s := range sources { - if s[keyComponent] != s[keyFolder] { + comp, _ := s[keyComponent].(string) + folder, _ := s[keyFolder].(string) + if comp != folder { return true } } return false }cmd/list/sources.go (1)
452-486: String-based error detection is fragile but pragmatic.The pattern matching on error messages provides helpful context but could break if underlying error messages change. Consider documenting the expected error patterns or using typed errors in the future.
That said, the fallback to a generic error ensures nothing breaks if patterns don't match.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1996 +/- ##
==========================================
+ Coverage 75.15% 75.19% +0.03%
==========================================
Files 784 785 +1
Lines 71802 72455 +653
==========================================
+ Hits 53965 54482 +517
- Misses 14359 14478 +119
- Partials 3478 3495 +17
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
- Create new documentation for `atmos list sources` unified view command
- Update terraform/helmfile/packer source list docs:
- Remove outdated "not implemented" notes (commands ARE implemented)
- Fix --stack flag as optional (not required)
- Add optional [component] positional argument documentation
- Update examples to show flexible filtering options
- Add cross-reference to unified `atmos list sources` command
- Fix "alias" terminology to "unified view" in PRD and blog
- `atmos list sources` is NOT an alias, it's a distinct command that
shows all component types with a Type column
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ATMOS_FORCE_TTY was causing ANSI escape codes to leak into golden snapshots because it makes the app think it's running in a TTY environment with color support. Replace with COLUMNS=120 which provides consistent terminal width without enabling colors. The non-TTY detection (pipe-based execution in tests) correctly auto-disables colors, so NO_COLOR is not needed. This produces clean, readable snapshots without masking potential auto-downgrade bugs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add exponential backoff retry (3 attempts with 1s, 2s, 4s delays) to handle transient network timeouts when fetching the latest release from GitHub API during website builds. This fixes CI failures caused by intermittent network issues. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cmd/list/sources.go`:
- Around line 125-162: In initSourcesCommand, replace direct returns of raw
errors from checkAtmosConfig and sourcesParser.BindFlagsToViper by wrapping them
with the static error type (errUtils.ErrInvalidConfig) defined in
errors/errors.go; specifically, when checkAtmosConfig(...) or
sourcesParser.BindFlagsToViper(...) returns an err, return nil and a wrapped
error using errUtils.ErrInvalidConfig (preserving the original err as context)
instead of returning err directly so the initSourcesCommand error outputs comply
with the static error type policy.
In `@website/docs/cli/commands/list/list-sources.mdx`:
- Around line 123-128: Update the argument description for `component` to state
that filtering matches either the component name or the folder value stored in
metadata (metadata.component); edit the text under "Arguments" where `component`
is documented to say "Filter results to a specific component name or folder
(matches metadata.component). When provided, only shows sources for components
matching this name or folder across all stacks." to reflect the actual
implementation.
- Around line 72-94: The Helmfile example URIs are incorrect: replace the two
occurrences of "github.com/cloudposse/helmfiles//releases/nginx" with the
correct Helmfile-components chart path (e.g.,
"github.com/cloudposse/helmfile-components//charts/nginx") in the Example output
block and the filtered "List Sources in a Specific Stack" output so both the
full table and the filtered table use the consistent
"helmfile-components//charts/nginx" URI.
🧹 Nitpick comments (1)
website/blog/2026-01-18-source-list-command.mdx (1)
36-69: Clarify which command the dynamic column examples refer to. Otherwise readers may expectatmos list sourcesoutput without the Type column.✏️ Suggested tweak
-## Dynamic Output Columns +## Dynamic Output Columns + +Examples below use the type-specific command (`atmos terraform source list`). The unified command (`atmos list sources`) always includes the Type column.
- Wrap checkAtmosConfig and BindFlagsToViper errors with errUtils.ErrInvalidConfig to comply with static error type policy - Fix Helmfile example URI in docs (helmfiles//releases -> helmfile-components//charts) - Update component argument description to mention folder matching - Fix lint: add period to vendorCmd comment - Fix lint: check RegisterFlagCompletionFunc return value Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
|
💥 This pull request now has conflicts. Could you fix it @osterman? 🙏 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cmd/list/sources_test.go`:
- Around line 37-59: The test TestSourcesCommandFlags currently builds a
synthetic cobra.Command so it won't catch regressions on the real command;
change the test to get the real command instance (use the existing sourcesCmd or
the constructor that returns it) and assert flags on that object instead of the
locally created cmd: locate TestSourcesCommandFlags and replace use of the
inline &cobra.Command{...} with a call/reference to sourcesCmd (or
NewSourcesCommand()) and then Lookup("format") and Lookup("stack") on that real
command to validate DefValue and existence.
In `@pkg/provisioner/source/source_test.go`:
- Around line 546-559: The test uses a hardcoded POSIX-style absolute path which
breaks on Windows; update TestBuildComponentPath_AbsolutePath to construct the
expected absolute path using OS-aware utilities (e.g., call filepath.FromSlash
or build it with filepath.Join) instead of a literal string, and ensure you
reference the Terraform BasePath field on the atmosConfig passed to
buildComponentPath so the assertion compares the OS-normalized expected path to
the result from buildComponentPath.
- Around line 535-544: The test TestDetermineTargetDirectory_PreResolvedAbsPath
hardcodes the expected path with forward slashes which will fail on Windows;
update the assertion to build the expected path using filepath.Join (e.g.,
filepath.Join(atmosConfig.TerraformDirAbsolutePath, "vpc")) so it matches how
DetermineTargetDirectory constructs paths cross-platform.
🧹 Nitpick comments (2)
pkg/ui/formatter.go (1)
452-464: Missingperf.Trackin public function.Per coding guidelines, public functions should include performance tracking. Consider adding:
defer perf.Track(nil, "ui.FormatInline")()That said, this is a format helper likely called frequently in hot paths. If perf tracking would add overhead for spinner messages, it may be acceptable to skip here.
pkg/provisioner/source/source_test.go (1)
438-450: Consider consolidating into the existing table-driven test.This test could be added as another case in
TestGetComponentBasePath(lines 209-266) for consistency. Same applies toTestDetermineTargetDirectory_Packerbelow.♻️ Suggested consolidation
Add to the existing table in
TestGetComponentBasePath:{ name: "packer component type", atmosConfig: &schema.AtmosConfiguration{ Components: schema.Components{ Packer: schema.Packer{ BasePath: "components/packer", }, }, }, componentType: "packer", expected: "components/packer", },
aknysh
approved these changes
Jan 21, 2026
|
These changes were released in v1.204.1-rc.4. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
what
--stackflag optional foratmos terraform source listandatmos list sourcescommands[component]positional argument to filter by component name or folderFoldercolumn that appears only when component folder differs from instance nameTypecolumn toatmos list sourcesfor multi-type visibilitywhy
--stackmetadata.componentreferences
Implements enhancements outlined in the source provisioner PRD for comprehensive source listing capabilities.
Summary by CodeRabbit
New Features
atmos list sourcesto list component sources across Terraform, Helmfile, and Packer with optional component/stack filtering and multiple output formats.Documentation
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.