Merge pull request #2341 from pwittrock/master

Fix program path in runfn

kyaml/kio/filters/functiontypes.go

@@ -4,7 +4,6 @@
 package filters
 
 import (
-	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
@@ -17,9 +16,6 @@ var functionAnnotationKeys = []string{FunctionAnnotationKey, oldFunctionAnnotati
 
 // FunctionSpec defines a spec for running a function
 type FunctionSpec struct {
-	// Path defines the path for scoped functions
-	Path string `json:"path,omitempty" yaml:"path,omitempty"`
-
 	// Network is the name of the network to use from a container
 	Network string `json:"network,omitempty" yaml:"network,omitempty"`
 
@@ -85,20 +81,16 @@ func GetFunctionSpec(n *yaml.RNode) *FunctionSpec {
 		return nil
 	}
 
-	// path to the function, this will be mounted into the container
-	path := meta.Annotations[kioutil.PathAnnotation]
 	if fn := getFunctionSpecFromAnnotation(n, meta); fn != nil {
 		fn.Network = ""
 		fn.StorageMounts = []StorageMount{}
-		fn.Path = path
 		return fn
 	}
 
 	// legacy function specification for backwards compatibility
 	container := meta.Annotations["config.kubernetes.io/container"]
 	if container != "" {
-		return &FunctionSpec{
-			Path: path, Container: ContainerSpec{Image: container}}
+		return &FunctionSpec{Container: ContainerSpec{Image: container}}
 	}
 	return nil
 }

kyaml/runfn/runfn.go

@@ -67,7 +67,7 @@ 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 filters.FunctionSpec, api *yaml.RNode) kio.Filter
+		filter filters.FunctionSpec, api *yaml.RNode) (kio.Filter, error)
 }
 
 // Execute runs the command
@@ -215,7 +215,10 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) (
 			}
 			spec.Network = r.NetworkName
 		}
-		c := r.functionFilterProvider(*spec, api)
+		c, err := r.functionFilterProvider(*spec, api)
+		if err != nil {
+			return nil, err
+		}
 		if c == nil {
 			continue
 		}
@@ -299,7 +302,7 @@ func (r *RunFns) init() {
 }
 
 // ffp provides function filters
-func (r *RunFns) ffp(spec filters.FunctionSpec, api *yaml.RNode) kio.Filter {
+func (r *RunFns) ffp(spec filters.FunctionSpec, api *yaml.RNode) (kio.Filter, error) {
 	if !r.DisableContainers && spec.Container.Image != "" {
 		return &filters.ContainerFilter{
 			Image:         spec.Container.Image,
@@ -307,14 +310,31 @@ func (r *RunFns) ffp(spec filters.FunctionSpec, api *yaml.RNode) kio.Filter {
 			Network:       spec.Network,
 			StorageMounts: r.StorageMounts,
 			GlobalScope:   r.GlobalScope,
-		}
+		}, nil
 	}
 	if r.EnableStarlark && spec.Starlark.Path != "" {
+		// the script path is relative to the function config file
+		m, err := api.GetMeta()
+		if err != nil {
+			return nil, errors.Wrap(err)
+		}
+		p := m.Annotations[kioutil.PathAnnotation]
+		spec.Starlark.Path = path.Clean(spec.Starlark.Path)
+		if path.IsAbs(spec.Starlark.Path) {
+			return nil, errors.Errorf(
+				"absolute function path %s not allowed", spec.Starlark.Path)
+		}
+		if strings.HasPrefix(spec.Starlark.Path, "..") {
+			return nil, errors.Errorf(
+				"function path %s not allowed to start with ../", spec.Starlark.Path)
+		}
+		p = path.Join(r.Path, path.Dir(p), spec.Starlark.Path)
+
 		return &starlark.Filter{
 			Name:           spec.Starlark.Name,
-			Path:           spec.Starlark.Path,
+			Path:           p,
 			FunctionConfig: api,
-		}
+		}, nil
 	}
-	return nil
+	return nil, nil
 }

kyaml/runfn/runfn_test.go

