diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 7fc445c4c925..1b01113e6610 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -398,6 +398,18 @@ func WithAllowWildcard(b bool) RmOption { }) } +type excludeOnCopyAction struct { + patterns []string +} + +func (e *excludeOnCopyAction) SetCopyOption(i *CopyInfo) { + i.ExcludePatterns = append(i.ExcludePatterns, e.patterns...) +} + +func WithExcludePatterns(patterns []string) CopyOption { + return &excludeOnCopyAction{patterns} +} + type fileActionRm struct { file string info RmInfo diff --git a/docker-bake.hcl b/docker-bake.hcl index 2a4bd270a5cc..8489a9ac7aec 100644 --- a/docker-bake.hcl +++ b/docker-bake.hcl @@ -138,7 +138,7 @@ target "lint" { matrix = { buildtags = [ { name = "default", tags = "", target = "golangci-lint" }, - { name = "labs", tags = "dfrunsecurity dfparents", target = "golangci-lint" }, + { name = "labs", tags = "dfrunsecurity dfparents dfexcludepatterns", target = "golangci-lint" }, { name = "nydus", tags = "nydus", target = "golangci-lint" }, { name = "yaml", tags = "", target = "yamllint" }, { name = "proto", tags = "", target = "protolint" }, diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index e862ec022298..2abaecb230bd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -744,17 +744,18 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { } if err == nil { err = dispatchCopy(d, copyConfig{ - params: c.SourcesAndDest, - source: opt.buildContext, - isAddCommand: true, - cmdToPrint: c, - chown: c.Chown, - chmod: c.Chmod, - link: c.Link, - keepGitDir: c.KeepGitDir, - checksum: checksum, - location: c.Location(), - opt: opt, + params: c.SourcesAndDest, + excludePatterns: c.ExcludePatterns, + source: opt.buildContext, + isAddCommand: true, + cmdToPrint: c, + chown: c.Chown, + chmod: c.Chmod, + link: c.Link, + keepGitDir: c.KeepGitDir, + checksum: checksum, + location: c.Location(), + opt: opt, }) } if err == nil { @@ -796,16 +797,17 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { l = src.state } err = dispatchCopy(d, copyConfig{ - params: c.SourcesAndDest, - source: l, - isAddCommand: false, - cmdToPrint: c, - chown: c.Chown, - chmod: c.Chmod, - link: c.Link, - parents: c.Parents, - location: c.Location(), - opt: opt, + params: c.SourcesAndDest, + excludePatterns: c.ExcludePatterns, + source: l, + isAddCommand: false, + cmdToPrint: c, + chown: c.Chown, + chmod: c.Chmod, + link: c.Link, + parents: c.Parents, + location: c.Location(), + opt: opt, }) if err == nil { if len(cmd.sources) == 0 { @@ -1128,6 +1130,13 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { copyOpt = append(copyOpt, llb.WithUser(cfg.chown)) } + if len(cfg.excludePatterns) > 0 { + // in theory we don't need to check whether there are any exclude patterns, + // as an empty list is a no-op. However, performing the check makes + // the code easier to understand and costs virtually nothing. + copyOpt = append(copyOpt, llb.WithExcludePatterns(cfg.excludePatterns)) + } + var mode *os.FileMode if cfg.chmod != "" { p, err := strconv.ParseUint(cfg.chmod, 8, 32) @@ -1329,18 +1338,19 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { } type copyConfig struct { - params instructions.SourcesAndDest - source llb.State - isAddCommand bool - cmdToPrint fmt.Stringer - chown string - chmod string - link bool - keepGitDir bool - checksum digest.Digest - parents bool - location []parser.Range - opt dispatchOpt + params instructions.SourcesAndDest + excludePatterns []string + source llb.State + isAddCommand bool + cmdToPrint fmt.Stringer + chown string + chmod string + link bool + keepGitDir bool + checksum digest.Digest + parents bool + location []parser.Range + opt dispatchOpt } func dispatchMaintainer(d *dispatchState, c *instructions.MaintainerCommand) error { diff --git a/frontend/dockerfile/dockerfile2llb/exclude_patterns_test.go b/frontend/dockerfile/dockerfile2llb/exclude_patterns_test.go new file mode 100644 index 000000000000..4d7a5ab7fba9 --- /dev/null +++ b/frontend/dockerfile/dockerfile2llb/exclude_patterns_test.go @@ -0,0 +1,27 @@ +//go:build dfexcludepatterns +// +build dfexcludepatterns + +package dockerfile2llb + +import ( + "testing" + + "github.com/moby/buildkit/util/appcontext" + "github.com/stretchr/testify/assert" +) + +func TestDockerfileCopyExcludePatterns(t *testing.T) { + df := `FROM scratch +COPY --exclude=src/*.go --exclude=tmp/*.txt dir /sub/ +` + _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + assert.NoError(t, err) +} + +func TestDockerfileAddExcludePatterns(t *testing.T) { + df := `FROM scratch +ADD --exclude=src/*.go --exclude=tmp/*.txt dir /sub/ +` + _, _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + assert.NoError(t, err) +} diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 72c7feaa716f..f325b4301c2c 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -232,6 +232,7 @@ func init() { func TestIntegration(t *testing.T) { integration.Run(t, allTests, opts...) + integration.Run(t, securityTests, append(append(opts, securityOpts...), integration.WithMatrix("security.insecure", map[string]interface{}{ "granted": securityInsecureGranted, diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index 5acd5138a79b..48ce59439384 100644 --- a/frontend/dockerfile/docs/reference.md +++ b/frontend/dockerfile/docs/reference.md @@ -1138,8 +1138,8 @@ RUN apt-get update && apt-get install -y ... ADD has two forms: ```dockerfile -ADD [--chown=:] [--chmod=] [--checksum=] ... -ADD [--chown=:] [--chmod=] ["",... ""] +ADD [--chown=:] [--chmod= [--exclude=]... [--checksum=] ... +ADD [--chown=:] [--chmod= [--exclude=]... ["",... ""] ``` The latter form is required for paths containing whitespace. @@ -1158,6 +1158,11 @@ The latter form is required for paths containing whitespace. > Only octal notation is currently supported. Non-octal support is tracked in > [moby/buildkit#1951](https://github.com/moby/buildkit/issues/1951). +> **Note** +> +> The `--exclude` option can be specified multiple times and cause files matching its patterns not to be copied, +> even if the files paths match the pattern specified in ``. This feature requires the build tag `dfexcludepatterns`. + The `ADD` instruction copies new files, directories or remote file URLs from `` and adds them to the filesystem of the image at the path ``. @@ -1165,13 +1170,13 @@ Multiple `` resources may be specified but if they are files or directories, their paths are interpreted as relative to the source of the context of the build. -Each `` may contain wildcards and matching will be done using Go's +Each `` and `` may contain wildcards and matching will be done using Go's [filepath.Match](https://golang.org/pkg/path/filepath#Match) rules. For example: -To add all files starting with "hom": +To add all files starting with "hom", excluding all files with `.txt` and `.md` extensions: ```dockerfile -ADD hom* /mydir/ +ADD --exclude=*.txt --exclude=*.md hom* /mydir/ ``` In the example below, `?` is replaced with any single character, e.g., "home.txt". @@ -1366,8 +1371,8 @@ See [`COPY --link`](#copy---link). COPY has two forms: ```dockerfile -COPY [--chown=:] [--chmod=] ... -COPY [--chown=:] [--chmod=] ["",... ""] +COPY [--chown=:] [--chmod=] [--exclude=]... ... +COPY [--chown=:] [--chmod=] [--exclude=]... ["",... ""] ``` This latter form is required for paths containing whitespace @@ -1380,6 +1385,11 @@ This latter form is required for paths containing whitespace > translating user and group names to IDs restricts this feature to only be viable for > Linux OS-based containers. +> **Note** +> +> The `--exclude` option can be specified multiple times and cause files matching its patterns not to be copied, +> even if the files paths match the pattern specified in ``. This feature requires the build tag `dfexcludepatterns`. + The `COPY` instruction copies new files or directories from `` and adds them to the filesystem of the container at the path ``. @@ -1390,10 +1400,10 @@ of the build. Each `` may contain wildcards and matching will be done using Go's [filepath.Match](https://golang.org/pkg/path/filepath#Match) rules. For example: -To add all files starting with "hom": +To add all files starting with "hom", excluding all files with `.txt` and `.md` extensions: ```dockerfile -COPY hom* /mydir/ +COPY --exclude=*.txt --exclude=*.md hom* /mydir/ ``` In the example below, `?` is replaced with any single character, e.g., "home.txt". diff --git a/frontend/dockerfile/exclude_patterns_test.go b/frontend/dockerfile/exclude_patterns_test.go new file mode 100644 index 000000000000..c04ecd541b07 --- /dev/null +++ b/frontend/dockerfile/exclude_patterns_test.go @@ -0,0 +1,254 @@ +//go:build dfexcludepatterns +// +build dfexcludepatterns + +package dockerfile + +import ( + "io/fs" + "net/http" + "net/http/httptest" + "os" + "path" + "path/filepath" + "testing" + + "github.com/containerd/continuity/fs/fstest" + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/frontend/dockerui" + "github.com/moby/buildkit/util/testutil/integration" + "github.com/stretchr/testify/require" + "github.com/tonistiigi/fsutil" +) + +var excludedFilesTests = integration.TestFuncs( + testExcludedFilesOnCopy, +) + +func init() { + allTests = append(allTests, excludedFilesTests...) +} + +// See #4439 +func testExcludedFilesOnCopy(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + ctx := sb.Context() + + c, err := client.New(ctx, sb.Address()) + require.NoError(t, err) + defer c.Close() + + srcDir := integration.Tmpdir( + t, + fstest.CreateDir("dir1", fs.ModePerm), + fstest.CreateDir("dir2", fs.ModePerm), + fstest.CreateDir("dir3", fs.ModePerm), + fstest.CreateFile("dir1/file-101.png", []byte(`2`), 0600), + fstest.CreateFile("dir1/file-102.txt", []byte(`3`), 0600), + fstest.CreateFile("dir1/file-103.jpeg", []byte(`4`), 0600), + fstest.CreateFile("dir2/file-201.pdf", []byte(`6`), 0600), + fstest.CreateFile("dir2/file-202.jpeg", []byte(`7`), 0600), + fstest.CreateFile("dir2/file-203.png", []byte(`8`), 0600), + fstest.CreateFile("dir3/file-301.mp3", []byte(`9`), 0600), + fstest.CreateFile("dir3/file-302.mpeg", []byte(`10`), 0600), + ) + + runTest := func(dockerfile []byte, localMounts map[string]fsutil.FS) { + f := getFrontend(t, sb) + + destDir := integration.Tmpdir(t) + + dockerDir := integration.Tmpdir( + t, + fstest.CreateFile(dockerui.DefaultDockerfileName, dockerfile, 0600), + ) + + if localMounts == nil { + localMounts = map[string]fsutil.FS{} + } + + localMounts[dockerui.DefaultLocalNameDockerfile] = dockerDir + + _, err = f.Solve(ctx, c, client.SolveOpt{ + LocalMounts: localMounts, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: destDir.Name, + }, + }, + }, nil) + + require.NoError(t, err) + + testCases := []struct { + filename string + excluded bool + expectedContent string + }{ + // Files copied with COPY command + {filename: "builder1/1/dir1/file-101.png", excluded: true}, + {filename: "builder1/1/dir1/file-102.txt", excluded: false, expectedContent: `3`}, + {filename: "builder1/1/dir1/file-103.jpeg", excluded: true}, + {filename: "builder1/1/dir2/file-201.pdf", excluded: false, expectedContent: `6`}, + {filename: "builder1/1/dir2/file-202.jpeg", excluded: true}, + {filename: "builder1/1/dir2/file-203.png", excluded: true}, + {filename: "builder1/1/dir3/file-301.mp3", excluded: false, expectedContent: `9`}, + {filename: "builder1/1/dir3/file-302.mpeg", excluded: false, expectedContent: `10`}, + + // files copied with COPY command + {filename: "builder2/2/dir1/file-101.png", excluded: false, expectedContent: `2`}, + {filename: "builder2/2/dir1/file-102.txt", excluded: true}, + {filename: "builder2/2/dir1/file-103.jpeg", excluded: false, expectedContent: `4`}, + {filename: "builder2/2/dir2/file-201.pdf", excluded: true}, + {filename: "builder2/2/dir2/file-202.jpeg", excluded: false, expectedContent: `7`}, + {filename: "builder2/2/dir2/file-203.png", excluded: false, expectedContent: `8`}, + {filename: "builder2/2/dir3/file-301.mp3", excluded: false, expectedContent: `9`}, + {filename: "builder2/2/dir3/file-302.mpeg", excluded: false, expectedContent: `10`}, + + // Files copied with ADD command + {filename: "builder3/3/file-301.mp3", excluded: true}, + {filename: "builder3/3/file-302.mpeg", excluded: false, expectedContent: `10`}, + + // Files copied with COPY wildcard + {filename: "builder4/4/file-101.png", excluded: true}, + {filename: "builder4/4/file-102.txt", excluded: true}, + {filename: "builder4/4/file-103.jpeg", excluded: true}, + {filename: "builder4/4/file-201.pdf", excluded: true}, + {filename: "builder4/4/file-202.jpeg", excluded: true}, + {filename: "builder4/4/file-203.png", excluded: true}, + {filename: "builder4/4/file-301.mp3", excluded: true}, + {filename: "builder4/4/file-301.mp3", excluded: true}, + {filename: "builder4/4/file-302.mpeg", excluded: false, expectedContent: `10`}, + + // Files copied with ADD wildcard + {filename: "builder5/5/file-101.png", excluded: true}, + {filename: "builder5/5/file-102.txt", excluded: false, expectedContent: `3`}, + {filename: "builder5/5/file-103.jpeg", excluded: true}, + {filename: "builder5/5/file-201.pdf", excluded: true}, + {filename: "builder5/5/file-202.jpeg", excluded: true}, + {filename: "builder5/5/file-203.png", excluded: true}, + {filename: "builder5/5/file-301.mp3", excluded: true}, + {filename: "builder5/5/file-301.mp3", excluded: true}, + {filename: "builder5/5/file-302.mpeg", excluded: true}, + } + + for _, tc := range testCases { + dt, err := os.ReadFile(path.Join(destDir.Name, tc.filename)) + if tc.excluded { + require.NotNilf(t, err, "File %s should not exist: %v", tc.filename, err) + continue + } + + require.NoErrorf(t, err, "File %s should exist", tc.filename) + require.Equalf(t, tc.expectedContent, string(dt), "File %s does not have matched content", tc.filename) + } + + // As all files are excluded, the directory must be empty + items, err := os.ReadDir(path.Join(destDir.Name, `builder6/6`)) + require.NoError(t, err) + require.Empty(t, items) + } + + // First, test with reading from a plain directory + runTest([]byte(` +# ignore all the image files +FROM scratch as builder1 +COPY --exclude=*/*.png --exclude=*/*.jpeg . /1 + +# ignore all document files +FROM scratch as builder2 +COPY --exclude=*/*.pdf --exclude=*/*.txt . /2 + +# ignore all mp3 files +FROM scratch as builder3 +ADD --exclude=*.mp3 dir3/ /3/ + +# Ignore everything, keeping only .mpeg files, using src as a wildcard. +# it's a fake example, just to be sure that src as wildcards work as expected. +FROM scratch as builder4 +COPY --exclude=*.mp3 --exclude=*.jpeg --exclude=*.png --exclude=*.pdf --exclude=*.txt dir* /4/ + +# Ignore everything, keeping only txt files. +# it's a fake example, just to be sure that src as wildcards work as expected. +FROM scratch as builder5 +ADD --exclude=*.mp* --exclude=*.jpeg --exclude=*.png --exclude=*.pdf dir* /5 + +# Exclude all files. No idea how it could be useful, but it should not be forbidden. +FROM scratch as builder6 +ADD --exclude=* dir* /6 + +FROM scratch +COPY --from=builder1 / /builder1 +COPY --from=builder2 / /builder2 +COPY --from=builder3 / /builder3 +COPY --from=builder4 / /builder4 +COPY --from=builder5 / /builder5 +COPY --from=builder6 / /builder6 +`), map[string]fsutil.FS{ + dockerui.DefaultLocalNameContext: srcDir, + }) + + // Load context from a git repository + { + gitCommands := []string{ + "git init", + "git config --local user.email test", + "git config --local user.name test", + "git add *", + "git commit -m 0.1", + "git tag 0.1", + "git update-server-info", + } + + err = runShell(srcDir.Name, gitCommands...) + require.NoError(t, err) + + server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(srcDir.Name)))) + + defer server.Close() + serverURL := server.URL + + // In this example, all the ADD commands were replaced by COPY, as ADD does not support + // the `--from=` flag, and we want to keep the assertions intact. + runTest([]byte(` +FROM scratch AS base +ARG REPO="`+serverURL+`/.git" +ARG TAG="0.1" +ADD --keep-git-dir=true ${REPO}#${TAG} / + +# ignore all the image files +FROM scratch as builder1 +COPY --from=base --exclude=*/*.png --exclude=*/*.jpeg . /1 + +# ignore all document files +FROM scratch as builder2 +COPY --from=base --exclude=*/*.pdf --exclude=*/*.txt . /2 + +# ignore all mp3 files +FROM scratch as builder3 +COPY --from=base --exclude=*.mp3 dir3/ /3/ + +# Ignore everything, keeping only .mpeg files, using src as a wildcard. +# it's a fake example, just to be sure that src as wildcards work as expected. +FROM scratch as builder4 +COPY --from=base --exclude=*.mp3 --exclude=*.jpeg --exclude=*.png --exclude=*.pdf --exclude=*.txt dir* /4/ + +# Ignore everything, keeping only txt files. +# it's a fake example, just to be sure that src as wildcards work as expected. +FROM scratch as builder5 +COPY --from=base --exclude=*.mp* --exclude=*.jpeg --exclude=*.png --exclude=*.pdf dir* /5 + +# Exclude all files. No idea how it could be useful, but it should not be forbidden. +FROM scratch as builder6 +COPY --from=base --exclude=* dir* /6 + +FROM scratch +COPY --from=builder1 / /builder1 +COPY --from=builder2 / /builder2 +COPY --from=builder3 / /builder3 +COPY --from=builder4 / /builder4 +COPY --from=builder5 / /builder5 +COPY --from=builder6 / /builder6 +`), nil) + } +} diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go index 08b99d1fa199..87252f9f17dd 100644 --- a/frontend/dockerfile/instructions/commands.go +++ b/frontend/dockerfile/instructions/commands.go @@ -239,11 +239,12 @@ func (s *SourcesAndDest) ExpandRaw(expander SingleWordExpander) error { type AddCommand struct { withNameAndCode SourcesAndDest - Chown string - Chmod string - Link bool - KeepGitDir bool // whether to keep .git dir, only meaningful for git sources - Checksum string + Chown string + Chmod string + Link bool + ExcludePatterns []string + KeepGitDir bool // whether to keep .git dir, only meaningful for git sources + Checksum string } func (c *AddCommand) Expand(expander SingleWordExpander) error { @@ -270,11 +271,12 @@ func (c *AddCommand) Expand(expander SingleWordExpander) error { type CopyCommand struct { withNameAndCode SourcesAndDest - From string - Chown string - Chmod string - Link bool - Parents bool // parents preserves directory structure + From string + Chown string + Chmod string + Link bool + ExcludePatterns []string + Parents bool // parents preserves directory structure } func (c *CopyCommand) Expand(expander SingleWordExpander) error { diff --git a/frontend/dockerfile/instructions/exclude_pattern_feature.go b/frontend/dockerfile/instructions/exclude_pattern_feature.go new file mode 100644 index 000000000000..0c1421b40d47 --- /dev/null +++ b/frontend/dockerfile/instructions/exclude_pattern_feature.go @@ -0,0 +1,8 @@ +//go:build dfexcludepatterns +// +build dfexcludepatterns + +package instructions + +func init() { + excludePatternsEnabled = true +} diff --git a/frontend/dockerfile/instructions/parse.go b/frontend/dockerfile/instructions/parse.go index 0ce7f37d5d4e..1d41d9f2c224 100644 --- a/frontend/dockerfile/instructions/parse.go +++ b/frontend/dockerfile/instructions/parse.go @@ -20,6 +20,8 @@ import ( "github.com/pkg/errors" ) +var excludePatternsEnabled = false + type parseRequest struct { command string args []string @@ -280,10 +282,26 @@ func parseSourcesAndDest(req parseRequest, command string) (*SourcesAndDest, err }, nil } +func stringValuesFromFlagIfPossible(f *Flag) []string { + if f == nil { + return nil + } + + return f.StringValues +} + func parseAdd(req parseRequest) (*AddCommand, error) { if len(req.args) < 2 { return nil, errNoDestinationArgument("ADD") } + + var flExcludes *Flag + + // silently ignore if not -labs + if excludePatternsEnabled { + flExcludes = req.flags.AddStrings("exclude") + } + flChown := req.flags.AddString("chown", "") flChmod := req.flags.AddString("chmod", "") flLink := req.flags.AddBool("link", false) @@ -306,6 +324,7 @@ func parseAdd(req parseRequest) (*AddCommand, error) { Link: flLink.Value == "true", KeepGitDir: flKeepGitDir.Value == "true", Checksum: flChecksum.Value, + ExcludePatterns: stringValuesFromFlagIfPossible(flExcludes), }, nil } @@ -313,6 +332,14 @@ func parseCopy(req parseRequest) (*CopyCommand, error) { if len(req.args) < 2 { return nil, errNoDestinationArgument("COPY") } + + var flExcludes *Flag + + // silently ignore if not -labs + if excludePatternsEnabled { + flExcludes = req.flags.AddStrings("exclude") + } + flChown := req.flags.AddString("chown", "") flFrom := req.flags.AddString("from", "") flChmod := req.flags.AddString("chmod", "") @@ -335,6 +362,7 @@ func parseCopy(req parseRequest) (*CopyCommand, error) { Chmod: flChmod.Value, Link: flLink.Value == "true", Parents: (flParents.Value == "true") && parentsEnabled, // silently ignore if not -labs + ExcludePatterns: stringValuesFromFlagIfPossible(flExcludes), }, nil } diff --git a/frontend/dockerfile/release/labs/tags b/frontend/dockerfile/release/labs/tags index 6b98e6bd8dd2..d3cfc1aed80b 100644 --- a/frontend/dockerfile/release/labs/tags +++ b/frontend/dockerfile/release/labs/tags @@ -1 +1 @@ -dfrunsecurity dfparents +dfrunsecurity dfparents dfexcludepatterns