Skip to content

Commit

Permalink
Fix issue mounting files
Browse files Browse the repository at this point in the history
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 <cpuguy83@gmail.com>
  • Loading branch information
cpuguy83 committed Dec 6, 2024
1 parent 3325e64 commit c0baa5c
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 27 deletions.
22 changes: 20 additions & 2 deletions helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@ import (
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
)

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

var disableDiffMerge atomic.Bool

// DisableDiffMerge allows disabling the use of [llb.Diff] and [llb.Merge] in favor of [llb.Copy].
Expand Down Expand Up @@ -491,12 +504,17 @@ func WithRepoData(repos []PackageRepositoryConfig, sOpts SourceOpts, opts ...llb
// Returns a run option for mounting the state (i.e., packages/metadata) for a single repo
func repoDataAsMount(config PackageRepositoryConfig, sOpts SourceOpts, opts ...llb.ConstraintsOpt) (llb.RunOption, error) {
var mounts []llb.RunOption

for _, data := range config.Data {
repoState, err := data.Spec.AsMount(data.Dest, sOpts, opts...)
repoState, err := data.Spec.AsMount(internalMountSourceName, sOpts, opts...)
if err != nil {
return nil, err
}
mounts = append(mounts, llb.AddMount(data.Dest, repoState))
if SourceIsDir(data.Spec) {
mounts = append(mounts, llb.AddMount(data.Dest, repoState))
} else {
mounts = append(mounts, llb.AddMount(data.Dest, repoState, llb.SourcePath(internalMountSourceName)))
}
}

return WithRunOptions(mounts...), nil
Expand Down
4 changes: 2 additions & 2 deletions source.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ func generateSourceFromImage(st llb.State, cmd *Command, sOpts SourceOpts, subPa
return llb.Scratch(), err
}

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

if !SourceIsDir(src.Spec) {
mountOpt = append(mountOpt, llb.SourcePath(src.Dest))
mountOpt = append(mountOpt, llb.SourcePath(internalMountSourceName))
}
baseRunOpts = append(baseRunOpts, llb.AddMount(src.Dest, srcSt, mountOpt...))
}
Expand Down
3 changes: 2 additions & 1 deletion source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ func TestSourceDockerImage(t *testing.T) {
src.DockerImage = &img

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

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

func checkContainsMount(t *testing.T, mounts []*pb.Mount, expect expectMount) {
t.Helper()
for _, mnt := range mounts {
if mountMatches(mnt, expect) {
return
Expand Down
150 changes: 128 additions & 22 deletions test/source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,35 +75,141 @@ func TestSourceCmd(t *testing.T) {
})

t.Run("with mounted file", func(t *testing.T) {
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
spec := testSpec()
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
{
Command: `grep 'foo bar' /foo`,
},
{
Command: `mkdir -p /output; cp /foo /output/foo`,
},
}
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
{
Dest: "/foo",
Spec: dalec.Source{
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "foo bar",
t.Parallel()
t.Run("at root", func(t *testing.T) {
t.Parallel()
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
spec := testSpec()
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
{
Command: `grep 'foo bar' /foo`,
},
{
Command: `mkdir -p /output; cp /foo /output/foo`,
},
}
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
{
Dest: "/foo",
Spec: dalec.Source{
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "foo bar",
},
},
},
},
},
}
}

req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
res := solveT(ctx, t, gwc, req)
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
res := solveT(ctx, t, gwc, req)

checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
})
})
t.Run("nested", func(t *testing.T) {
t.Parallel()
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
spec := testSpec()
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
{
Command: `grep 'foo bar' /tmp/foo`,
},
{
Command: `mkdir -p /output; cp /tmp/foo /output/foo`,
},
}
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
{
Dest: "/tmp/foo",
Spec: dalec.Source{
Inline: &dalec.SourceInline{
File: &dalec.SourceInlineFile{
Contents: "foo bar",
},
},
},
},
}

checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
res := solveT(ctx, t, gwc, req)

checkFile(ctx, t, filepath.Join(sourceName, "foo"), res, []byte("foo bar"))
})
})
})

t.Run("with mounted dir", func(t *testing.T) {
t.Parallel()
t.Run("at root", func(t *testing.T) {
t.Parallel()
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
spec := testSpec()
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
{
Command: `grep 'foo bar' /foo/bar`,
},
{
Command: `mkdir -p /output; cp -r /foo /output/foo`,
},
}
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
{
Dest: "/foo",
Spec: dalec.Source{
Inline: &dalec.SourceInline{
Dir: &dalec.SourceInlineDir{
Files: map[string]*dalec.SourceInlineFile{
"bar": {Contents: "foo bar"},
},
},
},
},
},
}

req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
res := solveT(ctx, t, gwc, req)

checkFile(ctx, t, filepath.Join(sourceName, "foo/bar"), res, []byte("foo bar"))
})
})
t.Run("nested", func(t *testing.T) {
t.Parallel()
testEnv.RunTest(ctx, t, func(ctx context.Context, gwc gwclient.Client) {
spec := testSpec()
spec.Sources[sourceName].DockerImage.Cmd.Steps = []*dalec.BuildStep{
{
Command: `grep 'foo bar' /tmp/foo/bar`,
},
{
Command: `mkdir -p /output; cp -r /tmp/foo /output/foo`,
},
}
spec.Sources[sourceName].DockerImage.Cmd.Mounts = []dalec.SourceMount{
{
Dest: "/tmp/foo",
Spec: dalec.Source{
Inline: &dalec.SourceInline{
Dir: &dalec.SourceInlineDir{
Files: map[string]*dalec.SourceInlineFile{
"bar": {Contents: "foo bar"},
},
},
},
},
},
}

req := newSolveRequest(withBuildTarget("debug/sources"), withSpec(ctx, t, spec))
res := solveT(ctx, t, gwc, req)

checkFile(ctx, t, filepath.Join(sourceName, "foo/bar"), res, []byte("foo bar"))
})
})

})
}

func TestSourceBuild(t *testing.T) {
Expand Down

0 comments on commit c0baa5c

Please sign in to comment.