Add support to --chown flag to ADD command

integration/dockerfiles-with-context/issue-57/Dockerfile

@@ -0,0 +1,27 @@
+# Copyright 2020 Google, Inc. All rights reserved.
+#
+# 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.
+
+FROM busybox
+
+RUN addgroup -g 1001 -S appgroup \
+  && adduser -u 1005 -S al -G appgroup \
+  && adduser -u 1004 -S bob -G appgroup
+
+# Add local file with and without chown
+ADD --chown=1005:appgroup a.txt /a.txt
+ADD b.txt /b.txt
+
+# Add remote file with and without chown
+ADD --chown=bob:1001 https://raw.githubusercontent.com/GoogleContainerTools/kaniko/master/README.md /r1.txt
+ADD https://raw.githubusercontent.com/GoogleContainerTools/kaniko/master/README.md /r2.txt

pkg/commands/add.go

@@ -47,6 +47,11 @@ type AddCommand struct {
 func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
 
+	uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs)
+	if err != nil {
+		return errors.Wrap(err, "getting user group from chowm")
+	}
+
 	srcs, dest, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.buildcontext, replacementEnvs)
 	if err != nil {
 		return err
@@ -66,7 +71,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 				return err
 			}
 			logrus.Infof("Adding remote URL %s to %s", src, urlDest)
-			if err := util.DownloadFileToDest(src, urlDest); err != nil {
+			if err := util.DownloadFileToDest(src, urlDest, uid, gid); err != nil {
 				return errors.Wrap(err, "downloading remote source file")
 			}
 			a.snapshotFiles = append(a.snapshotFiles, urlDest)
@@ -94,6 +99,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	copyCmd := CopyCommand{
 		cmd: &instructions.CopyCommand{
 			SourcesAndDest: append(unresolvedSrcs, dest),
+			Chown:          a.cmd.Chown,
 		},
 		buildcontext: a.buildcontext,
 	}

pkg/commands/copy.go

@@ -32,11 +32,6 @@ import (
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
-// for testing
-var (
-	getUIDAndGID = util.GetUIDAndGIDFromString
-)
-
 type CopyCommand struct {
 	BaseCommand
 	cmd           *instructions.CopyCommand
@@ -51,8 +46,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	}
 
 	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
-
+	uid, gid, err := util.GetUserGroup(c.cmd.Chown, replacementEnvs)
-	uid, gid, err := getUserGroup(c.cmd.Chown, replacementEnvs)
 	if err != nil {
 		return errors.Wrap(err, "getting user group from chowm")
 	}
@@ -121,21 +115,6 @@ 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

pkg/commands/copy_test.go

@@ -391,58 +391,6 @@ func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
 	}
 }
 
-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)
-		})
-	}
-}
-
 func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 	setupDirs := func(t *testing.T) (string, string) {
 		testDir, err := ioutil.TempDir("", "")

pkg/util/command_util.go

@@ -35,6 +35,11 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// for testing
+var (
+	getUIDAndGID = GetUIDAndGIDFromString
+)
+
 const (
 	pathSeparator = "/"
 )
@@ -335,6 +340,21 @@ Loop:
 	return nil
 }
 
+func GetUserGroup(chownStr string, env []string) (int64, int64, error) {
+	if chownStr == "" {
+		return DoNotChangeUID, DoNotChangeGID, nil
+	}
+	chown, err := 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
+}
+
 // 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.

pkg/util/command_util_test.go

@@ -475,6 +475,58 @@ func Test_RemoteUrls(t *testing.T) {
 
 }
 
+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)
+		})
+	}
+}
+
 func TestResolveEnvironmentReplacementList(t *testing.T) {
 	type args struct {
 		values     []string

pkg/util/fs_util.go

@@ -534,14 +534,13 @@ func AddVolumePathToWhitelist(path string) {
 // 	1. If <src> is a remote file URL:
 // 		- destination will have permissions of 0600
 // 		- If remote file has HTTP Last-Modified header, we set the mtime of the file to that timestamp
-func DownloadFileToDest(rawurl, dest string) error {
+func DownloadFileToDest(rawurl, dest string, uid, gid int64) error {
 	resp, err := http.Get(rawurl)
 	if err != nil {
 		return err
 	}
 	defer resp.Body.Close()
-	// TODO: set uid and gid according to current user
+	if err := CreateFile(dest, resp.Body, 0600, uint32(uid), uint32(gid)); err != nil {
-	if err := CreateFile(dest, resp.Body, 0600, 0, 0); err != nil {
 		return err
 	}
 	mTime := time.Time{}