Skip to content

Commit

Permalink
Fix bugs with .dockerignore and improve integration test
Browse files Browse the repository at this point in the history
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 (!)
  • Loading branch information
Priya Wadhwa committed Oct 31, 2018
1 parent 6ef616b commit 8a4b083
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 74 deletions.
52 changes: 24 additions & 28 deletions cmd/executor/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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)) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
112 changes: 112 additions & 0 deletions integration/dockerignore.go
Original file line number Diff line number Diff line change
@@ -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"}

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", path}
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
}
41 changes: 0 additions & 41 deletions integration/images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
25 changes: 25 additions & 0 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
8 changes: 3 additions & 5 deletions pkg/dockerfile/dockerfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

0 comments on commit 8a4b083

Please sign in to comment.