copy: unit test for chown

Signed-off-by: Yoan Blanc <yoan@dosimple.ch>

go.mod

@@ -46,7 +46,7 @@ require (
 	github.com/opentracing/opentracing-go v1.0.2 // indirect
 	github.com/otiai10/copy v1.0.2
 	github.com/pelletier/go-buffruneio v0.2.0 // indirect
-	github.com/pkg/errors v0.8.1
+	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v0.9.0-pre1.0.20180210140205-a40133b69fbd // indirect
 	github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910 // indirect
 	github.com/prometheus/common v0.0.0-20180518154759-7600349dcfe1 // indirect

go.sum

@@ -291,6 +291,8 @@ github.com/pkg/errors v0.8.0 h1:WdK/asTD0HN+q6hsWO3/vpuAkAr+tw6aNJNDFFf0+qw=
 github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
 github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
+github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
+github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pkg/profile v1.2.1/go.mod h1:hJw3o1OdXxsrSjjVksARp5W95eeEaEfptyVZyv6JUPA=
 github.com/pmezard/go-difflib v0.0.0-20151028094244-d8ed2627bdf0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=

pkg/commands/add.go

@@ -49,7 +49,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 
 	uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs)
 	if err != nil {
-		return errors.Wrap(err, "getting user group from chowm")
+		return errors.Wrap(err, "getting user group from chown")
 	}
 
 	srcs, dest, err := util.ResolveEnvAndWildcards(a.cmd.SourcesAndDest, a.buildcontext, replacementEnvs)

pkg/commands/copy.go

@@ -32,6 +32,11 @@ import (
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
+// for testing
+var (
+	getUserGroup = util.GetUserGroup
+)
+
 type CopyCommand struct {
 	BaseCommand
 	cmd           *instructions.CopyCommand
@@ -46,9 +51,9 @@ 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")
+		return errors.Wrap(err, "getting user group from chown")
 	}
 
 	srcs, dest, err := util.ResolveEnvAndWildcards(c.cmd.SourcesAndDest, c.buildcontext, replacementEnvs)

pkg/commands/copy_test.go

@@ -23,12 +23,14 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"testing"
 
 	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
 	"github.com/GoogleContainerTools/kaniko/testutil"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
 
@@ -539,6 +541,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		testutil.CheckDeepEqual(t, files[0].Name(), "bam.txt")
 
 	})
+
 	t.Run("copy symlink file to a dir", func(t *testing.T) {
 		testDir, srcDir := setupDirs(t)
 		defer os.RemoveAll(testDir)
@@ -573,6 +576,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		}
 		testutil.CheckDeepEqual(t, linkName, "dam.txt")
 	})
+
 	t.Run("copy deadlink symlink file to a dir", func(t *testing.T) {
 		testDir, srcDir := setupDirs(t)
 		defer os.RemoveAll(testDir)
@@ -653,6 +657,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 			testutil.CheckDeepEqual(t, expected[i].Mode(), f.Mode())
 		}
 	})
+
 	t.Run("copy dir with a symlink to a file outside of current src dir", func(t *testing.T) {
 		testDir, srcDir := setupDirs(t)
 		defer os.RemoveAll(testDir)
@@ -705,6 +710,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		}
 		testutil.CheckDeepEqual(t, linkName, targetPath)
 	})
+
 	t.Run("copy src symlink dir to a dir", func(t *testing.T) {
 		testDir, srcDir := setupDirs(t)
 		defer os.RemoveAll(testDir)
@@ -741,6 +747,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 			testutil.CheckDeepEqual(t, expected[i].Mode(), f.Mode())
 		}
 	})
+
 	t.Run("copy src dir to a dest dir which is a symlink", func(t *testing.T) {
 		testDir, srcDir := setupDirs(t)
 		defer os.RemoveAll(testDir)
@@ -789,6 +796,7 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		}
 		testutil.CheckDeepEqual(t, linkName, dest)
 	})
