From 44bed94f0eb6a35ec95f668592efc6bcddac1e18 Mon Sep 17 00:00:00 2001 From: bjhaid Date: Tue, 9 Oct 2018 12:37:07 +0000 Subject: [PATCH] Opt out of chowning and chmoding from kubectl cp. `kubectl cp` relies on tar to extract the copied file/directory in the container, tar by default attempts to chown/chmod the extracted file after extraction if the user is the "superuser"(root) ``` --same-owner try extracting files with the same ownership as exists in the archive (default for superuser) -p, --preserve-permissions, --same-permissions extract information about file permissions (default for superuser) ``` This fails in environment where the container runs as root but is not granted the OWNER or CHOWN capability. Before this patch below was the behavior of `kubectl cp` ``` kubectl cp README.md foo-67b6fcbd4c-qjlt6:/tmp tar: README.md: Cannot change ownership to uid 1000, gid 1000: Operation not permitted tar: Exiting with failure status due to previous errors command terminated with exit code 2 kubectl exec -it foo-67b6fcbd4c-qjlt6 -- ls -l /tmp/README.md -rw------- 1 1000 1000 3179 Oct 7 22:00 /tmp/README.md ``` After this patch ``` kubectl cp -x a foo-67b6fcbd4c-qjlt6:/ kubectl exec -it foo-67b6fcbd4c-qjlt6 -- ls -l /tmp/README.md -rw-r--r-- 1 root root 3179 Oct 7 22:00 /tmp/README.md ``` --- pkg/kubectl/cmd/cp/BUILD | 1 + pkg/kubectl/cmd/cp/cp.go | 43 +++++++++++++---------- pkg/kubectl/cmd/cp/cp_test.go | 66 ++++++++++++++++++++++++++++++++++- 3 files changed, 90 insertions(+), 20 deletions(-) diff --git a/pkg/kubectl/cmd/cp/BUILD b/pkg/kubectl/cmd/cp/BUILD index e28178b191..feabd7aba3 100644 --- a/pkg/kubectl/cmd/cp/BUILD +++ b/pkg/kubectl/cmd/cp/BUILD @@ -23,6 +23,7 @@ go_test( srcs = ["cp_test.go"], embed = [":go_default_library"], deps = [ + "//pkg/kubectl/cmd/exec:go_default_library", "//pkg/kubectl/cmd/testing:go_default_library", "//pkg/kubectl/scheme:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", diff --git a/pkg/kubectl/cmd/cp/cp.go b/pkg/kubectl/cmd/cp/cp.go index 3a4ea6d331..54f3f336f4 100644 --- a/pkg/kubectl/cmd/cp/cp.go +++ b/pkg/kubectl/cmd/cp/cp.go @@ -67,8 +67,9 @@ var ( ) type CopyOptions struct { - Container string - Namespace string + Container string + Namespace string + NoPreserve bool ClientConfig *restclient.Config Clientset kubernetes.Interface @@ -98,6 +99,7 @@ func NewCmdCp(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.C }, } cmd.Flags().StringVarP(&o.Container, "container", "c", o.Container, "Container name. If omitted, the first container in the pod will be chosen") + cmd.Flags().BoolVarP(&o.NoPreserve, "no-preserve", "", false, "The copied file/directory's ownership and permissions will not be preserved in the container") return cmd } @@ -179,7 +181,7 @@ func (o *CopyOptions) Run(args []string) error { if len(srcSpec.PodName) != 0 && len(destSpec.PodName) != 0 { if _, err := os.Stat(args[0]); err == nil { - return o.copyToPod(fileSpec{File: args[0]}, destSpec) + return o.copyToPod(fileSpec{File: args[0]}, destSpec, &exec.ExecOptions{}) } return fmt.Errorf("src doesn't exist in local filesystem") } @@ -188,7 +190,7 @@ func (o *CopyOptions) Run(args []string) error { return o.copyFromPod(srcSpec, destSpec) } if len(destSpec.PodName) != 0 { - return o.copyToPod(srcSpec, destSpec) + return o.copyToPod(srcSpec, destSpec, &exec.ExecOptions{}) } return fmt.Errorf("one of src or dest must be a remote file specification") } @@ -216,7 +218,7 @@ func (o *CopyOptions) checkDestinationIsDir(dest fileSpec) error { return o.execute(options) } -func (o *CopyOptions) copyToPod(src, dest fileSpec) error { +func (o *CopyOptions) copyToPod(src, dest fileSpec, options *exec.ExecOptions) error { if len(src.File) == 0 || len(dest.File) == 0 { return errFileCannotBeEmpty } @@ -238,30 +240,33 @@ func (o *CopyOptions) copyToPod(src, dest fileSpec) error { err := makeTar(src.File, dest.File, writer) cmdutil.CheckErr(err) }() + var cmdArr []string // TODO: Improve error messages by first testing if 'tar' is present in the container? - cmdArr := []string{"tar", "xf", "-"} + if o.NoPreserve { + cmdArr = []string{"tar", "--no-same-permissions", "--no-same-owner", "-xf", "-"} + } else { + cmdArr = []string{"tar", "-xf", "-"} + } destDir := path.Dir(dest.File) if len(destDir) > 0 { cmdArr = append(cmdArr, "-C", destDir) } - options := &exec.ExecOptions{ - StreamOptions: exec.StreamOptions{ - IOStreams: genericclioptions.IOStreams{ - In: reader, - Out: o.Out, - ErrOut: o.ErrOut, - }, - Stdin: true, - - Namespace: dest.PodNamespace, - PodName: dest.PodName, + options.StreamOptions = exec.StreamOptions{ + IOStreams: genericclioptions.IOStreams{ + In: reader, + Out: o.Out, + ErrOut: o.ErrOut, }, + Stdin: true, - Command: cmdArr, - Executor: &exec.DefaultRemoteExecutor{}, + Namespace: dest.PodNamespace, + PodName: dest.PodName, } + + options.Command = cmdArr + options.Executor = &exec.DefaultRemoteExecutor{} return o.execute(options) } diff --git a/pkg/kubectl/cmd/cp/cp_test.go b/pkg/kubectl/cmd/cp/cp_test.go index 89e56c3165..75f6438a20 100644 --- a/pkg/kubectl/cmd/cp/cp_test.go +++ b/pkg/kubectl/cmd/cp/cp_test.go @@ -26,6 +26,7 @@ import ( "os/exec" "path" "path/filepath" + "reflect" "strings" "testing" @@ -35,6 +36,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest/fake" + kexec "k8s.io/kubernetes/pkg/kubectl/cmd/exec" cmdtesting "k8s.io/kubernetes/pkg/kubectl/cmd/testing" "k8s.io/kubernetes/pkg/kubectl/scheme" ) @@ -573,7 +575,7 @@ func TestCopyToPod(t *testing.T) { } opts.Complete(tf, cmd) t.Run(name, func(t *testing.T) { - err = opts.copyToPod(src, dest) + err = opts.copyToPod(src, dest, &kexec.ExecOptions{}) //If error is NotFound error , it indicates that the //request has been sent correctly. //Treat this as no error. @@ -587,6 +589,68 @@ func TestCopyToPod(t *testing.T) { } } +func TestCopyToPodNoPreserve(t *testing.T) { + tf := cmdtesting.NewTestFactory().WithNamespace("test") + ns := scheme.Codecs + codec := scheme.Codecs.LegacyCodec(scheme.Scheme.PrioritizedVersionsAllGroups()...) + + tf.Client = &fake.RESTClient{ + GroupVersion: schema.GroupVersion{Group: "", Version: "v1"}, + NegotiatedSerializer: ns, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + responsePod := &v1.Pod{} + return &http.Response{StatusCode: http.StatusNotFound, Header: cmdtesting.DefaultHeader(), Body: ioutil.NopCloser(bytes.NewReader([]byte(runtime.EncodeOrDie(codec, responsePod))))}, nil + }), + } + + tf.ClientConfigVal = cmdtesting.DefaultClientConfig() + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() + + cmd := NewCmdCp(tf, ioStreams) + + srcFile, err := ioutil.TempDir("", "test") + if err != nil { + t.Errorf("unexpected error: %v", err) + t.FailNow() + } + defer os.RemoveAll(srcFile) + + tests := map[string]struct { + expectedCmd []string + nopreserve bool + }{ + "copy to pod no preserve user and permissions": { + expectedCmd: []string{"tar", "--no-same-permissions", "--no-same-owner", "-xf", "-", "-C", "."}, + nopreserve: true, + }, + "copy to pod preserve user and permissions": { + expectedCmd: []string{"tar", "-xf", "-", "-C", "."}, + nopreserve: false, + }, + } + opts := NewCopyOptions(ioStreams) + src := fileSpec{ + File: srcFile, + } + dest := fileSpec{ + PodNamespace: "pod-ns", + PodName: "pod-name", + File: "foo", + } + opts.Complete(tf, cmd) + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + options := &kexec.ExecOptions{} + opts.NoPreserve = test.nopreserve + err = opts.copyToPod(src, dest, options) + if !(reflect.DeepEqual(test.expectedCmd, options.Command)) { + t.Errorf("expected cmd: %v, got: %v", test.expectedCmd, options.Command) + } + }) + } +} + func TestValidate(t *testing.T) { tests := []struct { name string