From f6c06b58ef217bb2b11f550a03ceeb06ba314dee Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Wed, 16 Sep 2020 16:20:50 -0700 Subject: [PATCH] Change network to a boolean --- kyaml/fn/runtime/container/container.go | 11 +- kyaml/fn/runtime/container/container_test.go | 10 +- kyaml/fn/runtime/runtimeutil/functiontypes.go | 16 +-- .../runtime/runtimeutil/runtimeutil_test.go | 17 +-- kyaml/runfn/runfn.go | 13 ++- kyaml/runfn/runfn_test.go | 105 +++++++++++++++++- 6 files changed, 129 insertions(+), 43 deletions(-) diff --git a/kyaml/fn/runtime/container/container.go b/kyaml/fn/runtime/container/container.go index 4b82419d5..e361f2e30 100644 --- a/kyaml/fn/runtime/container/container.go +++ b/kyaml/fn/runtime/container/container.go @@ -154,13 +154,17 @@ func (c *Filter) setupExec() { // getArgs returns the command + args to run to spawn the container func (c *Filter) getCommand() (string, []string) { + network := runtimeutil.NetworkNameNone + if c.ContainerSpec.Network { + network = runtimeutil.NetworkNameHost + } // 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. args := []string{"run", "--rm", // delete the container afterward "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", // attach stdin, stdout, stderr - "--network", string(c.ContainerSpec.Network.Name), + "--network", string(network), // added security options "--user", c.User.String(), @@ -186,10 +190,5 @@ func NewContainer(spec runtimeutil.ContainerSpec) Filter { f.ContainerSpec.User = runtimeutil.UserNobody } - // default network name is none - if f.ContainerSpec.Network.Name == "" { - 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 23d83c58d..2cb72f203 100644 --- a/kyaml/fn/runtime/container/container_test.go +++ b/kyaml/fn/runtime/container/container_test.go @@ -55,17 +55,15 @@ metadata: "run", "--rm", "-i", "-a", "STDIN", "-a", "STDOUT", "-a", "STDERR", - "--network", "test-1", + "--network", "host", "--user", "nobody", "--security-opt=no-new-privileges", }, instance: NewContainer( runtimeutil.ContainerSpec{ - Image: "example.com:version", - Network: runtimeutil.ContainerNetwork{ - Name: "test-1", - }, - User: "nobody", + Image: "example.com:version", + Network: true, + User: "nobody", }, ), }, diff --git a/kyaml/fn/runtime/runtimeutil/functiontypes.go b/kyaml/fn/runtime/runtimeutil/functiontypes.go index 36546bd74..263fbd6fc 100644 --- a/kyaml/fn/runtime/runtimeutil/functiontypes.go +++ b/kyaml/fn/runtime/runtimeutil/functiontypes.go @@ -38,8 +38,8 @@ const ( type ContainerNetworkName string const ( - NetworkNameNone ContainerNetworkName = "none" - NetworkNameEmpty ContainerNetworkName = "" + NetworkNameNone ContainerNetworkName = "none" + NetworkNameHost ContainerNetworkName = "host" ) const defaultEnvValue string = "true" @@ -166,7 +166,7 @@ type ContainerSpec struct { Image string `json:"image,omitempty" yaml:"image,omitempty"` // Network defines network specific configuration - Network ContainerNetwork `json:"network,omitempty" yaml:"network,omitempty"` + Network bool `json:"network,omitempty" yaml:"network,omitempty"` // Mounts are the storage or directories to mount into the container StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"` @@ -178,15 +178,6 @@ type ContainerSpec struct { Env []string `json:"envs,omitempty" yaml:"envs,omitempty"` } -// ContainerNetwork -type ContainerNetwork struct { - // Required specifies that function requires a network - Required bool `json:"required,omitempty" yaml:"required,omitempty"` - - // Name is the name of the network to use from a container - Name ContainerNetworkName `json:"name,omitempty" yaml:"name,omitempty"` -} - // StarlarkSpec defines how to run a function as a starlark program type StarlarkSpec struct { Name string `json:"name,omitempty" yaml:"name,omitempty"` @@ -237,7 +228,6 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec { } if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil { - fn.Container.Network.Name = NetworkNameEmpty fn.StorageMounts = []StorageMount{} return fn } diff --git a/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go b/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go index 3083851b8..bafdb5907 100644 --- a/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go +++ b/kyaml/fn/runtime/runtimeutil/runtimeutil_test.go @@ -1208,14 +1208,12 @@ metadata: config.kubernetes.io/function: |- container: image: foo:v1.0.0 - network: - required: true + network: true `, expectedFn: ` container: image: foo:v1.0.0 - network: - required: true + network: true `, }, @@ -1324,8 +1322,7 @@ metadata: configFn: container: image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 - network: - required: true + network: true `, required: true, }, @@ -1337,8 +1334,7 @@ metadata: configFn: container: image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 - network: - required: false + network: false `, required: false, }, @@ -1363,8 +1359,7 @@ metadata: config.kubernetes.io/function: | container: image: gcr.io/kustomize-functions/example-tshirt:v0.1.0 - network: - required: true + network: true `, required: true, }, @@ -1376,7 +1371,7 @@ metadata: return } fn := GetFunctionSpec(cfg) - assert.Equal(t, tc.required, fn.Container.Network.Required) + assert.Equal(t, tc.required, fn.Container.Network) } } diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index a2ab90dad..e0a890212 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -294,12 +294,13 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) ( for i := range fns { api := fns[i] spec := runtimeutil.GetFunctionSpec(api) - if spec.Container.Network.Required { - if !r.Network { - // 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 = runtimeutil.ContainerNetworkName(r.NetworkName) + if spec == nil { + // resource doesn't have function spec + continue + } + if spec.Container.Network && !r.Network { + // 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() { diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go index dff30c4ef..68d852c2c 100644 --- a/kyaml/runfn/runfn_test.go +++ b/kyaml/runfn/runfn_test.go @@ -244,6 +244,17 @@ metadata: out: []string{"gcr.io/example.com/image:v1.0.0"}, }, + {name: "no function spec", + in: []f{ + { + explicitFunction: true, + value: ` +foo: bar +`, + }, + }, + }, + // Test // // @@ -685,6 +696,98 @@ metadata: } } +func TestRunFns_network(t *testing.T) { + tests := []struct { + name string + input string + network bool + expectNetwork bool + error string + }{ + { + name: "imperative false, declarative false", + input: ` +metadata: + annotations: + config.kubernetes.io/function: | + container: + image: a + network: false +`, + network: false, + expectNetwork: false, + }, + { + name: "imperative true, declarative false", + input: ` +metadata: + annotations: + config.kubernetes.io/function: | + container: + image: a + network: false +`, + network: true, + expectNetwork: false, + }, + { + name: "imperative true, declarative true", + input: ` +metadata: + annotations: + config.kubernetes.io/function: | + container: + image: a + network: true +`, + network: true, + expectNetwork: true, + }, + { + name: "imperative false, declarative true", + input: ` +metadata: + annotations: + config.kubernetes.io/function: | + container: + image: a + network: true +`, + network: false, + error: "network required but not enabled with --network", + }, + } + + for i := range tests { + tt := tests[i] + fn := yaml.MustParse(tt.input) + t.Run(tt.name, func(t *testing.T) { + // init the instance + r := &RunFns{ + Functions: []*yaml.RNode{fn}, + Network: tt.network, + } + r.init() + + _, fltrs, _, err := r.getNodesAndFilters() + if tt.error != "" { + if !assert.EqualError(t, err, tt.error) { + t.FailNow() + } + return + } + if !assert.NoError(t, err) { + t.FailNow() + } + + fltr := fltrs[0].(*container.Filter) + if !assert.Equal(t, tt.expectNetwork, fltr.Network) { + t.FailNow() + } + }) + } +} + func TestCmd_Execute(t *testing.T) { dir := setupTest(t) defer os.RemoveAll(dir) @@ -987,7 +1090,7 @@ func getFilterProvider(t *testing.T) func(runtimeutil.FunctionSpec, *yaml.RNode) } } -func TestRunfns_mergeContainerEnv(t *testing.T) { +func TestRunFns_mergeContainerEnv(t *testing.T) { testcases := []struct { name string instance RunFns