Skip to content

Commit 8021a3e

Browse files
author
Aaron Lehmann
committed
Use fixed fileutils matching functions
This is important for two reasons: 1) Keeps caching logic consistent with recent fsutil changes to use these functions (also vendored here). 2) Allows us to move forward with removal of the original buggy Matches implementation in moby/moby. Signed-off-by: Aaron Lehmann <[email protected]>
1 parent 7fa9e57 commit 8021a3e

32 files changed

Lines changed: 1179 additions & 1154 deletions

cache/contenthash/checksum.go

Lines changed: 56 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,11 @@ type Hashed interface {
7979
Digest() digest.Digest
8080
}
8181

82-
type IncludedPath struct {
83-
Path string
84-
Record *CacheRecord
82+
type includedPath struct {
83+
path string
84+
record *CacheRecord
85+
matchedIncludePattern bool
86+
matchedExcludePattern bool
8587
}
8688

8789
type cacheManager struct {
@@ -404,30 +406,30 @@ func (cc *cacheContext) Checksum(ctx context.Context, mountable cache.Mountable,
404406

405407
if opts.FollowLinks {
406408
for i, w := range includedPaths {
407-
if w.Record.Type == CacheRecordTypeSymlink {
408-
dgst, err := cc.checksumFollow(ctx, m, w.Path, opts.FollowLinks)
409+
if w.record.Type == CacheRecordTypeSymlink {
410+
dgst, err := cc.checksumFollow(ctx, m, w.path, opts.FollowLinks)
409411
if err != nil {
410412
return "", err
411413
}
412-
includedPaths[i].Record = &CacheRecord{Digest: dgst}
414+
includedPaths[i].record = &CacheRecord{Digest: dgst}
413415
}
414416
}
415417
}
416418
if len(includedPaths) == 0 {
417419
return digest.FromBytes([]byte{}), nil
418420
}
419421

420-
if len(includedPaths) == 1 && path.Base(p) == path.Base(includedPaths[0].Path) {
421-
return includedPaths[0].Record.Digest, nil
422+
if len(includedPaths) == 1 && path.Base(p) == path.Base(includedPaths[0].path) {
423+
return includedPaths[0].record.Digest, nil
422424
}
423425

424426
digester := digest.Canonical.Digester()
425427
for i, w := range includedPaths {
426428
if i != 0 {
427429
digester.Hash().Write([]byte{0})
428430
}
429-
digester.Hash().Write([]byte(path.Base(w.Path)))
430-
digester.Hash().Write([]byte(w.Record.Digest))
431+
digester.Hash().Write([]byte(path.Base(w.path)))
432+
digester.Hash().Write([]byte(w.record.Digest))
431433
}
432434
return digester.Digest(), nil
433435
}
@@ -456,7 +458,7 @@ func (cc *cacheContext) checksumFollow(ctx context.Context, m *mount, p string,
456458
}
457459
}
458460

459-
func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, opts ChecksumOpts) ([]*IncludedPath, error) {
461+
func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, opts ChecksumOpts) ([]*includedPath, error) {
460462
cc.mu.Lock()
461463
defer cc.mu.Unlock()
462464

@@ -501,7 +503,7 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
501503
}
502504
}
503505

504-
includedPaths := make([]*IncludedPath, 0, 2)
506+
includedPaths := make([]*includedPath, 0, 2)
505507

506508
txn := cc.tree.Txn()
507509
root = txn.Root()
@@ -524,7 +526,7 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
524526
}
525527

526528
var (
527-
parentDirHeaders []*IncludedPath
529+
parentDirHeaders []*includedPath
528530
lastMatchedDir string
529531
)
530532

@@ -533,11 +535,15 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
533535

534536
for len(parentDirHeaders) != 0 {
535537
lastParentDir := parentDirHeaders[len(parentDirHeaders)-1]
536-
if strings.HasPrefix(fn, lastParentDir.Path+"/") {
538+
if strings.HasPrefix(fn, lastParentDir.path+"/") {
537539
break
538540
}
539541
parentDirHeaders = parentDirHeaders[:len(parentDirHeaders)-1]
540542
}
543+
var parentDir *includedPath
544+
if len(parentDirHeaders) != 0 {
545+
parentDir = parentDirHeaders[len(parentDirHeaders)-1]
546+
}
541547

542548
dirHeader := false
543549
if len(k) > 0 && k[len(k)-1] == byte(0) {
@@ -550,6 +556,7 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
550556
}
551557
}
552558

559+
maybeIncludedPath := &includedPath{path: fn}
553560
var shouldInclude bool
554561
if opts.Wildcard {
555562
if p != "" && (lastMatchedDir == "" || !strings.HasPrefix(fn, lastMatchedDir+"/")) {
@@ -564,7 +571,13 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
564571
lastMatchedDir = fn
565572
}
566573

567-
shouldInclude, err = shouldIncludePath(strings.TrimSuffix(strings.TrimPrefix(fn+"/", lastMatchedDir+"/"), "/"), includePatternMatcher, excludePatternMatcher)
574+
shouldInclude, err = shouldIncludePath(
575+
strings.TrimSuffix(strings.TrimPrefix(fn+"/", lastMatchedDir+"/"), "/"),
576+
includePatternMatcher,
577+
excludePatternMatcher,
578+
maybeIncludedPath,
579+
parentDir,
580+
)
568581
if err != nil {
569582
return nil, err
570583
}
@@ -574,7 +587,13 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
574587
continue
575588
}
576589

