code review

kyaml/fn/runtime/container/container.go

@@ -123,18 +123,7 @@ import (
 //             ├── deployment_foo.yaml
 //             └── service_bar.yaml
 type Filter struct {
-
-	// Image is the container image to use to create a container.
-	Image string `yaml:"image,omitempty"`
-
-	// Network is the container network to use.
-	Network string `yaml:"network,omitempty"`
-
-	// StorageMounts is a list of storage options that the container will have mounted.
-	StorageMounts []runtimeutil.StorageMount `yaml:"mounts,omitempty"`
-
-	// User username used to run the application in container,
-	User string
+	runtimeutil.ContainerSpec `json:",inline" yaml:",inline"`
 
 	Exec runtimeexec.Filter
 }
@@ -174,13 +163,8 @@ func (c *Filter) getCommand() (string, []string) {
 	// was run from the cli.
 
 	network := "none"
-	if c.Network != "" {
-		network = c.Network
-	}
-	// run as nobody by default
-	user := c.User
-	if user == "" {
-		user = "nobody"
+	if c.Network.Name != "" {
+		network = c.Network.Name
 	}
 	args := []string{"run",
 		"--rm",                                              // delete the container afterward
@@ -188,7 +172,7 @@ func (c *Filter) getCommand() (string, []string) {
 		"--network", network,
 
 		// added security options
-		"--user", user,
+		"--user", c.User.String(),
 		"--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
 	}

kyaml/fn/runtime/container/container_test.go

@@ -38,7 +38,12 @@ metadata:
 				"--user", "nobody",
 				"--security-opt=no-new-privileges",
 			},
-			instance: Filter{Image: "example.com:version"},
+			instance: Filter{
+				ContainerSpec: runtimeutil.ContainerSpec{
+					Image: "example.com:version",
+					User:  "nobody",
+				},
+			},
 		},
 
 		{
@@ -56,7 +61,15 @@ metadata:
 				"--user", "nobody",
 				"--security-opt=no-new-privileges",
 			},
-			instance: Filter{Image: "example.com:version", Network: "test-1"},
+			instance: Filter{
+				ContainerSpec: runtimeutil.ContainerSpec{
+					Image: "example.com:version",
+					Network: runtimeutil.ContainerNetwork{
+						Name: "test-1",
+					},
+					User: "nobody",
+				},
+			},
 		},
 
 		{
@@ -79,12 +92,15 @@ metadata:
 				"--mount", fmt.Sprintf("type=%s,source=%s,target=%s,readonly", "tmpfs", "", "/local/"),
 			},
 			instance: Filter{
-				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/"},
+				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/"},
+					},
+					User: "nobody",
 				},
 			},
 		},
@@ -104,8 +120,10 @@ metadata:
 				"--security-opt=no-new-privileges",
 			},
 			instance: Filter{
-				Image: "example.com:version",
-				User:  "root",
+				ContainerSpec: runtimeutil.ContainerSpec{
+					Image: "example.com:version",
+					User:  "root",
+				},
 			},
 		},
 	}
@@ -203,7 +221,7 @@ metadata:
 	}
 }
 func TestFilter_String(t *testing.T) {
-	instance := Filter{Image: "foo"}
+	instance := Filter{ContainerSpec: runtimeutil.ContainerSpec{Image: "foo"}}
 	if !assert.Equal(t, "foo", instance.String()) {
 		t.FailNow()
 	}
@@ -234,7 +252,7 @@ func TestFilter_ExitCode(t *testing.T) {
 func TestIgnoreEnv(t *testing.T) {
 	os.Setenv(tmpDirEnvKey, "")
 
-	fltr := Filter{Image: "example.com:version"}
+	fltr := Filter{ContainerSpec: runtimeutil.ContainerSpec{Image: "example.com:version"}}
 	_, args := fltr.getCommand()
 	for _, arg := range args {
 		if arg == tmpDirEnvKey {

kyaml/fn/runtime/runtimeutil/functiontypes.go

@@ -18,11 +18,23 @@ 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"
+)
+
 // FunctionSpec defines a spec for running a function
 type FunctionSpec struct {
-	// Network is the name of the network to use from a container
-	Network string `json:"network,omitempty" yaml:"network,omitempty"`
-
 	DeferFailure bool `json:"deferFailure,omitempty" yaml:"deferFailure,omitempty"`
 
 	// Container is the spec for running a function as a container
@@ -54,13 +66,16 @@ type ContainerSpec struct {
 	StorageMounts []StorageMount `json:"mounts,omitempty" yaml:"mounts,omitempty"`
 
 	// User is the username/uid that application runs as in continer
-	User string `json:"user,omitempty" yaml:"user,omitempty"`
+	User ContainerUser `json:"user,omitempty" yaml:"user,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 string `json:"name,omitempty" yaml:"name,omitempty"`
 }
 
 // StarlarkSpec defines how to run a function as a starlark program
@@ -113,7 +128,7 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec {
 	}
 
 	if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil {
-		fn.Network = ""
+		fn.Container.Network.Name = ""
 		fn.StorageMounts = []StorageMount{}
 		return fn
 	}

kyaml/runfn/runfn.go

@@ -89,7 +89,7 @@ type RunFns struct {
 		filter runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter, error)
 
 	// User username used to run the application in container,
-	User string
+	User runtimeutil.ContainerUser
 }
 
 // Execute runs the command
@@ -280,8 +280,17 @@ 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.Network = r.NetworkName
+			spec.Container.Network.Name = 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 {
 			return nil, err
@@ -383,17 +392,14 @@ func (r *RunFns) ffp(spec runtimeutil.FunctionSpec, api *yaml.RNode) (kio.Filter
 		atomic.AddUint32(&r.resultsCount, 1)
 	}
 	if !r.DisableContainers && spec.Container.Image != "" {
-		// command line username has higher priority
-		user := spec.Container.User
-		if r.User != "" {
-			user = r.User
-		}
 		// TODO: Add a test for this behavior
 		cf := &container.Filter{
-			Image:         spec.Container.Image,
-			Network:       spec.Network,
-			StorageMounts: r.StorageMounts,
-			User:          user,
+			ContainerSpec: runtimeutil.ContainerSpec{
+				Image:         spec.Container.Image,
+				Network:       spec.Container.Network,
+				StorageMounts: r.StorageMounts,
+				User:          spec.Container.User,
+			},
 		}
 		cf.Exec.FunctionConfig = api
 		cf.Exec.GlobalScope = r.GlobalScope

kyaml/runfn/runfn_test.go

@@ -60,7 +60,7 @@ kind:
 		return
 	}
 	filter, _ := instance.functionFilterProvider(spec, api)
-	cf := &container.Filter{Image: "example.com:version"}
+	cf := &container.Filter{ContainerSpec: runtimeutil.ContainerSpec{Image: "example.com:version"}}
 	cf.Exec.FunctionConfig = api
 	assert.Equal(t, cf, filter)
 }
@@ -90,7 +90,7 @@ kind:
 		return
 	}
 	filter, _ := instance.functionFilterProvider(spec, api)
-	cf := &container.Filter{Image: "example.com:version"}
+	cf := &container.Filter{ContainerSpec: runtimeutil.ContainerSpec{Image: "example.com:version"}}
 	cf.Exec.FunctionConfig = api
 	cf.Exec.GlobalScope = true
 	assert.Equal(t, cf, filter)