From 4fc00a80a32133eb0916b21b9fb1ef446fc9956a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 25 Jul 2023 10:03:39 +0200 Subject: [PATCH 1/6] frontend/dockerfile/dockerignore: cleanup unit test - don't use a temp-file for the test as all we need is a reader - use a const and string-literal for the test-content, which makes it slightly more readable - don't use hard-coded tests for each line, but use an "expected" slice - don't fail early if line-numbers don't match Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 01b25b13d9b78fef4b24f7c659e349a01ee3f488) Signed-off-by: Sebastiaan van Stijn --- .../dockerignore/dockerignore_test.go | 77 +++++++++---------- 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/frontend/dockerfile/dockerignore/dockerignore_test.go b/frontend/dockerfile/dockerignore/dockerignore_test.go index c3cd07884cf8..86176fdff388 100644 --- a/frontend/dockerfile/dockerignore/dockerignore_test.go +++ b/frontend/dockerfile/dockerignore/dockerignore_test.go @@ -1,61 +1,54 @@ package dockerignore import ( - "os" - "path/filepath" + "strings" "testing" ) func TestReadAll(t *testing.T) { - di, err := ReadAll(nil) + actual, err := ReadAll(nil) if err != nil { - t.Fatalf("Expected not to have error, got %v", err) + t.Errorf("Expected no error, got %v", err) } - - if diLen := len(di); diLen != 0 { - t.Fatalf("Expected to have zero dockerignore entry, got %d", diLen) + if entries := len(actual); entries != 0 { + t.Fatalf("Expected to have zero entries, got %d", entries) } - diName := filepath.Join(t.TempDir(), ".dockerignore") - content := "test1\n/test2\n/a/file/here\n\nlastfile\n# this is a comment\n! /inverted/abs/path\n!\n! \n" - err = os.WriteFile(diName, []byte(content), 0600) - if err != nil { - t.Fatal(err) - } + const content = `test1 +/test2 +/a/file/here - diFd, err := os.Open(diName) - if err != nil { - t.Fatal(err) +lastfile +# this is a comment +! /inverted/abs/path +! +! ` + + expected := []string{ + "test1", + "test2", // according to https://docs.docker.com/engine/reference/builder/#dockerignore-file, /foo/bar should be treated as foo/bar + "a/file/here", // according to https://docs.docker.com/engine/reference/builder/#dockerignore-file, /foo/bar should be treated as foo/bar + "lastfile", + "!inverted/abs/path", + "!", + "!", } - defer diFd.Close() - di, err = ReadAll(diFd) + actual, err = ReadAll(strings.NewReader(content)) if err != nil { - t.Fatal(err) + t.Error(err) } - if len(di) != 7 { - t.Fatalf("Expected 7 entries, got %v", len(di)) - } - if di[0] != "test1" { - t.Fatal("First element is not test1") - } - if di[1] != "test2" { // according to https://docs.docker.com/engine/reference/builder/#dockerignore-file, /foo/bar should be treated as foo/bar - t.Fatal("Second element is not test2") - } - if di[2] != "a/file/here" { // according to https://docs.docker.com/engine/reference/builder/#dockerignore-file, /foo/bar should be treated as foo/bar - t.Fatal("Third element is not a/file/here") - } - if di[3] != "lastfile" { - t.Fatal("Fourth element is not lastfile") - } - if di[4] != "!inverted/abs/path" { - t.Fatal("Fifth element is not !inverted/abs/path") - } - if di[5] != "!" { - t.Fatalf("Sixth element is not !, but %s", di[5]) - } - if di[6] != "!" { - t.Fatalf("Seventh element is not !, but %s", di[6]) + if len(actual) != len(expected) { + t.Errorf("Expected %d entries, got %v", len(expected), len(actual)) + } + for i, expectedLine := range expected { + if i >= len(actual) { + t.Errorf(`missing line %d: expected: "%s", got none`, i+1, expectedLine) + continue + } + if actual[i] != expectedLine { + t.Errorf(`line %d: expected: "%s", got: "%s"`, i+1, expectedLine, actual[i]) + } } } From fd607e3386982ff3b7b27a07ca3440026108a5a1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 25 Jul 2023 10:26:14 +0200 Subject: [PATCH 2/6] frontend/dockerfile/dockerignore: touch-up godoc and code Use "doc links" where possible, and better describe the function. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit d11608118968a01cb7f91ab7c2ff9e9e3fe2920c) Signed-off-by: Sebastiaan van Stijn --- .../dockerfile/dockerignore/dockerignore.go | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/frontend/dockerfile/dockerignore/dockerignore.go b/frontend/dockerfile/dockerignore/dockerignore.go index e7f29ae8df68..c4406c414647 100644 --- a/frontend/dockerfile/dockerignore/dockerignore.go +++ b/frontend/dockerfile/dockerignore/dockerignore.go @@ -10,19 +10,29 @@ import ( "github.com/pkg/errors" ) -// ReadAll reads a .dockerignore file and returns the list of file patterns -// to ignore. Note this will trim whitespace from each line as well -// as use GO's "clean" func to get the shortest/cleanest path for each. +// ReadAll reads an ignore file from a reader and returns the list of file +// patterns to ignore, applying the following rules: +// +// - An UTF8 BOM header (if present) is stripped. +// - Lines starting with "#" are considered comments and are skipped. +// +// For remaining lines: +// +// - Leading and trailing whitespace is removed from each ignore pattern. +// - It uses [filepath.Clean] to get the shortest/cleanest path for +// ignore patterns. +// - Leading forward-slashes ("/") are removed from ignore patterns, +// so "/some/path" and "some/path" are considered equivalent. func ReadAll(reader io.Reader) ([]string, error) { if reader == nil { return nil, nil } - scanner := bufio.NewScanner(reader) var excludes []string currentLine := 0 - utf8bom := []byte{0xEF, 0xBB, 0xBF} + + scanner := bufio.NewScanner(reader) for scanner.Scan() { scannedBytes := scanner.Bytes() // We trim UTF8 BOM From 5bc1358ce18a6e5c8397326a9adf503ea6c4efbf Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 25 Jul 2023 10:32:19 +0200 Subject: [PATCH 3/6] frontend/dockerfile/dockerignore: remove hard-coded filename from error While this function would usually be used for read a `.dockerignore` file, it accepts a Reader and can also be used to handle ignore patterns from other files (e.g. `Dockerfile.dockerignore`) or other sources. The error was also wrapped multiple times in some code-paths, which could lead to an error being formatted as: failed to parse dockerignore: error reading .dockerignore: Let's remove mention of the `.dockerignore` filename from the error, and leave it to the caller to include the filename. This patch also brings the MainContext dockerignore error inline with the NamedContext dockerignore error, now printing the exact name of the file. Co-authored-by: Justin Chadwell Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 7eb2ceafc3f3a5e601ab8759244d7d6144d92329) Signed-off-by: Sebastiaan van Stijn --- frontend/dockerfile/dockerignore/dockerignore.go | 4 +--- frontend/dockerui/config.go | 7 +++++-- frontend/dockerui/namedcontext.go | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/frontend/dockerfile/dockerignore/dockerignore.go b/frontend/dockerfile/dockerignore/dockerignore.go index c4406c414647..2bd9f1c9e451 100644 --- a/frontend/dockerfile/dockerignore/dockerignore.go +++ b/frontend/dockerfile/dockerignore/dockerignore.go @@ -6,8 +6,6 @@ import ( "io" "path/filepath" "strings" - - "github.com/pkg/errors" ) // ReadAll reads an ignore file from a reader and returns the list of file @@ -69,7 +67,7 @@ func ReadAll(reader io.Reader) ([]string, error) { excludes = append(excludes, pattern) } if err := scanner.Err(); err != nil { - return nil, errors.Wrap(err, "error reading .dockerignore") + return nil, err } return excludes, nil } diff --git a/frontend/dockerui/config.go b/frontend/dockerui/config.go index 12ec2c6880e0..a83c49940b43 100644 --- a/frontend/dockerui/config.go +++ b/frontend/dockerui/config.go @@ -80,7 +80,8 @@ type Client struct { g flightcontrol.Group[*buildContext] bopts client.BuildOpts - dockerignore []byte + dockerignore []byte + dockerignoreName string } type SBOM struct { @@ -375,6 +376,7 @@ func (bc *Client) ReadEntrypoint(ctx context.Context, lang string, opts ...llb.L }) if err == nil { bc.dockerignore = dt + bc.dockerignoreName = bctx.filename + ".dockerignore" } return &Source{ @@ -435,13 +437,14 @@ func (bc *Client) MainContext(ctx context.Context, opts ...llb.LocalOption) (*ll dt = []byte{} } bc.dockerignore = dt + bc.dockerignoreName = DefaultDockerignoreName } var excludes []string if len(bc.dockerignore) != 0 { excludes, err = dockerignore.ReadAll(bytes.NewBuffer(bc.dockerignore)) if err != nil { - return nil, errors.Wrap(err, "failed to parse dockerignore") + return nil, errors.Wrapf(err, "failed parsing %s", bc.dockerignoreName) } } diff --git a/frontend/dockerui/namedcontext.go b/frontend/dockerui/namedcontext.go index 6a441c50822e..8f004baa92bd 100644 --- a/frontend/dockerui/namedcontext.go +++ b/frontend/dockerui/namedcontext.go @@ -208,7 +208,7 @@ func (bc *Client) namedContextRecursive(ctx context.Context, name string, nameWi if len(dt) != 0 { excludes, err = dockerignore.ReadAll(bytes.NewBuffer(dt)) if err != nil { - return nil, nil, err + return nil, nil, errors.Wrapf(err, "failed parsing %s", DefaultDockerignoreName) } } } From 13489822374a0976aa68af31ae2823ae09cc35ac Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 23 Aug 2023 00:11:08 +0200 Subject: [PATCH 4/6] vendor: github.com/moby/patternmatcher v0.6.0 - integrate frontend/dockerfile/dockerignore from buildkit full diff: https://github.com/moby/patternmatcher/compare/v0.5.0...v0.6.0 Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 9f013f49353f7c8f33b7ae9c1550b8174ccb3b20) Signed-off-by: Sebastiaan van Stijn --- go.mod | 2 +- go.sum | 4 ++-- vendor/modules.txt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 3a0f30b56ebf..cae356e8498e 100644 --- a/go.mod +++ b/go.mod @@ -45,7 +45,7 @@ require ( github.com/klauspost/compress v1.16.3 github.com/mitchellh/hashstructure/v2 v2.0.2 github.com/moby/locker v1.0.1 - github.com/moby/patternmatcher v0.5.0 + github.com/moby/patternmatcher v0.6.0 github.com/moby/sys/mountinfo v0.6.2 github.com/moby/sys/signal v0.7.0 github.com/morikuni/aec v1.0.0 diff --git a/go.sum b/go.sum index 8d45ddd3c5af..cdeadc543062 100644 --- a/go.sum +++ b/go.sum @@ -906,8 +906,8 @@ github.com/mitchellh/osext v0.0.0-20151018003038-5e2d6d41470f/go.mod h1:OkQIRizQ github.com/moby/buildkit v0.8.1/go.mod h1:/kyU1hKy/aYCuP39GZA9MaKioovHku57N6cqlKZIaiQ= github.com/moby/locker v1.0.1 h1:fOXqR41zeveg4fFODix+1Ch4mj/gT0NE1XJbp/epuBg= github.com/moby/locker v1.0.1/go.mod h1:S7SDdo5zpBK84bzzVlKr2V0hz+7x9hWbYC/kq7oQppc= -github.com/moby/patternmatcher v0.5.0 h1:YCZgJOeULcxLw1Q+sVR636pmS7sPEn1Qo2iAN6M7DBo= -github.com/moby/patternmatcher v0.5.0/go.mod h1:hDPoyOpDY7OrrMDLaYoY3hf52gNCR/YOUYxkhApJIxc= +github.com/moby/patternmatcher v0.6.0 h1:GmP9lR19aU5GqSSFko+5pRqHi+Ohk1O69aFiKkVGiPk= +github.com/moby/patternmatcher v0.6.0/go.mod h1:hDPoyOpDY7OrrMDLaYoY3hf52gNCR/YOUYxkhApJIxc= github.com/moby/sys/mount v0.1.0/go.mod h1:FVQFLDRWwyBjDTBNQXDlWnSFREqOo3OKX9aqhmeoo74= github.com/moby/sys/mount v0.1.1/go.mod h1:FVQFLDRWwyBjDTBNQXDlWnSFREqOo3OKX9aqhmeoo74= github.com/moby/sys/mount v0.3.3 h1:fX1SVkXFJ47XWDoeFW4Sq7PdQJnV2QIDZAqjNqgEjUs= diff --git a/vendor/modules.txt b/vendor/modules.txt index 76695844bad4..4b50df602980 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -632,7 +632,7 @@ github.com/mitchellh/hashstructure/v2 # github.com/moby/locker v1.0.1 ## explicit; go 1.13 github.com/moby/locker -# github.com/moby/patternmatcher v0.5.0 +# github.com/moby/patternmatcher v0.6.0 ## explicit; go 1.19 github.com/moby/patternmatcher # github.com/moby/sys/mount v0.3.3 From b91df2ce1ce6ff4502b8d864da08cc9dd14cc89b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 23 Aug 2023 00:12:18 +0200 Subject: [PATCH 5/6] replace dockerfile/dockerignore with patternmatcher/ignorefile The BuildKit dockerignore package was integrated in the patternmatcher repository / module. This patch updates our uses of the BuildKit package with its new location. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit cb63b9ccba1a90ecf3e4d474825296c3371f565f) Signed-off-by: Sebastiaan van Stijn --- .../dockerignore/dockerignore_deprecated.go | 12 +++++ .../dockerignore/dockerignore_test.go | 54 ------------------- .../patternmatcher/ignorefile/ignorefile.go | 2 +- vendor/modules.txt | 1 + 4 files changed, 14 insertions(+), 55 deletions(-) create mode 100644 frontend/dockerfile/dockerignore/dockerignore_deprecated.go delete mode 100644 frontend/dockerfile/dockerignore/dockerignore_test.go rename frontend/dockerfile/dockerignore/dockerignore.go => vendor/github.com/moby/patternmatcher/ignorefile/ignorefile.go (98%) diff --git a/frontend/dockerfile/dockerignore/dockerignore_deprecated.go b/frontend/dockerfile/dockerignore/dockerignore_deprecated.go new file mode 100644 index 000000000000..002e46a9e81f --- /dev/null +++ b/frontend/dockerfile/dockerignore/dockerignore_deprecated.go @@ -0,0 +1,12 @@ +package dockerignore + +import ( + "io" + + "github.com/moby/patternmatcher/ignorefile" +) + +// ReadAll is an alias for [ignorefile.ReadAll]. +func ReadAll(reader io.Reader) ([]string, error) { + return ignorefile.ReadAll(reader) +} diff --git a/frontend/dockerfile/dockerignore/dockerignore_test.go b/frontend/dockerfile/dockerignore/dockerignore_test.go deleted file mode 100644 index 86176fdff388..000000000000 --- a/frontend/dockerfile/dockerignore/dockerignore_test.go +++ /dev/null @@ -1,54 +0,0 @@ -package dockerignore - -import ( - "strings" - "testing" -) - -func TestReadAll(t *testing.T) { - actual, err := ReadAll(nil) - if err != nil { - t.Errorf("Expected no error, got %v", err) - } - if entries := len(actual); entries != 0 { - t.Fatalf("Expected to have zero entries, got %d", entries) - } - - const content = `test1 -/test2 -/a/file/here - -lastfile -# this is a comment -! /inverted/abs/path -! -! ` - - expected := []string{ - "test1", - "test2", // according to https://docs.docker.com/engine/reference/builder/#dockerignore-file, /foo/bar should be treated as foo/bar - "a/file/here", // according to https://docs.docker.com/engine/reference/builder/#dockerignore-file, /foo/bar should be treated as foo/bar - "lastfile", - "!inverted/abs/path", - "!", - "!", - } - - actual, err = ReadAll(strings.NewReader(content)) - if err != nil { - t.Error(err) - } - - if len(actual) != len(expected) { - t.Errorf("Expected %d entries, got %v", len(expected), len(actual)) - } - for i, expectedLine := range expected { - if i >= len(actual) { - t.Errorf(`missing line %d: expected: "%s", got none`, i+1, expectedLine) - continue - } - if actual[i] != expectedLine { - t.Errorf(`line %d: expected: "%s", got: "%s"`, i+1, expectedLine, actual[i]) - } - } -} diff --git a/frontend/dockerfile/dockerignore/dockerignore.go b/vendor/github.com/moby/patternmatcher/ignorefile/ignorefile.go similarity index 98% rename from frontend/dockerfile/dockerignore/dockerignore.go rename to vendor/github.com/moby/patternmatcher/ignorefile/ignorefile.go index 2bd9f1c9e451..94ea5a0ef550 100644 --- a/frontend/dockerfile/dockerignore/dockerignore.go +++ b/vendor/github.com/moby/patternmatcher/ignorefile/ignorefile.go @@ -1,4 +1,4 @@ -package dockerignore +package ignorefile import ( "bufio" diff --git a/vendor/modules.txt b/vendor/modules.txt index 4b50df602980..500dad70b301 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -635,6 +635,7 @@ github.com/moby/locker # github.com/moby/patternmatcher v0.6.0 ## explicit; go 1.19 github.com/moby/patternmatcher +github.com/moby/patternmatcher/ignorefile # github.com/moby/sys/mount v0.3.3 ## explicit; go 1.16 github.com/moby/sys/mount From 8c4c57a0861b4c34a22e4be825b71f716a86dc4f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 22 Aug 2023 23:07:41 +0200 Subject: [PATCH 6/6] deprecate frontend/dockerfile/dockerignore This package was moved to github.com/moby/patternmatcher/ignorefile. Mark the alias as deprecated. Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 376e9c858c8cd75d9d556bc6c0ab79635dd778d0) Signed-off-by: Sebastiaan van Stijn --- frontend/dockerfile/dockerignore/dockerignore_deprecated.go | 2 ++ frontend/dockerui/config.go | 4 ++-- frontend/dockerui/namedcontext.go | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/frontend/dockerfile/dockerignore/dockerignore_deprecated.go b/frontend/dockerfile/dockerignore/dockerignore_deprecated.go index 002e46a9e81f..c30712bf5007 100644 --- a/frontend/dockerfile/dockerignore/dockerignore_deprecated.go +++ b/frontend/dockerfile/dockerignore/dockerignore_deprecated.go @@ -7,6 +7,8 @@ import ( ) // ReadAll is an alias for [ignorefile.ReadAll]. +// +// Deprecated: use [ignorefile.ReadAll] instead. func ReadAll(reader io.Reader) ([]string, error) { return ignorefile.ReadAll(reader) } diff --git a/frontend/dockerui/config.go b/frontend/dockerui/config.go index a83c49940b43..0ae30245ed64 100644 --- a/frontend/dockerui/config.go +++ b/frontend/dockerui/config.go @@ -15,10 +15,10 @@ import ( "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/exporter/containerimage/image" "github.com/moby/buildkit/frontend/attestations" - "github.com/moby/buildkit/frontend/dockerfile/dockerignore" "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/flightcontrol" + "github.com/moby/patternmatcher/ignorefile" digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" @@ -442,7 +442,7 @@ func (bc *Client) MainContext(ctx context.Context, opts ...llb.LocalOption) (*ll var excludes []string if len(bc.dockerignore) != 0 { - excludes, err = dockerignore.ReadAll(bytes.NewBuffer(bc.dockerignore)) + excludes, err = ignorefile.ReadAll(bytes.NewBuffer(bc.dockerignore)) if err != nil { return nil, errors.Wrapf(err, "failed parsing %s", bc.dockerignoreName) } diff --git a/frontend/dockerui/namedcontext.go b/frontend/dockerui/namedcontext.go index 8f004baa92bd..dd44b7d8fe89 100644 --- a/frontend/dockerui/namedcontext.go +++ b/frontend/dockerui/namedcontext.go @@ -11,9 +11,9 @@ import ( "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/exporter/containerimage/image" - "github.com/moby/buildkit/frontend/dockerfile/dockerignore" "github.com/moby/buildkit/frontend/gateway/client" "github.com/moby/buildkit/util/imageutil" + "github.com/moby/patternmatcher/ignorefile" "github.com/pkg/errors" ) @@ -206,7 +206,7 @@ func (bc *Client) namedContextRecursive(ctx context.Context, name string, nameWi }) // error ignored if len(dt) != 0 { - excludes, err = dockerignore.ReadAll(bytes.NewBuffer(dt)) + excludes, err = ignorefile.ReadAll(bytes.NewBuffer(dt)) if err != nil { return nil, nil, errors.Wrapf(err, "failed parsing %s", DefaultDockerignoreName) }