-
-
Notifications
You must be signed in to change notification settings - Fork 105
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improve atmos list components
view
#828
base: main
Are you sure you want to change the base?
Changes from all commits
c32d12f
53c74ef
a3fec3d
d192dcf
382851e
6b4f28e
ef8a25d
277e5b7
2555c35
bb305d1
2b5cb97
964ba87
dafdfc8
c24bf75
3b2425d
82a8a73
925d0e7
2c85549
1fa0f47
f368057
d939c70
fa0c9f2
b4bc19f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -3,13 +3,14 @@ package cmd | |||||||
import ( | ||||||||
"fmt" | ||||||||
|
||||||||
"github.com/fatih/color" | ||||||||
"github.com/spf13/cobra" | ||||||||
|
||||||||
e "github.com/cloudposse/atmos/internal/exec" | ||||||||
"github.com/cloudposse/atmos/pkg/config" | ||||||||
l "github.com/cloudposse/atmos/pkg/list" | ||||||||
"github.com/cloudposse/atmos/pkg/schema" | ||||||||
u "github.com/cloudposse/atmos/pkg/utils" | ||||||||
"github.com/fatih/color" | ||||||||
"github.com/spf13/cobra" | ||||||||
) | ||||||||
|
||||||||
// listComponentsCmd lists atmos components | ||||||||
|
@@ -24,6 +25,7 @@ var listComponentsCmd = &cobra.Command{ | |||||||
checkAtmosConfig() | ||||||||
|
||||||||
stackFlag, _ := cmd.Flags().GetString("stack") | ||||||||
abstractFlag, _ := cmd.Flags().GetBool("abstract") | ||||||||
|
||||||||
configAndStacksInfo := schema.ConfigAndStacksInfo{} | ||||||||
atmosConfig, err := config.InitCliConfig(configAndStacksInfo, true) | ||||||||
|
@@ -38,7 +40,7 @@ var listComponentsCmd = &cobra.Command{ | |||||||
return | ||||||||
} | ||||||||
|
||||||||
output, err := l.FilterAndListComponents(stackFlag, stacksMap) | ||||||||
output, err := l.FilterAndListComponents(stackFlag, abstractFlag, stacksMap, atmosConfig.Components.List) | ||||||||
if err != nil { | ||||||||
u.PrintMessageInColor(fmt.Sprintf("Error: %v"+"\n", err), color.New(color.FgYellow)) | ||||||||
return | ||||||||
|
@@ -50,5 +52,6 @@ var listComponentsCmd = &cobra.Command{ | |||||||
|
||||||||
func init() { | ||||||||
listComponentsCmd.PersistentFlags().StringP("stack", "s", "", "Filter components by stack (e.g., atmos list components -s stack1)") | ||||||||
listComponentsCmd.PersistentFlags().Bool("abstract", false, "Filter abstract component if true") | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The inverted logic here is IMO a bit confusing. Can't we use something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For certain, we want to hide abstract by default. We can use a type parameter as you suggest. That makes the default Please also add a and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Revise flag definition to align with user requirements. Based on the discussion in past reviews:
-listComponentsCmd.PersistentFlags().Bool("abstract", false, "Filter abstract component if true")
+listComponentsCmd.PersistentFlags().String("type", "real", "Filter components by type: real|abstract|all")
+listComponentsCmd.PersistentFlags().String("kind", "any", "Filter components by kind: terraform|helmfile|any") 📝 Committable suggestion
Suggested change
|
||||||||
listCmd.AddCommand(listComponentsCmd) | ||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -18,6 +18,16 @@ | |||||||
base_path: "." | ||||||||
|
||||||||
components: | ||||||||
list: | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Support a configurable default
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The default should be |
||||||||
columns: | ||||||||
- name: Component | ||||||||
value: '{{ .atmos_component }}' | ||||||||
- name: Stack | ||||||||
value: '{{ .atmos_stack }}' | ||||||||
- name: Tenant | ||||||||
value: '{{ .vars.tenant }}' | ||||||||
- name: Region | ||||||||
value: '{{ .vars.region }}' | ||||||||
terraform: | ||||||||
# Optional `command` specifies the executable to be called by `atmos` when running Terraform commands | ||||||||
# If not defined, `terraform` is used | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,14 +2,28 @@ package list | |
|
||
import ( | ||
"fmt" | ||
"os" | ||
"regexp" | ||
"sort" | ||
"strings" | ||
|
||
"github.com/charmbracelet/lipgloss" | ||
"github.com/charmbracelet/lipgloss/table" | ||
"github.com/samber/lo" | ||
"golang.org/x/term" | ||
|
||
"github.com/cloudposse/atmos/internal/exec" | ||
"github.com/cloudposse/atmos/pkg/schema" | ||
) | ||
|
||
type tableData struct { | ||
header []string | ||
rows [][]string | ||
colWidths []int | ||
} | ||
|
||
// getStackComponents extracts Terraform components from the final map of stacks | ||
func getStackComponents(stackData any) ([]string, error) { | ||
func getStackComponents(stackData any, abstractFlag bool, listFields []string) ([]string, error) { | ||
stackMap, ok := stackData.(map[string]any) | ||
if !ok { | ||
return nil, fmt.Errorf("could not parse stacks") | ||
|
@@ -25,41 +39,281 @@ func getStackComponents(stackData any) ([]string, error) { | |
return nil, fmt.Errorf("could not parse Terraform components") | ||
} | ||
|
||
return lo.Keys(terraformComponents), nil | ||
var uniqueKeys []string | ||
|
||
if abstractFlag { | ||
uniqueKeys = lo.Keys(terraformComponents) | ||
} else { | ||
uniqueKeys = exec.FilterAbstractComponents(terraformComponents) | ||
} | ||
result := make([]string, 0) | ||
|
||
for _, dataKey := range uniqueKeys { | ||
data := terraformComponents[dataKey] | ||
dataMap, ok := data.(map[string]any) | ||
if !ok { | ||
return nil, fmt.Errorf("unexpected data type for component '%s'", dataKey) | ||
} | ||
rowData := make([]string, 0) | ||
for _, key := range listFields { | ||
value, found := resolveKey(dataMap, key) | ||
if !found { | ||
value = "-" | ||
} | ||
rowData = append(rowData, fmt.Sprintf("%s", value)) | ||
} | ||
result = append(result, strings.Join(rowData, "\t\t")) | ||
} | ||
return result, nil | ||
} | ||
|
||
// FilterAndListComponents filters and lists components based on the given stack | ||
func FilterAndListComponents(stackFlag string, stacksMap map[string]any) (string, error) { | ||
components := []string{} | ||
// resolveKey resolves a key from a map, supporting nested keys with dot notation | ||
func resolveKey(data map[string]any, key string) (any, bool) { | ||
// Remove leading dot from the key (e.g., `.vars.tenant` -> `vars.tenant`) | ||
key = strings.TrimPrefix(key, ".") | ||
|
||
// Split key on `.` | ||
parts := strings.Split(key, ".") | ||
current := data | ||
|
||
// Traverse the map for each part | ||
for i, part := range parts { | ||
if i == len(parts)-1 { | ||
// Return the value for the last part | ||
if value, exists := current[part]; exists { | ||
return value, true | ||
} | ||
return nil, false | ||
} | ||
|
||
// Traverse deeper | ||
if nestedMap, ok := current[part].(map[string]any); ok { | ||
current = nestedMap | ||
} else { | ||
return nil, false | ||
} | ||
} | ||
|
||
return nil, false | ||
} | ||
|
||
// parseColumns extracts the header and list fields from the listConfig | ||
func parseColumns(listConfig schema.ListConfig) ([]string, []string, error) { | ||
if len(listConfig.Columns) == 0 { | ||
return nil, nil, fmt.Errorf("no columns configured") | ||
} | ||
header := make([]string, 0) | ||
listFields := make([]string, 0) | ||
re := regexp.MustCompile(`\{\{\s*(.*?)\s*\}\}`) | ||
|
||
for _, col := range listConfig.Columns { | ||
if col.Value == "" { | ||
return nil, nil, fmt.Errorf("empty value for column name %s", col.Name) | ||
} | ||
header = append(header, col.Name) | ||
match := re.FindStringSubmatch(col.Value) | ||
if len(match) > 1 { | ||
listFields = append(listFields, match[1]) | ||
} else { | ||
return nil, nil, fmt.Errorf("invalid value format for column name %s", col.Name) | ||
} | ||
} | ||
return header, listFields, nil | ||
} | ||
pkbhowmick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// collectComponents gathers components for the specified stack or all stacks | ||
func collectComponents(stackFlag string, abstractFlag bool, stacksMap map[string]any, listFields []string) ([][]string, error) { | ||
components := [][]string{} | ||
|
||
if stackFlag != "" { | ||
// Filter components for the specified stack | ||
if stackData, ok := stacksMap[stackFlag]; ok { | ||
stackComponents, err := getStackComponents(stackData) | ||
stackComponents, err := getStackComponents(stackData, abstractFlag, listFields) | ||
if err != nil { | ||
return "", fmt.Errorf("error processing stack '%s': %w", stackFlag, err) | ||
return nil, fmt.Errorf("error processing stack '%s': %w", stackFlag, err) | ||
} | ||
for _, c := range stackComponents { | ||
components = append(components, strings.Fields(c)) | ||
} | ||
components = append(components, stackComponents...) | ||
} else { | ||
return "", fmt.Errorf("stack '%s' not found", stackFlag) | ||
return nil, fmt.Errorf("stack '%s' not found", stackFlag) | ||
} | ||
} else { | ||
// Get all components from all stacks | ||
// Collect components from all stacks | ||
var errors []string | ||
for _, stackData := range stacksMap { | ||
stackComponents, err := getStackComponents(stackData) | ||
stackComponents, err := getStackComponents(stackData, abstractFlag, listFields) | ||
if err != nil { | ||
errors = append(errors, err.Error()) | ||
continue // Skip invalid stacks | ||
} | ||
components = append(components, stackComponents...) | ||
for _, c := range stackComponents { | ||
components = append(components, strings.Fields(c)) | ||
} | ||
} | ||
if len(errors) > 0 { | ||
return components, fmt.Errorf("errors processing stacks: %s", strings.Join(errors, "; ")) | ||
} | ||
} | ||
return components, nil | ||
} | ||
|
||
// processComponents deduplicates, sorts, and calculates column widths | ||
func processComponents(header []string, components [][]string) ([][]string, []int) { | ||
uniqueComponents := lo.UniqBy(components, func(item []string) string { | ||
return strings.Join(item, "\t") | ||
}) | ||
sort.Slice(uniqueComponents, func(i, j int) bool { | ||
return strings.Join(uniqueComponents[i], "\t") < strings.Join(uniqueComponents[j], "\t") | ||
}) | ||
|
||
colWidths := make([]int, len(header)) | ||
for i, h := range header { | ||
colWidths[i] = len(h) | ||
} | ||
for _, row := range uniqueComponents { | ||
for i, field := range row { | ||
if len(field) > colWidths[i] { | ||
colWidths[i] = len(field) | ||
} | ||
} | ||
} | ||
|
||
// Remove duplicates and sort components | ||
components = lo.Uniq(components) | ||
sort.Strings(components) | ||
return uniqueComponents, colWidths | ||
} | ||
|
||
if len(components) == 0 { | ||
const ( | ||
purple = lipgloss.Color("99") // Purple color for headers and borders | ||
gray = lipgloss.Color("245") // Gray for odd rows | ||
lightGray = lipgloss.Color("241") // Light gray for even rows | ||
) | ||
Comment on lines
+185
to
+189
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are refactoring how colors are handled in: |
||
|
||
// Fallback for non-TTY environments | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we get a machine readable output, too? The current style without any column headings and the like would be fine but maybe just a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that’s a good suggestion. I think we support a |
||
func printSimpleTable(data tableData) { | ||
// Print headers | ||
fmt.Println(data.header) | ||
|
||
// Print rows | ||
for _, row := range data.rows { | ||
fmt.Println(row) | ||
} | ||
} | ||
|
||
func generateTable(data tableData) { | ||
// Check if TTY is attached | ||
if !term.IsTerminal(int(os.Stdout.Fd())) { | ||
// Degrade to a simple tabular format | ||
printSimpleTable(data) | ||
return | ||
} | ||
|
||
// Dynamically calculate column widths | ||
columnWidths := calculateColumnWidths(data) | ||
|
||
// Renderer for styling | ||
re := lipgloss.NewRenderer(os.Stdout) | ||
|
||
// Define styles | ||
var ( | ||
HeaderStyle = re.NewStyle(). | ||
Foreground(purple). | ||
Bold(true). | ||
Align(lipgloss.Center) // Header style | ||
|
||
CellStyle = re.NewStyle(). | ||
Padding(0, 1) // Base style for rows | ||
|
||
OddRowStyle = CellStyle.Foreground(gray) // Style for odd rows | ||
EvenRowStyle = CellStyle.Foreground(lightGray) // Style for even rows | ||
|
||
BorderStyle = lipgloss.NewStyle(). | ||
Foreground(gray) | ||
) | ||
|
||
// Create the table with headers, rows, and styles | ||
t := table.New(). | ||
Border(lipgloss.ThickBorder()). | ||
BorderStyle(BorderStyle). | ||
StyleFunc(func(row, col int) lipgloss.Style { | ||
var style lipgloss.Style | ||
|
||
switch { | ||
case row == table.HeaderRow: | ||
return HeaderStyle // Style for header | ||
case row%2 == 0: | ||
style = EvenRowStyle // Even rows | ||
default: | ||
style = OddRowStyle // Odd rows | ||
} | ||
|
||
// Apply dynamic width to each column | ||
style = style.Width(columnWidths[col]) | ||
|
||
return style | ||
}). | ||
Headers(data.header...). | ||
Rows(data.rows...) | ||
|
||
// Render and print the table | ||
fmt.Println(t) | ||
} | ||
|
||
// Calculate the maximum width for each column | ||
func calculateColumnWidths(data tableData) []int { | ||
columnCount := len(data.header) | ||
columnWidths := make([]int, columnCount) | ||
|
||
// Check headers | ||
for i, header := range data.header { | ||
if len(header) > columnWidths[i] { | ||
columnWidths[i] = len(header) | ||
} | ||
} | ||
|
||
// Check rows | ||
for _, row := range data.rows { | ||
for i, cell := range row { | ||
if len(cell) > columnWidths[i] { | ||
columnWidths[i] = len(cell) | ||
} | ||
} | ||
} | ||
|
||
// Add padding for aesthetics | ||
for i := range columnWidths { | ||
columnWidths[i] += 2 // Add 2 spaces for padding | ||
} | ||
|
||
return columnWidths | ||
} | ||
|
||
// FilterAndListComponents orchestrates the process | ||
func FilterAndListComponents(stackFlag string, abstractFlag bool, stacksMap map[string]any, listConfig schema.ListConfig) (string, error) { | ||
// Step 1: Parse columns | ||
header, listFields, err := parseColumns(listConfig) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
// Step 2: Collect components | ||
components, err := collectComponents(stackFlag, abstractFlag, stacksMap, listFields) | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
// Step 3: Process components | ||
processedComponents, colWidths := processComponents(header, components) | ||
if len(processedComponents) == 0 { | ||
return "No components found", nil | ||
} | ||
return strings.Join(components, "\n") + "\n", nil | ||
|
||
// Step 4: Format and display table | ||
data := tableData{ | ||
header: header, | ||
rows: processedComponents, | ||
colWidths: colWidths, | ||
} | ||
generateTable(data) | ||
|
||
return "", nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using
--type
flag instead of--abstract
.Based on the discussion in issue #833, using a
--type
flag with valuesall|real|abstract
would provide a more intuitive interface than a boolean flag.