Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 3 additions & 25 deletions add.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/tonistiigi/dchapes-mode"
"go.podman.io/buildah/copier"
"go.podman.io/buildah/define"
"go.podman.io/buildah/internal/excludes"
"go.podman.io/buildah/internal/tmpdir"
"go.podman.io/buildah/internal/urlsource"
"go.podman.io/buildah/pkg/chrootuser"
Expand Down Expand Up @@ -253,29 +254,6 @@ func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string,
return nil
}

// includeDirectoryAnyway returns true if "path" is a prefix for an exception
// known to "pm". If "path" is a directory that "pm" claims matches its list
// of patterns, but "pm"'s list of exclusions contains a pattern for which
// "path" is a prefix, then IncludeDirectoryAnyway() will return true.
// This is not always correct, because it relies on the directory part of any
// exception paths to be specified without wildcards.
func includeDirectoryAnyway(path string, pm *fileutils.PatternMatcher) bool {
if !pm.Exclusions() {
return false
}
prefix := strings.TrimPrefix(path, string(os.PathSeparator)) + string(os.PathSeparator)
for _, pattern := range pm.Patterns() {
if !pattern.Exclusion() {
continue
}
spec := strings.TrimPrefix(pattern.String(), string(os.PathSeparator))
if strings.HasPrefix(spec, prefix) {
return true
}
}
return false
}

