Skip to content

Commit

Permalink
Merge pull request #4270 from jsternberg/invalid-caching-multiple-bin…
Browse files Browse the repository at this point in the history
…d-mounts

solver: correctly set the content selector with multiple bind mounts references
  • Loading branch information
tonistiigi authored Sep 25, 2023
2 parents 5ed1f0b + dc60842 commit 77b9e11
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 5 deletions.
22 changes: 17 additions & 5 deletions solver/llbsolver/ops/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,16 +227,28 @@ func (e *ExecOp) getMountDeps() ([]dep, error) {
return nil, errors.Errorf("invalid mountinput %v", m)
}

sel := m.Selector
if sel != "" {
sel = path.Join("/", sel)
deps[m.Input].Selectors = append(deps[m.Input].Selectors, sel)
}
// Mark the selector path as used. In this section, we need to
// record root selectors so the selection criteria isn't narrowed
// erroneously.
sel := path.Join("/", m.Selector)
deps[m.Input].Selectors = append(deps[m.Input].Selectors, sel)

if (!m.Readonly || m.Dest == pb.RootMount) && m.Output != -1 { // exclude read-only rootfs && read-write mounts
deps[m.Input].NoContentBasedHash = true
}
}

// Remove extraneous selectors that may have been generated from above.
for i, dep := range deps {
for _, sel := range dep.Selectors {
// If the root path is included in the list of selectors,
// this is the same as if no selector was used. Zero out this field.
if sel == "/" {
deps[i].Selectors = nil
break
}
}
}
return deps, nil
}

Expand Down
72 changes: 72 additions & 0 deletions solver/llbsolver/ops/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package ops
import (
"testing"

"github.com/moby/buildkit/solver"
"github.com/moby/buildkit/solver/pb"
digest "github.com/opencontainers/go-digest"
"github.com/stretchr/testify/require"
)

Expand All @@ -25,3 +28,72 @@ func TestDedupPaths(t *testing.T) {
res = dedupePaths([]string{"foo/bar/baz", "foo/bara", "foo/bar/bax", "foo/bar"})
require.Equal(t, []string{"foo/bar", "foo/bara"}, res)
}

func TestExecOp_getMountDeps(t *testing.T) {
v1 := &vertex{name: "local://context"}
v2 := &vertex{
name: "foo",
inputs: []solver.Edge{
{Vertex: v1, Index: 0},
},
}
op2, err := NewExecOp(v2, &pb.Op_Exec{
Exec: &pb.ExecOp{
Meta: &pb.Meta{
Args: []string{"/bin/bash", "-l"},
},
Mounts: []*pb.Mount{
{
Input: pb.Empty,
Dest: "/",
Readonly: true,
Output: pb.SkipOutput,
},
{
Input: pb.InputIndex(0),
Selector: "b.txt",
Dest: "/test/b.txt",
Output: pb.SkipOutput,
},
{
Input: pb.InputIndex(0),
Dest: "/test/data",
Output: pb.SkipOutput,
},
},
},
}, nil, nil, nil, nil, nil, nil)
require.NoError(t, err)

deps, err := op2.getMountDeps()
require.NoError(t, err)

require.Len(t, deps, 1)
require.Len(t, deps[0].Selectors, 0)
require.False(t, deps[0].NoContentBasedHash)
}

type vertex struct {
name string
inputs []solver.Edge
}

func (v *vertex) Digest() digest.Digest {
return digest.FromString(v.name)
}

func (v *vertex) Sys() interface{} {
return v
}

func (v *vertex) Options() solver.VertexOptions {
return solver.VertexOptions{}
}

func (v *vertex) Inputs() []solver.Edge {
return v.inputs
}

func (v *vertex) Name() string {
return v.name
}

0 comments on commit 77b9e11

Please sign in to comment.