From 11049fa0bb8e581c74b751dbaba48234c8c711e3 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 15 Sep 2020 17:08:48 -0700 Subject: [PATCH] add as-current-user for fn run --- kyaml/fn/runtime/container/container.go | 34 ++++++--- kyaml/fn/runtime/container/container_test.go | 72 ++++++++++--------- kyaml/fn/runtime/runtimeutil/functiontypes.go | 18 ----- kyaml/runfn/runfn.go | 29 ++++---- kyaml/runfn/runfn_test.go | 10 ++- 5 files changed, 87 insertions(+), 76 deletions(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index e361f2e30..b9468c7b7 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -5,6 +5,7 @@ package container import ( "fmt" + "os/user" runtimeexec "sigs.k8s.io/kustomize/kyaml/fn/runtime/exec" "sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil" @@ -124,6 +125,8 @@ type Filter struct { runtimeutil.ContainerSpec `json:",inline" yaml:",inline"` Exec runtimeexec.Filter + + UIDGID string } func (c Filter) String() string { @@ -167,7 +170,7 @@ func (c *Filter) getCommand() (string, []string) { "--network", string(network), // added security options - "--user", c.User.String(), + "--user", c.UIDGID, "--security-opt=no-new-privileges", // don't allow the user to escalate privileges // note: don't make fs readonly because things like heredoc rely on writing tmp files } @@ -182,13 +185,28 @@ func (c *Filter) getCommand() (string, []string) { return "docker", a } -// NewContainer returns a new container filter -func NewContainer(spec runtimeutil.ContainerSpec) Filter { - f := Filter{ContainerSpec: spec} - // default user is nobody - if f.ContainerSpec.User.IsEmpty() { - f.ContainerSpec.User = runtimeutil.UserNobody +// getUIDGID will return "nobody" if asCurrentUser is false. Otherwise +// return "uid:gid" according to current user who runs the command. +func getUIDGID(asCurrentUser bool) (string, error) { + if !asCurrentUser { + return "nobody", nil } - return f + u, err := user.Current() + if err != nil { + return "", err + } + return fmt.Sprintf("%s:%s", u.Uid, u.Gid), nil +} + +// NewContainer returns a new container filter +func NewContainer(spec runtimeutil.ContainerSpec, asCurrentUser bool) (Filter, error) { + f := Filter{ContainerSpec: spec} + u, err := getUIDGID(asCurrentUser) + if err != nil { + return f, err + } + f.UIDGID = u + + return f, nil } diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index 2cb72f203..903fc92b3 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -6,6 +6,7 @@ package container import ( "bytes" "fmt" + "os/user" "testing" "github.com/stretchr/testify/assert" @@ -15,11 +16,16 @@ import ( ) func TestFilter_setupExec(t *testing.T) { + u, err := user.Current() + if err != nil { + t.Fatal(err) + } var tests = []struct { name string functionConfig string expectedArgs []string - instance Filter + containerSpec runtimeutil.ContainerSpec + asCurrentUser bool }{ { name: "command", @@ -36,12 +42,9 @@ metadata: "--user", "nobody", "--security-opt=no-new-privileges", }, - instance: NewContainer( - runtimeutil.ContainerSpec{ - Image: "example.com:version", - User: "nobody", - }, - ), + containerSpec: runtimeutil.ContainerSpec{ + Image: "example.com:version", + }, }, { @@ -63,9 +66,8 @@ metadata: runtimeutil.ContainerSpec{ Image: "example.com:version", Network: true, - User: "nobody", }, - ), + }, }, { @@ -87,21 +89,18 @@ metadata: "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "volume", "myvol", "/local/"), "--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "tmpfs", "", "/local/"), }, - instance: NewContainer( - runtimeutil.ContainerSpec{ - Image: "example.com:version", - StorageMounts: []runtimeutil.StorageMount{ - {MountType: "bind", Src: "/mount/path", DstPath: "/local/"}, - {MountType: "bind", Src: "/mount/pathrw", DstPath: "/localrw/", ReadWriteMode: true}, - {MountType: "volume", Src: "myvol", DstPath: "/local/"}, - {MountType: "tmpfs", Src: "", DstPath: "/local/"}, - }, - User: "nobody", + containerSpec: runtimeutil.ContainerSpec{ + Image: "example.com:version", + StorageMounts: []runtimeutil.StorageMount{ + {MountType: "bind", Src: "/mount/path", DstPath: "/local/"}, + {MountType: "bind", Src: "/mount/pathrw", DstPath: "/localrw/", ReadWriteMode: true}, + {MountType: "volume", Src: "myvol", DstPath: "/local/"}, + {MountType: "tmpfs", Src: "", DstPath: "/local/"}, }, - ), + }, }, { - name: "root user", + name: "as current user", functionConfig: `apiVersion: apps/v1 kind: Deployment metadata: @@ -112,15 +111,13 @@ metadata: "--rm", "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", "--network", "none", - "--user", "root", + "--user", fmt.Sprintf("%s:%s", u.Uid, u.Gid), "--security-opt=no-new-privileges", }, - instance: NewContainer( - runtimeutil.ContainerSpec{ - Image: "example.com:version", - User: "root", - }, - ), + containerSpec: runtimeutil.ContainerSpec{ + Image: "example.com:version", + }, + asCurrentUser: true, }, } @@ -131,18 +128,23 @@ metadata: if !assert.NoError(t, err) { t.FailNow() } - tt.instance.Exec.FunctionConfig = cfg - tt.instance.Env = append(tt.instance.Env, "KYAML_TEST=FOO") - tt.instance.setupExec() + + instance, err := NewContainer(tt.containerSpec, tt.asCurrentUser) + if err != nil { + t.Fatal(err) + } + instance.Exec.FunctionConfig = cfg + instance.Env = append(instance.Env, "KYAML_TEST=FOO") + instance.setupExec() tt.expectedArgs = append(tt.expectedArgs, - runtimeutil.NewContainerEnvFromStringSlice(tt.instance.Env).GetDockerFlags()...) - tt.expectedArgs = append(tt.expectedArgs, tt.instance.Image) + runtimeutil.NewContainerEnvFromStringSlice(instance.Env).GetDockerFlags()...) + tt.expectedArgs = append(tt.expectedArgs, instance.Image) - if !assert.Equal(t, "docker", tt.instance.Exec.Path) { + if !assert.Equal(t, "docker", instance.Exec.Path) { t.FailNow() } - if !assert.Equal(t, tt.expectedArgs, tt.instance.Exec.Args) { + if !assert.Equal(t, tt.expectedArgs, instance.Exec.Args) { t.FailNow() } }) diff --git a/kyaml/fn/runtime/runtimeutil/functiontypes.go b/kyaml/fn/runtime/runtimeutil/functiontypes.go index 263fbd6fc..83e7ff0ec 100644 --- a/kyaml/fn/runtime/runtimeutil/functiontypes.go +++ b/kyaml/fn/runtime/runtimeutil/functiontypes.go @@ -19,21 +19,6 @@ const ( var functionAnnotationKeys = []string{FunctionAnnotationKey, oldFunctionAnnotationKey} -// ContainerUser is a type for username/uid used in container -type ContainerUser string - -func (u *ContainerUser) String() string { - return string(*u) -} - -func (u *ContainerUser) IsEmpty() bool { - return string(*u) == "" -} - -const ( - UserNobody ContainerUser = "nobody" -) - // ContainerNetworkName is a type for network name used in container type ContainerNetworkName string @@ -171,9 +156,6 @@ type ContainerSpec struct { // Mounts are the storage or directories to mount into the container StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"` - // User is the username/uid that application runs as in continer - User ContainerUser `json:"user,omitempty" yaml:"user,omitempty"` - // Env is a slice of env string that will be exposed to container Env []string `json:"envs,omitempty" yaml:"envs,omitempty"` } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index f6c0dc75a..a7d76dc54 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -85,8 +85,9 @@ type RunFns struct { functionFilterProvider func( filter runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter, error) - // User username used to run the application in container, - User runtimeutil.ContainerUser + // AsCurrentUser is a boolean to indicate whether docker container should use + // the uid and gid that run the command + AsCurrentUser bool // Env contains environment variables that will be exported to container Env []string @@ -299,10 +300,7 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) ( // TODO(eddiezane): Provide error info about which function needs the network return fltrs, errors.Errorf("network required but not enabled with --network") } - // command line username and envs has higher priority - if !r.User.IsEmpty() { - spec.Container.User = r.User - } + // merge envs from imperative and declarative spec.Container.Env = r.mergeContainerEnv(spec.Container.Env) c, err := r.functionFilterProvider(*spec, api) @@ -407,13 +405,18 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter } if !r.DisableContainers && spec.Container.Image != "" { // TODO: Add a test for this behavior - c := container.NewContainer(runtimeutil.ContainerSpec{ - Image: spec.Container.Image, - Network: spec.Container.Network, - StorageMounts: r.StorageMounts, - User: spec.Container.User, - Env: spec.Container.Env, - }) + c, err := container.NewContainer( + runtimeutil.ContainerSpec{ + Image: spec.Container.Image, + Network: spec.Container.Network, + StorageMounts: r.StorageMounts, + Env: spec.Container.Env, + }, + r.AsCurrentUser, + ) + if err != nil { + return nil, err + } cf := &c cf.Exec.FunctionConfig = api cf.Exec.GlobalScope = r.GlobalScope diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index 68d852c2c..937651744 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -60,7 +60,10 @@ kind: return } filter, _ := instance.functionFilterProvider(spec, api) - c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}) + c, err := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}, false) + if err != nil { + t.Fatal(err) + } cf := &c cf.Exec.FunctionConfig = api assert.Equal(t, cf, filter) @@ -91,7 +94,10 @@ kind: return } filter, _ := instance.functionFilterProvider(spec, api) - c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}) + c, err := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}, false) + if err != nil { + t.Fatal(err) + } cf := &c cf.Exec.FunctionConfig = api cf.Exec.GlobalScope = true