Skip to content

Commit 5f5656b

Browse files
committed
fix: Preventing parse errors in stack generation in worktrees
Right now, users with negations in their filter expressions aren't preventing reading parsing from happening in worktrees during stack generation. This is because we aren't properly propagating the non-Git filters from the parent discovery down to child discoveries. This fix this by properly propagating the non-Git filters from the parent discovery down to child discoveries.
1 parent b1ca435 commit 5f5656b

4 files changed

Lines changed: 217 additions & 86 deletions

File tree

internal/discovery/phase_worktree.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,14 @@ func (p *WorktreePhase) discoverInWorktree(
196196
return nil, err
197197
}
198198

199+
// Propagate non-git filters from the parent discovery to the worktree sub-discovery.
200+
// Git expressions are excluded to avoid infinite recursion (the worktree phase is
201+
// already handling them). All other filters (path, attribute, negation) are included
202+
// so that exclusions and type constraints apply within sub-discoveries.
203+
allFilters := append(filters, discovery.filters.ExcludingGitFilters()...)
204+
199205
subDiscovery := NewDiscovery(wt.Path).
200-
WithFilters(filters).
206+
WithFilters(allFilters).
201207
WithDiscoveryContext(discoveryContext).
202208
WithNumWorkers(p.numWorkers)
203209

@@ -312,10 +318,12 @@ func (p *WorktreePhase) walkChangedStack(
312318
errs = make([]error, 0, 2) //nolint:mnd
313319
)
314320

