Merge pull request #1143 from greut/fix-chown

unit tests on copy + chown

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

@@ -287,10 +287,10 @@ github.com/pelletier/go-buffruneio v0.2.0/go.mod h1:JkE26KsDizTr40EUHkXVtNPvgGtb
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/peterbourgon/diskv v2.0.1+incompatible h1:UBdAOUP5p4RWqPBg048CAvpKN+vxiaj6gdUUzhl4XmI=
 github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU=
-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)
+		u, e := user.LookupId(userStr)
-		if err != nil {
+		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)
 	}
 }
 

vendor/github.com/pkg/errors/.travis.yml

@@ -1,15 +1,10 @@
 language: go
 go_import_path: github.com/pkg/errors
 go:
-  - 1.4.x
-  - 1.5.x
-  - 1.6.x
-  - 1.7.x
-  - 1.8.x
-  - 1.9.x
-  - 1.10.x
   - 1.11.x
+  - 1.12.x
+  - 1.13.x
   - tip
 
 script:
-  - go test -v ./...
+  - make check

vendor/github.com/pkg/errors/Makefile

@@ -0,0 +1,44 @@
+PKGS := github.com/pkg/errors
+SRCDIRS := $(shell go list -f '{{.Dir}}' $(PKGS))
+GO := go
+
+check: test vet gofmt misspell unconvert staticcheck ineffassign unparam
+
+test: 
+	$(GO) test $(PKGS)
+
+vet: | test
+	$(GO) vet $(PKGS)
+
+staticcheck:
+	$(GO) get honnef.co/go/tools/cmd/staticcheck
+	staticcheck -checks all $(PKGS)
+
+misspell:
+	$(GO) get github.com/client9/misspell/cmd/misspell
+	misspell \
+		-locale GB \
+		-error \
+		*.md *.go
+
+unconvert:
+	$(GO) get github.com/mdempsky/unconvert
+	unconvert -v $(PKGS)
+
+ineffassign:
+	$(GO) get github.com/gordonklaus/ineffassign
+	find $(SRCDIRS) -name '*.go' | xargs ineffassign
+
+pedantic: check errcheck
+
+unparam:
+	$(GO) get mvdan.cc/unparam
+	unparam ./...
+
+errcheck:
+	$(GO) get github.com/kisielk/errcheck
+	errcheck $(PKGS)
+
+gofmt:  
+	@echo Checking code is gofmted
+	@test -z "$(shell gofmt -s -l -d -e $(SRCDIRS) | tee /dev/stderr)"

vendor/github.com/pkg/errors/README.md