@@ -55,7 +55,7 @@ kind:
 	if !assert.NoError(t, err) {
 		return
 	}
-	filter := instance.functionFilterProvider(spec, api)
+	filter, _ := instance.functionFilterProvider(spec, api)
 	assert.Equal(t, &filters.ContainerFilter{Image: "example.com:version", Config: api}, filter)
 }
 
@@ -83,7 +83,7 @@ kind:
 	if !assert.NoError(t, err) {
 		return
 	}
-	filter := instance.functionFilterProvider(spec, api)
+	filter, _ := instance.functionFilterProvider(spec, api)
 	assert.Equal(t, &filters.ContainerFilter{
 		Image: "example.com:version", Config: api, GlobalScope: true}, filter)
 }
@@ -196,6 +196,13 @@ func TestRunFns_getFilters(t *testing.T) {
 		in []f
 		// images to be run in a specific order
 		out []string
+
+		// images to be run in a specific order -- computed from directory path
+		outFn func(string) []string
+
+		// expected Error
+		error string
+
 		// name of the test
 		name string
 		// value to set for NoFunctionsFromInput
@@ -486,7 +493,54 @@ metadata:
 				},
 			},
 			enableStarlark: true,
-			out:            []string{"name:  path: a/b/c url:  program:"},
+			outFn: func(path string) []string {
+				return []string{
+					fmt.Sprintf("name:  path: %s/foo/a/b/c url:  program:", path)}
+			},
+		},
+
+		// Test
+		//
+		//
+		{name: "starlark-function-absolute",
+			in: []f{
+				{
+					path: filepath.Join("foo", "bar.yaml"),
+					value: `
+apiVersion: example.com/v1alpha1
+kind: ExampleFunction
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      starlark:
+        path: /a/b/c
+`,
+				},
+			},
+			enableStarlark: true,
+			error:          "absolute function path /a/b/c not allowed",
+		},
+
+		// Test
+		//
+		//
+		{name: "starlark-function-escape-parent",
+			in: []f{
+				{
+					path: filepath.Join("foo", "bar.yaml"),
+					value: `
+apiVersion: example.com/v1alpha1
+kind: ExampleFunction
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      starlark:
+        path: ../a/b/c
+`,
+				},
+			},
+			enableStarlark: true,
+			error:          "function path ../a/b/c not allowed to start with ../",
 		},
 
 		{name: "starlark-function-disabled",
@@ -569,6 +623,14 @@ metadata:
 			// get the filters which would be run
 			var results []string
 			_, fltrs, _, err := r.getNodesAndFilters()
+
+			if tt.error != "" {
+				if !assert.EqualError(t, err, tt.error) {
+					t.FailNow()
+				}
+				return
+			}
+
 			if !assert.NoError(t, err) {
 				t.FailNow()
 			}
@@ -577,8 +639,14 @@ metadata:
 			}
 
 			// compare the actual ordering to the expected ordering
-			if !assert.Equal(t, tt.out, results) {
-				t.FailNow()
+			if tt.outFn != nil {
+				if !assert.Equal(t, tt.outFn(d), results) {
+					t.FailNow()
+				}
+			} else {
+				if !assert.Equal(t, tt.out, results) {
+					t.FailNow()
+				}
 			}
 		})
 	}
@@ -748,8 +816,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(filters.FunctionSpec, *yaml.RNode) kio.Filter {
-	return func(f filters.FunctionSpec, node *yaml.RNode) kio.Filter {
+func getFilterProvider(t *testing.T) func(filters.FunctionSpec, *yaml.RNode) (kio.Filter, error) {
+	return func(f filters.FunctionSpec, node *yaml.RNode) (kio.Filter, error) {
 		// parse the filter from the input
 		filter := yaml.YFilter{}
 		b := &bytes.Buffer{}
@@ -765,6 +833,6 @@ func getFilterProvider(t *testing.T) func(filters.FunctionSpec, *yaml.RNode) kio
 
 		return filters.Modifier{
 			Filters: []yaml.YFilter{{Filter: yaml.Lookup("kind")}, filter},
-		}
+		}, nil
 	}
 }