Skip to content

Implement: atmos list values#1036

Merged
aknysh merged 182 commits intomainfrom
DEV-2802
Apr 1, 2025
Merged

Implement: atmos list values#1036
aknysh merged 182 commits intomainfrom
DEV-2802

Conversation

@Cerebrovinny
Copy link
Collaborator

@Cerebrovinny Cerebrovinny commented Feb 7, 2025

What

  • Add atmos list values CLI command to list component values across stacks
  • Add atmos list metadata CLI command to list component metadata across stacks
  • Add atmos list settings CLI command to list component settings across stacks
  • Update docs
  • Add unit tests

Why

  • Query and filter component configurations across multiple stacks using YQ expressions
  • Compare values, metadata, and settings across different environments without having to manually inspect stack files
  • Output results in various formats (table, JSON, YAML, CSV, TSV) for easy integration with other tools
  • Before this, users had to manually check each stack file to compare configurations

Examples

atmos list values

# List all values for a component
atmos list values vpc

# Filter with YQ query
atmos list values vpc --query .vars

# Get specific field across stacks
atmos list values vpc --query .vars.region

# Match specific stacks
atmos list values vpc --stack '*-prod-*'

# Filter specific stacks with query
atmos list values vpc --query .vars.tags --stack '*-ue2-*'

# Include abstract components
atmos list values vpc --abstract

# Output in different formats
atmos list values vpc --format json
atmos list values vpc --format yaml
atmos list values vpc --format csv
atmos list values vpc --format tsv

# Limit number of columns
atmos list values vpc --max-columns 3

atmos list metadata

# List all metadata 
atmos list metadata

# Filter with YQ query
atmos list metadata --query .type

# Get specific metadata field
atmos list metadata --query .inherits

# Match specific stacks
atmos list metadata --stack 'dev-*'

# Filter specific stacks with query
atmos list metadata --query .type --stack '*-ue2-*'

# Output as JSON
atmos list metadata --format json

atmos list settings

# List all settings
atmos list settings

# Filter with YQ query
atmos list settings --query .terraform

# Get specific settings field
atmos list settings --query .terraform.backend

# Match specific stacks
atmos list settings --stack 'staging-*'

# Filter specific stacks with query
atmos list settings --query .terraform.backend --stack '*-prod-*'

# Output as YAML
atmos list settings --format yaml

Evidences:

Evidence Updated at 2025-03-17
Screenshot 2025-03-17 at 07 44 51
Screenshot 2025-03-17 at 07 45 25
Screenshot 2025-03-17 at 07 45 46
Screenshot 2025-03-17 at 07 46 17
Screenshot 2025-03-17 at 07 46 33

Evidence Updated at 2025-02-24
CleanShot 2025-02-24 at 09 11 01
Screenshot 2025-02-24 at 09 11 21

Screenshot 2025-02-24 at 09 11 50

Screenshot 2025-02-24 at 09 12 00

Table too wide
CleanShot 2025-02-24 at 23 12 57

CleanShot 2025-02-24 at 23 12 18

references

Summary by CodeRabbit

  • New Features

    • Introduced new CLI commands for listing component values, variables, metadata, settings, and workflows.
    • Enhanced filtering capabilities including glob patterns and support for various output formats (table, JSON, YAML, CSV, TSV).
    • Added options to include abstract components and to disable specific processing features.
  • Documentation

    • Updated and expanded usage guides with comprehensive examples for new commands and flags, including detailed descriptions of functionality and usage.
  • Tests

    • Extended test coverage to ensure robust flag handling and error reporting for the newly added functionalities, including validation of command arguments and error scenarios.

@Cerebrovinny Cerebrovinny marked this pull request as ready for review February 14, 2025 10:44
@Cerebrovinny Cerebrovinny requested a review from a team as a code owner February 14, 2025 10:44
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 14, 2025

📝 Walkthrough

Walkthrough

This 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

