Repository: GoogleContainerTools/kpt
Pull request #2058 (2021-05-20 04:20:11)

Round trip Kptfile for consistent formatting
This PR is to add round tripping for reading and writing Kptfile as a last step for render and eval commands. This is a workaround till we integrate openapi schema of Kptfile. 

Drawback: Comments will be lost. But this problem exists with Get , Update and Live init commands. That should be fixed anyways.


Patch:
diff --git a/internal/cmdrender/executor.go b/internal/cmdrender/executor.go
index 474dd644..25c2240d 100644
--- a/internal/cmdrender/executor.go
+++ b/internal/cmdrender/executor.go
@@ -445,9 +445,6 @@ func fnChain(ctx context.Context, hctx *hydrationContext, pkgPath types.UniquePa
 
 // trackInputFiles records file paths of input resources in the hydration context.
 func trackInputFiles(hctx *hydrationContext, input []*yaml.RNode) error {
-	if err := detectPathConflicts(input); err != nil {
-		return err
-	}
 	if hctx.inputFiles == nil {
 		hctx.inputFiles = sets.String{}
 	}
@@ -464,9 +461,6 @@ func trackInputFiles(hctx *hydrationContext, input []*yaml.RNode) error {
 // trackOutputfiles records the file paths of output resources in the hydration
 // context. It should be invoked post hydration.
 func trackOutputFiles(hctx *hydrationContext) error {
-	if err := detectPathConflicts(hctx.root.resources); err != nil {
-		return err
-	}
 	outputSet := sets.String{}
 
 	for _, r := range hctx.root.resources {
@@ -480,28 +474,6 @@ func trackOutputFiles(hctx *hydrationContext) error {
 	return nil
 }
 
-// detectPathConflicts returns an error if the same index/path is on multiple resources
-func detectPathConflicts(nodes []*yaml.RNode) error {
-	// map has structure path -> index -> bool
-	// to keep track of paths and indexes found
-	pathIndexes := make(map[string]map[string]bool)
-	for _, node := range nodes {
-		fp, index, err := kioutil.GetFileAnnotations(node)
-		if err != nil {
-			return err
-		}
-		fp = path.Clean(fp)
-		if pathIndexes[fp] == nil {
-			pathIndexes[fp] = make(map[string]bool)
-		}
-		if _, ok := pathIndexes[fp][index]; ok {
-			return fmt.Errorf("resource at path %q and index %q already exists", fp, index)
-		}
-		pathIndexes[fp][index] = true
-	}
-	return nil
-}
-
 // pruneResources compares the input and output of the hydration and prunes
 // resources that are no longer present in the output of the hydration.
 func pruneResources(hctx *hydrationContext) error {
diff --git a/internal/fnruntime/runner.go b/internal/fnruntime/runner.go
index ff14e1d2..86623d78 100644
--- a/internal/fnruntime/runner.go
+++ b/internal/fnruntime/runner.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -31,6 +32,7 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/fn/framework"
 	"sigs.k8s.io/kustomize/kyaml/fn/runtime/runtimeutil"
 	"sigs.k8s.io/kustomize/kyaml/kio"
+	"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
@@ -120,6 +122,9 @@ func (fr *FunctionRunner) do(input []*yaml.RNode) (output []*yaml.RNode, err err
 	fnResult := fr.fnResult
 
 	output, err = fr.filter.Filter(input)
+	if pathErr := enforcePathInvariants(output); pathErr != nil {
+		return output, pathErr
+	}
 
 	// parse the results irrespective of the success/failure of fn exec
 	resultErr := parseStructuredResult(fr.filter.Results, fnResult)
@@ -208,6 +213,33 @@ func printFnExecErr(ctx context.Context, fnErr *ExecError) {
 	pr.OptPrintf(printOpt.Stderr(), "  Exit code: %d\n\n", fnErr.ExitCode)
 }
 
+// path (location) of a KRM resources is tracked in a special key in
+// metadata.annotation field. enforcePathInvariants throws an error if there is a path
+// to a file outside the package, or if the same index/path is on multiple resources
+func enforcePathInvariants(nodes []*yaml.RNode) error {
+	// map has structure path -> index -> bool
+	// to keep track of paths and indexes found
+	pathIndexes := make(map[string]map[string]bool)
+	for _, node := range nodes {
+		currPath, index, err := kioutil.GetFileAnnotations(node)
+		if err != nil {
+			return err
+		}
+		fp := path.Clean(currPath)
+		if strings.HasPrefix(fp, "../") {
+			return fmt.Errorf("function must not modify resources outside of package: resource has path %s", currPath)
+		}
+		if pathIndexes[fp] == nil {
+			pathIndexes[fp] = make(map[string]bool)
+		}
+		if _, ok := pathIndexes[fp][index]; ok {
+			return fmt.Errorf("resource at path %q and index %q already exists", fp, index)
+		}
+		pathIndexes[fp][index] = true
+	}
+	return nil
+}
+
 // multiLineFormatter knows how to format multiple lines in pretty format
 // that can be displayed to an end user.
 type multiLineFormatter struct {
diff --git a/internal/util/pkgutil/pkgutil.go b/internal/util/pkgutil/pkgutil.go
index 09a1d34c..ed4f4e4a 100644
--- a/internal/util/pkgutil/pkgutil.go
+++ b/internal/util/pkgutil/pkgutil.go
@@ -24,6 +24,7 @@ import (
 
 	"github.com/GoogleContainerTools/kpt/internal/pkg"
 	kptfilev1alpha2 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2"
+	"github.com/GoogleContainerTools/kpt/pkg/kptfile/kptfileutil"
 	"sigs.k8s.io/kustomize/kyaml/copyutil"
 	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/kio/filters"
@@ -293,6 +294,43 @@ func FormatPackage(pkgPath string) {
 		// do not throw error if formatting fails
 		return
 	}
+	err = RoundTripKptfilesInPkg(pkgPath)
+	if err != nil {
+		// do not throw error if formatting fails
+		return
+	}
+}
+
+// RoundTripKptfilesInPkg reads and writes all Kptfiles in the package including
+// subpackages. This is used to format Kptfiles in the order of go structures
+// TODO: phanimarupaka remove this method after addressing https://github.com/GoogleContainerTools/kpt/issues/2052
+func RoundTripKptfilesInPkg(pkgPath string) error {
+	paths, err := pkg.Subpackages(pkgPath, pkg.All, true)
+	if err != nil {
+		return err
+	}
+
+	var pkgsPaths []string
+	for _, path := range paths {
+		// join pkgPath as the paths are relative to pkgPath
+		pkgsPaths = append(pkgsPaths, filepath.Join(pkgPath, path))
+	}
+	// include root package as well
+	pkgsPaths = append(pkgsPaths, pkgPath)
+
+	for _, pkgPath := range pkgsPaths {
+		kf, err := kptfileutil.ReadFile(pkgPath)
+		if err != nil {
+			// do not throw error if formatting fails
+			return err
+		}
+		err = kptfileutil.WriteFile(pkgPath, kf)
+		if err != nil {
+			// do not throw error if formatting fails
+			return err
+		}
+	}
+	return nil
 }
 
 // Exists returns true if a file or directory exists on the provided path,


Test patch:
diff --git a/e2e/testdata/fn-eval/consistent-formatting/.expected/diff.patch b/e2e/testdata/fn-eval/consistent-formatting/.expected/diff.patch
index a8d96bdb..5b210d26 100644
--- a/e2e/testdata/fn-eval/consistent-formatting/.expected/diff.patch
+++ b/e2e/testdata/fn-eval/consistent-formatting/.expected/diff.patch
@@ -1,75 +1,42 @@
 diff --git a/Kptfile b/Kptfile
-index 74670e7..9d4cd4a 100644
+index 74670e7..154c558 100644
 --- a/Kptfile
 +++ b/Kptfile
-@@ -2,30 +2,25 @@ apiVersion: kpt.dev/v1alpha2
+@@ -2,16 +2,7 @@ apiVersion: kpt.dev/v1alpha2
  kind: Kptfile
  metadata:
    name: nginx
-+  namespace: staging
- info:
-   description: describe this package
+-info:
+-  description: describe this package
 -  site: https://github.com/GoogleContainerTools/kpt
-   emails:
-     - foo@gmail.com
+-  emails:
+-    - foo@gmail.com
 -  license: license text
-   keywords:
-     - tag1
-     - tag2
-+  license: license text
-   man: nginx man text
--upstream:
--  type: git
--  git:
--    repo: https://github.com/GoogleContainerTools/kpt
--    directory: package-examples/nginx
--    ref: v0.2
--  updateStrategy: resource-merge
--upstreamLock:
--  type: git
--  git:
--    repo: https://github.com/GoogleContainerTools/kpt
--    directory: package-examples/nginx
--    ref: package-examples/nginx/v0.2
--    commit: 4d2aa98b45ddee4b5fa45fbca16f2ff887de9efb
+-  keywords:
+-    - tag1
+-    - tag2
+-  man: nginx man text
++  namespace: staging
+ upstream:
+   type: git
+   git:
+@@ -26,6 +17,16 @@ upstreamLock:
+     directory: package-examples/nginx
+     ref: package-examples/nginx/v0.2
+     commit: 4d2aa98b45ddee4b5fa45fbca16f2ff887de9efb
++info:
 +  site: https://github.com/GoogleContainerTools/kpt
-+inventory:
-+  name: inventory-00933591
-+  namespace: some-space
-+  labels:
-+    foo: bar
-+  annotations:
-+    abc: def
-+  inventoryID: 92c234b7e9267815b0c3e17c9e4d7139a16c104f-1620493522822890000
++  emails:
++    - foo@gmail.com
++  license: license text
++  description: describe this package
++  keywords:
++    - tag1
++    - tag2
++  man: nginx man text
  pipeline:
    mutators:
      - image: gcr.io/kpt-fn/set-namespace:v0.1
-@@ -39,11 +34,17 @@ pipeline:
-       configPath: ./fn-config.yaml
-     - image: gcr.io/kpt-fn/starlark:v0.1
-       configPath: path/to/foo-star.yaml
--inventory:
--  namespace: some-space
--  name: inventory-00933591
--  inventoryID: 92c234b7e9267815b0c3e17c9e4d7139a16c104f-1620493522822890000
--  labels:
--    foo: bar
--  annotations:
--    abc: def
-+upstream:
-+  type: git
-+  updateStrategy: resource-merge
-+  git:
-+    directory: package-examples/nginx
-+    ref: v0.2
-+    repo: https://github.com/GoogleContainerTools/kpt
-+upstreamLock:
-+  type: git
-+  git:
-+    commit: 4d2aa98b45ddee4b5fa45fbca16f2ff887de9efb
-+    directory: package-examples/nginx
-+    ref: package-examples/nginx/v0.2
-+    repo: https://github.com/GoogleContainerTools/kpt
 diff --git a/resources.yaml b/resources.yaml
 index c0f48b0..9e31a48 100644
 --- a/resources.yaml
diff --git a/e2e/testdata/fn-render/path-index-ancestor/.expected/diff.patch b/e2e/testdata/fn-render/generator-absolute-path/.expected/diff.patch
similarity index 69%
rename from e2e/testdata/fn-render/path-index-ancestor/.expected/diff.patch
rename to e2e/testdata/fn-render/generator-absolute-path/.expected/diff.patch
index 10d977a8..18f40825 100644
--- a/e2e/testdata/fn-render/path-index-ancestor/.expected/diff.patch
+++ b/e2e/testdata/fn-render/generator-absolute-path/.expected/diff.patch
@@ -1,8 +1,8 @@
-diff --git a/deployment_httpbin.yaml b/deployment_httpbin.yaml
+diff --git a/another/file/out.yaml b/another/file/out.yaml
 new file mode 100644
 index 0000000..fa83ef1
 --- /dev/null
-+++ b/deployment_httpbin.yaml
++++ b/another/file/out.yaml
 @@ -0,0 +1,13 @@
 +apiVersion: apps/v1
 +kind: Deployment
@@ -16,4 +16,4 @@ index 0000000..fa83ef1
 +        - name: httpbin
 +          image: kennethreitz/httpbin
 +          ports:
-+            - containerPort: 9876
++            - containerPort: 9876
\ No newline at end of file
diff --git a/e2e/testdata/fn-render/generator-absolute-path/.krmignore b/e2e/testdata/fn-render/generator-absolute-path/.krmignore
new file mode 100644
index 00000000..9d7a4007
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-absolute-path/.krmignore
@@ -0,0 +1,1 @@
+.expected
diff --git a/e2e/testdata/fn-render/path-index-current/db/Kptfile b/e2e/testdata/fn-render/generator-absolute-path/Kptfile
similarity index 74%
rename from e2e/testdata/fn-render/path-index-current/db/Kptfile
rename to e2e/testdata/fn-render/generator-absolute-path/Kptfile
index 142ea1fd..30810450 100644
--- a/e2e/testdata/fn-render/path-index-current/db/Kptfile
+++ b/e2e/testdata/fn-render/generator-absolute-path/Kptfile
@@ -5,4 +5,4 @@ metadata:
 pipeline:
   mutators:
     - image: gcr.io/kpt-fn/starlark:unstable
-      configPath: starlark-httpbin-gen.yaml
+      configPath: starlark-httpbin.yaml
diff --git a/e2e/testdata/fn-render/path-index-current/db/resources.yaml b/e2e/testdata/fn-render/generator-absolute-path/resources.yaml
similarity index 100%
rename from e2e/testdata/fn-render/path-index-current/db/resources.yaml
rename to e2e/testdata/fn-render/generator-absolute-path/resources.yaml
diff --git a/e2e/testdata/fn-render/generator-absolute-path/starlark-httpbin.yaml b/e2e/testdata/fn-render/generator-absolute-path/starlark-httpbin.yaml
new file mode 100644
index 00000000..196d240a
--- /dev/null
+++ b/e2e/testdata/fn-render/generator-absolute-path/starlark-httpbin.yaml
@@ -0,0 +1,60 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+apiVersion: fn.kpt.dev/v1alpha1
+kind: StarlarkRun
+metadata:
+  name: httpbin-gen
+source: |-
+  httpbin_deployment = {
+     "apiVersion": "apps/v1",
+     "kind": "Deployment",
+     "metadata": {
+        "name": "httpbin",
+        "annotations": {
+          "config.kubernetes.io/index": 0,
+          # generates resource in parent directory
+          "config.kubernetes.io/path": "/another/file/out.yaml"
+        }
+     },
+     "spec": {
+        "replicas": 4,
+        "template": {
+           "spec": {
+              "containers": [
+                {
+                  "name": "httpbin",
+                  "image": "kennethreitz/httpbin",
+                  "ports": [
+                    {
+                       "containerPort": 9876
+                    }
+                  ]
+                }
+              ]
+           }
+        }
+    }
+  }
+  # filter to return if resource is HTTPBin resource
+  def is_httpbin(r):
+    return r["apiVersion"] == "apps/v1" and r["kind"] == "Deployment" and r["metadata"]["name"] == "httpbin"
+
+  def ensure_httpbin(resources):
+    for r in resources:
+      if is_httpbin(r):
+        return
+
+    resources.append(httpbin_deployment)
+
+  ensure_httpbin(ctx.resource_list["items"])
diff --git a/e2e/testdata/fn-render/inline-fnconfig/Kptfile b/e2e/testdata/fn-render/inline-fnconfig/Kptfile
index 5a2a6ef8..0f9559d2 100644
--- a/e2e/testdata/fn-render/inline-fnconfig/Kptfile
+++ b/e2e/testdata/fn-render/inline-fnconfig/Kptfile
@@ -8,7 +8,7 @@ pipeline:
       configMap:
         namespace: staging
     - image: gcr.io/kpt-fn/set-label:unstable
-      config: # inline config
+      config:
         apiVersion: v1
         kind: ConfigMap
         data:
diff --git a/e2e/testdata/fn-render/path-index-ancestor/.expected/config.yaml b/e2e/testdata/fn-render/path-index-ancestor/.expected/config.yaml
index bd487dc9..77383b91 100644
--- a/e2e/testdata/fn-render/path-index-ancestor/.expected/config.yaml
+++ b/e2e/testdata/fn-render/path-index-ancestor/.expected/config.yaml
@@ -12,8 +12,5 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# This package is nonIdempotent because functions in the subpackage (db)
-# do not have knowledge of the resources in the parent package. Therefore
-# when it runs twice, it tries to recreate the same resource in the parent,
-# resulting in a filepath conflict and throwing an error.
-nonIdempotent: true
\ No newline at end of file
+exitCode: 1
+stdErr: "Error: function must not modify resources outside of package: resource has path ../deployment_httpbin.yaml"
\ No newline at end of file
diff --git a/e2e/testdata/fn-render/path-index-current/.expected/diff.patch b/e2e/testdata/fn-render/path-index-current/.expected/diff.patch
index 6212e438..2b504552 100644
--- a/e2e/testdata/fn-render/path-index-current/.expected/diff.patch
+++ b/e2e/testdata/fn-render/path-index-current/.expected/diff.patch
@@ -1,8 +1,8 @@
-diff --git a/db/deployment_httpbin.yaml b/db/deployment_httpbin.yaml
+diff --git a/deployment_httpbin.yaml b/deployment_httpbin.yaml
 new file mode 100644
 index 0000000..fa83ef1
 --- /dev/null
-+++ b/db/deployment_httpbin.yaml
++++ b/deployment_httpbin.yaml
 @@ -0,0 +1,13 @@
 +apiVersion: apps/v1
 +kind: Deployment
diff --git a/e2e/testdata/fn-render/path-index-current/Kptfile b/e2e/testdata/fn-render/path-index-current/Kptfile
index 7b3bc56d..375117ba 100644
--- a/e2e/testdata/fn-render/path-index-current/Kptfile
+++ b/e2e/testdata/fn-render/path-index-current/Kptfile
@@ -2,3 +2,7 @@ apiVersion: kpt.dev/v1alpha2
 kind: Kptfile
 metadata:
   name: app-with-generator
+pipeline:
+  mutators:
+    - image: gcr.io/kpt-fn/starlark:unstable
+      configPath: starlark-httpbin-gen.yaml
diff --git a/e2e/testdata/fn-render/path-index-current/db/starlark-httpbin-gen.yaml b/e2e/testdata/fn-render/path-index-current/starlark-httpbin-gen.yaml
similarity index 100%
rename from e2e/testdata/fn-render/path-index-current/db/starlark-httpbin-gen.yaml
rename to e2e/testdata/fn-render/path-index-current/starlark-httpbin-gen.yaml
diff --git a/e2e/testdata/fn-render/path-index-duplicate/.expected/config.yaml b/e2e/testdata/fn-render/path-index-duplicate/.expected/config.yaml
index 08c0556e..78c04966 100644
--- a/e2e/testdata/fn-render/path-index-duplicate/.expected/config.yaml
+++ b/e2e/testdata/fn-render/path-index-duplicate/.expected/config.yaml
@@ -18,4 +18,4 @@ exitCode: 1
 stdErr: 'resource at path "resources.yaml" and index "0" already exists'
 stdOut: |
   [RUNNING] "gcr.io/kpt-fn/starlark:unstable"
-  [PASS] "gcr.io/kpt-fn/starlark:unstable"
+  [FAIL] "gcr.io/kpt-fn/starlark:unstable"
diff --git a/e2e/testdata/fn-render/path-index-outofpackage/.expected/config.yaml b/e2e/testdata/fn-render/path-index-outofpackage/.expected/config.yaml
index 54fb5f42..a2e30ce7 100644
--- a/e2e/testdata/fn-render/path-index-outofpackage/.expected/config.yaml
+++ b/e2e/testdata/fn-render/path-index-outofpackage/.expected/config.yaml
@@ -12,8 +12,5 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# This package is nonIdempotent because functions in the subpackage (db)
-# do not have knowledge of the resources outside of this package. Therefore
-# when it runs twice, it tries to recreate the same resource in the directory,
-# resulting in a filepath conflict and throwing an error.
-nonIdempotent: true
+exitCode: 1
+stdErr: "Error: function must not modify resources outside of package: resource has path ../notpkg/deployment_httpbin.yaml"
\ No newline at end of file
diff --git a/e2e/testdata/fn-render/path-index-outofpackage/.expected/diff.patch b/e2e/testdata/fn-render/path-index-outofpackage/.expected/diff.patch
deleted file mode 100644
index be184eb6..00000000
--- a/e2e/testdata/fn-render/path-index-outofpackage/.expected/diff.patch
+++ /dev/null
@@ -1,19 +0,0 @@
-diff --git a/notpkg/deployment_httpbin.yaml b/notpkg/deployment_httpbin.yaml
-new file mode 100644
-index 0000000..fa83ef1
---- /dev/null
-+++ b/notpkg/deployment_httpbin.yaml
-@@ -0,0 +1,13 @@
-+apiVersion: apps/v1
-+kind: Deployment
-+metadata:
-+  name: httpbin
-+spec:
-+  replicas: 4
-+  template:
-+    spec:
-+      containers:
-+        - name: httpbin
-+          image: kennethreitz/httpbin
-+          ports:
-+            - containerPort: 9876
diff --git a/e2e/testdata/fn-render/validate-generated-resource/Kptfile b/e2e/testdata/fn-render/validate-generated-resource/Kptfile
index 15001326..1590d625 100644
--- a/e2e/testdata/fn-render/validate-generated-resource/Kptfile
+++ b/e2e/testdata/fn-render/validate-generated-resource/Kptfile
@@ -4,8 +4,8 @@ metadata:
   name: db
 pipeline:
   mutators:
-    - image: gcr.io/kpt-fn/starlark:unstable # generates httpbin deployment
+    - image: gcr.io/kpt-fn/starlark:unstable
       configPath: starlark-httpbin-gen.yaml
   validators:
-    - image: gcr.io/kpt-fn/starlark:unstable # validates httpbin deployment exists
+    - image: gcr.io/kpt-fn/starlark:unstable
       configPath: starlark-httpbin-val.yaml
diff --git a/internal/cmdrender/executor_test.go b/internal/cmdrender/executor_test.go
deleted file mode 100644
index f4fba4e7..00000000
--- a/internal/cmdrender/executor_test.go
+++ /dev/null
@@ -1,266 +0,0 @@
-// Copyright 2021 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package cmdrender
-
-import (
-	"bytes"
-	"fmt"
-	"strings"
-	"testing"
-
-	"sigs.k8s.io/kustomize/kyaml/kio"
-)
-
-func TestDetectPathConflicts(t *testing.T) {
-	tests := map[string]struct {
-		input       string // input
-		expectedErr string // expected result
-	}{
-		"duplicate": {
-			input: `apiVersion: v1
-kind: Custom
-metadata:
-  name: a
-  annotations:
-    config.kubernetes.io/path: 'my/path/custom.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/custom.yaml'
-    config.kubernetes.io/index: '0'
-`,
-			expectedErr: `resource at path "my/path/custom.yaml" and index "0" already exists`,
-		},
-		"duplicate with `./` prefix": {
-			input: `apiVersion: v1
-kind: Custom
-metadata:
-  name: a
-  annotations:
-    config.kubernetes.io/path: 'my/path/custom.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: './my/path/custom.yaml'
-    config.kubernetes.io/index: '0'
-`,
-			expectedErr: `resource at path "my/path/custom.yaml" and index "0" already exists`,
-		},
-		"duplicate path, not index": {
-			input: `apiVersion: v1
-kind: Custom
-metadata:
-  name: a
-  annotations:
-    config.kubernetes.io/path: 'my/path/custom.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/custom.yaml'
-    config.kubernetes.io/index: '1'
-`,
-		},
-		"duplicate index, not path": {
-			input: `apiVersion: v1
-kind: Custom
-metadata:
-  name: a
-  annotations:
-    config.kubernetes.io/path: 'my/path/a.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '0'
-`,
-		},
-		"larger number of resources with duplicate": {
-			input: `apiVersion: v1
-kind: Custom
-metadata:
-  name: a
-  annotations:
-    config.kubernetes.io/path: 'my/path/a.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/a.yaml'
-    config.kubernetes.io/index: '1'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '1'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '2'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/c.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/c.yaml'
-    config.kubernetes.io/index: '1'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '1'
-`,
-			expectedErr: `resource at path "my/path/b.yaml" and index "1" already exists`,
-		},
-		"larger number of resources without duplicates": {
-			input: `apiVersion: v1
-kind: Custom
-metadata:
-  name: a
-  annotations:
-    config.kubernetes.io/path: 'my/path/a.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/a.yaml'
-    config.kubernetes.io/index: '1'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '1'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '2'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/c.yaml'
-    config.kubernetes.io/index: '0'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/c.yaml'
-    config.kubernetes.io/index: '1'
----
-apiVersion: v1
-kind: Custom
-metadata:
-  name: b
-  annotations:
-    config.kubernetes.io/path: 'my/path/b.yaml'
-    config.kubernetes.io/index: '3'
-`,
-		},
-	}
-	for _, tc := range tests {
-		out := &bytes.Buffer{}
-		r := kio.ByteReadWriter{
-			Reader:                bytes.NewBufferString(tc.input),
-			Writer:                out,
-			KeepReaderAnnotations: true,
-			OmitReaderAnnotations: true,
-		}
-		n, err := r.Read()
-		if err != nil {
-			fmt.Println("1")
-			t.FailNow()
-		}
-		err = detectPathConflicts(n)
-		if err != nil && tc.expectedErr == "" {
-			t.Errorf("unexpected error %s", err.Error())
-			t.FailNow()
-		}
-		if tc.expectedErr != "" && err == nil {
-			t.Errorf("expected error %s", tc.expectedErr)
-			t.FailNow()
-		}
-		if tc.expectedErr != "" && !strings.Contains(err.Error(), tc.expectedErr) {
-			t.Errorf("wanted error %s, got %s", tc.expectedErr, err.Error())
-			t.FailNow()
-		}
-	}
-}
diff --git a/internal/fnruntime/runner_test.go b/internal/fnruntime/runner_test.go
index 0fdc69e9..3ec3dd6f 100644
--- a/internal/fnruntime/runner_test.go
+++ b/internal/fnruntime/runner_test.go
@@ -17,14 +17,17 @@
 package fnruntime
 
 import (
+	"bytes"
 	"io/ioutil"
 	"os"
 	"path"
+	"strings"
 	"testing"
 
 	"github.com/GoogleContainerTools/kpt/internal/types"
 	kptfilev1alpha2 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2"
 	"github.com/stretchr/testify/assert"
+	"sigs.k8s.io/kustomize/kyaml/kio"
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
@@ -163,3 +166,284 @@ func TestMultilineFormatter(t *testing.T) {
 		})
 	}
 }
+
+func TestEnforcePathInvariants(t *testing.T) {
+	tests := map[string]struct {
+		input       string // input
+		expectedErr string // expected result
+	}{
+		"duplicate": {
+			input: `apiVersion: v1
+kind: Custom
+metadata:
+  name: a
+  annotations:
+    config.kubernetes.io/path: 'my/path/custom.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/custom.yaml'
+    config.kubernetes.io/index: '0'
+`,
+			expectedErr: `resource at path "my/path/custom.yaml" and index "0" already exists`,
+		},
+		"duplicate with `./` prefix": {
+			input: `apiVersion: v1
+kind: Custom
+metadata:
+  name: a
+  annotations:
+    config.kubernetes.io/path: 'my/path/custom.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: './my/path/custom.yaml'
+    config.kubernetes.io/index: '0'
+`,
+			expectedErr: `resource at path "my/path/custom.yaml" and index "0" already exists`,
+		},
+		"duplicate path, not index": {
+			input: `apiVersion: v1
+kind: Custom
+metadata:
+  name: a
+  annotations:
+    config.kubernetes.io/path: 'my/path/custom.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/custom.yaml'
+    config.kubernetes.io/index: '1'
+`,
+		},
+		"duplicate index, not path": {
+			input: `apiVersion: v1
+kind: Custom
+metadata:
+  name: a
+  annotations:
+    config.kubernetes.io/path: 'my/path/a.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '0'
+`,
+		},
+		"larger number of resources with duplicate": {
+			input: `apiVersion: v1
+kind: Custom
+metadata:
+  name: a
+  annotations:
+    config.kubernetes.io/path: 'my/path/a.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/a.yaml'
+    config.kubernetes.io/index: '1'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '1'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '2'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/c.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/c.yaml'
+    config.kubernetes.io/index: '1'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '1'
+`,
+			expectedErr: `resource at path "my/path/b.yaml" and index "1" already exists`,
+		},
+		"larger number of resources without duplicates": {
+			input: `apiVersion: v1
+kind: Custom
+metadata:
+  name: a
+  annotations:
+    config.kubernetes.io/path: 'my/path/a.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/a.yaml'
+    config.kubernetes.io/index: '1'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '1'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '2'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/c.yaml'
+    config.kubernetes.io/index: '0'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/c.yaml'
+    config.kubernetes.io/index: '1'
+---
+apiVersion: v1
+kind: Custom
+metadata:
+  name: b
+  annotations:
+    config.kubernetes.io/path: 'my/path/b.yaml'
+    config.kubernetes.io/index: '3'
+`,
+		},
+
+		"no error": {
+			input: `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: my-stateful-set
+  annotations:
+    config.kubernetes.io/path: my-stateful-set.yaml
+spec:
+  replicas: 3
+`,
+		},
+		"with ../ prefix": {
+			input: `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: my-stateful-set
+  annotations:
+    config.kubernetes.io/path: ../my-stateful-set.yaml
+spec:
+  replicas: 3
+
+`,
+			expectedErr: "function must not modify resources outside of package: resource has path ../my-stateful-set.yaml",
+		},
+		"with nested ../ in path": {
+			input: `
+apiVersion: apps/v1
+kind: StatefulSet
+metadata:
+  name: my-stateful-set
+  annotations:
+    config.kubernetes.io/path: a/b/../../../my-stateful-set.yaml
+spec:
+  replicas: 3
+`,
+			expectedErr: "function must not modify resources outside of package: resource has path a/b/../../../my-stateful-set.yaml",
+		},
+	}
+	for _, tc := range tests {
+		out := &bytes.Buffer{}
+		r := kio.ByteReadWriter{
+			Reader:                bytes.NewBufferString(tc.input),
+			Writer:                out,
+			KeepReaderAnnotations: true,
+			OmitReaderAnnotations: true,
+		}
+		n, err := r.Read()
+		if err != nil {
+			t.FailNow()
+		}
+		err = enforcePathInvariants(n)
+		if err != nil && tc.expectedErr == "" {
+			t.Errorf("unexpected error %s", err.Error())
+			t.FailNow()
+		}
+		if tc.expectedErr != "" && err == nil {
+			t.Errorf("expected error %s", tc.expectedErr)
+			t.FailNow()
+		}
+		if tc.expectedErr != "" && !strings.Contains(err.Error(), tc.expectedErr) {
+			t.Errorf("wanted error %s, got %s", tc.expectedErr, err.Error())
+			t.FailNow()
+		}
+	}
+}