577-
shouldInclude, err = shouldIncludePath(strings.TrimSuffix(strings.TrimPrefix(fn+"/", p+"/"), "/"), includePatternMatcher, excludePatternMatcher)
590+
shouldInclude, err = shouldIncludePath(
591+
strings.TrimSuffix(strings.TrimPrefix(fn+"/", p+"/"), "/"),
592+
includePatternMatcher,
593+
excludePatternMatcher,
594+
maybeIncludedPath,
595+
parentDir,
596+
)
578597
if err != nil {
579598
return nil, err
580599
}
@@ -599,17 +618,18 @@ func (cc *cacheContext) includedPaths(ctx context.Context, m *mount, p string, o
599618
// dir.
600619
shouldInclude = false
601620
}
621+
maybeIncludedPath.record = cr
602622

603623
if !shouldInclude {
604624
if cr.Type == CacheRecordTypeDirHeader {
605625
// We keep track of non-included parent dir headers in case an
606626
// include pattern matches a file inside one of these dirs.
607-
parentDirHeaders = append(parentDirHeaders, &IncludedPath{Path: fn, Record: cr})
627+
parentDirHeaders = append(parentDirHeaders, maybeIncludedPath)
608628
}
609629
} else {
610630
includedPaths = append(includedPaths, parentDirHeaders...)
611631
parentDirHeaders = nil
612-
includedPaths = append(includedPaths, &IncludedPath{Path: fn, Record: cr})
632+
includedPaths = append(includedPaths, maybeIncludedPath)
613633
}
614634
k, _, kOk = iter.Next()
615635
}
@@ -624,22 +644,38 @@ func shouldIncludePath(
624644
candidate string,
625645
includePatternMatcher *fileutils.PatternMatcher,
626646
excludePatternMatcher *fileutils.PatternMatcher,
647+
maybeIncludedPath *includedPath,
648+
parentDir *includedPath,
627649
) (bool, error) {
650+
var (
651+
m bool
652+
err error
653+
)
628654
if includePatternMatcher != nil {
629-
m, err := includePatternMatcher.Matches(candidate)
655+
if parentDir != nil {
656+
m, err = includePatternMatcher.MatchesUsingParentResult(candidate, parentDir.matchedIncludePattern)
657+
} else {
658+
m, err = includePatternMatcher.MatchesOrParentMatches(candidate)
659+
}
630660
if err != nil {
631661
return false, errors.Wrap(err, "failed to match includepatterns")
632662
}
663+
maybeIncludedPath.matchedIncludePattern = m
633664
if !m {
634665
return false, nil
635666
}
636667
}
637668

638669
if excludePatternMatcher != nil {
639-
m, err := excludePatternMatcher.Matches(candidate)
670+
if parentDir != nil {
671+
m, err = excludePatternMatcher.MatchesUsingParentResult(candidate, parentDir.matchedExcludePattern)
672+
} else {
673+
m, err = excludePatternMatcher.MatchesOrParentMatches(candidate)
674+
}
640675
if err != nil {
641676
return false, errors.Wrap(err, "failed to match excludepatterns")
642677
}
678+
maybeIncludedPath.matchedExcludePattern = m
643679
if m {
644680
return false, nil
645681
}

cache/contenthash/checksum_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,16 @@ func TestChecksumIncludeDoubleStar(t *testing.T) {
636636
require.NoError(t, err)
637637
require.Equal(t, dgstDoubleStar, dgst)
638638

639+
// **/... pattern (https://github.com/moby/moby/issues/41433)
640+
dgst, err = cc.Checksum(context.TODO(), ref, "prefix/a", ChecksumOpts{IncludePatterns: []string{"**/foo", "**/report"}}, nil)
641+
require.NoError(t, err)
642+
require.Equal(t, dgstDoubleStar, dgst)
643+
644+
// Same, with Wildcard = true
645+
dgst, err = cc.Checksum(context.TODO(), ref, "prefix/a", ChecksumOpts{IncludePatterns: []string{"**/foo", "**/report"}, Wildcard: true}, nil)
646+
require.NoError(t, err)
647+
require.Equal(t, dgstDoubleStar, dgst)
648+
639649
}
640650

641651
func TestChecksumIncludeSymlink(t *testing.T) {

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ require (
8383
github.com/shurcooL/sanitized_anchor_name v1.0.0 // indirect
8484
github.com/sirupsen/logrus v1.8.1
8585
github.com/stretchr/testify v1.7.0
86-
github.com/tonistiigi/fsutil v0.0.0-20210609172227-d72af97c0eaf
86+
github.com/tonistiigi/fsutil v0.0.0-20210818161904-4442383b5028
8787
github.com/tonistiigi/go-actions-cache v0.0.0-20210714033416-b93d7f1b2e70
8888
github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea
8989
github.com/tonistiigi/vt100 v0.0.0-20210615222946-8066bb97264f
@@ -118,7 +118,7 @@ require (
118118
)
119119

120120
replace (
121-
github.com/docker/docker => github.com/docker/docker v20.10.3-0.20210609100121-ef4d47340142+incompatible
121+
github.com/docker/docker => github.com/docker/docker v20.10.3-0.20210817025855-ba2adeebdb8d+incompatible
122122
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc => github.com/tonistiigi/opentelemetry-go-contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.0.0-20210714055410-d010b05b4939
123123
go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace => github.com/tonistiigi/opentelemetry-go-contrib/instrumentation/net/http/httptrace/otelhttptrace v0.0.0-20210714055410-d010b05b4939
124124
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp => github.com/tonistiigi/opentelemetry-go-contrib/instrumentation/net/http/otelhttp v0.0.0-20210714055410-d010b05b4939

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,8 @@ github.com/docker/distribution v2.6.0-rc.1.0.20180327202408-83389a148052+incompa
385385
github.com/docker/distribution v2.7.1-0.20190205005809-0d3efadf0154+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
386386
github.com/docker/distribution v2.7.1+incompatible h1:a5mlkVzth6W5A4fOsS3D2EO5BUmsJpcB+cRlLU7cSug=
387387
github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w=
388-
github.com/docker/docker v20.10.3-0.20210609100121-ef4d47340142+incompatible h1:CKSQs5KedtaAdusBPAJQS7cN1PibFX4RuThbwHgJrJE=
389-
github.com/docker/docker v20.10.3-0.20210609100121-ef4d47340142+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
388+
github.com/docker/docker v20.10.3-0.20210817025855-ba2adeebdb8d+incompatible h1:tSd7TeZCH0j9m4P14bfe/eO1KYawrt3DztHI8gZAmLM=
389+
github.com/docker/docker v20.10.3-0.20210817025855-ba2adeebdb8d+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk=
390390
github.com/docker/docker-credential-helpers v0.6.3 h1:zI2p9+1NQYdnG6sMU26EX4aVGlqbInSQxQXLvzJ4RPQ=
391391
github.com/docker/docker-credential-helpers v0.6.3/go.mod h1:WRaJzqw3CTB9bk10avuGsjVBZsD05qeibJ1/TYlvc0Y=
392392
github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKohAFqRJQ=
@@ -1078,8 +1078,8 @@ github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1
10781078
github.com/tommy-muehle/go-mnd v1.1.1/go.mod h1:dSUh0FtTP8VhvkL1S+gUR1OKd9ZnSaozuI6r3m6wOig=
10791079
github.com/tommy-muehle/go-mnd v1.3.1-0.20200224220436-e6f9a994e8fa/go.mod h1:dSUh0FtTP8VhvkL1S+gUR1OKd9ZnSaozuI6r3m6wOig=
10801080
github.com/tonistiigi/fsutil v0.0.0-20201103201449-0834f99b7b85/go.mod h1:a7cilN64dG941IOXfhJhlH0qB92hxJ9A1ewrdUmJ6xo=
1081-
github.com/tonistiigi/fsutil v0.0.0-20210609172227-d72af97c0eaf h1:L0ixhsTk9j+dVnIvF6aiVCxPiaFvwTOyJxqimPq44p8=
1082-
github.com/tonistiigi/fsutil v0.0.0-20210609172227-d72af97c0eaf/go.mod h1:lJAxK//iyZ3yGbQswdrPTxugZIDM7sd4bEsD0x3XMHk=
1081+
github.com/tonistiigi/fsutil v0.0.0-20210818161904-4442383b5028 h1:uEkkUFMCPtzz1HVOa42u15OHems1ugiRt172tSRTWSk=
1082+
github.com/tonistiigi/fsutil v0.0.0-20210818161904-4442383b5028/go.mod h1:E6osHKls9ix67jofYQ61RQKwlJhqJOZM2hintp+49iI=
10831083
github.com/tonistiigi/go-actions-cache v0.0.0-20210714033416-b93d7f1b2e70 h1:+ZlFs3Tl5qYZJvX2PxfZxGlVXz847LsOJGyNVU5pCHo=
10841084
github.com/tonistiigi/go-actions-cache v0.0.0-20210714033416-b93d7f1b2e70/go.mod h1:dNS+PPTqGnSl80x3wEyWWCHeON5xiBGtcM0uD6CgHNU=
10851085
github.com/tonistiigi/opentelemetry-go-contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.0.0-20210714055410-d010b05b4939 h1:s6wDMZYNyWt8KvkjhrMpOthFPgI3JB8ipJS+eCV/psg=

vendor/github.com/docker/docker/api/common.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/github.com/docker/docker/api/swagger.yaml

Lines changed: 44 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

vendor/github.com/docker/docker/api/types/client.go

Lines changed: 12 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)