Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/exec/describe_affected_deleted.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func processAllComponentsAsDeleted(
var deleted []schema.Affected

// Process each component type.
for _, componentType := range []string{cfg.TerraformComponentType, cfg.HelmfileComponentType, cfg.PackerComponentType} {
for _, componentType := range componentSectionSearchOrder() {
componentTypeSection, ok := remoteComponentsSection[componentType].(map[string]any)
if !ok {
continue
Expand Down Expand Up @@ -164,7 +164,7 @@ func processDeletedComponentsInStack(
var deleted []schema.Affected

// Process each component type.
for _, componentType := range []string{cfg.TerraformComponentType, cfg.HelmfileComponentType, cfg.PackerComponentType} {
for _, componentType := range componentSectionSearchOrder() {
remoteTypeSection, ok := remoteComponentsSection[componentType].(map[string]any)
if !ok {
continue
Expand Down
49 changes: 49 additions & 0 deletions internal/exec/describe_affected_deleted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,55 @@ func TestDetectDeletedComponents_ComponentDeleted(t *testing.T) {
assert.Contains(t, deleted[0].AffectedAll, affectedReasonDeleted)
}

// TestDetectDeletedComponents_HelmAndKubernetesComponentDeleted guards against
// helm/kubernetes deletions going undetected (both were previously omitted
// from the hardcoded [terraform, helmfile, packer] search list).
func TestDetectDeletedComponents_HelmAndKubernetesComponentDeleted(t *testing.T) {
registerFakeComponentTypes(t, cfg.HelmComponentType, cfg.KubernetesComponentType)

atmosConfig := &schema.AtmosConfiguration{}

remoteStacks := map[string]any{
"dev-us-east-1": map[string]any{
"components": map[string]any{
cfg.HelmComponentType: map[string]any{
"nginx-ingress": map[string]any{
"vars": map[string]any{"replicas": 3},
},
},
cfg.KubernetesComponentType: map[string]any{
"cert-manager": map[string]any{
"vars": map[string]any{"namespace": "cert-manager"},
},
},
},
},
}

// Both deleted in HEAD.
currentStacks := map[string]any{
"dev-us-east-1": map[string]any{
"components": map[string]any{
cfg.HelmComponentType: map[string]any{},
cfg.KubernetesComponentType: map[string]any{},
},
},
}

deleted, err := detectDeletedComponents(&remoteStacks, &currentStacks, atmosConfig, "")
require.NoError(t, err)
require.Len(t, deleted, 2)

byComponent := make(map[string]schema.Affected, len(deleted))
for _, d := range deleted {
byComponent[d.Component] = d
}
require.Contains(t, byComponent, "nginx-ingress")
require.Contains(t, byComponent, "cert-manager")
assert.Equal(t, cfg.HelmComponentType, byComponent["nginx-ingress"].ComponentType)
assert.Equal(t, cfg.KubernetesComponentType, byComponent["cert-manager"].ComponentType)
}

// TestDetectDeletedComponents_EntireStackDeleted tests detection when an entire stack is deleted.
func TestDetectDeletedComponents_EntireStackDeleted(t *testing.T) {
atmosConfig := &schema.AtmosConfiguration{}
Expand Down
18 changes: 18 additions & 0 deletions internal/exec/describe_affected_optimizations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2004,6 +2004,9 @@ func TestIsComponentFolderChangedCoverage(t *testing.T) {
Packer: schema.Packer{
BasePath: "components/packer",
},
Helm: schema.Helm{
BasePath: "components/helm",
},
},
}

Expand Down Expand Up @@ -2057,6 +2060,21 @@ func TestIsComponentFolderChangedCoverage(t *testing.T) {
assert.True(t, changed)
})

t.Run("helm component changed", func(t *testing.T) {
helmPath := filepath.Join(tempDir, "components/helm/nginx-ingress")
err := os.MkdirAll(helmPath, 0o755)
require.NoError(t, err)
helmFile := filepath.Join(helmPath, "values.yaml")
err = os.WriteFile(helmFile, []byte("replicas: 3"), 0o644)
require.NoError(t, err)

changedFiles := []string{helmFile}

changed, err := isComponentFolderChanged("nginx-ingress", cfg.HelmComponentType, atmosConfig, changedFiles)
require.NoError(t, err)
assert.True(t, changed)
})

t.Run("unsupported component type", func(t *testing.T) {
changedFiles := []string{}

Expand Down
2 changes: 2 additions & 0 deletions internal/exec/describe_affected_utils_2.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,8 @@ func isComponentFolderChanged(
componentPath = filepath.Join(atmosConfig.BasePath, atmosConfig.Components.Packer.BasePath, component)
case cfg.KubernetesComponentType:
componentPath = filepath.Join(atmosConfig.BasePath, atmosConfig.Components.Kubernetes.BasePath, component)
case cfg.HelmComponentType:
componentPath = filepath.Join(atmosConfig.BasePath, atmosConfig.Components.Helm.BasePath, component)
default:
return false, fmt.Errorf("%w: %s", errUtils.ErrUnsupportedComponentType, componentType)
}
Expand Down
19 changes: 19 additions & 0 deletions internal/exec/describe_component.go
Original file line number Diff line number Diff line change
Expand Up @@ -659,6 +659,25 @@ func FilterComputedFields(componentSection map[string]any) map[string]any {
"component": true,
"hooks": true,
"flags": true,
// Helm-specific sections (built-in types don't get container's pass-through;
// without these, `describe component` silently strips them under the default
// `describe.component.filter: schema` mode).
cfg.ChartSectionName: true,
cfg.ValuesSectionName: true,
cfg.ValuesFilesSectionName: true,
cfg.RepositoriesSectionName: true,
// Kubernetes-specific sections.
cfg.ProviderSectionName: true,
cfg.PathsSectionName: true,
cfg.ManifestsSectionName: true,
cfg.RenderSectionName: true,
// Cross-type sections (generate: terraform/kubernetes/helm; source: terraform/
// helmfile/packer/kubernetes/helm; provision: helm/kubernetes) — also previously
// missing from this whitelist for every type that defines them, not just CFN's
// future sections.
cfg.GenerateSectionName: true,
cfg.SourceSectionName: true,
cfg.ProvisionSectionName: true,
}

filtered := make(map[string]any)
Expand Down
49 changes: 47 additions & 2 deletions internal/exec/describe_component_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,8 +698,15 @@ func TestDescribeComponentWithProvenance(t *testing.T) {
// Filter computed fields
filtered := FilterComputedFields(result.ComponentSection)

// Verify filtered section only has stack-defined fields
allowedFields := []string{"vars", "settings", "env", "backend", "metadata", "overrides", "providers", "imports", "dependencies", "provision", "component", "hooks"}
// Verify filtered section only has stack-defined fields. Mirrors FilterComputedFields'
// fieldsToKeep exactly so this doesn't drift out of sync as more type-specific
// sections (helm/kubernetes) are added to the whitelist.
allowedFields := []string{
"vars", "settings", "env", "backend", "metadata", "overrides", "providers", "imports",
"dependencies", "component", "hooks",
"chart", "values", "values_files", "repositories",
"provider", "paths", "manifests", "render", "generate", "source", "provision",
}
for k := range filtered {
assert.Contains(t, allowedFields, k, "Filtered component section should only contain stack-defined fields")
}
Expand Down Expand Up @@ -829,6 +836,44 @@ func TestFilterComputedFields(t *testing.T) {
input: nil,
expected: map[string]any{},
},
{
name: "Keeps helm-specific fields",
input: map[string]any{
"chart": "some-chart",
"values": map[string]any{"key": "value"},
"values_files": []string{"values.yaml"},
"repositories": []any{map[string]any{"name": "repo"}},
"atmos_component": "test-component",
},
expected: map[string]any{
"chart": "some-chart",
"values": map[string]any{"key": "value"},
"values_files": []string{"values.yaml"},
"repositories": []any{map[string]any{"name": "repo"}},
},
},
{
name: "Keeps kubernetes-specific fields",
input: map[string]any{
"provider": "kubectl",
"paths": []string{"manifests/"},
"manifests": map[string]any{"key": "value"},
"render": map[string]any{"output": "yaml"},
"generate": map[string]any{"enabled": true},
"source": map[string]any{"uri": "github.com/acme/manifests"},
"provision": map[string]any{"default": "cluster"},
"atmos_component": "test-component",
},
expected: map[string]any{
"provider": "kubectl",
"paths": []string{"manifests/"},
"manifests": map[string]any{"key": "value"},
"render": map[string]any{"output": "yaml"},
"generate": map[string]any{"enabled": true},
"source": map[string]any{"uri": "github.com/acme/manifests"},
"provision": map[string]any{"default": "cluster"},
},
},
}

for _, tt := range tests {
Expand Down
26 changes: 0 additions & 26 deletions internal/exec/describe_dependents.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,29 +649,3 @@ func hasDependencyEntries(depsSection map[string]any) bool {
}
return false
}

// findComponentSectionInCachedStacks extracts a component section from pre-computed stacks.
// Returns nil if the stack or component is not found (caller falls back to ExecuteDescribeComponent).
func findComponentSectionInCachedStacks(stacks map[string]any, stackName, componentName string) map[string]any {
stackSection, ok := stacks[stackName].(map[string]any)
if !ok {
return nil
}
componentsSection, ok := stackSection["components"].(map[string]any)
if !ok {
return nil
}
// Check terraform components (the common case).
if tfSection, ok := componentsSection["terraform"].(map[string]any); ok {
if comp, ok := tfSection[componentName].(map[string]any); ok {
return comp
}
}
// Check helmfile components.
if hfSection, ok := componentsSection["helmfile"].(map[string]any); ok {
if comp, ok := hfSection[componentName].(map[string]any); ok {
return comp
}
}
return nil
}
42 changes: 42 additions & 0 deletions internal/exec/describe_dependents_component_section.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package exec

import (
comp "github.com/cloudposse/atmos/pkg/component"
cfg "github.com/cloudposse/atmos/pkg/config"
)

// findComponentSectionInCachedStacks extracts a component section from pre-computed stacks.
// Returns nil if the stack or component is not found (caller falls back to ExecuteDescribeComponent).
func findComponentSectionInCachedStacks(stacks map[string]any, stackName, componentName string) map[string]any {
stackSection, ok := stacks[stackName].(map[string]any)
if !ok {
return nil
}
componentsSection, ok := stackSection["components"].(map[string]any)
if !ok {
return nil
}

for _, componentType := range componentSectionSearchOrder() {
typeSection, ok := componentsSection[componentType].(map[string]any)
if !ok {
continue
}
if compSection, ok := typeSection[componentName].(map[string]any); ok {
return compSection
}
}
return nil
}

// componentSectionSearchOrder returns every component-type section name to search when
// resolving a dependency's component config. Combines the legacy types that predate the
// component-provider registry (terraform/helmfile/packer, never registered via
// component.Register) with every dynamically registered provider type (helm, kubernetes,
// ansible, container, emulator, and any future type such as aws/cloudformation), so a new
// registered component type needs no additional touch here.
func componentSectionSearchOrder() []string {
types := []string{cfg.TerraformComponentType, cfg.HelmfileComponentType, cfg.PackerComponentType}
types = append(types, comp.ListTypes()...)
return types
}
83 changes: 83 additions & 0 deletions internal/exec/describe_dependents_index_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,57 @@
package exec

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

comp "github.com/cloudposse/atmos/pkg/component"
cfg "github.com/cloudposse/atmos/pkg/config"
"github.com/cloudposse/atmos/pkg/schema"
)

// fakeComponentProvider is a minimal comp.ComponentProvider stub used to
// populate the component registry in tests. Internal/exec's test binary
// cannot import the real pkg/component/{helm,kubernetes,...} packages to
// trigger their init()-time registration (those packages import
// internal/exec, which would be an import cycle), so tests that need to
// prove behavior generalizes over "whatever is registered" register a fake
// provider directly instead.
type fakeComponentProvider struct {
componentType string
}

func (f *fakeComponentProvider) GetType() string { return f.componentType }
func (f *fakeComponentProvider) GetGroup() string { return "Test" }
func (f *fakeComponentProvider) GetBasePath(_ *schema.AtmosConfiguration) string { return "" }

func (f *fakeComponentProvider) ListComponents(_ context.Context, _ string, _ map[string]any) ([]string, error) {
return nil, nil
}

func (f *fakeComponentProvider) ValidateComponent(_ map[string]any) error { return nil }
func (f *fakeComponentProvider) Execute(_ *comp.ExecutionContext) error { return nil }
func (f *fakeComponentProvider) GenerateArtifacts(_ *comp.ExecutionContext) error {
return nil
}
func (f *fakeComponentProvider) GetAvailableCommands() []string { return nil }

// registerFakeComponentTypes registers a fake provider for each given type in
// the shared component registry, and restores an empty registry via
// t.Cleanup. Callers must not run in parallel with other tests that touch
// the registry (none in this package call t.Parallel(), so sequential
// per-package test execution keeps this safe).
func registerFakeComponentTypes(t *testing.T, types ...string) {
t.Helper()
comp.Reset()
for _, typ := range types {
require.NoError(t, comp.Register(&fakeComponentProvider{componentType: typ}))
}
t.Cleanup(comp.Reset)
}

func TestBuildDependencyIndex_Empty(t *testing.T) {
idx := buildDependencyIndex(map[string]any{})
assert.Empty(t, idx, "empty stacks should produce empty index")
Expand Down Expand Up @@ -217,6 +260,46 @@ func TestFindComponentSectionInCachedStacks_Helmfile(t *testing.T) {
assert.Equal(t, "nginx", section["vars"].(map[string]any)["chart"])
}

// TestFindComponentSectionInCachedStacks_RegisteredProviderTypes guards
// against a dependency resolving to nothing for any component type
// registered via the component-provider registry (previously only
// terraform/helmfile were checked here).
func TestFindComponentSectionInCachedStacks_RegisteredProviderTypes(t *testing.T) {
registerFakeComponentTypes(t, cfg.HelmComponentType, cfg.KubernetesComponentType)
require.NotEmpty(t, comp.ListTypes(), "test setup should have registered fake provider types")

for _, componentType := range comp.ListTypes() {
stacks := map[string]any{
"dev-use1": map[string]any{
"components": map[string]any{
componentType: map[string]any{
"widget": map[string]any{
"vars": map[string]any{"marker": componentType},
},
},
},
},
}

section := findComponentSectionInCachedStacks(stacks, "dev-use1", "widget")
require.NotNil(t, section, "component type %q should resolve", componentType)
assert.Equal(t, componentType, section["vars"].(map[string]any)["marker"])
}
}

func TestComponentSectionSearchOrder_IncludesLegacyAndRegisteredTypes(t *testing.T) {
registerFakeComponentTypes(t, cfg.HelmComponentType, cfg.KubernetesComponentType)
require.NotEmpty(t, comp.ListTypes(), "test setup should have registered fake provider types")

order := componentSectionSearchOrder()
assert.Contains(t, order, cfg.TerraformComponentType)
assert.Contains(t, order, cfg.HelmfileComponentType)
assert.Contains(t, order, cfg.PackerComponentType)
for _, componentType := range comp.ListTypes() {
assert.Contains(t, order, componentType)
}
}

func TestFindComponentSectionInCachedStacks_InvalidStackSection(t *testing.T) {
// Stack section is not a map.
stacks := map[string]any{"bad": "not-a-map"}
Expand Down
Loading
Loading