Skip to content

Commit c404433

Browse files
cpuguy83cpuguy83
committed
Fix issue mounting files
Mounts were using the destination path as the source name when generating a source, this made it so mounts would only work correctly if mounted at the root of the filesystem since source names generally should not have path separators. Signed-off-by: Brian Goff <[email protected]>
1 parent 1c69008 commit c404433

File tree

6 files changed

+243
-29
lines changed

6 files changed

+243
-29
lines changed

‎frontend/test_runner.go

+20-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,19 @@ import (
1717
"github.com/pkg/errors"
1818
)
1919

20+
const (
21+
// This is used as the source name for sources in specified in `SourceMount`
22+
// For any sources we need to mount we need to give the source a name.
23+
// We don't actually care about the name here *except* the way file-backed
24+
// sources work the name of the file becomes the source name.
25+
// So we at least need to track it.
26+
// Source names must also not contain path separators or it can screw up the logic.
27+
//
28+
// To note, the name of the source affects how the source is cached, so this
29+
// should just be a single specific name so we can get maximal cache re-use.
30+
internalMountSourceName = "src"
31+
)
32+
2033
// Run tests runs the tests defined in the spec against the given target container.
2134
func RunTests(ctx context.Context, client gwclient.Client, spec *dalec.Spec, ref gwclient.Reference, withTestDeps llb.StateOption, target string) error {
2235
if skipVar := client.BuildOpts().Opts["build-arg:"+"DALEC_SKIP_TESTS"]; skipVar != "" {
@@ -80,11 +93,16 @@ func RunTests(ctx context.Context, client gwclient.Client, spec *dalec.Spec, ref
8093
pg := llb.ProgressGroup(identity.NewID(), "Test: "+path.Join(target, test.Name), false)
8194

8295
for _, sm := range test.Mounts {
83-
st, err := sm.Spec.AsMount(sm.Dest, sOpt, pg)
96+
st, err := sm.Spec.AsMount(internalMountSourceName, sOpt, pg)
8497
if err != nil {
8598
return err
8699
}
87-
opts = append(opts, llb.AddMount(sm.Dest, st, llb.SourcePath(sm.Spec.Path)))
100+
101+
if dalec.SourceIsDir(sm.Spec) {
102+
opts = append(opts, llb.AddMount(sm.Dest, st, llb.SourcePath(sm.Spec.Path)))
103+
} else {
104+
opts = append(opts, llb.AddMount(sm.Dest, st, llb.SourcePath(internalMountSourceName)))
105+
}
88106
}
89107

90108
opts = append(opts, pg)

‎helpers.go

+20-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,19 @@ import (
1818
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
1919
)
2020

21+
const (
22+
// This is used as the source name for sources in specified in `SourceMount`
23+
// For any sources we need to mount we need to give the source a name.
24+
// We don't actually care about the name here *except* the way file-backed
25+
// sources work the name of the file becomes the source name.
26+
// So we at least need to track it.
27+
// Source names must also not contain path separators or it can screw up the logic.
28+
//
29+
// To note, the name of the source affects how the source is cached, so this
30+
// should just be a single specific name so we can get maximal cache re-use.
31+
internalMountSourceName = "src"
32+
)
33+
2134
var disableDiffMerge atomic.Bool
2235

2336
// DisableDiffMerge allows disabling the use of [llb.Diff] and [llb.Merge] in favor of [llb.Copy].
@@ -491,12 +504,17 @@ func WithRepoData(repos []PackageRepositoryConfig, sOpts SourceOpts, opts ...llb
491504
// Returns a run option for mounting the state (i.e., packages/metadata) for a single repo
492505
func repoDataAsMount(config PackageRepositoryConfig, sOpts SourceOpts, opts ...llb.ConstraintsOpt) (llb.RunOption, error) {
493506
var mounts []llb.RunOption
507+
494508
for _, data := range config.Data {
495-
repoState, err := data.Spec.AsMount(data.Dest, sOpts, opts...)
509+
repoState, err := data.Spec.AsMount(internalMountSourceName, sOpts, opts...)
496510
if err != nil {
497511
return nil, err
498512
}
499-
mounts = append(mounts, llb.AddMount(data.Dest, repoState))
513+
if SourceIsDir(data.Spec) {
514+
mounts = append(mounts, llb.AddMount(data.Dest, repoState))
515+
} else {
516+
mounts = append(mounts, llb.AddMount(data.Dest, repoState, llb.SourcePath(internalMountSourceName)))
517+
}
500518
}
501519

502520
return WithRunOptions(mounts...), nil

‎source.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ func generateSourceFromImage(st llb.State, cmd *Command, sOpts SourceOpts, subPa
307307
return llb.Scratch(), err
308308
}
309309

310-
srcSt, err := src.Spec.AsMount(src.Dest, sOpts, opts...)
310+
srcSt, err := src.Spec.AsMount(internalMountSourceName, sOpts, opts...)
311311
if err != nil {
312312
return llb.Scratch(), err
313313
}
@@ -322,7 +322,7 @@ func generateSourceFromImage(st llb.State, cmd *Command, sOpts SourceOpts, subPa
322322
}
323323

324324
if !SourceIsDir(src.Spec) {
325-
mountOpt = append(mountOpt, llb.SourcePath(src.Dest))
325+
mountOpt = append(mountOpt, llb.SourcePath(internalMountSourceName))
326326
}
327327
baseRunOpts = append(baseRunOpts, llb.AddMount(src.Dest, srcSt, mountOpt...))
328328
}

‎source_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ func TestSourceDockerImage(t *testing.T) {
324324
src.DockerImage = &img
325325

326326
ops := getSourceOp(ctx, t, src)
327-
fileMountCheck := []expectMount{{dest: "/filedest", selector: "/filedest", typ: pb.MountType_BIND}}
327+
fileMountCheck := []expectMount{{dest: "/filedest", selector: internalMountSourceName, typ: pb.MountType_BIND}}
328328
checkCmd(t, ops[2:], &src, [][]expectMount{noMountCheck, fileMountCheck})
329329
})
330330

@@ -985,6 +985,7 @@ func mountMatches(gotMount *pb.Mount, wantMount expectMount) bool {
985985
}
986986

987987
func checkContainsMount(t *testing.T, mounts []*pb.Mount, expect expectMount) {
988+
t.Helper()
988989
for _, mnt := range mounts {
989990
if mountMatches(mnt, expect) {
990991
return

‎test/azlinux_test.go

+71
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,77 @@ echo "$BAR" > bar.txt
480480
},
481481

482482
Tests: []*dalec.TestSpec{
483+
{
484+
Name: "Verify source mounts work",
485+
Mounts: []dalec.SourceMount{
486+
{
487+
Dest: "/foo",
488+
Spec: dalec.Source{
489+
Inline: &dalec.SourceInline{
490+
File: &dalec.SourceInlineFile{
491+
Contents: "hello world",
492+
},
493+
},
494+
},
495+
},
496+
{
497+
Dest: "/nested/foo",
498+
Spec: dalec.Source{
499+
Inline: &dalec.SourceInline{
500+
File: &dalec.SourceInlineFile{
501+
Contents: "hello world nested",
502+
},
503+
},
504+
},
505+
},
506+
{
507+
Dest: "/dir",
508+
Spec: dalec.Source{
509+
Inline: &dalec.SourceInline{
510+
Dir: &dalec.SourceInlineDir{
511+
Files: map[string]*dalec.SourceInlineFile{
512+
"foo": {Contents: "hello from dir"},
513+
},
514+
},
515+
},
516+
},
517+
},
518+
{
519+
Dest: "/nested/dir",
520+
Spec: dalec.Source{
521+
Inline: &dalec.SourceInline{
522+
Dir: &dalec.SourceInlineDir{
523+
Files: map[string]*dalec.SourceInlineFile{
524+
"foo": {Contents: "hello from nested dir"},
525+
},
526+
},
527+
},
528+
},
529+
},
530+
},
531+
Steps: []dalec.TestStep{
532+
{
533+
Command: "/bin/sh -c 'cat /foo'",
534+
Stdout: dalec.CheckOutput{Equals: "hello world"},
535+
Stderr: dalec.CheckOutput{Empty: true},
536+
},
537+
{
538+
Command: "/bin/sh -c 'cat /nested/foo'",
539+
Stdout: dalec.CheckOutput{Equals: "hello world nested"},
540+
Stderr: dalec.CheckOutput{Empty: true},
541+
},
542+
{
543+
Command: "/bin/sh -c 'cat /dir/foo'",
544+
Stdout: dalec.CheckOutput{Equals: "hello from dir"},
545+
Stderr: dalec.CheckOutput{Empty: true},
546+
},
547+
{
548+
Command: "/bin/sh -c 'cat /nested/dir/foo'",
549+
Stdout: dalec.CheckOutput{Equals: "hello from nested dir"},
550+
Stderr: dalec.CheckOutput{Empty: true},
551+
},
552+
},
553+
},
483554
{
484555
Name: "Check that the binary artifacts execute and provide the expected output",
485556
Steps: []dalec.TestStep{

‎test/source_test.go

+128-22
Original file line numberDiff line numberDiff line change
@@ -75,35 +75,141 @@ func TestSourceCmd(t *testing.T) {
7575
})
7676

7777
t.Run("with mounted file", func(t *testing.T) {
78-
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
79-
spec := testSpec()
80-
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
81-
{
82-
Command: `grep 'foo bar' /foo`,
83-
},
84-
{
85-
Command: `mkdir -p /output; cp /foo /output/foo`,
86-
},
87-
}
88-
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
89-
{
90-
Dest: "/foo",
91-
Spec: dalec.Source{
92-
Inline: &dalec.SourceInline{
93-
File: &dalec.SourceInlineFile{
94-
Contents: "foo bar",
78+
t.Parallel()
79+
t.Run("at root", func(t *testing.T) {
80+
t.Parallel()
81+
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
82+
spec := testSpec()
83+
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
84+
{
85+
Command: `grep 'foo bar' /foo`,
86+
},
87+
{
88+
Command: `mkdir -p /output; cp /foo /output/foo`,
89+
},
90+
}
91+
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
92+
{
93+
Dest: "/foo",
94+
Spec: dalec.Source{
95+
Inline: &dalec.SourceInline{
96+
File: &dalec.SourceInlineFile{
97+
Contents: "foo bar",
98+
},
9599
},
96100
},
97101
},
98-
},
99-
}
102+
}
100103

101-
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
102-
res := solveT(ctx, t, gwc, req)
104+
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
105+
res := solveT(ctx, t, gwc, req)
106+
107+
checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
108+
})
109+
})
110+
t.Run("nested", func(t *testing.T) {
111+
t.Parallel()
112+
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
113+
spec := testSpec()
114+
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
115+
{
116+
Command: `grep 'foo bar' /tmp/foo`,
117+
},
118+
{
119+
Command: `mkdir -p /output; cp /tmp/foo /output/foo`,
120+
},
121+
}
122+
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
123+
{
124+
Dest: "/tmp/foo",
125+
Spec: dalec.Source{
126+
Inline: &dalec.SourceInline{
127+
File: &dalec.SourceInlineFile{
128+
Contents: "foo bar",
129+
},
130+
},
131+
},
132+
},
133+
}
103134

104-
checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
135+
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
136+
res := solveT(ctx, t, gwc, req)
137+
138+
checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
139+
})
105140
})
106141
})
142+
143+
t.Run("with mounted dir", func(t *testing.T) {
144+
t.Parallel()
145+
t.Run("at root", func(t *testing.T) {
146+
t.Parallel()
147+
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
148+
spec := testSpec()
149+
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
150+
{
151+
Command: `grep 'foo bar' /foo/bar`,
152+
},
153+
{
154+
Command: `mkdir -p /output; cp -r /foo /output/foo`,
155+
},
156+
}
157+
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
158+
{
159+
Dest: "/foo",
160+
Spec: dalec.Source{
161+
Inline: &dalec.SourceInline{
162+
Dir: &dalec.SourceInlineDir{
163+
Files: map[string]*dalec.SourceInlineFile{
164+
"bar": {Contents: "foo bar"},
165+
},
166+
},
167+
},
168+
},
169+
},
170+
}
171+
172+
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
173+
res := solveT(ctx, t, gwc, req)
174+
175+
checkFile(ctx, t, filepath.Join(sourceName, "foo/bar"), res, []byte("foo bar"))
176+
})
177+
})
178+
t.Run("nested", func(t *testing.T) {
179+
t.Parallel()
180+
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
181+
spec := testSpec()
182+
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
183+
{
184+
Command: `grep 'foo bar' /tmp/foo/bar`,
185+
},
186+
{
187+
Command: `mkdir -p /output; cp -r /tmp/foo /output/foo`,
188+
},
189+
}
190+
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
191+
{
192+
Dest: "/tmp/foo",
193+
Spec: dalec.Source{
194+
Inline: &dalec.SourceInline{
195+
Dir: &dalec.SourceInlineDir{
196+
Files: map[string]*dalec.SourceInlineFile{
197+
"bar": {Contents: "foo bar"},
198+
},
199+
},
200+
},
201+
},
202+
},
203+
}
204+
205+
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
206+
res := solveT(ctx, t, gwc, req)
207+
208+
checkFile(ctx, t, filepath.Join(sourceName, "foo/bar"), res, []byte("foo bar"))
209+
})
210+
})
211+
212+
})
107213
}
108214

109215
func TestSourceBuild(t *testing.T) {

0 commit comments

Comments
 (0)