diff --git a/add.go b/add.go index e73aa8846da..fa2992c838e 100644 --- a/add.go +++ b/add.go @@ -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" @@ -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 @@ -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)) if err != nil { return fmt.Errorf("checking if %q(%q) is excluded: %w", globbed, rel, err) } @@ -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 { diff --git a/copier/copier.go b/copier/copier.go index 091cf468bd7..2e874433346 100644 --- a/copier/copier.go +++ b/copier/copier.go @@ -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" @@ -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 diff --git a/copier/copier_test.go b/copier/copier_test.go index 715a6d69b22..f6e101f78c9 100644 --- a/copier/copier_test.go +++ b/copier/copier_test.go @@ -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", @@ -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", diff --git a/internal/excludes/excludes.go b/internal/excludes/excludes.go new file mode 100644 index 00000000000..d27dc248add --- /dev/null +++ b/internal/excludes/excludes.go @@ -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 +} diff --git a/internal/excludes/excludes_test.go b/internal/excludes/excludes_test.go new file mode 100644 index 00000000000..2a9c517a109 --- /dev/null +++ b/internal/excludes/excludes_test.go @@ -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, + }, + } + 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) + }) + } +} diff --git a/tests/bud.bats b/tests/bud.bats index bbf13ccbf6e..5c3f22490b8 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -590,8 +590,8 @@ _EOF @test "bud with .dockerignore #1" { _prefetch alpine busybox - run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore/Dockerfile $BUDFILES/dockerignore - expect_output --substring 'building.*"COPY subdir \./".*no such file or directory' + # https://github.com/containers/buildah/issues/6615 + run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore/Dockerfile $BUDFILES/dockerignore run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore/Dockerfile.succeed $BUDFILES/dockerignore @@ -605,7 +605,9 @@ _EOF run_buildah 1 run myctr ls -l sub2.txt - run_buildah 1 run myctr ls -l subdir/ + # !*/sub1* un-excludes subdir/sub1.txt, nothing else + run_buildah run myctr find subdir -mindepth 1 -print + assert "$output" = "subdir/sub1.txt" } @test "bud --layers with --mount type bind should burst cache if symlink is changed" { @@ -1014,8 +1016,8 @@ this is the output of test12" @test "bud with .containerignore" { _prefetch alpine busybox - run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile $BUDFILES/containerignore - expect_output --substring 'building.*"COPY subdir \./".*no such file or directory' + # https://github.com/containers/buildah/issues/6615 + run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile $BUDFILES/containerignore run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/containerignore/Dockerfile.succeed $BUDFILES/containerignore @@ -1029,7 +1031,9 @@ this is the output of test12" run_buildah 1 run myctr ls -l sub2.txt - run_buildah 1 run myctr ls -l subdir/ + # !*/sub1* un-excludes subdir/sub1.txt, nothing else + run_buildah run myctr find subdir -mindepth 1 -print + assert "$output" = "subdir/sub1.txt" } @test "bud with .dockerignore - unmatched" { @@ -1101,8 +1105,8 @@ symlink(subdir)" @test "bud with .dockerignore #6" { _prefetch alpine busybox - run_buildah 125 build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore6/Dockerfile $BUDFILES/dockerignore6 - expect_output --substring 'building.*"COPY subdir \./".*no such file or directory' + # https://github.com/containers/buildah/issues/6615 + run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore6/Dockerfile $BUDFILES/dockerignore6 run_buildah build -t testbud $WITH_POLICY_JSON -f $BUDFILES/dockerignore6/Dockerfile.succeed $BUDFILES/dockerignore6 @@ -1116,7 +1120,9 @@ symlink(subdir)" run_buildah 1 run myctr ls -l sub2.txt - run_buildah 1 run myctr ls -l subdir/ + # !*/sub1* un-excludes subdir/sub1.txt, nothing else + run_buildah run myctr find subdir -mindepth 1 -print + assert "$output" = "subdir/sub1.txt" } @test "build with --platform without OS" { @@ -4934,6 +4940,33 @@ _EOF assert "$output" !~ file2 } +# https://github.com/containers/buildah/issues/6615 +@test "bud copy with .dockerignore wildcard negation" { + _prefetch alpine + mytmpdir=${TEST_SCRATCH_DIR}/my-dir-wildcard + mkdir -p $mytmpdir/cmd + echo "package main" > $mytmpdir/cmd/main.go + echo "module test" > $mytmpdir/go.mod + echo "# readme" > $mytmpdir/README.md + + cat > $mytmpdir/.dockerignore << _EOF +** +!go.mod +!**/*.go +_EOF + + cat > $mytmpdir/Containerfile << _EOF +FROM alpine +COPY . /upload/ +RUN find /upload -type f +_EOF + + run_buildah build -t testbud $WITH_POLICY_JSON ${mytmpdir} + expect_output --substring "/upload/go.mod" + expect_output --substring "/upload/cmd/main.go" + assert "$output" !~ "README" +} + @test "bud-copy-workdir" { target=testimage run_buildah build $WITH_POLICY_JSON -t ${target} $BUDFILES/copy-workdir @@ -5951,8 +5984,9 @@ EOF run_buildah 1 run myctr ls -l sub2.txt expect_output --substring "ls: sub2.txt: No such file or directory" - run_buildah 1 run myctr ls -l subdir/ - expect_output --substring "ls: subdir/: No such file or directory" + # !*/sub1* un-excludes subdir/sub1.txt, nothing else + run_buildah run myctr find subdir -mindepth 1 -print + assert "$output" = "subdir/sub1.txt" } @test "bud with network options" { diff --git a/tests/conformance/conformance_test.go b/tests/conformance/conformance_test.go index 1e42820b550..ba8e67ac44f 100644 --- a/tests/conformance/conformance_test.go +++ b/tests/conformance/conformance_test.go @@ -2300,10 +2300,13 @@ var internalTestCases = []testCase{ }, { - name: "copy-integration1", - contextDir: "dockerignore/integration1", - shouldFailAt: 3, - failureRegex: "(no such file or directory)|(file not found)|(file does not exist)", + name: "copy-integration1", + contextDir: "dockerignore/integration1", + // #6615: Docker comparison skipped because archive.TarWithOptions + // has the same wildcard directory-descent bug (moby/moby#30018) + // and BuildKit's server-side filtering uses different parent-prefix + // semantics (moby/moby#45608). + withoutDocker: true, }, { @@ -3084,6 +3087,13 @@ var internalTestCases = []testCase{ }, { + // https://github.com/podman-container-tools/buildah/issues/6615 + // + // Docker comparison skipped: client-side filtering has the same + // wildcard directory-descent bug (moby/moby#30018), and + // BuildKit's server-side filtering does not do parent-prefix + // matching so it excludes fewer files than buildah + // (moby/moby#45608, moby/moby#42788, moby/moby#40319). name: "dockerignore-exclude-kind-of-deep-subdir-dot", dockerfileContents: strings.Join([]string{ "FROM scratch", @@ -3100,9 +3110,9 @@ var internalTestCases = []testCase{ } return nil }, + withoutDocker: true, fsSkip: []string{"(dir):subdir:mtime"}, failOnExtraFSContent: true, - compatScratchConfig: types.OptionalBoolTrue, }, { @@ -3122,9 +3132,10 @@ var internalTestCases = []testCase{ } return nil }, + // #6615: see dockerignore-exclude-kind-of-deep-subdir-dot + withoutDocker: true, fsSkip: []string{"(dir):subdir:mtime"}, failOnExtraFSContent: true, - compatScratchConfig: types.OptionalBoolTrue, }, { @@ -3144,9 +3155,10 @@ var internalTestCases = []testCase{ } return nil }, + // #6615: see dockerignore-exclude-kind-of-deep-subdir-dot + withoutDocker: true, fsSkip: []string{"(dir):subdir:mtime"}, failOnExtraFSContent: true, - compatScratchConfig: types.OptionalBoolTrue, }, { @@ -3166,9 +3178,10 @@ var internalTestCases = []testCase{ } return nil }, + // #6615: see dockerignore-exclude-kind-of-deep-subdir-dot + withoutDocker: true, fsSkip: []string{"(dir):subdir:mtime"}, failOnExtraFSContent: true, - compatScratchConfig: types.OptionalBoolTrue, }, { @@ -3580,6 +3593,13 @@ var internalTestCases = []testCase{ failureRegex: "(no such file or directory)|(file not found)|(file does not exist)", }, + { + name: "dockerignore-allowlist-wildcard-negation", + contextDir: "dockerignore/allowlist/wildcard-negation", + // #6615: see dockerignore-exclude-kind-of-deep-subdir-dot + withoutDocker: true, + }, + { name: "tar-g", contextDir: "tar-g", @@ -3588,15 +3608,19 @@ var internalTestCases = []testCase{ }, { - name: "dockerignore-exceptions-skip", - contextDir: "dockerignore/exceptions-skip", + name: "dockerignore-exceptions-skip", + contextDir: "dockerignore/exceptions-skip", + // #6615: see dockerignore-exclude-kind-of-deep-subdir-dot + withoutDocker: true, fsSkip: []string{"(dir):volume:mtime"}, failOnExtraFSContent: true, }, { - name: "dockerignore-exceptions-weirdness-1", - contextDir: "dockerignore/exceptions-weirdness-1", + name: "dockerignore-exceptions-weirdness-1", + contextDir: "dockerignore/exceptions-weirdness-1", + // #6615: see dockerignore-exclude-kind-of-deep-subdir-dot + withoutDocker: true, fsSkip: []string{"(dir):newdir:mtime", "(dir):newdir:(dir):subdir:mtime"}, failOnExtraFSContent: true, }, diff --git a/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/.dockerignore b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/.dockerignore new file mode 100644 index 00000000000..da90c5c7142 --- /dev/null +++ b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/.dockerignore @@ -0,0 +1,3 @@ +** +!go.mod +!**/*.go diff --git a/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/Dockerfile b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/Dockerfile new file mode 100644 index 00000000000..a62f7e6eba0 --- /dev/null +++ b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/Dockerfile @@ -0,0 +1,4 @@ +FROM mirror.gcr.io/busybox +COPY . /upload/ +RUN test -f /upload/go.mod +RUN test -f /upload/cmd/main.go diff --git a/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/cmd/main.go b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/cmd/main.go new file mode 100644 index 00000000000..a3dd973f069 --- /dev/null +++ b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/cmd/main.go @@ -0,0 +1,7 @@ +package main + +import "fmt" + +func main() { + fmt.Println("Hello, World!") +} diff --git a/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/go.mod b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/go.mod new file mode 100644 index 00000000000..47e39f59a37 --- /dev/null +++ b/tests/conformance/testdata/dockerignore/allowlist/wildcard-negation/go.mod @@ -0,0 +1,3 @@ +module example.com/wildcard-negation + +go 1.25 diff --git a/tests/copy.bats b/tests/copy.bats index a113d0992d2..994769c373d 100644 --- a/tests/copy.bats +++ b/tests/copy.bats @@ -546,7 +546,9 @@ parents/y/b.txt" run_buildah 1 run $from ls -l sub2.txt - run_buildah 1 run $from ls -l subdir/ + # !*/sub1* un-excludes subdir/sub1.txt, nothing else + run_buildah run $from find subdir -mindepth 1 -print + assert "$output" = "subdir/sub1.txt" } @test "copy-preserving-extended-attributes" {