From 3c255846583dc9bdb54a2f811a4fe5e9cbccb73a Mon Sep 17 00:00:00 2001
From: Morten Torkildsen <mortent@google.com>
Date: Thu, 17 Dec 2020 12:13:42 -0800
Subject: [PATCH] Multiple declarative functions in the same file should
 execute in order

---
 kyaml/runfn/runfn.go      | 32 ++++++++++++--
 kyaml/runfn/runfn_test.go | 88 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+), 4 deletions(-)

diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go
index bb669ce09..5f5481797 100644
--- a/kyaml/runfn/runfn.go
+++ b/kyaml/runfn/runfn.go
@@ -11,6 +11,7 @@ import (
 	"path"
 	"path/filepath"
 	"sort"
+	"strconv"
 	"strings"
 	"sync/atomic"
 
@@ -252,7 +253,10 @@ func (r RunFns) getFunctionsFromInput(nodes []*yaml.RNode) ([]kio.Filter, error)
 	if err != nil {
 		return nil, err
 	}
-	sortFns(buff)
+	err = sortFns(buff)
+	if err != nil {
+		return nil, err
+	}
 	return r.getFunctionFilters(false, buff.Nodes...)
 }
 
@@ -331,12 +335,33 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) (
 }
 
 // sortFns sorts functions so that functions with the longest paths come first
-func sortFns(buff *kio.PackageBuffer) {
+func sortFns(buff *kio.PackageBuffer) error {
+	var outerErr error
 	// 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 := filepath.ToSlash(mi.Annotations[kioutil.PathAnnotation])
+
+		mj, _ := buff.Nodes[j].GetMeta()
+		pj := filepath.ToSlash(mj.Annotations[kioutil.PathAnnotation])
+
+		// If the path is the same, we decide the ordering based on the
+		// index annotation.
+		if pi == pj {
+			iIndex, err := strconv.Atoi(mi.Annotations[kioutil.IndexAnnotation])
+			if err != nil {
+				outerErr = err
+				return false
+			}
+			jIndex, err := strconv.Atoi(mj.Annotations[kioutil.IndexAnnotation])
+			if err != nil {
+				outerErr = err
+				return false
+			}
+			return iIndex < jIndex
+		}
+
 		if filepath.Base(path.Dir(pi)) == "functions" {
 			// don't count the functions dir, the functions are scoped 1 level above
 			pi = filepath.Dir(path.Dir(pi))
@@ -344,8 +369,6 @@ func sortFns(buff *kio.PackageBuffer) {
 			pi = filepath.Dir(pi)
 		}
 
-		mj, _ := buff.Nodes[j].GetMeta()
-		pj := filepath.ToSlash(mj.Annotations[kioutil.PathAnnotation])
 		if filepath.Base(path.Dir(pj)) == "functions" {
 			// don't count the functions dir, the functions are scoped 1 level above
 			pj = filepath.Dir(path.Dir(pj))
@@ -374,6 +397,7 @@ func sortFns(buff *kio.PackageBuffer) {
 		// sort by path names if depths are equal
 		return pi < pj
 	})
+	return outerErr
 }
 
 // init initializes the RunFns with a containerFilterProvider.
diff --git a/kyaml/runfn/runfn_test.go b/kyaml/runfn/runfn_test.go
index 80fabc9a2..f5045977a 100644
--- a/kyaml/runfn/runfn_test.go
+++ b/kyaml/runfn/runfn_test.go
@@ -734,6 +734,94 @@ metadata:
 	}
 }
 
+func TestRunFns_sortFns(t *testing.T) {
+	testCases := []struct {
+		name           string
+		nodes          []*yaml.RNode
+		expectedImages []string
+		expectedErrMsg string
+	}{
+		{
+			name: "multiple functions in the same file are ordered by index",
+			nodes: []*yaml.RNode{
+				yaml.MustParse(`
+metadata:
+  annotations:
+    config.kubernetes.io/path: functions.yaml
+    config.kubernetes.io/index: 1
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`),
+				yaml.MustParse(`
+metadata:
+  annotations:
+    config.kubernetes.io/path: functions.yaml
+    config.kubernetes.io/index: 0
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`),
+			},
+			expectedImages: []string{"b", "a"},
+		},
+		{
+			name: "non-integer value in index annotation is an error",
+			nodes: []*yaml.RNode{
+				yaml.MustParse(`
+metadata:
+  annotations:
+    config.kubernetes.io/path: functions.yaml
+    config.kubernetes.io/index: 0
+    config.kubernetes.io/function: |
+      container:
+        image: a
+`),
+				yaml.MustParse(`
+metadata:
+  annotations:
+    config.kubernetes.io/path: functions.yaml
+    config.kubernetes.io/index: abc
+    config.kubernetes.io/function: |
+      container:
+        image: b
+`),
+			},
+			expectedErrMsg: "strconv.Atoi: parsing \"abc\": invalid syntax",
+		},
+	}
+
+	for i := range testCases {
+		test := testCases[i]
+		t.Run(test.name, func(t *testing.T) {
+			packageBuff := &kio.PackageBuffer{
+				Nodes: test.nodes,
+			}
+
+			err := sortFns(packageBuff)
+			if test.expectedErrMsg != "" {
+				if !assert.Error(t, err) {
+					t.FailNow()
+				}
+				assert.Equal(t, test.expectedErrMsg, err.Error())
+				return
+			}
+
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			var images []string
+			for _, n := range packageBuff.Nodes {
+				spec := runtimeutil.GetFunctionSpec(n)
+				images = append(images, spec.Container.Image)
+			}
+
+			assert.Equal(t, test.expectedImages, images)
+		})
+	}
+}
+
 func TestRunFns_network(t *testing.T) {
 	tests := []struct {
 		name          string