From 5f4e2f136633d241444129517a4e75ebf44065c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tinjo=20Sch=C3=B6ni?= <32767367+tscni@users.noreply.github.com> Date: Thu, 8 Oct 2020 21:47:14 +0200 Subject: [PATCH] Fix .dockerignore for build context copies in later stages (#1447) * Extend .dockerignore integration test with copies in later stages .dockerignore should continue to apply when copying from the build context in later stages, but it currently doesn't * Replace excluded global with passed along FileContext struct This new FileContext struct allows much cleaner handling of context specific file exclusions. The global excluded file state is no longer needed. Additionally this also fixes the issue where excluded files aren't being applied for build context copies in later build stages. --- cmd/executor/cmd/root.go | 2 +- .../dockerfiles/Dockerfile_test_dockerignore | 12 +++- pkg/commands/add.go | 12 ++-- pkg/commands/commands.go | 7 ++- pkg/commands/copy.go | 22 +++---- pkg/commands/copy_test.go | 32 +++++----- pkg/executor/build.go | 17 +++--- pkg/executor/build_test.go | 17 +++--- pkg/executor/composite_cache.go | 10 ++-- pkg/executor/composite_cache_test.go | 51 ++++++++-------- pkg/util/command_util.go | 16 ++--- pkg/util/command_util_test.go | 7 ++- pkg/util/fs_util.go | 60 +++++++++++-------- pkg/util/fs_util_test.go | 11 ++-- 14 files changed, 150 insertions(+), 126 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 983c78169..e53f84102 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -243,7 +243,7 @@ func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) strin // 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, "", util.DoNotChangeUID, util.DoNotChangeGID); err != nil { + if _, err := util.CopyFile(opts.DockerfilePath, constants.DockerfilePath, util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID); err != nil { return errors.Wrap(err, "copying dockerfile") } opts.DockerfilePath = constants.DockerfilePath diff --git a/integration/dockerfiles/Dockerfile_test_dockerignore b/integration/dockerfiles/Dockerfile_test_dockerignore index 70667623d..d82504940 100644 --- a/integration/dockerfiles/Dockerfile_test_dockerignore +++ b/integration/dockerfiles/Dockerfile_test_dockerignore @@ -7,5 +7,13 @@ COPY ignore/* /foo From base as first COPY --from=base /foo ignore/bar -FROM first -COPY --from=first ignore/* /fooAnother/ \ No newline at end of file +# Make sure that .dockerignore also applies for later stages +FROM scratch as base2 +COPY ignore/* /foo + +From base2 as second +COPY --from=base2 /foo ignore/bar + +FROM scratch +COPY --from=first ignore/* /fooAnother/ +COPY --from=second ignore/* /fooAnother2/ diff --git a/pkg/commands/add.go b/pkg/commands/add.go index b39b5eb98..ae86f3538 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -32,7 +32,7 @@ import ( type AddCommand struct { BaseCommand cmd *instructions.AddCommand - buildcontext string + fileContext util.FileContext snapshotFiles []string } @@ -52,7 +52,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui return errors.Wrap(err, "getting user group from chown") } - srcs, dest, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.buildcontext, replacementEnvs) + srcs, dest, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.fileContext, replacementEnvs) if err != nil { return err } @@ -64,7 +64,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui // 1. Download and copy it to the specified dest // Else, add to the list of unresolved sources for _, src := range srcs { - fullPath := filepath.Join(a.buildcontext, src) + fullPath := filepath.Join(a.fileContext.Root, src) if util.IsSrcRemoteFileURL(src) { urlDest, err := util.URLDestinationFilepath(src, dest, config.WorkingDir, replacementEnvs) if err != nil { @@ -101,7 +101,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui SourcesAndDest: append(unresolvedSrcs, dest), Chown: a.cmd.Chown, }, - buildcontext: a.buildcontext, + fileContext: a.fileContext, } if err := copyCmd.ExecuteCommand(config, buildArgs); err != nil { @@ -124,7 +124,7 @@ func (a *AddCommand) String() string { func (a *AddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) { replacementEnvs := buildArgs.ReplacementEnvs(config.Env) - srcs, _, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.buildcontext, replacementEnvs) + srcs, _, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.fileContext, replacementEnvs) if err != nil { return nil, err } @@ -137,7 +137,7 @@ func (a *AddCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfi if util.IsFileLocalTarArchive(src) { continue } - fullPath := filepath.Join(a.buildcontext, src) + fullPath := filepath.Join(a.fileContext.Root, src) files = append(files, fullPath) } diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 2864113ee..61e6fd497 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -18,6 +18,7 @@ package commands import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + "github.com/GoogleContainerTools/kaniko/pkg/util" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/pkg/errors" @@ -59,7 +60,7 @@ type DockerCommand interface { ShouldDetectDeletedFiles() bool } -func GetCommand(cmd instructions.Command, buildcontext string, useNewRun bool) (DockerCommand, error) { +func GetCommand(cmd instructions.Command, fileContext util.FileContext, useNewRun bool) (DockerCommand, error) { switch c := cmd.(type) { case *instructions.RunCommand: if useNewRun { @@ -67,7 +68,7 @@ func GetCommand(cmd instructions.Command, buildcontext string, useNewRun bool) ( } return &RunCommand{cmd: c}, nil case *instructions.CopyCommand: - return &CopyCommand{cmd: c, buildcontext: buildcontext}, nil + return &CopyCommand{cmd: c, fileContext: fileContext}, nil case *instructions.ExposeCommand: return &ExposeCommand{cmd: c}, nil case *instructions.EnvCommand: @@ -75,7 +76,7 @@ func GetCommand(cmd instructions.Command, buildcontext string, useNewRun bool) ( case *instructions.WorkdirCommand: return &WorkdirCommand{cmd: c}, nil case *instructions.AddCommand: - return &AddCommand{cmd: c, buildcontext: buildcontext}, nil + return &AddCommand{cmd: c, fileContext: fileContext}, nil case *instructions.CmdCommand: return &CmdCommand{cmd: c}, nil case *instructions.EntrypointCommand: diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 7de8c639f..b4ae40956 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -39,14 +39,14 @@ var ( type CopyCommand struct { BaseCommand cmd *instructions.CopyCommand - buildcontext string + fileContext util.FileContext snapshotFiles []string } func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { // Resolve from if c.cmd.From != "" { - c.buildcontext = filepath.Join(kConfig.KanikoDir, c.cmd.From) + c.fileContext = util.FileContext{Root: filepath.Join(kConfig.KanikoDir, c.cmd.From)} } replacementEnvs := buildArgs.ReplacementEnvs(config.Env) @@ -55,14 +55,14 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu return errors.Wrap(err, "getting user group from chown") } - srcs, dest, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.buildcontext, replacementEnvs) + srcs, dest, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.fileContext, replacementEnvs) if err != nil { return errors.Wrap(err, "resolving src") } // For each source, iterate through and copy it over for _, src := range srcs { - fullPath := filepath.Join(c.buildcontext, src) + fullPath := filepath.Join(c.fileContext.Root, src) fi, err := os.Lstat(fullPath) if err != nil { @@ -89,14 +89,14 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu } if fi.IsDir() { - copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext, uid, gid) + copiedFiles, err := util.CopyDir(fullPath, destPath, c.fileContext, uid, gid) if err != nil { return errors.Wrap(err, "copying dir") } c.snapshotFiles = append(c.snapshotFiles, copiedFiles...) } else if util.IsSymlink(fi) { // If file is a symlink, we want to copy the target file to destPath - exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext) + exclude, err := util.CopySymlink(fullPath, destPath, c.fileContext) if err != nil { return errors.Wrap(err, "copying symlink") } @@ -106,7 +106,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu c.snapshotFiles = append(c.snapshotFiles, destPath) } else { // ... Else, we want to copy over a file - exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext, uid, gid) + exclude, err := util.CopyFile(fullPath, destPath, c.fileContext, uid, gid) if err != nil { return errors.Wrap(err, "copying file") } @@ -130,7 +130,7 @@ func (c *CopyCommand) String() string { } func (c *CopyCommand) FilesUsedFromContext(config *v1.Config, buildArgs *dockerfile.BuildArgs) ([]string, error) { - return copyCmdFilesUsedFromContext(config, buildArgs, c.cmd, c.buildcontext) + return copyCmdFilesUsedFromContext(config, buildArgs, c.cmd, c.fileContext) } func (c *CopyCommand) MetadataOnly() bool { @@ -186,7 +186,7 @@ func resolveIfSymlink(destPath string) (string, error) { func copyCmdFilesUsedFromContext( config *v1.Config, buildArgs *dockerfile.BuildArgs, cmd *instructions.CopyCommand, - buildcontext string, + fileContext util.FileContext, ) ([]string, error) { // We don't use the context if we're performing a copy --from. if cmd.From != "" { @@ -196,7 +196,7 @@ func copyCmdFilesUsedFromContext( replacementEnvs := buildArgs.ReplacementEnvs(config.Env) srcs, _, err := util.ResolveEnvAndWildcards( - cmd.SourcesAndDest, buildcontext, replacementEnvs, + cmd.SourcesAndDest, fileContext, replacementEnvs, ) if err != nil { return nil, err @@ -204,7 +204,7 @@ func copyCmdFilesUsedFromContext( files := []string{} for _, src := range srcs { - fullPath := filepath.Join(buildcontext, src) + fullPath := filepath.Join(fileContext.Root, src) files = append(files, fullPath) } diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 280ed0ec2..d189df260 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -26,6 +26,7 @@ import ( "testing" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/GoogleContainerTools/kaniko/testutil" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" @@ -113,6 +114,7 @@ func TestCopyExecuteCmd(t *testing.T) { Env: []string{}, WorkingDir: tempDir, } + fileContext := util.FileContext{Root: tempDir} for _, test := range copyTests { t.Run(test.name, func(t *testing.T) { @@ -122,7 +124,7 @@ func TestCopyExecuteCmd(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: test.sourcesAndDest, }, - buildcontext: tempDir, + fileContext: fileContext, } buildArgs := copySetUpBuildArgs() @@ -275,7 +277,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{srcDir, "dest"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -307,7 +309,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{filepath.Join(srcDir, "bam.txt"), "dest/"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -334,7 +336,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{filepath.Join(srcDir, "bam.txt"), "dest"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -363,7 +365,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{filepath.Join(srcDir, "bam.txt"), "dest"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -392,7 +394,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{filepath.Join(srcDir, "sym.link"), "dest/"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -437,7 +439,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{filepath.Join(srcDir, "dead.link"), "dest/"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -478,7 +480,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{"another", "dest"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -524,7 +526,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{srcDir, "dest"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -568,7 +570,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{"another", "dest"}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -611,7 +613,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{srcDir, linkedDest}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -656,7 +658,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{fmt.Sprintf("%s/bam.txt", srcDir), linkedDest}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -705,7 +707,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { SourcesAndDest: []string{fmt.Sprintf("%s/bam.txt", srcDir), testDir}, Chown: "alice:group", }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -750,7 +752,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { SourcesAndDest: []string{fmt.Sprintf("%s/bam.txt", srcDir), testDir}, Chown: "missing:missing", }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ @@ -781,7 +783,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { cmd: &instructions.CopyCommand{ SourcesAndDest: []string{srcDir, dest}, }, - buildcontext: testDir, + fileContext: util.FileContext{Root: testDir}, } cfg := &v1.Config{ diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 17f7753de..3de971cc9 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -73,6 +73,7 @@ type stageBuilder struct { baseImageDigest string finalCacheKey string opts *config.KanikoOptions + fileContext util.FileContext cmds []commands.DockerCommand args *dockerfile.BuildArgs crossStageDeps map[int][]string @@ -84,7 +85,7 @@ type stageBuilder struct { } // newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage -func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, crossStageDeps map[int][]string, dcm map[string]string, sid map[string]string, stageNameToIdx map[string]string) (*stageBuilder, error) { +func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, crossStageDeps map[int][]string, dcm map[string]string, sid map[string]string, stageNameToIdx map[string]string, fileContext util.FileContext) (*stageBuilder, error) { sourceImage, err := image_util.RetrieveSourceImage(stage, opts) if err != nil { return nil, err @@ -117,6 +118,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, cross snapshotter: snapshotter, baseImageDigest: digest.String(), opts: opts, + fileContext: fileContext, crossStageDeps: crossStageDeps, digestToCacheKey: dcm, stageIdxToDigest: sid, @@ -127,7 +129,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, cross } for _, cmd := range s.stage.Commands { - command, err := commands.GetCommand(cmd, opts.SrcContext, opts.RunV2) + command, err := commands.GetCommand(cmd, fileContext, opts.RunV2) if err != nil { return nil, err } @@ -187,10 +189,8 @@ func (s *stageBuilder) populateCompositeKey(command fmt.Stringer, files []string compositeKey = s.populateCopyCmdCompositeKey(command, v.From(), compositeKey) } - srcCtx := s.opts.SrcContext - for _, f := range files { - if err := compositeKey.AddPath(f, srcCtx); err != nil { + if err := compositeKey.AddPath(f, s.fileContext); err != nil { return compositeKey, err } } @@ -572,7 +572,8 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { } stageNameToIdx := ResolveCrossStageInstructions(kanikoStages) - if err := util.GetExcludedFiles(opts.DockerfilePath, opts.SrcContext); err != nil { + fileContext, err := util.NewFileContextFromDockerfile(opts.DockerfilePath, opts.SrcContext) + if err != nil { return nil, err } @@ -586,16 +587,14 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { } logrus.Infof("Built cross stage deps: %v", crossStageDependencies) - util.IsFirstStage = true for index, stage := range kanikoStages { - sb, err := newStageBuilder(opts, stage, crossStageDependencies, digestToCacheKey, stageIdxToDigest, stageNameToIdx) + sb, err := newStageBuilder(opts, stage, crossStageDependencies, digestToCacheKey, stageIdxToDigest, stageNameToIdx, fileContext) if err != nil { return nil, err } if err := sb.build(); err != nil { return nil, errors.Wrap(err, "error building stage") } - util.IsFirstStage = false reviewConfig(stage, &sb.cf.Config) diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index cfb655138..351755a87 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -31,6 +31,7 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/commands" "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/google/go-cmp/cmp" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -679,7 +680,7 @@ func Test_stageBuilder_populateCompositeKey(t *testing.T) { } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { - sb := &stageBuilder{opts: &config.KanikoOptions{SrcContext: "workspace"}} + sb := &stageBuilder{fileContext: util.FileContext{Root: "workspace"}} ck := CompositeCache{} ck1, err := sb.populateCompositeKey(tc.cmd1.command, []string{}, ck, tc.cmd1.args, tc.cmd1.env) @@ -720,7 +721,7 @@ func Test_stageBuilder_build(t *testing.T) { filePath := filepath.Join(dir, file) ch := NewCompositeCache("", "meow") - ch.AddPath(filePath, "") + ch.AddPath(filePath, util.FileContext{}) hash, err := ch.Hash() if err != nil { t.Errorf("couldn't create hash %v", err) @@ -753,7 +754,7 @@ func Test_stageBuilder_build(t *testing.T) { filePath := filepath.Join(dir, file) ch := NewCompositeCache("", "meow") - ch.AddPath(filePath, "") + ch.AddPath(filePath, util.FileContext{}) hash, err := ch.Hash() if err != nil { t.Errorf("couldn't create hash %v", err) @@ -856,7 +857,7 @@ COPY %s bar.txt // hash1 is the read cachekey for the first layer expectedCacheKeys: []string{hash1}, pushedCacheKeys: []string{}, - commands: getCommands(dir, cmds), + commands: getCommands(util.FileContext{Root: dir}, cmds), } }(), func() testcase { @@ -872,7 +873,7 @@ COPY %s bar.txt filePath := filepath.Join(dir, filename) ch := NewCompositeCache("", fmt.Sprintf("COPY %s bar.txt", filename)) - ch.AddPath(filePath, "") + ch.AddPath(filePath, util.FileContext{}) // copy hash _, err = ch.Hash() @@ -932,7 +933,7 @@ RUN foobar image: image, expectedCacheKeys: []string{runHash}, pushedCacheKeys: []string{}, - commands: getCommands(dir, cmds), + commands: getCommands(util.FileContext{Root: dir}, cmds), } }(), func() testcase { @@ -1139,12 +1140,12 @@ func assertCacheKeys(t *testing.T, expectedCacheKeys, actualCacheKeys []string, } } -func getCommands(dir string, cmds []instructions.Command) []commands.DockerCommand { +func getCommands(fileContext util.FileContext, cmds []instructions.Command) []commands.DockerCommand { outCommands := make([]commands.DockerCommand, 0) for _, c := range cmds { cmd, err := commands.GetCommand( c, - dir, + fileContext, false, ) if err != nil { diff --git a/pkg/executor/composite_cache.go b/pkg/executor/composite_cache.go index d6a624481..2c1e00d25 100644 --- a/pkg/executor/composite_cache.go +++ b/pkg/executor/composite_cache.go @@ -55,7 +55,7 @@ func (s *CompositeCache) Hash() (string, error) { return util.SHA256(strings.NewReader(s.Key())) } -func (s *CompositeCache) AddPath(p, context string) error { +func (s *CompositeCache) AddPath(p string, context util.FileContext) error { sha := sha256.New() fi, err := os.Lstat(p) if err != nil { @@ -70,13 +70,13 @@ func (s *CompositeCache) AddPath(p, context string) error { // Only add the hash of this directory to the key // if there is any ignored content. - if !empty || !util.ExcludeFile(p, context) { + if !empty || !context.ExcludesFile(p) { s.keys = append(s.keys, k) } return nil } - if util.ExcludeFile(p, context) { + if context.ExcludesFile(p) { return nil } fh, err := util.CacheHasher()(p) @@ -92,14 +92,14 @@ func (s *CompositeCache) AddPath(p, context string) error { } // HashDir returns a hash of the directory. -func hashDir(p, context string) (bool, string, error) { +func hashDir(p string, context util.FileContext) (bool, string, error) { sha := sha256.New() empty := true if err := filepath.Walk(p, func(path string, fi os.FileInfo, err error) error { if err != nil { return err } - exclude := util.ExcludeFile(path, context) + exclude := context.ExcludesFile(path) if exclude { return nil } diff --git a/pkg/executor/composite_cache_test.go b/pkg/executor/composite_cache_test.go index 764364e93..668226207 100644 --- a/pkg/executor/composite_cache_test.go +++ b/pkg/executor/composite_cache_test.go @@ -80,7 +80,7 @@ func Test_CompositeCache_AddPath_dir(t *testing.T) { fn := func() string { r := NewCompositeCache() - if err := r.AddPath(tmpDir, ""); err != nil { + if err := r.AddPath(tmpDir, util.FileContext{}); err != nil { t.Errorf("expected error to be nil but was %v", err) } @@ -118,7 +118,7 @@ func Test_CompositeCache_AddPath_file(t *testing.T) { p := tmpfile.Name() fn := func() string { r := NewCompositeCache() - if err := r.AddPath(p, ""); err != nil { + if err := r.AddPath(p, util.FileContext{}); err != nil { t.Errorf("expected error to be nil but was %v", err) } @@ -158,26 +158,23 @@ func createFilesystemStructure(root string, directories, files []string) error { return nil } -func setIgnoreContext(content string) error { +func setIgnoreContext(content string) (util.FileContext, error) { + var fileContext util.FileContext dockerIgnoreDir, err := ioutil.TempDir("", "") if err != nil { - return err + return fileContext, err } defer os.RemoveAll(dockerIgnoreDir) err = ioutil.WriteFile(dockerIgnoreDir+".dockerignore", []byte(content), 0644) if err != nil { - return err + return fileContext, err } - err = util.GetExcludedFiles(dockerIgnoreDir, "") - if err != nil { - return err - } - return nil + return util.NewFileContextFromDockerfile(dockerIgnoreDir, "") } -func hashDirectory(dirpath string) (string, error) { +func hashDirectory(dirpath string, fileContext util.FileContext) (string, error) { cache1 := NewCompositeCache() - err := cache1.AddPath(dirpath, dirpath) + err := cache1.AddPath(dirpath, fileContext) if err != nil { return "", err } @@ -217,6 +214,8 @@ func Test_CompositeKey_AddPath_Works(t *testing.T) { }, } + fileContext := util.FileContext{} + for _, test := range tests { t.Run(test.name, func(t *testing.T) { testDir1, err := ioutil.TempDir("", "") @@ -239,11 +238,11 @@ func Test_CompositeKey_AddPath_Works(t *testing.T) { t.Fatalf("Error creating filesytem structure: %s", err) } - hash1, err := hashDirectory(testDir1) + hash1, err := hashDirectory(testDir1, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } - hash2, err := hashDirectory(testDir2) + hash2, err := hashDirectory(testDir2, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } @@ -288,6 +287,8 @@ func Test_CompositeKey_AddPath_WithExtraFile_Works(t *testing.T) { }, } + fileContext := util.FileContext{} + for _, test := range tests { t.Run(test.name, func(t *testing.T) { testDir1, err := ioutil.TempDir("", "") @@ -315,11 +316,11 @@ func Test_CompositeKey_AddPath_WithExtraFile_Works(t *testing.T) { t.Fatalf("Error creating filesytem structure: %s", err) } - hash1, err := hashDirectory(testDir1) + hash1, err := hashDirectory(testDir1, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } - hash2, err := hashDirectory(testDir2) + hash2, err := hashDirectory(testDir2, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } @@ -364,6 +365,8 @@ func Test_CompositeKey_AddPath_WithExtraDir_Works(t *testing.T) { }, } + fileContext := util.FileContext{} + for _, test := range tests { t.Run(test.name, func(t *testing.T) { testDir1, err := ioutil.TempDir("", "") @@ -391,11 +394,11 @@ func Test_CompositeKey_AddPath_WithExtraDir_Works(t *testing.T) { t.Fatalf("Error creating filesytem structure: %s", err) } - hash1, err := hashDirectory(testDir1) + hash1, err := hashDirectory(testDir1, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } - hash2, err := hashDirectory(testDir2) + hash2, err := hashDirectory(testDir2, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } @@ -440,7 +443,7 @@ func Test_CompositeKey_AddPath_WithExtraFilIgnored_Works(t *testing.T) { }, } - err := setIgnoreContext("**/extra") + fileContext, err := setIgnoreContext("**/extra") if err != nil { t.Fatalf("Error setting exlusion context: %s", err) } @@ -472,11 +475,11 @@ func Test_CompositeKey_AddPath_WithExtraFilIgnored_Works(t *testing.T) { t.Fatalf("Error creating filesytem structure: %s", err) } - hash1, err := hashDirectory(testDir1) + hash1, err := hashDirectory(testDir1, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } - hash2, err := hashDirectory(testDir2) + hash2, err := hashDirectory(testDir2, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } @@ -521,7 +524,7 @@ func Test_CompositeKey_AddPath_WithExtraDirIgnored_Works(t *testing.T) { }, } - err := setIgnoreContext("**/extra") + fileContext, err := setIgnoreContext("**/extra") if err != nil { t.Fatalf("Error setting exlusion context: %s", err) } @@ -553,11 +556,11 @@ func Test_CompositeKey_AddPath_WithExtraDirIgnored_Works(t *testing.T) { t.Fatalf("Error creating filesytem structure: %s", err) } - hash1, err := hashDirectory(testDir1) + hash1, err := hashDirectory(testDir1, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } - hash2, err := hashDirectory(testDir2) + hash2, err := hashDirectory(testDir2, fileContext) if err != nil { t.Fatalf("Failed to calculate hash: %s", err) } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 77de80491..b049aa06c 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -85,7 +85,7 @@ func ResolveEnvironmentReplacement(value string, envs []string, isFilepath bool) return fp, nil } -func ResolveEnvAndWildcards(sd instructions.SourcesAndDest, buildcontext string, envs []string) ([]string, string, error) { +func ResolveEnvAndWildcards(sd instructions.SourcesAndDest, fileContext FileContext, envs []string) ([]string, string, error) { // First, resolve any environment replacement resolvedEnvs, err := ResolveEnvironmentReplacementList(sd, envs, true) if err != nil { @@ -96,11 +96,11 @@ func ResolveEnvAndWildcards(sd instructions.SourcesAndDest, buildcontext string, } dest := resolvedEnvs[len(resolvedEnvs)-1] // Resolve wildcards and get a list of resolved sources - srcs, err := ResolveSources(resolvedEnvs[0:len(resolvedEnvs)-1], buildcontext) + srcs, err := ResolveSources(resolvedEnvs[0:len(resolvedEnvs)-1], fileContext.Root) if err != nil { return nil, "", errors.Wrap(err, "failed to resolve sources") } - err = IsSrcsValid(sd, srcs, buildcontext) + err = IsSrcsValid(sd, srcs, fileContext) return srcs, dest, err } @@ -220,14 +220,14 @@ func URLDestinationFilepath(rawurl, dest, cwd string, envs []string) (string, er return destPath, nil } -func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []string, root string) error { +func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []string, fileContext FileContext) error { srcs := srcsAndDest[:len(srcsAndDest)-1] dest := srcsAndDest[len(srcsAndDest)-1] if !ContainsWildcards(srcs) { totalSrcs := 0 for _, src := range srcs { - if ExcludeFile(src, root) { + if fileContext.ExcludesFile(src) { continue } totalSrcs++ @@ -242,7 +242,7 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []stri if IsSrcRemoteFileURL(resolvedSources[0]) { return nil } - path := filepath.Join(root, resolvedSources[0]) + path := filepath.Join(fileContext.Root, resolvedSources[0]) fi, err := os.Lstat(path) if err != nil { return errors.Wrap(err, fmt.Sprintf("failed to get fileinfo for %v", path)) @@ -259,12 +259,12 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []stri continue } src = filepath.Clean(src) - files, err := RelativeFiles(src, root) + files, err := RelativeFiles(src, fileContext.Root) if err != nil { return errors.Wrap(err, "failed to get relative files") } for _, file := range files { - if ExcludeFile(file, root) { + if fileContext.ExcludesFile(file) { continue } totalFiles++ diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 018d9637b..616ad61e3 100644 --- a/pkg/util/command_util_test.go +++ b/pkg/util/command_util_test.go @@ -478,10 +478,11 @@ 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 { - t.Fatalf("error getting excluded files: %v", err) + fileContext, err := NewFileContextFromDockerfile("", buildContextPath) + if err != nil { + t.Fatalf("error creating file context: %v", err) } - err := IsSrcsValid(test.srcsAndDest, test.resolvedSources, buildContextPath) + err = IsSrcsValid(test.srcsAndDest, test.resolvedSources, fileContext) testutil.CheckError(t, test.shouldErr, err) }) } diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 694e3e739..3576a3977 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -74,8 +74,10 @@ var ignorelist = initialIgnoreList var volumes = []string{} -var excluded []string -var IsFirstStage = true +type FileContext struct { + Root string + ExcludedFiles []string +} type ExtractFunction func(string, *tar.Header, io.Reader) error @@ -581,7 +583,7 @@ func DetermineTargetFileOwnership(fi os.FileInfo, uid, gid int64) (int64, int64) // CopyDir copies the file or directory at src to dest // It returns a list of files it copied over -func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { +func CopyDir(src, dest string, context FileContext, uid, gid int64) ([]string, error) { files, err := RelativeFiles("", src) if err != nil { return nil, errors.Wrap(err, "copying dir") @@ -593,7 +595,7 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { if err != nil { return nil, errors.Wrap(err, "copying dir") } - if ExcludeFile(fullPath, buildcontext) { + if context.ExcludesFile(fullPath) { logrus.Debugf("%s found in .dockerignore, ignoring", src) continue } @@ -608,12 +610,12 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } } else if IsSymlink(fi) { // If file is a symlink, we want to create the same relative symlink - if _, err := CopySymlink(fullPath, destPath, buildcontext); err != nil { + if _, err := CopySymlink(fullPath, destPath, context); err != nil { return nil, err } } else { // ... Else, we want to copy over a file - if _, err := CopyFile(fullPath, destPath, buildcontext, uid, gid); err != nil { + if _, err := CopyFile(fullPath, destPath, context, uid, gid); err != nil { return nil, err } } @@ -623,8 +625,8 @@ func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) { } // CopySymlink copies the symlink at src to dest. -func CopySymlink(src, dest, buildcontext string) (bool, error) { - if ExcludeFile(src, buildcontext) { +func CopySymlink(src, dest string, context FileContext) (bool, error) { + if context.ExcludesFile(src) { logrus.Debugf("%s found in .dockerignore, ignoring", src) return true, nil } @@ -644,8 +646,8 @@ func CopySymlink(src, dest, buildcontext string) (bool, error) { } // CopyFile copies the file at src to dest -func CopyFile(src, dest, buildcontext string, uid, gid int64) (bool, error) { - if ExcludeFile(src, buildcontext) { +func CopyFile(src, dest string, context FileContext, uid, gid int64) (bool, error) { + if context.ExcludesFile(src) { logrus.Debugf("%s found in .dockerignore, ignoring", src) return true, nil } @@ -669,40 +671,46 @@ func CopyFile(src, dest, buildcontext string, uid, gid int64) (bool, error) { return false, CreateFile(dest, srcFile, fi.Mode(), uint32(uid), uint32(gid)) } -// GetExcludedFiles gets a list of files to exclude from the .dockerignore -func GetExcludedFiles(dockerfilepath string, buildcontext string) error { - path := dockerfilepath + ".dockerignore" +func NewFileContextFromDockerfile(dockerfilePath, buildcontext string) (FileContext, error) { + fileContext := FileContext{Root: buildcontext} + excludedFiles, err := getExcludedFiles(dockerfilePath, buildcontext) + if err != nil { + return fileContext, err + } + fileContext.ExcludedFiles = excludedFiles + return fileContext, nil +} + +// getExcludedFiles returns a list of files to exclude from the .dockerignore +func getExcludedFiles(dockerfilePath, buildcontext string) ([]string, error) { + path := dockerfilePath + ".dockerignore" if !FilepathExists(path) { path = filepath.Join(buildcontext, ".dockerignore") } if !FilepathExists(path) { - return nil + return nil, nil } logrus.Infof("Using dockerignore file: %v", path) contents, err := ioutil.ReadFile(path) if err != nil { - return errors.Wrap(err, "parsing .dockerignore") + return nil, errors.Wrap(err, "parsing .dockerignore") } reader := bytes.NewBuffer(contents) - excluded, err = dockerignore.ReadAll(reader) - return err + return dockerignore.ReadAll(reader) } -// ExcludeFile returns true if the .dockerignore specified this file should be ignored -func ExcludeFile(path, buildcontext string) bool { - // Apply dockerfile excludes for first stage only - if !IsFirstStage { - return false - } - if HasFilepathPrefix(path, buildcontext, false) { +// ExcludesFile returns true if the file context specified this file should be ignored. +// Usually this is specified via .dockerignore +func (c FileContext) ExcludesFile(path string) bool { + if HasFilepathPrefix(path, c.Root, false) { var err error - path, err = filepath.Rel(buildcontext, path) + path, err = filepath.Rel(c.Root, path) if err != nil { logrus.Errorf("unable to get relative path, including %s in build: %v", path, err) return false } } - match, err := fileutils.Matches(path, excluded) + match, err := fileutils.Matches(path, c.ExcludedFiles) if err != nil { logrus.Errorf("error matching, including %s in build: %v", path, err) return false diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 5901c484a..f5d83c91a 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -880,7 +880,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, FileContext{}); err != nil { t.Fatal(err) } if _, err := os.Lstat(dest); err != nil { @@ -966,19 +966,20 @@ func Test_correctDockerignoreFileIsUsed(t *testing.T) { }, } for _, tt := range tests { - if err := GetExcludedFiles(tt.args.dockerfilepath, tt.args.buildcontext); err != nil { + fileContext, err := NewFileContextFromDockerfile(tt.args.dockerfilepath, tt.args.buildcontext) + if 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) { + if !fileContext.ExcludesFile(excl) { 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) { + if fileContext.ExcludesFile(incl) { t.Errorf("'%v' not included", incl) } }) @@ -1004,7 +1005,7 @@ func Test_CopyFile_skips_self(t *testing.T) { t.Fatal(err) } - ignored, err := CopyFile(tempFile, tempFile, "", DoNotChangeUID, DoNotChangeGID) + ignored, err := CopyFile(tempFile, tempFile, FileContext{}, DoNotChangeUID, DoNotChangeGID) if err != nil { t.Fatal(err) }