321+
parentFilters := discovery.filters.ExcludingGitFilters()
322+
315323
discoveryGroup.Go(func() error {
316324
fromDiscovery := NewDiscovery(fromStack.Path()).
317325
WithDiscoveryContext(fromDiscoveryContext).
318-
WithFilters(filter.Filters{}).
326+
WithFilters(parentFilters).
319327
WithNumWorkers(p.numWorkers)
320328

321329
var fromDiscoveryErr error
@@ -343,7 +351,7 @@ func (p *WorktreePhase) walkChangedStack(
343351
discoveryGroup.Go(func() error {
344352
toDiscovery := NewDiscovery(toStack.Path()).
345353
WithDiscoveryContext(toDiscoveryContext).
346-
WithFilters(filter.Filters{}).
354+
WithFilters(parentFilters).
347355
WithNumWorkers(p.numWorkers)
348356

349357
var toDiscoveryErr error

internal/discovery/phase_worktree_integration_test.go

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2365,3 +2365,93 @@ unit "myapp" {
23652365
"Expected at least one unit to be discovered when a read file changes, "+
23662366
"but got no units. All components: %v", componentPaths)
23672367
}
2368+
2369+
// TestWorktreePhase_Integration_NegatedFiltersAppliedInWorktreeSubDiscoveries tests that
2370+
// negated path filters (e.g., from .terragrunt-filters) are applied within worktree
2371+
// sub-discoveries, not just during the final filter evaluation.
2372+
// This is a regression test for #5821: source catalog units in worktrees were being
2373+
// discovered and parsed despite being excluded by a negated path filter, because the
2374+
// worktree sub-discoveries did not receive the exclusion filters.
2375+
func TestWorktreePhase_Integration_NegatedFiltersAppliedInWorktreeSubDiscoveries(t *testing.T) {
2376+
t.Parallel()
2377+
2378+
tmpDir, runner := setupGitRepo(t)
2379+
2380+
// Create two units: one that should be discovered and one that should be excluded.
2381+
createUnit(t, tmpDir, "app", `# App unit`)
2382+
// This catalog unit references values.* — it's a template that only works
2383+
// when generated through a stack. Without the fix, the worktree sub-discovery
2384+
// tries to parse it and fails with "Unknown variable: values".
2385+
createUnit(t, tmpDir, "catalog/units/svc", `
2386+
locals {
2387+
environment = values.environment
2388+
}
2389+
`)
2390+
2391+
commitChanges(t, runner, "Initial commit")
2392+
2393+
// Modify both units
2394+
err := os.WriteFile(filepath.Join(tmpDir, "app", "terragrunt.hcl"), []byte(`# Modified app`), 0o644)
2395+
require.NoError(t, err)
2396+
2397+
err = os.WriteFile(filepath.Join(tmpDir, "catalog", "units", "svc", "terragrunt.hcl"), []byte(`
2398+
locals {
2399+
environment = values.environment
2400+
region = "us-east-1"
2401+
}
2402+
`), 0o644)
2403+
require.NoError(t, err)
2404+
2405+
commitChanges(t, runner, "Modify both units")
2406+
2407+
l := logger.CreateLogger()
2408+
2409+
// Parse filters: git expression + negated path that excludes catalog
2410+
filterQueries := []string{"[HEAD~1...HEAD]", "!./catalog/**"}
2411+
filters, parseErr := filter.ParseFilterQueries(l, filterQueries)
2412+
require.NoError(t, parseErr)
2413+
2414+
w, err := worktrees.NewWorktrees(t.Context(), l, worktrees.WorktreeOpts{
2415+
WorkingDir: tmpDir,
2416+
GitExpressions: filters.UniqueGitFilters(),
2417+
})
2418+
require.NoError(t, err)
2419+
2420+
t.Cleanup(func() {
2421+
cleanupErr := w.Cleanup(context.WithoutCancel(t.Context()), l)
2422+
require.NoError(t, cleanupErr)
2423+
})
2424+
2425+
opts := options.NewTerragruntOptions()
2426+
opts.WorkingDir = tmpDir
2427+
opts.RootWorkingDir = tmpDir
2428+
2429+
d := discovery.NewDiscovery(tmpDir).
2430+
WithDiscoveryContext(&component.DiscoveryContext{
2431+
WorkingDir: tmpDir,
2432+
Cmd: "plan",
2433+
}).
2434+
WithWorktrees(w).
2435+
WithRelationships().
2436+
WithFilters(filters)
2437+
2438+
components, err := d.Discover(t.Context(), l, opts)
2439+
require.NoError(t, err)
2440+
2441+
unitPaths := components.Filter(component.UnitKind).Paths()
2442+
2443+
worktreePair := w.WorktreePairs["[HEAD~1...HEAD]"]
2444+
require.NotEmpty(t, worktreePair)
2445+
2446+
toWorktree := worktreePair.ToWorktree.Path
2447+
2448+
// The app unit should be discovered (it's in the git diff and not excluded)
2449+
assert.Contains(t, unitPaths, filepath.Join(toWorktree, "app"),
2450+
"app unit should be discovered")
2451+
2452+
// The catalog unit should NOT be discovered (excluded by !./catalog/**)
2453+
for _, p := range unitPaths {
2454+
assert.NotContains(t, p, "catalog",
2455+
"catalog units should be excluded by the negated filter, but found: %s", p)
2456+
}
2457+
}

internal/filter/filters.go

Lines changed: 104 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,21 @@ func (f Filters) RequiresParse() (Expression, bool) {
9393
return nil, false
9494
}
9595

96+
// ExcludingGitFilters returns all filters that do not contain a git expression.
97+
// Git expressions are excluded because they are handled by the worktree phase
98+
// itself and would cause infinite recursion if propagated to sub-discoveries.
99+
func (f Filters) ExcludingGitFilters() Filters {
100+
result := make(Filters, 0, len(f))
101+
102+
for _, filter := range f {
103+
if !containsGitExpression(filter.expr) {
104+
result = append(result, filter)
105+
}
106+
}
107+
108+
return result
109+
}
110+
96111
// DependencyGraphExpressions returns all target expressions from graph expressions that require dependency traversal.
97112
func (f Filters) DependencyGraphExpressions() []Expression {
98113
targets := make([]Expression, 0, len(f))
@@ -139,73 +154,15 @@ func (f Filters) UniqueGitFilters() GitExpressions {
139154

140155
// RestrictToStacks returns a new Filters object with only the filters that are restricted to stacks.
141156
func (f Filters) RestrictToStacks() Filters {
142-
return slices.Collect(func(yield func(*Filter) bool) {
143-
for _, filter := range f {
144-
if filter.expr.IsRestrictedToStacks() && !yield(filter) {
145-
return
146-
}
147-
}
148-
})
149-
}
150-
151-
// collectGraphExpressionTargetsWithDependencies collects target expressions from GraphExpression nodes that have IncludeDependencies set.
152-
func collectGraphExpressionTargetsWithDependencies(expr Expression) []Expression {
153-
var targets []Expression
154-
155-
WalkExpressions(expr, func(e Expression) bool {
156-
if graphExpr, ok := e.(*GraphExpression); ok && graphExpr.IncludeDependencies {
157-
targets = append(targets, graphExpr.Target)
158-
}
159-
160-
return true
161-
})
162-
163-
return targets
164-
}
165-
166-
// collectGraphExpressionTargetsWithDependents collects target expressions from GraphExpression nodes that have IncludeDependents set.
167-
func collectGraphExpressionTargetsWithDependents(expr Expression) []Expression {
168-
var targets []Expression
169-
170-
WalkExpressions(expr, func(e Expression) bool {
171-
if graphExpr, ok := e.(*GraphExpression); ok && graphExpr.IncludeDependents {
172-
targets = append(targets, graphExpr.Target)
173-
}
174-
175-
return true
176-
})
177-
178-
return targets
179-
}
180-
181-
// collectWorktreeExpressions collects worktree expressions from GitExpression nodes.
182-
func collectWorktreeExpressions(expr Expression) []*GitExpression {
183-
var targets []*GitExpression
184-
185-
WalkExpressions(expr, func(e Expression) bool {
186-
if gitExpr, ok := e.(*GitExpression); ok {
187-
targets = append(targets, gitExpr)
188-
}
157+
result := make(Filters, 0, len(f))
189158

190-
return true
191-
})
192-
193-
return targets
194-
}
195-
196-
// collectGitReferences collects Git references from GitExpression nodes.
197-
func collectGitReferences(expr Expression) []string {
198-
var refs []string
199-
200-
WalkExpressions(expr, func(e Expression) bool {
201-
if gitExpr, ok := e.(*GitExpression); ok {
202-
refs = append(refs, gitExpr.FromRef, gitExpr.ToRef)
159+
for _, filter := range f {
160+
if filter.expr.IsRestrictedToStacks() {
161+
result = append(result, filter)
203162
}
163+
}
204164

205-
return true
206-
})
207-
208-
return refs
165+
return result
209166
}
210167

211168
// Evaluate applies all filters with union (OR) semantics in two phases:
@@ -307,6 +264,37 @@ func (f Filters) EvaluateOnFiles(l log.Logger, files []string, workingDir string
307264
return f.Evaluate(l, comps)
308265
}
309266

267+
// String returns a JSON array representation of all filter strings.
268+
func (f Filters) String() string {
269+
filterStrings := make([]string, len(f))
270+
for i, filter := range f {
271+
filterStrings[i] = filter.String()
272+
}
273+
274+
jsonBytes, err := json.Marshal(filterStrings)
275+
if err != nil {
276+
return "[]"
277+
}
278+
279+
return string(jsonBytes)
280+
}
281+
282+
// containsGitExpression returns true if the expression tree contains a GitExpression.
283+
func containsGitExpression(expr Expression) bool {
284+
found := false
285+
286+
WalkExpressions(expr, func(e Expression) bool {
287+
if _, ok := e.(*GitExpression); ok {
288+
found = true
289+
return false
290+
}
291+
292+
return true
293+
})
294+
295+
return found
296+
}
297+
310298
func initialComponents(l log.Logger, positiveFilters []*Filter, components component.Components) (component.Components, error) {
311299
if len(positiveFilters) == 0 {
312300
return components, nil
@@ -333,17 +321,58 @@ func initialComponents(l log.Logger, positiveFilters []*Filter, components compo
333321
return remaining, nil
334322
}
335323

336-
// String returns a JSON array representation of all filter strings.
337-
func (f Filters) String() string {
338-
filterStrings := make([]string, len(f))
339-
for i, filter := range f {
340-
filterStrings[i] = filter.String()
341-
}
324+
func collectGraphExpressionTargetsWithDependencies(expr Expression) []Expression {
325+
var targets []Expression
342326

343-
jsonBytes, err := json.Marshal(filterStrings)
344-
if err != nil {
345-
return "[]"
346-
}
327+
WalkExpressions(expr, func(e Expression) bool {
328+
if graphExpr, ok := e.(*GraphExpression); ok && graphExpr.IncludeDependencies {
329+
targets = append(targets, graphExpr.Target)
330+
}
347331

348-
return string(jsonBytes)
332+
return true
333+
})
334+
335+
return targets
336+
}
337+
338+
func collectGraphExpressionTargetsWithDependents(expr Expression) []Expression {
339+
var targets []Expression
340+
341+
WalkExpressions(expr, func(e Expression) bool {
342+
if graphExpr, ok := e.(*GraphExpression); ok && graphExpr.IncludeDependents {
343+
targets = append(targets, graphExpr.Target)
344+
}
345+
346+
return true
347+
})
348+
349+
return targets
350+
}
351+
352+
func collectGitReferences(expr Expression) []string {
353+
var refs []string
354+
355+
WalkExpressions(expr, func(e Expression) bool {
356+
if gitExpr, ok := e.(*GitExpression); ok {
357+
refs = append(refs, gitExpr.FromRef, gitExpr.ToRef)
358+
}
359+
360+
return true
361+
})
362+
363+
return refs
364+
}
365+
366+
func collectWorktreeExpressions(expr Expression) []*GitExpression {
367+
var targets []*GitExpression
368+
369+
WalkExpressions(expr, func(e Expression) bool {
370+
if gitExpr, ok := e.(*GitExpression); ok {
371+
targets = append(targets, gitExpr)
372+
}
373+
374+
return true
375+
})
376+
377+
return targets
349378
}

internal/stacks/generate/generate.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,7 @@ func worktreeStacksToGenerate(
380380

381381
g.Go(func() error {
382382
// Discover all stacks in both worktrees for generation.
383-
allFromStacks, err := discoverStacks(ctx, l, opts, pair.FromWorktree, nil)
383+
allFromStacks, err := discoverStacks(ctx, l, opts, pair.FromWorktree, false)
384384
if err != nil {
385385
mu.Lock()
386386

@@ -390,12 +390,10 @@ func worktreeStacksToGenerate(
390390
return nil
391391
}
392392

393-
// Appending the reading filter to the "to" discovery forces all stacks through
393+
// The "to" discovery uses readFiles to force all stacks through
394394
// the parse phase (populating Reading()), while still returning every stack
395395
// (because they all match type=stack).
396-
readingFilters := filter.Filters{filter.NewFilter(parseExpr, parseExpr.String())}
397-
398-
allToStacks, err := discoverStacks(ctx, l, opts, pair.ToWorktree, readingFilters)
396+
allToStacks, err := discoverStacks(ctx, l, opts, pair.ToWorktree, true)
399397
if err != nil {
400398
mu.Lock()
401399

@@ -474,22 +472,28 @@ func worktreeStacksToGenerate(
474472
return stacksToGenerate.ToComponents(), nil
475473
}
476474

477-
// discoverStacks discovers stacks in a worktree using the given additional filters.
475+
// discoverStacks discovers stacks in a worktree.
478476
// User-provided filters from opts.Filters are included (restricted to stacks) so that
479477
// explicit exclusions like --filter '!./land-mine | type=stack' are respected.
478+
// When readFiles is true, all discovered stacks are parsed to populate their Reading
479+
// attribute (used by reading-affected detection).
480480
func discoverStacks(
481481
ctx context.Context,
482482
l log.Logger,
483483
opts *options.TerragruntOptions,
484484
wt worktrees.Worktree,
485-
additionalFilters filter.Filters,
485+
readFiles bool,
486486
) (component.Components, error) {
487-
allFilters := slices.Concat(stackTypeFilter(), opts.Filters.RestrictToStacks(), additionalFilters)
487+
allFilters := slices.Concat(stackTypeFilter(), opts.Filters.RestrictToStacks())
488488

489489
d := discovery.NewDiscovery(wt.Path).
490490
WithSuppressParseErrors().
491491
WithFilters(allFilters)
492492

493+
if readFiles {
494+
d = d.WithReadFiles()
495+
}
496+
493497
components, err := d.Discover(ctx, l, opts)
494498
if err != nil {
495499
return nil, errors.Errorf("failed to discover stacks in worktree %s: %w", wt.Ref, err)

0 commit comments

Comments
 (0)