Skip to content

Commit 4005b88

Browse files
ostermanclaude
andcommitted
fix(steps): resolve relative paths against step.WorkingDirectory
The archive, file, workdir, junit, and container build step handlers resolved relative source/destination/path/glob/context fields via template substitution only, then let filesystem calls resolve them against the Atmos process's own cwd instead of step.WorkingDirectory. This surfaced most visibly for `type: archive` hooks, since the hooks engine correctly defaults working_directory to the component path but the handler never read it back. Add a shared BaseHandler.ResolveInWorkingDirectory helper that anchors a relative resolved value to step.WorkingDirectory (falling back to process cwd when unset, matching prior behavior), and apply it across the five affected handlers. container_build.go additionally anchors Dockerfile to the resolved Context rather than WorkingDirectory directly, matching Docker's own convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent d2b8e81 commit 4005b88

15 files changed

Lines changed: 374 additions & 36 deletions

pkg/hooks/step_engine_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1031,6 +1031,43 @@ func TestStepEngineRunsArchiveType(t *testing.T) {
10311031
assert.Equal(t, "handler.js", r.File[0].Name)
10321032
}
10331033

1034+
// TestStepEngineRunsArchiveTypeWithRelativeWorkingDirectory reproduces the
1035+
// originally-reported regression: a `type: archive` hook step with relative
1036+
// source/destination must resolve them against step.WorkingDirectory (either
1037+
// explicitly set here, or defaulted by setDefaultStepWorkingDirectory to the
1038+
// resolved component path), not against the Atmos process's own cwd.
1039+
func TestStepEngineRunsArchiveTypeWithRelativeWorkingDirectory(t *testing.T) {
1040+
workDir := t.TempDir()
1041+
require.NoError(t, os.MkdirAll(filepath.Join(workDir, "src"), 0o755))
1042+
require.NoError(t, os.WriteFile(filepath.Join(workDir, "src", "handler.js"), []byte("exports.handler = 1;"), 0o644))
1043+
1044+
// Process cwd differs from working_directory — this is the crux of the bug.
1045+
t.Chdir(t.TempDir())
1046+
1047+
hook := &Hook{
1048+
Kind: stepKindName,
1049+
Type: "archive",
1050+
With: map[string]any{
1051+
"source": "src",
1052+
"destination": "out/handler.zip",
1053+
"working_directory": workDir,
1054+
},
1055+
}
1056+
1057+
out, err := stepEngine{}.Run(stepExecContext(hook))
1058+
require.NoError(t, err)
1059+
require.NotNil(t, out)
1060+
require.NotNil(t, out.Summary)
1061+
assert.Equal(t, StatusSuccess, out.Summary.Status)
1062+
1063+
wantDest := filepath.Join(workDir, "out", "handler.zip")
1064+
r, err := zip.OpenReader(wantDest)
1065+
require.NoError(t, err)
1066+
defer r.Close()
1067+
require.Len(t, r.File, 1)
1068+
assert.Equal(t, "handler.js", r.File[0].Name)
1069+
}
1070+
10341071
func TestVerifyStepsHookTypes(t *testing.T) {
10351072
t.Run("known types", func(t *testing.T) {
10361073
hook := &Hook{

pkg/runner/step/archive.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func (h *ArchiveHandler) Validate(step *schema.WorkflowStep) error {
7070
func (h *ArchiveHandler) Execute(ctx context.Context, step *schema.WorkflowStep, vars *Variables) (*StepResult, error) {
7171
defer perf.Track(nil, "step.ArchiveHandler.Execute")()
7272

73-
opts, action, err := resolveArchiveOptions(step, vars)
73+
opts, action, err := h.resolveArchiveOptions(step, vars)
7474
if err != nil {
7575
return nil, err
7676
}
@@ -85,19 +85,19 @@ func (h *ArchiveHandler) Execute(ctx context.Context, step *schema.WorkflowStep,
8585
WithMetadata("source", opts.Source), nil
8686
}
8787

88-
func resolveArchiveOptions(step *schema.WorkflowStep, vars *Variables) (archive.PackOptions, archive.Action, error) {
88+
func (h *ArchiveHandler) resolveArchiveOptions(step *schema.WorkflowStep, vars *Variables) (archive.PackOptions, archive.Action, error) {
8989
source, err := archiveSourceString(step)
9090
if err != nil {
9191
return archive.PackOptions{}, "", err
9292
}
93-
source, err = vars.Resolve(source)
93+
source, err = h.ResolveInWorkingDirectory(step, vars, source, "source")
9494
if err != nil {
95-
return archive.PackOptions{}, "", fmt.Errorf("step '%s': failed to resolve source: %w", step.Name, err)
95+
return archive.PackOptions{}, "", err
9696
}
9797

98-
destination, err := vars.Resolve(step.Destination)
98+
destination, err := h.ResolveInWorkingDirectory(step, vars, step.Destination, "destination")
9999
if err != nil {
100-
return archive.PackOptions{}, "", fmt.Errorf("step '%s': failed to resolve destination: %w", step.Name, err)
100+
return archive.PackOptions{}, "", err
101101
}
102102
format, err := vars.Resolve(step.Format)
103103
if err != nil {

pkg/runner/step/archive_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,44 @@ func TestArchiveHandler_Execute(t *testing.T) {
107107
assert.Equal(t, "exports.handler = 1;", string(content))
108108
}
109109

110+
// TestArchiveHandler_Execute_ResolvesRelativePathsAgainstWorkingDirectory
111+
// reproduces the reported regression: a relative source/destination must
112+
// resolve against step.WorkingDirectory, not the Atmos process's own cwd
113+
// (e.g. when the step runs as a component lifecycle hook).
114+
func TestArchiveHandler_Execute_ResolvesRelativePathsAgainstWorkingDirectory(t *testing.T) {
115+
workDir := t.TempDir()
116+
require.NoError(t, os.MkdirAll(filepath.Join(workDir, "src"), 0o755))
117+
require.NoError(t, os.WriteFile(filepath.Join(workDir, "src", "handler.js"), []byte("exports.handler = 1;"), 0o644))
118+
119+
// Pin the process cwd to an unrelated scratch dir so this test actually
120+
// distinguishes "resolved against WorkingDirectory" from "resolved
121+
// against cwd" — the pre-fix behavior.
122+
t.Chdir(t.TempDir())
123+
124+
handler := mustGetArchiveHandler(t)
125+
step := &schema.WorkflowStep{
126+
Name: "pkg",
127+
Type: "archive",
128+
Source: "src",
129+
Destination: "out/handler.zip",
130+
WorkingDirectory: workDir,
131+
}
132+
require.NoError(t, handler.Validate(step))
133+
134+
result, err := handler.Execute(context.Background(), step, NewVariables())
135+
require.NoError(t, err)
136+
require.NotNil(t, result)
137+
138+
wantDest := filepath.Join(workDir, "out", "handler.zip")
139+
assert.Equal(t, wantDest, result.Value)
140+
141+
r, err := zip.OpenReader(wantDest)
142+
require.NoError(t, err)
143+
defer r.Close()
144+
require.Len(t, r.File, 1)
145+
assert.Equal(t, "handler.js", r.File[0].Name)
146+
}
147+
110148
func TestArchiveHandler_Execute_ResolvesTemplatedFields(t *testing.T) {
111149
dir := t.TempDir()
112150
src := filepath.Join(dir, "src")

pkg/runner/step/container_actions_extra_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package step
22

33
import (
44
"context"
5+
"path/filepath"
56
"testing"
67

78
"github.com/stretchr/testify/assert"
@@ -152,6 +153,44 @@ func TestBuildConfigResolutionErrors(t *testing.T) {
152153
}
153154
}
154155

156+
func TestBuildBuildConfigResolvesContextAndDockerfileAgainstWorkingDirectory(t *testing.T) {
157+
h := &ContainerHandler{}
158+
workDir := t.TempDir()
159+
160+
t.Run("relative context and dockerfile", func(t *testing.T) {
161+
cfg, err := h.buildBuildConfig(&schema.WorkflowStep{
162+
Name: "build",
163+
WorkingDirectory: workDir,
164+
Build: &schema.ContainerBuildStep{Context: "docker", Dockerfile: "Dockerfile.prod"},
165+
}, NewVariables())
166+
require.NoError(t, err)
167+
assert.Equal(t, filepath.Join(workDir, "docker"), cfg.Context)
168+
assert.Equal(t, filepath.Join(workDir, "docker", "Dockerfile.prod"), cfg.Dockerfile)
169+
})
170+
171+
t.Run("defaults: dockerfile lands inside context, not working directory directly", func(t *testing.T) {
172+
cfg, err := h.buildBuildConfig(&schema.WorkflowStep{
173+
Name: "build",
174+
WorkingDirectory: workDir,
175+
Build: &schema.ContainerBuildStep{Context: "docker"},
176+
}, NewVariables())
177+
require.NoError(t, err)
178+
assert.Equal(t, filepath.Join(workDir, "docker"), cfg.Context)
179+
assert.Equal(t, filepath.Join(workDir, "docker", "Dockerfile"), cfg.Dockerfile)
180+
})
181+
182+
t.Run("absolute dockerfile is not re-anchored to context", func(t *testing.T) {
183+
absDockerfile := filepath.Join(t.TempDir(), "Dockerfile.custom")
184+
cfg, err := h.buildBuildConfig(&schema.WorkflowStep{
185+
Name: "build",
186+
WorkingDirectory: workDir,
187+
Build: &schema.ContainerBuildStep{Context: "docker", Dockerfile: absDockerfile},
188+
}, NewVariables())
189+
require.NoError(t, err)
190+
assert.Equal(t, absDockerfile, cfg.Dockerfile)
191+
})
192+
}
193+
155194
func TestRunConfigResolutionErrors(t *testing.T) {
156195
h := &ContainerHandler{}
157196
vars := NewVariables()

pkg/runner/step/container_build.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package step
33
import (
44
"context"
55
"fmt"
6+
"path/filepath"
67
"strings"
78

89
"github.com/cloudposse/atmos/pkg/container"
@@ -94,14 +95,17 @@ func buildSpinnerMessage(verb, image string) string {
9495

9596
func (h *ContainerHandler) buildBuildConfig(step *schema.WorkflowStep, vars *Variables) (*container.BuildConfig, error) {
9697
build := effectiveBuildStep(step)
97-
contextDir, err := resolveOptional(vars, defaultString(build.Context, "."), "build.context", step.Name)
98+
contextDir, err := h.ResolveInWorkingDirectory(step, vars, defaultString(build.Context, "."), "build.context")
9899
if err != nil {
99100
return nil, err
100101
}
101102
dockerfile, err := resolveOptional(vars, defaultString(build.Dockerfile, "Dockerfile"), "build.dockerfile", step.Name)
102103
if err != nil {
103104
return nil, err
104105
}
106+
if dockerfile != "" && !filepath.IsAbs(dockerfile) {
107+
dockerfile = filepath.Join(contextDir, dockerfile)
108+
}
105109
target, err := resolveOptional(vars, build.Target, "build.target", step.Name)
106110
if err != nil {
107111
return nil, err

pkg/runner/step/container_runtime_fake_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,12 @@ func TestContainerHandlerExecuteBuildPassesBuildxDriverAndCacheToDocker(t *testi
108108
}, vars)
109109
require.NoError(t, err)
110110

111+
cwd, err := os.Getwd()
112+
require.NoError(t, err)
113+
111114
args := fakeRuntimeArgs(t, argsPath)
112115
assert.Contains(t, args, "buildx\tcreate\t--name\tatmos-test-builder\t--driver\tdocker-container\t--driver-opt\timage=mirror.gcr.io/moby/buildkit:buildx-stable-1")
113-
assert.Contains(t, args, "buildx\tbuild\t--builder\tatmos-test-builder\t--cache-from\tref=registry.example.com/app:buildcache,type=registry\t--cache-to\tmode=max,ref=registry.example.com/app:buildcache,type=registry\t-t\tapp:local\t-f\tDockerfile\t.")
116+
assert.Contains(t, args, "buildx\tbuild\t--builder\tatmos-test-builder\t--cache-from\tref=registry.example.com/app:buildcache,type=registry\t--cache-to\tmode=max,ref=registry.example.com/app:buildcache,type=registry\t-t\tapp:local\t-f\t"+filepath.Join(cwd, "Dockerfile")+"\t"+cwd)
114117
}
115118

116119
func TestContainerHandlerExecutePushPassesResolvedTagsAndRuntimeEnvToDocker(t *testing.T) {

pkg/runner/step/container_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,11 @@ func TestContainerHandlerActionBlocks(t *testing.T) {
149149
},
150150
}, vars)
151151
require.NoError(t, err)
152-
assert.Equal(t, ".", buildCfg.Context)
152+
cwd, err := os.Getwd()
153+
require.NoError(t, err)
154+
assert.Equal(t, cwd, buildCfg.Context)
153155
assert.Equal(t, "buildx", buildCfg.Engine)
154-
assert.Equal(t, "Dockerfile", buildCfg.Dockerfile)
156+
assert.Equal(t, filepath.Join(cwd, "Dockerfile"), buildCfg.Dockerfile)
155157
assert.Equal(t, []string{"app:test"}, buildCfg.Tags)
156158
assert.Equal(t, map[string]string{"VERSION": "1.0.0"}, buildCfg.Args)
157159
assert.Equal(t, "runtime", buildCfg.Target)

pkg/runner/step/file.go

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -91,24 +91,14 @@ func (h *FileHandler) Execute(ctx context.Context, step *schema.WorkflowStep, va
9191
return NewStepResult(fullPath), nil
9292
}
9393

94-
// resolveStartPath resolves and validates the starting path for file scanning.
94+
// resolveStartPath resolves and validates the starting path for file scanning,
95+
// anchoring a relative path to step.WorkingDirectory.
9596
func (h *FileHandler) resolveStartPath(step *schema.WorkflowStep, vars *Variables) (string, error) {
9697
startPath := step.Path
9798
if startPath == "" {
9899
startPath = "."
99-
} else {
100-
var err error
101-
startPath, err = vars.Resolve(startPath)
102-
if err != nil {
103-
return "", fmt.Errorf("step '%s': failed to resolve path: %w", step.Name, err)
104-
}
105-
}
106-
107-
absPath, err := filepath.Abs(startPath)
108-
if err != nil {
109-
return "", fmt.Errorf("step '%s': failed to resolve path: %w", step.Name, err)
110100
}
111-
return absPath, nil
101+
return h.ResolveInWorkingDirectory(step, vars, startPath, "path")
112102
}
113103

114104
// collectFiles walks the directory and collects matching files.

pkg/runner/step/file_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
1010

11+
errUtils "github.com/cloudposse/atmos/errors"
1112
"github.com/cloudposse/atmos/pkg/schema"
1213
)
1314

@@ -70,7 +71,24 @@ func TestFileHandler_ResolveStartPath(t *testing.T) {
7071

7172
_, err := fileHandler.resolveStartPath(step, vars)
7273
assert.Error(t, err)
73-
assert.Contains(t, err.Error(), "failed to resolve path")
74+
assert.ErrorIs(t, err, errUtils.ErrTemplateEvaluation)
75+
})
76+
77+
t.Run("relative path resolves against working directory, not cwd", func(t *testing.T) {
78+
workDir := t.TempDir()
79+
require.NoError(t, os.MkdirAll(filepath.Join(workDir, "sub"), 0o755))
80+
t.Chdir(t.TempDir())
81+
82+
step := &schema.WorkflowStep{
83+
Name: "test",
84+
Path: "sub",
85+
WorkingDirectory: workDir,
86+
}
87+
vars := NewVariables()
88+
89+
path, err := fileHandler.resolveStartPath(step, vars)
90+
require.NoError(t, err)
91+
assert.Equal(t, filepath.Join(workDir, "sub"), path)
7492
})
7593
}
7694

pkg/runner/step/handler_base.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package step
33
import (
44
"context"
55
"fmt"
6+
"os"
7+
"path/filepath"
68

79
errUtils "github.com/cloudposse/atmos/errors"
810
"github.com/cloudposse/atmos/pkg/perf"
@@ -194,3 +196,78 @@ func (h BaseHandler) ResolveCommand(ctx context.Context, step *schema.WorkflowSt
194196
}
195197
return resolved, nil
196198
}
199+
200+
// ResolveInWorkingDirectory resolves a Go-template field, then anchors a
201+
// relative result to step.WorkingDirectory (itself template-resolved),
202+
// returning an absolute path. An empty raw value short-circuits to "" (no
203+
// resolve, no join) like the other Resolve* helpers. A value that resolves
204+
// to "" or to an already-absolute path passes through unchanged — it is
205+
// never re-anchored to WorkingDirectory.
206+
func (h BaseHandler) ResolveInWorkingDirectory(step *schema.WorkflowStep, vars *Variables, value, field string) (string, error) {
207+
defer perf.Track(nil, "step.BaseHandler.ResolveInWorkingDirectory")()
208+
209+
if value == "" {
210+
return "", nil
211+
}
212+
resolved, err := vars.Resolve(value)
213+
if err != nil {
214+
return "", errUtils.Build(errUtils.ErrTemplateEvaluation).
215+
WithCause(err).
216+
WithContext("step", step.Name).
217+
WithContext("field", field).
218+
Err()
219+
}
220+
if resolved == "" || filepath.IsAbs(resolved) {
221+
return resolved, nil
222+
}
223+
224+
workDir, err := h.resolveWorkingDirectory(step, vars)
225+
if err != nil {
226+
return "", err
227+
}
228+
return filepath.Join(workDir, resolved), nil
229+
}
230+
231+
// resolveWorkingDirectory resolves step.WorkingDirectory (itself a possible
232+
// template) to an absolute base directory, falling back to the process's
233+
// current working directory when unset — preserving the historical
234+
// filepath.Abs-against-cwd behavior for steps that never set
235+
// working_directory.
236+
func (h BaseHandler) resolveWorkingDirectory(step *schema.WorkflowStep, vars *Variables) (string, error) {
237+
defer perf.Track(nil, "step.BaseHandler.resolveWorkingDirectory")()
238+
239+
workDir := step.WorkingDirectory
240+
if workDir != "" {
241+
resolved, err := vars.Resolve(workDir)
242+
if err != nil {
243+
return "", errUtils.Build(errUtils.ErrTemplateEvaluation).
244+
WithCause(err).
245+
WithContext("step", step.Name).
246+
WithContext("field", "working_directory").
247+
Err()
248+
}
249+
workDir = resolved
250+
}
251+
if workDir == "" {
252+
cwd, err := os.Getwd()
253+
if err != nil {
254+
return "", errUtils.Build(errUtils.ErrPathResolution).
255+
WithCause(err).
256+
WithContext("step", step.Name).
257+
Err()
258+
}
259+
workDir = cwd
260+
}
261+
if !filepath.IsAbs(workDir) {
262+
abs, err := filepath.Abs(workDir)
263+
if err != nil {
264+
return "", errUtils.Build(errUtils.ErrPathResolution).
265+
WithCause(err).
266+
WithContext("step", step.Name).
267+
WithContext("field", "working_directory").
268+
Err()
269+
}
270+
workDir = abs
271+
}
272+
return workDir, nil
273+
}

0 commit comments

Comments
 (0)