feat: Adding support for --filter in stack generate
#5073
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR restructures stack generation and output functionality by extracting them into dedicated internal packages ( Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant Discovery
participant Generate as internal/stacks/generate
participant Output as internal/stacks/output
participant Config as config
User->>CLI: stack generate --filter "type=stack"
CLI->>Discovery: NewForStackGenerate(opts)
Discovery->>Discovery: Parse filter queries, RestrictToStacks()
CLI->>Generate: GenerateStacks(ctx, log, opts)
Generate->>Generate: ListStackFilesMaybeWithDiscovery()
Generate->>Generate: BuildStackTopology(stackFiles)
loop Per Level
Generate->>Generate: generateLevel(levelNodes)
Generate->>Config: GenerateStackFile(ctx, opts, stackFile)
Generate->>Generate: discoverAndAddNewNodes()
end
User->>CLI: stack output
CLI->>Output: StackOutput(ctx, log, opts)
Output->>Generate: ListStackFilesMaybeWithDiscovery()
loop Per Stack File
Output->>Config: ReadStackValues(ctx, stackFile)
Output->>Config: ReadUnitOutputs(ctx, unit)
end
Output->>Config: GoTypeToCty(outputMap)
Output->>CLI: cty.Value result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
yhakbar
force-pushed
the
feat/adding-support-for-filter-in-stack-generate
branch
from
1c407a2 to
2272053
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/filter/filters.go (1)
105-114: Consider a simpler implementation for better readability.The current implementation using
slices.Collectwith an iterator function is correct but may be harder to understand for developers unfamiliar with Go 1.23's iterator patterns. A more straightforward approach would improve code clarity:// RestrictToStacks returns a new Filters object with only the filters that are restricted to stacks. func (f Filters) RestrictToStacks() Filters { - return slices.Collect(func(yield func(*Filter) bool) { - for _, filter := range f { - if filter.expr.IsRestrictedToStacks() && !yield(filter) { - return - } - } - }) + result := make(Filters, 0, len(f)) + for _, filter := range f { + if filter.expr.IsRestrictedToStacks() { + result = append(result, filter) + } + } + return result }This alternative is more explicit, easier to read, and has predictable performance characteristics with pre-allocated capacity.
test/integration_stacks_test.go (1)
1598-1637: Verify the test name aligns with what's being tested.The test name includes "WithRacing" but there are no explicit concurrent operations or race condition tests visible in the implementation. The test primarily validates:
- Nested stack generation produces correct topology
- Level counts are accurate (1 root, 3 mid-level, 9 leaf)
- Generated units are correct
- Re-running generation in a dirty directory succeeds
If "Racing" refers to concurrent operations within
stack generateinternals, consider clarifying this in a comment. Otherwise, consider renaming toTestStackGenerationWithNestedTopologyfor clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
cli/commands/run/cli.go(2 hunks)cli/commands/stack/stack.go(3 hunks)config/config.go(1 hunks)config/config_as_cty.go(31 hunks)config/config_helpers.go(1 hunks)config/config_partial.go(2 hunks)config/cty_helpers.go(2 hunks)config/exclude.go(1 hunks)config/feature_flag.go(4 hunks)config/locals.go(1 hunks)config/stack.go(4 hunks)config/stack_test.go(0 hunks)internal/discovery/constructor.go(2 hunks)internal/filter/ast.go(9 hunks)internal/filter/ast_test.go(1 hunks)internal/filter/filters.go(2 hunks)internal/filter/filters_test.go(1 hunks)internal/stacks/generate/generate.go(1 hunks)internal/stacks/output/output.go(1 hunks)test/fixtures/stacks/nested/live/terragrunt.stack.hcl(0 hunks)test/integration_stacks_test.go(4 hunks)
💤 Files with no reviewable changes (2)
- test/fixtures/stacks/nested/live/terragrunt.stack.hcl
- config/stack_test.go
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
⚙️ CodeRabbit configuration file
Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.
Files:
internal/discovery/constructor.gointernal/filter/filters.gocli/commands/stack/stack.goconfig/config_helpers.goconfig/stack.goconfig/config_partial.gointernal/stacks/output/output.goconfig/exclude.goconfig/locals.gointernal/filter/ast.goconfig/config.gointernal/filter/ast_test.gocli/commands/run/cli.gointernal/stacks/generate/generate.goconfig/feature_flag.gointernal/filter/filters_test.gotest/integration_stacks_test.goconfig/cty_helpers.goconfig/config_as_cty.go
🧠 Learnings (6)
📚 Learning: 2025-02-10T13:36:19.542Z
Learnt from: levkohimins
Repo: gruntwork-io/terragrunt PR: 3723
File: cli/commands/stack/action.go:160-160
Timestamp: 2025-02-10T13:36:19.542Z
Learning: The project uses a custom error package `github.com/gruntwork-io/terragrunt/internal/errors` which provides similar functionality to `fmt.Errorf` but includes stack traces. Prefer using this package's error functions (e.g., `errors.Errorf`, `errors.New`) over the standard library's error handling.
Applied to files:
internal/filter/filters.gocli/commands/stack/stack.goconfig/stack.gocli/commands/run/cli.gotest/integration_stacks_test.go
📚 Learning: 2025-04-17T13:02:28.098Z
Learnt from: yhakbar
Repo: gruntwork-io/terragrunt PR: 4169
File: cli/commands/hcl/validate/cli.go:29-60
Timestamp: 2025-04-17T13:02:28.098Z
Learning: Avoid shadowing imported packages with local variables in Go code, such as using a variable named `flags` when the `github.com/gruntwork-io/terragrunt/cli/flags` package is imported. Use more specific variable names like `flagSet` instead.
Applied to files:
cli/commands/stack/stack.goconfig/stack.gocli/commands/run/cli.goconfig/feature_flag.go
📚 Learning: 2025-08-19T16:05:54.723Z
Learnt from: Resonance1584
Repo: gruntwork-io/terragrunt PR: 4683
File: go.mod:86-90
Timestamp: 2025-08-19T16:05:54.723Z
Learning: When analyzing Go module dependencies for removal, always check for both direct imports and API usage across all Go files in the repository, not just a quick search. The github.com/mattn/go-zglob library is used for filesystem walking and glob expansion in multiple Terragrunt files including util/file.go, format commands, and AWS provider patch functionality.
Applied to files:
cli/commands/stack/stack.goconfig/stack.gocli/commands/run/cli.gointernal/stacks/generate/generate.gotest/integration_stacks_test.go
📚 Learning: 2025-02-10T23:20:04.295Z
Learnt from: yhakbar
Repo: gruntwork-io/terragrunt PR: 3868
File: docs-starlight/patches/@astrojs%[email protected]:33-33
Timestamp: 2025-02-10T23:20:04.295Z
Learning: In Terragrunt projects, all `.hcl` files can be assumed to be Terragrunt configurations by default, with specific exceptions like `.terraform.lock.hcl` that need explicit handling.
Applied to files:
config/stack.go
📚 Learning: 2025-11-03T04:40:01.000Z
Learnt from: ThisGuyCodes
Repo: gruntwork-io/terragrunt PR: 5041
File: test/fixtures/hclvalidate/valid/duplicate-attributes-in-required-providers/main.tf:2-7
Timestamp: 2025-11-03T04:40:01.000Z
Learning: In the terragrunt repository, test fixtures under test/fixtures/hclvalidate/valid/ are intentionally testing that INPUT validation succeeds even when Terraform code contains syntax errors or other issues unrelated to input validation (e.g., duplicate attributes, circular references, invalid locals). The "valid" designation means "valid for input validation purposes" not "syntactically valid Terraform/OpenTofu code".
Applied to files:
test/integration_stacks_test.go
📚 Learning: 2025-03-06T23:44:09.413Z
Learnt from: partcyborg
Repo: gruntwork-io/terragrunt PR: 3974
File: config/config_partial.go:448-456
Timestamp: 2025-03-06T23:44:09.413Z
Learning: The TerragruntConfig struct in config/config.go does contain an Engine field that's used to store engine configuration data.
Applied to files:
config/config_as_cty.go
🧬 Code graph analysis (17)
internal/discovery/constructor.go (3)
internal/experiment/experiment.go (2)
Experiments(46-46)FilterFlag(36-36)internal/discovery/discovery.go (1)
Discovery(83-167)internal/filter/filters.go (1)
ParseFilterQueries(21-48)
internal/filter/filters.go (1)
internal/filter/filter.go (1)
Filter(9-13)
cli/commands/stack/stack.go (2)
internal/stacks/generate/generate.go (1)
GenerateStacks(47-86)internal/stacks/output/output.go (1)
StackOutput(52-180)
config/config_helpers.go (1)
config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
config/stack.go (3)
internal/worker/worker.go (1)
Pool(33-46)config/config.go (1)
DefaultStackFile(49-49)config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
config/config_partial.go (1)
config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
internal/stacks/output/output.go (7)
options/options.go (1)
TerragruntOptions(99-329)pkg/log/log.go (2)
Debugf(72-74)Warnf(92-94)internal/stacks/generate/generate.go (1)
ListStackFilesMaybeWithDiscovery(254-288)config/stack.go (6)
Unit(56-63)StackConfig(49-53)ReadValues(545-578)ReadStackConfigFile(396-411)StackDir(33-33)GetUnitDir(698-704)telemetry/context.go (1)
TelemeterFromContext(23-31)config/config_as_cty.go (1)
GoTypeToCty(804-833)config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
config/exclude.go (1)
config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
config/locals.go (1)
config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
config/config.go (1)
config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
internal/filter/ast_test.go (1)
internal/filter/ast.go (6)
Expression(11-24)NewPathFilter(36-38)NewAttributeFilter(74-76)NewPrefixExpression(137-139)NewInfixExpression(176-178)NewGraphExpression(224-236)
cli/commands/run/cli.go (1)
internal/stacks/generate/generate.go (1)
GenerateStacks(47-86)
internal/stacks/generate/generate.go (7)
options/options.go (1)
TerragruntOptions(99-329)internal/worker/worker.go (1)
NewWorkerPool(49-63)util/file.go (2)
FileExists(61-64)WalkDirWithSymlinks(1046-1125)config/stack.go (2)
GenerateStackFile(78-102)Stack(66-73)config/config.go (1)
DefaultStackFile(49-49)internal/experiment/experiment.go (3)
Experiments(46-46)FilterFlag(36-36)Symlinks(18-18)internal/discovery/constructor.go (2)
NewForStackGenerate(133-150)StackGenerateOptions(36-40)
config/feature_flag.go (1)
config/config_as_cty.go (1)
GoTypeToCty(804-833)
internal/filter/filters_test.go (1)
internal/filter/filters.go (1)
ParseFilterQueries(21-48)
test/integration_stacks_test.go (5)
test/helpers/package.go (5)
RunTerragruntCommandWithOutput(1007-1011)CleanupTerraformFolder(882-889)CopyEnvironment(89-105)CreateGitRepo(1103-1108)RunTerragrunt(979-983)test/helpers/test_helpers.go (1)
IsExperimentMode(65-71)test/helpers/logger/logger.go (1)
CreateLogger(9-14)internal/stacks/generate/generate.go (1)
BuildStackTopology(129-141)config/config.go (2)
DefaultStackFile(49-49)DefaultTerragruntConfigPath(48-48)
config/config_as_cty.go (1)
config/cty_helpers.go (1)
ConvertValuesMapToCtyVal(233-246)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: license_check / License Check
- GitHub Check: Pull Request has non-contributor approval
- GitHub Check: build-and-test
🔇 Additional comments (22)
internal/filter/ast.go (3)
22-23: LGTM!The new
IsRestrictedToStacks()method is a clean extension to the Expression interface that enables filtering logic to identify stack-specific filters.
73-76: LGTM!The public constructors provide a consistent API for creating AST nodes and encapsulate the struct initialization details. This is good practice for maintainability and testability.
Also applies to: 136-139, 175-178, 223-236
126-128: LGTM!The
IsRestrictedToStacks()implementations are logically correct:
AttributeFilter: correctly identifiestype=stackas stack-restrictedPrefixExpression: correctly identifies negated non-stack types (e.g.,!type=unit) as stack-restrictedInfixExpression: correctly applies OR semantics for the|operatorAlso applies to: 149-166, 206-213
cli/commands/stack/stack.go (1)
16-17: LGTM!The refactoring to use dedicated
internal/stacks/generateandinternal/stacks/outputpackages improves code organization without altering functionality.Also applies to: 50-50, 82-82
cli/commands/run/cli.go (1)
17-17: LGTM!Consistent refactoring to use the relocated
generate.GenerateStacksfunction from the dedicated internal package.Also applies to: 150-150
config/config.go (1)
2045-2045: LGTM!The update to use the exported
ConvertValuesMapToCtyValfunction is consistent with the broader refactoring to make internal helpers publicly available across the config package.config/feature_flag.go (1)
99-99: LGTM!All references updated to use the exported
GoTypeToCtyfunction, maintaining consistency with the broader refactoring effort.Also applies to: 108-108, 125-125, 135-135
config/config_partial.go (1)
177-177: LGTM!The updates to use
ConvertValuesMapToCtyValin bothDecodeBaseBlocksandflagsAsCtyare consistent with the package-wide refactoring to export helper functions.Also applies to: 219-219
internal/filter/ast_test.go (1)
10-72: LGTM!Excellent test coverage for the new
IsRestrictedToStacks()functionality. The test cases comprehensively cover all expression types and important scenarios:
- Path and attribute filters
- Negation semantics (e.g.,
!type=unitcorrectly identified as stack-restricted)- Union operations with stack filters on both left and right sides
- Graph expressions
The table-driven approach with parallel execution is idiomatic and efficient.
config/exclude.go (1)
134-134: LGTM! Function call updated to use exported variant.The change from
convertValuesMapToCtyValtoConvertValuesMapToCtyValaligns with the refactor to export this helper function.internal/filter/filters_test.go (1)
513-555: LGTM! Comprehensive test coverage for stack restriction feature.The test suite covers the key scenarios for
RestrictToStacks():
- Empty filters edge case
- Single and multiple filter cases
- Verification that only
type=stackfilters are retainedTest structure follows established patterns in the file.
config/config_helpers.go (1)
1228-1228: LGTM! Function call updated to use exported variant.Consistent with the refactoring to export
ConvertValuesMapToCtyValacross the config package.config/locals.go (1)
109-109: LGTM! Function call updated to use exported variant.The change aligns with the broader refactor to export conversion utilities across the config package.
internal/discovery/constructor.go (1)
132-150: LGTM! Well-structured constructor for stack generation.The
NewForStackGenerateconstructor correctly:
- Gates filter support behind the experiment flag
- Parses filter queries with proper error handling
- Restricts filters to stack-type components only via
RestrictToStacks()(line 145)The use of
RestrictToStacks()ensures that only filters matching stacks are applied during stack generation, which is the expected behavior for this command.config/cty_helpers.go (2)
232-246: LGTM! Function exported with correct capitalization.The function is now exported as
ConvertValuesMapToCtyValwith its documentation updated. The implementation remains unchanged, preserving existing behavior while making it accessible to other packages.
289-289: LGTM! Call site updated to use exported function name.config/config_as_cty.go (2)
85-85: LGTM! Systematic refactor to use exported conversion functions.All call sites updated consistently from:
goTypeToCty→GoTypeToCtyconvertValuesMapToCtyVal→ConvertValuesMapToCtyValThe refactor makes these utilities accessible to external packages without changing behavior.
Also applies to: 107-107, 116-116, 163-163, 257-257, 269-269, 287-287, 296-296, 309-309, 327-327, 336-336, 345-345, 366-366, 375-375, 393-393, 405-405
802-833: LGTM! Function exported and documented correctly.The
GoTypeToCtyfunction is now exported with proper capitalization. The implementation correctly handles:
- Map types with recursive conversion
- Other types via gocty.ImpliedType
- Error propagation throughout
test/integration_stacks_test.go (3)
114-117: LGTM! Formatting improvements for readability.Multi-line function calls improve readability with no behavior change.
Also applies to: 477-480
1537-1596: LGTM! Comprehensive filter integration test.The test properly validates the new
--filterflag functionality:
- Correctly skips when experiment mode is disabled
- Tests selective stack generation (dev vs. prod)
- Cleans up between test phases to ensure isolation
- Verifies both positive and negative cases (what is generated and what is not)
The test structure follows established patterns in the file.
1639-1709: LGTM! Well-structured test helpers.The helper functions are well-designed:
setupNestedStackFixture: Creates a realistic 3-level nested stack structure
- 1 live stack → 3 foo stacks → 9 final stacks/units
verifyGeneratedUnits: Validates expected counts (9 units, 12 stacks)
- Counts match the structure: 3 foo + 9 final = 12 stacks
findStackFiles: Simple recursive file finderThe fixture structure and assertions are consistent with the expected nested topology.
internal/stacks/generate/generate.go (1)
66-83: Fix the level loop so the code compiles
for level := range maxLeveldoes not compile—rangecannot iterate over anint. This currently breaks the build.Apply this diff to use a normal counted loop and keep the cycle detection:
- const maxLevel = 1024 - for level := range maxLevel { + const maxLevel = 1024 + for level := 0; level < maxLevel; level++ { if level == maxLevel-1 { return errors.Errorf("Cycle detected: maximum level (%d) exceeded", maxLevel) }⛔ Skipped due to learnings
Learnt from: yhakbar Repo: gruntwork-io/terragrunt PR: 3723 File: internal/cloner/cloner.go:163-0 Timestamp: 2025-02-06T21:13:52.071Z Learning: Go supports iterating over integers using range syntax (e.g. `for i := range n`). This is valid syntax and should not be flagged as an error.Learnt from: yhakbar Repo: gruntwork-io/terragrunt PR: 3723 File: internal/cloner/cloner.go:163-0 Timestamp: 2025-02-06T21:13:52.071Z Learning: Go 1.22 and later versions support iterating over integers using range syntax (e.g. `for i := range n`). This feature was introduced in February 2024 and provides a cleaner alternative to traditional `for i := 0; i < n; i++` loops.Learnt from: yhakbar Repo: gruntwork-io/terragrunt PR: 4034 File: cli/commands/list/action.go:636-642 Timestamp: 2025-03-19T11:38:58.959Z Learning: Go 1.22 (released February 2024) introduced the ability to use `for range` with integers. The syntax `for range N { ... }` iterates N times (from 0 to N-1), providing a cleaner way to repeat operations a specific number of times without an explicit counter variable.Learnt from: yhakbar Repo: gruntwork-io/terragrunt PR: 4034 File: cli/commands/list/action.go:636-642 Timestamp: 2025-03-19T11:38:58.959Z Learning: Go 1.22 (released February 2024) introduced the ability to use `for range` with integers. The syntax `for range N { ... }` iterates from 0 to N-1, providing a cleaner way to repeat operations a specific number of times without an explicit counter variable.Learnt from: yhakbar Repo: gruntwork-io/terragrunt PR: 3723 File: internal/cloner/cloner.go:176-0 Timestamp: 2025-02-06T21:12:49.153Z Learning: Using `for i := range n` is valid Go syntax for iterating over integers, where n is the upper bound (exclusive) of the range. This is equivalent to `for i := 0; i < n; i++` but uses newer Go syntax.Learnt from: yhakbar Repo: gruntwork-io/terragrunt PR: 4070 File: internal/remotestate/backend/gcs/client.go:274-284 Timestamp: 2025-03-24T14:42:18.517Z Learning: The syntax `for retries := range maxRetriesWaitingForGcsBucket` is valid in the Terragrunt codebase. While not standard Go, the pattern `for x := range someInt` is an accepted idiom in this project.
| for _, node := range levelNodes { | ||
| if generatedFiles[node.FilePath] { | ||
| continue | ||
| } | ||
|
|
||
| generatedFiles[node.FilePath] = true | ||
|
|
||
| // Before attempting to generate the stack file, we need to double-check that the file exists. | ||
| // Generation at a higher level might have resulted in this file being removed. | ||
| if !util.FileExists(node.FilePath) { | ||
| continue | ||
| } | ||
|
|
||
| wp.Submit(func() error { | ||
| return config.GenerateStackFile(ctx, l, opts, wp, node.FilePath) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Avoid reusing the same worker pool for nested submissions
Each stack task uses wp to submit more work (GenerateStackFile calls pool.Submit). When Parallelism is 1 (or otherwise saturated), the worker executing the stack job blocks forever trying to submit to an already-full semaphore, deadlocking the whole command.
Apply this diff to give each stack its own pool and wait on it after scheduling the nested work:
- wp.Submit(func() error {
- return config.GenerateStackFile(ctx, l, opts, wp, node.FilePath)
- })
+ wp.Submit(func() error {
+ stackPool := worker.NewWorkerPool(opts.Parallelism)
+ defer stackPool.Stop()
+
+ if err := config.GenerateStackFile(ctx, l, opts, stackPool, node.FilePath); err != nil {
+ return err
+ }
+
+ return stackPool.Wait()
+ })🤖 Prompt for AI Agents
internal/stacks/generate/generate.go lines 95-111: the code reuses the outer
worker pool `wp` when submitting nested work inside GenerateStackFile, which
deadlocks if the outer pool is saturated (e.g., Parallelism=1); create a new
worker pool for each stack (with the desired parallelism), pass that new pool
into config.GenerateStackFile instead of the shared `wp`, submit nested tasks to
that pool, and wait on/close that new pool after scheduling the nested work to
ensure the nested tasks complete and avoid leaking goroutines.
| for stackPath, stackName := range declaredStacks { | ||
| if strings.Contains(path, stackPath) { | ||
| stackNames = append(stackNames, stackName) | ||
| nameToPath[stackName] = stackPath | ||
| } | ||
| } | ||
|
|
||
| // Sort stackNames based on the length of stackPath to ensure correct order | ||
| stackNamesSorted := make([]string, len(stackNames)) | ||
| copy(stackNamesSorted, stackNames) | ||
|
|
||
| for i := range stackNamesSorted { | ||
| for j := i + 1; j < len(stackNamesSorted); j++ { | ||
| // Compare lengths of the actual paths from the nameToPath map, not the declaredStacks lookup | ||
| if len(nameToPath[stackNamesSorted[i]]) < len(nameToPath[stackNamesSorted[j]]) { | ||
| stackNamesSorted[i], stackNamesSorted[j] = stackNamesSorted[j], stackNamesSorted[i] | ||
| } | ||
| } | ||
| } | ||
|
|
||
| stackKey := unit.Name | ||
| if len(stackNamesSorted) > 0 { | ||
| stackKey = strings.Join(stackNamesSorted, ".") + "." + unit.Name | ||
| } |
There was a problem hiding this comment.
Fix stack ancestry detection and ordering
Using strings.Contains to decide ancestry causes false positives (e.g., /foo/bar2 matches /foo/bar), and the manual sort currently outputs child-first keys (child.parent.unit) instead of the documented parent.child.unit. Both issues break the hierarchy in the aggregated outputs.
Apply this diff to tighten the parent detection and sort parents from shallow to deep; remember to add the sort package to the imports:
- for stackPath, stackName := range declaredStacks {
- if strings.Contains(path, stackPath) {
- stackNames = append(stackNames, stackName)
- nameToPath[stackName] = stackPath
- }
- }
-
- // Sort stackNames based on the length of stackPath to ensure correct order
- stackNamesSorted := make([]string, len(stackNames))
- copy(stackNamesSorted, stackNames)
-
- for i := range stackNamesSorted {
- for j := i + 1; j < len(stackNamesSorted); j++ {
- // Compare lengths of the actual paths from the nameToPath map, not the declaredStacks lookup
- if len(nameToPath[stackNamesSorted[i]]) < len(nameToPath[stackNamesSorted[j]]) {
- stackNamesSorted[i], stackNamesSorted[j] = stackNamesSorted[j], stackNamesSorted[i]
- }
- }
- }
-
- stackKey := unit.Name
- if len(stackNamesSorted) > 0 {
- stackKey = strings.Join(stackNamesSorted, ".") + "." + unit.Name
- }
+ for stackPath, stackName := range declaredStacks {
+ rel, err := filepath.Rel(stackPath, path)
+ if err != nil {
+ continue
+ }
+
+ if rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) {
+ stackNames = append(stackNames, stackName)
+ nameToPath[stackName] = stackPath
+ }
+ }
+
+ sort.Slice(stackNames, func(i, j int) bool {
+ return len(nameToPath[stackNames[i]]) < len(nameToPath[stackNames[j]])
+ })
+
+ stackKey := unit.Name
+ if len(stackNames) > 0 {
+ stackKey = strings.Join(stackNames, ".") + "." + unit.Name
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for stackPath, stackName := range declaredStacks { | |
| if strings.Contains(path, stackPath) { | |
| stackNames = append(stackNames, stackName) | |
| nameToPath[stackName] = stackPath | |
| } | |
| } | |
| // Sort stackNames based on the length of stackPath to ensure correct order | |
| stackNamesSorted := make([]string, len(stackNames)) | |
| copy(stackNamesSorted, stackNames) | |
| for i := range stackNamesSorted { | |
| for j := i + 1; j < len(stackNamesSorted); j++ { | |
| // Compare lengths of the actual paths from the nameToPath map, not the declaredStacks lookup | |
| if len(nameToPath[stackNamesSorted[i]]) < len(nameToPath[stackNamesSorted[j]]) { | |
| stackNamesSorted[i], stackNamesSorted[j] = stackNamesSorted[j], stackNamesSorted[i] | |
| } | |
| } | |
| } | |
| stackKey := unit.Name | |
| if len(stackNamesSorted) > 0 { | |
| stackKey = strings.Join(stackNamesSorted, ".") + "." + unit.Name | |
| } | |
| for stackPath, stackName := range declaredStacks { | |
| rel, err := filepath.Rel(stackPath, path) | |
| if err != nil { | |
| continue | |
| } | |
| if rel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator))) { | |
| stackNames = append(stackNames, stackName) | |
| nameToPath[stackName] = stackPath | |
| } | |
| } | |
| sort.Slice(stackNames, func(i, j int) bool { | |
| return len(nameToPath[stackNames[i]]) < len(nameToPath[stackNames[j]]) | |
| }) | |
| stackKey := unit.Name | |
| if len(stackNames) > 0 { | |
| stackKey = strings.Join(stackNames, ".") + "." + unit.Name | |
| } |
🤖 Prompt for AI Agents
internal/stacks/output/output.go around lines 136 to 159: the current logic uses
strings.Contains to detect stack ancestry (causing false positives like
/foo/bar2 matching /foo/bar) and the manual swap sort produces child-first
ordering; replace the Contains check with a proper prefix/path check (e.g.,
strings.HasPrefix or path-aware comparison ensuring stackPath is a directory
ancestor of path) when collecting stackNames and nameToPath, then sort
stackNames by increasing length of their corresponding nameToPath entries
(shallow → deep) using the sort package so the final stackKey becomes
parent.child.unit (add import "sort").
|
I deferred addressing the notes from CodeRabbit to #5083 , as this PR doesn't introduce that logic, it just moves it to an internal package. |
| defaultStackFile = "terragrunt.stack.hcl" | ||
| unitDirPerm = 0755 | ||
| valueFilePerm = 0644 | ||
| generationMaxPath = 1024 |
There was a problem hiding this comment.
Looks like generationMaxPath is not used
There was a problem hiding this comment.
Will look to get rid of that in the future.
Description
Adds support for
--filterinstack generate.TODOs
Read the Gruntwork contribution guidelines.
Release Notes (draft)
Added / Removed / Updated [X].
Migration Guide
Summary by CodeRabbit
Release Notes
New Features
Refactor