From fa15242719d359195421637d7bd7844a0c4607cd Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Mon, 24 Aug 2020 11:40:57 -0700 Subject: [PATCH] refactor network name in kyaml container --- kyaml/fn/runtime/container/container.go | 23 +++++++++++++----- kyaml/fn/runtime/container/container_test.go | 24 +++++++++---------- kyaml/fn/runtime/runtimeutil/functiontypes.go | 24 +++++++++++++++++-- kyaml/runfn/runfn.go | 21 +++++++--------- kyaml/runfn/runfn_test.go | 6 +++-- 5 files changed, 63 insertions(+), 35 deletions(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index 9f34f1512..3975e003b 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -161,15 +161,10 @@ func (c *Filter) getCommand() (string, []string) { // run the container using docker. this is simpler than using the docker // libraries, and ensures things like auth work the same as if the container // was run from the cli. - - network := "none" - if c.Network.Name != "" { - network = c.Network.Name - } args := []string{"run", "--rm", // delete the container afterward "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", // attach stdin, stdout, stderr - "--network", network, + "--network", c.ContainerSpec.Network.Name.String(), // added security options "--user", c.User.String(), @@ -198,3 +193,19 @@ func (c *Filter) getCommand() (string, []string) { a := append(args, c.Image) return "docker", a } + +// NewContainer returns a new container instance +func NewContainer(spec runtimeutil.ContainerSpec) Filter { + f := Filter{ContainerSpec: spec} + // default user is nobody + if f.ContainerSpec.User.IsEmpty() { + f.ContainerSpec.User = runtimeutil.UserNobody + } + + // default network name is none + if f.ContainerSpec.Network.Name.IsEmpty() { + f.ContainerSpec.Network.Name = runtimeutil.NetworkNameNone + } + + return f +} diff --git a/kyaml/fn/runtime/container/container_test.go b/kyaml/fn/runtime/container/container_test.go index bd6f0e6b3..c0833f5b2 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -38,12 +38,12 @@ metadata: "--user", "nobody", "--security-opt=no-new-privileges", }, - instance: Filter{ - ContainerSpec: runtimeutil.ContainerSpec{ + instance: NewContainer( + runtimeutil.ContainerSpec{ Image: "example.com:version", User: "nobody", }, - }, + ), }, { @@ -61,15 +61,15 @@ metadata: "--user", "nobody", "--security-opt=no-new-privileges", }, - instance: Filter{ - ContainerSpec: runtimeutil.ContainerSpec{ + instance: NewContainer( + runtimeutil.ContainerSpec{ Image: "example.com:version", Network: runtimeutil.ContainerNetwork{ Name: "test-1", }, User: "nobody", }, - }, + ), }, { @@ -91,8 +91,8 @@ 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: Filter{ - ContainerSpec: runtimeutil.ContainerSpec{ + instance: NewContainer( + runtimeutil.ContainerSpec{ Image: "example.com:version", StorageMounts: []runtimeutil.StorageMount{ {MountType: "bind", Src: "/mount/path", DstPath: "/local/"}, @@ -102,7 +102,7 @@ metadata: }, User: "nobody", }, - }, + ), }, { name: "root user", @@ -119,12 +119,12 @@ metadata: "--user", "root", "--security-opt=no-new-privileges", }, - instance: Filter{ - ContainerSpec: runtimeutil.ContainerSpec{ + instance: NewContainer( + runtimeutil.ContainerSpec{ Image: "example.com:version", User: "root", }, - }, + ), }, } diff --git a/kyaml/fn/runtime/runtimeutil/functiontypes.go b/kyaml/fn/runtime/runtimeutil/functiontypes.go index 7eb1c7056..3eac14399 100644 --- a/kyaml/fn/runtime/runtimeutil/functiontypes.go +++ b/kyaml/fn/runtime/runtimeutil/functiontypes.go @@ -33,6 +33,26 @@ const ( UserNobody ContainerUser = "nobody" ) +// ContainerNetworkName is a type for network name used in container +type ContainerNetworkName string + +func (n *ContainerNetworkName) String() string { + return string(*n) +} + +func (n *ContainerNetworkName) IsEmpty() bool { + return string(*n) == "" +} + +func (n *ContainerNetworkName) Set(s string) { + *n = ContainerNetworkName(s) +} + +const ( + NetworkNameNone ContainerNetworkName = "none" + NetworkNameEmpty ContainerNetworkName = "" +) + // FunctionSpec defines a spec for running a function type FunctionSpec struct { DeferFailure bool `json:"deferFailure,omitempty" yaml:"deferFailure,omitempty"` @@ -75,7 +95,7 @@ type ContainerNetwork struct { Required bool `json:"required,omitempty" yaml:"required,omitempty"` // Name is the name of the network to use from a container - Name string `json:"name,omitempty" yaml:"name,omitempty"` + Name ContainerNetworkName `json:"name,omitempty" yaml:"name,omitempty"` } // StarlarkSpec defines how to run a function as a starlark program @@ -128,7 +148,7 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec { } if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil { - fn.Container.Network.Name = "" + fn.Container.Network.Name = NetworkNameEmpty fn.StorageMounts = []StorageMount{} return fn } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index c757967e2..59f44a170 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -280,16 +280,12 @@ 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") } - spec.Container.Network.Name = r.NetworkName + spec.Container.Network.Name.Set(r.NetworkName) } // command line username has higher priority if r.User != "" { spec.Container.User = r.User } - // default user is nobody - if spec.Container.User.IsEmpty() { - spec.Container.User = runtimeutil.UserNobody - } c, err := r.functionFilterProvider(*spec, api) if err != nil { @@ -393,14 +389,13 @@ 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 - cf := &container.Filter{ - ContainerSpec: runtimeutil.ContainerSpec{ - Image: spec.Container.Image, - Network: spec.Container.Network, - StorageMounts: r.StorageMounts, - User: spec.Container.User, - }, - } + c := container.NewContainer(runtimeutil.ContainerSpec{ + Image: spec.Container.Image, + Network: spec.Container.Network, + StorageMounts: r.StorageMounts, + User: spec.Container.User, + }) + cf := &c cf.Exec.FunctionConfig = api cf.Exec.GlobalScope = r.GlobalScope cf.Exec.ResultsFile = resultsFile diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index 8599609fc..55cb2d429 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -60,7 +60,8 @@ kind: return } filter, _ := instance.functionFilterProvider(spec, api) - cf := &container.Filter{ContainerSpec: runtimeutil.ContainerSpec{Image: "example.com:version"}} + c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}) + cf := &c cf.Exec.FunctionConfig = api assert.Equal(t, cf, filter) } @@ -90,7 +91,8 @@ kind: return } filter, _ := instance.functionFilterProvider(spec, api) - cf := &container.Filter{ContainerSpec: runtimeutil.ContainerSpec{Image: "example.com:version"}} + c := container.NewContainer(runtimeutil.ContainerSpec{Image: "example.com:version"}) + cf := &c cf.Exec.FunctionConfig = api cf.Exec.GlobalScope = true assert.Equal(t, cf, filter)