diff --git a/integration/dockerfiles-with-context/issue-57/Dockerfile b/integration/dockerfiles-with-context/issue-57/Dockerfile new file mode 100644 index 000000000..fdcb4377e --- /dev/null +++ b/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 diff --git a/integration/dockerfiles-with-context/issue-57/a.txt b/integration/dockerfiles-with-context/issue-57/a.txt new file mode 100644 index 000000000..e69de29bb diff --git a/integration/dockerfiles-with-context/issue-57/b.txt b/integration/dockerfiles-with-context/issue-57/b.txt new file mode 100644 index 000000000..e69de29bb diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 57b6a08f2..1f740fff2 100644 --- a/pkg/commands/add.go +++ b/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, } diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index fb084e208..a213735bc 100644 --- a/pkg/commands/copy.go +++ b/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 := getUserGroup(c.cmd.Chown, replacementEnvs) + uid, gid, err := util.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 diff --git a/pkg/commands/copy_test.go b/pkg/commands/copy_test.go index b57fda9c5..3e684d8f2 100755 --- a/pkg/commands/copy_test.go +++ b/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("", "") diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index 2880c167d..e55594eab 100644 --- a/pkg/util/command_util.go +++ b/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. diff --git a/pkg/util/command_util_test.go b/pkg/util/command_util_test.go index 472646388..46cb34ca5 100644 --- a/pkg/util/command_util_test.go +++ b/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 diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 457ee886a..8f09b5b7e 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -534,14 +534,13 @@ func AddVolumePathToWhitelist(path string) { // 1. If 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, 0, 0); err != nil { + if err := CreateFile(dest, resp.Body, 0600, uint32(uid), uint32(gid)); err != nil { return err } mTime := time.Time{}