Conversation
📝 WalkthroughWalkthroughThis pull request introduces several new CLI commands and associated functionality to list component values, variables, metadata, settings, and workflows across multiple stacks. It adds new functions for filtering, formatting, and flag handling, as well as comprehensive unit tests and updated documentation. The control flow now includes enhanced processing of flags, error handling with custom error types, and dynamic output formatting in various formats such as table, JSON, YAML, CSV, and TSV. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Cobra CLI (listValuesCmd)
participant F as listValues Function
participant L as FilterAndListValues
participant P as Formatter (e.g., TableFormatter)
participant O as CLI Output
U->>C: Run "atmos list values <component>" or "atmos list vars <component>"
C->>C: Validate arguments and retrieve flags
C->>F: Call listValues with component and flags
F->>L: Process filtering and apply query (e.g., ".vars" for alias)
L->>F: Return filtered component values or error
F->>P: Format the output based on specified format option
P->>F: Return formatted output
F->>C: Return final output or error status
C->>O: Display output to user
Assessment against linked issues
Possibly related issues
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (11)
pkg/list/list_workflows.go (1)
202-230: Simplify the default case logic.The default case handles both empty format and "table" format with duplicated string joining logic. This can be simplified to reduce code duplication.
Consider this refactor:
default: + var output strings.Builder + output.WriteString(strings.Join(header, delimiter) + utils.GetLineEnding()) + for _, row := range rows { + output.WriteString(strings.Join(row, delimiter) + utils.GetLineEnding()) + } + // If format is empty or "table", use table format if format == "" && term.IsTTYSupportForStdout() { // Create a styled table for TTY t := table.New(). // ... existing table configuration ... Headers(header...). Rows(rows...) return t.String() + utils.GetLineEnding(), nil } - // Default to simple tabular format for non-TTY or when format is explicitly "table" - var output strings.Builder - output.WriteString(strings.Join(header, delimiter) + utils.GetLineEnding()) - for _, row := range rows { - output.WriteString(strings.Join(row, delimiter) + utils.GetLineEnding()) - } return output.String(), nilpkg/list/list_values.go (3)
23-31: Consider removing the unused function.The static analysis tool confirms that
getMapKeysis unused. If there's no future usage planned, removing it will reduce clutter.-// getMapKeys returns a sorted slice of map keys -func getMapKeys(m map[string]interface{}) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - return keys -}🧰 Tools
🪛 golangci-lint (1.62.2)
24-24: func
getMapKeysis unused(unused)
34-324: Refactor the large function.
FilterAndListValueshandles multiple steps: filtering stacks, applying queries, formatting output, etc. Splitting it into smaller helper functions would improve readability and maintainability.
265-272: Handle CSV field quoting.Concatenating fields with a plain delimiter can corrupt CSV data if the field itself contains the delimiter or newline. Consider quoting fields for robust CSV output.
pkg/list/list_values_test.go (1)
56-148: Expand test coverage.You might test nested or array-based query paths to confirm filtering works for complex data structures. It would further validate the code’s resilience.
cmd/list_values.go (2)
35-63: Avoid repeated flag error-handling.Error-handling for each flag is repeated. Consolidating this into a small helper would reduce duplication and simplify reading.
95-116: Evaluate independent command structure.
listVarsCmdsets a single flag before reusinglistValuesCmd.Run. This works but might complicate future expansions. Consider a dedicated or unified approach for better clarity.website/docs/cli/commands/list/list-values.mdx (4)
31-42: Consider enhancing flag documentation with example values.While the flag descriptions are clear, adding example values would make them more user-friendly.
<dt><code>--query string</code></dt> - <dd>JMESPath query to filter values (e.g., ".vars" to show only variables)</dd> + <dd>JMESPath query to filter values (e.g., ".vars" to show only variables, ".config.vpc" to show VPC configuration)</dd> <dt><code>--format string</code></dt> - <dd>Output format: `table`, `json`, `csv`, `tsv` (default "`table`")</dd> + <dd>Output format: `table`, `json`, `csv`, `tsv` (default "`table`"). Example: --format=json</dd>
58-58: Replace TODO placeholder with actual JMESPath query examples.The placeholder needs to be replaced with practical JMESPath query examples to help users understand how to filter values effectively.
Would you like me to help generate some practical JMESPath query examples? Here's a suggestion:
-TODO: define more outputs +# Show only networking configuration +atmos list values vpc --query ".config.networking" + +# Filter for specific environment variables +atmos list values vpc --query ".vars | [?contains(name, 'ENV')]" + +# Show components with specific tags +atmos list values vpc --query ".metadata.tags"
86-86: Add example command output to enhance documentation.Replace the TODO with actual command output to help users understand what to expect.
Would you like me to help generate an example output? Here's a suggestion:
-TODO: define example output +Component: vpc + +┌─────────────┬──────────────┬──────────────┬──────────────┐ +│ Key │ dev-ue1 │ staging-ue1 │ prod-ue1 │ +├─────────────┼──────────────┼──────────────┼──────────────┤ +│ cidr_block │ 10.0.0.0/16 │ 10.1.0.0/16 │ 10.2.0.0/16 │ +│ enable_flow │ true │ true │ true │ +│ max_azs │ 3 │ 3 │ 3 │ +└─────────────┴──────────────┴──────────────┴──────────────┘
91-92: Enhance typography in related commands section.Replace hyphens with em dashes for better readability.
-[atmos list components](/cli/commands/list/component) - List available components -[atmos describe component](/cli/commands/describe/component) - Show detailed information about a component +[atmos list components](/cli/commands/list/component) — List available components +[atmos describe component](/cli/commands/describe/component) — Show detailed information about a component🧰 Tools
🪛 LanguageTool
[typographical] ~91-~91: To join two clauses or introduce examples, consider using an em dash.
Context: ...omponents](/cli/commands/list/component) - List available components - [atmos descr...(DASH_RULE)
[typographical] ~92-~92: To join two clauses or introduce examples, consider using an em dash.
Context: ...onent](/cli/commands/describe/component) - Show detailed information about a compon...(DASH_RULE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/list_values.go(1 hunks)pkg/list/list_values.go(1 hunks)pkg/list/list_values_test.go(1 hunks)pkg/list/list_workflows.go(1 hunks)pkg/list/list_workflows_test.go(7 hunks)website/docs/cli/commands/list/list-values.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/list/list-values.mdx
[typographical] ~91-~91: To join two clauses or introduce examples, consider using an em dash.
Context: ...omponents](/cli/commands/list/component) - List available components - [atmos descr...
(DASH_RULE)
[typographical] ~92-~92: To join two clauses or introduce examples, consider using an em dash.
Context: ...onent](/cli/commands/describe/component) - Show detailed information about a compon...
(DASH_RULE)
🪛 golangci-lint (1.62.2)
pkg/list/list_values.go
24-24: func getMapKeys is unused
(unused)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/list/list_workflows.go (1)
25-25: LGTM! New format constants follow the established pattern.The addition of
FormatYAMLandFormatTSVconstants maintains consistency with the existing format naming convention.Also applies to: 27-27
pkg/list/list_workflows_test.go (1)
76-76: LGTM! File permission notation updated to modern format.The change to use the
0oprefix for octal file permissions is more explicit and follows modern Go conventions.Also applies to: 89-89, 100-100, 118-118, 136-136, 310-310
website/docs/cli/commands/list/list-values.mdx (1)
1-28: Strong work on the command description!The command description effectively communicates the purpose and value proposition of the
atmos list valuescommand. The tabular view explanation is particularly clear and helpful.
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
pkg/list/utils/utils.go (1)
27-69: Component existence checking needs optimization.The function correctly checks if a component exists across stacks, but there are several areas for improvement:
- The implementation only checks for terraform components, ignoring other potential component types
- The code makes multiple type assertions without detailed error handling
- Using
ExecuteDescribeStacksis expensive for just checking component existenceConsider using a more efficient approach by directly accessing component configuration instead of running the describe stacks command, and add support for other component types:
func CheckComponentExists(atmosConfig *schema.AtmosConfiguration, componentName string) bool { if componentName == "" { return false } // Extract component name from path if needed parts := strings.Split(componentName, "/") baseName := parts[len(parts)-1] - // Get all stacks to check for the component - stacksMap, err := e.ExecuteDescribeStacks(*atmosConfig, "", nil, nil, nil, false, false, false, false, nil) - if err != nil { - return false - } - - // Process all stacks to find the component - for _, stackData := range stacksMap { - stackMap, ok := stackData.(map[string]interface{}) - if !ok { - continue - } - - componentsMap, ok := stackMap["components"].(map[string]interface{}) - if !ok { - continue - } - - terraformComponents, ok := componentsMap["terraform"].(map[string]interface{}) - if !ok { - continue - } - - // Check if the component exists in this stack - _, exists := terraformComponents[baseName] - if exists { - return true - } - } + // Get component manifests directly + componentManifests, err := atmosConfig.GetComponentManifests("") + if err != nil { + return false + } + + // Check if component exists in any stack + for _, manifest := range componentManifests { + // Check across different component types (terraform, helmfile, etc.) + for _, componentType := range manifest.ComponentTypes { + components, ok := manifest.Components[componentType] + if !ok { + continue + } + + if _, exists := components[baseName]; exists { + return true + } + } + } return false }Note: This suggested implementation assumes the existence of a
GetComponentManifestsmethod and appropriate structure in theAtmosConfiguration. Adjust according to the actual API available.cmd/list_settings.go (2)
58-69: Consider extracting "settings" as a constant.Defining a constant like
KeySettings = "settings"(similar toKeyMetadata) in a shared package improves consistency and future maintainability.-func setupSettingsOptions(commonFlags fl.CommonFlags, componentFilter string) *l.FilterOptions { - return &l.FilterOptions{ - Component: "settings", +const KeySettings = "settings" +func setupSettingsOptions(commonFlags fl.CommonFlags, componentFilter string) *l.FilterOptions { + return &l.FilterOptions{ + Component: KeySettings, ComponentFilter: componentFilter, // ... } }
88-93: Align error handling with similar commands.Here,
&listerrors.CommonFlagsError{Cause: err}is used, whereas the metadata command uses a different error type for failing to retrieve common flags. For consistency, consider using a uniform error approach across commands.cmd/list_metadata.go (2)
95-101: Standardize error types for common flags retrieval.Here,
QueryErroris used to wrap errors fromfl.GetCommonListFlags. Inlist_settings.go, a different error type is used for the same scenario. Adopting one uniform error type simplifies debugging.
155-161: Consider logging the exact query.When logging “No metadata found,” including the resolved query in the message can help troubleshoot filtered outputs.
if errors.As(err, &noValuesErr) { - logNoMetadataFoundMessage(params.ComponentFilter) + log.Info(fmt.Sprintf("No metadata found for component '%s' with query '%s'", params.ComponentFilter, params.CommonFlags.Query)) return "", nil }cmd/list_values.go (3)
57-59: Remove redundant argument check in RunE.
cobra.ExactArgs(1)already ensures that exactly one argument is provided. The explicit check here duplicates that behavior.- if len(args) != 1 { - return ErrInvalidArguments - }
273-275: Remove unreachable error scenario.Because
Args: cobra.ExactArgs(1)is enforced, this condition won't occur at runtime. Streamline the code by removing it.-func listValues(cmd *cobra.Command, args []string) (string, error) { - if len(args) == 0 { - return "", ErrComponentNameRequired - } +func listValues(cmd *cobra.Command, args []string) (string, error) {
248-269: Refactor repeated stack initialization logic.This logic mirrors similar commands (
listSettings,listMetadata). Extracting a shared helper function for "init config + describe stacks + check component" could DRY the code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
cmd/list_errors_test.go(1 hunks)cmd/list_metadata.go(1 hunks)cmd/list_settings.go(1 hunks)cmd/list_values.go(1 hunks)pkg/list/utils/utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/list_errors_test.go
🧰 Additional context used
🧠 Learnings (1)
cmd/list_settings.go (2)
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2025-03-27T21:06:28.283Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-03-27T21:06:28.284Z
Learning: In the Atmos project, -1 is used as a special value to denote unlimited columns in table outputs, following common software conventions.
🧬 Code Definitions (3)
pkg/list/utils/utils.go (1)
pkg/list/errors/types.go (1)
NoValuesFoundError(9-12)
cmd/list_settings.go (7)
pkg/list/flags.go (2)
AddCommonListFlags(21-27)GetCommonListFlags(30-76)cmd/cmd_utils.go (1)
AddStackCompletion(715-720)pkg/list/list_values.go (2)
FilterOptions(55-64)FilterAndListValues(67-97)pkg/list/format/formatter.go (2)
Format(8-8)FormatCSV(14-14)pkg/list/errors/types.go (5)
CommonFlagsError(82-84)InitConfigError(95-97)ComponentDefinitionNotFoundError(183-185)DescribeStacksError(108-110)NoValuesFoundError(9-12)pkg/list/format/delimited.go (2)
DefaultTSVDelimiter(14-14)DefaultCSVDelimiter(13-13)pkg/list/utils/utils.go (2)
CheckComponentExists(29-69)IsEmptyTable(18-25)
cmd/list_metadata.go (7)
pkg/list/flags.go (2)
AddCommonListFlags(21-27)GetCommonListFlags(30-76)cmd/cmd_utils.go (1)
AddStackCompletion(715-720)pkg/list/list_values.go (3)
FilterOptions(55-64)KeyMetadata(29-29)FilterAndListValues(67-97)pkg/list/format/formatter.go (2)
Format(8-8)FormatCSV(14-14)pkg/list/errors/types.go (5)
QueryError(32-35)InitConfigError(95-97)ComponentDefinitionNotFoundError(183-185)DescribeStacksError(108-110)NoValuesFoundError(9-12)pkg/list/format/delimited.go (1)
DefaultCSVDelimiter(13-13)pkg/list/utils/utils.go (2)
CheckComponentExists(29-69)IsEmptyTable(18-25)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Summary
🔇 Additional comments (2)
pkg/list/utils/utils.go (2)
11-15: Clear and effective error type checking function.The function correctly identifies
NoValuesFoundErrortypes using a type assertion. The naming and documentation also clearly communicate the purpose.
17-25: Well-implemented table emptiness check.The function correctly handles both empty strings and tables with only headers (4 or fewer newlines). The direct return statement at line 24 follows the code scanning recommendation to simplify conditional return statements.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
website/docs/cli/commands/list/list-values.mdx (2)
112-136: Helpful Nested Object Display Instructions
The section on handling nested objects is clear, providing both guidance and an example JSON output.Consider using a typographical ellipsis (… instead of three dots) in placeholders (e.g.,
{...}) if it aligns with your documentation style guidelines.🧰 Tools
🪛 LanguageTool
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ... table format, nested objects appear as{...}placeholders 2. Use--format jsono...(ELLIPSIS)
137-141: Related Commands Enhance Navigation
Listing related commands is beneficial for users looking to explore further functionalities.For improved readability, consider using an em dash (—) instead of a hyphen to join the command link with its description. For example:
[atmos list components](/cli/commands/list/components) — List available components[atmos describe component](/cli/commands/describe/component) — Show detailed information about a component🧰 Tools
🪛 LanguageTool
[typographical] ~139-~139: To join two clauses or introduce examples, consider using an em dash.
Context: ...mponents](/cli/commands/list/components) - List available components - [atmos descr...(DASH_RULE)
[typographical] ~140-~140: To join two clauses or introduce examples, consider using an em dash.
Context: ...onent](/cli/commands/describe/component) - Show detailed information about a compon...(DASH_RULE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
pkg/list/flags/flags.go(1 hunks)website/docs/cli/commands/list/list-values.mdx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/list/flags/flags.go
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/list/list-values.mdx
[style] ~116-~116: Consider using the typographical ellipsis character here instead.
Context: ... table format, nested objects appear as {...} placeholders 2. Use --format json o...
(ELLIPSIS)
[typographical] ~139-~139: To join two clauses or introduce examples, consider using an em dash.
Context: ...mponents](/cli/commands/list/components) - List available components - [atmos descr...
(DASH_RULE)
[typographical] ~140-~140: To join two clauses or introduce examples, consider using an em dash.
Context: ...onent](/cli/commands/describe/component) - Show detailed information about a compon...
(DASH_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (12)
website/docs/cli/commands/list/list-values.mdx (12)
1-7: Front Matter Metadata Looks Great
The front matter is well-structured with a clear title, id, sidebar label, and sidebar class name, which will help with navigation and organization in the docs.
8-11: Clear Title and Introduction
The title and introductory description effectively convey that the command displays component values across all stacks, setting the right expectations from the start.
12-17: Usage Section is Concise and Clear
The usage section provides a straightforward command syntax in a shell code block. This clarity helps users quickly understand how to invoke the command.
18-25: Detailed Command Description
The description elaborates on the tabular view, explicating that each column represents a stack and each row represents a configuration key. This detailed explanation is very helpful for users.
26-30: Well-Articulated Use Cases
The list of benefits—comparing configurations across environments, verifying values, and understanding component setups—provides clear guidance on when and why to use the command.
31-45: Comprehensive Flags Section
The flags are well-documented using<dl>,<dt>, and<dd>elements. Each flag has a clear explanation of its purpose, making it easy for users to understand the available options.
46-57: Informative Examples Section
The examples provided (e.g., listing all values for a component) are clear and practical. They illustrate the basic usage effectively, which is great for user comprehension.
58-69: Effective Custom Query Examples
The examples showing how to filter values using the--queryflag are well chosen. Demonstrating queries for specific variables, environment settings, and network configurations illustrates the flexibility of the command very nicely.
70-74: Clear Example for Including Abstract Components
The example demonstrating the use of the--abstractflag is concise and easy to understand. It clearly shows how to include abstract components in the output.
75-79: Well-Presented Column Limitation Example
The example using the--max-columnsflag is straightforward. It clearly communicates how to restrict the output to a specified number of columns.
80-94: Diverse Output Format Examples
The examples for outputting in JSON, CSV, and TSV formats are comprehensive. The additional note regarding terminal width issues for table format is a valuable reminder for users handling wide datasets.
95-111: Clear and Detailed Example Output
The provided example output is detailed and visually represents the expected layout of the command’s results. This aids users in understanding the format and structure of the output.
|
These changes were released in v1.170.0. |
4 participants
What
atmos list valuesCLI command to list component values across stacksatmos list metadataCLI command to list component metadata across stacksatmos list settingsCLI command to list component settings across stacksWhy
Examples
atmos list values
atmos list metadata
atmos list settings
Evidences:
Evidence Updated at 2025-03-17
Evidence Updated at 2025-02-24
Table too wide
references
Summary by CodeRabbit
New Features
Documentation
Tests