code review

kyaml/fn/runtime/container/container.go

@@ -175,7 +175,7 @@ func (c *Filter) getCommand() (string, []string) {
 		args = append(args, "--mount", storageMount.String())
 	}
 
-	args = append(args, c.Envs.GetDockerFlags()...)
+	args = append(args, c.Env.GetDockerFlags()...)
 	a := append(args, c.Image)
 	return "docker", a
 }

kyaml/fn/runtime/container/container_test.go

@@ -135,14 +135,12 @@ metadata:
 				t.FailNow()
 			}
 			tt.instance.Exec.FunctionConfig = cfg
-
+			tt.instance.Env.AddKeyValue("KYAML_TEST", "FOO")
-			tt.instance.Envs.AddKeyValue("KYAML_TEST", "FOO")
-			tt.expectedArgs = append(tt.expectedArgs, tt.instance.Envs.GetDockerFlags()...)
-
-			tt.expectedArgs = append(tt.expectedArgs, tt.instance.Image)
-
 			tt.instance.setupExec()
 
+			tt.expectedArgs = append(tt.expectedArgs, tt.instance.Env.GetDockerFlags()...)
+			tt.expectedArgs = append(tt.expectedArgs, tt.instance.Image)
+
 			if !assert.Equal(t, "docker", tt.instance.Exec.Path) {
 				t.FailNow()
 			}

kyaml/fn/runtime/runtimeutil/functiontypes.go

@@ -43,26 +43,22 @@ const (
 )
 const defaultEnvValue string = "true"
 
