Fix bugs with .dockerignore and improve integration test

I improved handling of the .dockerignore file by:

1. Using docker's parser to parse the .dockerignore and using their
helper functions to determine if a file should be deleted
2. Copying the Dockerfile we are building to /kaniko/Dockerfile so that
if the Dockerfile is specified in .dockerignore it won't be deleted, and
if it is specified in the .dockerignore it won't end up in the final
image
3. I also improved the integration test to create a temp directory with
files to ignore, and updated the .dockerignore to include exclusions (!)

cmd/executor/cmd/root.go

@@ -28,6 +28,7 @@ import (
 	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
 	"github.com/GoogleContainerTools/kaniko/pkg/executor"
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
+	"github.com/docker/docker/pkg/fileutils"
 	"github.com/genuinetools/amicontained/container"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -62,10 +63,10 @@ var RootCmd = &cobra.Command{
 		if err := resolveSourceContext(); err != nil {
 			return errors.Wrap(err, "error resolving source context")
 		}
-		if err := removeIgnoredFiles(); err != nil {
-			return errors.Wrap(err, "error removing .dockerignore files from build context")
+		if err := resolveDockerfilePath(); err != nil {
+			return errors.Wrap(err, "error resolving dockerfile path")
 		}
-		return resolveDockerfilePath()
+		return removeIgnoredFiles()
 	},
 	Run: func(cmd *cobra.Command, args []string) {
 		if !checkContained() {
@@ -144,7 +145,7 @@ func resolveDockerfilePath() error {
 			return errors.Wrap(err, "getting absolute path for dockerfile")
 		}
 		opts.DockerfilePath = abs
-		return nil
+		return copyDockerfile()
 	}
 	// Otherwise, check if the path relative to the build context exists
 	if util.FilepathExists(filepath.Join(opts.SrcContext, opts.DockerfilePath)) {
@@ -153,11 +154,21 @@ func resolveDockerfilePath() error {
 			return errors.Wrap(err, "getting absolute path for src context/dockerfile path")
 		}
 		opts.DockerfilePath = abs
-		return nil
+		return copyDockerfile()
 	}
 	return errors.New("please provide a valid path to a Dockerfile within the build context with --dockerfile")
 }
 
+// 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 {
+		return errors.Wrap(err, "copying dockerfile")
+	}
+	opts.DockerfilePath = constants.DockerfilePath
+	return nil
+}
+
 // resolveSourceContext unpacks the source context if it is a tar in a bucket
 // it resets srcContext to be the path to the unpacked build context within the image
 func resolveSourceContext() error {
@@ -197,33 +208,18 @@ func removeIgnoredFiles() error {
 		return err
 	}
 	logrus.Infof("Removing ignored files from build context: %s", ignore)
-	for r, i := range ignore {
-		ignore[r] = filepath.Clean(filepath.Join(opts.SrcContext, i))
+	files, err := util.RelativeFiles("", opts.SrcContext)
+	if err != nil {
+		return errors.Wrap(err, "getting all files in src context")
 	}
-	// remove all files in .dockerignore
-	return filepath.Walk(opts.SrcContext, func(path string, fi os.FileInfo, _ error) error {
-		if ignoreFile(path, ignore) {
-			if err := os.RemoveAll(path); err != nil {
-				// don't return error, because this path could have been removed already
-				logrus.Debugf("error removing %s from buildcontext", path)
+	for _, f := range files {
+		if rm, _ := fileutils.Matches(f, ignore); rm {
+			if err := os.RemoveAll(f); err != nil {
+				logrus.Errorf("Error removing %s from build context", f)
 			}
 		}
-		return nil
-	})
-}
-
-// ignoreFile returns true if the path matches any of the paths in ignore
-func ignoreFile(path string, ignore []string) bool {
-	for _, i := range ignore {
-		matched, err := filepath.Match(i, path)
-		if err != nil {
-			return false
-		}
-		if matched {
-			return true
-		}
 	}
-	return false
+	return nil
 }
 
 func exit(err error) {

integration/dockerignore.go

@@ -0,0 +1,112 @@
+/*
+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
+}

integration/images.go

@@ -37,7 +37,6 @@ const (
 	buildContextPath = "/workspace"
 	cacheDir         = "/workspace/cache"
 	baseImageToCache = "gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0"
-	testDirPath      = "context/test"
 )
 
 // Arguments to build Dockerfiles with, used for both docker and kaniko builds
@@ -55,8 +54,6 @@ var argsMap = map[string][]string{
 	"Dockerfile_test_multistage": {"file=/foo2"},
 }
 
-var filesToIgnore = []string{"context/test/*"}
-
 // Arguments to build Dockerfiles with when building with docker
 var additionalDockerFlagsMap = map[string][]string{
 	"Dockerfile_test_target": {"--target=second"},
@@ -161,25 +158,12 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do
 			"."},
 			additionalFlags...)...,
 	)
-	if err := setupTestDir(); err != nil {
-		return err
-	}
-	if err := generateDockerIgnore(); err != nil {
-		return err
-	}
 
 	_, err := RunCommandWithoutTest(dockerCmd)
 	if err != nil {
 		return fmt.Errorf("Failed to build image %s with docker command \"%s\": %s", dockerImage, dockerCmd.Args, err)
 	}
 
-	if err := setupTestDir(); err != nil {
-		return err
-	}
-	if err := generateDockerIgnore(); err != nil {
-		return err
-	}
-
 	contextFlag := "-c"
 	contextPath := buildContextPath
 	for _, d := range bucketContextTests {
@@ -269,28 +253,3 @@ func (d *DockerFileBuilder) buildCachedImages(imageRepo, cacheRepo, dockerfilesP
 	}
 	return nil
 }
-
-func setupTestDir() error {
-	if err := os.MkdirAll(testDirPath, 0750); err != nil {
-		return err
-	}
-	p := filepath.Join(testDirPath, "foo")
-	f, err := os.Create(p)
-	if err != nil {
-		return err
-	}
-	return f.Close()
-}
-
-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
-}

integration/integration_test.go

@@ -275,6 +275,31 @@ func TestCache(t *testing.T) {
 	}
 }
 
+func TestDockerignore(t *testing.T) {
+	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

pkg/constants/constants.go

@@ -33,6 +33,9 @@ const (
 
 	Author = "kaniko"
 
+	// DockerfilePath is the path the Dockerfile is copied to
+	DockerfilePath = "/kaniko/Dockerfile"
+
 	// ContextTar is the default name of the tar uploaded to GCS buckets
 	ContextTar = "context.tar.gz"
 

pkg/dockerfile/dockerfile.go

@@ -26,6 +26,7 @@ import (
 
 	"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"
@@ -183,9 +184,6 @@ func ParseDockerignore(opts *config.KanikoOptions) ([]string, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "parsing .dockerignore")
 	}
-	return strings.FieldsFunc(string(contents), split), nil
-}
-
-func split(r rune) bool {
-	return r == '\n' || r == ' '
+	reader := bytes.NewBuffer(contents)
+	return dockerignore.ReadAll(reader)
 }