+
 	t.Run("copy src file to a dest dir which is a symlink", func(t *testing.T) {
 		testDir, srcDir := setupDirs(t)
 		defer os.RemoveAll(testDir)
@@ -835,4 +843,83 @@ func TestCopyCommand_ExecuteCommand_Extended(t *testing.T) {
 		}
 		testutil.CheckDeepEqual(t, linkName, dest)
 	})
+
+	t.Run("copy src file to a dest dir with chown", func(t *testing.T) {
+		testDir, srcDir := setupDirs(t)
+		defer os.RemoveAll(testDir)
+
+		original := getUserGroup
+		defer func() { getUserGroup = original }()
+
+		uid := os.Getuid()
+		gid := os.Getgid()
+
+		getUserGroup = func(userStr string, _ []string) (int64, int64, error) {
+			return int64(uid), int64(gid), nil
+		}
+
+		cmd := CopyCommand{
+			cmd: &instructions.CopyCommand{
+				SourcesAndDest: []string{fmt.Sprintf("%s/bam.txt", srcDir), testDir},
+				Chown:          "alice:group",
+			},
+			buildcontext: testDir,
+		}
+
+		cfg := &v1.Config{
+			Cmd:        nil,
+			Env:        []string{},
+			WorkingDir: testDir,
+		}
+
+		err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{}))
+		testutil.CheckNoError(t, err)
+
+		actual, err := ioutil.ReadDir(filepath.Join(testDir))
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		testutil.CheckDeepEqual(t, "bam.txt", actual[0].Name())
+
+		if stat, ok := actual[0].Sys().(*syscall.Stat_t); ok {
+			if int(stat.Uid) != uid {
+				t.Errorf("uid don't match, got %d, expected %d", stat.Uid, uid)
+			}
+			if int(stat.Gid) != gid {
+				t.Errorf("gid don't match, got %d, expected %d", stat.Gid, gid)
+			}
+		}
+	})
+
+	t.Run("copy src file to a dest dir with chown and random user", func(t *testing.T) {
+		testDir, srcDir := setupDirs(t)
+		defer os.RemoveAll(testDir)
+
+		original := getUserGroup
+		defer func() { getUserGroup = original }()
+
+		getUserGroup = func(userStr string, _ []string) (int64, int64, error) {
+			return 12345, 12345, nil
+		}
+
+		cmd := CopyCommand{
+			cmd: &instructions.CopyCommand{
+				SourcesAndDest: []string{fmt.Sprintf("%s/bam.txt", srcDir), testDir},
+				Chown:          "missing:missing",
+			},
+			buildcontext: testDir,
+		}
+
+		cfg := &v1.Config{
+			Cmd:        nil,
+			Env:        []string{},
+			WorkingDir: testDir,
+		}
+
+		err := cmd.ExecuteCommand(cfg, dockerfile.NewBuildArgs([]string{}))
+		if !errors.Is(err, os.ErrPermission) {
+			testutil.CheckNoError(t, err)
+		}
+	})
 }

pkg/util/command_util.go

@@ -344,14 +344,17 @@ 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
 }
 
@@ -370,15 +373,18 @@ func GetUIDAndGIDFromString(userGroupString string, fallbackToUID bool) (uint32,
 	if err != nil {
 		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
 }
 
@@ -422,11 +428,15 @@ func Lookup(userStr string) (*user.User, error) {
 		if _, ok := err.(user.UnknownUserError); !ok {
 			return nil, err
 		}
+
 		// Lookup by id
-		userObj, err = user.LookupId(userStr)
-		if err != nil {
+		u, e := user.LookupId(userStr)
+		if e != nil {
 			return nil, err
 		}
+
+		userObj = u
 	}
+
 	return userObj, nil
 }

testutil/util.go

@@ -70,7 +70,7 @@ func CheckError(t *testing.T, shouldErr bool, err error) {
 
 func CheckNoError(t *testing.T, err error) {
 	if err != nil {
-		t.Error(err)
+		t.Errorf("%+v", err)
 	}
 }