Merge pull request #2076 from aaronlehmann/missing-mounts-in-cachemap

Fix missing mounts in execOp cache map

client/client_test.go

@@ -98,6 +98,7 @@ func TestIntegration(t *testing.T) {
 		testSharedCacheMounts,
 		testLockedCacheMounts,
 		testDuplicateCacheMount,
+		testRunCacheWithMounts,
 		testParallelLocalBuilds,
 		testSecretMounts,
 		testExtraHosts,
@@ -2771,6 +2772,33 @@ func testDuplicateCacheMount(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 }
 
+func testRunCacheWithMounts(t *testing.T, sb integration.Sandbox) {
+	requiresLinux(t)
+	c, err := New(context.TODO(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	busybox := llb.Image("busybox:latest")
+
+	out := busybox.Run(llb.Shlex(`sh -e -c "[[ -f /m1/sbin/apk ]]"`))
+	out.AddMount("/m1", llb.Image("alpine:latest"), llb.Readonly)
+
+	def, err := out.Marshal(context.TODO())
+	require.NoError(t, err)
+
+	_, err = c.Solve(context.TODO(), def, SolveOpt{}, nil)
+	require.NoError(t, err)
+
+	out = busybox.Run(llb.Shlex(`sh -e -c "[[ -f /m1/sbin/apk ]]"`))
+	out.AddMount("/m1", llb.Image("busybox:latest"), llb.Readonly)
+
+	def, err = out.Marshal(context.TODO())
+	require.NoError(t, err)
+
+	_, err = c.Solve(context.TODO(), def, SolveOpt{}, nil)
+	require.Error(t, err)
+}
+
 func testCacheMountNoCache(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
 	c, err := New(context.TODO(), sb.Address())

solver/llbsolver/ops/exec.go

@@ -69,8 +69,8 @@ func cloneExecOp(old *pb.ExecOp) pb.ExecOp {
 	}
 	n.Meta = &meta
 	n.Mounts = nil
-	for i := range n.Mounts {
-		m := *n.Mounts[i]
+	for i := range old.Mounts {
+		m := *old.Mounts[i]
 		n.Mounts = append(n.Mounts, &m)
 	}
 	return n
@@ -97,6 +97,22 @@ func (e *execOp) CacheMap(ctx context.Context, g session.Group, index int) (*sol
 		}
 	}
 
+	// Special case for cache compatibility with buggy versions that wrongly
+	// excluded Exec.Mounts: for the default case of one root mount (i.e. RUN
+	// inside a Dockerfile), do not include the mount when generating the cache
+	// map.
+	if len(op.Mounts) == 1 &&
+		op.Mounts[0].Dest == "/" &&
+		op.Mounts[0].Selector == "" &&
+		!op.Mounts[0].Readonly &&
+		op.Mounts[0].MountType == pb.MountType_BIND &&
+		op.Mounts[0].CacheOpt == nil &&
+		op.Mounts[0].SSHOpt == nil &&
+		op.Mounts[0].SecretOpt == nil &&
+		op.Mounts[0].ResultID == "" {
+		op.Mounts = nil
+	}
+
 	dt, err := json.Marshal(struct {
 		Type    string
 		Exec    *pb.ExecOp