Skip to content
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

Filter out empty results from describe stacks #764

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions cmd/describe_stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,7 @@ func init() {

describeStacksCmd.PersistentFlags().Bool("process-templates", true, "Enable/disable Go template processing in Atmos stack manifests when executing the command: atmos describe stacks --process-templates=false")

describeStacksCmd.PersistentFlags().Bool("include-empty-stacks", false, "Include stacks with no components in the output: atmos describe stacks --include-empty-stacks")

describeCmd.AddCommand(describeStacksCmd)
}
2 changes: 1 addition & 1 deletion internal/exec/atmos.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func ExecuteAtmosCmd() error {

// Get a map of stacks and components in the stacks
// Don't process `Go` templates in Atmos stack manifests since we just need to display the stack and component names in the TUI
stacksMap, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, false)
stacksMap, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, false, false)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/exec/describe_affected_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func executeDescribeAffected(
u.LogTrace(cliConfig, fmt.Sprintf("Current HEAD: %s", localRepoHead))
u.LogTrace(cliConfig, fmt.Sprintf("BASE: %s", remoteRepoHead))

currentStacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, nil, false, true)
currentStacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, nil, false, true, false)
if err != nil {
return nil, nil, nil, err
}
Expand Down Expand Up @@ -444,7 +444,7 @@ func executeDescribeAffected(
return nil, nil, nil, err
}

remoteStacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, nil, true, true)
remoteStacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, nil, true, true, false)
if err != nil {
return nil, nil, nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion internal/exec/describe_dependents.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func ExecuteDescribeDependents(
var ok bool

// Get all stacks with all components
stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, true)
stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, true, false)
if err != nil {
return nil, err
}
Expand Down
116 changes: 110 additions & 6 deletions internal/exec/describe_stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ func ExecuteDescribeStacksCmd(cmd *cobra.Command, args []string) error {
return err
}

includeEmptyStacks, err := cmd.Flags().GetBool("include-empty-stacks")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
includeEmptyStacks, err := cmd.Flags().GetBool("include-empty-stacks")
includeEmptyStacks, err := flags.GetBool("include-empty-stacks")

if err != nil {
return err
}

