fix: ignore file ownership for copy from context

pkg/commands/add.go

@@ -56,7 +56,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 		chmod = fs.FileMode(0o600)
 	}
 
-	uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs)
+	uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs, "")
 	if err != nil {
 		return errors.Wrap(err, "getting user group from chown")
 	}

pkg/commands/copy.go

@@ -53,7 +53,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	}
 
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
-	uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs)
+	uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs, c.From())
 	logrus.Debugf("found uid %v and gid %v for chown string %v", uid, gid, c.cmd.Chown)
 	if err != nil {
 		return errors.Wrap(err, "getting user group from chown")

pkg/commands/copy_test.go

@@ -1006,7 +1006,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		uid := os.Getuid()
 		gid := os.Getgid()
 
-		getUserGroup = func(userStr string, _ []string) (int64, int64, error) {
+		getUserGroup = func(userStr string, _ []string, _ string) (int64, int64, error) {
 			return int64(uid), int64(gid), nil
 		}
 
@@ -1051,7 +1051,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		original := getUserGroup
 		defer func() { getUserGroup = original }()
 
-		getUserGroup = func(userStr string, _ []string) (int64, int64, error) {
+		getUserGroup = func(userStr string, _ []string, _ string) (int64, int64, error) {
 			return 12345, 12345, nil
 		}
 

pkg/commands/workdir.go

@@ -65,7 +65,7 @@ func (w *WorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile
 
 		if config.User != "" {
 			logrus.Debugf("Fetching uid and gid for USER '%s'", config.User)
-			uid, gid, err = util.GetUserGroup(config.User, replacementEnvs)
+			uid, gid, err = util.GetUserGroup(config.User, replacementEnvs, "")
 			if err != nil {
 				return errors.Wrapf(err, "identifying uid and gid for user %s", config.User)
 			}

pkg/executor/build.go

@@ -86,10 +86,11 @@ type stageBuilder struct {
 	snapshotter      snapShotter
 	layerCache       cache.LayerCache
 	pushLayerToCache cachePusher
+	isCacheProbe     bool
 }
 
 // newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage
-func newStageBuilder(args *dockerfile.BuildArgs, 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) {
+func newStageBuilder(args *dockerfile.BuildArgs, 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, isCacheProbe bool) (*stageBuilder, error) {
 	sourceImage, err := image_util.RetrieveSourceImage(stage, opts)
 	if err != nil {
 		return nil, err
@@ -138,6 +139,7 @@ func newStageBuilder(args *dockerfile.BuildArgs, opts *config.KanikoOptions, sta
 		stageIdxToDigest: sid,
 		layerCache:       newLayerCache(opts),
 		pushLayerToCache: pushLayerToCache,
+		isCacheProbe:     isCacheProbe,
 	}
 
 	for _, cmd := range s.stage.Commands {
@@ -227,11 +229,18 @@ func (s *stageBuilder) populateCompositeKey(command commands.DockerCommand, file
 		}
 	}
 
+	var addPathOptions []AddPathOption
+	if f, ok := command.(interface{ From() string }); ok {
+		if f.From() == "" && s.isCacheProbe {
+			addPathOptions = append(addPathOptions, IgnoreOwnerAndGroup())
+		}
+	}
+
 	// Add the next command to the cache key.
 	compositeKey.AddKey(command.String())
 
 	for _, f := range files {
-		if err := compositeKey.AddPath(f, s.fileContext); err != nil {
+		if err := compositeKey.AddPath(f, s.fileContext, addPathOptions...); err != nil {
 			return compositeKey, err
 		}
 	}
@@ -842,7 +851,8 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) {
 			digestToCacheKey,
 			stageIdxToDigest,
 			stageNameToIdx,
-			fileContext)
+			fileContext,
+			false)
 
 		logrus.Infof("Building stage '%v' [idx: '%v', base-idx: '%v']",
 			stage.BaseName, stage.Index, stage.BaseImageIndex)
@@ -992,7 +1002,8 @@ func DoCacheProbe(opts *config.KanikoOptions) (v1.Image, error) {
 			digestToCacheKey,
 			stageIdxToDigest,
 			stageNameToIdx,
-			fileContext)
+			fileContext,
+			true)
 		if err != nil {
 			return nil, err
 		}

pkg/executor/composite_cache.go

@@ -55,15 +55,31 @@ func (s *CompositeCache) Hash() (string, error) {
 	return util.SHA256(strings.NewReader(s.Key()))
 }
 
-func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
+type addPathOptions struct {
+	ignoreOwnerAndGroup bool
+}
+
+type AddPathOption func(*addPathOptions)
+
+func IgnoreOwnerAndGroup() func(*addPathOptions) {
+	return func(o *addPathOptions) {
+		o.ignoreOwnerAndGroup = true
+	}
+}
+
+func (s *CompositeCache) AddPath(p string, context util.FileContext, fopts ...AddPathOption) error {
+	var opts addPathOptions
+	for _, f := range fopts {
+		f(&opts)
+	}
 	sha := sha256.New()
 	fi, err := filesystem.FS.Lstat(p)
 	if err != nil {
 		return errors.Wrap(err, "could not add path")
 	}
 
 	if fi.Mode().IsDir() {
-		empty, k, err := hashDir(p, context)
+		empty, k, err := hashDir(opts.ignoreOwnerAndGroup, p, context)
 		if err != nil {
 			return err
 		}
@@ -79,7 +95,7 @@ func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
 	if context.ExcludesFile(p) {
 		return nil
 	}
-	fh, err := util.CacheHasher()(p)
+	fh, err := util.CacheHasher(opts.ignoreOwnerAndGroup)(p)
 	if err != nil {
 		return err
 	}
@@ -92,7 +108,7 @@ func (s *CompositeCache) AddPath(p string, context util.FileContext) error {
 }
 
 // HashDir returns a hash of the directory.
-func hashDir(p string, context util.FileContext) (bool, string, error) {
+func hashDir(ignoreOwnerAndGroup bool, p string, context util.FileContext) (bool, string, error) {
 	sha := sha256.New()
 	empty := true
 	if err := filesystem.Walk(p, func(path string, fi os.FileInfo, err error) error {
@@ -104,7 +120,7 @@ func hashDir(p string, context util.FileContext) (bool, string, error) {
 			return nil
 		}
 
-		fileHash, err := util.CacheHasher()(path)
+		fileHash, err := util.CacheHasher(ignoreOwnerAndGroup)(path)
 		if err != nil {
 			return err
 		}

pkg/util/command_util.go

@@ -354,9 +354,17 @@ Loop:
 	return nil
 }
 
-func GetUserGroup(chownStr string, env []string) (int64, int64, error) {
+func GetUserGroup(chownStr string, env []string, from string) (int64, int64, error) {
+	// All files and directories copied from the build context are created with a
+	// UID and GID of 0 unless the optional --chown flag specifies a given
+	// username, groupname, or UID/GID combination to request specific ownership
+	// of the copied content.
 	if chownStr == "" {
-		return DoNotChangeUID, DoNotChangeGID, nil
+		if from == "" {
+			return 0, 0, nil
+		} else {
+			return DoNotChangeUID, DoNotChangeGID, nil
+		}
 	}
 
 	chown, err := ResolveEnvironmentReplacement(chownStr, env, false)

pkg/util/command_util_test.go

@@ -536,6 +536,7 @@ func TestGetUserGroup(t *testing.T) {
 		description  string
 		chown        string
 		env          []string
+		from         string
 		mockIDGetter func(userStr string, groupStr string) (uint32, uint32, error)
 		// needed, in case uid is a valid number, but group is a name
 		mockGroupIDGetter func(groupStr string) (*user.Group, error)
@@ -571,6 +572,16 @@ func TestGetUserGroup(t *testing.T) {
 			mockIDGetter: func(string, string) (uint32, uint32, error) {
 				return 0, 0, fmt.Errorf("should not be called")
 			},
+			expectedU: 0,
+			expectedG: 0,
+		},
+		{
+			description: "empty chown and non-empty from",
+			chown:       "",
+			from:        "foo",
+			mockIDGetter: func(string, string) (uint32, uint32, error) {
+				return 100, 1000, nil
+			},
 			expectedU: -1,
 			expectedG: -1,
 		},
@@ -582,7 +593,7 @@ func TestGetUserGroup(t *testing.T) {
 				getUIDAndGIDFunc = originalIDGetter
 			}()
 			getUIDAndGIDFunc = tc.mockIDGetter
-			uid, gid, err := GetUserGroup(tc.chown, tc.env)
+			uid, gid, err := GetUserGroup(tc.chown, tc.env, tc.from)
 			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, uid, tc.expectedU)
 			testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, gid, tc.expectedG)
 		})

pkg/util/util.go

@@ -24,9 +24,7 @@ import (
 	"io"
 	"math"
 	"os"
-	"path/filepath"
 	"strconv"
-	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -96,7 +94,7 @@ type CacheHasherFileInfoSum interface {
 }
 
 // CacheHasher takes into account everything the regular hasher does except for mtime
-func CacheHasher() func(string) (string, error) {
+func CacheHasher(ignoreOwnerAndGroup bool) func(string) (string, error) {
 	hasher := func(p string) (string, error) {
 		h := md5.New()
 		fi, err := filesystem.FS.Lstat(p)
@@ -114,18 +112,7 @@ func CacheHasher() func(string) (string, error) {
 
 		h.Write([]byte(fi.Mode().String()))
 
-		// Cian: this is a disgusting hack, but it removes the need for the
-		// envbuilder binary to be owned by root when doing a cache probe.
-		// We want to ignore UID and GID changes for the envbuilder binary
-		// specifically. When building and pushing an image using the envbuilder
-		// image, the embedded envbuilder binary will most likely be owned by
-		// root:root. However, when performing a cache probe operation, it is more
-		// likely that the file will be owned by the UID/GID that is running
-		// envbuilder, which in this case is not guaranteed to be root.
-		// Let's just pretend that it is, cross our fingers, and hope for the best.
-		lyingAboutOwnership := !fi.IsDir() &&
-			strings.HasSuffix(filepath.Clean(filepath.Dir(p)), ".envbuilder.tmp")
-		if lyingAboutOwnership {
+		if ignoreOwnerAndGroup {
 			h.Write([]byte(strconv.FormatUint(uint64(0), 36)))
 			h.Write([]byte(","))
 			h.Write([]byte(strconv.FormatUint(uint64(0), 36)))