Skip to content

Commit e208ec8

Browse files
committed
fix: mirror plan files from the working directory under --out-dir for Git filters
1 parent 7377d20 commit e208ec8

10 files changed

Lines changed: 378 additions & 121 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
version: "v1.1.4"
3+
category: "bug-fixes"
4+
---
5+
6+
#### `--out-dir` writes plans to the same place with and without a Git filter
7+
8+
`run --all` with [`--out-dir`](/reference/cli/commands/run#out-dir) mirrors each unit under the output directory. When a Git-based filter selected the unit and `--working-dir` was a subdirectory of the repository, the mirrored path started at the root of the repository rather than at the working directory, so a unit at `live/unit` was written to `<out-dir>/live/unit` with a filter and to `<out-dir>/unit` without one.
9+
10+
Splitting a run in two, planning with a filter and applying the same units later without one, then failed: the apply looked for each plan under a path the plan had not written, and tofu reported that it could not load the file as a plan. Both ways of selecting a unit now mirror it from `--working-dir`, including units generated by a stack. A unit that a Git filter selects from outside `--working-dir` is still mirrored from the root of the repository, which keeps it within the output directory.

internal/component/component.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ const (
6969
// because it can help us determine how the
7070
// Component should be run or enqueued later.
7171
type DiscoveryContext struct {
72-
WorkingDir string
73-
Ref string
72+
WorkingDir string
73+
OutputKeyBase string
74+
Ref string
7475

7576
origin Origin
7677

internal/component/component_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package component_test
22

33
import (
4+
"path/filepath"
45
"sync"
56
"sync/atomic"
67
"testing"
@@ -272,3 +273,72 @@ func TestUnitGuardConfigParseWithRacing(t *testing.T) {
272273
assert.Equal(t, int32(1), parses.Load(), "config should be parsed exactly once")
273274
assert.NotNil(t, unit.Config(), "config should be populated after parsing")
274275
}
276+
277+
// TestUnitOutputFile checks that the same unit is mirrored under the output folder at the
278+
// same path whether it was found on the filesystem or in a Git worktree, and that a unit
279+
// found outside the working directory is still mirrored inside the output folder.
280+
func TestUnitOutputFile(t *testing.T) {
281+
t.Parallel()
282+
283+
base := t.TempDir()
284+
workingDir := filepath.Join(base, "repo", "live")
285+
worktreeRoot := filepath.Join(base, "worktree")
286+
outputFolder := filepath.Join(base, "plans")
287+
288+
testCases := []struct {
289+
discoveryContext *component.DiscoveryContext
290+
name string
291+
unitPath string
292+
expected string
293+
}{
294+
{
295+
name: "filesystem discovery",
296+
unitPath: filepath.Join(workingDir, "unit2"),
297+
discoveryContext: &component.DiscoveryContext{
298+
WorkingDir: workingDir,
299+
},
300+
expected: filepath.Join(outputFolder, "unit2", "tfplan.tfplan"),
301+
},
302+
{
303+
name: "worktree discovery",
304+
unitPath: filepath.Join(worktreeRoot, "live", "unit2"),
305+
discoveryContext: &component.DiscoveryContext{
306+
WorkingDir: worktreeRoot,
307+
OutputKeyBase: filepath.Join(worktreeRoot, "live"),
308+
},
309+
expected: filepath.Join(outputFolder, "unit2", "tfplan.tfplan"),
310+
},
311+
{
312+
name: "worktree discovery outside the working directory",
313+
unitPath: filepath.Join(worktreeRoot, "other", "unit3"),
314+
discoveryContext: &component.DiscoveryContext{
315+
WorkingDir: worktreeRoot,
316+
OutputKeyBase: filepath.Join(worktreeRoot, "live"),
317+
},
318+
expected: filepath.Join(outputFolder, "other", "unit3", "tfplan.tfplan"),
319+
},
320+
{
321+
name: "worktree discovery without a mapped working directory",
322+
unitPath: filepath.Join(worktreeRoot, "live", "unit2"),
323+
discoveryContext: &component.DiscoveryContext{
324+
WorkingDir: worktreeRoot,
325+
},
326+
expected: filepath.Join(outputFolder, "live", "unit2", "tfplan.tfplan"),
327+
},
328+
}
329+
330+
for _, tc := range testCases {
331+
t.Run(tc.name, func(t *testing.T) {
332+
t.Parallel()
333+
334+
unit := component.NewUnit(tc.unitPath).WithDiscoveryContext(tc.discoveryContext)
335+
336+
assert.Equal(t, tc.expected, unit.OutputFile(workingDir, outputFolder))
337+
assert.Equal(
338+
t,
339+
filepath.Join(filepath.Dir(tc.expected), "tfplan.json"),
340+
unit.OutputJSONFile(workingDir, outputFolder),
341+
)
342+
})
343+
}
344+
}

internal/component/unit.go

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -340,10 +340,7 @@ func (u *Unit) planFilePath(rootWorkingDir, outputFolder, fileName string) strin
340340
return ""
341341
}
342342

343-
// Use discoveryContext.WorkingDir as base (always populated).
344-
// This is critical for git-based filters where units are discovered in temporary worktrees.
345-
// Using rootWorkingDir would cause relative paths to escape the outputFolder.
346-
relPath, err := filepath.Rel(u.discoveryContext.WorkingDir, u.path)
343+
relPath, err := filepath.Rel(u.outputKeyBase(), u.path)
347344
if err != nil {
348345
relPath = u.path
349346
}
@@ -358,3 +355,21 @@ func (u *Unit) planFilePath(rootWorkingDir, outputFolder, fileName string) strin
358355

359356
return filepath.Join(dir, fileName)
360357
}
358+
359+
// outputKeyBase returns the directory that plan output paths are computed relative to.
360+
func (u *Unit) outputKeyBase() string {
361+
// A unit found by a Git filter lives in a temporary worktree, and its discovery context
362+
// records the worktree root, so mirroring from there keys the plan by the unit's path from
363+
// the root of the repository while a filesystem walk keys the same unit by its path from
364+
// the working directory. OutputKeyBase is the working directory's counterpart inside the
365+
// worktree, which makes the two agree.
366+
//
367+
// A unit outside the working directory, which only a Git filter reaches, keys off the
368+
// worktree root instead: that always contains it, so the mirrored path stays inside the
369+
// output folder.
370+
if base := u.discoveryContext.OutputKeyBase; base != "" && util.HasPathPrefix(u.path, base) {
371+
return base
372+
}
373+
374+
return u.discoveryContext.WorkingDir
375+
}

internal/discovery/phase_worktree.go

Lines changed: 60 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ func (p *WorktreePhase) discoverInWorktree(
223223
discoveryContext := discovery.discoveryContext.Copy()
224224
discoveryContext.Ref = wt.Ref
225225
discoveryContext.WorkingDir = wt.Path
226+
discoveryContext.OutputKeyBase = discovery.worktrees.WorkingDir(ctx, wt.Path)
226227
discoveryContext.SuggestOrigin(component.OriginWorktreeDiscovery)
227228

228229
if discoveryContext.Args != nil {
@@ -446,61 +447,39 @@ func (p *WorktreePhase) walkChangedStack(
446447

447448
parentFilters := discovery.filters.ExcludingGitFilters()
448449

449-
discoveryGroup.Go(func() error {
450-
fromDiscovery := NewDiscovery(fromStack.Path()).
451-
WithDiscoveryContext(fromDiscoveryContext).
452-
WithFilters(parentFilters).
453-
WithNumWorkers(p.numWorkers)
454-
455-
var fromDiscoveryErr error
456-
457-
fromComponents, fromDiscoveryErr = fromDiscovery.Discover(discoveryCtx, l, v, input.Opts)
458-
if fromDiscoveryErr != nil {
459-
mu.Lock()
460-
461-
errs = append(errs, fromDiscoveryErr)
462-
463-
mu.Unlock()
464-
465-
return nil
466-
}
467-
468-
for _, c := range fromComponents {
469-
dc := c.DiscoveryContext().CopyWithNewOrigin(component.OriginWorktreeDiscovery)
470-
dc.WorkingDir = fromStack.DiscoveryContext().WorkingDir
471-
c.SetDiscoveryContext(dc)
472-
}
473-
474-
return nil
475-
})
476-
477-
discoveryGroup.Go(func() error {
478-
toDiscovery := NewDiscovery(toStack.Path()).
479-
WithDiscoveryContext(toDiscoveryContext).
480-
WithFilters(parentFilters).
481-
WithNumWorkers(p.numWorkers)
450+
for _, side := range []struct {
451+
stack *component.Stack
452+
discoveryContext *component.DiscoveryContext
453+
components *component.Components
454+
}{
455+
{stack: fromStack, discoveryContext: fromDiscoveryContext, components: &fromComponents},
456+
{stack: toStack, discoveryContext: toDiscoveryContext, components: &toComponents},
457+
} {
458+
discoveryGroup.Go(func() error {
459+
components, err := p.walkStackSide(
460+
discoveryCtx,
461+
l,
462+
v,
463+
input,
464+
side.stack,
465+
side.discoveryContext,
466+
parentFilters,
467+
)
468+
if err != nil {
469+
mu.Lock()
482470

483-
var toDiscoveryErr error
471+
errs = append(errs, err)
484472

485-
toComponents, toDiscoveryErr = toDiscovery.Discover(discoveryCtx, l, v, input.Opts)
486-
if toDiscoveryErr != nil {
487-
mu.Lock()
473+
mu.Unlock()
488474

489-
errs = append(errs, toDiscoveryErr)
475+
return nil
476+
}
490477

491-
mu.Unlock()
478+
*side.components = components
492479

493480
return nil
494-
}
495-
496-
for _, c := range toComponents {
497-
dc := c.DiscoveryContext().CopyWithNewOrigin(component.OriginWorktreeDiscovery)
498-
dc.WorkingDir = toStack.DiscoveryContext().WorkingDir
499-
c.SetDiscoveryContext(dc)
500-
}
501-
502-
return nil
503-
})
481+
})
482+
}
504483

505484
if err = discoveryGroup.Wait(); err != nil {
506485
return nil, err
@@ -571,6 +550,38 @@ func (p *WorktreePhase) walkChangedStack(
571550
return finalComponents, nil
572551
}
573552

553+
// walkStackSide walks one side of a changed stack and records the components it finds as
554+
// worktree discoveries rooted at that side's worktree.
555+
func (p *WorktreePhase) walkStackSide(
556+
ctx context.Context,
557+
l log.Logger,
558+
v *venv.Venv,
559+
input *PhaseInput,
560+
stack *component.Stack,
561+
discoveryContext *component.DiscoveryContext,
562+
filters filter.Filters,
563+
) (component.Components, error) {
564+
discovery := input.Discovery
565+
566+
components, err := NewDiscovery(stack.Path()).
567+
WithDiscoveryContext(discoveryContext).
568+
WithFilters(filters).
569+
WithNumWorkers(p.numWorkers).
570+
Discover(ctx, l, v, input.Opts)
571+
if err != nil {
572+
return nil, err
573+
}
574+
575+
for _, c := range components {
576+
dc := c.DiscoveryContext().CopyWithNewOrigin(component.OriginWorktreeDiscovery)
577+
dc.WorkingDir = stack.DiscoveryContext().WorkingDir
578+
dc.OutputKeyBase = discovery.worktrees.WorkingDir(ctx, dc.WorkingDir)
579+
c.SetDiscoveryContext(dc)
580+
}
581+
582+
return components, nil
583+
}
584+
574585
// ComponentPair represents a pair of matched components from different worktrees.
575586
type ComponentPair struct {
576587
FromComponent component.Component

0 commit comments

Comments
 (0)