Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add as-current-user for fn run #2992

Merged
merged 3 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions kyaml/fn/runtime/container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ type Filter struct {
runtimeutil.ContainerSpec `json:",inline" yaml:",inline"`

Exec runtimeexec.Filter

UIDGID string
}

func (c Filter) String() string {
Expand Down Expand Up @@ -167,7 +169,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
}
Expand All @@ -183,12 +185,8 @@ func (c *Filter) getCommand() (string, []string) {
}

// 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
}
func NewContainer(spec runtimeutil.ContainerSpec, uidgid string) Filter {
f := Filter{ContainerSpec: spec, UIDGID: uidgid}

return f
}
75 changes: 35 additions & 40 deletions kyaml/fn/runtime/container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ func TestFilter_setupExec(t *testing.T) {
name string
functionConfig string
expectedArgs []string
instance Filter
containerSpec runtimeutil.ContainerSpec
UIDGID string
}{
{
name: "command",
Expand All @@ -36,12 +37,10 @@ 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",
},
UIDGID: "nobody",
},

{
Expand All @@ -59,13 +58,11 @@ metadata:
"--user", "nobody",
"--security-opt=no-new-privileges",
},
instance: NewContainer(
runtimeutil.ContainerSpec{
Image: "example.com:version",
Network: true,
User: "nobody",
},
),
containerSpec: runtimeutil.ContainerSpec{
Image: "example.com:version",
Network: true,
},
UIDGID: "nobody",
},

{
Expand All @@ -87,21 +84,19 @@ 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/"},
},
),
},
UIDGID: "nobody",
},
{
name: "root user",
name: "as current user",
functionConfig: `apiVersion: apps/v1
kind: Deployment
metadata:
Expand All @@ -112,15 +107,13 @@ metadata:
"--rm",
"-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR",
"--network", "none",
"--user", "root",
"--user", "1:2",
"--security-opt=no-new-privileges",
},
instance: NewContainer(
runtimeutil.ContainerSpec{
Image: "example.com:version",
User: "root",
},
),
containerSpec: runtimeutil.ContainerSpec{
Image: "example.com:version",
},
UIDGID: "1:2",
},
}

Expand All @@ -131,18 +124,20 @@ 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 := NewContainer(tt.containerSpec, tt.UIDGID)
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()
}
})
Expand Down
18 changes: 0 additions & 18 deletions kyaml/fn/runtime/runtimeutil/functiontypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"`
}
Expand Down
53 changes: 37 additions & 16 deletions kyaml/runfn/runfn.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io"
"os"
"os/user"
"path"
"path/filepath"
"sort"
Expand Down Expand Up @@ -83,10 +84,11 @@ type RunFns struct {
// functionFilterProvider provides a filter to perform the function.
// this is a variable so it can be mocked in tests
functionFilterProvider func(
filter runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter, error)
filter runtimeutil.FunctionSpec, api *yaml.RNode, currentUser currentUserFunc) (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
Expand Down Expand Up @@ -299,13 +301,10 @@ 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)
c, err := r.functionFilterProvider(*spec, api, user.Current)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -397,8 +396,24 @@ func (r *RunFns) init() {
}
}

type currentUserFunc func() (*user.User, error)

// getUIDGID will return "nobody" if asCurrentUser is false. Otherwise
// return "uid:gid" according to the return from currentUser function.
func getUIDGID(asCurrentUser bool, currentUser currentUserFunc) (string, error) {
if !asCurrentUser {
return "nobody", nil
}

u, err := currentUser()
if err != nil {
return "", err
}
return fmt.Sprintf("%s:%s", u.Uid, u.Gid), nil
}

