diff --git a/builder/remotecontext/detect.go b/builder/remotecontext/detect.go index 9b126ef775..09f4d28434 100644 --- a/builder/remotecontext/detect.go +++ b/builder/remotecontext/detect.go @@ -130,7 +130,7 @@ func removeDockerfile(c modifiableContext, filesToRemove ...string) error { f.Close() filesToRemove = append([]string{".dockerignore"}, filesToRemove...) for _, fileToRemove := range filesToRemove { - if rm, _ := fileutils.Matches(fileToRemove, excludes); rm { + if rm, _ := fileutils.MatchesOrParentMatches(fileToRemove, excludes); rm { if err := c.Remove(fileToRemove); err != nil { logrus.Errorf("failed to remove %s: %v", fileToRemove, err) } diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index 0eeedce098..a8a2338732 100644 --- a/pkg/archive/archive.go +++ b/pkg/archive/archive.go @@ -808,6 +808,11 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) for _, include := range options.IncludeFiles { rebaseName := options.RebaseNames[include] + var ( + parentMatched []bool + parentDirs []string + ) + walkRoot := getWalkRoot(srcPath, include) filepath.Walk(walkRoot, func(filePath string, f os.FileInfo, err error) error { if err != nil { @@ -834,11 +839,29 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) // is asking for that file no matter what - which is true // for some files, like .dockerignore and Dockerfile (sometimes) if include != relFilePath { - skip, err = pm.Matches(relFilePath) + for len(parentDirs) != 0 { + lastParentDir := parentDirs[len(parentDirs)-1] + if strings.HasPrefix(relFilePath, lastParentDir+string(os.PathSeparator)) { + break + } + parentDirs = parentDirs[:len(parentDirs)-1] + parentMatched = parentMatched[:len(parentMatched)-1] + } + + if len(parentMatched) != 0 { + skip, err = pm.MatchesUsingParentResult(relFilePath, parentMatched[len(parentMatched)-1]) + } else { + skip, err = pm.MatchesOrParentMatches(relFilePath) + } if err != nil { logrus.Errorf("Error matching %s: %v", relFilePath, err) return err } + + if f.IsDir() { + parentDirs = append(parentDirs, relFilePath) + parentMatched = append(parentMatched, skip) + } } if skip { diff --git a/pkg/fileutils/fileutils.go b/pkg/fileutils/fileutils.go index fd4fb7f08f..da4ffd13e2 100644 --- a/pkg/fileutils/fileutils.go +++ b/pkg/fileutils/fileutils.go @@ -55,8 +55,16 @@ func NewPatternMatcher(patterns []string) (*PatternMatcher, error) { return pm, nil } -// Matches matches path against all the patterns. Matches is not safe to be -// called concurrently +// Matches returns true if "file" matches any of the patterns +// and isn't excluded by any of the subsequent patterns. +// +// The "file" argument should be a slash-delimited path. +// +// Matches is not safe to call concurrently. +// +// This implementation is buggy (it only checks a single parent dir against the +// pattern) and will be removed soon. Use either MatchesOrParentMatches or +// MatchesUsingParentResult instead. func (pm *PatternMatcher) Matches(file string) (bool, error) { matched := false file = filepath.FromSlash(file) @@ -64,10 +72,11 @@ func (pm *PatternMatcher) Matches(file string) (bool, error) { parentPathDirs := strings.Split(parentPath, string(os.PathSeparator)) for _, pattern := range pm.patterns { - negative := false - - if pattern.exclusion { - negative = true + // Skip evaluation if this is an inclusion and the filename + // already matched the pattern, or it's an exclusion and it has + // not matched the pattern yet. + if pattern.exclusion != matched { + continue } match, err := pattern.match(file) @@ -83,13 +92,88 @@ func (pm *PatternMatcher) Matches(file string) (bool, error) { } if match { - matched = !negative + matched = !pattern.exclusion } } return matched, nil } +// MatchesOrParentMatches returns true if "file" matches any of the patterns +// and isn't excluded by any of the subsequent patterns. +// +// The "file" argument should be a slash-delimited path. +// +// Matches is not safe to call concurrently. +func (pm *PatternMatcher) MatchesOrParentMatches(file string) (bool, error) { + matched := false + file = filepath.FromSlash(file) + parentPath := filepath.Dir(file) + parentPathDirs := strings.Split(parentPath, string(os.PathSeparator)) + + for _, pattern := range pm.patterns { + // Skip evaluation if this is an inclusion and the filename + // already matched the pattern, or it's an exclusion and it has + // not matched the pattern yet. + if pattern.exclusion != matched { + continue + } + + match, err := pattern.match(file) + if err != nil { + return false, err + } + + if !match && parentPath != "." { + // Check to see if the pattern matches one of our parent dirs. + for i := range parentPathDirs { + match, _ = pattern.match(strings.Join(parentPathDirs[:i+1], string(os.PathSeparator))) + if match { + break + } + } + } + + if match { + matched = !pattern.exclusion + } + } + + return matched, nil +} + +// MatchesUsingParentResult returns true if "file" matches any of the patterns +// and isn't excluded by any of the subsequent patterns. The functionality is +// the same as Matches, but as an optimization, the caller keeps track of +// whether the parent directory matched. +// +// The "file" argument should be a slash-delimited path. +// +// MatchesUsingParentResult is not safe to call concurrently. +func (pm *PatternMatcher) MatchesUsingParentResult(file string, parentMatched bool) (bool, error) { + matched := parentMatched + file = filepath.FromSlash(file) + + for _, pattern := range pm.patterns { + // Skip evaluation if this is an inclusion and the filename + // already matched the pattern, or it's an exclusion and it has + // not matched the pattern yet. + if pattern.exclusion != matched { + continue + } + + match, err := pattern.match(file) + if err != nil { + return false, err + } + + if match { + matched = !pattern.exclusion + } + } + return matched, nil +} + // Exclusions returns true if any of the patterns define exclusions func (pm *PatternMatcher) Exclusions() bool { return pm.exclusions @@ -118,7 +202,6 @@ func (p *Pattern) Exclusion() bool { } func (p *Pattern) match(path string) (bool, error) { - if p.regexp == nil { if err := p.compile(); err != nil { return false, filepath.ErrBadPattern @@ -210,6 +293,9 @@ func (p *Pattern) compile() error { // Matches returns true if file matches any of the patterns // and isn't excluded by any of the subsequent patterns. +// +// This implementation is buggy (it only checks a single parent dir against the +// pattern) and will be removed soon. Use MatchesOrParentMatches instead. func Matches(file string, patterns []string) (bool, error) { pm, err := NewPatternMatcher(patterns) if err != nil { @@ -225,6 +311,23 @@ func Matches(file string, patterns []string) (bool, error) { return pm.Matches(file) } +// MatchesOrParentMatches returns true if file matches any of the patterns +// and isn't excluded by any of the subsequent patterns. +func MatchesOrParentMatches(file string, patterns []string) (bool, error) { + pm, err := NewPatternMatcher(patterns) + if err != nil { + return false, err + } + file = filepath.Clean(file) + + if file == "." { + // Don't let them exclude everything, kind of silly. + return false, nil + } + + return pm.MatchesOrParentMatches(file) +} + // CopyFile copies from src to dst until either EOF is reached // on src or an error occurs. It verifies src exists and removes // the dst if it exists. diff --git a/pkg/fileutils/fileutils_test.go b/pkg/fileutils/fileutils_test.go index 36064b6f5f..1e93b3e29d 100644 --- a/pkg/fileutils/fileutils_test.go +++ b/pkg/fileutils/fileutils_test.go @@ -328,6 +328,8 @@ func TestMatches(t *testing.T) { {"dir/**", "dir/file/", true}, {"dir/**", "dir/dir2/file", true}, {"dir/**", "dir/dir2/file/", true}, + {"**/dir", "dir", true}, + {"**/dir", "dir/file", true}, {"**/dir2/*", "dir/dir2/file", true}, {"**/dir2/*", "dir/dir2/file/", true}, {"**/dir2/**", "dir/dir2/dir3/file", true}, @@ -380,13 +382,37 @@ func TestMatches(t *testing.T) { }...) } - for _, test := range tests { - desc := fmt.Sprintf("pattern=%q text=%q", test.pattern, test.text) - pm, err := NewPatternMatcher([]string{test.pattern}) - assert.NilError(t, err, desc) - res, _ := pm.Matches(test.text) - assert.Check(t, is.Equal(test.pass, res), desc) - } + t.Run("MatchesOrParentMatches", func(t *testing.T) { + for _, test := range tests { + desc := fmt.Sprintf("pattern=%q text=%q", test.pattern, test.text) + pm, err := NewPatternMatcher([]string{test.pattern}) + assert.NilError(t, err, desc) + res, _ := pm.MatchesOrParentMatches(test.text) + assert.Check(t, is.Equal(test.pass, res), desc) + } + }) + + t.Run("MatchesUsingParentResult", func(t *testing.T) { + for _, test := range tests { + desc := fmt.Sprintf("pattern=%q text=%q", test.pattern, test.text) + pm, err := NewPatternMatcher([]string{test.pattern}) + assert.NilError(t, err, desc) + + parentPath := filepath.Dir(filepath.FromSlash(test.text)) + parentPathDirs := strings.Split(parentPath, string(os.PathSeparator)) + + parentMatched := false + if parentPath != "." { + for i := range parentPathDirs { + parentMatched, _ = pm.MatchesUsingParentResult(strings.Join(parentPathDirs[:i+1], "/"), parentMatched) + } + } + + res, _ := pm.MatchesUsingParentResult(test.text, parentMatched) + assert.Check(t, is.Equal(test.pass, res), desc) + } + }) + } func TestCleanPatterns(t *testing.T) {