Skip to content

Commit

Permalink
contenthash: fix issues on symlinks on parent paths
Browse files Browse the repository at this point in the history 
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
  • Loading branch information
@tonistiigi
tonistiigi committed Sep 21, 2018
1 parent a93a96b commit 9bf5431
Show file tree
Hide file tree
Showing 3 changed files with 336 additions and 16 deletions.
118 changes: 102 additions & 16 deletions cache/contenthash/checksum.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"path/filepath"
"sync"

"github.com/containerd/continuity/fs"
"github.com/docker/docker/pkg/locker"
iradix "github.com/hashicorp/go-immutable-radix"
"github.com/hashicorp/golang-lru/simplelru"
Expand Down Expand Up @@ -400,7 +399,11 @@ func (cc *cacheContext) commitActiveTransaction() {

func (cc *cacheContext) lazyChecksum(ctx context.Context, m *mount, p string) (*CacheRecord, error) {
root := cc.tree.Root()
if cc.needsScan(root, p) {
scan, err := cc.needsScan(root, p)
if err != nil {
return nil, err
}
if scan {
if err := cc.scanPath(ctx, m, p); err != nil {
return nil, err
}
Expand All @@ -418,13 +421,13 @@ func (cc *cacheContext) lazyChecksum(ctx context.Context, m *mount, p string) (*
}

func (cc *cacheContext) checksum(ctx context.Context, root *iradix.Node, txn *iradix.Txn, m *mount, k []byte) (*CacheRecord, bool, error) {
v, ok := root.Get(k)

if !ok {
k, cr, err := getFollowLinks(root, k)
if err != nil {
return nil, false, err
}
if cr == nil {
return nil, false, errors.Wrapf(errNotFound, "%s not found", convertKeyToPath(k))
}
cr := v.(*CacheRecord)

if cr.Digest != "" {
return cr, false, nil
}
Expand Down Expand Up @@ -491,17 +494,37 @@ func (cc *cacheContext) checksum(ctx context.Context, root *iradix.Node, txn *ir
return cr2, true, nil
}

func (cc *cacheContext) needsScan(root *iradix.Node, p string) bool {
// needsScan returns false if path is in the tree or a parent path is in tree
// and subpath is missing
func (cc *cacheContext) needsScan(root *iradix.Node, p string) (bool, error) {
var linksWalked int
return cc.needsScanFollow(root, p, &linksWalked)
}

func (cc *cacheContext) needsScanFollow(root *iradix.Node, p string, linksWalked *int) (bool, error) {
if p == "/" {
p = ""
}
if _, ok := root.Get(convertPathToKey([]byte(p))); !ok {
if v, ok := root.Get(convertPathToKey([]byte(p))); !ok {
if p == "" {
return true
return true, nil
}
return cc.needsScanFollow(root, path.Clean(path.Dir(p)), linksWalked)
} else {
cr := v.(*CacheRecord)
if cr.Type == CacheRecordTypeSymlink {
if *linksWalked > 255 {
return false, errTooManyLinks
}
*linksWalked++
link := path.Clean(cr.Linkname)
if !path.IsAbs(cr.Linkname) {
link = path.Join("/", path.Dir(p), link)
}
return cc.needsScanFollow(root, link, linksWalked)
}
return cc.needsScan(root, path.Clean(path.Dir(p)))
}
return false
return false, nil
}

func (cc *cacheContext) scanPath(ctx context.Context, m *mount, p string) (retErr error) {
Expand All @@ -513,14 +536,23 @@ func (cc *cacheContext) scanPath(ctx context.Context, m *mount, p string) (retEr
return err
}

parentPath, err := fs.RootPath(mp, filepath.FromSlash(d))
n := cc.tree.Root()
txn := cc.tree.Txn()

parentPath, err := rootPath(mp, filepath.FromSlash(d), func(p, link string) error {
cr := &CacheRecord{
Type: CacheRecordTypeSymlink,
Linkname: filepath.ToSlash(link),
}
k := []byte(filepath.Join("/", filepath.ToSlash(p)))
k = convertPathToKey(k)
txn.Insert(k, cr)
return nil
})
if err != nil {
return err
}

n := cc.tree.Root()
txn := cc.tree.Txn()

err = filepath.Walk(parentPath, func(path string, fi os.FileInfo, err error) error {
if err != nil {
return errors.Wrapf(err, "failed to walk %s", path)
Expand Down Expand Up @@ -566,6 +598,45 @@ func (cc *cacheContext) scanPath(ctx context.Context, m *mount, p string) (retEr
return nil
}

func getFollowLinks(root *iradix.Node, k []byte) ([]byte, *CacheRecord, error) {
var linksWalked int
return getFollowLinksWalk(root, k, &linksWalked)
}

func getFollowLinksWalk(root *iradix.Node, k []byte, linksWalked *int) ([]byte, *CacheRecord, error) {
v, ok := root.Get(k)
if ok {
return k, v.(*CacheRecord), nil
}
if len(k) == 0 {
return nil, nil, nil
}

dir, file := splitKey(k)

_, parent, err := getFollowLinksWalk(root, dir, linksWalked)
if err != nil {
return nil, nil, err
}
if parent != nil && parent.Type == CacheRecordTypeSymlink {
*linksWalked++
if *linksWalked > 255 {
return nil, nil, errors.Errorf("too many links")
}
dirPath := path.Clean(string(convertKeyToPath(dir)))
if dirPath == "." || dirPath == "/" {
dirPath = ""
}
link := parent.Linkname
if !path.IsAbs(link) {
link = path.Join("/", path.Join(path.Dir(dirPath), link))
}
return getFollowLinksWalk(root, append(convertPathToKey([]byte(link)), file...), linksWalked)
}

return nil, nil, nil
}

func prepareDigest(fp, p string, fi os.FileInfo) (digest.Digest, error) {
h, err := NewFileHash(fp, fi)
if err != nil {
Expand Down Expand Up @@ -632,3 +703,18 @@ func convertPathToKey(p []byte) []byte {
func convertKeyToPath(p []byte) []byte {
return bytes.Replace([]byte(p), []byte{0}, []byte("/"), -1)
}

func splitKey(k []byte) ([]byte, []byte) {
foundBytes := false
i := len(k) - 1
for {
if i <= 0 || foundBytes && k[i] == 0 {
break
}
if k[i] != 0 {
foundBytes = true
}
i--
}
return append([]byte{}, k[:i]...), k[i:]
}
127 changes: 127 additions & 0 deletions cache/contenthash/checksum_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,133 @@ func TestChecksumUnorderedFiles(t *testing.T) {
require.NotEqual(t, dgst, dgst2)
}

func TestSymlinkInPathScan(t *testing.T) {
t.Parallel()
tmpdir, err := ioutil.TempDir("", "buildkit-state")
require.NoError(t, err)
defer os.RemoveAll(tmpdir)

snapshotter, err := native.NewSnapshotter(filepath.Join(tmpdir, "snapshots"))
require.NoError(t, err)
cm := setupCacheManager(t, tmpdir, snapshotter)
defer cm.Close()

ch := []string{
"ADD d0 dir",
"ADD d0/sub dir",
"ADD d0/sub/foo file data0",
"ADD d0/def symlink sub",
}
ref := createRef(t, cm, ch)

dgst, err := Checksum(context.TODO(), ref, "d0/def/foo")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)

dgst, err = Checksum(context.TODO(), ref, "d0/def/foo")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)

err = ref.Release(context.TODO())
require.NoError(t, err)
}

func TestSymlinkNeedsScan(t *testing.T) {
t.Parallel()
tmpdir, err := ioutil.TempDir("", "buildkit-state")
require.NoError(t, err)
defer os.RemoveAll(tmpdir)

snapshotter, err := native.NewSnapshotter(filepath.Join(tmpdir, "snapshots"))
require.NoError(t, err)
cm := setupCacheManager(t, tmpdir, snapshotter)
defer cm.Close()

ch := []string{
"ADD c0 dir",
"ADD c0/sub dir",
"ADD c0/sub/foo file data0",
"ADD d0 dir",
"ADD d0/d1 dir",
"ADD d0/d1/def symlink ../../c0/sub",
}
ref := createRef(t, cm, ch)

// scan the d0 path containing the symlink that doesn't get followed
_, err = Checksum(context.TODO(), ref, "d0/d1")
require.NoError(t, err)

dgst, err := Checksum(context.TODO(), ref, "d0/d1/def/foo")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)

err = ref.Release(context.TODO())
require.NoError(t, err)
}

func TestSymlinkInPathHandleChange(t *testing.T) {
t.Parallel()
tmpdir, err := ioutil.TempDir("", "buildkit-state")
require.NoError(t, err)
defer os.RemoveAll(tmpdir)

snapshotter, err := native.NewSnapshotter(filepath.Join(tmpdir, "snapshots"))
require.NoError(t, err)
cm := setupCacheManager(t, tmpdir, snapshotter)
defer cm.Close()

ch := []string{
"ADD d1 dir",
"ADD d1/sub dir",
"ADD d1/sub/foo file data0",
"ADD d1/sub/bar symlink /link",
"ADD d1/sub/baz symlink ../../../link",
"ADD d1/sub/bay symlink ../../../../link/.", // weird link
"ADD d1/def symlink sub",
"ADD sub dir",
"ADD sub/d0 dir",
"ADD sub/d0/abc file data0",
"ADD sub/d0/def symlink abc",
"ADD sub/d0/ghi symlink nosuchfile",
"ADD link symlink sub/d0",
}

ref := createRef(t, cm, nil)

cc, err := newCacheContext(ref.Metadata())
require.NoError(t, err)

err = emit(cc.HandleChange, changeStream(ch))
require.NoError(t, err)

dgst, err := cc.Checksum(context.TODO(), ref, "d1/def/foo")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)

dgst, err = cc.Checksum(context.TODO(), ref, "d1/def/bar/abc")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst)

dgstFileData0, err := cc.Checksum(context.TODO(), ref, "sub/d0")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgstDirD0)

dgstFileData0, err = cc.Checksum(context.TODO(), ref, "d1/def/baz")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgstDirD0)

dgstFileData0, err = cc.Checksum(context.TODO(), ref, "d1/def/bay")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgstDirD0)

dgstFileData0, err = cc.Checksum(context.TODO(), ref, "link")
require.NoError(t, err)
require.Equal(t, dgstFileData0, dgstDirD0)

err = ref.Release(context.TODO())
require.NoError(t, err)
}

func TestPersistence(t *testing.T) {
t.Parallel()
tmpdir, err := ioutil.TempDir("", "buildkit-state")
Expand Down

0 comments on commit 9bf5431

Please sign in to comment.