From dc608427ea01e4217946cff042f0e90a9f800574 Mon Sep 17 00:00:00 2001
From: "Jonathan A. Sternberg" <jonathan.sternberg@docker.com>
Date: Fri, 22 Sep 2023 12:28:44 -0500
Subject: [PATCH] solver: correctly set the content selector with multiple bind
 mounts references

Correctly set the content based selector when multiple bind mounts refer
to the same source. Previously, a selector that referred to the root
filesystem would be ignored. This is because a blank selector refers to
the root filesystem.

When two bind mounts referred to the same dependency, one mount would
add a selector while the other would be skipped. This caused the cache
key to be only computed based on the more narrow filesystem which caused
erroneous cache hits.

Now, the creation of the selector includes the root filesystem for
consideration. It fills in `/` as the selector and then removes it later
so that we don't narrow the selection in an invalid way.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
---
 solver/llbsolver/ops/exec.go      | 22 +++++++---
 solver/llbsolver/ops/exec_test.go | 72 +++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+), 5 deletions(-)

diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go
index eee0dd39fb6e..4966269262b7 100644
--- a/solver/llbsolver/ops/exec.go
+++ b/solver/llbsolver/ops/exec.go
@@ -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
 }
 
diff --git a/solver/llbsolver/ops/exec_test.go b/solver/llbsolver/ops/exec_test.go
index fcbf2799c654..bd40b81e74e2 100644
--- a/solver/llbsolver/ops/exec_test.go
+++ b/solver/llbsolver/ops/exec_test.go
@@ -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"
 )
 
@@ -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
+}