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

Fix IncludePattern/ExcludePattern matching 

The transformation to rootedPatterns seems very wrong and inconsistent
with what the copy logic did. Change it to match the copy logic, and add
more testing.

Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
master
Aaron Lehmann 2021-08-10 18:14:42 -07:00
parent b6ba966a68
commit de785737db
2 changed files with 63 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
cache/contenthash/checksum.go vendored
View File

@ -487,11 +487,7 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
var includePatternMatcher *fileutils.PatternMatcher var includePatternMatcher *fileutils.PatternMatcher
if len(opts.IncludePatterns) != 0 { if len(opts.IncludePatterns) != 0 {
rootedIncludePatterns := make([]string, len(opts.IncludePatterns)) includePatternMatcher, err = fileutils.NewPatternMatcher(opts.IncludePatterns)
for i, includePattern := range opts.IncludePatterns {
rootedIncludePatterns[i] = keyPath(includePattern)
}
includePatternMatcher, err = fileutils.NewPatternMatcher(rootedIncludePatterns)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "invalid includepatterns: %s", opts.IncludePatterns) return nil, errors.Wrapf(err, "invalid includepatterns: %s", opts.IncludePatterns)
} }
@ -499,11 +495,7 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
var excludePatternMatcher *fileutils.PatternMatcher var excludePatternMatcher *fileutils.PatternMatcher
if len(opts.ExcludePatterns) != 0 { if len(opts.ExcludePatterns) != 0 {
rootedExcludePatterns := make([]string, len(opts.ExcludePatterns)) excludePatternMatcher, err = fileutils.NewPatternMatcher(opts.ExcludePatterns)
for i, excludePattern := range opts.ExcludePatterns {
rootedExcludePatterns[i] = keyPath(excludePattern)
}
excludePatternMatcher, err = fileutils.NewPatternMatcher(rootedExcludePatterns)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "invalid excludepatterns: %s", opts.ExcludePatterns) return nil, errors.Wrapf(err, "invalid excludepatterns: %s", opts.ExcludePatterns)
} }
@ -557,8 +549,10 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
continue continue
} }
} }
var shouldInclude bool
if opts.Wildcard { if opts.Wildcard {
if lastMatchedDir == "" || !strings.HasPrefix(fn, lastMatchedDir+"/") { if p != "" && (lastMatchedDir == "" || !strings.HasPrefix(fn, lastMatchedDir+"/")) {
include, err := path.Match(p, fn) include, err := path.Match(p, fn)
if err != nil { if err != nil {
return nil, err return nil, err
@ -569,15 +563,23 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
} }
lastMatchedDir = fn lastMatchedDir = fn
} }
} else if !strings.HasPrefix(fn+"/", p+"/") {
shouldInclude, err = shouldIncludePath(strings.TrimSuffix(strings.TrimPrefix(fn+"/", lastMatchedDir+"/"), "/"), includePatternMatcher, excludePatternMatcher)
if err != nil {
return nil, err
}
} else {
if !strings.HasPrefix(fn+"/", p+"/") {
k, _, kOk = iter.Next() k, _, kOk = iter.Next()
continue continue
} }
shouldInclude, err := shouldIncludePath(fn, includePatternMatcher, excludePatternMatcher) shouldInclude, err = shouldIncludePath(strings.TrimSuffix(strings.TrimPrefix(fn+"/", p+"/"), "/"), includePatternMatcher, excludePatternMatcher)
if err != nil { if err != nil {
return nil, err return nil, err
} }
}
if !shouldInclude && !dirHeader { if !shouldInclude && !dirHeader {
k, _, kOk = iter.Next() k, _, kOk = iter.Next()
continue continue

66
cache/contenthash/checksum_test.go vendored
View File

@ -438,6 +438,14 @@ func TestChecksumBasicFile(t *testing.T) {
func TestChecksumIncludeExclude(t *testing.T) { func TestChecksumIncludeExclude(t *testing.T) {
t.Parallel() t.Parallel()
t.Run("wildcard_false", func(t *testing.T) { testChecksumIncludeExclude(t, false) })
t.Run("wildcard_true", func(t *testing.T) { testChecksumIncludeExclude(t, true) })
}
func testChecksumIncludeExclude(t *testing.T, wildcard bool) {
t.Parallel()
tmpdir, err := ioutil.TempDir("", "buildkit-state") tmpdir, err := ioutil.TempDir("", "buildkit-state")
require.NoError(t, err) require.NoError(t, err)
defer os.RemoveAll(tmpdir) defer os.RemoveAll(tmpdir)
@ -461,37 +469,43 @@ func TestChecksumIncludeExclude(t *testing.T) {
cc, err := newCacheContext(ref.Metadata(), nil) cc, err := newCacheContext(ref.Metadata(), nil)
require.NoError(t, err) require.NoError(t, err)
dgst, err := cc.Checksum(context.TODO(), ref, "foo", ChecksumOpts{IncludePatterns: []string{"foo"}}, nil) opts := func(opts ChecksumOpts) ChecksumOpts {
require.NoError(t, err) opts.Wildcard = wildcard
return opts
}
require.Equal(t, dgstFileData0, dgst) dgstFoo, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"foo"}}), nil)
dgstFoo, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"foo"}}, nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstFileData0, dgst) require.NotEqual(t, digest.FromBytes([]byte{}), dgstFoo)
dgstFooBar, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"foo", "bar"}}, nil) dgstFooBar, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"foo", "bar"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.NotEqual(t, digest.FromBytes([]byte{}), dgstFooBar)
require.NotEqual(t, dgstFoo, dgstFooBar) require.NotEqual(t, dgstFoo, dgstFooBar)
dgstD0, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0"}}, nil) dgstD0, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0"}}), nil)
require.NoError(t, err) require.NoError(t, err)
dgstD1, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d1"}}, nil) require.NotEqual(t, digest.FromBytes([]byte{}), dgstD0)
dgstD1, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d1"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.NotEqual(t, digest.FromBytes([]byte{}), dgstD1)
dgstD0Star, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0/*"}}, nil) dgstD0Star, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0/*"}}), nil)
require.NoError(t, err) require.NoError(t, err)
dgstD0AStar, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0/a*"}}, nil) require.NotEqual(t, digest.FromBytes([]byte{}), dgstD0Star)
dgstD0AStar, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0/a*"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstD0Star, dgstD0AStar) require.Equal(t, dgstD0Star, dgstD0AStar)
dgstD1Star, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d1/*"}}, nil) dgstD1Star, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d1/*"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.NotEqual(t, digest.FromBytes([]byte{}), dgstD1Star)
// Nothing matches pattern, but d2's metadata should be captured in the // Nothing matches pattern, but d2's metadata should be captured in the
// checksum if d2 exists // checksum if d2 exists
dgstD2Foo, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d2/foo"}}, nil) dgstD2Foo, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d2/foo"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, digest.FromBytes([]byte{}), dgstD2Foo)
err = ref.Release(context.TODO()) err = ref.Release(context.TODO())
require.NoError(t, err) require.NoError(t, err)
@ -514,47 +528,53 @@ func TestChecksumIncludeExclude(t *testing.T) {
cc, err = newCacheContext(ref.Metadata(), nil) cc, err = newCacheContext(ref.Metadata(), nil)
require.NoError(t, err) require.NoError(t, err)
dgstFoo2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"foo"}}, nil) dgstFoo2, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"foo"}}), nil)
require.NoError(t, err) require.NoError(t, err)
dgstFooBar2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"foo", "bar"}}, nil) dgstFooBar2, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"foo", "bar"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstFoo, dgstFoo2) require.Equal(t, dgstFoo, dgstFoo2)
require.Equal(t, dgstFooBar, dgstFooBar2) require.Equal(t, dgstFooBar, dgstFooBar2)
dgstD02, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0"}}, nil) dgstD02, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.NotEqual(t, dgstD0, dgstD02) require.NotEqual(t, dgstD0, dgstD02)
require.NotEqual(t, digest.FromBytes([]byte{}), dgstD02)
dgstD12, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d1"}}, nil) dgstD12, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d1"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstD1, dgstD12) require.Equal(t, dgstD1, dgstD12)
dgstD0Star2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0/*"}}, nil) dgstD0Star2, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0/*"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.NotEqual(t, dgstD0Star, dgstD0Star2) require.NotEqual(t, dgstD0Star, dgstD0Star2)
dgstD0AStar2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0/a*"}}, nil) dgstD0AStar2, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0/a*"}}), nil)
require.NoError(t, err) require.NoError(t, err)
// new file does not match the include pattern, so the digest should stay the same // new file does not match the include pattern, so the digest should stay the same
require.Equal(t, dgstD0AStar, dgstD0AStar2) require.Equal(t, dgstD0AStar, dgstD0AStar2)
require.NotEqual(t, digest.FromBytes([]byte{}), dgstD0AStar2)
dgstStarStarABC, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"**/abc"}}, nil) dgstStarStarABC, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"**/abc"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstD0AStar, dgstStarStarABC) require.Equal(t, dgstD0AStar, dgstStarStarABC)
dgstD1Star2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d1/*"}}, nil) dgstD1Star2, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d1/*"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstD1Star, dgstD1Star2) require.Equal(t, dgstD1Star, dgstD1Star2)
dgstD0StarExclude, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d0/*"}, ExcludePatterns: []string{"d0/xyz"}}, nil) dgstD0StarExclude, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d0/*"}, ExcludePatterns: []string{"d0/xyz"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstD0Star, dgstD0StarExclude) require.Equal(t, dgstD0Star, dgstD0StarExclude)
dgstD2Foo2, err := cc.Checksum(context.TODO(), ref, "", ChecksumOpts{IncludePatterns: []string{"d2/foo"}}, nil) dgstD2Foo2, err := cc.Checksum(context.TODO(), ref, "", opts(ChecksumOpts{IncludePatterns: []string{"d2/foo"}}), nil)
require.NoError(t, err) require.NoError(t, err)
require.Equal(t, dgstD2Foo, dgstD2Foo2) require.Equal(t, dgstD2Foo, dgstD2Foo2)
dgstD2Foo3, err := cc.Checksum(context.TODO(), ref, "d2", opts(ChecksumOpts{IncludePatterns: []string{"foo"}}), nil)
require.NoError(t, err)
require.Equal(t, dgstD2Foo, dgstD2Foo3)
err = ref.Release(context.TODO()) err = ref.Release(context.TODO())
require.NoError(t, err) require.NoError(t, err)
} }