Skip to content

Commit

Permalink
feat: Added --chmod for ADD and COPY command. Fixes #2850
Browse files Browse the repository at this point in the history
  • Loading branch information
mschneider82 committed Apr 19, 2024
1 parent a0105f6 commit c3500f5
Show file tree
Hide file tree
Showing 8 changed files with 195 additions and 62 deletions.
8 changes: 4 additions & 4 deletions cmd/executor/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd

import (
"fmt"
"io/fs"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -300,7 +301,7 @@ func checkKanikoDir(dir string) error {
if dir != constants.DefaultKanikoPath {

// The destination directory may be across a different partition, so we cannot simply rename/move the directory in this case.
if _, err := util.CopyDir(constants.DefaultKanikoPath, dir, util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID); err != nil {
if _, err := util.CopyDir(constants.DefaultKanikoPath, dir, util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID, fs.FileMode(0o600), true); err != nil {
return err
}

Expand All @@ -321,7 +322,6 @@ func checkContained() bool {

// checkNoDeprecatedFlags return an error if deprecated flags are used.
func checkNoDeprecatedFlags() {

// In version >=2.0.0 make it fail (`Warn` -> `Fatal`)
if opts.CustomPlatformDeprecated != "" {
logrus.Warn("Flag --customPlatform is deprecated. Use: --custom-platform")
Expand Down Expand Up @@ -391,12 +391,12 @@ func resolveEnvironmentBuildArgs(arguments []string, resolver func(string) strin
// copy Dockerfile to /kaniko/Dockerfile so that if it's specified in the .dockerignore
// it won't be copied into the image
func copyDockerfile() error {
if _, err := util.CopyFile(opts.DockerfilePath, config.DockerfilePath, util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID); err != nil {
if _, err := util.CopyFile(opts.DockerfilePath, config.DockerfilePath, util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID, fs.FileMode(0o600), true); err != nil {
return errors.Wrap(err, "copying dockerfile")
}
dockerignorePath := opts.DockerfilePath + ".dockerignore"
if util.FilepathExists(dockerignorePath) {
if _, err := util.CopyFile(dockerignorePath, config.DockerfilePath+".dockerignore", util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID); err != nil {
if _, err := util.CopyFile(dockerignorePath, config.DockerfilePath+".dockerignore", util.FileContext{}, util.DoNotChangeUID, util.DoNotChangeGID, fs.FileMode(0o600), true); err != nil {
return errors.Wrap(err, "copying Dockerfile.dockerignore")
}
}
Expand Down
45 changes: 45 additions & 0 deletions integration/dockerfiles/Dockerfile_test_addcopy_chmod
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
FROM alpine@sha256:5ce5f501c457015c4b91f91a15ac69157d9b06f1a75cf9107bf2b62e0843983a
# Create dev user and group, with id 1001
RUN yes | adduser -u 1001 dev
RUN yes | adduser -u 1002 other

ADD --chmod=0666 context/foo /path/file666
ADD context/foo /path/file

ADD --chmod=777 context/qux /path/dir777
ADD context/qux /path/dir

# ADD tests
RUN test "$(stat -c "%a" /path/file666)" = "666"
RUN test "$(stat -c "%a" /path/file)" = "775"

RUN test "$(stat -c "%a" /path/dir777)" = "777"
RUN ls -la /path/dir777/
RUN test "$(stat -c "%a" /path/dir777/qup)" = "777"
RUN test "$(stat -c "%a" /path/dir777/quw)" = "777"
RUN test "$(stat -c "%a" /path/dir777/quw/que)" = "777"

RUN test "$(stat -c "%a" /path/dir)" = "775"
RUN test "$(stat -c "%a" /path/dir/qup)" = "664"
RUN test "$(stat -c "%a" /path/dir/quw)" = "775"
RUN test "$(stat -c "%a" /path/dir/quw/que)" = "664"

# COPY tests

COPY --chmod=0755 context/foo /path/copyfile755
COPY context/foo /path/copyfile
COPY --chmod=755 context/qux /path/copydir755
COPY context/qux /path/copydir

RUN test "$(stat -c "%a" /path/copyfile755)" = "755"
RUN test "$(stat -c "%a" /path/copyfile)" = "775"

RUN test "$(stat -c "%a" /path/copydir755)" = "755"
RUN test "$(stat -c "%a" /path/copydir755/qup)" = "755"
RUN test "$(stat -c "%a" /path/copydir755/quw)" = "755"
RUN test "$(stat -c "%a" /path/copydir755/quw/que)" = "755"

RUN test "$(stat -c "%a" /path/copydir)" = "775"
RUN test "$(stat -c "%a" /path/copydir/qup)" = "664"
RUN test "$(stat -c "%a" /path/copydir/quw)" = "775"
RUN test "$(stat -c "%a" /path/copydir/quw/que)" = "664"
12 changes: 11 additions & 1 deletion pkg/commands/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package commands

import (
"io/fs"
"path/filepath"

v1 "github.com/google/go-containerregistry/pkg/v1"
Expand Down Expand Up @@ -47,6 +48,14 @@ type AddCommand struct {
func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
replacementEnvs := buildArgs.ReplacementEnvs(config.Env)

chmod, useDefaultChmod, err := util.GetChmod(a.cmd.Chmod, replacementEnvs)
if err != nil {
return errors.Wrap(err, "getting permissions from chmod")
}
if useDefaultChmod {
chmod = fs.FileMode(0o600)
}

uid, gid, err := util.GetUserGroup(a.cmd.Chown, replacementEnvs)
if err != nil {
return errors.Wrap(err, "getting user group from chown")
Expand All @@ -71,7 +80,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, uid, gid); err != nil {
if err := util.DownloadFileToDest(src, urlDest, uid, gid, chmod); err != nil {
return errors.Wrap(err, "downloading remote source file")
}
a.snapshotFiles = append(a.snapshotFiles, urlDest)
Expand Down Expand Up @@ -100,6 +109,7 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
cmd: &instructions.CopyCommand{
SourcesAndDest: instructions.SourcesAndDest{SourcePaths: unresolvedSrcs, DestPath: dest},
Chown: a.cmd.Chown,
Chmod: a.cmd.Chmod,
},
fileContext: a.fileContext,
}
Expand Down
9 changes: 7 additions & 2 deletions pkg/commands/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
return errors.Wrap(err, "resolving src")
}

chmod, useDefaultChmod, err := util.GetChmod(c.cmd.Chmod, replacementEnvs)
if err != nil {
return errors.Wrap(err, "getting permissions from chmod")
}

// For each source, iterate through and copy it over
for _, src := range srcs {
fullPath := filepath.Join(c.fileContext.Root, src)
Expand Down Expand Up @@ -93,7 +98,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
}

if fi.IsDir() {
copiedFiles, err := util.CopyDir(fullPath, destPath, c.fileContext, uid, gid)
copiedFiles, err := util.CopyDir(fullPath, destPath, c.fileContext, uid, gid, chmod, useDefaultChmod)
if err != nil {
return errors.Wrap(err, "copying dir")
}
Expand All @@ -110,7 +115,7 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
c.snapshotFiles = append(c.snapshotFiles, destPath)
} else {
// ... Else, we want to copy over a file
exclude, err := util.CopyFile(fullPath, destPath, c.fileContext, uid, gid)
exclude, err := util.CopyFile(fullPath, destPath, c.fileContext, uid, gid, chmod, useDefaultChmod)
if err != nil {
return errors.Wrap(err, "copying file")
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/util/command_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package util

import (
"fmt"
"io/fs"
"net/url"
"os"
"os/user"
Expand Down Expand Up @@ -370,6 +371,24 @@ func GetUserGroup(chownStr string, env []string) (int64, int64, error) {
return int64(uid32), int64(gid32), nil
}

func GetChmod(chmodStr string, env []string) (chmod fs.FileMode, useDefault bool, err error) {
if chmodStr == "" {
return fs.FileMode(0o600), true, nil
}

chmodStr, err = ResolveEnvironmentReplacement(chmodStr, env, false)
if err != nil {
return 0, false, err
}

mode, err := strconv.ParseUint(chmodStr, 8, 32)
if err != nil {
return 0, false, errors.Wrap(err, "parsing value from chmod")
}
chmod = fs.FileMode(mode)
return
}

// Extract user and group id from a string formatted 'user:group'.
// UserID and GroupID don't need to be present on the system.
func getUIDAndGIDFromString(userGroupString string) (uint32, uint32, error) {
Expand Down
48 changes: 44 additions & 4 deletions pkg/util/command_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package util

import (
"fmt"
"io/fs"
"os/user"
"reflect"
"sort"
Expand Down Expand Up @@ -308,7 +309,8 @@ var updateConfigEnvTests = []struct {
{
Key: "foo",
Value: "baz",
}},
},
},
config: &v1.Config{},
replacementEnvs: []string{},
expectedEnv: []string{"key=var", "foo=baz"},
Expand All @@ -326,7 +328,8 @@ var updateConfigEnvTests = []struct {
{
Key: "foo",
Value: "$argarg",
}},
},
},
config: &v1.Config{},
replacementEnvs: []string{"var=/test/with'chars'/", "not=used", "argarg=\"a\"b\""},
expectedEnv: []string{"key=/var/run", "env=/test/with'chars'/", "foo=\"a\"b\""},
Expand All @@ -340,7 +343,8 @@ var updateConfigEnvTests = []struct {
{
Key: "bob",
Value: "cool",
}},
},
},
config: &v1.Config{Env: []string{"bob=used", "more=test"}},
replacementEnvs: []string{},
expectedEnv: []string{"bob=cool", "more=test", "alice=nice"},
Expand Down Expand Up @@ -585,6 +589,43 @@ func TestGetUserGroup(t *testing.T) {
}
}

func TestGetChmod(t *testing.T) {
tests := []struct {
description string
chmod string
env []string
expected fs.FileMode
shdErr bool
}{
{
description: "non empty chmod",
chmod: "0755",
env: []string{},
expected: fs.FileMode(0o755),
},
{
description: "non empty chmod with env replacement",
chmod: "$foo",
env: []string{"foo=0750"},
expected: fs.FileMode(0o750),
},
{
description: "empty chmod string",
expected: fs.FileMode(0o600),
},
}
for _, tc := range tests {
t.Run(tc.description, func(t *testing.T) {
defaultChmod := fs.FileMode(0o600)
chmod, useDefault, err := GetChmod(tc.chmod, tc.env)
if useDefault {
chmod = defaultChmod
}
testutil.CheckErrorAndDeepEqual(t, tc.shdErr, err, tc.expected, chmod)
})
}
}

func TestResolveEnvironmentReplacementList(t *testing.T) {
type args struct {
values []string
Expand Down Expand Up @@ -806,7 +847,6 @@ func TestLookupUser(t *testing.T) {
testutil.CheckErrorAndDeepEqual(t, tt.wantErr, err, tt.expected, got)
})
}

}

func TestIsSrcRemoteFileURL(t *testing.T) {
Expand Down
Loading

0 comments on commit c3500f5

Please sign in to comment.