// globbedToGlobbable takes a pathname which might include the '[', *, or ?
// characters, and converts it into a glob pattern that matches itself by
// marking the '[' characters as _not_ the beginning of match ranges and
Expand Down Expand Up @@ -707,7 +685,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
}
// Check for dockerignore-style exclusion of this item.
if rel != "." {
excluded, err := pm.Matches(filepath.ToSlash(rel)) //nolint:staticcheck
excluded, err := pm.IsMatch(filepath.ToSlash(rel))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably out of scope, but the deprecated call could be fixed there as well? https://github.com/podman-container-tools/buildah/blob/main/copier/copier.go#L1087

if err != nil {
return fmt.Errorf("checking if %q(%q) is excluded: %w", globbed, rel, err)
}
Expand All @@ -716,7 +694,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
// directories can only be skipped if we don't have to allow for the
// possibility of finding things to include under them
globInfo := localSourceStat.Results[globbed]
if !globInfo.IsDir || !includeDirectoryAnyway(rel, pm) {
if !globInfo.IsDir || !excludes.ShouldDescendExcludedDir(rel, pm) {
continue
}
} else {
Expand Down
33 changes: 7 additions & 26 deletions copier/copier.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import (
"unicode"

"github.com/sirupsen/logrus"
"github.com/tonistiigi/dchapes-mode"
mode "github.com/tonistiigi/dchapes-mode"
"go.podman.io/buildah/internal/excludes"
"go.podman.io/image/v5/pkg/compression"
"go.podman.io/image/v5/types"
"go.podman.io/storage/pkg/archive"
Expand Down Expand Up @@ -1527,32 +1528,12 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
}
if skip {
if d.IsDir() {
// if there are no "include
// this anyway" patterns at
// all, we don't need to
// descend into this particular
// directory if it's a directory
if !pm.Exclusions() {
return filepath.SkipDir
}
// if there are exclusion
// patterns for which this
// path is a prefix, we
// need to keep descending
for _, pattern := range pm.Patterns() {
if !pattern.Exclusion() {
continue
}
spec := strings.Trim(pattern.String(), string(os.PathSeparator))
trimmedPath := strings.Trim(skippedPath, string(os.PathSeparator))
if strings.HasPrefix(spec+string(os.PathSeparator), trimmedPath) {
// we can't just skip over
// this directory
return nil
}
// check if a negation pattern
// means we should descend into
// this excluded directory
if excludes.ShouldDescendExcludedDir(skippedPath, pm) {
return nil
}
// there are exclusions, but
// none of them apply here
return filepath.SkipDir
}
// skip this item, but if we're
Expand Down
4 changes: 2 additions & 2 deletions copier/copier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1029,7 +1029,7 @@ func testGetMultiple(t *testing.T) {
"file-b",
"link-c",
"hlink-0",
// "subdir-a/file-c", // strings.HasPrefix("**/*-c", "subdir-a/") is false
"subdir-a/file-c",
"subdir-b/",
"subdir-b/file-n",
"subdir-b/file-o",
Expand Down Expand Up @@ -1155,8 +1155,8 @@ func testGetMultiple(t *testing.T) {
pattern: ".",
exclude: []string{"*", "!**/*-c"},
items: []string{
// "subdir-a/file-c", // strings.HasPrefix("**/*-c", "subdir-a/") is false
"link-c",
"subdir-a/file-c",
"subdir-c/",
"subdir-c/file-p",
"subdir-c/file-q",
Expand Down
53 changes: 53 additions & 0 deletions internal/excludes/excludes.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package excludes

import (
"os"
"path/filepath"
"strings"

"go.podman.io/storage/pkg/fileutils"
)

// ShouldDescendExcludedDir checks whether an excluded directory should still be
// descended into because a negation pattern in pm might match files under it.
// It handles literal prefix matches (e.g. !cmd/main.go for dir "cmd") and
// wildcard negations (e.g. !**/*.go, !*/*.go). The wildcard check extracts
// the literal prefix before the first wildcard and may intentionally
// overmatch (descend into directories that won't ultimately contain matches),
// which is safe because actual file-level matching happens later.
func ShouldDescendExcludedDir(dirPath string, pm *fileutils.PatternMatcher) bool {
if pm == nil || !pm.Exclusions() {
return false
}
dir := filepath.ToSlash(strings.Trim(dirPath, string(os.PathSeparator)))
for _, pattern := range pm.Patterns() {
if !pattern.Exclusion() {
continue
}
slashPattern := filepath.ToSlash(strings.Trim(pattern.String(), string(os.PathSeparator)))

// Literal-prefix check: the negation spec starts with this
// directory path, for example: !cmd/main.go matches dir "cmd"
if strings.HasPrefix(slashPattern, dir+"/") {
return true
}

// Wildcard-aware check: extract the literal prefix before
// the first wildcard character (*, ?, [), for example: !cmd/**/*.go matches dir "cmd"
// if the directory is at or under that literal prefix, a file beneath this
// directory could match the negation, so keep descending.
if firstWild := strings.IndexAny(slashPattern, "*?["); firstWild >= 0 {
var literalPrefix string
if idx := strings.LastIndex(slashPattern[:firstWild], "/"); idx >= 0 {
literalPrefix = slashPattern[:idx]
}
if literalPrefix == "" {
return true
}
if dir == literalPrefix || strings.HasPrefix(dir, literalPrefix+"/") {
return true
}
}
}
return false
}
175 changes: 175 additions & 0 deletions internal/excludes/excludes_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
package excludes

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.podman.io/storage/pkg/fileutils"
)

func TestShouldDescendExcludedDir(t *testing.T) {
tests := []struct {
name string
path string
patterns []string
want bool
}{
{
name: "nil matcher",
path: "cmd",
patterns: nil,
want: false,
},
{
name: "no exclusions",
path: "cmd",
patterns: []string{"*"},
want: false,
},
{
name: "literal prefix match",
path: "cmd",
patterns: []string{"*", "!cmd/main.go"},
want: true,
},
{
name: "literal prefix no match",
path: "other",
patterns: []string{"*", "!cmd/main.go"},
want: false,
},
{
name: "double star at start matches any dir",
path: "cmd",
patterns: []string{"**", "!**/*.go"},
want: true,
},
{
name: "double star at start matches nested dir",
path: "cmd/sub",
patterns: []string{"**", "!**/*.go"},
want: true,
},
{
name: "double star with prefix matches dir under prefix",
path: "cmd/sub",
patterns: []string{"**", "!cmd/**/*.go"},
want: true,
},
{
name: "double star with prefix no match for other dir",
path: "other",
patterns: []string{"**", "!cmd/**/*.go"},
want: false,
},
{
name: "single star at start matches any dir",
path: "cmd",
patterns: []string{"*", "!*/*.go"},
want: true,
},
{
name: "single star at start matches nested dir",
path: "cmd/sub",
patterns: []string{"*", "!*/*.go"},
want: true,
},
{
name: "single star with prefix matches dir under prefix",
path: "src/pkg",
patterns: []string{"**", "!src/*/*.go"},
want: true,
},
{
name: "single star with prefix no match for other dir",
path: "other",
patterns: []string{"**", "!src/*/*.go"},
want: false,
},
{
name: "leading slash is stripped",
path: "/cmd",
patterns: []string{"*", "!cmd/main.go"},
want: true,
},
{
name: "deep nested with double star prefix",
path: "src/internal/pkg",
patterns: []string{"**", "!src/**/*.go"},
want: true,
},
{
name: "dir prefix match is not a partial match",
path: "cmds",
patterns: []string{"*", "!cmd/main.go"},
want: false,
},
{
name: "wildcard mid-segment descends parent dir",
path: "cmd/images",
patterns: []string{"**", "!cmd/image*/main.go"},
want: true,
},
{
name: "wildcard mid-segment matches parent",
path: "cmd",
patterns: []string{"**", "!cmd/image*"},
want: true,
},
{
name: "question mark wildcard matches any dir",
path: "cmd",
patterns: []string{"**", "!cm?/*.go"},
want: true,
},
{
name: "question mark wildcard no literal prefix matches any dir",
path: "other",
patterns: []string{"**", "!?md/*.go"},
want: true,
},
{
name: "bracket wildcard matches dir",
path: "cmd",
patterns: []string{"**", "!cm[d]/*.go"},
want: true,
},
{
name: "bracket wildcard no literal prefix matches any dir",
path: "other",
patterns: []string{"**", "![c]md/*.go"},
want: true,
},
{
name: "bracket wildcard with prefix matches dir under prefix",
path: "src/cmd",
patterns: []string{"**", "!src/cm[d]/*.go"},
want: true,
},
{
name: "bracket wildcard with prefix no match for other dir",
path: "other",
patterns: []string{"**", "!src/cm[d]/*.go"},
want: false,
},
{
name: "non-exclusion patterns are ignored",
path: "cmd",
patterns: []string{"cmd/**/*.go"},
want: false,
},
}
Comment thread
nalind marked this conversation as resolved.
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var pm *fileutils.PatternMatcher
if tt.patterns != nil {
var err error
pm, err = fileutils.NewPatternMatcher(tt.patterns)
require.NoError(t, err)
}
got := ShouldDescendExcludedDir(tt.path, pm)
assert.Equal(t, tt.want, got, "ShouldDescendExcludedDir(%q)", tt.path)
})
}
}
Loading