From ffc372a63bca75e5b7c680c981d2444fc4017897 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Thu, 19 Mar 2020 13:22:24 -0700 Subject: [PATCH 1/5] refactor to add unit tests --- pkg/commands/commands.go | 7 - pkg/commands/copy.go | 8 +- pkg/commands/copy_test.go | 40 +++++- pkg/commands/run.go | 3 +- pkg/config/init.go | 31 +++++ pkg/executor/build.go | 40 +++--- pkg/executor/build_test.go | 6 +- pkg/executor/copy_multistage_test.go | 186 +++++++++++++++++++++++++++ pkg/filesystem/resolve.go | 4 +- pkg/snapshot/snapshot.go | 6 +- pkg/util/command_util.go | 5 +- pkg/util/fs_util.go | 41 +++--- pkg/util/fs_util_test.go | 44 ++++++- pkg/util/tar_util.go | 13 +- 14 files changed, 362 insertions(+), 72 deletions(-) create mode 100644 pkg/config/init.go create mode 100644 pkg/executor/copy_multistage_test.go diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index cc58d51c73..a2ea7788d1 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -17,7 +17,6 @@ limitations under the License. package commands import ( - "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" @@ -25,12 +24,6 @@ import ( "github.com/sirupsen/logrus" ) -var RootDir string - -func init() { - RootDir = constants.RootDir -} - type CurrentCacheKey func() (string, error) type DockerCommand interface { diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 59aa9dde2b..e31efe2f3a 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -22,7 +22,7 @@ import ( "path/filepath" "strings" - "github.com/GoogleContainerTools/kaniko/pkg/constants" + kConfig "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -47,7 +47,7 @@ type CopyCommand struct { func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { // Resolve from if c.cmd.From != "" { - c.buildcontext = filepath.Join(constants.KanikoDir, c.cmd.From) + c.buildcontext = filepath.Join(kConfig.KanikoDir, c.cmd.From) } replacementEnvs := buildArgs.ReplacementEnvs(config.Env) @@ -74,7 +74,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu } cwd := config.WorkingDir if cwd == "" { - cwd = constants.RootDir + cwd = kConfig.RootDir } destPath, err := util.DestinationFilepath(fullPath, dest, cwd) @@ -191,7 +191,7 @@ func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *docke cr.layer = layers[0] cr.readSuccess = true - cr.extractedFiles, err = util.GetFSFromLayers(RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout()) + cr.extractedFiles, err = util.GetFSFromLayers(kConfig.RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout()) logrus.Debugf("extractedFiles: %s", cr.extractedFiles) if err != nil { diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index 0200cd4f7a..dbcacfd114 100755 --- a/pkg/commands/copy_test.go +++ b/pkg/commands/copy_test.go @@ -405,7 +405,6 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { if err := os.MkdirAll(dir, 0777); err != nil { t.Fatal(err) } - file := filepath.Join(dir, "bam.txt") if err := ioutil.WriteFile(file, []byte("meow"), 0777); err != nil { @@ -418,6 +417,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { if err := os.Symlink("dam.txt", filepath.Join(dir, "sym.link")); err != nil { t.Fatal(err) } + return testDir, filepath.Base(dir) } @@ -922,4 +922,42 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) { testutil.CheckNoError(t, err) } }) + + t.Run("copy src dir with relative symlinks in a dir", func(t *testing.T) { + testDir, srcDir := setupDirs(t) + defer os.RemoveAll(testDir) + + // Make another dir inside bar with a relative symlink + dir := filepath.Join(testDir, srcDir, "another") + if err := os.MkdirAll(dir, 0777); err != nil { + t.Fatal(err) + } + os.Symlink("../bam.txt", filepath.Join(dir, "bam_relative.txt")) + + dest := filepath.Join(testDir, "copy") + cmd := CopyCommand{ + cmd: &instructions.CopyCommand{ + SourcesAndDest: []string{srcDir, dest}, + }, + buildcontext: testDir, + } + + cfg := &v1.Config{ + Cmd: nil, + Env: []string{}, + WorkingDir: testDir, + } + err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{})) + testutil.CheckNoError(t, err) + actual, err := ioutil.ReadDir(filepath.Join(dest, "another")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, "bam_relative.txt", actual[0].Name()) + linkName, err := os.Readlink(filepath.Join(dest, "another", "bam_relative.txt")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, "../bam.txt", linkName) + }) } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index b60f185b37..23b7f5fd74 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -24,6 +24,7 @@ import ( "strings" "syscall" + kConfig "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" @@ -202,7 +203,7 @@ func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *docker cr.readSuccess = true cr.extractedFiles, err = util.GetFSFromLayers( - constants.RootDir, + kConfig.RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout(), diff --git a/pkg/config/init.go b/pkg/config/init.go new file mode 100644 index 0000000000..3fca483f7f --- /dev/null +++ b/pkg/config/init.go @@ -0,0 +1,31 @@ +/* +Copyright 2020 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 config + +import ( + "github.com/GoogleContainerTools/kaniko/pkg/constants" +) + +var RootDir string +var KanikoDir string +var WhitelistPath string + +func init() { + RootDir = constants.RootDir + KanikoDir = constants.KanikoDir + WhitelistPath = constants.WhitelistPath +} diff --git a/pkg/executor/build.go b/pkg/executor/build.go index e8d09d6f0d..f497163b89 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -97,7 +97,7 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage, cross return nil, err } l := snapshot.NewLayeredMap(hasher, util.CacheHasher()) - snapshotter := snapshot.NewSnapshotter(l, constants.RootDir) + snapshotter := snapshot.NewSnapshotter(l, config.RootDir) digest, err := sourceImage.Digest() if err != nil { @@ -298,7 +298,7 @@ func (s *stageBuilder) build() error { if shouldUnpack { t := timing.Start("FS Unpacking") - if _, err := util.GetFSFromImage(constants.RootDir, s.image, util.ExtractFile); err != nil { + if _, err := util.GetFSFromImage(config.RootDir, s.image, util.ExtractFile); err != nil { return errors.Wrap(err, "failed to get filesystem from image") } @@ -307,7 +307,7 @@ func (s *stageBuilder) build() error { logrus.Info("Skipping unpacking as no commands require it.") } - if err := util.DetectFilesystemWhitelist(constants.WhitelistPath); err != nil { + if err := util.DetectFilesystemWhitelist(config.WhitelistPath); err != nil { return errors.Wrap(err, "failed to check filesystem whitelist") } @@ -524,7 +524,6 @@ func CalculateDependencies(opts *config.KanikoOptions) (map[int][]string, error) if err != nil { return nil, err } - depGraph[i] = append(depGraph[i], resolved[0:len(resolved)-1]...) } case *instructions.EnvCommand: @@ -629,20 +628,23 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { if err != nil { return nil, err } - dstDir := filepath.Join(constants.KanikoDir, strconv.Itoa(index)) + dstDir := filepath.Join(config.KanikoDir, strconv.Itoa(index)) if err := os.MkdirAll(dstDir, 0644); err != nil { - return nil, err + return nil, errors.Wrap(err, + fmt.Sprintf("to create workspace for stage %s", + stageIdxToDigest[strconv.Itoa(index)], + )) } for _, p := range filesToSave { logrus.Infof("Saving file %s for later use", p) - if err := util.CopyFileOrSymlink(p, dstDir); err != nil { - return nil, err + if err := util.CopyFileOrSymlink(p, dstDir, config.RootDir); err != nil { + return nil, errors.Wrap(err, "could not save file") } } // Delete the filesystem if err := util.DeleteFilesystem(); err != nil { - return nil, err + return nil, errors.Wrap(err, fmt.Sprintf("deleting file system after satge %d", index)) } } @@ -653,17 +655,15 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { // If a file is a symlink, it also returns the target file. func filesToSave(deps []string) ([]string, error) { srcFiles := []string{} - for _, src := range deps { - srcs, err := filepath.Glob(src) - if err != nil { - return nil, err - } - for _, f := range srcs { - if link, err := util.EvalSymLink(f); err == nil { - srcFiles = append(srcFiles, link) - } - srcFiles = append(srcFiles, f) + srcs, err := util.ResolveSources(deps, config.RootDir) + if err != nil { + return nil, errors.Wrap(err, "resolving sources to save") + } + for _, f := range srcs { + if link, err := util.EvalSymLink(f); err == nil { + srcFiles = append(srcFiles, link) } + srcFiles = append(srcFiles, f) } return srcFiles, nil } @@ -717,7 +717,7 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e func extractImageToDependencyDir(name string, image v1.Image) error { t := timing.Start("Extracting Image to Dependency Dir") defer timing.DefaultRun.Stop(t) - dependencyDir := filepath.Join(constants.KanikoDir, name) + dependencyDir := filepath.Join(config.KanikoDir, name) if err := os.MkdirAll(dependencyDir, 0755); err != nil { return err } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 8310671732..20391a3a0d 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -1129,9 +1129,9 @@ COPY %s bar.txt for key, value := range tc.args { sb.args.AddArg(key, &value) } - tmp := commands.RootDir + tmp := config.RootDir if tc.rootDir != "" { - commands.RootDir = tc.rootDir + config.RootDir = tc.rootDir } err := sb.build() if err != nil { @@ -1141,7 +1141,7 @@ COPY %s bar.txt assertCacheKeys(t, tc.expectedCacheKeys, lc.receivedKeys, "receive") assertCacheKeys(t, tc.pushedCacheKeys, keys, "push") - commands.RootDir = tmp + config.RootDir = tmp }) } diff --git a/pkg/executor/copy_multistage_test.go b/pkg/executor/copy_multistage_test.go new file mode 100644 index 0000000000..f7bb66efe0 --- /dev/null +++ b/pkg/executor/copy_multistage_test.go @@ -0,0 +1,186 @@ +/* +Copyright 2020 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 executor + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/GoogleContainerTools/kaniko/pkg/constants" + "github.com/GoogleContainerTools/kaniko/testutil" +) + +func TestCopyCommand_Multistage(t *testing.T) { + t.Run("copy a file across multistage", func(t *testing.T) { + testDir, fn := setupMultistageTests(t) + defer fn() + dockerFile := fmt.Sprintf(` +FROM scratch as first +COPY foo/bam.txt copied/ +ENV test test + +From scratch as second +COPY --from=first copied/bam.txt output/bam.txt`) + ioutil.WriteFile(filepath.Join(testDir, "workspace", "Dockerfile"), []byte(dockerFile), 0755) + opts := &config.KanikoOptions{ + DockerfilePath: filepath.Join(testDir, "workspace", "Dockerfile"), + SrcContext: filepath.Join(testDir, "workspace"), + SnapshotMode: constants.SnapshotModeFull, + } + _, err := DoBuild(opts) + testutil.CheckNoError(t, err) + // Check Image has one layer bam.txt + files, err := ioutil.ReadDir(filepath.Join(testDir, "output")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, 1, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "bam.txt") + + }) + + t.Run("copy a file across multistage into a directory", func(t *testing.T) { + testDir, fn := setupMultistageTests(t) + defer fn() + dockerFile := fmt.Sprintf(` +FROM scratch as first +COPY foo/bam.txt copied/ +ENV test test + +From scratch as second +COPY --from=first copied/bam.txt output/`) + ioutil.WriteFile(filepath.Join(testDir, "workspace", "Dockerfile"), []byte(dockerFile), 0755) + opts := &config.KanikoOptions{ + DockerfilePath: filepath.Join(testDir, "workspace", "Dockerfile"), + SrcContext: filepath.Join(testDir, "workspace"), + SnapshotMode: constants.SnapshotModeFull, + } + _, err := DoBuild(opts) + files, err := ioutil.ReadDir(filepath.Join(testDir, "output")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, 1, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "bam.txt") + }) + t.Run("copy directory across multistage into a directory", func(t *testing.T) { + testDir, fn := setupMultistageTests(t) + defer fn() + dockerFile := fmt.Sprintf(` +FROM scratch as first +COPY foo copied +ENV test test + +From scratch as second +COPY --from=first copied another`) + ioutil.WriteFile(filepath.Join(testDir, "workspace", "Dockerfile"), []byte(dockerFile), 0755) + opts := &config.KanikoOptions{ + DockerfilePath: filepath.Join(testDir, "workspace", "Dockerfile"), + SrcContext: filepath.Join(testDir, "workspace"), + SnapshotMode: constants.SnapshotModeFull, + } + _, err := DoBuild(opts) + testutil.CheckNoError(t, err) + // Check Image has one layer bam.txt + files, err := ioutil.ReadDir(filepath.Join(testDir, "another")) + if err != nil { + t.Fatal(err) + } + testutil.CheckDeepEqual(t, 2, len(files)) + testutil.CheckDeepEqual(t, files[0].Name(), "bam.link") + testutil.CheckDeepEqual(t, files[1].Name(), "bam.txt") + // TODO fix this + // path := filepath.Join(testDir, "output/another", "bam.link") + //linkName, err := os.Readlink(path) + //if err != nil { + // t.Fatal(err) + //} + //testutil.CheckDeepEqual(t, linkName, "bam.txt") + }) + +} + +func setupMultistageTests(t *testing.T) (string, func()) { + testDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + + // Create workspace with files, dirs, and symlinks + // workspace tree: + // /root + // /kaniko + // /workspace + // - /foo + // - bam.txt + // - bam.link -> bam.txt + // - /bin + // - exec.link -> ../exec + // exec + + // Make directory for stage or else the executor will create with permissions 0664 + // and we will run into issue https://github.com/golang/go/issues/22323 + if err := os.MkdirAll(filepath.Join(testDir, "kaniko/0"), 0755); err != nil { + t.Fatal(err) + } + workspace := filepath.Join(testDir, "workspace") + // Make foo + if err := os.MkdirAll(filepath.Join(workspace, "foo"), 0755); err != nil { + t.Fatal(err) + } + file := filepath.Join(workspace, "foo", "bam.txt") + if err := ioutil.WriteFile(file, []byte("meow"), 0755); err != nil { + t.Fatal(err) + } + os.Symlink("bam.txt", filepath.Join(workspace, "foo", "bam.link")) + + // Make a file with contents link + file = filepath.Join(workspace, "exec") + if err := ioutil.WriteFile(file, []byte("woof"), 0755); err != nil { + t.Fatal(err) + } + // Make bin + if err := os.MkdirAll(filepath.Join(workspace, "bin"), 0755); err != nil { + t.Fatal(err) + } + os.Symlink("../exec", filepath.Join(workspace, "bin", "exec.link")) + + // set up config + config.RootDir = testDir + config.KanikoDir = fmt.Sprintf("%s/%s", testDir, "kaniko") + // Write a whitelist path + if err := os.MkdirAll(filepath.Join(testDir, "proc"), 0755); err != nil { + t.Fatal(err) + } + mFile := filepath.Join(testDir, "proc/mountinfo") + mountInfo := fmt.Sprintf( + `36 35 98:0 /kaniko %s/kaniko rw,noatime master:1 - ext3 /dev/root rw,errors=continue +36 35 98:0 /proc %s/proc rw,noatime master:1 - ext3 /dev/root rw,errors=continue +`, testDir, testDir) + if err := ioutil.WriteFile(mFile, []byte(mountInfo), 0644); err != nil { + t.Fatal(err) + } + config.WhitelistPath = mFile + return testDir, func() { + config.KanikoDir = constants.KanikoDir + config.RootDir = constants.RootDir + config.WhitelistPath = constants.WhitelistPath + } +} diff --git a/pkg/filesystem/resolve.go b/pkg/filesystem/resolve.go index a753e8776c..0ea1ac9b6d 100644 --- a/pkg/filesystem/resolve.go +++ b/pkg/filesystem/resolve.go @@ -20,6 +20,7 @@ import ( "os" "path/filepath" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -89,7 +90,6 @@ func ResolvePaths(paths []string, wl []util.WhitelistEntry) (pathsToAdd []string // Also add parent directories to keep the permission of them correctly. pathsToAdd = filesWithParentDirs(pathsToAdd) - return } @@ -130,7 +130,7 @@ func resolveSymlinkAncestor(path string) (string, error) { newPath := filepath.Clean(path) loop: - for newPath != "/" { + for newPath != config.RootDir { fi, err := os.Lstat(newPath) if err != nil { return "", errors.Wrap(err, "resolvePaths: failed to lstat") diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index e64e28dd38..7b3df5661e 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -28,14 +28,14 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/timing" "github.com/karrick/godirwalk" - "github.com/GoogleContainerTools/kaniko/pkg/constants" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/sirupsen/logrus" ) // For testing -var snapshotPathPrefix = constants.KanikoDir +var snapshotPathPrefix = config.KanikoDir // Snapshotter holds the root directory from which to take snapshots, and a list of snapshots taken type Snapshotter struct { @@ -63,7 +63,7 @@ func (s *Snapshotter) Key() (string, error) { // TakeSnapshot takes a snapshot of the specified files, avoiding directories in the whitelist, and creates // a tarball of the changed files. Return contents of the tarball, and whether or not any files were changed func (s *Snapshotter) TakeSnapshot(files []string) (string, error) { - f, err := ioutil.TempFile(snapshotPathPrefix, "") + f, err := ioutil.TempFile(config.KanikoDir, "") if err != nil { return "", err } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 22f8e5683e..67343685c9 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -26,13 +26,14 @@ import ( "strconv" "strings" - "github.com/GoogleContainerTools/kaniko/pkg/constants" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/moby/buildkit/frontend/dockerfile/parser" "github.com/moby/buildkit/frontend/dockerfile/shell" "github.com/pkg/errors" "github.com/sirupsen/logrus" + + "github.com/GoogleContainerTools/kaniko/pkg/config" ) // for testing @@ -145,7 +146,7 @@ func matchSources(srcs, files []string) ([]string, error) { src = filepath.Clean(src) for _, file := range files { if filepath.IsAbs(src) { - file = filepath.Join(constants.RootDir, file) + file = filepath.Join(config.RootDir, file) } matched, err := filepath.Match(src, file) if err != nil { diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 8f09b5b7e4..b19b8e31a0 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -33,7 +33,7 @@ import ( otiai10Cpy "github.com/otiai10/copy" - "github.com/GoogleContainerTools/kaniko/pkg/constants" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/docker/docker/builder/dockerignore" "github.com/docker/docker/pkg/fileutils" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -51,7 +51,7 @@ type WhitelistEntry struct { var initialWhitelist = []WhitelistEntry{ { - Path: "/kaniko", + Path: config.KanikoDir, PrefixMatchOnly: false, }, { @@ -125,7 +125,7 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e return nil, errors.New("must supply an extract function") } - if err := DetectFilesystemWhitelist(constants.WhitelistPath); err != nil { + if err := DetectFilesystemWhitelist(config.WhitelistPath); err != nil { return nil, err } @@ -188,7 +188,7 @@ func GetFSFromLayers(root string, layers []v1.Layer, opts ...FSOpt) ([]string, e // DeleteFilesystem deletes the extracted image file system func DeleteFilesystem() error { logrus.Info("Deleting filesystem...") - return filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, err error) error { + return filepath.Walk(config.RootDir, func(path string, info os.FileInfo, err error) error { if err != nil { // ignore errors when deleting. return nil @@ -209,7 +209,7 @@ func DeleteFilesystem() error { logrus.Debugf("Not deleting %s, as it contains a whitelisted path", path) return nil } - if path == constants.RootDir { + if path == config.RootDir { return nil } return os.RemoveAll(path) @@ -388,7 +388,7 @@ func CheckWhitelist(path string) bool { } func checkWhitelistRoot(root string) bool { - if root == constants.RootDir { + if root == config.RootDir { return false } return CheckWhitelist(root) @@ -423,7 +423,7 @@ func DetectFilesystemWhitelist(path string) error { } continue } - if lineArr[4] != constants.RootDir { + if lineArr[4] != config.RootDir { logrus.Tracef("Appending %s from line: %s", lineArr[4], line) whitelist = append(whitelist, WhitelistEntry{ Path: lineArr[4], @@ -463,16 +463,18 @@ func RelativeFiles(fp string, root string) ([]string, error) { // ParentDirectories returns a list of paths to all parent directories // Ex. /some/temp/dir -> [/, /some, /some/temp, /some/temp/dir] func ParentDirectories(path string) []string { - path = filepath.Clean(path) - dirs := strings.Split(path, "/") - dirPath := constants.RootDir - paths := []string{constants.RootDir} - for index, dir := range dirs { - if dir == "" || index == (len(dirs)-1) { - continue + dir := filepath.Clean(path) + var paths []string + for { + if dir == filepath.Clean(config.RootDir) || dir == "" || dir == "." { + break } - dirPath = filepath.Join(dirPath, dir) - paths = append(paths, dirPath) + dir, _ = filepath.Split(dir) + dir = filepath.Clean(dir) + paths = append(paths, dir) + } + if len(paths) == 0 { + paths = append(paths, config.RootDir) } return paths } @@ -484,7 +486,7 @@ func ParentDirectoriesWithoutLeadingSlash(path string) []string { path = filepath.Clean(path) dirs := strings.Split(path, "/") dirPath := "" - paths := []string{constants.RootDir} + paths := []string{config.RootDir} for index, dir := range dirs { if dir == "" || index == (len(dirs)-1) { continue @@ -824,12 +826,13 @@ func getSymlink(path string) error { // For cross stage dependencies kaniko must persist the referenced path so that it can be used in // the dependent stage. For symlinks we copy the target path because copying the symlink would // result in a dead link -func CopyFileOrSymlink(src string, destDir string) error { +func CopyFileOrSymlink(src string, destDir string, root string) error { destFile := filepath.Join(destDir, src) + src = filepath.Join(root, src) if fi, _ := os.Lstat(src); IsSymlink(fi) { link, err := os.Readlink(src) if err != nil { - return err + return errors.Wrap(err, "copying file or symlink") } if err := createParentDirectory(destFile); err != nil { return err diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 93638b6d0b..b560330b2f 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -30,6 +30,7 @@ import ( "testing" "time" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/mocks/go-containerregistry/mockv1" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/golang/mock/gomock" @@ -157,11 +158,13 @@ func Test_ParentDirectories(t *testing.T) { tests := []struct { name string path string + rootDir string expected []string }{ { - name: "regular path", - path: "/path/to/dir", + name: "regular path", + path: "/path/to/dir", + rootDir: "/", expected: []string{ "/", "/path", @@ -169,17 +172,50 @@ func Test_ParentDirectories(t *testing.T) { }, }, { - name: "current directory", - path: ".", + name: "current directory", + path: ".", + rootDir: "/", expected: []string{ "/", }, }, + { + name: "non / root directory", + path: "/tmp/kaniko/test/another/dir", + rootDir: "/tmp/kaniko/", + expected: []string{ + "/tmp/kaniko", + "/tmp/kaniko/test", + "/tmp/kaniko/test/another", + }, + }, + { + name: "non / root director same path", + path: "/tmp/123", + rootDir: "/tmp/123", + expected: []string{ + "/tmp/123", + }, + }, + { + name: "non / root directory path", + path: "/tmp/120162240/kaniko", + rootDir: "/tmp/120162240", + expected: []string{ + "/tmp/120162240", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + original := config.RootDir + defer func() { config.RootDir = original }() + config.RootDir = tt.rootDir actual := ParentDirectories(tt.path) + sort.Strings(actual) + sort.Strings(tt.expected) + testutil.CheckErrorAndDeepEqual(t, false, nil, tt.expected, actual) }) } diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index 823e42f048..af83925ca1 100644 --- a/pkg/util/tar_util.go +++ b/pkg/util/tar_util.go @@ -28,6 +28,7 @@ import ( "strings" "syscall" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/docker/docker/pkg/archive" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -76,14 +77,14 @@ func (t *Tar) AddFileToTar(p string) error { return err } - if p != "/" { - // Docker uses no leading / in the tarball - hdr.Name = strings.TrimLeft(p, "/") - } else { + if p == config.RootDir { // allow entry for / to preserve permission changes etc. (currently ignored anyway by Docker runtime) - hdr.Name = p + hdr.Name = "/" + } else { + // Docker uses no leading / in the tarball + hdr.Name = strings.TrimPrefix(p, config.RootDir) + hdr.Name = strings.TrimLeft(hdr.Name, "/") } - // rootfs may not have been extracted when using cache, preventing uname/gname from resolving // this makes this layer unnecessarily differ from a cached layer which does contain this information hdr.Uname = "" From caaf6c8adf36acd789a7af066e7f42547050439d Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 24 Mar 2020 10:50:59 -0700 Subject: [PATCH 2/5] fix tests --- pkg/executor/build_test.go | 7 ++++++- pkg/snapshot/snapshot_test.go | 5 ++++- pkg/util/command_util.go | 1 - 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 20391a3a0d..da0b6d44d7 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -371,10 +371,15 @@ func Test_filesToSave(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tmpDir, err := ioutil.TempDir("", "") + original := config.RootDir + config.RootDir = tmpDir if err != nil { t.Errorf("error creating tmpdir: %s", err) } - defer os.RemoveAll(tmpDir) + defer func () { + config.RootDir = original + os.RemoveAll(tmpDir) + }() for _, f := range tt.files { p := filepath.Join(tmpDir, f) diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index 19fc9c4cbc..a6553228d9 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/pkg/snapshot/snapshot_test.go @@ -25,6 +25,7 @@ import ( "strings" "testing" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/pkg/errors" @@ -159,7 +160,6 @@ func TestSnapshotFSChangePermissions(t *testing.T) { if err == io.EOF { break } - t.Logf("Info %s in tar", hdr.Name) foundFiles = append(foundFiles, hdr.Name) if _, isFile := snapshotFiles[hdr.Name]; !isFile { t.Fatalf("File %s unexpectedly in tar", hdr.Name) @@ -458,8 +458,11 @@ func setUpTest() (string, *Snapshotter, func(), error) { return "", nil, nil, errors.Wrap(err, "initializing snapshotter") } + original := config.KanikoDir + config.KanikoDir = testDir cleanup := func() { os.RemoveAll(snapshotPath) + config.KanikoDir = original dirCleanUp() } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 67343685c9..d2a604c1ed 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -131,7 +131,6 @@ func ResolveSources(srcs []string, root string) ([]string, error) { return nil, errors.Wrap(err, "matching sources") } logrus.Debugf("Resolved sources to %v", resolved) - fmt.Println("end of resolve sources") return resolved, nil } From 2ea28fc7f511aae116631eab3cb31f1a2d010b35 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 25 Mar 2020 09:43:59 -0700 Subject: [PATCH 3/5] revert back to previous file to save logic --- pkg/executor/build.go | 26 ++++++++++++++++++-------- pkg/executor/build_test.go | 10 +++------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index f497163b89..ecaba849f5 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -655,15 +655,25 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { // If a file is a symlink, it also returns the target file. func filesToSave(deps []string) ([]string, error) { srcFiles := []string{} - srcs, err := util.ResolveSources(deps, config.RootDir) - if err != nil { - return nil, errors.Wrap(err, "resolving sources to save") - } - for _, f := range srcs { - if link, err := util.EvalSymLink(f); err == nil { - srcFiles = append(srcFiles, link) + for _, src := range deps { + srcs, err := filepath.Glob(filepath.Join(config.RootDir, src)) + if err != nil { + return nil, err + } + for _, f := range srcs { + if link, err := util.EvalSymLink(f); err == nil { + link, err = filepath.Rel(config.RootDir, link) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("could not find relative path to %s", config.RootDir)) + } + srcFiles = append(srcFiles, link) + } + f, err = filepath.Rel(config.RootDir, f) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("could not find relative path to %s", config.RootDir)) + } + srcFiles = append(srcFiles, f) } - srcFiles = append(srcFiles, f) } return srcFiles, nil } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index da0b6d44d7..c49ec2519a 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -376,8 +376,8 @@ func Test_filesToSave(t *testing.T) { if err != nil { t.Errorf("error creating tmpdir: %s", err) } - defer func () { - config.RootDir = original + defer func() { + config.RootDir = original os.RemoveAll(tmpDir) }() @@ -396,11 +396,7 @@ func Test_filesToSave(t *testing.T) { fp.Close() } - args := []string{} - for _, arg := range tt.args { - args = append(args, filepath.Join(tmpDir, arg)) - } - got, err := filesToSave(args) + got, err := filesToSave(tt.args) if err != nil { t.Errorf("got err: %s", err) } From 9567b755dd84f9dec60030bb1f245b1f89f09da0 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 25 Mar 2020 10:09:33 -0700 Subject: [PATCH 4/5] fix unit tests --- pkg/executor/build_test.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index c49ec2519a..e52961e8c9 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -400,14 +400,10 @@ func Test_filesToSave(t *testing.T) { if err != nil { t.Errorf("got err: %s", err) } - want := []string{} - for _, w := range tt.want { - want = append(want, filepath.Join(tmpDir, w)) - } - sort.Strings(want) + sort.Strings(tt.want) sort.Strings(got) - if !reflect.DeepEqual(got, want) { - t.Errorf("filesToSave() = %v, want %v", got, want) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("filesToSave() = %v, want %v", got, tt.want) } }) } From 340ca79fbb0b8ceb32eda63748bcc437fd55e527 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Wed, 25 Mar 2020 11:01:12 -0700 Subject: [PATCH 5/5] lint --- pkg/executor/copy_multistage_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/executor/copy_multistage_test.go b/pkg/executor/copy_multistage_test.go index f7bb66efe0..f28e6f16e3 100644 --- a/pkg/executor/copy_multistage_test.go +++ b/pkg/executor/copy_multistage_test.go @@ -73,6 +73,7 @@ COPY --from=first copied/bam.txt output/`) SnapshotMode: constants.SnapshotModeFull, } _, err := DoBuild(opts) + testutil.CheckNoError(t, err) files, err := ioutil.ReadDir(filepath.Join(testDir, "output")) if err != nil { t.Fatal(err)