@@ -41,11 +41,18 @@ default:
 
 [Read the package documentation for more information](https://godoc.org/github.com/pkg/errors).
 
+## Roadmap
+
+With the upcoming [Go2 error proposals](https://go.googlesource.com/proposal/+/master/design/go2draft.md) this package is moving into maintenance mode. The roadmap for a 1.0 release is as follows:
+
+- 0.9. Remove pre Go 1.9 and Go 1.10 support, address outstanding pull requests (if possible)
+- 1.0. Final release.
+
 ## Contributing
 
-We welcome pull requests, bug fixes and issue reports. With that said, the bar for adding new symbols to this package is intentionally set high.
+Because of the Go2 errors changes, this package is not accepting proposals for new functionality. With that said, we welcome pull requests, bug fixes and issue reports. 
 
-Before proposing a change, please discuss your change by raising an issue.
+Before sending a PR, please discuss your change by raising an issue.
 
 ## License
 

vendor/github.com/pkg/errors/errors.go

@@ -82,7 +82,7 @@
 //
 //     if err, ok := err.(stackTracer); ok {
 //             for _, f := range err.StackTrace() {
-//                     fmt.Printf("%+s:%d", f)
+//                     fmt.Printf("%+s:%d\n", f, f)
 //             }
 //     }
 //
@@ -159,6 +159,9 @@ type withStack struct {
 
 func (w *withStack) Cause() error { return w.error }
 
+// Unwrap provides compatibility for Go 1.13 error chains.
+func (w *withStack) Unwrap() error { return w.error }
+
 func (w *withStack) Format(s fmt.State, verb rune) {
 	switch verb {
 	case 'v':
@@ -241,6 +244,9 @@ type withMessage struct {
 func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() }
 func (w *withMessage) Cause() error  { return w.cause }
 
+// Unwrap provides compatibility for Go 1.13 error chains.
+func (w *withMessage) Unwrap() error { return w.cause }
+
 func (w *withMessage) Format(s fmt.State, verb rune) {
 	switch verb {
 	case 'v':

vendor/github.com/pkg/errors/go113.go

@@ -0,0 +1,38 @@
+// +build go1.13
+
+package errors
+
+import (
+	stderrors "errors"
+)
+
+// Is reports whether any error in err's chain matches target.
+//
+// The chain consists of err itself followed by the sequence of errors obtained by
+// repeatedly calling Unwrap.
+//
+// An error is considered to match a target if it is equal to that target or if
+// it implements a method Is(error) bool such that Is(target) returns true.
+func Is(err, target error) bool { return stderrors.Is(err, target) }
+
+// As finds the first error in err's chain that matches target, and if so, sets
+// target to that error value and returns true.
+//
+// The chain consists of err itself followed by the sequence of errors obtained by
+// repeatedly calling Unwrap.
+//
+// An error matches target if the error's concrete value is assignable to the value
+// pointed to by target, or if the error has a method As(interface{}) bool such that
+// As(target) returns true. In the latter case, the As method is responsible for
+// setting target.
+//
+// As will panic if target is not a non-nil pointer to either a type that implements
+// error, or to any interface type. As returns false if err is nil.
+func As(err error, target interface{}) bool { return stderrors.As(err, target) }
+
+// Unwrap returns the result of calling the Unwrap method on err, if err's
+// type contains an Unwrap method returning error.
+// Otherwise, Unwrap returns nil.
+func Unwrap(err error) error {
+	return stderrors.Unwrap(err)
+}

vendor/github.com/pkg/errors/stack.go

@@ -5,10 +5,13 @@ import (
 	"io"
 	"path"
 	"runtime"
+	"strconv"
 	"strings"
 )
 
 // Frame represents a program counter inside a stack frame.
+// For historical reasons if Frame is interpreted as a uintptr
+// its value represents the program counter + 1.
 type Frame uintptr
 
 // pc returns the program counter for this frame;
@@ -37,6 +40,15 @@ func (f Frame) line() int {
 	return line
 }
 
+// name returns the name of this function, if known.
+func (f Frame) name() string {
+	fn := runtime.FuncForPC(f.pc())
+	if fn == nil {
+		return "unknown"
+	}
+	return fn.Name()
+}
+
 // Format formats the frame according to the fmt.Formatter interface.
 //
 //    %s    source file
@@ -54,22 +66,16 @@ func (f Frame) Format(s fmt.State, verb rune) {
 	case 's':
 		switch {
 		case s.Flag('+'):
-			pc := f.pc()
+			io.WriteString(s, f.name())
-			fn := runtime.FuncForPC(pc)
+			io.WriteString(s, "\n\t")
-			if fn == nil {
+			io.WriteString(s, f.file())
-				io.WriteString(s, "unknown")
-			} else {
-				file, _ := fn.FileLine(pc)
-				fmt.Fprintf(s, "%s\n\t%s", fn.Name(), file)
-			}
 		default:
 			io.WriteString(s, path.Base(f.file()))
 		}
 	case 'd':
-		fmt.Fprintf(s, "%d", f.line())
+		io.WriteString(s, strconv.Itoa(f.line()))
 	case 'n':
-		name := runtime.FuncForPC(f.pc()).Name()
+		io.WriteString(s, funcname(f.name()))
-		io.WriteString(s, funcname(name))
 	case 'v':
 		f.Format(s, 's')
 		io.WriteString(s, ":")
@@ -77,6 +83,16 @@ func (f Frame) Format(s fmt.State, verb rune) {
 	}
 }
 
+// MarshalText formats a stacktrace Frame as a text string. The output is the
+// same as that of fmt.Sprintf("%+v", f), but without newlines or tabs.
+func (f Frame) MarshalText() ([]byte, error) {
+	name := f.name()
+	if name == "unknown" {
+		return []byte(name), nil
+	}
+	return []byte(fmt.Sprintf("%s %s:%d", name, f.file(), f.line())), nil
+}
+
 // StackTrace is stack of Frames from innermost (newest) to outermost (oldest).
 type StackTrace []Frame
 
@@ -94,18 +110,32 @@ func (st StackTrace) Format(s fmt.State, verb rune) {
 		switch {
 		case s.Flag('+'):
 			for _, f := range st {
-				fmt.Fprintf(s, "\n%+v", f)
+				io.WriteString(s, "\n")
+				f.Format(s, verb)
 			}
 		case s.Flag('#'):
 			fmt.Fprintf(s, "%#v", []Frame(st))
 		default:
-			fmt.Fprintf(s, "%v", []Frame(st))
+			st.formatSlice(s, verb)
 		}
 	case 's':
-		fmt.Fprintf(s, "%s", []Frame(st))
+		st.formatSlice(s, verb)
 	}
 }
 
+// formatSlice will format this StackTrace into the given buffer as a slice of
+// Frame, only valid when called with '%s' or '%v'.
+func (st StackTrace) formatSlice(s fmt.State, verb rune) {
+	io.WriteString(s, "[")
+	for i, f := range st {
+		if i > 0 {
+			io.WriteString(s, " ")
+		}
+		f.Format(s, verb)
+	}
+	io.WriteString(s, "]")
+}
+
 // stack represents a stack of program counters.
 type stack []uintptr
 

vendor/modules.txt

@@ -396,7 +396,7 @@ github.com/otiai10/copy
 github.com/pelletier/go-buffruneio
 # github.com/peterbourgon/diskv v2.0.1+incompatible
 github.com/peterbourgon/diskv
-# github.com/pkg/errors v0.8.1
+# github.com/pkg/errors v0.9.1
 ## explicit
 github.com/pkg/errors
 # github.com/prometheus/client_golang v0.9.0-pre1.0.20180210140205-a40133b69fbd