Skip to content

Commit

Permalink
Merge pull request #981 from cvgw/u/cgwippern/fix-issue-940
Browse files Browse the repository at this point in the history
Fix #940 set modtime when extracting
  • Loading branch information
tejal29 authored Jan 21, 2020
2 parents 649a0ed + 4e8bdb3 commit d362359
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 13 deletions.
13 changes: 13 additions & 0 deletions pkg/util/fs_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -225,23 +226,34 @@ 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) {
if err := os.RemoveAll(path); err != nil {
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)
Expand Down Expand Up @@ -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
}
Expand Down
48 changes: 35 additions & 13 deletions pkg/util/fs_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"sort"
"strings"
"testing"
"time"

"github.com/GoogleContainerTools/kaniko/testutil"
)
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -618,21 +634,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")),
Expand All @@ -644,7 +666,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{
Expand Down Expand Up @@ -688,7 +710,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{
Expand All @@ -699,7 +721,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")),
Expand All @@ -711,7 +733,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"),
Expand Down

0 comments on commit d362359

Please sign in to comment.