diff --git a/cache/contenthash/checksum.go b/cache/contenthash/checksum.go index dbc2d9187cb0..e505d05eac23 100644 --- a/cache/contenthash/checksum.go +++ b/cache/contenthash/checksum.go @@ -506,21 +506,44 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o txn := cc.tree.Txn() root = txn.Root() var ( - updated bool - iter *iradix.Iterator - k []byte - kOk bool + updated bool + iter *iradix.Iterator + k []byte + kOk bool + origPrefix string + resolvedPrefix string ) iter = root.Iterator() if opts.Wildcard { - k, _, kOk = iter.Next() + origPrefix, k, kOk, err = wildcardPrefix(root, p) + if err != nil { + return nil, err + } } else { - k = convertPathToKey([]byte(p)) - if _, kOk = root.Get(k); kOk { + origPrefix = p + k = convertPathToKey([]byte(origPrefix)) + + // We need to resolve symlinks here, in case the base path + // involves a symlink. That will match fsutil behavior of + // calling functions such as stat and walk. + var cr *CacheRecord + k, cr, err = getFollowLinks(root, k, true) + if err != nil { + return nil, err + } + kOk = (cr != nil) + } + + if origPrefix != "" { + if kOk { iter.SeekLowerBound(append(append([]byte{}, k...), 0)) } + + resolvedPrefix = string(convertKeyToPath(k)) + } else { + k, _, kOk = iter.Next() } var ( @@ -531,6 +554,20 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o for kOk { fn := string(convertKeyToPath(k)) + // Convert the path prefix from what we found in the prefix + // tree to what the argument specified. + // + // For example, if the original 'p' argument was /a/b and there + // is a symlink a->c, we want fn to be /a/b/foo rather than + // /c/b/foo. This is necessary to ensure correct pattern + // matching. + // + // When wildcards are enabled, this translation applies to the + // portion of 'p' before any wildcards. + if strings.HasPrefix(fn, resolvedPrefix) { + fn = origPrefix + strings.TrimPrefix(fn, resolvedPrefix) + } + for len(parentDirHeaders) != 0 { lastParentDir := parentDirHeaders[len(parentDirHeaders)-1] if strings.HasPrefix(fn, lastParentDir.Path+"/") { @@ -570,8 +607,7 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o } } else { if !strings.HasPrefix(fn+"/", p+"/") { - k, _, kOk = iter.Next() - continue + break } shouldInclude, err = shouldIncludePath(strings.TrimSuffix(strings.TrimPrefix(fn+"/", p+"/"), "/"), includePatternMatcher, excludePatternMatcher) @@ -648,6 +684,82 @@ func shouldIncludePath( return true, nil } +func wildcardPrefix(root *iradix.Node, p string) (string, []byte, bool, error) { + // For consistency with what the copy implementation in fsutil + // does: split pattern into non-wildcard prefix and rest of + // pattern, then follow symlinks when resolving the non-wildcard + // prefix. + + d1, d2 := splitWildcards(p) + if d1 == "/" { + return "", nil, false, nil + } + + linksWalked := 0 + k, cr, err := getFollowLinksWalk(root, convertPathToKey([]byte(d1)), true, &linksWalked) + if err != nil { + return "", k, false, err + } + + if d2 != "" && cr != nil && cr.Type == CacheRecordTypeSymlink { + // getFollowLinks only handles symlinks in path + // components before the last component, so + // handle last component in d1 specially. + resolved := string(convertKeyToPath(k)) + for { + v, ok := root.Get(k) + + if !ok { + return d1, k, false, nil + } + if v.(*CacheRecord).Type != CacheRecordTypeSymlink { + break + } + + linksWalked++ + if linksWalked > 255 { + return "", k, false, errors.Errorf("too many links") + } + + resolved := cleanLink(resolved, v.(*CacheRecord).Linkname) + k = convertPathToKey([]byte(resolved)) + } + } + return d1, k, cr != nil, nil +} + +func splitWildcards(p string) (d1, d2 string) { + parts := strings.Split(path.Join(p), "/") + var p1, p2 []string + var found bool + for _, p := range parts { + if !found && containsWildcards(p) { + found = true + } + if p == "" { + p = "/" + } + if !found { + p1 = append(p1, p) + } else { + p2 = append(p2, p) + } + } + return filepath.Join(p1...), filepath.Join(p2...) +} + +func containsWildcards(name string) bool { + for i := 0; i < len(name); i++ { + ch := name[i] + if ch == '\\' { + i++ + } else if ch == '*' || ch == '?' || ch == '[' { + return true + } + } + return false +} + func (cc *cacheContext) checksumNoFollow(ctx context.Context, m *mount, p string) (*CacheRecord, error) { p = keyPath(p) @@ -929,14 +1041,8 @@ func getFollowLinksWalk(root *iradix.Node, k []byte, follow bool, linksWalked *i if *linksWalked > 255 { return nil, nil, errors.Errorf("too many links") } - dirPath := path.Clean(string(convertKeyToPath(dir))) - if dirPath == "." || dirPath == "/" { - dirPath = "" - } - link := path.Clean(parent.Linkname) - if !path.IsAbs(link) { - link = path.Join("/", path.Join(path.Dir(dirPath), link)) - } + + link := cleanLink(string(convertKeyToPath(dir)), parent.Linkname) return getFollowLinksWalk(root, append(convertPathToKey([]byte(link)), file...), follow, linksWalked) } } @@ -948,6 +1054,18 @@ func getFollowLinksWalk(root *iradix.Node, k []byte, follow bool, linksWalked *i return k, nil, nil } +func cleanLink(dir, linkname string) string { + dirPath := path.Clean(dir) + if dirPath == "." || dirPath == "/" { + dirPath = "" + } + link := path.Clean(linkname) + if !path.IsAbs(link) { + return path.Join("/", path.Join(path.Dir(dirPath), link)) + } + return link +} + func prepareDigest(fp, p string, fi os.FileInfo) (digest.Digest, error) { h, err := NewFileHash(fp, fi) if err != nil { diff --git a/cache/contenthash/checksum_test.go b/cache/contenthash/checksum_test.go index c83b3c02c61a..a674b7ef49a8 100644 --- a/cache/contenthash/checksum_test.go +++ b/cache/contenthash/checksum_test.go @@ -639,8 +639,6 @@ func TestChecksumIncludeDoubleStar(t *testing.T) { } func TestChecksumIncludeSymlink(t *testing.T) { - t.Skip("Test will fail until https://github.com/moby/buildkit/issues/2300 is fixed") - t.Parallel() tmpdir, err := ioutil.TempDir("", "buildkit-state") require.NoError(t, err) @@ -653,10 +651,13 @@ func TestChecksumIncludeSymlink(t *testing.T) { ch := []string{ "ADD data dir", - "ADD data/d1 dir", + "ADD data/d0 dir", + "ADD data/d0/d1 dir", + "ADD data/d0/d1/d2 dir", "ADD mnt dir", "ADD mnt/data symlink ../data", - "ADD data/d1/foo file abc", + "ADD data/d0/d1/d2/foo file abc", + "ADD data/symlink-to-d0 symlink d0", } ref := createRef(t, cm, ch) @@ -664,20 +665,50 @@ func TestChecksumIncludeSymlink(t *testing.T) { cc, err := newCacheContext(ref.Metadata(), nil) require.NoError(t, err) - dgst, err := cc.Checksum(context.TODO(), ref, "data/d1", ChecksumOpts{IncludePatterns: []string{"**/foo"}}, nil) + dgstD0, err := cc.Checksum(context.TODO(), ref, "data/d0", ChecksumOpts{IncludePatterns: []string{"**/foo"}}, nil) require.NoError(t, err) // File should be included - require.NotEqual(t, digest.FromBytes([]byte{}), dgst) + require.NotEqual(t, digest.FromBytes([]byte{}), dgstD0) - dgst, err = cc.Checksum(context.TODO(), ref, "mnt/data/d1", ChecksumOpts{IncludePatterns: []string{"**/foo"}}, nil) + dgstMntD0, err := cc.Checksum(context.TODO(), ref, "mnt/data/d0", ChecksumOpts{IncludePatterns: []string{"**/foo"}}, nil) require.NoError(t, err) // File should be included despite symlink - require.NotEqual(t, digest.FromBytes([]byte{}), dgst) + require.Equal(t, dgstD0, dgstMntD0) + + dgstD2, err := cc.Checksum(context.TODO(), ref, "data/d0/d1/d2", ChecksumOpts{IncludePatterns: []string{"**/foo"}}, nil) + require.NoError(t, err) + // File should be included + require.NotEqual(t, digest.FromBytes([]byte{}), dgstD2) + + dgstMntD2, err := cc.Checksum(context.TODO(), ref, "mnt/data/d0/d1/d2", ChecksumOpts{IncludePatterns: []string{"**/foo"}}, nil) + require.NoError(t, err) + // File should be included despite symlink + require.Equal(t, dgstD2, dgstMntD2) // Same, with Wildcard = true - dgst, err = cc.Checksum(context.TODO(), ref, "mnt/data/d1", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) + dgstMntD0Wildcard, err := cc.Checksum(context.TODO(), ref, "mnt/data/d0", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) + require.NoError(t, err) + require.Equal(t, dgstD0, dgstMntD0Wildcard) + + dgstMntD0Wildcard2, err := cc.Checksum(context.TODO(), ref, "mnt/data/d*", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) + require.NoError(t, err) + require.Equal(t, dgstD0, dgstMntD0Wildcard2) + + dgstMntD2Wildcard, err := cc.Checksum(context.TODO(), ref, "mnt/data/d0/d1/d2", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) + require.NoError(t, err) + require.Equal(t, dgstD2, dgstMntD2Wildcard) + + dgstMntD2Wildcard2, err := cc.Checksum(context.TODO(), ref, "mnt/data/d0/d1/d*", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) + require.NoError(t, err) + require.Equal(t, dgstD2, dgstMntD2Wildcard2) + + dgstMntInnerWildcard, err := cc.Checksum(context.TODO(), ref, "mnt/data/d0/d*/d2", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) + require.NoError(t, err) + require.Equal(t, dgstD2, dgstMntInnerWildcard) + + dgstMntInnerWildcard2, err := cc.Checksum(context.TODO(), ref, "mnt/data/symlink-to-d0/d*/d2", ChecksumOpts{IncludePatterns: []string{"**/foo"}, Wildcard: true}, nil) require.NoError(t, err) - require.NotEqual(t, digest.FromBytes([]byte{}), dgst) + require.Equal(t, dgstD2, dgstMntInnerWildcard2) } func TestHandleChange(t *testing.T) {