exec: fix pruning cache mounts with parent ref on no-cache

On a build with no-cache, cache mounts were not pruned correctly
if the mount was on top of another ref. This also appeared in
Dockerfile when mode/uid/gid was set because implicit parent
ref is created in these cases in order to change the permissions
of a subdir that is used as a cache mount base.

Because it is not possible to know ahead of time what ref
will become the parent of cache mount during build, all cache
mounts matching the ID that have a parent will be pruned.

Signed-off-by: Tonis Tiigi <[email protected]>

cache/metadata.go

@@ -44,7 +44,7 @@ const blobchainIndex = "blobchainid:"
 const chainIndex = "chainid:"
 
 type MetadataStore interface {
-	Search(context.Context, string) ([]RefMetadata, error)
+	Search(context.Context, string, bool) ([]RefMetadata, error)
 }
 
 type RefMetadata interface {
@@ -71,6 +71,7 @@ type RefMetadata interface {
 
 	// generic getters/setters for external packages
 	GetString(string) string
+	Get(string) *metadata.Value
 	SetString(key, val, index string) error
 
 	GetExternal(string) ([]byte, error)
@@ -79,15 +80,15 @@ type RefMetadata interface {
 	ClearValueAndIndex(string, string) error
 }
 
-func (cm *cacheManager) Search(ctx context.Context, idx string) ([]RefMetadata, error) {
+func (cm *cacheManager) Search(ctx context.Context, idx string, prefixOnly bool) ([]RefMetadata, error) {
 	cm.mu.Lock()
 	defer cm.mu.Unlock()
-	return cm.search(ctx, idx)
+	return cm.search(ctx, idx, prefixOnly)
 }
 
 // callers must hold cm.mu lock
-func (cm *cacheManager) search(ctx context.Context, idx string) ([]RefMetadata, error) {
-	sis, err := cm.MetadataStore.Search(ctx, idx)
+func (cm *cacheManager) search(ctx context.Context, idx string, prefixOnly bool) ([]RefMetadata, error) {
+	sis, err := cm.MetadataStore.Search(ctx, idx, prefixOnly)
 	if err != nil {
 		return nil, err
 	}
@@ -119,12 +120,12 @@ func (cm *cacheManager) getMetadata(id string) (*cacheMetadata, bool) {
 
 // callers must hold cm.mu lock
 func (cm *cacheManager) searchBlobchain(ctx context.Context, id digest.Digest) ([]RefMetadata, error) {
-	return cm.search(ctx, blobchainIndex+id.String())
+	return cm.search(ctx, blobchainIndex+id.String(), false)
 }
 
 // callers must hold cm.mu lock
 func (cm *cacheManager) searchChain(ctx context.Context, id digest.Digest) ([]RefMetadata, error) {
-	return cm.search(ctx, chainIndex+id.String())
+	return cm.search(ctx, chainIndex+id.String(), false)
 }
 
 type cacheMetadata struct {
@@ -486,6 +487,10 @@ func (md *cacheMetadata) GetString(key string) string {
 	return str
 }
 
+func (md *cacheMetadata) Get(key string) *metadata.Value {
+	return md.si.Get(key)
+}
+
 func (md *cacheMetadata) GetStringSlice(key string) []string {
 	v := md.si.Get(key)
 	if v == nil {

cache/metadata/metadata.go

@@ -83,7 +83,7 @@ func (s *Store) Probe(index string) (bool, error) {
 	return exists, errors.WithStack(err)
 }
 
-func (s *Store) Search(ctx context.Context, index string) ([]*StorageItem, error) {
+func (s *Store) Search(ctx context.Context, index string, prefix bool) ([]*StorageItem, error) {
 	var out []*StorageItem
 	err := s.db.View(func(tx *bolt.Tx) error {
 		b := tx.Bucket([]byte(indexBucket))
@@ -94,12 +94,18 @@ func (s *Store) Search(ctx context.Context, index string) ([]*StorageItem, error
 		if main == nil {
 			return nil
 		}
-		index = indexKey(index, "")
+		if !prefix {
+			index = indexKey(index, "")
+		}
 		c := b.Cursor()
 		k, _ := c.Seek([]byte(index))
 		for {
 			if k != nil && strings.HasPrefix(string(k), index) {
-				itemID := strings.TrimPrefix(string(k), index)
+				idx := strings.LastIndex(string(k), "::")
+				if idx == -1 {
+					continue
+				}
+				itemID := string(k[idx+2:])
 				k, _ = c.Next()
 				b := main.Bucket([]byte(itemID))
 				if b == nil {

cache/metadata/metadata_test.go

@@ -142,14 +142,14 @@ func TestIndexes(t *testing.T) {
 	}
 
 	ctx := context.Background()
-	sis, err := s.Search(ctx, "tag:baz")
+	sis, err := s.Search(ctx, "tag:baz", false)
 	require.NoError(t, err)
 	require.Equal(t, 2, len(sis))
 
 	require.Equal(t, "foo1", sis[0].ID())
 	require.Equal(t, "foo3", sis[1].ID())
 
-	sis, err = s.Search(ctx, "tag:bax")
+	sis, err = s.Search(ctx, "tag:bax", false)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(sis))
 
@@ -158,7 +158,7 @@ func TestIndexes(t *testing.T) {
 	err = s.Clear("foo1")
 	require.NoError(t, err)
 
-	sis, err = s.Search(ctx, "tag:baz")
+	sis, err = s.Search(ctx, "tag:baz", false)
 	require.NoError(t, err)
 	require.Equal(t, 1, len(sis))
 

frontend/dockerfile/dockerfile2llb/convert_runmount.go

@@ -57,7 +57,7 @@ func setCacheUIDGID(m *instructions.Mount, st llb.State) llb.State {
 	if m.Mode != nil {
 		mode = os.FileMode(*m.Mode)
 	}
-	return st.File(llb.Mkdir("/cache", mode, llb.WithUIDGID(uid, gid)), llb.WithCustomName("[internal] settings cache mount permissions"))
+	return st.File(llb.Mkdir("/cache", mode, llb.WithUIDGID(uid, gid)), llb.WithCustomName("[internal] setting cache mount permissions"))
 }
 
 func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*dispatchState, opt dispatchOpt) ([]llb.RunOption, error) {

frontend/dockerfile/dockerfile_test.go

@@ -100,6 +100,7 @@ var allTests = integration.TestFuncs(
 	testReproducibleIDs,
 	testImportExportReproducibleIDs,
 	testNoCache,
+	testCacheMountModeNoCache,
 	testDockerfileFromHTTP,
 	testBuiltinArgs,
 	testPullScratch,
@@ -5065,6 +5066,74 @@ COPY --from=s1 unique2 /
 	require.NotEqual(t, string(unique2Dir1), string(unique2Dir3))
 }
 
+// moby/buildkit#5305
+func testCacheMountModeNoCache(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox AS base
+ARG FOO=abc
+RUN --mount=type=cache,target=/cache,mode=0773 touch /cache/$FOO && ls -l /cache | wc -l > /out
+
+FROM scratch
+COPY --from=base /out /
+`)
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	destDir := t.TempDir()
+
+	opt := client.SolveOpt{
+		FrontendAttrs: map[string]string{},
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}
+
+	_, err = f.Solve(sb.Context(), c, opt, nil)
+	require.NoError(t, err)
+
+	opt.FrontendAttrs["no-cache"] = ""
+
+	dt, err := os.ReadFile(filepath.Join(destDir, "out"))
+	require.NoError(t, err)
+	require.Equal(t, "2\n", string(dt))
+
+	opt.FrontendAttrs["build-arg:FOO"] = "def"
+
+	_, err = f.Solve(sb.Context(), c, opt, nil)
+	require.NoError(t, err)
+
+	dt, err = os.ReadFile(filepath.Join(destDir, "out"))
+	require.NoError(t, err)
+	require.Equal(t, "2\n", string(dt))
+
+	// safety check without no-cache
+	delete(opt.FrontendAttrs, "no-cache")
+	opt.FrontendAttrs["build-arg:FOO"] = "ghi"
+
+	_, err = f.Solve(sb.Context(), c, opt, nil)
+	require.NoError(t, err)
+
+	dt, err = os.ReadFile(filepath.Join(destDir, "out"))
+	require.NoError(t, err)
+	require.Equal(t, "3\n", string(dt))
+}
+
 func testPlatformArgsImplicit(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)

solver/llbsolver/bridge.go

@@ -144,12 +144,8 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp
 	}
 
 	if len(dpc.ids) > 0 {
-		ids := make([]string, 0, len(dpc.ids))
-		for id := range dpc.ids {
-			ids = append(ids, id)
-		}
 		if err := b.eachWorker(func(w worker.Worker) error {
-			return w.PruneCacheMounts(ctx, ids)
+			return w.PruneCacheMounts(ctx, dpc.ids)
 		}); err != nil {
 			return nil, err
 		}

solver/llbsolver/mounts/mount.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -116,7 +117,7 @@ func (g *cacheRefGetter) getRefCacheDirNoCache(ctx context.Context, key string,
 	cacheRefsLocker.Lock(key)
 	defer cacheRefsLocker.Unlock(key)
 	for {
-		sis, err := SearchCacheDir(ctx, g.cm, key)
+		sis, err := SearchCacheDir(ctx, g.cm, key, false)
 		if err != nil {
 			return nil, err
 		}
@@ -542,13 +543,24 @@ func (r *cacheRef) Release(ctx context.Context) error {
 const keyCacheDir = "cache-dir"
 const cacheDirIndex = keyCacheDir + ":"
 
-func SearchCacheDir(ctx context.Context, store cache.MetadataStore, id string) ([]CacheRefMetadata, error) {
+func SearchCacheDir(ctx context.Context, store cache.MetadataStore, id string, withNested bool) ([]CacheRefMetadata, error) {
 	var results []CacheRefMetadata
-	mds, err := store.Search(ctx, cacheDirIndex+id)
+	key := cacheDirIndex + id
+	if withNested {
+		key += ":"
+	}
+	mds, err := store.Search(ctx, key, withNested)
 	if err != nil {
 		return nil, err
 	}
 	for _, md := range mds {
+		if withNested {
+			v := md.Get(keyCacheDir)
+			// skip partial ids but allow id without ref ID
+			if v == nil || v.Index != key && !strings.HasPrefix(v.Index, key) {
+				continue
+			}
+		}
 		results = append(results, CacheRefMetadata{md})
 	}
 	return results, nil

solver/llbsolver/vertex.go

@@ -125,7 +125,7 @@ func ValidateEntitlements(ent entitlements.Set) LoadOpt {
 }
 
 type detectPrunedCacheID struct {
-	ids map[string]struct{}
+	ids map[string]bool
 }
 
 func (dpc *detectPrunedCacheID) Load(op *pb.Op, md *pb.OpMetadata, opt *solver.VertexOptions) error {
@@ -142,9 +142,10 @@ func (dpc *detectPrunedCacheID) Load(op *pb.Op, md *pb.OpMetadata, opt *solver.V
 						id = m.Dest
 					}
 					if dpc.ids == nil {
-						dpc.ids = map[string]struct{}{}
+						dpc.ids = map[string]bool{}
 					}
-					dpc.ids[id] = struct{}{}
+					// value shows in mount is on top of a ref
+					dpc.ids[id] = m.Input != -1
 				}
 			}
 		}

source/git/source.go

@@ -746,7 +746,7 @@ const gitSnapshotIndex = keyGitSnapshot + "::"
 
 func search(ctx context.Context, store cache.MetadataStore, key string, idx string) ([]cacheRefMetadata, error) {
 	var results []cacheRefMetadata
-	mds, err := store.Search(ctx, idx+key)
+	mds, err := store.Search(ctx, idx+key, false)
 	if err != nil {
 		return nil, err
 	}

source/http/source.go

@@ -480,7 +480,7 @@ func getFileName(urlStr, manualFilename string, resp *http.Response) string {
 
 func searchHTTPURLDigest(ctx context.Context, store cache.MetadataStore, dgst digest.Digest) ([]cacheRefMetadata, error) {
 	var results []cacheRefMetadata
-	mds, err := store.Search(ctx, string(dgst))
+	mds, err := store.Search(ctx, string(dgst), false)
 	if err != nil {
 		return nil, err
 	}

source/local/source.go

@@ -334,7 +334,7 @@ const sharedKeyIndex = keySharedKey + ":"
 
 func searchSharedKey(ctx context.Context, store cache.MetadataStore, k string) ([]cacheRefMetadata, error) {
 	var results []cacheRefMetadata
-	mds, err := store.Search(ctx, sharedKeyIndex+k)
+	mds, err := store.Search(ctx, sharedKeyIndex+k, false)
 	if err != nil {
 		return nil, err
 	}

worker/base/worker.go

@@ -341,13 +341,13 @@ func (w *Worker) ResolveOp(v solver.Vertex, s frontend.FrontendLLBBridge, sm *se
 	return nil, errors.Errorf("could not resolve %v", v)
 }
 
-func (w *Worker) PruneCacheMounts(ctx context.Context, ids []string) error {
+func (w *Worker) PruneCacheMounts(ctx context.Context, ids map[string]bool) error {
 	mu := mounts.CacheMountsLocker()
 	mu.Lock()
 	defer mu.Unlock()
 
-	for _, id := range ids {
-		mds, err := mounts.SearchCacheDir(ctx, w.CacheMgr, id)
+	for id, nested := range ids {
+		mds, err := mounts.SearchCacheDir(ctx, w.CacheMgr, id, nested)
 		if err != nil {
 			return err
 		}

worker/worker.go

@@ -35,7 +35,7 @@ type Worker interface {
 	Exporter(name string, sm *session.Manager) (exporter.Exporter, error)
 	Prune(ctx context.Context, ch chan client.UsageInfo, opt ...client.PruneInfo) error
 	FromRemote(ctx context.Context, remote *solver.Remote) (cache.ImmutableRef, error)
-	PruneCacheMounts(ctx context.Context, ids []string) error
+	PruneCacheMounts(ctx context.Context, ids map[string]bool) error
 	ContentStore() *containerdsnapshot.Store
 	Executor() executor.Executor
 	CacheManager() cache.Manager