Do not include a cache mount's ID in the ExecOp's cachemap

A cache ID should not have any impact on whether or not a step should be
re-run any more than the content of that cache does (or rather,
doesn't).

Signed-off-by: Brian Goff <[email protected]>

solver/llbsolver/ops/exec.go

@@ -90,20 +90,52 @@ func cloneExecOp(old *pb.ExecOp) pb.ExecOp {
 	n.Mounts = nil
 	for i := range old.Mounts {
 		m := *old.Mounts[i]
+
+		if m.CacheOpt != nil {
+			co := *m.CacheOpt
+			m.CacheOpt = &co
+		}
+
 		n.Mounts = append(n.Mounts, &m)
 	}
 	return n
 }
 
 func (e *ExecOp) CacheMap(ctx context.Context, g session.Group, index int) (*solver.CacheMap, bool, error) {
 	op := cloneExecOp(e.op)
+
 	for i := range op.Meta.ExtraHosts {
 		h := op.Meta.ExtraHosts[i]
 		h.IP = ""
 		op.Meta.ExtraHosts[i] = h
 	}
+
+	checkShouldClearCacheOpts := func(m *pb.Mount) bool {
+		if m.CacheOpt == nil {
+			return false
+		}
+
+		if m.CacheOpt.ID == m.Dest {
+			return false
+		}
+
+		_, trimmed, ok := strings.Cut(m.CacheOpt.ID, "/")
+		if ok && trimmed == m.Dest {
+			// This is probably a dockerfile cache "namespace"
+			return false
+		}
+
+		return true
+	}
+
 	for i := range op.Mounts {
-		op.Mounts[i].Selector = ""
+		m := op.Mounts[i]
+		m.Selector = ""
+
+		if checkShouldClearCacheOpts(m) {
+			m.CacheOpt.ID = ""
+			m.CacheOpt.Sharing = 0
+		}
 	}
 	op.Meta.ProxyEnv = nil
 

solver/llbsolver/ops/exec_test.go

@@ -1,8 +1,12 @@
 package ops
 
 import (
+	"context"
 	"testing"
 
+	"github.com/moby/buildkit/identity"
+	"github.com/moby/buildkit/session"
+	"github.com/moby/buildkit/solver/pb"
 	"github.com/stretchr/testify/require"
 )
 
@@ -28,3 +32,74 @@ func TestDedupePaths(t *testing.T) {
 	res = dedupePaths([]string{"/", "/foo"})
 	require.Equal(t, []string{"/"}, res)
 }
+
+func TestExecOpCacheMap(t *testing.T) {
+	type testCase struct {
+		name     string
+		op1, op2 *ExecOp
+		xMatch   bool
+	}
+
+	newExecOp := func(opts ...func(*ExecOp)) *ExecOp {
+		op := &ExecOp{op: &pb.ExecOp{Meta: &pb.Meta{}}}
+		for _, opt := range opts {
+			opt(op)
+		}
+		return op
+	}
+
+	withNewMount := func(p string, cache *pb.CacheOpt) func(*ExecOp) {
+		return func(op *ExecOp) {
+			m := &pb.Mount{
+				Dest:  p,
+				Input: pb.InputIndex(op.numInputs),
+				// Generate a new selector for each mount since this should not effect the cache key.
+				// This helps exercise that code path.
+				Selector: identity.NewID(),
+			}
+			if cache != nil {
+				m.CacheOpt = cache
+				m.MountType = pb.MountType_CACHE
+			}
+			op.op.Mounts = append(op.op.Mounts, m)
+			op.numInputs++
+		}
+	}
+
+	withEmptyMounts := func(op *ExecOp) {
+		op.op.Mounts = []*pb.Mount{}
+	}
+
+	testCases := []testCase{
+		{name: "empty", op1: newExecOp(), op2: newExecOp(), xMatch: true},
+		{name: "empty vs with non-nil but empty mounts should match", op1: newExecOp(), op2: newExecOp(withEmptyMounts), xMatch: true},
+		{name: "both non-nil but empty mounts should match", op1: newExecOp(withEmptyMounts), op2: newExecOp(withEmptyMounts), xMatch: true},
+		{name: "non-nil but empty mounts vs with mounts should not match", op1: newExecOp(withEmptyMounts), op2: newExecOp(withNewMount("/foo", nil)), xMatch: false},
+		{name: "mounts to different paths should not match", op1: newExecOp(withNewMount("/foo", nil)), op2: newExecOp(withNewMount("/bar", nil)), xMatch: false},
+		{name: "mounts to same path should match", op1: newExecOp(withNewMount("/foo", nil)), op2: newExecOp(withNewMount("/foo", nil)), xMatch: true},
+		{name: "cache mount should not match non-cache mount at same path", op1: newExecOp(withNewMount("/foo", &pb.CacheOpt{ID: "someID"})), op2: newExecOp(withNewMount("/foo", nil)), xMatch: false},
+		{name: "different cache id's at the same path should match", op1: newExecOp(withNewMount("/foo", &pb.CacheOpt{ID: "someID"})), op2: newExecOp(withNewMount("/foo", &pb.CacheOpt{ID: "someOtherID"})), xMatch: true},
+	}
+
+	ctx := context.Background()
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			m1, ok, err := tc.op1.CacheMap(ctx, session.NewGroup(t.Name()), 1)
+			require.NoError(t, err)
+			require.True(t, ok)
+
+			m2, ok, err := tc.op2.CacheMap(ctx, session.NewGroup(t.Name()), 1)
+			require.NoError(t, err)
+			require.True(t, ok)
+
+			if tc.xMatch {
+				require.Equal(t, m1.Digest, m2.Digest, "\n\nm1: %+v\nm2: %+v", m1, m2)
+			} else {
+				require.NotEqual(t, m1.Digest, m2.Digest, "\n\nm1: %+v\nm2: %+v", m1, m2)
+			}
+		})
+	}
+}