// ffp provides function filters
func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter, error) {
func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode, currentUser currentUserFunc) (kio.Filter, error) {
var resultsFile string
if r.ResultsDir != "" {
resultsFile = filepath.Join(r.ResultsDir, fmt.Sprintf(
Expand All @@ -407,13 +422,19 @@ 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,
})
uidgid, err := getUIDGID(r.AsCurrentUser, currentUser)
if err != nil {
return nil, err
}
c := container.NewContainer(
runtimeutil.ContainerSpec{
Image: spec.Container.Image,
Network: spec.Container.Network,
StorageMounts: r.StorageMounts,
Env: spec.Container.Env,
},
uidgid,
)
cf := &c
cf.Exec.FunctionConfig = api
cf.Exec.GlobalScope = r.GlobalScope
Expand Down
52 changes: 45 additions & 7 deletions kyaml/runfn/runfn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io/ioutil"
"os"
"os/user"
"path/filepath"
"runtime"
"strings"
Expand Down Expand Up @@ -38,6 +39,13 @@ replace: StatefulSet
`
)

func currentUser() (*user.User, error) {
return &user.User{
Uid: "1",
Gid: "2",
}, nil
}

func TestRunFns_init(t *testing.T) {
instance := RunFns{}
instance.init()
Expand All @@ -59,8 +67,38 @@ kind:
if !assert.NoError(t, err) {
return
}
filter, _ := instance.functionFilterProvider(spec, api)
c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"})
filter, _ := instance.functionFilterProvider(spec, api, currentUser)
c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}, "nobody")
cf := &c
cf.Exec.FunctionConfig = api
assert.Equal(t, cf, filter)
}

func TestRunFns_initAsCurrentUser(t *testing.T) {
instance := RunFns{
AsCurrentUser: true,
}
instance.init()
if !assert.Equal(t, instance.Input, os.Stdin) {
t.FailNow()
}
if !assert.Equal(t, instance.Output, os.Stdout) {
t.FailNow()
}

api, err := yaml.Parse(`apiVersion: apps/v1
kind:
`)
spec := runtimeutil.FunctionSpec{
Container: runtimeutil.ContainerSpec{
Image: "example.com:version",
},
}
if !assert.NoError(t, err) {
return
}
filter, _ := instance.functionFilterProvider(spec, api, currentUser)
c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}, "1:2")
cf := &c
cf.Exec.FunctionConfig = api
assert.Equal(t, cf, filter)
Expand Down Expand Up @@ -90,8 +128,8 @@ kind:
if !assert.NoError(t, err) {
return
}
filter, _ := instance.functionFilterProvider(spec, api)
c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"})
filter, _ := instance.functionFilterProvider(spec, api, currentUser)
c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}, "nobody")
cf := &c
cf.Exec.FunctionConfig = api
cf.Exec.GlobalScope = true
Expand Down Expand Up @@ -863,7 +901,7 @@ replace: StatefulSet
var fltrs []*TestFilter
instance := RunFns{
Path: dir,
functionFilterProvider: func(f runtimeutil.FunctionSpec, node *yaml.RNode) (kio.Filter, error) {
functionFilterProvider: func(f runtimeutil.FunctionSpec, node *yaml.RNode, currentUser currentUserFunc) (kio.Filter, error) {
tf := &TestFilter{
Exit: errors.Errorf("message: %s", f.Container.Image),
}
Expand Down Expand Up @@ -1069,8 +1107,8 @@ func setupTest(t *testing.T) string {
// getFilterProvider fakes the creation of a filter, replacing the ContainerFiler with
// a filter to s/kind: Deployment/kind: StatefulSet/g.
// this can be used to simulate running a filter.
func getFilterProvider(t *testing.T) func(runtimeutil.FunctionSpec, *yaml.RNode) (kio.Filter, error) {
return func(f runtimeutil.FunctionSpec, node *yaml.RNode) (kio.Filter, error) {
func getFilterProvider(t *testing.T) func(runtimeutil.FunctionSpec, *yaml.RNode, currentUserFunc) (kio.Filter, error) {
return func(f runtimeutil.FunctionSpec, node *yaml.RNode, currentUser currentUserFunc) (kio.Filter, error) {
// parse the filter from the input
filter := yaml.YFilter{}
b := &bytes.Buffer{}
Expand Down