componentsCsv, err := flags.GetString("components")
if err != nil {
return err
Expand Down Expand Up @@ -97,6 +102,7 @@ func ExecuteDescribeStacksCmd(cmd *cobra.Command, args []string) error {
sections,
false,
processTemplates,
includeEmptyStacks,
)
if err != nil {
return err
Expand All @@ -119,6 +125,7 @@ func ExecuteDescribeStacks(
sections []string,
ignoreMissingFiles bool,
processTemplates bool,
includeEmptyStacks bool,
) (map[string]any, error) {

stacksMap, _, err := FindStacksMap(cliConfig, ignoreMissingFiles)
Expand All @@ -127,6 +134,7 @@ func ExecuteDescribeStacks(
}

finalStacksMap := make(map[string]any)
processedStacks := make(map[string]bool)
var varsSection map[string]any
var metadataSection map[string]any
var settingsSection map[string]any
Expand All @@ -136,12 +144,48 @@ func ExecuteDescribeStacks(
var backendSection map[string]any
var backendTypeSection string
var stackName string
context := schema.Context{}

for stackFileName, stackSection := range stacksMap {
var context schema.Context

// Delete the stack-wide imports
delete(stackSection.(map[string]any), "imports")

// Check if components section exists and has explicit components
hasExplicitComponents := false
if componentsSection, ok := stackSection.(map[string]any)["components"]; ok {
if componentsSection != nil {
if terraformSection, ok := componentsSection.(map[string]any)["terraform"].(map[string]any); ok {
hasExplicitComponents = len(terraformSection) > 0
}
if helmfileSection, ok := componentsSection.(map[string]any)["helmfile"].(map[string]any); ok {
hasExplicitComponents = hasExplicitComponents || len(helmfileSection) > 0
}
}
}

// Also check for imports
hasImports := false
if importsSection, ok := stackSection.(map[string]any)["import"].([]any); ok {
hasImports = len(importsSection) > 0
}

// Skip stacks without components or imports when includeEmptyStacks is false
if !includeEmptyStacks && !hasExplicitComponents && !hasImports {
continue
}

stackName = stackFileName
if processedStacks[stackName] {
continue
}
processedStacks[stackName] = true

if !u.MapKeyExists(finalStacksMap, stackName) {
finalStacksMap[stackName] = make(map[string]any)
finalStacksMap[stackName].(map[string]any)["components"] = make(map[string]any)
}

if componentsSection, ok := stackSection.(map[string]any)["components"].(map[string]any); ok {

if len(componentTypes) == 0 || u.SliceContainsString(componentTypes, "terraform") {
Expand Down Expand Up @@ -242,9 +286,13 @@ func ExecuteDescribeStacks(

if stackName == "" {
stackName = stackFileName
} else if strings.HasPrefix(stackFileName, "deploy/") {
Copy link
Member

Choose a reason for hiding this comment

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

@Cerebrovinny why do we need a special treatment for the deploy/ folder?
The deploy/ was just an example in the examples folder, it should not be a special name, and should not be hardcoded in Atmos code

// If we have a deploy/ prefixed version, use that as the canonical name
stackName = stackFileName
}

if !u.MapKeyExists(finalStacksMap, stackName) {
// Only create the stack entry if it doesn't exist or if we're using the canonical name
if !u.MapKeyExists(finalStacksMap, stackName) || strings.HasPrefix(stackName, "deploy/") {
Cerebrovinny marked this conversation as resolved.
Show resolved Hide resolved
finalStacksMap[stackName] = make(map[string]any)
}

Expand Down Expand Up @@ -430,9 +478,13 @@ func ExecuteDescribeStacks(

if stackName == "" {
stackName = stackFileName
} else if strings.HasPrefix(stackFileName, "deploy/") {
// If we have a deploy/ prefixed version, use that as the canonical name
stackName = stackFileName
}

if !u.MapKeyExists(finalStacksMap, stackName) {
// Only create the stack entry if it doesn't exist or if we're using the canonical name
if !u.MapKeyExists(finalStacksMap, stackName) || strings.HasPrefix(stackName, "deploy/") {
finalStacksMap[stackName] = make(map[string]any)
}

Expand Down Expand Up @@ -511,11 +563,63 @@ func ExecuteDescribeStacks(
}
}
}
}

// Filter out empty stacks after processing all stack files
if !includeEmptyStacks {
for stackName := range finalStacksMap {
if stackName == "" {
delete(finalStacksMap, stackName)
continue
}

stackEntry := finalStacksMap[stackName].(map[string]any)
componentsSection, hasComponents := stackEntry["components"].(map[string]any)

Cerebrovinny marked this conversation as resolved.
Show resolved Hide resolved
if !hasComponents {
delete(finalStacksMap, stackName)
continue
}

// Check if any component type (terraform/helmfile) has components
hasNonEmptyComponents := false
for _, components := range componentsSection {
if compTypeMap, ok := components.(map[string]any); ok {
for _, comp := range compTypeMap {
if compContent, ok := comp.(map[string]any); ok {
// Check for any meaningful content
relevantSections := []string{"vars", "metadata", "settings", "env", "workspace"}
for _, section := range relevantSections {
if _, hasSection := compContent[section]; hasSection {
hasNonEmptyComponents = true
break
}
}
}
}
}
if hasNonEmptyComponents {
break
}
}

// Filter out empty stacks (stacks without any components)
if st, ok := finalStacksMap[stackName].(map[string]any); ok {
if len(st) == 0 {
if !hasNonEmptyComponents {
delete(finalStacksMap, stackName)
continue
}

// Check for duplicate stacks (deploy/ prefix)
if strings.HasPrefix(stackName, "deploy/") {
baseStackName := strings.TrimPrefix(stackName, "deploy/")
delete(finalStacksMap, baseStackName)
}
}
} else {
// When including empty stacks, we still need to handle deploy/ prefix duplicates
for stackName := range finalStacksMap {
if strings.HasPrefix(stackName, "deploy/") {
baseStackName := strings.TrimPrefix(stackName, "deploy/")
delete(finalStacksMap, baseStackName)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/describe/describe_stacks.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ func ExecuteDescribeStacks(
componentTypes []string,
sections []string,
ignoreMissingFiles bool,
includeEmptyStacks bool,
) (map[string]any, error) {

return e.ExecuteDescribeStacks(cliConfig, filterByStack, components, componentTypes, sections, ignoreMissingFiles, true)
return e.ExecuteDescribeStacks(cliConfig, filterByStack, components, componentTypes, sections, ignoreMissingFiles, false, includeEmptyStacks)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return e.ExecuteDescribeStacks(cliConfig, filterByStack, components, componentTypes, sections, ignoreMissingFiles, false, includeEmptyStacks)
return e.ExecuteDescribeStacks(cliConfig, filterByStack, components, componentTypes, sections, ignoreMissingFiles, true, includeEmptyStacks)

Copy link
Member

Choose a reason for hiding this comment

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

this ExecuteDescribeStacks function is called by the terraform-provider-utils, we should process Go templates

}
89 changes: 81 additions & 8 deletions pkg/describe/describe_stacks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestDescribeStacks(t *testing.T) {
cliConfig, err := cfg.InitCliConfig(configAndStacksInfo, true)
assert.Nil(t, err)

stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false)
stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, false)
assert.Nil(t, err)

dependentsYaml, err := u.ConvertToYAML(stacks)
Expand All @@ -32,7 +32,7 @@ func TestDescribeStacksWithFilter1(t *testing.T) {

stack := "tenant1-ue2-dev"

stacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, nil, false)
stacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, nil, false, false)
assert.Nil(t, err)

dependentsYaml, err := u.ConvertToYAML(stacks)
Expand All @@ -49,7 +49,7 @@ func TestDescribeStacksWithFilter2(t *testing.T) {
stack := "tenant1-ue2-dev"
components := []string{"infra/vpc"}

stacks, err := ExecuteDescribeStacks(cliConfig, stack, components, nil, nil, false)
stacks, err := ExecuteDescribeStacks(cliConfig, stack, components, nil, nil, false, false)
assert.Nil(t, err)

dependentsYaml, err := u.ConvertToYAML(stacks)
Expand All @@ -66,7 +66,7 @@ func TestDescribeStacksWithFilter3(t *testing.T) {
stack := "tenant1-ue2-dev"
sections := []string{"vars"}

stacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, sections, false)
stacks, err := ExecuteDescribeStacks(cliConfig, stack, nil, nil, sections, false, false)
assert.Nil(t, err)

dependentsYaml, err := u.ConvertToYAML(stacks)
Expand All @@ -83,7 +83,7 @@ func TestDescribeStacksWithFilter4(t *testing.T) {
componentTypes := []string{"terraform"}
sections := []string{"none"}

stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, componentTypes, sections, false)
stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, componentTypes, sections, false, false)
assert.Nil(t, err)

dependentsYaml, err := u.ConvertToYAML(stacks)
Expand All @@ -101,7 +101,7 @@ func TestDescribeStacksWithFilter5(t *testing.T) {
components := []string{"top-level-component1"}
sections := []string{"vars"}

stacks, err := ExecuteDescribeStacks(cliConfig, "", components, componentTypes, sections, false)
stacks, err := ExecuteDescribeStacks(cliConfig, "", components, componentTypes, sections, false, false)
assert.Nil(t, err)
assert.Equal(t, 8, len(stacks))

Expand Down Expand Up @@ -133,7 +133,7 @@ func TestDescribeStacksWithFilter6(t *testing.T) {
components := []string{"top-level-component1"}
sections := []string{"workspace"}

stacks, err := ExecuteDescribeStacks(cliConfig, "tenant1-ue2-dev", components, componentTypes, sections, false)
stacks, err := ExecuteDescribeStacks(cliConfig, "tenant1-ue2-dev", components, componentTypes, sections, false, false)
assert.Nil(t, err)
assert.Equal(t, 1, len(stacks))

Expand All @@ -160,7 +160,7 @@ func TestDescribeStacksWithFilter7(t *testing.T) {
components := []string{"test/test-component-override-3"}
sections := []string{"workspace"}

stacks, err := ExecuteDescribeStacks(cliConfig, stack, components, componentTypes, sections, false)
stacks, err := ExecuteDescribeStacks(cliConfig, stack, components, componentTypes, sections, false, false)
assert.Nil(t, err)
assert.Equal(t, 1, len(stacks))

Expand All @@ -175,3 +175,76 @@ func TestDescribeStacksWithFilter7(t *testing.T) {
assert.Nil(t, err)
t.Log(stacksYaml)
}

func TestDescribeStacksWithEmptyStacks(t *testing.T) {
configAndStacksInfo := schema.ConfigAndStacksInfo{}

cliConfig, err := cfg.InitCliConfig(configAndStacksInfo, true)
assert.Nil(t, err)

stacks, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, false)
assert.Nil(t, err)

initialStackCount := len(stacks)

stacksWithEmpty, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, true)
assert.Nil(t, err)

assert.Greater(t, len(stacksWithEmpty), initialStackCount, "Should include more stacks when empty stacks are included")

foundEmptyStack := false
for _, stackContent := range stacksWithEmpty {
if components, ok := stackContent.(map[string]any)["components"].(map[string]any); ok {
if len(components) == 0 || (len(components) == 1 && len(components["terraform"].(map[string]any)) == 0) {
foundEmptyStack = true
break
}
}
}
assert.True(t, foundEmptyStack, "Should find at least one empty stack")
}
Cerebrovinny marked this conversation as resolved.
Show resolved Hide resolved

func TestDescribeStacksWithVariousEmptyStacks(t *testing.T) {
configAndStacksInfo := schema.ConfigAndStacksInfo{}

cliConfig, err := cfg.InitCliConfig(configAndStacksInfo, true)
assert.Nil(t, err)

stacksWithoutEmpty, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, false)
assert.Nil(t, err)
initialCount := len(stacksWithoutEmpty)

stacksWithEmpty, err := ExecuteDescribeStacks(cliConfig, "", nil, nil, nil, false, true)
assert.Nil(t, err)

assert.Greater(t, len(stacksWithEmpty), initialCount, "Should have more stacks when including empty ones")

var (
emptyStacks []string
nonEmptyStacks []string
)

for stackName, stackContent := range stacksWithEmpty {
if stack, ok := stackContent.(map[string]any); ok {
if components, hasComponents := stack["components"].(map[string]any); hasComponents {
// Check for completely empty components
if len(components) == 0 || (len(components) == 1 && len(components["terraform"].(map[string]any)) == 0) {
emptyStacks = append(emptyStacks, stackName)
continue
}

// If we have any components at all, consider it non-empty
for _, compType := range components {
if compMap, ok := compType.(map[string]any); ok && len(compMap) > 0 {
nonEmptyStacks = append(nonEmptyStacks, stackName)
break
}
}
}
}
}

// Verify we found both types of stacks
assert.NotEmpty(t, emptyStacks, "Should find at least one empty stack")
assert.NotEmpty(t, nonEmptyStacks, "Should find at least one non-empty stack")
}