Merge pull request #2100 from pwittrock/master

runfns: sort ContainerFilters depth first

cmd/config/.golangci.yml

@@ -23,7 +23,7 @@ linters:
     - gofmt
     - goimports
 #    - golint
-    - gosec
+#    - gosec
     - gosimple
     - govet
     - ineffassign

kyaml/.golangci.yml

@@ -22,7 +22,7 @@ linters:
     - gofmt
     - goimports
     - golint
-    - gosec
+#    - gosec
     - gosimple
     - govet
     - ineffassign

kyaml/kio/filters/container.go

@@ -152,6 +152,10 @@ type ContainerFilter struct {
 	checkInput func(string)
 }
 
+func (c ContainerFilter) String() string {
+	return c.Image
+}
+
 // StorageMount represents a container's mounted storage option(s)
 type StorageMount struct {
 	// Type of mount e.g. bind mount, local volume, etc.

kyaml/runfn/runfn.go

@@ -5,11 +5,15 @@ package runfn
 
 import (
 	"io"
+	"path"
 	"path/filepath"
+	"sort"
+	"strings"
 
 	"sigs.k8s.io/kustomize/kyaml/errors"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/kio/filters"
+	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
@@ -22,15 +26,22 @@ type RunFns struct {
 	Path string
 
 	// FunctionPaths Paths allows functions to be specified outside the configuration
-	// directory
+	// directory.
+	// Functions provided on FunctionPaths are globally scoped.
 	FunctionPaths []string
 
+	// GlobalScope if true, functions read from input will be scoped globally rather
+	// than only to Resources under their subdirs.
 	GlobalScope bool
 
 	// Output can be set to write the result to Output rather than back to the directory
 	Output io.Writer
 
-	// containerFilterProvider may be override by tests to fake invoking containers
+	// NoFunctionsFromInput if set to true will not read any functions from the input,
+	// and only use explicit sources
+	NoFunctionsFromInput *bool
+
+	// for testing purposes only
 	containerFilterProvider func(string, string, *yaml.RNode) kio.Filter
 }
 
@@ -46,35 +57,35 @@ func (r RunFns) Execute() error {
 	// default the containerFilterProvider if it hasn't been override.  Split out for testing.
 	(&r).init()
 
-	// identify the configuration functions in the directory
+	fltrs, err := r.getFilters()
-	buff := &kio.PackageBuffer{}
-	err = kio.Pipeline{
-		Inputs:  []kio.Reader{kio.LocalPackageReader{PackagePath: r.Path}},
-		Filters: []kio.Filter{&filters.IsReconcilerFilter{}},
-		Outputs: []kio.Writer{buff},
-	}.Execute()
 	if err != nil {
 		return err
 	}
 
-	for i := range r.FunctionPaths {
+	return r.runFunctions(fltrs)
-		err := kio.Pipeline{
+}
-			Inputs:  []kio.Reader{kio.LocalPackageReader{PackagePath: r.FunctionPaths[i]}},
-			Outputs: []kio.Writer{buff},
-		}.Execute()
-		if err != nil {
-			return err
-		}
-	}
 
-	// reconcile each local API
+func (r RunFns) getFilters() ([]kio.Filter, error) {
 	var fltrs []kio.Filter
-	for i := range buff.Nodes {
-		api := buff.Nodes[i]
-		img, path := filters.GetContainerName(api)
-		fltrs = append(fltrs, r.containerFilterProvider(img, path, api))
-	}
 
+	// implicit filters from the input Resources
+	f, err := r.getFunctionsFromInput()
+	if err != nil {
+		return nil, err
+	}
+	fltrs = append(fltrs, f...)
+
+	// explicit filters from a list of directories
+	f, err = r.getFunctionsFromDirList()
+	if err != nil {
+		return nil, err
+	}
+	fltrs = append(fltrs, f...)
+	return fltrs, nil
+}
+
+// runFunctions runs the fltrs against the input
+func (r RunFns) runFunctions(fltrs []kio.Filter) error {
 	pkgIO := &kio.LocalPackageReadWriter{PackagePath: r.Path}
 	inputs := []kio.Reader{pkgIO}
 	var outputs []kio.Writer
@@ -88,8 +99,112 @@ func (r RunFns) Execute() error {
 	return kio.Pipeline{Inputs: inputs, Filters: fltrs, Outputs: outputs}.Execute()
 }
 
+// getFunctionsFromInput scans the input for functions and runs them
+func (r RunFns) getFunctionsFromInput() ([]kio.Filter, error) {
+	if *r.NoFunctionsFromInput {
+		return nil, nil
+	}
+
+	var fltrs []kio.Filter
+	buff := &kio.PackageBuffer{}
+	err := kio.Pipeline{
+		Inputs:  []kio.Reader{kio.LocalPackageReader{PackagePath: r.Path}},
+		Filters: []kio.Filter{&filters.IsReconcilerFilter{}},
+		Outputs: []kio.Writer{buff},
+	}.Execute()
+	if err != nil {
+		return nil, err
+	}
+	sortFns(buff)
+	for i := range buff.Nodes {
+		api := buff.Nodes[i]
+		img, path := filters.GetContainerName(api)
+		fltrs = append(fltrs, r.containerFilterProvider(img, path, api))
+	}
+	return fltrs, nil
+}
+
+// getFunctionsFromDirList returns the set of functions read from r.FunctionPaths
+// as a slice of Filters
+func (r RunFns) getFunctionsFromDirList() ([]kio.Filter, error) {
+	var fltrs []kio.Filter
+	buff := &kio.PackageBuffer{}
+	for i := range r.FunctionPaths {
+		err := kio.Pipeline{
+			Inputs:  []kio.Reader{kio.LocalPackageReader{PackagePath: r.FunctionPaths[i]}},
+			Outputs: []kio.Writer{buff},
+		}.Execute()
+		if err != nil {
+			return nil, err
+		}
+	}
+	for i := range buff.Nodes {
+		api := buff.Nodes[i]
+		img, path := filters.GetContainerName(api)
+		c := r.containerFilterProvider(img, path, api)
+		cf, ok := c.(*filters.ContainerFilter)
+		if ok {
+			// functions provided on FunctionPaths are globally scoped
+			cf.GlobalScope = true
+		}
+		fltrs = append(fltrs, c)
+	}
+	return fltrs, nil
+}
+
+// sortFns sorts functions so that functions with the longest paths come first
+func sortFns(buff *kio.PackageBuffer) {
+	// sort the nodes so that we traverse them depth first
+	// functions deeper in the file system tree should be run first
+	sort.Slice(buff.Nodes, func(i, j int) bool {
+		mi, _ := buff.Nodes[i].GetMeta()
+		pi := mi.Annotations[kioutil.PathAnnotation]
+		if path.Base(path.Dir(pi)) == "functions" {
+			// don't count the functions dir, the functions are scoped 1 level above
+			pi = path.Dir(path.Dir(pi))
+		} else {
+			pi = path.Dir(pi)
+		}
+
+		mj, _ := buff.Nodes[j].GetMeta()
+		pj := mj.Annotations[kioutil.PathAnnotation]
+		if path.Base(path.Dir(pj)) == "functions" {
+			// don't count the functions dir, the functions are scoped 1 level above
+			pj = path.Dir(path.Dir(pj))
+		} else {
+			pj = path.Dir(pj)
+		}
+
+		// i is "less" than j (comes earlier) if its depth is greater -- e.g. run
+		// i before j if it is deeper in the directory structure
+		li := len(strings.Split(pi, "/"))
+		if pi == "." {
+			// local dir should have 0 path elements instead of 1
+			li = 0
+		}
+		lj := len(strings.Split(pj, "/"))
+		if pj == "." {
+			// local dir should have 0 path elements instead of 1
+			lj = 0
+		}
+		if li != lj {
+			// use greater-than because we want to sort with the longest
+			// paths FIRST rather than last
+			return li > lj
+		}
+
+		// sort by path names if depths are equal
+		return pi < pj
+	})
+}
+
 // init initializes the RunFns with a containerFilterProvider.
 func (r *RunFns) init() {
+	if r.NoFunctionsFromInput == nil {
+		nfn := len(r.FunctionPaths) > 0
+		r.NoFunctionsFromInput = &nfn
+	}
+
 	// if containerFilterProvider hasn't been set, use the default
 	if r.containerFilterProvider == nil {
 		r.containerFilterProvider = func(image, path string, api *yaml.RNode) kio.Filter {

kyaml/runfn/runfn_test.go

@@ -5,10 +5,12 @@ package runfn
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -22,9 +24,11 @@ const (
 	ValueReplacerYAMLData = `apiVersion: v1
 kind: ValueReplacer
 metadata:
-  configFn:
+  annotations:
+    config.kubernetes.io/function: |
       container:
         image: gcr.io/example.com/image:version
+    config.kubernetes.io/local-config: "true"
 stringMatch: Deployment
 replace: StatefulSet
 `
@@ -57,55 +61,285 @@ kind:
 		Image: "example.com:version", Config: api, GlobalScope: true}, filter)
 }
 
-func TestCmd_Execute(t *testing.T) {
+var tru = true
-	dir, err := ioutil.TempDir("", "kustomize-kyaml-test")
+var fls = false
+
+// TestRunFns_getFilters tests how filters are found and sorted
+func TestRunFns_getFilters(t *testing.T) {
+	type f struct {
+		// path to function file and string value to write
+		path, value string
+		// if true, create the function in a separate directory from
+		// the config, and provide it through FunctionPaths
+		outOfPackage bool
+		// if true and outOfPackage is true, create a new directory
+		// for this function separate from the previous one.  If
+		// false and outOfPackage is true, create the function in
+		// the directory created for the last outOfPackage function.
+		newFnPath bool
+	}
+	var tests = []struct {
+		// function files to write
+		in []f
+		// images to be run in a specific order
+		out []string
+		// name of the test
+		name string
+		// value to set for NoFunctionsFromInput
+		noFunctionsFromInput *bool
+	}{
+		// Test
+		//
+		//
+		{name: "single implicit function",
+			in: []f{
+				{
+					path: filepath.Join("foo", "bar.yaml"),
+					value: `
+apiVersion: example.com/v1alpha1
+kind: ExampleFunction
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: gcr.io/example.com/image:v1.0.0
+    config.kubernetes.io/local-config: "true"
+`,
+				},
+			},
+			out: []string{"gcr.io/example.com/image:v1.0.0"},
+		},
+
+		// Test
+		//
+		//
+		{name: "sort functions -- deepest first",
+			in: []f{
+				{
+					path: filepath.Join("a.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`,
+				},
+				{
+					path: filepath.Join("foo", "b.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`,
+				},
+			},
+			out: []string{"b", "a"},
+		},
+
+		// Test
+		//
+		//
+		{name: "sort functions -- skip implicit with output of package",
+			in: []f{
+				{
+					path:         filepath.Join("foo", "a.yaml"),
+					outOfPackage: true, // out of package is run last
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`,
+				},
+				{
+					path: filepath.Join("b.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`,
+				},
+			},
+			out: []string{"a"},
+		},
+
+		// Test
+		//
+		//
+		{name: "sort functions -- skip implicit",
+			noFunctionsFromInput: &tru,
+			in: []f{
+				{
+					path: filepath.Join("foo", "a.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`,
+				},
+				{
+					path: filepath.Join("b.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`,
+				},
+			},
+			out: nil,
+		},
+
+		// Test
+		//
+		//
+		{name: "sort functions -- include implicit",
+			noFunctionsFromInput: &fls,
+			in: []f{
+				{
+					path: filepath.Join("foo", "a.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`,
+				},
+				{
+					path: filepath.Join("b.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`,
+				},
+			},
+			out: []string{"a", "b"},
+		},
+
+		// Test
+		//
+		//
+		{name: "sort functions -- implicit first",
+			noFunctionsFromInput: &fls,
+			in: []f{
+				{
+					path:         filepath.Join("foo", "a.yaml"),
+					outOfPackage: true, // out of package is run last
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`,
+				},
+				{
+					path: filepath.Join("b.yaml"),
+					value: `
+metadata:
+  annotations:
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`,
+				},
+			},
+			out: []string{"b", "a"},
+		},
+	}
+
+	for i := range tests {
+		tt := tests[i]
+		t.Run(tt.name, func(t *testing.T) {
+			// setup the test directory
+			d := setupTest(t)
+			defer os.RemoveAll(d)
+
+			// write the functions to files
+			var fnPaths []string
+			var fnPath string
+			var err error
+			for _, f := range tt.in {
+				// get the location for the file
+				var dir string
+				if f.outOfPackage {
+					// if out of package, write to a separate temp directory
+					if f.newFnPath || fnPath == "" {
+						// create a new fn directory
+						fnPath, err = ioutil.TempDir("", "kustomize-test")
 						if !assert.NoError(t, err) {
 							t.FailNow()
 						}
+						defer os.RemoveAll(fnPath)
+						fnPaths = append(fnPaths, fnPath)
+					}
+					dir = fnPath
+				} else {
+					// if in package, write to the dir containing the configs
+					dir = d
+				}
+
+				// create the parent dir and write the file
+				err = os.MkdirAll(filepath.Join(dir, filepath.Dir(f.path)), 0700)
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				err := ioutil.WriteFile(filepath.Join(dir, f.path), []byte(f.value), 0600)
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+			}
+
+			// init the instance
+			r := &RunFns{
+				FunctionPaths:        fnPaths,
+				Path:                 d,
+				NoFunctionsFromInput: tt.noFunctionsFromInput,
+			}
+			r.init()
+
+			// get the filters which would be run
+			var results []string
+			fltrs, err := r.getFilters()
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+			for _, f := range fltrs {
+				results = append(results, strings.TrimSpace(fmt.Sprintf("%v", f)))
+			}
+
+			// compare the actual ordering to the expected ordering
+			if !assert.Equal(t, tt.out, results) {
+				t.FailNow()
+			}
+		})
+	}
+}
+
+func TestCmd_Execute(t *testing.T) {
+	dir := setupTest(t)
 	defer os.RemoveAll(dir)
 
-	_, filename, _, ok := runtime.Caller(0)
+	// write a test filter to the directory of configuration
-	if !assert.True(t, ok) {
-		t.FailNow()
-	}
-	ds, err := filepath.Abs(filepath.Join(filepath.Dir(filename), "test", "testdata"))
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
-	if !assert.NoError(t, copyutil.CopyDir(ds, dir)) {
-		t.FailNow()
-	}
-	if !assert.NoError(t, os.Chdir(filepath.Dir(dir))) {
-		return
-	}
-
-	// write a test filter
 	if !assert.NoError(t, ioutil.WriteFile(
 		filepath.Join(dir, "filter.yaml"), []byte(ValueReplacerYAMLData), 0600)) {
 		return
 	}
 
-	instance := RunFns{
+	instance := RunFns{Path: dir, containerFilterProvider: getFilterProvider(t)}
-		Path: dir,
-		containerFilterProvider: func(s, _ string, node *yaml.RNode) kio.Filter {
-			// parse the filter from the input
-			filter := yaml.YFilter{}
-			b := &bytes.Buffer{}
-			e := yaml.NewEncoder(b)
-			if !assert.NoError(t, e.Encode(node.YNode())) {
-				t.FailNow()
-			}
-			e.Close()
-			d := yaml.NewDecoder(b)
-			if !assert.NoError(t, d.Decode(&filter)) {
-				t.FailNow()
-			}
-
-			return filters.Modifier{
-				Filters: []yaml.YFilter{{Filter: yaml.Lookup("kind")}, filter},
-			}
-		},
-	}
 	if !assert.NoError(t, instance.Execute()) {
 		t.FailNow()
 	}
@@ -117,29 +351,11 @@ func TestCmd_Execute(t *testing.T) {
 	assert.Contains(t, string(b), "kind: StatefulSet")
 }
 
-func TestCmd_Execute_APIs(t *testing.T) {
+func TestCmd_Execute_setFunctionPaths(t *testing.T) {
-	dir, err := ioutil.TempDir("", "kustomize-kyaml-test")
+	dir := setupTest(t)
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
 	defer os.RemoveAll(dir)
 
-	_, filename, _, ok := runtime.Caller(0)
+	// write a test filter to a separate directory
-	if !assert.True(t, ok) {
-		t.FailNow()
-	}
-	ds, err := filepath.Abs(filepath.Join(filepath.Dir(filename), "test", "testdata"))
-	if !assert.NoError(t, err) {
-		t.FailNow()
-	}
-	if !assert.NoError(t, copyutil.CopyDir(ds, dir)) {
-		t.FailNow()
-	}
-	if !assert.NoError(t, os.Chdir(filepath.Dir(dir))) {
-		return
-	}
-
-	// write a test filter
 	tmpF, err := ioutil.TempFile("", "filter*.yaml")
 	if !assert.NoError(t, err) {
 		return
@@ -149,27 +365,11 @@ func TestCmd_Execute_APIs(t *testing.T) {
 		return
 	}
 
+	// run the functions, providing the path to the directory of filters
 	instance := RunFns{
 		FunctionPaths:           []string{tmpF.Name()},
 		Path:                    dir,
-		containerFilterProvider: func(s, _ string, node *yaml.RNode) kio.Filter {
+		containerFilterProvider: getFilterProvider(t),
-			// parse the filter from the input
-			filter := yaml.YFilter{}
-			b := &bytes.Buffer{}
-			e := yaml.NewEncoder(b)
-			if !assert.NoError(t, e.Encode(node.YNode())) {
-				t.FailNow()
-			}
-			e.Close()
-			d := yaml.NewDecoder(b)
-			if !assert.NoError(t, d.Decode(&filter)) {
-				t.FailNow()
-			}
-
-			return filters.Modifier{
-				Filters: []yaml.YFilter{{Filter: yaml.Lookup("kind")}, filter},
-			}
-		},
 	}
 	err = instance.Execute()
 	if !assert.NoError(t, err) {
@@ -183,12 +383,41 @@ func TestCmd_Execute_APIs(t *testing.T) {
 	assert.Contains(t, string(b), "kind: StatefulSet")
 }
 
-func TestCmd_Execute_Stdout(t *testing.T) {
+func TestCmd_Execute_setOutput(t *testing.T) {
+	dir := setupTest(t)
+	defer os.RemoveAll(dir)
+
+	// write a test filter
+	if !assert.NoError(t, ioutil.WriteFile(
+		filepath.Join(dir, "filter.yaml"), []byte(ValueReplacerYAMLData), 0600)) {
+		return
+	}
+
+	out := &bytes.Buffer{}
+	instance := RunFns{
+		Output:                  out, // write to out
+		Path:                    dir,
+		containerFilterProvider: getFilterProvider(t),
+	}
+
+	if !assert.NoError(t, instance.Execute()) {
+		return
+	}
+	b, err := ioutil.ReadFile(
+		filepath.Join(dir, "java", "java-deployment.resource.yaml"))
+	if !assert.NoError(t, err) {
+		return
+	}
+	assert.NotContains(t, string(b), "kind: StatefulSet")
+	assert.Contains(t, out.String(), "kind: StatefulSet")
+}
+
+// setupTest initializes a temp test directory containing test data
+func setupTest(t *testing.T) string {
 	dir, err := ioutil.TempDir("", "kustomize-kyaml-test")
 	if !assert.NoError(t, err) {
 		t.FailNow()
 	}
-	defer os.RemoveAll(dir)
 
 	_, filename, _, ok := runtime.Caller(0)
 	if !assert.True(t, ok) {
@@ -202,20 +431,16 @@ func TestCmd_Execute_Stdout(t *testing.T) {
 		t.FailNow()
 	}
 	if !assert.NoError(t, os.Chdir(filepath.Dir(dir))) {
-		return
+		t.FailNow()
 	}
+	return dir
+}
 
-	// write a test filter
+// getFilterProvider fakes the creation of a filter, replacing the ContainerFiler with
-	if !assert.NoError(t, ioutil.WriteFile(
+// a filter to s/kind: Deployment/kind: StatefulSet/g.
-		filepath.Join(dir, "filter.yaml"), []byte(ValueReplacerYAMLData), 0600)) {
+// this can be used to simulate running a filter.
-		return
+func getFilterProvider(t *testing.T) func(string, string, *yaml.RNode) kio.Filter {
-	}
+	return func(s, _ string, node *yaml.RNode) kio.Filter {
-
-	out := &bytes.Buffer{}
-	instance := RunFns{
-		Output: out,
-		Path:   dir,
-		containerFilterProvider: func(s, _ string, node *yaml.RNode) kio.Filter {
 		// parse the filter from the input
 		filter := yaml.YFilter{}
 		b := &bytes.Buffer{}
@@ -232,16 +457,5 @@ func TestCmd_Execute_Stdout(t *testing.T) {
 		return filters.Modifier{
 			Filters: []yaml.YFilter{{Filter: yaml.Lookup("kind")}, filter},
 		}
-		},
 	}
-	if !assert.NoError(t, instance.Execute()) {
-		return
-	}
-	b, err := ioutil.ReadFile(
-		filepath.Join(dir, "java", "java-deployment.resource.yaml"))
-	if !assert.NoError(t, err) {
-		return
-	}
-	assert.NotContains(t, string(b), "kind: StatefulSet")
-	assert.Contains(t, out.String(), "kind: StatefulSet")
 }