File(s) Change Summary
cmd/list_values.go, cmd/list_metadata.go, cmd/list_settings.go Introduced new CLI commands (listValuesCmd, listVarsCmd, listMetadataCmd, listSettingsCmd) with flag initialization, alias (e.g. list vars for list values --query .vars), and error handling.
pkg/list/list_values.go, pkg/list/list_workflows.go, pkg/list/errors/types.go Added functions for filtering and listing component values, updated output format validation, and implemented various custom error types for improved error reporting.
pkg/list/flags.go, pkg/list/flags/flags.go Defined common flag structures and retrieval functions for list commands to standardize flag handling across commands.
pkg/list/format/*
(delimited.go, formatter.go, json.go, table.go, yaml.go, formatter_test.go)
Established a formatting package with multiple formatters (table, JSON, YAML, CSV, TSV), including corresponding tests to ensure accurate output formatting.
pkg/utils/*
(wildcard.go, markdown_utils.go, wildcard_test.go)
Added wildcard matching utility and refactored markdown error printing for enhanced testability and output consistency.
cmd/markdown/* and website/docs/cli/commands/list/* Expanded and enhanced documentation for the new CLI commands with examples covering usage, flags, and output formats.
Test files:
pkg/list/list_values_test.go, pkg/list/flags_test.go, cmd/list_values_internal_test.go, cmd/list_errors_test.go
Added unit tests covering filtering logic, flag parsing, argument validation, and error handling for the new list commands.
.golangci.yml Updated linting configuration by removing the gofmt linter and adjusting gocognit exclusion rules.

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
Loading

Assessment against linked issues

Objective Addressed Explanation
Implement the atmos list values command and its alias atmos list vars to list component values across stacks (DEV-2802)

Possibly related issues

  • Ability to List Component Variables #1044: The changes introduce the listVarsCmd command as an alias for listing component variables, aligning directly with the issue's proposal for displaying component variables across stacks.

Suggested labels

minor

Suggested reviewers

  • osterman

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or @auto-summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @auto-title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(), nil
pkg/list/list_values.go (3)

23-31: Consider removing the unused function.

The static analysis tool confirms that getMapKeys is 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 getMapKeys is unused

(unused)


34-324: Refactor the large function.

FilterAndListValues handles 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.

listVarsCmd sets a single flag before reusing listValuesCmd.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

📥 Commits

Reviewing files that changed from the base of the PR and between 5a55781 and 9060a00.

📒 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 FormatYAML and FormatTSV constants 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 0o prefix 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 values command. The tabular view explanation is particularly clear and helpful.

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 15, 2025
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <erik@cloudposse.com>
@mergify mergify bot added triage Needs triage and removed triage Needs triage labels Feb 15, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (16)
cmd/cmd_utils.go (2)

759-761: Return an error or log when componentName is empty

Currently, returning false without context might make debugging difficult. Consider logging or returning an error to inform callers when componentName is empty.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 758-761: cmd/cmd_utils.go#L758-L761
Added lines #L758 - L761 were not covered by tests


768-772: Provide context for DescribeStacks errors

Rather than returning false silently, consider logging or returning the underlying error from ExecuteDescribeStacks to surface potential misconfiguration or retrieval issues.

pkg/list/utils/utils.go (2)

9-13: Consider using errors.As for improved error checks

Directly asserting with err.(*errors.NoValuesFoundError) might be replaced by errors.As(err, &target) for greater flexibility if the error is wrapped in the future.


22-24: Simplify the condition

Use a direct return in the condition:

- if newlineCount <= 4 {
-     return true
- }
- return false
+ return newlineCount <= 4
🧰 Tools
🪛 GitHub Check: golangci-lint

[failure] 22-22:
S1008: should use 'return newlineCount <= 4' instead of 'if newlineCount <= 4 { return true }; return false'

cmd/list_settings.go (4)

57-67: Validate defaulting to “settings” component

setupSettingsOptions hardcodes Component: "settings". If new component types need referencing, ensure this remains correct or configurable.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 57-67: cmd/list_settings.go#L57-L67
Added lines #L57 - L67 were not covered by tests


71-76: Add coverage for logNoSettingsFoundMessage

A small test verifying console output or logging behavior around missing settings would increase confidence in this path.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 71-76: cmd/list_settings.go#L71-L76
Added lines #L71 - L76 were not covered by tests


93-97: Enforce CSV delimiter only if truly CSV

Auto-switching the delimiter is convenient, but be cautious if a user explicitly sets a custom TSV delimiter that matches the default. Document or verify this edge case.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 93-97: cmd/list_settings.go#L93-L97
Added lines #L93 - L97 were not covered by tests


111-134: Validate unified stack retrieval

getStacksMapForSettings replicates logic in other commands. If you intend consistent behavior across commands, consider centralizing stack retrieval to reduce duplication.

cmd/list_metadata.go (3)

56-73: Confirm default .metadata usage

setupMetadataOptions defaults query to .metadata. If a user intends to query deeper metadata paths, ensure no conflicts arise from resetting an empty query.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 57-61: cmd/list_metadata.go#L57-L61
Added lines #L57 - L61 were not covered by tests


[warning] 63-72: cmd/list_metadata.go#L63-L72
Added lines #L63 - L72 were not covered by tests


114-121: Return consistent typed errors

While a QueryError is returned, other code returns wrapped static errors. Ensuring uniform patterns across commands clarifies error handling.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 114-121: cmd/list_metadata.go#L114-L121
Added lines #L114 - L121 were not covered by tests

🪛 GitHub Check: golangci-lint

[failure] 114-114:
function-length: maximum number of lines per function exceeded; max 60 but got 62


171-174: Add coverage to no-output scenario

The final check for empty or minimal table output is untested. Write a quick scenario to confirm correct behavior when no metadata is returned.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 171-174: cmd/list_metadata.go#L171-L174
Added lines #L171 - L174 were not covered by tests

cmd/list_values.go (2)

55-57: Consider using static wrapped errors for improved consistency.

Instead of inline error messages, define static errors to wrap them for clarity.

For example:

-if len(args) != 1 {
-  return fmt.Errorf("invalid arguments: the command requires one argument 'component'")
+if len(args) != 1 {
+  return fmt.Errorf("%w: the command requires one argument 'component'", ErrComponentNameRequired)
}
🧰 Tools
🪛 GitHub Check: golangci-lint

[failure] 56-56:
do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf("invalid arguments: the command requires one argument 'component'")"


205-205: End comment with a period.

For clarity and consistency with the codebase style, add a period at the end of the comment on line 205.

🧰 Tools
🪛 GitHub Check: golangci-lint

[failure] 205-205:
Comment should end in a period

pkg/list/list_values.go (2)

66-97: Consider extracting sub-steps to smaller helper functions.

The FilterAndListValues function’s responsibilities—extracting, filtering, querying, and formatting—could be decomposed into smaller, more specific helpers. This would enhance maintainability.


163-197: Clarify "select" usage in YQ expression for KeySettings.

Using select(.settings // .terraform.settings // .components.terraform.*.settings) can be fragile if additional nested keys are later introduced. Consider a more explicit or parameterized approach.

pkg/list/format/delimited.go (1)

69-77: Remove redundant break statements.

In Go, break after each case is implicit. These breaks can be removed without altering logic, aligning with gosimple S1023.

-    break
...
-    break
🧰 Tools
🪛 golangci-lint (1.64.8)

[error] 69-69: S1023: redundant break statement

(gosimple)


[error] 73-73: S1023: redundant break statement

(gosimple)


[error] 77-77: S1023: redundant break statement

(gosimple)

🪛 GitHub Check: golangci-lint

[failure] 69-69:
S1023: redundant break statement


[failure] 73-73:
S1023: redundant break statement


[failure] 77-77:
S1023: redundant break statement

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c89eb31 and 99cc20f.

📒 Files selected for processing (11)
  • cmd/cmd_utils.go (1 hunks)
  • cmd/list_metadata.go (1 hunks)
  • cmd/list_settings.go (1 hunks)
  • cmd/list_values.go (1 hunks)
  • pkg/list/errors/types.go (1 hunks)
  • pkg/list/format/delimited.go (1 hunks)
  • pkg/list/format/formatter_test.go (1 hunks)
  • pkg/list/format/table.go (1 hunks)
  • pkg/list/list_values.go (1 hunks)
  • pkg/list/list_values_test.go (1 hunks)
  • pkg/list/utils/utils.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/list/list_values_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 (7)
pkg/list/utils/utils.go (1)
pkg/list/errors/types.go (1)
  • NoValuesFoundError (9-12)
pkg/list/format/table.go (2)
pkg/list/list_values.go (1)
  • KeyValue (45-45)
pkg/list/format/formatter.go (5)
  • TableFormatter (37-39)
  • Format (8-8)
  • FormatOptions (19-24)
  • DelimitedFormatter (52-55)
  • FormatCSV (14-14)
cmd/cmd_utils.go (3)
pkg/schema/schema.go (1)
  • AtmosConfiguration (13-45)
internal/exec/describe_stacks.go (1)
  • ExecuteDescribeStacks (148-689)
pkg/describe/describe_stacks.go (1)
  • ExecuteDescribeStacks (9-19)
cmd/list_values.go (6)
pkg/list/errors/types.go (3)
  • ComponentVarsNotFoundError (165-167)
  • NoValuesFoundError (9-12)
  • ComponentDefinitionNotFoundError (183-185)
pkg/list/flags.go (1)
  • AddCommonListFlags (21-27)
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/format/delimited.go (1)
  • DefaultCSVDelimiter (13-13)
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 (5)
  • Format (8-8)
  • FormatJSON (12-12)
  • FormatYAML (13-13)
  • FormatCSV (14-14)
  • FormatTSV (15-15)
pkg/list/errors/types.go (8)
  • ComponentMetadataNotFoundError (174-176)
  • NoValuesFoundError (9-12)
  • NoMetadataFoundError (60-62)
  • MetadataFilteringError (69-71)
  • QueryError (32-35)
  • InitConfigError (95-97)
  • ComponentDefinitionNotFoundError (183-185)
  • DescribeStacksError (108-110)
pkg/list/utils/utils.go (2)
  • IsNoValuesFoundError (10-13)
  • IsEmptyTable (16-27)
pkg/list/format/delimited.go (2)
  • DefaultTSVDelimiter (14-14)
  • DefaultCSVDelimiter (13-13)
pkg/list/list_values.go (3)
pkg/list/list_workflows.go (2)
  • FormatTable (23-23)
  • ValidateFormat (31-42)
pkg/list/format/formatter.go (5)
  • FormatTable (11-11)
  • ValidateFormat (77-88)
  • NewFormatter (58-74)
  • Format (8-8)
  • FormatOptions (19-24)
pkg/list/errors/types.go (3)
  • NoValuesFoundError (9-12)
  • NoComponentSettingsFoundError (143-145)
  • ComponentMetadataNotFoundError (174-176)
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 (1)
  • Format (8-8)
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 (1)
  • IsEmptyTable (16-27)
🪛 GitHub Check: golangci-lint
pkg/list/utils/utils.go

[failure] 22-22:
S1008: should use 'return newlineCount <= 4' instead of 'if newlineCount <= 4 { return true }; return false'

pkg/list/format/table.go

[failure] 57-57:
S1023: redundant break statement

cmd/cmd_utils.go

[failure] 798-798:
file-length-limit: file length is 609 lines, which exceeds the limit of 500

cmd/list_values.go

[failure] 56-56:
do not define dynamic errors, use wrapped static errors instead: "fmt.Errorf("invalid arguments: the command requires one argument 'component'")"


[failure] 205-205:
Comment should end in a period


[failure] 214-214:
function-length: maximum number of lines per function exceeded; max 60 but got 75

cmd/list_metadata.go

[failure] 76-76:
func handleMetadataError is unused


[failure] 114-114:
function-length: maximum number of lines per function exceeded; max 60 but got 62

pkg/list/format/delimited.go

[failure] 69-69:
S1023: redundant break statement


[failure] 73-73:
S1023: redundant break statement


[failure] 77-77:
S1023: redundant break statement

🪛 golangci-lint (1.64.8)
pkg/list/format/table.go

[error] 57-57: S1023: redundant break statement

(gosimple)

pkg/list/format/delimited.go

[error] 69-69: S1023: redundant break statement

(gosimple)


[error] 73-73: S1023: redundant break statement

(gosimple)


[error] 77-77: S1023: redundant break statement

(gosimple)

🪛 GitHub Check: codecov/patch
cmd/cmd_utils.go

[warning] 758-761: cmd/cmd_utils.go#L758-L761
Added lines #L758 - L761 were not covered by tests


[warning] 764-771: cmd/cmd_utils.go#L764-L771
Added lines #L764 - L771 were not covered by tests


[warning] 774-777: cmd/cmd_utils.go#L774-L777
Added lines #L774 - L777 were not covered by tests


[warning] 780-782: cmd/cmd_utils.go#L780-L782
Added lines #L780 - L782 were not covered by tests


[warning] 785-787: cmd/cmd_utils.go#L785-L787
Added lines #L785 - L787 were not covered by tests


[warning] 791-794: cmd/cmd_utils.go#L791-L794
Added lines #L791 - L794 were not covered by tests


[warning] 797-797: cmd/cmd_utils.go#L797
Added line #L797 was not covered by tests

cmd/list_metadata.go

[warning] 32-37: cmd/list_metadata.go#L32-L37
Added lines #L32 - L37 were not covered by tests


[warning] 39-40: cmd/list_metadata.go#L39-L40
Added lines #L39 - L40 were not covered by tests


[warning] 57-61: cmd/list_metadata.go#L57-L61
Added lines #L57 - L61 were not covered by tests


[warning] 63-72: cmd/list_metadata.go#L63-L72
Added lines #L63 - L72 were not covered by tests


[warning] 76-93: cmd/list_metadata.go#L76-L93
Added lines #L76 - L93 were not covered by tests


[warning] 98-100: cmd/list_metadata.go#L98-L100
Added lines #L98 - L100 were not covered by tests


[warning] 102-102: cmd/list_metadata.go#L102
Added line #L102 was not covered by tests


[warning] 106-111: cmd/list_metadata.go#L106-L111
Added lines #L106 - L111 were not covered by tests


[warning] 114-121: cmd/list_metadata.go#L114-L121
Added lines #L114 - L121 were not covered by tests


[warning] 124-128: cmd/list_metadata.go#L124-L128
Added lines #L124 - L128 were not covered by tests


[warning] 130-133: cmd/list_metadata.go#L130-L133
Added lines #L130 - L133 were not covered by tests


[warning] 136-140: cmd/list_metadata.go#L136-L140
Added lines #L136 - L140 were not covered by tests


[warning] 142-145: cmd/list_metadata.go#L142-L145
Added lines #L142 - L145 were not covered by tests


[warning] 149-153: cmd/list_metadata.go#L149-L153
Added lines #L149 - L153 were not covered by tests


[warning] 155-168: cmd/list_metadata.go#L155-L168
Added lines #L155 - L168 were not covered by tests


[warning] 171-174: cmd/list_metadata.go#L171-L174
Added lines #L171 - L174 were not covered by tests


[warning] 176-176: cmd/list_metadata.go#L176
Added line #L176 was not covered by tests

cmd/list_settings.go

[warning] 33-38: cmd/list_settings.go#L33-L38
Added lines #L33 - L38 were not covered by tests


[warning] 40-41: cmd/list_settings.go#L40-L41
Added lines #L40 - L41 were not covered by tests


[warning] 57-67: cmd/list_settings.go#L57-L67
Added lines #L57 - L67 were not covered by tests


[warning] 71-76: cmd/list_settings.go#L71-L76
Added lines #L71 - L76 were not covered by tests


[warning] 87-91: cmd/list_settings.go#L87-L91
Added lines #L87 - L91 were not covered by tests


[warning] 93-97: cmd/list_settings.go#L93-L97
Added lines #L93 - L97 were not covered by tests

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Acceptance Tests (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (10)
cmd/cmd_utils.go (1)

775-795: Confirm only terraform components must be checked

The logic checks the "terraform" map exclusively. If other component types (helmfile, etc.) also need existence checks, expand the scope or clarify that terraform is the sole target.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 780-782: cmd/cmd_utils.go#L780-L782
Added lines #L780 - L782 were not covered by tests


[warning] 785-787: cmd/cmd_utils.go#L785-L787
Added lines #L785 - L787 were not covered by tests


[warning] 791-794: cmd/cmd_utils.go#L791-L794
Added lines #L791 - L794 were not covered by tests

pkg/list/format/table.go (2)

30-43: Straightforward key extraction logic.

The function is concise and has clear sorting logic. Nice work.


188-214: Good separation of TTY vs. non-TTY outputs.

The fallback to CSV formatting is a smart approach for consistent output in scripts. Keep an eye on the width calculation to handle edge cases gracefully.

pkg/list/format/formatter_test.go (3)

31-44: Excellent negative test coverage.

Testing JSON formatting with invalid data (channel) is thorough.


61-111: Good CSV/TSV coverage.

You’re verifying delimiter usage and output structure. Nice approach.


219-269: Validating format strings thoroughly.

Covering both valid and invalid formats ensures robust error checking. Nicely done.

pkg/list/list_values.go (2)

120-161: Good error handling and debug logging.

This code clearly handles the scenario of zero extracted values and provides helpful debug logs. Nicely done.


241-251: Straightforward chaining of stack filtering.

The stepwise approach to apply stack pattern filtering first, then limit columns, makes the logic simple to follow.

pkg/list/format/delimited.go (1)

19-27: Good separation of concerns in Format implementation.

The method effectively delegates delimiter setup, key extraction, row generation, and final output building, keeping the code modular.

pkg/list/errors/types.go (1)

8-19: Well-structured component-specific error.

The NoValuesFoundError clearly conveys missing values for a given component and query, making debugging simpler.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (12)
cmd/list_errors_test.go (2)

68-69: Merge variable declaration with assignment.

These lines trigger a lint warning (S1021). Merging them improves readability and conforms to idiomatic Go:

-	var mockRunFunc func(cmd *cobra.Command, args []string) (string, error)
-	mockRunFunc = tc.runFunc
+	mockRunFunc := tc.runFunc
🧰 Tools
🪛 golangci-lint (1.64.8)

[error] 68-68: S1021: should merge variable declaration with assignment on next line

(gosimple)

🪛 GitHub Check: golangci-lint

[failure] 68-68:
S1021: should merge variable declaration with assignment on next line


170-171: Merge variable declaration with assignment again.

Same lint issue (S1021) appears here. Merge these lines as well for consistency:

-	var mockRunFunc func(cmd *cobra.Command, args []string) (string, error)
-	mockRunFunc = tc.runFunc
+	mockRunFunc := tc.runFunc
🧰 Tools
🪛 golangci-lint (1.64.8)

[error] 170-170: S1021: should merge variable declaration with assignment on next line

(gosimple)

🪛 GitHub Check: golangci-lint

[failure] 170-170:
S1021: should merge variable declaration with assignment on next line

cmd/list_metadata.go (3)

22-41: Add higher-level tests for RunE execution.

These lines invoke listMetadata in the RunE function. Consider adding an integration-style test for the Cobra command to confirm behavior and capture coverage for lines 32-40.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 32-37: cmd/list_metadata.go#L32-L37
Added lines #L32 - L37 were not covered by tests


[warning] 39-40: cmd/list_metadata.go#L39-L40
Added lines #L39 - L40 were not covered by tests


56-73: Consider testing setupMetadataOptions.

A small unit test verifying the resulting query, format, and stack pattern would ensure changes here remain robust.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 57-61: cmd/list_metadata.go#L57-L61
Added lines #L57 - L61 were not covered by tests


[warning] 63-72: cmd/list_metadata.go#L63-L72
Added lines #L63 - L72 were not covered by tests


119-167: Refine error handling and coverage.

While listMetadata nicely differentiates error types, lines 119-167 exhibit partial coverage according to codecov warnings. Adding more direct tests (or command-level tests) would confirm the correct reporting of each error scenario.

Do you want me to open a new issue or provide a test suite for these uncovered lines?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 119-123: cmd/list_metadata.go#L119-L123
Added lines #L119 - L123 were not covered by tests


[warning] 126-130: cmd/list_metadata.go#L126-L130
Added lines #L126 - L130 were not covered by tests


[warning] 132-135: cmd/list_metadata.go#L132-L135
Added lines #L132 - L135 were not covered by tests


[warning] 139-143: cmd/list_metadata.go#L139-L143
Added lines #L139 - L143 were not covered by tests


[warning] 145-158: cmd/list_metadata.go#L145-L158
Added lines #L145 - L158 were not covered by tests


[warning] 161-164: cmd/list_metadata.go#L161-L164
Added lines #L161 - L164 were not covered by tests


[warning] 166-166: cmd/list_metadata.go#L166
Added line #L166 was not covered by tests

pkg/list/format/table.go (2)

187-213: High complexity in Format.

Lines 187-213 incorporate multiple checks (TTY, width calculations, etc.). Consider splitting responsibilities into smaller helper functions to reduce complexity.


205-207: Potential static error usage.

This dynamic error is consistent with your approach, but you could wrap ErrTableTooWide with %w for consistent error inspection—particularly if you wish to differentiate table-width issues from other errors.

-	return "", errors.Errorf("%s (width: %d > %d).",
-		ErrTableTooWide.Error(), estimatedWidth, terminalWidth)
+	return "", fmt.Errorf("%w (width: %d > %d).",
+		ErrTableTooWide, estimatedWidth, terminalWidth)
cmd/list_values.go (2)

206-207: Add period to comment.

Static analysis indicates this comment should end with a period for consistency with other comments.

-// logNoValuesFoundMessage logs an appropriate message when no values or vars are found
+// logNoValuesFoundMessage logs an appropriate message when no values or vars are found.
🧰 Tools
🪛 GitHub Check: golangci-lint

[failure] 206-206:
Comment should end in a period


215-291: Function exceeds recommended length.

The listValues function is 75 lines long, exceeding the 60-line limit. Consider refactoring into smaller functions for better readability and maintainability.

You could split this function into several smaller functions:

  1. A function to validate arguments
  2. A function to initialize config and check component existence
  3. A function to process stacks and apply filters

Example refactoring approach:

func listValues(cmd *cobra.Command, args []string) (string, error) {
    // Ensure we have a component name
    if len(args) == 0 {
        return "", ErrComponentNameRequired
    }
    componentName := args[0]

    // Get all flags and options
    filterOptions, processingFlags, err := getListValuesFlags(cmd)
    if err != nil {
        return "", err
    }

    // Configure component filtering options
    configureComponentFiltering(filterOptions, componentName)

+   // Initialize config and validate component
+   stacksMap, err := initConfigAndValidateComponent(componentName, processingFlags)
+   if err != nil {
+       return "", err
+   }
    
-   // Use ComponentFilter instead of Component for filtering by component name
-   // This allows the extractComponentValues function to handle it properly
-   filterOptions.ComponentFilter = componentName
-
-   // For vars command (using .vars query), we clear the Component field
-   // to let the system determine the correct query path
-   if filterOptions.Query == ".vars" {
-       // Using ComponentFilter with empty Component
-       // lets the system build the correct YQ expression
-       filterOptions.Component = ""
-   } else {
-       // For other cases where we're not querying vars, use the standard approach
-       filterOptions.Component = componentName
-   }
-
-   // Log the component name
-   log.Debug("Processing component",
-       "component", componentName,
-       "component_field", filterOptions.Component,
-       "component_filter", filterOptions.ComponentFilter)
-
-   // Initialize CLI config
-   configAndStacksInfo := schema.ConfigAndStacksInfo{}
-   atmosConfig, err := config.InitCliConfig(configAndStacksInfo, true)
-   if err != nil {
-       return "", fmt.Errorf(ErrFmtWrapErr, ErrInitializingCLIConfig, err)
-   }
-
-   // Check if the component exists
-   if !checkComponentExists(&atmosConfig, componentName) {
-       return "", &listerrors.ComponentDefinitionNotFoundError{Component: componentName}
-   }
-
-   // Get all stacks
-   stacksMap, err := e.ExecuteDescribeStacks(atmosConfig, "", nil, nil, nil, false, processingFlags.Templates, processingFlags.Functions, false, nil)
-   if err != nil {
-       return "", fmt.Errorf(ErrFmtWrapErr, ErrDescribingStacks, err)
-   }

+   // Process and filter values
+   return processAndFilterValues(stacksMap, filterOptions, componentName, processingFlags)
}

+// configureComponentFiltering sets up the component filtering options
+func configureComponentFiltering(filterOptions *l.FilterOptions, componentName string) {
+   filterOptions.ComponentFilter = componentName
+
+   if filterOptions.Query == ".vars" {
+       // Using ComponentFilter with empty Component
+       // lets the system build the correct YQ expression
+       filterOptions.Component = ""
+   } else {
+       // For other cases where we're not querying vars, use the standard approach
+       filterOptions.Component = componentName
+   }
+
+   // Log the component name
+   log.Debug("Processing component",
+       "component", componentName,
+       "component_field", filterOptions.Component,
+       "component_filter", filterOptions.ComponentFilter)
+}
+
+// initConfigAndValidateComponent initializes the CLI config and validates the component exists
+func initConfigAndValidateComponent(componentName string, processingFlags *fl.ProcessingFlags) (map[string]interface{}, error) {
+   // Initialize CLI config
+   configAndStacksInfo := schema.ConfigAndStacksInfo{}
+   atmosConfig, err := config.InitCliConfig(configAndStacksInfo, true)
+   if err != nil {
+       return nil, fmt.Errorf(ErrFmtWrapErr, ErrInitializingCLIConfig, err)
+   }
+
+   // Check if the component exists
+   if !checkComponentExists(&atmosConfig, componentName) {
+       return nil, &listerrors.ComponentDefinitionNotFoundError{Component: componentName}
+   }
+
+   // Get all stacks
+   stacksMap, err := e.ExecuteDescribeStacks(atmosConfig, "", nil, nil, nil, false, processingFlags.Templates, processingFlags.Functions, false, nil)
+   if err != nil {
+       return nil, fmt.Errorf(ErrFmtWrapErr, ErrDescribingStacks, err)
+   }
+
+   return stacksMap, nil
+}
+
+// processAndFilterValues applies filtering and formatting to component values
+func processAndFilterValues(stacksMap map[string]interface{}, filterOptions *l.FilterOptions, componentName string, processingFlags *fl.ProcessingFlags) (string, error) {
+   // Log the filter options
+   log.Debug("Filtering values",
+       "component", componentName,
+       "componentFilter", filterOptions.ComponentFilter,
+       "query", filterOptions.Query,
+       "includeAbstract", filterOptions.IncludeAbstract,
+       "maxColumns", filterOptions.MaxColumns,
+       "format", filterOptions.FormatStr,
+       "stackPattern", filterOptions.StackPattern,
+       "processTemplates", processingFlags.Templates,
+       "processYamlFunctions", processingFlags.Functions)
+
+   // Filter and list component values across stacks
+   output, err := list.FilterAndListValues(stacksMap, filterOptions)
+   if err != nil {
+       var noValuesErr *listerrors.NoValuesFoundError
+       if errors.As(err, &noValuesErr) {
+           logNoValuesFoundMessage(componentName, filterOptions.Query)
+           return "", nil
+       }
+       return "", err
+   }
+
+   return output, nil
+}
🧰 Tools
🪛 GitHub Check: golangci-lint

[failure] 215-215:
function-length: maximum number of lines per function exceeded; max 60 but got 75

pkg/list/format/delimited.go (3)

57-79: Simplify the switch case structure in getValueKeysFromStacks.

The function has redundant break statements and could be simplified for better readability.

func getValueKeysFromStacks(data map[string]interface{}, keys []string) []string {
	var valueKeys []string

	for _, stackName := range keys {
		stackData := data[stackName]

		switch typedData := stackData.(type) {
		case map[string]interface{}:
			if varsData, ok := typedData["vars"].(map[string]interface{}); ok {
				for k := range varsData {
					valueKeys = append(valueKeys, k)
				}
-				break
			} else {
+				// If no vars map found, use top-level keys
+				for k := range typedData {
+					valueKeys = append(valueKeys, k)
+				}
			}
-
-			for k := range typedData {
-				valueKeys = append(valueKeys, k)
-			}

		case []interface{}, nil:
			valueKeys = []string{ValueKey}

		default:
			valueKeys = []string{ValueKey}
		}

		if len(valueKeys) > 0 {
			break
		}
	}
	sort.Strings(valueKeys)
	return valueKeys
}

112-121: Add comment to explain case handling in generateValueKeyRows.

The switch case differentiates between maps with value keys and other types. Adding a comment would improve readability.

switch typedData := stackData.(type) {
case map[string]interface{}:
+   // If the map has a ValueKey entry, use that specific value
+   // Otherwise format the entire map
    if val, ok := typedData[ValueKey]; ok {
        value = formatValue(val)
    } else {
        value = formatValue(typedData)
    }
default:
+   // For non-map types, format the entire value
    value = formatValue(stackData)
}

186-204: Enhance error handling in formatValue.

When JSON marshaling fails, more context would help with debugging. Consider logging the error or providing more details in the fallback string.

func formatValue(value interface{}) string {
	switch v := value.(type) {
	case string:
		return v
	case []interface{}:
		var values []string
		for _, item := range v {
			values = append(values, fmt.Sprintf("%v", item))
		}
		return strings.Join(values, ",")
	case map[string]interface{}:
		jsonBytes, err := json.Marshal(v)
		if err != nil {
-			return fmt.Sprintf("%v", v)
+			// Provide fallback with error context
+			return fmt.Sprintf("Error marshaling JSON: %v, Raw: %v", err, v)
		}
		return string(jsonBytes)
	default:
		return fmt.Sprintf("%v", v)
	}
}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99cc20f and bd7844b.

📒 Files selected for processing (5)
  • cmd/list_errors_test.go (1 hunks)
  • cmd/list_metadata.go (1 hunks)
  • cmd/list_values.go (1 hunks)
  • pkg/list/format/delimited.go (1 hunks)
  • pkg/list/format/table.go (1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
cmd/list_values.go (3)
pkg/list/errors/types.go (3)
  • ComponentVarsNotFoundError (165-167)
  • NoValuesFoundError (9-12)
  • ComponentDefinitionNotFoundError (183-185)
pkg/list/flags.go (1)
  • AddCommonListFlags (21-27)
pkg/list/list_values.go (2)
  • FilterOptions (55-64)
  • FilterAndListValues (67-97)
🪛 GitHub Check: codecov/patch
cmd/list_metadata.go

[warning] 32-37: cmd/list_metadata.go#L32-L37
Added lines #L32 - L37 were not covered by tests


[warning] 39-40: cmd/list_metadata.go#L39-L40
Added lines #L39 - L40 were not covered by tests


[warning] 57-61: cmd/list_metadata.go#L57-L61
Added lines #L57 - L61 were not covered by tests


[warning] 63-72: cmd/list_metadata.go#L63-L72
Added lines #L63 - L72 were not covered by tests


[warning] 76-81: cmd/list_metadata.go#L76-L81
Added lines #L76 - L81 were not covered by tests


[warning] 92-99: cmd/list_metadata.go#L92-L99
Added lines #L92 - L99 were not covered by tests


[warning] 101-105: cmd/list_metadata.go#L101-L105
Added lines #L101 - L105 were not covered by tests


[warning] 107-110: cmd/list_metadata.go#L107-L110
Added lines #L107 - L110 were not covered by tests


[warning] 112-116: cmd/list_metadata.go#L112-L116
Added lines #L112 - L116 were not covered by tests


[warning] 119-123: cmd/list_metadata.go#L119-L123
Added lines #L119 - L123 were not covered by tests


[warning] 126-130: cmd/list_metadata.go#L126-L130
Added lines #L126 - L130 were not covered by tests


[warning] 132-135: cmd/list_metadata.go#L132-L135
Added lines #L132 - L135 were not covered by tests


[warning] 139-143: cmd/list_metadata.go#L139-L143
Added lines #L139 - L143 were not covered by tests


[warning] 145-158: cmd/list_metadata.go#L145-L158
Added lines #L145 - L158 were not covered by tests


[warning] 161-164: cmd/list_metadata.go#L161-L164
Added lines #L161 - L164 were not covered by tests


[warning] 166-166: cmd/list_metadata.go#L166
Added line #L166 was not covered by tests

🪛 golangci-lint (1.64.8)
cmd/list_errors_test.go

[error] 68-68: S1021: should merge variable declaration with assignment on next line

(gosimple)


[error] 170-170: S1021: should merge variable declaration with assignment on next line

(gosimple)

🪛 GitHub Check: golangci-lint
cmd/list_errors_test.go

[failure] 68-68:
S1021: should merge variable declaration with assignment on next line


[failure] 170-170:
S1021: should merge variable declaration with assignment on next line

cmd/list_values.go

[failure] 206-206:
Comment should end in a period


[failure] 215-215:
function-length: maximum number of lines per function exceeded; max 60 but got 75

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Acceptance Tests (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (18)
cmd/list_errors_test.go (3)

12-25: Nice setup function.

The setupTestCommand function clearly initializes the necessary Cobra flags for testing. This modular approach makes it easy to mock command execution. No issues found here.


27-59: Robust table-driven tests.

Your test cases clearly define the scenario of "component not found," and the expected output is well-documented. This approach will simplify future maintenance and extension of tests.


90-118: Well-structured scenario checks.

You have clear coverage of multiple "no values found" use cases, ensuring each condition is tested. This test strategy is comprehensive and straightforward.

cmd/list_metadata.go (1)

75-83: Use consistent logging.

logNoMetadataFoundMessage uses log.Info. Verify that other similar branches use the same log level for consistent user feedback.

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 76-81: cmd/list_metadata.go#L76-L81
Added lines #L76 - L81 were not covered by tests

pkg/list/format/table.go (4)

30-44: Efficient key extraction.

extractAndSortKeys is simple and clear. The early slice truncation respects maxColumns effectively.


45-70: Check for arrays vs maps.

extractValueKeys gracefully handles different data structures. This is good for flexibility. Just be mindful that returning immediately for arrays or “default” objects leads to a single [value] key, bypassing further iteration.

Consider verifying whether more advanced logic might be needed when arrays nest deeper. Scripts or extended tests may be needed if nested handling is required.


78-107: Comprehensive row creation.

createRows systematically handles both single-key scenarios and multiple nested keys. This approach is straightforward and easy to modify.


109-134: Concise multi-type formatting.

formatTableCellValue covers many data types and truncates lengthy values. This is user-friendly and keeps table columns tidy.

cmd/list_values.go (7)

13-13: Rename package alias to be more descriptive.

The alias l for the list package is used for logging in other parts of the codebase. Consider renaming to list for clarity and consistency.

-import l "github.com/cloudposse/atmos/pkg/list"
+import list "github.com/cloudposse/atmos/pkg/list"

47-54: Use backticks for multi-line strings.

For better readability and maintenance, use backtick syntax instead of string concatenation.

-	Example: "atmos list values vpc\n" +
-		"atmos list values vpc --abstract\n" +
-		"atmos list values vpc --query '.vars'\n" +
-		"atmos list values vpc --query '.vars.region'\n" +
-		"atmos list values vpc --format json\n" +
-		"atmos list values vpc --format yaml\n" +
-		"atmos list values vpc --format csv",
+	Example: `atmos list values vpc
+atmos list values vpc --abstract
+atmos list values vpc --query '.vars'
+atmos list values vpc --query '.vars.region'
+atmos list values vpc --format json
+atmos list values vpc --format yaml
+atmos list values vpc --format csv`,

77-82: Use backticks for multi-line strings.

For better readability and maintenance, use backtick syntax instead of string concatenation.

-	Example: "atmos list vars vpc\n" +
-		"atmos list vars vpc --abstract\n" +
-		"atmos list vars vpc --max-columns 5\n" +
-		"atmos list vars vpc --format json\n" +
-		"atmos list vars vpc --format yaml\n" +
-		"atmos list vars vpc --format csv",
+	Example: `atmos list vars vpc
+atmos list vars vpc --abstract
+atmos list vars vpc --max-columns 5
+atmos list vars vpc --format json
+atmos list vars vpc --format yaml
+atmos list vars vpc --format csv`,

88-91: Check error return value when setting query flag.

The error returned from cmd.Flags().Set("query", ".vars") is correctly checked and handled. This is good practice to prevent silent failures.


93-107: Well-structured error handling with typed errors.

Using errors.As() to check for specific error types is the correct approach. This provides better error handling than string matching.


21-29: Good use of static error variables.

Defining error variables at the package level is a good practice for consistent error handling and reuse. The error messages are clear and specific.


167-203: Well-structured function for flag handling.

The getListValuesFlags function effectively extracts all flags and handles errors appropriately. The use of predefined error variables with wrapping is good practice.

pkg/list/format/delimited.go (3)

18-27: Well-structured Format method with extracted helper functions.

The Format method has been effectively decomposed into smaller helper functions, making it much more maintainable. This addresses the cognitive complexity concerns noted in previous reviews.


12-16: Good use of named constants.

Using named constants like ValueKey makes the code more maintainable and addresses the "add-constant" issue noted in previous reviews.


151-172: Clear method naming and documentation.

The function name generatePropertyKeyRows clearly describes its purpose, and the comment explains the difference between this function and generateValueKeyRows. This makes the code self-documenting.

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 28, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The implementation only checks for terraform components, ignoring other potential component types
  2. The code makes multiple type assertions without detailed error handling
  3. Using ExecuteDescribeStacks is expensive for just checking component existence

Consider 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 GetComponentManifests method and appropriate structure in the AtmosConfiguration. 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 to KeyMetadata) 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, QueryError is used to wrap errors from fl.GetCommonListFlags. In list_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

📥 Commits

Reviewing files that changed from the base of the PR and between bd7844b and 5b8f77d.

📒 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 NoValuesFoundError types 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.

coderabbitai[bot]
coderabbitai bot previously approved these changes Mar 28, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 json o...

(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

📥 Commits

Reviewing files that changed from the base of the PR and between 2e5ce5a and c4f950f.

📒 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 --query flag 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 --abstract flag 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-columns flag 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.

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link

github-actions bot commented Apr 6, 2025

These changes were released in v1.170.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-release Do not create a new release (wait for additional code changes)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants