From b0b36ed85a9a85ffb1b2075622be3c4cd7111a82 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 10 Dec 2018 14:32:28 -0800 Subject: [PATCH] Re-add support for .dockerignore file This PR adds support for the dockerignore file. Previously when kaniko had support for the dockerignore file, kaniko first went through the build context and deleted files that were meant to be ignored. This resulted in a really bad bug where files in user mounted volumes would be deleted (my bad). This time around, instead of modifying the build context at all, kaniko will check if a file should be excluded when executing ADD/COPY commands. If a file should be excluded (based on the .dockerignore) it won't be copied over from the buildcontext and shouldn't end up in the final image. I also added a .dockerignore file and Dockerfile as an integration test, which should fail if the dockerignore is not being processed correctly or if files aren't being excluded correctly. Also, I removed all the integration testing from the previous version of the dockerignore support. --- cmd/executor/cmd/root.go | 2 +- integration/.dockerignore | 3 + .../dockerfiles/Dockerfile_test_dockerignore | 5 + integration/dockerignore.go | 112 ------------------ integration/ignore/bar | 0 integration/ignore/baz | 0 integration/ignore/foo | 0 integration/integration_test.go | 28 ----- pkg/commands/copy.go | 14 ++- pkg/constants/constants.go | 3 + pkg/dockerfile/dockerfile.go | 19 --- pkg/util/command_util.go | 16 ++- pkg/util/command_util_test.go | 51 ++++++-- pkg/util/fs_util.go | 76 +++++++++--- pkg/util/fs_util_test.go | 2 +- 15 files changed, 139 insertions(+), 192 deletions(-) create mode 100644 integration/.dockerignore create mode 100644 integration/dockerfiles/Dockerfile_test_dockerignore delete mode 100644 integration/dockerignore.go create mode 100644 integration/ignore/bar create mode 100644 integration/ignore/baz create mode 100644 integration/ignore/foo diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 8e22850c6..303f1ef4c 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -162,7 +162,7 @@ func resolveDockerfilePath() error { // copy Dockerfile to /kaniko/Dockerfile so that if it's specified in the .dockerignore // it won't be copied into the image func copyDockerfile() error { - if err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath); err != nil { + if _, err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath, ""); err != nil { return errors.Wrap(err, "copying dockerfile") } opts.DockerfilePath = constants.DockerfilePath diff --git a/integration/.dockerignore b/integration/.dockerignore new file mode 100644 index 000000000..ec574e06f --- /dev/null +++ b/integration/.dockerignore @@ -0,0 +1,3 @@ +# A .dockerignore file to make sure dockerignore support works +ignore/** +!ignore/foo diff --git a/integration/dockerfiles/Dockerfile_test_dockerignore b/integration/dockerfiles/Dockerfile_test_dockerignore new file mode 100644 index 000000000..d646872ef --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_dockerignore @@ -0,0 +1,5 @@ +# This dockerfile makes sure the .dockerignore is working +# If so then ignore/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/* /foo diff --git a/integration/dockerignore.go b/integration/dockerignore.go deleted file mode 100644 index f16f2fec7..000000000 --- a/integration/dockerignore.go +++ /dev/null @@ -1,112 +0,0 @@ -/* -Copyright 2018 Google LLC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package integration - -import ( - "fmt" - "os" - "os/exec" - "path" - "path/filepath" - "runtime" - "strings" -) - -var filesToIgnore = []string{"ignore/fo*", "!ignore/foobar", "ignore/Dockerfile_test_ignore"} - -const ( - ignoreDir = "ignore" - ignoreDockerfile = "Dockerfile_test_ignore" - ignoreDockerfileContents = `FROM scratch - COPY . .` -) - -// Set up a test dir to ignore with the structure: -// ignore -// -- Dockerfile_test_ignore -// -- foo -// -- foobar - -func setupIgnoreTestDir() error { - if err := os.MkdirAll(ignoreDir, 0750); err != nil { - return err - } - // Create and write contents to dockerfile - path := filepath.Join(ignoreDir, ignoreDockerfile) - f, err := os.Create(path) - if err != nil { - return err - } - defer f.Close() - if _, err := f.Write([]byte(ignoreDockerfileContents)); err != nil { - return err - } - - additionalFiles := []string{"ignore/foo", "ignore/foobar"} - for _, add := range additionalFiles { - a, err := os.Create(add) - if err != nil { - return err - } - defer a.Close() - } - return generateDockerIgnore() -} - -// generate the .dockerignore file -func generateDockerIgnore() error { - f, err := os.Create(".dockerignore") - if err != nil { - return err - } - defer f.Close() - contents := strings.Join(filesToIgnore, "\n") - if _, err := f.Write([]byte(contents)); err != nil { - return err - } - return nil -} - -func generateDockerignoreImages(imageRepo string) error { - - dockerfilePath := filepath.Join(ignoreDir, ignoreDockerfile) - - dockerImage := strings.ToLower(imageRepo + dockerPrefix + ignoreDockerfile) - dockerCmd := exec.Command("docker", "build", - "-t", dockerImage, - "-f", path.Join(dockerfilePath), - ".") - _, err := RunCommandWithoutTest(dockerCmd) - if err != nil { - return fmt.Errorf("Failed to build image %s with docker command \"%s\": %s", dockerImage, dockerCmd.Args, err) - } - - _, ex, _, _ := runtime.Caller(0) - cwd := filepath.Dir(ex) - kanikoImage := GetKanikoImage(imageRepo, ignoreDockerfile) - kanikoCmd := exec.Command("docker", - "run", - "-v", os.Getenv("HOME")+"/.config/gcloud:/root/.config/gcloud", - "-v", cwd+":/workspace", - ExecutorImage, - "-f", path.Join(buildContextPath, dockerfilePath), - "-d", kanikoImage, - "-c", buildContextPath) - - _, err = RunCommandWithoutTest(kanikoCmd) - return err -} diff --git a/integration/ignore/bar b/integration/ignore/bar new file mode 100644 index 000000000..e69de29bb diff --git a/integration/ignore/baz b/integration/ignore/baz new file mode 100644 index 000000000..e69de29bb diff --git a/integration/ignore/foo b/integration/ignore/foo new file mode 100644 index 000000000..e69de29bb diff --git a/integration/integration_test.go b/integration/integration_test.go index c1d4dd6b0..418815c18 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -286,34 +286,6 @@ func TestCache(t *testing.T) { } } -func TestDockerignore(t *testing.T) { - // TODO (priyawadhwa@): remove this once .dockerignore is implemented correctly - t.Skip() - - t.Run(fmt.Sprintf("test_%s", ignoreDockerfile), func(t *testing.T) { - if err := setupIgnoreTestDir(); err != nil { - t.Fatalf("error setting up ignore test dir: %v", err) - } - if err := generateDockerignoreImages(config.imageRepo); err != nil { - t.Fatalf("error generating dockerignore test images: %v", err) - } - - dockerImage := GetDockerImage(config.imageRepo, ignoreDockerfile) - kanikoImage := GetKanikoImage(config.imageRepo, ignoreDockerfile) - - // container-diff - daemonDockerImage := daemonPrefix + dockerImage - containerdiffCmd := exec.Command("container-diff", "diff", - daemonDockerImage, kanikoImage, - "-q", "--type=file", "--type=metadata", "--json") - diff := RunCommand(containerdiffCmd, t) - t.Logf("diff = %s", string(diff)) - - expected := fmt.Sprintf(emptyContainerDiff, dockerImage, kanikoImage, dockerImage, kanikoImage) - checkContainerDiffOutput(t, diff, expected) - }) -} - type fileDiff struct { Name string Size int diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 1e8f196b8..46e7f08aa 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -70,22 +70,30 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu // we need to add '/' to the end to indicate the destination is a directory dest = filepath.Join(cwd, dest) + "/" } - copiedFiles, err := util.CopyDir(fullPath, dest) + copiedFiles, err := util.CopyDir(fullPath, dest, c.buildcontext) if err != nil { return err } c.snapshotFiles = append(c.snapshotFiles, copiedFiles...) } else if fi.Mode()&os.ModeSymlink != 0 { // If file is a symlink, we want to create the same relative symlink - if err := util.CopySymlink(fullPath, destPath); err != nil { + exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext) + if err != nil { return err } + if exclude { + continue + } c.snapshotFiles = append(c.snapshotFiles, destPath) } else { // ... Else, we want to copy over a file - if err := util.CopyFile(fullPath, destPath); err != nil { + exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext) + if err != nil { return err } + if exclude { + continue + } c.snapshotFiles = append(c.snapshotFiles, destPath) } } diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 4b8a763eb..1fe9ef394 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -66,6 +66,9 @@ const ( // Docker command names Cmd = "cmd" Entrypoint = "entrypoint" + + // Name of the .dockerignore file + Dockerignore = ".dockerignore" ) // KanikoBuildFiles is the list of files required to build kaniko diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 2a886d0f6..5fa415a8b 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -20,13 +20,11 @@ import ( "bytes" "fmt" "io/ioutil" - "path/filepath" "strconv" "strings" "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/util" - "github.com/docker/docker/builder/dockerignore" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/moby/buildkit/frontend/dockerfile/parser" "github.com/pkg/errors" @@ -172,20 +170,3 @@ func saveStage(index int, stages []instructions.Stage) bool { } return false } - -// DockerignoreExists returns true if .dockerignore exists in the source context -func DockerignoreExists(opts *config.KanikoOptions) bool { - path := filepath.Join(opts.SrcContext, ".dockerignore") - return util.FilepathExists(path) -} - -// ParseDockerignore returns a list of all paths in .dockerignore -func ParseDockerignore(opts *config.KanikoOptions) ([]string, error) { - path := filepath.Join(opts.SrcContext, ".dockerignore") - contents, err := ioutil.ReadFile(path) - if err != nil { - return nil, errors.Wrap(err, "parsing .dockerignore") - } - reader := bytes.NewBuffer(contents) - return dockerignore.ReadAll(reader) -} diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 828b9e827..c217fc74d 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -186,7 +186,14 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []stri dest := srcsAndDest[len(srcsAndDest)-1] if !ContainsWildcards(srcs) { - if len(srcs) > 1 && !IsDestDir(dest) { + totalSrcs := 0 + for _, src := range srcs { + if excludeFile(src, root) { + continue + } + totalSrcs++ + } + if totalSrcs > 1 && !IsDestDir(dest) { return errors.New("when specifying multiple sources in a COPY command, destination must be a directory and end in '/'") } } @@ -216,7 +223,12 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []stri if err != nil { return err } - totalFiles += len(files) + for _, file := range files { + if excludeFile(file, root) { + continue + } + totalFiles++ + } } if totalFiles == 0 { return errors.New("copy failed: no source files specified") diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index dd4ddbf21..d4acfe003 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -249,11 +249,13 @@ func Test_MatchSources(t *testing.T) { } var isSrcValidTests = []struct { + name string srcsAndDest []string resolvedSources []string shouldErr bool }{ { + name: "dest isn't directory", srcsAndDest: []string{ "context/foo", "context/bar", @@ -266,6 +268,7 @@ var isSrcValidTests = []struct { shouldErr: true, }, { + name: "dest is directory", srcsAndDest: []string{ "context/foo", "context/bar", @@ -278,6 +281,7 @@ var isSrcValidTests = []struct { shouldErr: false, }, { + name: "copy file to file", srcsAndDest: []string{ "context/bar/bam", "dest", @@ -288,16 +292,7 @@ var isSrcValidTests = []struct { shouldErr: false, }, { - srcsAndDest: []string{ - "context/foo", - "dest", - }, - resolvedSources: []string{ - "context/foo", - }, - shouldErr: false, - }, - { + name: "copy files with wildcards to dir", srcsAndDest: []string{ "context/foo", "context/b*", @@ -310,6 +305,7 @@ var isSrcValidTests = []struct { shouldErr: false, }, { + name: "copy multilple files with wildcards to file", srcsAndDest: []string{ "context/foo", "context/b*", @@ -322,6 +318,7 @@ var isSrcValidTests = []struct { shouldErr: true, }, { + name: "copy two files to file, one of which doesn't exist", srcsAndDest: []string{ "context/foo", "context/doesntexist*", @@ -333,6 +330,7 @@ var isSrcValidTests = []struct { shouldErr: false, }, { + name: "copy dir to dest not specified as dir", srcsAndDest: []string{ "context/", "dest", @@ -343,6 +341,7 @@ var isSrcValidTests = []struct { shouldErr: false, }, { + name: "copy url to file", srcsAndDest: []string{ testURL, "dest", @@ -352,12 +351,40 @@ var isSrcValidTests = []struct { }, shouldErr: false, }, + { + name: "copy two srcs, one excluded, to file", + srcsAndDest: []string{ + "ignore/foo", + "ignore/bar", + "dest", + }, + resolvedSources: []string{ + "ignore/foo", + "ignore/bar", + }, + shouldErr: false, + }, + { + name: "copy two srcs, both excluded, to file", + srcsAndDest: []string{ + "ignore/baz", + "ignore/bar", + "dest", + }, + resolvedSources: []string{ + "ignore/baz", + "ignore/bar", + }, + shouldErr: true, + }, } func Test_IsSrcsValid(t *testing.T) { for _, test := range isSrcValidTests { - err := IsSrcsValid(test.srcsAndDest, test.resolvedSources, buildContextPath) - testutil.CheckError(t, test.shouldErr, err) + t.Run(test.name, func(t *testing.T) { + err := IsSrcsValid(test.srcsAndDest, test.resolvedSources, buildContextPath) + testutil.CheckError(t, test.shouldErr, err) + }) } } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 8b4829844..c7c130ebd 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -19,7 +19,9 @@ package util import ( "archive/tar" "bufio" + "bytes" "io" + "io/ioutil" "net/http" "os" "path/filepath" @@ -27,10 +29,11 @@ import ( "syscall" "time" + "github.com/GoogleContainerTools/kaniko/pkg/constants" + "github.com/docker/docker/builder/dockerignore" + "github.com/docker/docker/pkg/fileutils" "github.com/google/go-containerregistry/pkg/v1" "github.com/pkg/errors" - - "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/sirupsen/logrus" ) @@ -462,7 +465,7 @@ func DownloadFileToDest(rawurl, dest string) error { // CopyDir copies the file or directory at src to dest // It returns a list of files it copied over -func CopyDir(src, dest string) ([]string, error) { +func CopyDir(src, dest, buildcontext string) ([]string, error) { files, err := RelativeFiles("", src) if err != nil { return nil, err @@ -474,6 +477,10 @@ func CopyDir(src, dest string) ([]string, error) { if err != nil { return nil, err } + if excludeFile(fullPath, buildcontext) { + logrus.Debugf("%s found in .dockerignore, ignoring", src) + continue + } destPath := filepath.Join(dest, file) if fi.IsDir() { logrus.Debugf("Creating directory %s", destPath) @@ -489,12 +496,12 @@ func CopyDir(src, dest string) ([]string, error) { } } else if fi.Mode()&os.ModeSymlink != 0 { // If file is a symlink, we want to create the same relative symlink - if err := CopySymlink(fullPath, destPath); err != nil { + if _, err := CopySymlink(fullPath, destPath, buildcontext); err != nil { return nil, err } } else { // ... Else, we want to copy over a file - if err := CopyFile(fullPath, destPath); err != nil { + if _, err := CopyFile(fullPath, destPath, buildcontext); err != nil { return nil, err } } @@ -504,37 +511,78 @@ func CopyDir(src, dest string) ([]string, error) { } // CopySymlink copies the symlink at src to dest -func CopySymlink(src, dest string) error { +func CopySymlink(src, dest, buildcontext string) (bool, error) { + if excludeFile(src, buildcontext) { + logrus.Debugf("%s found in .dockerignore, ignoring", src) + return true, nil + } link, err := os.Readlink(src) if err != nil { - return err + return false, err } if FilepathExists(dest) { if err := os.RemoveAll(dest); err != nil { - return err + return false, err } } - return os.Symlink(link, dest) + return false, os.Symlink(link, dest) } // CopyFile copies the file at src to dest -func CopyFile(src, dest string) error { +func CopyFile(src, dest, buildcontext string) (bool, error) { + if excludeFile(src, buildcontext) { + logrus.Debugf("%s found in .dockerignore, ignoring", src) + return true, nil + } fi, err := os.Stat(src) if err != nil { - return err + return false, err } logrus.Debugf("Copying file %s to %s", src, dest) srcFile, err := os.Open(src) if err != nil { - return err + return false, err } defer srcFile.Close() uid := fi.Sys().(*syscall.Stat_t).Uid gid := fi.Sys().(*syscall.Stat_t).Gid - return CreateFile(dest, srcFile, fi.Mode(), uid, gid) + return false, CreateFile(dest, srcFile, fi.Mode(), uid, gid) } -// HasFilepathPrefix checks if the given file path begins with prefix +// excludeFile returns true if the .dockerignore specified this file should be ignored +func excludeFile(path, buildcontext string) bool { + excluded, err := parseDockerignore(buildcontext) + if err != nil { + return false + } + if HasFilepathPrefix(path, buildcontext, false) { + path, err = filepath.Rel(buildcontext, path) + if err != nil { + return false + } + } + match, err := fileutils.Matches(path, excluded) + if err != nil { + return false + } + return match +} + +// parseDockerignore returns a list of all paths in .dockerignore +func parseDockerignore(buildcontext string) ([]string, error) { + path := filepath.Join(buildcontext, ".dockerignore") + if !FilepathExists(path) { + return nil, nil + } + contents, err := ioutil.ReadFile(path) + if err != nil { + return nil, errors.Wrap(err, "parsing .dockerignore") + } + reader := bytes.NewBuffer(contents) + return dockerignore.ReadAll(reader) +} + +// HasFilepathPrefix checks if the given file path begins with prefix func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool { path = filepath.Clean(path) prefix = filepath.Clean(prefix) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index b259552e1..0f34d0ba1 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -583,7 +583,7 @@ func TestCopySymlink(t *testing.T) { if err := os.Symlink(tc.linkTarget, link); err != nil { t.Fatal(err) } - if err := CopySymlink(link, dest); err != nil { + if _, err := CopySymlink(link, dest, ""); err != nil { t.Fatal(err) } got, err := os.Readlink(dest)