JohnHammond
/
buildkit
mirror of https://github.com/JohnHammond/buildkit.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2318 from aaronlehmann/follow-links-includedpaths 

Follow links in includedPaths to resolve incorrect caching when source path is behind symlink
master
Tõnis Tiigi 2021-09-08 10:43:26 -07:00 committed by GitHub
parent 91d2f2dc17 e9e6cec838
commit f5eb400a85
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 176 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

152
cache/contenthash/checksum.go vendored
View File

@ -516,21 +516,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 (
@ -541,6 +564,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+"/") {
@ -591,8 +628,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(
@ -692,6 +728,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)
@ -973,14 +1085,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)
}
}
@ -992,6 +1098,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 {

51
cache/contenthash/checksum_test.go vendored
View File

@ -649,8 +649,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)
@ -663,10 +661,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)
@ -674,20 +675,50 @@ func TestChecksumIncludeSymlink(t *testing.T) {
cc, err := newCacheContext(ref)
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.NotEqual(t, digest.FromBytes([]byte{}), dgst)
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.Equal(t, dgstD2, dgstMntInnerWildcard2)
}
func TestHandleChange(t *testing.T) {