From 4e8bdb39472058eca72dcf14953d6abbf86a6455 Mon Sep 17 00:00:00 2001 From: Cole Wippern Date: Mon, 20 Jan 2020 09:11:44 -0800 Subject: [PATCH] Fix #940 set modtime when extracting Certain utilities like Apt depend on modtime for certain files. Kaniko was not setting modtime when extracting files and so this broke things like apt. Kaniko now sets the file mod time to the value from the tar header. --- pkg/util/fs_util.go | 13 +++++++++++ pkg/util/fs_util_test.go | 48 +++++++++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 0792e1d66..3df9dd64e 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -215,6 +215,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error { switch hdr.Typeflag { case tar.TypeReg: logrus.Tracef("creating file %s", path) + // It's possible a file is in the tar before its directory, // or a file was copied over a directory prior to now fi, err := os.Stat(dir) @@ -225,6 +226,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error { return err } } + // Check if something already exists at path (symlinks etc.) // If so, delete it if FilepathExists(path) { @@ -232,16 +234,26 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error { return errors.Wrapf(err, "error removing %s to make way for new file.", path) } } + currFile, err := os.Create(path) if err != nil { return err } + if _, err = io.Copy(currFile, tr); err != nil { return err } + if err = setFilePermissions(path, mode, uid, gid); err != nil { return err } + + // We set AccessTime because its a required arg but we only care about + // ModTime. The file will get accessed again so AccessTime will change. + if err := os.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil { + return err + } + currFile.Close() case tar.TypeDir: logrus.Tracef("creating dir %s", path) @@ -647,6 +659,7 @@ func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int) error if err := os.MkdirAll(path, mode); err != nil { return err } + if err := os.Chown(path, uid, gid); err != nil { return err } diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 1ea074f58..f290af101 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -27,6 +27,7 @@ import ( "sort" "strings" "testing" + "time" "github.com/GoogleContainerTools/kaniko/testutil" ) @@ -445,6 +446,19 @@ func fileMatches(p string, c []byte) checker { } } +func timesMatch(p string, fTime time.Time) checker { + return func(root string, t *testing.T) { + fi, err := os.Stat(filepath.Join(root, p)) + if err != nil { + t.Fatalf("error statting file %s", p) + } + + if fi.ModTime().UTC() != fTime.UTC() { + t.Errorf("Expected modtime to equal %v but was %v", fTime, fi.ModTime()) + } + } +} + func permissionsMatch(p string, perms os.FileMode) checker { return func(root string, t *testing.T) { fi, err := os.Stat(filepath.Join(root, p)) @@ -488,14 +502,16 @@ func filesAreHardlinks(first, second string) checker { } } -func fileHeader(name string, contents string, mode int64) *tar.Header { +func fileHeader(name string, contents string, mode int64, fTime time.Time) *tar.Header { return &tar.Header{ - Name: name, - Size: int64(len(contents)), - Mode: mode, - Typeflag: tar.TypeReg, - Uid: os.Getuid(), - Gid: os.Getgid(), + Name: name, + Size: int64(len(contents)), + Mode: mode, + Typeflag: tar.TypeReg, + Uid: os.Getuid(), + Gid: os.Getgid(), + AccessTime: fTime, + ModTime: fTime, } } @@ -537,21 +553,27 @@ func TestExtractFile(t *testing.T) { checkers []checker } + defaultTestTime, err := time.Parse(time.RFC3339, "1912-06-23T00:00:00Z") + if err != nil { + t.Fatal(err) + } + tcs := []tc{ { name: "normal file", contents: []byte("helloworld"), - hdrs: []*tar.Header{fileHeader("./bar", "helloworld", 0644)}, + hdrs: []*tar.Header{fileHeader("./bar", "helloworld", 0644, defaultTestTime)}, checkers: []checker{ fileExists("/bar"), fileMatches("/bar", []byte("helloworld")), permissionsMatch("/bar", 0644), + timesMatch("/bar", defaultTestTime), }, }, { name: "normal file, directory does not exist", contents: []byte("helloworld"), - hdrs: []*tar.Header{fileHeader("./foo/bar", "helloworld", 0644)}, + hdrs: []*tar.Header{fileHeader("./foo/bar", "helloworld", 0644, defaultTestTime)}, checkers: []checker{ fileExists("/foo/bar"), fileMatches("/foo/bar", []byte("helloworld")), @@ -563,7 +585,7 @@ func TestExtractFile(t *testing.T) { name: "normal file, directory is created after", contents: []byte("helloworld"), hdrs: []*tar.Header{ - fileHeader("./foo/bar", "helloworld", 0644), + fileHeader("./foo/bar", "helloworld", 0644, defaultTestTime), dirHeader("./foo", 0722), }, checkers: []checker{ @@ -607,7 +629,7 @@ func TestExtractFile(t *testing.T) { name: "hardlink", tmpdir: "/tmp/hardlink", hdrs: []*tar.Header{ - fileHeader("/bin/gzip", "gzip-binary", 0751), + fileHeader("/bin/gzip", "gzip-binary", 0751, defaultTestTime), hardlinkHeader("/bin/uncompress", "/bin/gzip"), }, checkers: []checker{ @@ -618,7 +640,7 @@ func TestExtractFile(t *testing.T) { { name: "file with setuid bit", contents: []byte("helloworld"), - hdrs: []*tar.Header{fileHeader("./bar", "helloworld", 04644)}, + hdrs: []*tar.Header{fileHeader("./bar", "helloworld", 04644, defaultTestTime)}, checkers: []checker{ fileExists("/bar"), fileMatches("/bar", []byte("helloworld")), @@ -630,7 +652,7 @@ func TestExtractFile(t *testing.T) { contents: []byte("helloworld"), hdrs: []*tar.Header{ dirHeader("./foo", 01755), - fileHeader("./foo/bar", "helloworld", 0644), + fileHeader("./foo/bar", "helloworld", 0644, defaultTestTime), }, checkers: []checker{ fileExists("/foo/bar"),