Merge remote-tracking branch 'origin/master' into registry-mirror

pkg/commands/cache.go

@@ -0,0 +1,37 @@
+/*
+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 commands
+
+import v1 "github.com/google/go-containerregistry/pkg/v1"
+
+type Cached interface {
+	Layer() v1.Layer
+	ReadSuccess() bool
+}
+
+type caching struct {
+	layer       v1.Layer
+	readSuccess bool
+}
+
+func (c caching) Layer() v1.Layer {
+	return c.layer
+}
+
+func (c caching) ReadSuccess() bool {
+	return c.readSuccess
+}

pkg/commands/cache_test.go

@@ -0,0 +1,36 @@
+/*
+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 commands
+
+import (
+	"testing"
+)
+
+func Test_caching(t *testing.T) {
+	c := caching{layer: fakeLayer{}, readSuccess: true}
+
+	actual := c.Layer().(fakeLayer)
+	expected := fakeLayer{}
+	actualLen, expectedLen := len(actual.TarContent), len(expected.TarContent)
+	if actualLen != expectedLen {
+		t.Errorf("expected layer tar content to be %v but was %v", expectedLen, actualLen)
+	}
+
+	if !c.ReadSuccess() {
+		t.Errorf("expected ReadSuccess to be %v but was %v", true, c.ReadSuccess())
+	}
+}

pkg/commands/copy.go

@@ -32,6 +32,11 @@ import (
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
+// for testing
+var (
+	getUIDAndGID = util.GetUIDAndGIDFromString
+)
+
 type CopyCommand struct {
 	BaseCommand
 	cmd           *instructions.CopyCommand
@@ -47,6 +52,11 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 
+	uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs)
+	if err != nil {
+		return err
+	}
+
 	srcs, dest, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.buildcontext, replacementEnvs)
 	if err != nil {
 		return err
@@ -80,14 +90,14 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 		}
 
 		if fi.IsDir() {
-			copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext)
+			copiedFiles, err := util.CopyDir(fullPath, destPath, c.buildcontext, uid, gid)
 			if err != nil {
 				return err
 			}
 			c.snapshotFiles = append(c.snapshotFiles, copiedFiles...)
 		} else if util.IsSymlink(fi) {
-			// If file is a symlink, we want to create the same relative symlink
-			exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext)
+			// If file is a symlink, we want to copy the target file to destPath
+			exclude, err := util.CopySymlink(fullPath, destPath, c.buildcontext, uid, gid)
 			if err != nil {
 				return err
 			}
@@ -97,7 +107,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)
+			exclude, err := util.CopyFile(fullPath, destPath, c.buildcontext, uid, gid)
 			if err != nil {
 				return err
 			}
@@ -110,6 +120,21 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	return nil
 }
 
+func getUserGroup(chownStr string, env []string) (int64, int64, error) {
+	if chownStr == "" {
+		return util.DoNotChangeUID, util.DoNotChangeGID, nil
+	}
+	chown, err := util.ResolveEnvironmentReplacement(chownStr, env, false)
+	if err != nil {
+		return -1, -1, err
+	}
+	uid32, gid32, err := getUIDAndGID(chown, true)
+	if err != nil {
+		return -1, -1, err
+	}
+	return int64(uid32), int64(gid32), nil
+}
+
 // FilesToSnapshot should return an empty array if still nil; no files were changed
 func (c *CopyCommand) FilesToSnapshot() []string {
 	return c.snapshotFiles
@@ -153,6 +178,7 @@ func (c *CopyCommand) From() string {
 
 type CachingCopyCommand struct {
 	BaseCommand
+	caching
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.CopyCommand
@@ -173,6 +199,13 @@ func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *docke
 		return errors.Wrapf(err, "retrieve image layers")
 	}
 
+	if len(layers) != 1 {
+		return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
+	}
+
+	cr.layer = layers[0]
+	cr.readSuccess = true
+
 	cr.extractedFiles, err = util.GetFSFromLayers(RootDir, layers, util.ExtractFunc(cr.extractFn), util.IncludeWhiteout())
 
 	logrus.Debugf("extractedFiles: %s", cr.extractedFiles)

pkg/commands/copy_test.go

@@ -305,14 +305,14 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
 			c := &CachingCopyCommand{
 				img: fakeImage{},
 			}
-			tc := testCase{
-				desctiption: "with image containing no layers",
-			}
 			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
 				return nil
 			}
-			tc.command = c
-			return tc
+			return testCase{
+				desctiption: "with image containing no layers",
+				expectErr:   true,
+				command:     c,
+			}
 		}(),
 		func() testCase {
 			c := &CachingCopyCommand{
@@ -375,6 +375,67 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
 				}
 			}
 
+			if c.layer == nil && tc.expectLayer {
+				t.Error("expected the command to have a layer set but instead was nil")
+			} else if c.layer != nil && !tc.expectLayer {
+				t.Error("expected the command to have no layer set but instead found a layer")
+			}
+
+			if c.readSuccess != tc.expectLayer {
+				t.Errorf("expected read success to be %v but was %v", tc.expectLayer, c.readSuccess)
+			}
+		})
+	}
+}
+
+func TestGetUserGroup(t *testing.T) {
+	tests := []struct {
+		description string
+		chown       string
+		env         []string
+		mock        func(string, bool) (uint32, uint32, error)
+		expectedU   int64
+		expectedG   int64
+		shdErr      bool
+	}{
+		{
+			description: "non empty chown",
+			chown:       "some:some",
+			env:         []string{},
+			mock:        func(string, bool) (uint32, uint32, error) { return 100, 1000, nil },
+			expectedU:   100,
+			expectedG:   1000,
+		},
+		{
+			description: "non empty chown with env replacement",
+			chown:       "some:$foo",
+			env:         []string{"foo=key"},
+			mock: func(c string, t bool) (uint32, uint32, error) {
+				if c == "some:key" {
+					return 10, 100, nil
+				}
+				return 0, 0, fmt.Errorf("did not resolve environment variable")
+			},
+			expectedU: 10,
+			expectedG: 100,
+		},
+		{
+			description: "empty chown string",
+			mock: func(c string, t bool) (uint32, uint32, error) {
+				return 0, 0, fmt.Errorf("should not be called")
+			},
+			expectedU: -1,
+			expectedG: -1,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.description, func(t *testing.T) {
+			original := getUIDAndGID
+			defer func() { getUIDAndGID = original }()
+			getUIDAndGID = tc.mock
+			uid, gid, err := getUserGroup(tc.chown, tc.env)
+			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, uid, tc.expectedU)
+			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, gid, tc.expectedG)
 		})
 	}
 }

pkg/commands/run.go

@@ -21,7 +21,6 @@ import (
 	"os"
 	"os/exec"
 	"os/user"
-	"strconv"
 	"strings"
 	"syscall"
 
@@ -72,32 +71,10 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	var userStr string
 	// If specified, run the command as a specific user
 	if config.User != "" {
-		userAndGroup := strings.Split(config.User, ":")
-		userStr = userAndGroup[0]
-		var groupStr string
-		if len(userAndGroup) > 1 {
-			groupStr = userAndGroup[1]
-		}
-
-		uidStr, gidStr, err := util.GetUserFromUsername(userStr, groupStr)
+		uid, gid, err := util.GetUIDAndGIDFromString(config.User, false)
 		if err != nil {
 			return err
 		}
-
-		// uid and gid need to be uint32
-		uid64, err := strconv.ParseUint(uidStr, 10, 32)
-		if err != nil {
-			return err
-		}
-		uid := uint32(uid64)
-		var gid uint32
-		if gidStr != "" {
-			gid64, err := strconv.ParseUint(gidStr, 10, 32)
-			if err != nil {
-				return err
-			}
-			gid = uint32(gid64)
-		}
 		cmd.SysProcAttr.Credential = &syscall.Credential{Uid: uid, Gid: gid}
 	}
 	cmd.Env = addDefaultHOME(userStr, replacementEnvs)
@@ -184,6 +161,7 @@ func (r *RunCommand) ShouldCacheOutput() bool {
 
 type CachingRunCommand struct {
 	BaseCommand
+	caching
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.RunCommand
@@ -203,6 +181,13 @@ func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *docker
 		return errors.Wrap(err, "retrieving image layers")
 	}
 
+	if len(layers) != 1 {
+		return errors.New(fmt.Sprintf("expected %d layers but got %d", 1, len(layers)))
+	}
+
+	cr.layer = layers[0]
+	cr.readSuccess = true
+
 	cr.extractedFiles, err = util.GetFSFromLayers(
 		constants.RootDir,
 		layers,

pkg/commands/run_test.go

@@ -238,14 +238,16 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
 			c := &CachingRunCommand{
 				img: fakeImage{},
 			}
-			tc := testCase{
-				desctiption: "with image containing no layers",
-			}
+
 			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
 				return nil
 			}
-			tc.command = c
-			return tc
+
+			return testCase{
+				desctiption: "with image containing no layers",
+				expectErr:   true,
+				command:     c,
+			}
 		}(),
 		func() testCase {
 			c := &CachingRunCommand{
@@ -310,6 +312,16 @@ func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
 					t.Errorf("expected files used from context to be empty but was not")
 				}
 			}
+
+			if c.layer == nil && tc.expectLayer {
+				t.Error("expected the command to have a layer set but instead was nil")
+			} else if c.layer != nil && !tc.expectLayer {
+				t.Error("expected the command to have no layer set but instead found a layer")
+			}
+
+			if c.readSuccess != tc.expectLayer {
+				t.Errorf("expected read success to be %v but was %v", tc.expectLayer, c.readSuccess)
+			}
 		})
 	}
 }

pkg/commands/user.go

@@ -17,15 +17,22 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
 	"strings"
 
 	"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"
 	"github.com/sirupsen/logrus"
 )
 
+// for testing
+var (
+	Lookup = util.Lookup
+)
+
 type UserCommand struct {
 	BaseCommand
 	cmd *instructions.UserCommand
@@ -38,13 +45,13 @@ func (r *UserCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 	userStr, err := util.ResolveEnvironmentReplacement(userAndGroup[0], replacementEnvs, false)
 	if err != nil {
-		return err
+		return errors.Wrap(err, fmt.Sprintf("resolving user %s", userAndGroup[0]))
 	}
-	groupStr := userStr
+	var groupStr = setGroupDefault(userStr)
 	if len(userAndGroup) > 1 {
 		groupStr, err = util.ResolveEnvironmentReplacement(userAndGroup[1], replacementEnvs, false)
 		if err != nil {
-			return err
+			return errors.Wrap(err, fmt.Sprintf("resolving group %s", userAndGroup[1]))
 		}
 	}
 
@@ -57,3 +64,12 @@ func (r *UserCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 func (r *UserCommand) String() string {
 	return r.cmd.String()
 }
+
+func setGroupDefault(userStr string) string {
+	userObj, err := Lookup(userStr)
+	if err != nil {
+		logrus.Debugf("could not lookup user %s. Setting group empty", userStr)
+		return ""
+	}
+	return userObj.Gid
+}

pkg/commands/user_test.go

@@ -16,9 +16,12 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
+	"os/user"
 	"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"
@@ -27,54 +30,70 @@ import (
 
 var userTests = []struct {
 	user        string
+	userObj     *user.User
 	expectedUID string
 	expectedGID string
 }{
 	{
 		user:        "root",
+		userObj:     &user.User{Uid: "root", Gid: "root"},
 		expectedUID: "root:root",
 	},
 	{
 		user:        "root-add",
-		expectedUID: "root-add:root-add",
+		userObj:     &user.User{Uid: "root-add", Gid: "root"},
+		expectedUID: "root-add:root",
 	},
 	{
 		user:        "0",
+		userObj:     &user.User{Uid: "0", Gid: "0"},
 		expectedUID: "0:0",
 	},
 	{
 		user:        "fakeUser",
+		userObj:     &user.User{Uid: "fakeUser", Gid: "fakeUser"},
 		expectedUID: "fakeUser:fakeUser",
 	},
 	{
 		user:        "root:root",
+		userObj:     &user.User{Uid: "root", Gid: "some"},
 		expectedUID: "root:root",
 	},
 	{
 		user:        "0:root",
+		userObj:     &user.User{Uid: "0"},
 		expectedUID: "0:root",
 	},
 	{
 		user:        "root:0",
+		userObj:     &user.User{Uid: "root"},
 		expectedUID: "root:0",
 		expectedGID: "f0",
 	},
 	{
 		user:        "0:0",
+		userObj:     &user.User{Uid: "0"},
 		expectedUID: "0:0",
 	},
 	{
 		user:        "$envuser",
+		userObj:     &user.User{Uid: "root", Gid: "root"},
 		expectedUID: "root:root",
 	},
 	{
 		user:        "root:$envgroup",
+		userObj:     &user.User{Uid: "root"},
 		expectedUID: "root:grp",
 	},
 	{
 		user:        "some:grp",
+		userObj:     &user.User{Uid: "some"},
 		expectedUID: "some:grp",
 	},
+	{
+		user:        "some",
+		expectedUID: "some:",
+	},
 }
 
 func TestUpdateUser(t *testing.T) {
@@ -90,6 +109,13 @@ func TestUpdateUser(t *testing.T) {
 				User: test.user,
 			},
 		}
+		Lookup = func(_ string) (*user.User, error) {
+			if test.userObj != nil {
+				return test.userObj, nil
+			}
+			return nil, fmt.Errorf("error while looking up user")
+		}
+		defer func() { Lookup = util.Lookup }()
 		buildArgs := dockerfile.NewBuildArgs([]string{})
 		err := cmd.ExecuteCommand(cfg, buildArgs)
 		testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedUID, cfg.User)

pkg/constants/constants.go

@@ -17,9 +17,6 @@ limitations under the License.
 package constants
 
 const (
-	// DefaultLogLevel is the default log level
-	DefaultLogLevel = "info"
-
 	// RootDir is the path to the root directory
 	RootDir = "/"
 

pkg/executor/build.go

@@ -326,29 +326,47 @@ func (s *stageBuilder) build() error {
 			continue
 		}
 
-		tarPath, err := s.takeSnapshot(files)
-		if err != nil {
-			return errors.Wrap(err, "failed to take snapshot")
+		fn := func() bool {
+			switch v := command.(type) {
+			case commands.Cached:
+				return v.ReadSuccess()
+			default:
+				return false
+			}
 		}
 
-		logrus.Debugf("build: composite key for command %v %v", command.String(), compositeKey)
-		ck, err := compositeKey.Hash()
-		if err != nil {
-			return errors.Wrap(err, "failed to hash composite key")
-		}
+		if fn() {
+			v := command.(commands.Cached)
+			layer := v.Layer()
+			if err := s.saveLayerToImage(layer, command.String()); err != nil {
+				return err
+			}
+		} else {
+			tarPath, err := s.takeSnapshot(files)
+			if err != nil {
+				return errors.Wrap(err, "failed to take snapshot")
+			}
 
-		logrus.Debugf("build: cache key for command %v %v", command.String(), ck)
+			logrus.Debugf("build: composite key for command %v %v", command.String(), compositeKey)
+			ck, err := compositeKey.Hash()
+			if err != nil {
+				return errors.Wrap(err, "failed to hash composite key")
+			}
 
-		// Push layer to cache (in parallel) now along with new config file
-		if s.opts.Cache && command.ShouldCacheOutput() {
-			cacheGroup.Go(func() error {
-				return s.pushCache(s.opts, ck, tarPath, command.String())
-			})
-		}
-		if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil {
-			return errors.Wrap(err, "failed to save snapshot to image")
+			logrus.Debugf("build: cache key for command %v %v", command.String(), ck)
+
+			// Push layer to cache (in parallel) now along with new config file
+			if s.opts.Cache && command.ShouldCacheOutput() {
+				cacheGroup.Go(func() error {
+					return s.pushCache(s.opts, ck, tarPath, command.String())
+				})
+			}
+			if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil {
+				return errors.Wrap(err, "failed to save snapshot to image")
+			}
 		}
 	}
+
 	if err := cacheGroup.Wait(); err != nil {
 		logrus.Warnf("error uploading layer to cache: %s", err)
 	}
@@ -398,22 +416,40 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool {
 }
 
 func (s *stageBuilder) saveSnapshotToImage(createdBy string, tarPath string) error {
-	if tarPath == "" {
+	layer, err := s.saveSnapshotToLayer(tarPath)
+	if err != nil {
+		return err
+	}
+
+	if layer == nil {
 		return nil
 	}
+
+	return s.saveLayerToImage(layer, createdBy)
+}
+
+func (s *stageBuilder) saveSnapshotToLayer(tarPath string) (v1.Layer, error) {
+	if tarPath == "" {
+		return nil, nil
+	}
 	fi, err := os.Stat(tarPath)
 	if err != nil {
-		return errors.Wrap(err, "tar file path does not exist")
+		return nil, errors.Wrap(err, "tar file path does not exist")
 	}
 	if fi.Size() <= emptyTarSize {
 		logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
-		return nil
+		return nil, nil
 	}
 
 	layer, err := tarball.LayerFromFile(tarPath)
 	if err != nil {
-		return err
+		return nil, err
 	}
+
+	return layer, nil
+}
+func (s *stageBuilder) saveLayerToImage(layer v1.Layer, createdBy string) error {
+	var err error
 	s.image, err = mutate.Append(s.image,
 		mutate.Addendum{
 			Layer: layer,
@@ -575,7 +611,9 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) {
 		}
 		for _, p := range filesToSave {
 			logrus.Infof("Saving file %s for later use", p)
-			util.CopyFileOrSymlink(p, dstDir)
+			if err := util.CopyFileOrSymlink(p, dstDir); err != nil {
+				return nil, err
+			}
 		}
 
 		// Delete the filesystem

pkg/logging/logging.go

@@ -0,0 +1,64 @@
+/*
+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 logging
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+const (
+	// Default log level
+	DefaultLevel = "info"
+
+	// Text format
+	FormatText = "text"
+	// Colored text format
+	FormatColor = "color"
+	// JSON format
+	FormatJSON = "json"
+)
+
+// Configure sets the logrus logging level and formatter
+func Configure(level, format string) error {
+	lvl, err := logrus.ParseLevel(level)
+	if err != nil {
+		return errors.Wrap(err, "parsing log level")
+	}
+	logrus.SetLevel(lvl)
+
+	var formatter logrus.Formatter
+	switch format {
+	case FormatText:
+		formatter = &logrus.TextFormatter{
+			DisableColors: true,
+		}
+	case FormatColor:
+		formatter = &logrus.TextFormatter{
+			ForceColors: true,
+		}
+	case FormatJSON:
+		formatter = &logrus.JSONFormatter{}
+	default:
+		return fmt.Errorf("not a valid log format: %q. Please specify one of (text, color, json)", format)
+	}
+	logrus.SetFormatter(formatter)
+
+	return nil
+}

pkg/snapshot/layered_map.go

@@ -20,11 +20,13 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/timing"
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
+	"github.com/sirupsen/logrus"
 )
 
 type LayeredMap struct {
@@ -113,13 +115,18 @@ func (l *LayeredMap) Add(s string) error {
 // from the current layered map by its hashing function.
 // Returns true if the file is changed.
 func (l *LayeredMap) CheckFileChange(s string) (bool, error) {
-	oldV, ok := l.Get(s)
 	t := timing.Start("Hashing files")
 	defer timing.DefaultRun.Stop(t)
 	newV, err := l.hasher(s)
 	if err != nil {
+		// if this file does not exist in the new layer return.
+		if os.IsNotExist(err) {
+			logrus.Tracef("%s detected as changed but does not exist", s)
+			return false, nil
+		}
 		return false, err
 	}
+	oldV, ok := l.Get(s)
 	if ok && newV == oldV {
 		return false, nil
 	}

pkg/snapshot/snapshot.go

@@ -79,6 +79,8 @@ func (s *Snapshotter) TakeSnapshot(files []string) (string, error) {
 	// Also add parent directories to keep the permission of them correctly.
 	filesToAdd := filesWithParentDirs(files)
 
+	sort.Strings(filesToAdd)
+
 	// Add files to the layered map
 	for _, file := range filesToAdd {
 		if err := s.l.Add(file); err != nil {
@@ -161,7 +163,7 @@ func (s *Snapshotter) scanFullFilesystem() ([]string, []string, error) {
 		dir := filepath.Dir(path)
 		if _, ok := memFs[dir]; ok {
 			if s.l.MaybeAddWhiteout(path) {
-				logrus.Infof("Adding whiteout for %s", path)
+				logrus.Debugf("Adding whiteout for %s", path)
 				filesToWhiteOut = append(filesToWhiteOut, path)
 			}
 		}

pkg/snapshot/snapshot_test.go

@@ -302,6 +302,108 @@ func TestFileWithLinks(t *testing.T) {
 	}
 }
 
+func TestSnasphotPreservesFileOrder(t *testing.T) {
+	newFiles := map[string]string{
+		"foo":     "newbaz1",
+		"bar/bat": "baz",
+		"bar/qux": "quuz",
+		"qux":     "quuz",
+		"corge":   "grault",
+		"garply":  "waldo",
+		"fred":    "plugh",
+		"xyzzy":   "thud",
+	}
+
+	newFileNames := []string{}
+
+	for fileName := range newFiles {
+		newFileNames = append(newFileNames, fileName)
+	}
+
+	filesInTars := [][]string{}
+
+	for i := 0; i <= 2; i++ {
+		testDir, snapshotter, cleanup, err := setUpTest()
+		testDirWithoutLeadingSlash := strings.TrimLeft(testDir, "/")
+		defer cleanup()
+
+		if err != nil {
+			t.Fatal(err)
+		}
+		// Make some changes to the filesystem
+		if err := testutil.SetupFiles(testDir, newFiles); err != nil {
+			t.Fatalf("Error setting up fs: %s", err)
+		}
+
+		filesToSnapshot := []string{}
+		for _, file := range newFileNames {
+			filesToSnapshot = append(filesToSnapshot, filepath.Join(testDir, file))
+		}
+
+		// Take a snapshot
+		tarPath, err := snapshotter.TakeSnapshot(filesToSnapshot)
+
+		if err != nil {
+			t.Fatalf("Error taking snapshot of fs: %s", err)
+		}
+
+		f, err := os.Open(tarPath)
+		if err != nil {
+			t.Fatal(err)
+		}
+		tr := tar.NewReader(f)
+		filesInTars = append(filesInTars, []string{})
+		for {
+			hdr, err := tr.Next()
+			if err == io.EOF {
+				break
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			filesInTars[i] = append(filesInTars[i], strings.TrimPrefix(hdr.Name, testDirWithoutLeadingSlash))
+		}
+	}
+
+	// Check contents of all snapshots, make sure files appear in consistent order
+	for i := 1; i < len(filesInTars); i++ {
+		testutil.CheckErrorAndDeepEqual(t, false, nil, filesInTars[0], filesInTars[i])
+	}
+}
+
+func TestSnapshotOmitsUnameGname(t *testing.T) {
+	_, snapshotter, cleanup, err := setUpTest()
+
+	defer cleanup()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	tarPath, err := snapshotter.TakeSnapshotFS()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	f, err := os.Open(tarPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	tr := tar.NewReader(f)
+	for {
+		hdr, err := tr.Next()
+		if err == io.EOF {
+			break
+		}
+		if err != nil {
+			t.Fatal(err)
+		}
+		if hdr.Uname != "" || hdr.Gname != "" {
+			t.Fatalf("Expected Uname/Gname for %s to be empty: Uname = '%s', Gname = '%s'", hdr.Name, hdr.Uname, hdr.Gname)
+		}
+	}
+
+}
+
 func setupSymlink(dir string, link string, target string) error {
 	return os.Symlink(target, filepath.Join(dir, link))
 }

pkg/util/command_util.go

@@ -23,6 +23,7 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
+	"strconv"
 	"strings"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/constants"
@@ -326,18 +327,38 @@ Loop:
 	return nil
 }
 
-func GetUserFromUsername(userStr string, groupStr string) (string, string, error) {
-	// Lookup by username
-	userObj, err := user.Lookup(userStr)
+// Extract user and group id from a string formatted 'user:group'.
+// If fallbackToUID is set, the gid is equal to uid if the group is not specified
+// otherwise gid is set to zero.
+func GetUIDAndGIDFromString(userGroupString string, fallbackToUID bool) (uint32, uint32, error) {
+	userAndGroup := strings.Split(userGroupString, ":")
+	userStr := userAndGroup[0]
+	var groupStr string
+	if len(userAndGroup) > 1 {
+		groupStr = userAndGroup[1]
+	}
+
+	uidStr, gidStr, err := GetUserFromUsername(userStr, groupStr, fallbackToUID)
 	if err != nil {
-		if _, ok := err.(user.UnknownUserError); !ok {
-			return "", "", err
-		}
-		// Lookup by id
-		userObj, err = user.LookupId(userStr)
-		if err != nil {
-			return "", "", err
-		}
+		return 0, 0, err
+	}
+	// uid and gid need to be fit into uint32
+	uid64, err := strconv.ParseUint(uidStr, 10, 32)
+	if err != nil {
+		return 0, 0, err
+	}
+	gid64, err := strconv.ParseUint(gidStr, 10, 32)
+	if err != nil {
+		return 0, 0, err
+	}
+	return uint32(uid64), uint32(gid64), nil
+}
+
+func GetUserFromUsername(userStr string, groupStr string, fallbackToUID bool) (string, string, error) {
+	// Lookup by username
+	userObj, err := Lookup(userStr)
+	if err != nil {
+		return "", "", err
 	}
 
 	// Same dance with groups
@@ -356,10 +377,28 @@ func GetUserFromUsername(userStr string, groupStr string) (string, string, error
 	}
 
 	uid := userObj.Uid
-	gid := ""
+	gid := "0"
+	if fallbackToUID {
+		gid = userObj.Gid
+	}
 	if group != nil {
 		gid = group.Gid
 	}
 
 	return uid, gid, nil
 }
+
+func Lookup(userStr string) (*user.User, error) {
+	userObj, err := user.Lookup(userStr)
+	if err != nil {
+		if _, ok := err.(user.UnknownUserError); !ok {
+			return nil, err
+		}
+		// Lookup by id
+		userObj, err = user.LookupId(userStr)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return userObj, nil
+}

pkg/util/command_util_test.go

@@ -17,8 +17,11 @@ limitations under the License.
 package util
 
 import (
+	"fmt"
+	"os/user"
 	"reflect"
 	"sort"
+	"strconv"
 	"testing"
 
 	"github.com/GoogleContainerTools/kaniko/testutil"
@@ -526,3 +529,35 @@ func TestResolveEnvironmentReplacementList(t *testing.T) {
 		})
 	}
 }
+
+func Test_GetUIDAndGIDFromString(t *testing.T) {
+	currentUser, err := user.Current()
+	if err != nil {
+		t.Fatalf("Cannot get current user: %s", err)
+	}
+	groups, err := currentUser.GroupIds()
+	if err != nil || len(groups) == 0 {
+		t.Fatalf("Cannot get groups for current user: %s", err)
+	}
+	primaryGroupObj, err := user.LookupGroupId(groups[0])
+	if err != nil {
+		t.Fatalf("Could not lookup name of group %s: %s", groups[0], err)
+	}
+	primaryGroup := primaryGroupObj.Name
+
+	testCases := []string{
+		fmt.Sprintf("%s:%s", currentUser.Uid, currentUser.Gid),
+		fmt.Sprintf("%s:%s", currentUser.Username, currentUser.Gid),
+		fmt.Sprintf("%s:%s", currentUser.Uid, primaryGroup),
+		fmt.Sprintf("%s:%s", currentUser.Username, primaryGroup),
+	}
+	expectedU, _ := strconv.ParseUint(currentUser.Uid, 10, 32)
+	expectedG, _ := strconv.ParseUint(currentUser.Gid, 10, 32)
+	for _, tt := range testCases {
+		uid, gid, err := GetUIDAndGIDFromString(tt, false)
+		if uid != uint32(expectedU) || gid != uint32(expectedG) || err != nil {
+			t.Errorf("Could not correctly decode %s to uid/gid %d:%d. Result: %d:%d", tt, expectedU, expectedG,
+				uid, gid)
+		}
+	}
+}

pkg/util/fs_util.go

@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"math"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -40,6 +41,9 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+const DoNotChangeUID = -1
+const DoNotChangeGID = -1
+
 type WhitelistEntry struct {
 	Path            string
 	PrefixMatchOnly bool
@@ -56,6 +60,12 @@ var initialWhitelist = []WhitelistEntry{
 		Path:            "/etc/mtab",
 		PrefixMatchOnly: false,
 	},
+	{
+		// we whitelist /tmp/apt-key-gpghome, since the apt keys are added temporarily in this directory.
+		// from the base image
+		Path:            "/tmp/apt-key-gpghome",
+		PrefixMatchOnly: true,
+	},
 }
 
 var whitelist = initialWhitelist
@@ -297,7 +307,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
 		currFile.Close()
 	case tar.TypeDir:
 		logrus.Tracef("creating dir %s", path)
-		if err := mkdirAllWithPermissions(path, mode, uid, gid); err != nil {
+		if err := mkdirAllWithPermissions(path, mode, int64(uid), int64(gid)); err != nil {
 			return err
 		}
 
@@ -532,9 +542,21 @@ func DownloadFileToDest(rawurl, dest string) error {
 	return os.Chtimes(dest, mTime, mTime)
 }
 
+// DetermineTargetFileOwnership returns the user provided uid/gid combination.
+// If they are set to -1, the uid/gid from the original file is used.
+func DetermineTargetFileOwnership(fi os.FileInfo, uid, gid int64) (int64, int64) {
+	if uid <= DoNotChangeUID {
+		uid = int64(fi.Sys().(*syscall.Stat_t).Uid)
+	}
+	if gid <= DoNotChangeGID {
+		gid = int64(fi.Sys().(*syscall.Stat_t).Gid)
+	}
+	return uid, gid
+}
+
 // 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) ([]string, error) {
+func CopyDir(src, dest, buildcontext string, uid, gid int64) ([]string, error) {
 	files, err := RelativeFiles("", src)
 	if err != nil {
 		return nil, err
@@ -556,20 +578,18 @@ func CopyDir(src, dest, buildcontext string) ([]string, error) {
 			logrus.Tracef("Creating directory %s", destPath)
 
 			mode := fi.Mode()
-			uid := int(fi.Sys().(*syscall.Stat_t).Uid)
-			gid := int(fi.Sys().(*syscall.Stat_t).Gid)
-
+			uid, gid = DetermineTargetFileOwnership(fi, uid, gid)
 			if err := mkdirAllWithPermissions(destPath, mode, uid, gid); err != nil {
 				return nil, err
 			}
 		} 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, buildcontext, uid, gid); err != nil {
 				return nil, err
 			}
 		} else {
 			// ... Else, we want to copy over a file
-			if _, err := CopyFile(fullPath, destPath, buildcontext); err != nil {
+			if _, err := CopyFile(fullPath, destPath, buildcontext, uid, gid); err != nil {
 				return nil, err
 			}
 		}
@@ -579,12 +599,12 @@ func CopyDir(src, dest, buildcontext string) ([]string, error) {
 }
 
 // CopySymlink copies the symlink at src to dest
-func CopySymlink(src, dest, buildcontext string) (bool, error) {
+func CopySymlink(src, dest, buildcontext string, uid int64, gid int64) (bool, error) {
 	if ExcludeFile(src, buildcontext) {
 		logrus.Debugf("%s found in .dockerignore, ignoring", src)
 		return true, nil
 	}
-	link, err := os.Readlink(src)
+	link, err := filepath.EvalSymlinks(src)
 	if err != nil {
 		return false, err
 	}
@@ -596,11 +616,11 @@ func CopySymlink(src, dest, buildcontext string) (bool, error) {
 	if err := createParentDirectory(dest); err != nil {
 		return false, err
 	}
-	return false, os.Symlink(link, dest)
+	return CopyFile(link, dest, buildcontext, uid, gid)
 }
 
 // CopyFile copies the file at src to dest
-func CopyFile(src, dest, buildcontext string) (bool, error) {
+func CopyFile(src, dest, buildcontext string, uid, gid int64) (bool, error) {
 	if ExcludeFile(src, buildcontext) {
 		logrus.Debugf("%s found in .dockerignore, ignoring", src)
 		return true, nil
@@ -621,9 +641,8 @@ func CopyFile(src, dest, buildcontext string) (bool, error) {
 		return false, err
 	}
 	defer srcFile.Close()
-	uid := fi.Sys().(*syscall.Stat_t).Uid
-	gid := fi.Sys().(*syscall.Stat_t).Gid
-	return false, CreateFile(dest, srcFile, fi.Mode(), uid, gid)
+	uid, gid = DetermineTargetFileOwnership(fi, uid, gid)
+	return false, CreateFile(dest, srcFile, fi.Mode(), uint32(uid), uint32(gid))
 }
 
 // GetExcludedFiles gets a list of files to exclude from the .dockerignore
@@ -663,7 +682,7 @@ func ExcludeFile(path, buildcontext string) bool {
 	return match
 }
 
-// HasFilepathPrefix checks  if the given file path begins with prefix
+// HasFilepathPrefix checks if the given file path begins with prefix
 func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
 	prefix = filepath.Clean(prefix)
 	prefixArray := strings.Split(prefix, "/")
@@ -676,11 +695,15 @@ func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
 	if prefixMatchOnly && len(pathArray) == len(prefixArray) {
 		return false
 	}
+
 	for index := range prefixArray {
-		if prefixArray[index] == pathArray[index] {
-			continue
+		m, err := filepath.Match(prefixArray[index], pathArray[index])
+		if err != nil {
+			return false
+		}
+		if !m {
+			return false
 		}
-		return false
 	}
 	return true
 }
@@ -689,12 +712,15 @@ func Volumes() []string {
 	return volumes
 }
 
-func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int) error {
+func mkdirAllWithPermissions(path string, mode os.FileMode, uid, gid int64) error {
 	if err := os.MkdirAll(path, mode); err != nil {
 		return err
 	}
-
-	if err := os.Chown(path, uid, gid); err != nil {
+	if uid > math.MaxUint32 || gid > math.MaxUint32 {
+		// due to https://github.com/golang/go/issues/8537
+		return errors.New(fmt.Sprintf("Numeric User-ID or Group-ID greater than %v are not properly supported.", math.MaxUint32))
+	}
+	if err := os.Chown(path, int(uid), int(gid)); err != nil {
 		return err
 	}
 	// In some cases, MkdirAll doesn't change the permissions, so run Chmod
@@ -763,11 +789,15 @@ func getSymlink(path string) error {
 func CopyFileOrSymlink(src string, destDir string) error {
 	destFile := filepath.Join(destDir, src)
 	if fi, _ := os.Lstat(src); IsSymlink(fi) {
-		link, err := os.Readlink(src)
+		link, err := EvalSymLink(src)
 		if err != nil {
 			return err
 		}
-		return os.Symlink(link, destFile)
+		linkPath := filepath.Join(destDir, link)
+		if err := createParentDirectory(destFile); err != nil {
+			return err
+		}
+		return os.Symlink(linkPath, destFile)
 	}
 	return otiai10Cpy.Copy(src, destFile)
 }
@@ -791,7 +821,8 @@ func UpdateWhitelist(whitelistVarRun bool) {
 	if !whitelistVarRun {
 		return
 	}
-	whitelist = append(initialWhitelist, WhitelistEntry{
+	logrus.Trace("Adding /var/run to initialWhitelist ")
+	initialWhitelist = append(initialWhitelist, WhitelistEntry{
 		// /var/run is a special case. It's common to mount in /var/run/docker.sock or something similar
 		// which leads to a special mount on the /var/run/docker.sock file itself, but the directory to exist
 		// in the image with no way to tell if it came from the base image or not.

pkg/util/fs_util_test.go

@@ -65,6 +65,7 @@ func Test_DetectFilesystemWhitelist(t *testing.T) {
 		{"/dev/pts", false},
 		{"/sys", false},
 		{"/etc/mtab", false},
+		{"/tmp/apt-key-gpghome", true},
 	}
 	actualWhitelist := whitelist
 	sort.Slice(actualWhitelist, func(i, j int) bool {
@@ -258,6 +259,14 @@ func Test_CheckWhitelist(t *testing.T) {
 			},
 			want: false,
 		},
+		{
+			name: "prefix match only ",
+			args: args{
+				path:      "/tmp/apt-key-gpghome.xft/gpg.key",
+				whitelist: []WhitelistEntry{{"/tmp/apt-key-gpghome.*", true}},
+			},
+			want: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -826,16 +835,12 @@ 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, "", DoNotChangeUID, DoNotChangeGID); err != nil {
 				t.Fatal(err)
 			}
-			got, err := os.Readlink(dest)
-			if err != nil {
+			if _, err := os.Lstat(dest); err != nil {
 				t.Fatalf("error reading link %s: %s", link, err)
 			}
-			if got != tc.linkTarget {
-				t.Errorf("link target does not match: %s != %s", got, tc.linkTarget)
-			}
 		})
 	}
 }
@@ -954,7 +959,7 @@ func Test_CopyFile_skips_self(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	ignored, err := CopyFile(tempFile, tempFile, "")
+	ignored, err := CopyFile(tempFile, tempFile, "", DoNotChangeUID, DoNotChangeGID)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -1270,6 +1275,10 @@ func TestUpdateWhitelist(t *testing.T) {
 					Path:            "/var/run",
 					PrefixMatchOnly: false,
 				},
+				{
+					Path:            "/tmp/apt-key-gpghome",
+					PrefixMatchOnly: true,
+				},
 			},
 		},
 		{
@@ -1283,15 +1292,25 @@ func TestUpdateWhitelist(t *testing.T) {
 					Path:            "/etc/mtab",
 					PrefixMatchOnly: false,
 				},
+				{
+					Path:            "/tmp/apt-key-gpghome",
+					PrefixMatchOnly: true,
+				},
 			},
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			whitelist = initialWhitelist
-			defer func() { whitelist = initialWhitelist }()
+			original := initialWhitelist
+			defer func() { initialWhitelist = original }()
 			UpdateWhitelist(tt.whitelistVarRun)
-			testutil.CheckDeepEqual(t, tt.expected, whitelist)
+			sort.Slice(tt.expected, func(i, j int) bool {
+				return tt.expected[i].Path < tt.expected[j].Path
+			})
+			sort.Slice(initialWhitelist, func(i, j int) bool {
+				return initialWhitelist[i].Path < initialWhitelist[j].Path
+			})
+			testutil.CheckDeepEqual(t, tt.expected, initialWhitelist)
 		})
 	}
 }

pkg/util/tar_util.go

@@ -84,6 +84,11 @@ func (t *Tar) AddFileToTar(p string) error {
 		hdr.Name = p
 	}
 
+	// 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 = ""
+	hdr.Gname = ""
+
 	hardlink, linkDst := t.checkHardlink(p, i)
 	if hardlink {
 		hdr.Linkname = linkDst

pkg/util/util.go

@@ -28,25 +28,10 @@ import (
 	"syscall"
 
 	"github.com/minio/highwayhash"
-	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
-// ConfigureLogging sets the logrus logging level and forces logs to be colorful (!)
-func ConfigureLogging(logLevel string) error {
-	lvl, err := logrus.ParseLevel(logLevel)
-	if err != nil {
-		return errors.Wrap(err, "parsing log level")
-	}
-	logrus.SetLevel(lvl)
-	logrus.SetFormatter(&logrus.TextFormatter{
-		ForceColors: true,
-	})
-	return nil
-}
-
 // Hasher returns a hash function, used in snapshotting to determine if a file has changed
 func Hasher() func(string) (string, error) {
 	pool := sync.Pool{