From db12a77e6c835c31ec78da559dab2a4b1e30536e Mon Sep 17 00:00:00 2001 From: Victor Noel Date: Thu, 3 Oct 2019 16:32:11 +0200 Subject: [PATCH 1/3] Fix #776 --- .../Dockerfile_test_dockerignore_relative | 5 ++ ...le_test_dockerignore_relative.dockerignore | 3 ++ integration/ignore_relative/bar | 0 integration/ignore_relative/baz | 0 integration/ignore_relative/foo | 0 pkg/executor/build.go | 2 +- pkg/util/command_util_test.go | 2 +- pkg/util/fs_util.go | 8 +++- pkg/util/fs_util_test.go | 46 +++++++++++++++++++ 9 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_dockerignore_relative create mode 100644 integration/dockerfiles/Dockerfile_test_dockerignore_relative.dockerignore create mode 100644 integration/ignore_relative/bar create mode 100644 integration/ignore_relative/baz create mode 100644 integration/ignore_relative/foo diff --git a/integration/dockerfiles/Dockerfile_test_dockerignore_relative b/integration/dockerfiles/Dockerfile_test_dockerignore_relative new file mode 100644 index 0000000000..bc94268059 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_dockerignore_relative @@ -0,0 +1,5 @@ +# This dockerfile makes sure Dockerfile.dockerignore is working +# If so then ignore_relative/foo should copy to /foo +# If not, then this image won't build because it will attempt to copy three files to /foo, which is a file not a directory +FROM scratch +COPY ignore_relative/* /foo diff --git a/integration/dockerfiles/Dockerfile_test_dockerignore_relative.dockerignore b/integration/dockerfiles/Dockerfile_test_dockerignore_relative.dockerignore new file mode 100644 index 0000000000..7f5371bdd8 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_dockerignore_relative.dockerignore @@ -0,0 +1,3 @@ +# A .dockerignore file to make sure dockerignore support works +ignore_relative/** +!ignore_relative/foo diff --git a/integration/ignore_relative/bar b/integration/ignore_relative/bar new file mode 100644 index 0000000000..e69de29bb2 diff --git a/integration/ignore_relative/baz b/integration/ignore_relative/baz new file mode 100644 index 0000000000..e69de29bb2 diff --git a/integration/ignore_relative/foo b/integration/ignore_relative/foo new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/executor/build.go b/pkg/executor/build.go index e134e52b57..45f23cca39 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -427,7 +427,7 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { if err != nil { return nil, err } - if err := util.GetExcludedFiles(opts.SrcContext); err != nil { + if err := util.GetExcludedFiles(opts.DockerfilePath, opts.SrcContext); err != nil { return nil, err } // Some stages may refer to other random images, not previous stages diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 3e23425951..cd1a324a2a 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -390,7 +390,7 @@ var isSrcValidTests = []struct { func Test_IsSrcsValid(t *testing.T) { for _, test := range isSrcValidTests { t.Run(test.name, func(t *testing.T) { - if err := GetExcludedFiles(buildContextPath); err != nil { + if err := GetExcludedFiles("", buildContextPath); err != nil { t.Fatalf("error getting excluded files: %v", err) } err := IsSrcsValid(test.srcsAndDest, test.resolvedSources, buildContextPath) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 338caa02a3..cc9e4a4bde 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -554,11 +554,15 @@ func CopyFile(src, dest, buildcontext string) (bool, error) { } // GetExcludedFiles gets a list of files to exclude from the .dockerignore -func GetExcludedFiles(buildcontext string) error { - path := filepath.Join(buildcontext, ".dockerignore") +func GetExcludedFiles(dockerfilepath string, buildcontext string) error { + path := dockerfilepath + ".dockerignore" + if !FilepathExists(path) { + path = filepath.Join(buildcontext, ".dockerignore") + } if !FilepathExists(path) { return nil } + logrus.Infof("Using dockerignore file: %v", path) contents, err := ioutil.ReadFile(path) if err != nil { return errors.Wrap(err, "parsing .dockerignore") diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index c44908056f..37cb7b034e 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -694,3 +694,49 @@ func Test_childDirInWhitelist(t *testing.T) { }) } } + +func Test_correctDockerignoreFileIsUsed(t *testing.T) { + type args struct { + dockerfilepath string + buildcontext string + excluded string + notExcluded string + } + tests := []struct { + name string + args args + }{ + { + name: "relative dockerfile used", + args: args{ + dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore_relative", + buildcontext: "../../integration/", + excluded: "ignore_relative/bar", + notExcluded: "ignore_relative/foo", + }, + }, + { + name: "context dockerfile is used", + args: args{ + dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore", + buildcontext: "../../integration/", + excluded: "ignore/bar", + notExcluded: "ignore/foo", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + excluded = nil + if err := GetExcludedFiles(tt.args.dockerfilepath, tt.args.buildcontext); err != nil { + t.Fatal(err) + } + if excl := excludeFile(tt.args.excluded, tt.args.buildcontext); !excl { + t.Errorf("'%v' not excluded as expected", tt.args.excluded) + } + if excl := excludeFile(tt.args.notExcluded, tt.args.buildcontext); excl { + t.Errorf("'%v' excluded against expectation", tt.args.notExcluded) + } + }) + } +} From 5700de039dff08c4ca491f6992e71c13830b5f45 Mon Sep 17 00:00:00 2001 From: Victor Noel Date: Fri, 4 Oct 2019 10:08:39 +0200 Subject: [PATCH 2/3] Add more tests for #776 --- pkg/util/fs_util_test.go | 41 ++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 37cb7b034e..449df74645 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -699,8 +699,8 @@ func Test_correctDockerignoreFileIsUsed(t *testing.T) { type args struct { dockerfilepath string buildcontext string - excluded string - notExcluded string + excluded []string + included []string } tests := []struct { name string @@ -711,8 +711,8 @@ func Test_correctDockerignoreFileIsUsed(t *testing.T) { args: args{ dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore_relative", buildcontext: "../../integration/", - excluded: "ignore_relative/bar", - notExcluded: "ignore_relative/foo", + excluded: []string{"ignore_relative/bar"}, + included: []string{"ignore_relative/foo", "ignore/bar"}, }, }, { @@ -720,23 +720,28 @@ func Test_correctDockerignoreFileIsUsed(t *testing.T) { args: args{ dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore", buildcontext: "../../integration/", - excluded: "ignore/bar", - notExcluded: "ignore/foo", + excluded: []string{"ignore/bar"}, + included: []string{"ignore/foo", "ignore_relative/bar"}, }, }, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - excluded = nil - if err := GetExcludedFiles(tt.args.dockerfilepath, tt.args.buildcontext); err != nil { - t.Fatal(err) - } - if excl := excludeFile(tt.args.excluded, tt.args.buildcontext); !excl { - t.Errorf("'%v' not excluded as expected", tt.args.excluded) - } - if excl := excludeFile(tt.args.notExcluded, tt.args.buildcontext); excl { - t.Errorf("'%v' excluded against expectation", tt.args.notExcluded) - } - }) + if err := GetExcludedFiles(tt.args.dockerfilepath, tt.args.buildcontext); err != nil { + t.Fatal(err) + } + for _, excl := range tt.args.excluded { + t.Run(tt.name+" to exclude "+excl, func(t *testing.T) { + if !excludeFile(excl, tt.args.buildcontext) { + t.Errorf("'%v' not excluded", excl) + } + }) + } + for _, incl := range tt.args.included { + t.Run(tt.name+" to include "+incl, func(t *testing.T) { + if excludeFile(incl, tt.args.buildcontext) { + t.Errorf("'%v' not included", incl) + } + }) + } } } From ff7abba47bb721e36027dbe4b092089c7f0d4798 Mon Sep 17 00:00:00 2001 From: Victor Noel Date: Fri, 4 Oct 2019 10:31:40 +0200 Subject: [PATCH 3/3] Fix integration tests for #776 --- ...dockerignore_relative => Dockerfile_dockerignore_relative} | 4 +++- ...erignore => Dockerfile_dockerignore_relative.dockerignore} | 0 pkg/util/fs_util_test.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) rename integration/dockerfiles/{Dockerfile_test_dockerignore_relative => Dockerfile_dockerignore_relative} (56%) rename integration/dockerfiles/{Dockerfile_test_dockerignore_relative.dockerignore => Dockerfile_dockerignore_relative.dockerignore} (100%) diff --git a/integration/dockerfiles/Dockerfile_test_dockerignore_relative b/integration/dockerfiles/Dockerfile_dockerignore_relative similarity index 56% rename from integration/dockerfiles/Dockerfile_test_dockerignore_relative rename to integration/dockerfiles/Dockerfile_dockerignore_relative index bc94268059..6ef2bac592 100644 --- a/integration/dockerfiles/Dockerfile_test_dockerignore_relative +++ b/integration/dockerfiles/Dockerfile_dockerignore_relative @@ -1,5 +1,7 @@ +# This is not included in integration tests because docker build does not exploit Dockerfile.dockerignore +# See https://github.com/moby/moby/issues/12886#issuecomment-523706042 for more details # This dockerfile makes sure Dockerfile.dockerignore is working # If so then ignore_relative/foo should copy to /foo # If not, then this image won't build because it will attempt to copy three files to /foo, which is a file not a directory -FROM scratch +FROM scratch COPY ignore_relative/* /foo diff --git a/integration/dockerfiles/Dockerfile_test_dockerignore_relative.dockerignore b/integration/dockerfiles/Dockerfile_dockerignore_relative.dockerignore similarity index 100% rename from integration/dockerfiles/Dockerfile_test_dockerignore_relative.dockerignore rename to integration/dockerfiles/Dockerfile_dockerignore_relative.dockerignore diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 449df74645..886efcf8fe 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -709,7 +709,7 @@ func Test_correctDockerignoreFileIsUsed(t *testing.T) { { name: "relative dockerfile used", args: args{ - dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore_relative", + dockerfilepath: "../../integration/dockerfiles/Dockerfile_dockerignore_relative", buildcontext: "../../integration/", excluded: []string{"ignore_relative/bar"}, included: []string{"ignore_relative/foo", "ignore/bar"},