From 42a0c9c888eb4a3f9e893ae18c2692cf41e46a93 Mon Sep 17 00:00:00 2001 From: Leandro Santiago Date: Fri, 24 Nov 2023 13:53:41 +0100 Subject: [PATCH 1/7] Dockerfile frontend: expose exclude keyword to ADD and COPY commands It exposes the `ExcludePatterns` to the Dockerfile frontend, adding `--exclude=` option in the COPY and ADD commands, which will cause filepaths matching such patterns not to be copied. `--exclude` can be used multiple times. References https://github.com/moby/buildkit/issues/4439 Signed-off-by: Leandro Santiago --- frontend/dockerfile/dockerfile2llb/convert.go | 79 +++++++++++-------- .../dockerfile/dockerfile2llb/convert_test.go | 16 ++++ frontend/dockerfile/instructions/commands.go | 22 +++--- frontend/dockerfile/instructions/parse.go | 6 ++ 4 files changed, 80 insertions(+), 43 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index e862ec022298..2a7bf63d7fea 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 { @@ -1112,6 +1114,14 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo return nil } +type excludeOnCopy struct { + patterns []string +} + +func (e *excludeOnCopy) SetCopyOption(i *llb.CopyInfo) { + i.ExcludePatterns = append(i.ExcludePatterns, e.patterns...) +} + func dispatchCopy(d *dispatchState, cfg copyConfig) error { dest, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform) if err != nil { @@ -1128,6 +1138,8 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { copyOpt = append(copyOpt, llb.WithUser(cfg.chown)) } + copyOpt = append(copyOpt, &excludeOnCopy{cfg.excludePatterns}) + var mode *os.FileMode if cfg.chmod != "" { p, err := strconv.ParseUint(cfg.chmod, 8, 32) @@ -1329,18 +1341,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/convert_test.go b/frontend/dockerfile/dockerfile2llb/convert_test.go index bdc58cb2c928..b70b7c81fadc 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_test.go +++ b/frontend/dockerfile/dockerfile2llb/convert_test.go @@ -208,3 +208,19 @@ COPY --from=stage1 f2 /sub/ _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.EqualError(t, err, "circular dependency detected on stage: stage0") } + +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/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/parse.go b/frontend/dockerfile/instructions/parse.go index 0ce7f37d5d4e..e35355e5f2c1 100644 --- a/frontend/dockerfile/instructions/parse.go +++ b/frontend/dockerfile/instructions/parse.go @@ -284,6 +284,8 @@ func parseAdd(req parseRequest) (*AddCommand, error) { if len(req.args) < 2 { return nil, errNoDestinationArgument("ADD") } + + flExcludes := req.flags.AddStrings("exclude") flChown := req.flags.AddString("chown", "") flChmod := req.flags.AddString("chmod", "") flLink := req.flags.AddBool("link", false) @@ -306,6 +308,7 @@ func parseAdd(req parseRequest) (*AddCommand, error) { Link: flLink.Value == "true", KeepGitDir: flKeepGitDir.Value == "true", Checksum: flChecksum.Value, + ExcludePatterns: flExcludes.StringValues, }, nil } @@ -313,6 +316,8 @@ func parseCopy(req parseRequest) (*CopyCommand, error) { if len(req.args) < 2 { return nil, errNoDestinationArgument("COPY") } + + flExcludes := req.flags.AddStrings("exclude") flChown := req.flags.AddString("chown", "") flFrom := req.flags.AddString("from", "") flChmod := req.flags.AddString("chmod", "") @@ -335,6 +340,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: flExcludes.StringValues, }, nil } From e897dc657a483a51d7804ca6c3ab7c7d3cb77125 Mon Sep 17 00:00:00 2001 From: Leandro Santiago Date: Wed, 24 Jan 2024 18:02:52 +0100 Subject: [PATCH 2/7] Dockerfile: Document exclude patterns on COPY and ADD commands References https://github.com/moby/buildkit/issues/4439 Signed-off-by: Leandro Santiago --- frontend/dockerfile/docs/reference.md | 28 ++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index 5acd5138a79b..69e694469c9b 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 ``. + 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 ``. + 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". From 398423fcab461383eb7667db840277ab34c4c6c7 Mon Sep 17 00:00:00 2001 From: Leandro Santiago Date: Mon, 22 Jan 2024 14:23:35 +0100 Subject: [PATCH 3/7] Integration test for Dockerfile --exclude option This affects the --exclude option in the COPY and ADD commands on Dockerfiles. References https://github.com/moby/buildkit/issues/4439 Signed-off-by: Leandro Santiago --- frontend/dockerfile/dockerfile_test.go | 151 +++++++++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 72c7feaa716f..af52039fe8b1 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "io" + "io/fs" "math" "net/http" "net/http/httptest" @@ -180,6 +181,7 @@ var allTests = integration.TestFuncs( testFrontendDeduplicateSources, testDuplicateLayersProvenance, testSourcePolicyWithNamedContext, + testExcludedFilesOnCopy, ) // Tests that depend on the `security.*` entitlements @@ -232,6 +234,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, @@ -7333,3 +7336,151 @@ func fixedWriteCloser(wc io.WriteCloser) filesync.FileOutputFunc { return wc, nil } } + +// 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() + + dockerfile := []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 +`) + + f := getFrontend(t, sb) + + dir := integration.Tmpdir( + t, + fstest.CreateFile(dockerui.DefaultDockerfileName, dockerfile, 0600), + 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), + ) + + destDir := integration.Tmpdir(t) + + _, err = f.Solve(ctx, c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + 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) + } + + items, err := os.ReadDir(path.Join(destDir.Name, `builder6/6`)) + require.NoError(t, err) + require.Empty(t, items) +} From 516130a3c439845aa9250ec382d027f9d69f5141 Mon Sep 17 00:00:00 2001 From: Leandro Santiago Date: Tue, 13 Feb 2024 14:22:49 +0100 Subject: [PATCH 4/7] Add llb.WithExcludePatterns Signed-off-by: Leandro Santiago --- client/llb/fileop.go | 12 ++++++++++++ frontend/dockerfile/dockerfile2llb/convert.go | 10 +--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 7fc445c4c925..186ec36e3875 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -398,6 +398,18 @@ func WithAllowWildcard(b bool) RmOption { }) } +type excludeOnCopy struct { + patterns []string +} + +func (e *excludeOnCopy) SetCopyOption(i *CopyInfo) { + i.ExcludePatterns = append(i.ExcludePatterns, e.patterns...) +} + +func WithExcludePatterns(patterns []string) CopyOption { + return &excludeOnCopy{patterns} +} + type fileActionRm struct { file string info RmInfo diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 2a7bf63d7fea..54bc69b1a394 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1114,14 +1114,6 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo return nil } -type excludeOnCopy struct { - patterns []string -} - -func (e *excludeOnCopy) SetCopyOption(i *llb.CopyInfo) { - i.ExcludePatterns = append(i.ExcludePatterns, e.patterns...) -} - func dispatchCopy(d *dispatchState, cfg copyConfig) error { dest, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform) if err != nil { @@ -1138,7 +1130,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { copyOpt = append(copyOpt, llb.WithUser(cfg.chown)) } - copyOpt = append(copyOpt, &excludeOnCopy{cfg.excludePatterns}) + copyOpt = append(copyOpt, llb.WithExcludePatterns(cfg.excludePatterns)) var mode *os.FileMode if cfg.chmod != "" { From fc23da5b97ded842f116eca3ba66c6966d6ae861 Mon Sep 17 00:00:00 2001 From: Leandro Santiago Date: Tue, 13 Feb 2024 15:03:36 +0100 Subject: [PATCH 5/7] Move Dockerfile copy/add --exclude implementation to Labs Signed-off-by: Leandro Santiago --- client/llb/fileop.go | 6 +- docker-bake.hcl | 2 +- frontend/dockerfile/dockerfile2llb/convert.go | 7 +- .../dockerfile/dockerfile2llb/convert_test.go | 16 -- .../dockerfile2llb/exclude_patterns_test.go | 27 +++ frontend/dockerfile/dockerfile_test.go | 150 ---------------- frontend/dockerfile/docs/reference.md | 4 +- frontend/dockerfile/exclude_patterns_test.go | 170 ++++++++++++++++++ .../instructions/exclude_pattern_feature.go | 8 + frontend/dockerfile/instructions/parse.go | 30 +++- 10 files changed, 243 insertions(+), 177 deletions(-) create mode 100644 frontend/dockerfile/dockerfile2llb/exclude_patterns_test.go create mode 100644 frontend/dockerfile/exclude_patterns_test.go create mode 100644 frontend/dockerfile/instructions/exclude_pattern_feature.go diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 186ec36e3875..1b01113e6610 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -398,16 +398,16 @@ func WithAllowWildcard(b bool) RmOption { }) } -type excludeOnCopy struct { +type excludeOnCopyAction struct { patterns []string } -func (e *excludeOnCopy) SetCopyOption(i *CopyInfo) { +func (e *excludeOnCopyAction) SetCopyOption(i *CopyInfo) { i.ExcludePatterns = append(i.ExcludePatterns, e.patterns...) } func WithExcludePatterns(patterns []string) CopyOption { - return &excludeOnCopy{patterns} + return &excludeOnCopyAction{patterns} } type fileActionRm struct { 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 54bc69b1a394..2abaecb230bd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1130,7 +1130,12 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { copyOpt = append(copyOpt, llb.WithUser(cfg.chown)) } - copyOpt = append(copyOpt, llb.WithExcludePatterns(cfg.excludePatterns)) + 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 != "" { diff --git a/frontend/dockerfile/dockerfile2llb/convert_test.go b/frontend/dockerfile/dockerfile2llb/convert_test.go index b70b7c81fadc..bdc58cb2c928 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_test.go +++ b/frontend/dockerfile/dockerfile2llb/convert_test.go @@ -208,19 +208,3 @@ COPY --from=stage1 f2 /sub/ _, _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) assert.EqualError(t, err, "circular dependency detected on stage: stage0") } - -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/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 af52039fe8b1..f325b4301c2c 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "fmt" "io" - "io/fs" "math" "net/http" "net/http/httptest" @@ -181,7 +180,6 @@ var allTests = integration.TestFuncs( testFrontendDeduplicateSources, testDuplicateLayersProvenance, testSourcePolicyWithNamedContext, - testExcludedFilesOnCopy, ) // Tests that depend on the `security.*` entitlements @@ -7336,151 +7334,3 @@ func fixedWriteCloser(wc io.WriteCloser) filesync.FileOutputFunc { return wc, nil } } - -// 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() - - dockerfile := []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 -`) - - f := getFrontend(t, sb) - - dir := integration.Tmpdir( - t, - fstest.CreateFile(dockerui.DefaultDockerfileName, dockerfile, 0600), - 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), - ) - - destDir := integration.Tmpdir(t) - - _, err = f.Solve(ctx, c, client.SolveOpt{ - LocalMounts: map[string]fsutil.FS{ - dockerui.DefaultLocalNameDockerfile: dir, - dockerui.DefaultLocalNameContext: dir, - }, - 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) - } - - items, err := os.ReadDir(path.Join(destDir.Name, `builder6/6`)) - require.NoError(t, err) - require.Empty(t, items) -} diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index 69e694469c9b..48ce59439384 100644 --- a/frontend/dockerfile/docs/reference.md +++ b/frontend/dockerfile/docs/reference.md @@ -1161,7 +1161,7 @@ The latter form is required for paths containing whitespace. > **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 ``. +> 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 ``. @@ -1388,7 +1388,7 @@ This latter form is required for paths containing whitespace > **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 ``. +> 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 ``. diff --git a/frontend/dockerfile/exclude_patterns_test.go b/frontend/dockerfile/exclude_patterns_test.go new file mode 100644 index 000000000000..6cdae4e820ef --- /dev/null +++ b/frontend/dockerfile/exclude_patterns_test.go @@ -0,0 +1,170 @@ +//go:build dfexcludepatterns +// +build dfexcludepatterns + +package dockerfile + +import ( + "io/fs" + "os" + "path" + "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" +) + +func init() { + allTests = append(allTests, testExcludedFilesOnCopy) +} + +// 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() + + dockerfile := []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 +`) + + f := getFrontend(t, sb) + + dir := integration.Tmpdir( + t, + fstest.CreateFile(dockerui.DefaultDockerfileName, dockerfile, 0600), + 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), + ) + + destDir := integration.Tmpdir(t) + + _, err = f.Solve(ctx, c, client.SolveOpt{ + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + 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) + } + + items, err := os.ReadDir(path.Join(destDir.Name, `builder6/6`)) + require.NoError(t, err) + require.Empty(t, items) +} 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 e35355e5f2c1..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,12 +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") } - flExcludes := req.flags.AddStrings("exclude") + 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) @@ -308,7 +324,7 @@ func parseAdd(req parseRequest) (*AddCommand, error) { Link: flLink.Value == "true", KeepGitDir: flKeepGitDir.Value == "true", Checksum: flChecksum.Value, - ExcludePatterns: flExcludes.StringValues, + ExcludePatterns: stringValuesFromFlagIfPossible(flExcludes), }, nil } @@ -317,7 +333,13 @@ func parseCopy(req parseRequest) (*CopyCommand, error) { return nil, errNoDestinationArgument("COPY") } - flExcludes := req.flags.AddStrings("exclude") + 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", "") @@ -340,7 +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: flExcludes.StringValues, + ExcludePatterns: stringValuesFromFlagIfPossible(flExcludes), }, nil } From 0befd8fbdeb8f8918ad77ff132674ac7ac620cb7 Mon Sep 17 00:00:00 2001 From: Leandro Santiago Date: Wed, 14 Feb 2024 17:38:49 +0100 Subject: [PATCH 6/7] Dockerfile frontend: add Integration test for context from git Signed-off-by: Leandro Santiago --- frontend/dockerfile/exclude_patterns_test.go | 278 ++++++++++++------- 1 file changed, 181 insertions(+), 97 deletions(-) diff --git a/frontend/dockerfile/exclude_patterns_test.go b/frontend/dockerfile/exclude_patterns_test.go index 6cdae4e820ef..c04ecd541b07 100644 --- a/frontend/dockerfile/exclude_patterns_test.go +++ b/frontend/dockerfile/exclude_patterns_test.go @@ -5,8 +5,11 @@ package dockerfile import ( "io/fs" + "net/http" + "net/http/httptest" "os" "path" + "path/filepath" "testing" "github.com/containerd/continuity/fs/fstest" @@ -17,8 +20,12 @@ import ( "github.com/tonistiigi/fsutil" ) +var excludedFilesTests = integration.TestFuncs( + testExcludedFilesOnCopy, +) + func init() { - allTests = append(allTests, testExcludedFilesOnCopy) + allTests = append(allTests, excludedFilesTests...) } // See #4439 @@ -30,7 +37,120 @@ func testExcludedFilesOnCopy(t *testing.T, sb integration.Sandbox) { require.NoError(t, err) defer c.Close() - dockerfile := []byte(` + 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 @@ -64,107 +184,71 @@ 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", + } - f := getFrontend(t, sb) + err = runShell(srcDir.Name, gitCommands...) + require.NoError(t, err) - dir := integration.Tmpdir( - t, - fstest.CreateFile(dockerui.DefaultDockerfileName, dockerfile, 0600), - 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), - ) + server := httptest.NewServer(http.FileServer(http.Dir(filepath.Join(srcDir.Name)))) - destDir := integration.Tmpdir(t) - - _, err = f.Solve(ctx, c, client.SolveOpt{ - LocalMounts: map[string]fsutil.FS{ - dockerui.DefaultLocalNameDockerfile: dir, - dockerui.DefaultLocalNameContext: dir, - }, - Exports: []client.ExportEntry{ - { - Type: client.ExporterLocal, - OutputDir: destDir.Name, - }, - }, - }, nil) + defer server.Close() + serverURL := server.URL - require.NoError(t, err) + // 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} / - 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}, - } +# ignore all the image files +FROM scratch as builder1 +COPY --from=base --exclude=*/*.png --exclude=*/*.jpeg . /1 - 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 - } +# ignore all document files +FROM scratch as builder2 +COPY --from=base --exclude=*/*.pdf --exclude=*/*.txt . /2 - 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) - } +# ignore all mp3 files +FROM scratch as builder3 +COPY --from=base --exclude=*.mp3 dir3/ /3/ - items, err := os.ReadDir(path.Join(destDir.Name, `builder6/6`)) - require.NoError(t, err) - require.Empty(t, items) +# 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) + } } From 2167e90ed6544573c677093112cb3c13d7b88fb8 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Fri, 23 Feb 2024 11:22:56 -0800 Subject: [PATCH 7/7] dockerfile: add excludepatterns feature to labs Signed-off-by: Tonis Tiigi --- frontend/dockerfile/release/labs/tags | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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