-// ContainerEnvs contains the environment variables for the container
+// ContainerEnv defines the environment present in a container.
-type ContainerEnvs struct {
+type ContainerEnv struct {
-	// EnvsMap is a key-value map that will be set as env in container
+	// EnvVars is a key-value map that will be set as env in container
-	EnvsMap map[string]string `json:"envsMap,omitempty" yaml:"envsMap,omitempty"`
+	EnvVars map[string]string
 
-	// ExportKeys are only env key. Value will be the value in the host system
+	// VarsToExport are only env key. Value will be the value in the host system
-	ExportKeys []string `json:"exportKeys,omitempty" yaml:"exportKeys,omitempty"`
+	VarsToExport []string
 }
 
 // GetDockerFlags returns docker run style env flags
-func (ce *ContainerEnvs) GetDockerFlags() []string {
+func (ce *ContainerEnv) GetDockerFlags() []string {
-	envs := ce.EnvsMap
+	envs := ce.EnvVars
 	if envs == nil {
 		envs = make(map[string]string)
 	}
 
-	// default envs
-	envs["LOG_TO_STDERR"] = defaultEnvValue
-	envs["STRUCTURED_RESULTS"] = defaultEnvValue
-
 	flags := []string{}
 	// return in order to keep consistent among different runs
 	keys := []string{}
@@ -74,7 +70,7 @@ func (ce *ContainerEnvs) GetDockerFlags() []string {
 		flags = append(flags, "-e", key+"="+envs[key])
 	}
 
-	for _, key := range ce.ExportKeys {
+	for _, key := range ce.VarsToExport {
 		flags = append(flags, "-e", key)
 	}
 
@@ -82,16 +78,16 @@ func (ce *ContainerEnvs) GetDockerFlags() []string {
 }
 
 // AddKeyValue adds a key-value pair into the envs
-func (ce *ContainerEnvs) AddKeyValue(key, value string) {
+func (ce *ContainerEnv) AddKeyValue(key, value string) {
-	if ce.EnvsMap == nil {
+	if ce.EnvVars == nil {
-		ce.EnvsMap = make(map[string]string)
+		ce.EnvVars = make(map[string]string)
 	}
-	ce.EnvsMap[key] = value
+	ce.EnvVars[key] = value
 }
 
 // HasExportedKey returns true if the key is a exported key
-func (ce *ContainerEnvs) HasExportedKey(key string) bool {
+func (ce *ContainerEnv) HasExportedKey(key string) bool {
-	for _, k := range ce.ExportKeys {
+	for _, k := range ce.VarsToExport {
 		if k == key {
 			return true
 		}
@@ -100,16 +96,34 @@ func (ce *ContainerEnvs) HasExportedKey(key string) bool {
 }
 
 // AddKey adds a key into the envs
-func (ce *ContainerEnvs) AddKey(key string) {
+func (ce *ContainerEnv) AddKey(key string) {
 	if !ce.HasExportedKey(key) {
-		ce.ExportKeys = append(ce.ExportKeys, key)
+		ce.VarsToExport = append(ce.VarsToExport, key)
 	}
 }
 
-// NewContainerEnvs returns an empty instance of ContainerEnvs
+// NewContainerEnv returns a pointer to a new ContainerEnv
-func NewContainerEnvs() ContainerEnvs {
+func NewContainerEnv() *ContainerEnv {
-	var ce ContainerEnvs
+	var ce ContainerEnv
-	ce.EnvsMap = make(map[string]string)
+	ce.EnvVars = make(map[string]string)
+	// default envs
+	ce.EnvVars["LOG_TO_STDERR"] = defaultEnvValue
+	ce.EnvVars["STRUCTURED_RESULTS"] = defaultEnvValue
+	return &ce
+}
+
+// NewContainerEnvFromStringSlice returns a new ContainerEnv pointer with parsing
+// input envStr. envStr example: ["foo=bar", "baz"]
+func NewContainerEnvFromStringSlice(envStr []string) *ContainerEnv {
+	ce := NewContainerEnv()
+	for _, e := range envStr {
+		parts := strings.SplitN(e, "=", 2)
+		if len(parts) == 1 {
+			ce.AddKey(e)
+		} else {
+			ce.AddKeyValue(parts[0], parts[1])
+		}
+	}
 	return ce
 }
 
@@ -148,8 +162,11 @@ type ContainerSpec struct {
 	// User is the username/uid that application runs as in continer
 	User ContainerUser `json:"user,omitempty" yaml:"user,omitempty"`
 
-	// Envs are environment variables that will be exported to container
+	// EnvRaw is a slice of env string.
-	Envs ContainerEnvs `json:"envs,omitempty" yaml:"envs,omitempty"`
+	EnvRaw []string `json:"envs,omitempty" yaml:"envs,omitempty"`
+
+	// Env contains environment variables that will be exported to container
+	Env ContainerEnv `json:"-" yaml:"-"`
 }
 
 // ContainerNetwork
@@ -213,6 +230,7 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec {
 	if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil {
 		fn.Container.Network.Name = NetworkNameEmpty
 		fn.StorageMounts = []StorageMount{}
+		fn.Container.Env = *NewContainerEnvFromStringSlice(fn.Container.EnvRaw)
 		return fn
 	}
 

kyaml/fn/runtime/runtimeutil/runtimeutil_test.go

@@ -1429,36 +1429,25 @@ func Test_StringToStorageMount(t *testing.T) {
 	}
 }
 
-func TestContainerEnvs(t *testing.T) {
+func TestContainerEnvGetDockerFlags(t *testing.T) {
 	tests := []struct {
-		input  ContainerEnvs
+		input  *ContainerEnv
 		output []string
 	}{
 		{
-			input: ContainerEnvs{
+			input:  NewContainerEnvFromStringSlice([]string{"foo=bar"}),
-				EnvsMap: map[string]string{
-					"foo": "bar",
-				},
-			},
 			output: []string{"-e", "LOG_TO_STDERR=true", "-e", "STRUCTURED_RESULTS=true", "-e", "foo=bar"},
 		},
 		{
-			input: ContainerEnvs{
+			input:  NewContainerEnvFromStringSlice([]string{"foo"}),
-				ExportKeys: []string{"foo"},
-			},
 			output: []string{"-e", "LOG_TO_STDERR=true", "-e", "STRUCTURED_RESULTS=true", "-e", "foo"},
 		},
 		{
-			input: ContainerEnvs{
+			input:  NewContainerEnvFromStringSlice([]string{"foo=bar", "baz"}),
-				EnvsMap: map[string]string{
-					"foo": "bar",
-				},
-				ExportKeys: []string{"baz"},
-			},
 			output: []string{"-e", "LOG_TO_STDERR=true", "-e", "STRUCTURED_RESULTS=true", "-e", "foo=bar", "-e", "baz"},
 		},
 		{
-			input:  ContainerEnvs{},
+			input:  NewContainerEnv(),
 			output: []string{"-e", "LOG_TO_STDERR=true", "-e", "STRUCTURED_RESULTS=true"},
 		},
 	}
@@ -1468,3 +1457,63 @@ func TestContainerEnvs(t *testing.T) {
 		assert.Equal(t, tc.output, flags)
 	}
 }
+
+func TestGetContainerEnv(t *testing.T) {
+	tests := []struct {
+		input    string
+		expected ContainerEnv
+	}{
+		{
+			input: `
+apiVersion: v1
+kind: Foo
+metadata:
+  name: foo
+  configFn:
+    container:
+      image: gcr.io/kustomize-functions/example-tshirt:v0.1.0
+      envs:
+      - foo=bar
+`,
+			expected: *NewContainerEnvFromStringSlice([]string{"foo=bar"}),
+		},
+		{
+			input: `
+apiVersion: v1
+kind: Foo
+metadata:
+  name: foo
+  configFn:
+    container:
+      image: gcr.io/kustomize-functions/example-tshirt:v0.1.0
+      envs:
+      - foo=bar
+      - baz
+`,
+			expected: *NewContainerEnvFromStringSlice([]string{"foo=bar", "baz"}),
+		},
+		{
+			input: `
+apiVersion: v1
+kind: Foo
+metadata:
+  name: foo
+  configFn:
+    container:
+      image: gcr.io/kustomize-functions/example-tshirt:v0.1.0
+      envs:
+      - KUBECONFIG
+`,
+			expected: *NewContainerEnvFromStringSlice([]string{"KUBECONFIG"}),
+		},
+	}
+
+	for _, tc := range tests {
+		cfg, err := yaml.Parse(tc.input)
+		if !assert.NoError(t, err) {
+			return
+		}
+		fn := GetFunctionSpec(cfg)
+		assert.Equal(t, tc.expected, fn.Container.Env)
+	}
+}

kyaml/runfn/runfn.go

@@ -91,8 +91,8 @@ type RunFns struct {
 	// User username used to run the application in container,
 	User runtimeutil.ContainerUser
 
-	// Envs are environment variables that will be exported to container
+	// Env contains environment variables that will be exported to container
-	Envs runtimeutil.ContainerEnvs
+	Env runtimeutil.ContainerEnv
 }
 
 // Execute runs the command
@@ -272,14 +272,14 @@ func (r RunFns) getFunctionsFromFunctions() ([]kio.Filter, error) {
 	return r.getFunctionFilters(true, r.Functions...)
 }
 
-// mergeContainerEnvs will merge the envs specified by command line (imperative) and config
+// mergeContainerEnv will merge the envs specified by command line (imperative) and config
 // file (declarative). If they have same key, the imperative value will be respected.
-func (r RunFns) mergeContainerEnvs(envs runtimeutil.ContainerEnvs) runtimeutil.ContainerEnvs {
+func (r RunFns) mergeContainerEnv(envs runtimeutil.ContainerEnv) runtimeutil.ContainerEnv {
-	for key, value := range r.Envs.EnvsMap {
+	for key, value := range r.Env.EnvVars {
 		envs.AddKeyValue(key, value)
 	}
 
-	for _, key := range r.Envs.ExportKeys {
+	for _, key := range r.Env.VarsToExport {
 		envs.AddKey(key)
 	}
 
@@ -303,7 +303,7 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) (
 		if !r.User.IsEmpty() {
 			spec.Container.User = r.User
 		}
-		spec.Container.Envs = r.mergeContainerEnvs(spec.Container.Envs)
+		spec.Container.Env = r.mergeContainerEnv(spec.Container.Env)
 
 		c, err := r.functionFilterProvider(*spec, api)
 		if err != nil {
@@ -412,7 +412,7 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter
 			Network:       spec.Container.Network,
 			StorageMounts: r.StorageMounts,
 			User:          spec.Container.User,
-			Envs:          spec.Container.Envs,
+			Env:           spec.Container.Env,
 		})
 		cf := &c
 		cf.Exec.FunctionConfig = api

kyaml/runfn/runfn_test.go

@@ -987,102 +987,55 @@ func getFilterProvider(t *testing.T) func(runtimeutil.FunctionSpec, *yaml.RNode)
 	}
 }
 
-func TestRunfns_mergeContainerEnvs(t *testing.T) {
+func TestRunfns_mergeContainerEnv(t *testing.T) {
 	testcases := []struct {
 		name      string
 		instance  RunFns
-		inputEnvs runtimeutil.ContainerEnvs
+		inputEnvs runtimeutil.ContainerEnv
-		expect    runtimeutil.ContainerEnvs
+		expect    runtimeutil.ContainerEnv
 	}{
 		{
 			name:      "all empty",
 			instance:  RunFns{},
-			inputEnvs: runtimeutil.NewContainerEnvs(),
+			inputEnvs: *runtimeutil.NewContainerEnv(),
-			expect:    runtimeutil.NewContainerEnvs(),
+			expect:    *runtimeutil.NewContainerEnv(),
 		},
 		{
-			name:     "empty command line envs",
+			name:      "empty command line envs",
-			instance: RunFns{},
+			instance:  RunFns{},
-			inputEnvs: runtimeutil.ContainerEnvs{
+			inputEnvs: *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar"}),
-				EnvsMap: map[string]string{
+			expect:    *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar"}),
-					"foo": "bar",
-				},
-			},
-			expect: runtimeutil.ContainerEnvs{
-				EnvsMap: map[string]string{
-					"foo": "bar",
-				},
-			},
 		},
 		{
 			name: "empty declarative envs",
 			instance: RunFns{
-				Envs: runtimeutil.ContainerEnvs{
+				Env: *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar"}),
-					EnvsMap: map[string]string{
-						"foo": "bar",
-					},
-				},
-			},
-			inputEnvs: runtimeutil.NewContainerEnvs(),
-			expect: runtimeutil.ContainerEnvs{
-				EnvsMap: map[string]string{
-					"foo": "bar",
-				},
 			},
+			inputEnvs: *runtimeutil.NewContainerEnv(),
+			expect:    *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar"}),
 		},
 		{
 			name: "same key",
 			instance: RunFns{
-				Envs: runtimeutil.ContainerEnvs{
+				Env: *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar", "foo"}),
-					EnvsMap: map[string]string{
-						"foo": "bar",
-					},
-					ExportKeys: []string{"foo"},
-				},
-			},
-			inputEnvs: runtimeutil.ContainerEnvs{
-				EnvsMap: map[string]string{
-					"foo": "bar1",
-				},
-				ExportKeys: []string{"bar"},
-			},
-			expect: runtimeutil.ContainerEnvs{
-				EnvsMap: map[string]string{
-					"foo": "bar",
-				},
-				ExportKeys: []string{"bar", "foo"},
 			},
+			inputEnvs: *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar1", "bar"}),
+			expect:    *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar", "bar", "foo"}),
 		},
 		{
 			name: "same exported key",
 			instance: RunFns{
-				Envs: runtimeutil.ContainerEnvs{
+				Env: *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar", "foo"}),
-					EnvsMap: map[string]string{
-						"foo": "bar",
-					},
-					ExportKeys: []string{"foo"},
-				},
-			},
-			inputEnvs: runtimeutil.ContainerEnvs{
-				EnvsMap: map[string]string{
-					"foo1": "bar1",
-				},
-				ExportKeys: []string{"foo"},
-			},
-			expect: runtimeutil.ContainerEnvs{
-				EnvsMap: map[string]string{
-					"foo":  "bar",
-					"foo1": "bar1",
-				},
-				ExportKeys: []string{"foo"},
 			},
+			inputEnvs: *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo1=bar1", "foo"}),
+			expect:    *runtimeutil.NewContainerEnvFromStringSlice([]string{"foo=bar", "foo1=bar1", "foo"}),
 		},
 	}
 
 	for i := range testcases {
 		tc := testcases[i]
 		t.Run(tc.name, func(t *testing.T) {
-			envs := tc.instance.mergeContainerEnvs(tc.inputEnvs)
+			envs := tc.instance.mergeContainerEnv(tc.inputEnvs)
 			assert.Equal(t, tc.expect.GetDockerFlags(), envs.GetDockerFlags())
 		})
 	}