Repository: GoogleContainerTools/kpt (go)

Issue:
fn render invariant: functions should not be able to modify resources outside of their current package
When running `kpt fn render`, a function in a package should only be able to modify and generate resources within the current package. If it tries to write outside of its package, kpt should fail with error.

i.e. fn-render/path-index-ancestor and fn-render/path-index-outofpkg should fail with exit code 1. 

cc @frankfarzan @droot @Shell32-Natsu 

ref: https://github.com/GoogleContainerTools/kpt/issues/1559

Pull request:
enforce `fn render` path invariants
close #1958 

This PR does 4 things: 

1. If a function in a pipeline tries to write to a file outside of its current package, `kpt fn render` will throw the error: 

`function must not modify resources outside of package: resource has path <filepath>`

2. Moves detection of path conflicts to the `fnruntime` package

3. Adds a test `generator-absolute-path` to show current behavior of a path beginning with `/` (it is treated as a relative path)  

4. Updates the test `path-index-current` to be run without a subpackage.

Patch:
diff --git a/commands/livecmd.go b/commands/livecmd.go
index 700fd26f..5123bd9c 100644
--- a/commands/livecmd.go
+++ b/commands/livecmd.go
@@ -21,12 +21,12 @@ import (
 	"os"
 
 	"github.com/GoogleContainerTools/kpt/internal/cmdapply"
+	"github.com/GoogleContainerTools/kpt/internal/cmddestroy"
 	"github.com/GoogleContainerTools/kpt/internal/cmdliveinit"
 	"github.com/GoogleContainerTools/kpt/internal/cmdmigrate"
 	"github.com/GoogleContainerTools/kpt/internal/docs/generated/livedocs"
 	"github.com/GoogleContainerTools/kpt/internal/util/cfgflags"
 	"github.com/GoogleContainerTools/kpt/pkg/live"
-	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/destroy"
 	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/diff"
 	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/preview"
 	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/status"
@@ -92,10 +92,7 @@ func GetLiveCommand(ctx context.Context, _, version string) *cobra.Command {
 	diffCmd.Long = livedocs.DiffShort + "\n" + livedocs.DiffLong
 	diffCmd.Example = livedocs.DiffExamples
 
-	destroyCmd := destroy.GetDestroyRunner(dp, dl, ioStreams).Command
-	destroyCmd.Short = livedocs.DestroyShort
-	destroyCmd.Long = livedocs.DestroyShort + "\n" + livedocs.DestroyLong
-	destroyCmd.Example = livedocs.DestroyExamples
+	destroyCmd := cmddestroy.NewCommand(ctx, rgProvider, rgLoader, ioStreams)
 
 	statusCmd := status.GetStatusRunner(dp, dl).Command
 	statusCmd.Short = livedocs.StatusShort
diff --git a/internal/cmddestroy/cmddestroy.go b/internal/cmddestroy/cmddestroy.go
new file mode 100644
index 00000000..579b6c5a
--- /dev/null
+++ b/internal/cmddestroy/cmddestroy.go
@@ -0,0 +1,160 @@
+// 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 cmddestroy
+
+import (
+	"context"
+	"fmt"
+	"os"
+
+	"github.com/GoogleContainerTools/kpt/internal/cmdutil"
+	"github.com/GoogleContainerTools/kpt/internal/docs/generated/livedocs"
+	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/flagutils"
+	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/printers"
+	"github.com/spf13/cobra"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	"sigs.k8s.io/cli-utils/pkg/apply"
+	"sigs.k8s.io/cli-utils/pkg/common"
+	"sigs.k8s.io/cli-utils/pkg/inventory"
+	"sigs.k8s.io/cli-utils/pkg/manifestreader"
+	"sigs.k8s.io/cli-utils/pkg/provider"
+)
+
+func NewRunner(ctx context.Context, provider provider.Provider, loader manifestreader.ManifestLoader,
+	ioStreams genericclioptions.IOStreams) *Runner {
+
+	r := &Runner{
+		ctx:           ctx,
+		Destroyer:     apply.NewDestroyer(provider),
+		provider:      provider,
+		loader:        loader,
+		ioStreams:     ioStreams,
+		destroyRunner: runDestroy,
+	}
+	c := &cobra.Command{
+		Use:     "destroy [PKG_PATH | -]",
+		RunE:    r.runE,
+		PreRunE: r.preRunE,
+		Short:   livedocs.DestroyShort,
+		Long:    livedocs.DestroyShort + "\n" + livedocs.DestroyLong,
+		Example: livedocs.DestroyExamples,
+	}
+	r.Command = c
+
+	c.Flags().StringVar(&r.output, "output", printers.DefaultPrinter(),
+		fmt.Sprintf("Output format, must be one of %s", cmdutil.JoinStringsWithQuotes(printers.SupportedPrinters())))
+	c.Flags().StringVar(&r.inventoryPolicyString, flagutils.InventoryPolicyFlag, flagutils.InventoryPolicyStrict,
+		"It determines the behavior when the resources don't belong to current inventory. Available options "+
+			fmt.Sprintf("%q and %q.", flagutils.InventoryPolicyStrict, flagutils.InventoryPolicyAdopt))
+	return r
+}
+
+// NewCommand returns a cobra command.
+func NewCommand(ctx context.Context, provider provider.Provider, loader manifestreader.ManifestLoader,
+	ioStreams genericclioptions.IOStreams) *cobra.Command {
+	return NewRunner(ctx, provider, loader, ioStreams).Command
+}
+
+// Runner contains the run function that contains the cli functionality for the
+// destroy command.
+type Runner struct {
+	ctx        context.Context
+	Command    *cobra.Command
+	PreProcess func(info inventory.InventoryInfo, strategy common.DryRunStrategy) (inventory.InventoryPolicy, error)
+	Destroyer  *apply.Destroyer
+	provider   provider.Provider
+	loader     manifestreader.ManifestLoader
+	ioStreams  genericclioptions.IOStreams
+
+	output                string
+	inventoryPolicyString string
+
+	inventoryPolicy inventory.InventoryPolicy
+
+	// TODO(mortent): This is needed for now since we don't have a good way to
+	// stub out the Destroyer with an interface for testing purposes.
+	destroyRunner func(r *Runner, inv inventory.InventoryInfo) error
+}
+
+// preRunE validates the inventoryPolicy and the output type.
+func (r *Runner) preRunE(_ *cobra.Command, _ []string) error {
+	var err error
+	r.inventoryPolicy, err = flagutils.ConvertInventoryPolicy(r.inventoryPolicyString)
+	if err != nil {
+		return err
+	}
+
+	if found := printers.ValidatePrinterType(r.output); !found {
+		return fmt.Errorf("unknown output type %q", r.output)
+	}
+
+	return nil
+}
+
+// runE handles the input flags and args, sets up the Destroyer, and
+// invokes the
+func (r *Runner) runE(c *cobra.Command, args []string) error {
+	if len(args) == 0 {
+		// default to the current working directory
+		cwd, err := os.Getwd()
+		if err != nil {
+			return err
+		}
+		args = append(args, cwd)
+	}
+
+	reader, err := r.loader.ManifestReader(c.InOrStdin(), args[0])
+	if err != nil {
+		return err
+	}
+
+	objs, err := reader.Read()
+	if err != nil {
+		return err
+	}
+
+	inv, _, err := r.loader.InventoryInfo(objs)
+	if err != nil {
+		return err
+	}
+
+	// TODO(mortent): Figure out if we can do this differently.
+	if r.PreProcess != nil {
+		r.inventoryPolicy, err = r.PreProcess(inv, r.Destroyer.DryRunStrategy)
+		if err != nil {
+			return err
+		}
+	}
+
+	return r.destroyRunner(r, inv)
+}
+
+func runDestroy(r *Runner, inv inventory.InventoryInfo) error {
+	// Run the destroyer. It will return a channel where we can receive updates
+	// to keep track of progress and any issues.
+	err := r.Destroyer.Initialize()
+	if err != nil {
+		return err
+	}
+	option := &apply.DestroyerOption{
+		InventoryPolicy: r.inventoryPolicy,
+	}
+	ch := r.Destroyer.Run(inv, option)
+
+	// The printer will print updates from the channel. It will block
+	// until the channel is closed.
+	printer := printers.GetPrinter(r.output, r.ioStreams)
+	return printer.Print(ch, r.Destroyer.DryRunStrategy)
+}
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/thirdparty/cli-utils/destroy/cmddestroy.go b/thirdparty/cli-utils/destroy/cmddestroy.go
deleted file mode 100644
index c647cb64..00000000
--- a/thirdparty/cli-utils/destroy/cmddestroy.go
+++ /dev/null
@@ -1,114 +0,0 @@
-// Copyright 2020 The Kubernetes Authors.
-// SPDX-License-Identifier: Apache-2.0
-
-package destroy
-
-import (
-	"fmt"
-	"strings"
-
-	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/flagutils"
-	"github.com/GoogleContainerTools/kpt/thirdparty/cli-utils/printers"
-	"github.com/spf13/cobra"
-	"k8s.io/cli-runtime/pkg/genericclioptions"
-	cmdutil "k8s.io/kubectl/pkg/cmd/util"
-	"k8s.io/kubectl/pkg/util/i18n"
-	"sigs.k8s.io/cli-utils/pkg/apply"
-	"sigs.k8s.io/cli-utils/pkg/common"
-	"sigs.k8s.io/cli-utils/pkg/inventory"
-	"sigs.k8s.io/cli-utils/pkg/manifestreader"
-	"sigs.k8s.io/cli-utils/pkg/provider"
-)
-
-// GetDestroyRunner creates and returns the DestroyRunner which stores the cobra command.
-func GetDestroyRunner(provider provider.Provider, loader manifestreader.ManifestLoader, ioStreams genericclioptions.IOStreams) *DestroyRunner {
-	r := &DestroyRunner{
-		Destroyer: apply.NewDestroyer(provider),
-		ioStreams: ioStreams,
-		provider:  provider,
-		loader:    loader,
-	}
-	cmd := &cobra.Command{
-		Use:                   "destroy [PKG_PATH | -]",
-		DisableFlagsInUseLine: true,
-		Short:                 i18n.T("Destroy all the resources related to configuration"),
-		RunE:                  r.RunE,
-	}
-
-	cmd.Flags().StringVar(&r.output, "output", printers.DefaultPrinter(),
-		fmt.Sprintf("Output format, must be one of %s", strings.Join(printers.SupportedPrinters(), ",")))
-	cmd.Flags().StringVar(&r.inventoryPolicy, flagutils.InventoryPolicyFlag, flagutils.InventoryPolicyStrict,
-		"It determines the behavior when the resources don't belong to current inventory. Available options "+
-			fmt.Sprintf("%q and %q.", flagutils.InventoryPolicyStrict, flagutils.InventoryPolicyAdopt))
-
-	r.Command = cmd
-	return r
-}
-
-// DestroyCommand creates the DestroyRunner, returning the cobra command associated with it.
-func DestroyCommand(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *cobra.Command {
-	provider := provider.NewProvider(f)
-	loader := manifestreader.NewManifestLoader(f)
-	return GetDestroyRunner(provider, loader, ioStreams).Command
-}
-
-// DestroyRunner encapsulates data necessary to run the destroy command.
-type DestroyRunner struct {
-	Command    *cobra.Command
-	PreProcess func(info inventory.InventoryInfo, strategy common.DryRunStrategy) (inventory.InventoryPolicy, error)
-	ioStreams  genericclioptions.IOStreams
-	Destroyer  *apply.Destroyer
-	provider   provider.Provider
-	loader     manifestreader.ManifestLoader
-
-	output          string
-	inventoryPolicy string
-}
-
-func (r *DestroyRunner) RunE(cmd *cobra.Command, args []string) error {
-	if len(args) == 0 {
-		// default to the current working directory
-		args = append(args, ".")
-	}
-	inventoryPolicy, err := flagutils.ConvertInventoryPolicy(r.inventoryPolicy)
-	if err != nil {
-		return err
-	}
-
-	// Retrieve the inventory object.
-	reader, err := r.loader.ManifestReader(cmd.InOrStdin(), flagutils.PathFromArgs(args))
-	if err != nil {
-		return err
-	}
-	objs, err := reader.Read()
-	if err != nil {
-		return err
-	}
-	inv, _, err := r.loader.InventoryInfo(objs)
-	if err != nil {
-		return err
-	}
-
-	if r.PreProcess != nil {
-		inventoryPolicy, err = r.PreProcess(inv, r.Destroyer.DryRunStrategy)
-		if err != nil {
-			return err
-		}
-	}
-
-	// Run the destroyer. It will return a channel where we can receive updates
-	// to keep track of progress and any issues.
-	err = r.Destroyer.Initialize()
-	if err != nil {
-		return err
-	}
-	option := &apply.DestroyerOption{
-		InventoryPolicy: inventoryPolicy,
-	}
-	ch := r.Destroyer.Run(inv, option)
-
-	// The printer will print updates from the channel. It will block
-	// until the channel is closed.
-	printer := printers.GetPrinter(r.output, r.ioStreams)
-	return printer.Print(ch, r.Destroyer.DryRunStrategy)
-}


Test patch:
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/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/internal/cmddestroy/cmddestroy_test.go b/internal/cmddestroy/cmddestroy_test.go
new file mode 100644
index 00000000..e0e6a915
--- /dev/null
+++ b/internal/cmddestroy/cmddestroy_test.go
@@ -0,0 +1,116 @@
+// 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 cmddestroy
+
+import (
+	"path/filepath"
+	"testing"
+
+	"github.com/GoogleContainerTools/kpt/internal/printer/fake"
+	"github.com/GoogleContainerTools/kpt/internal/testutil"
+	kptfilev1alpha2 "github.com/GoogleContainerTools/kpt/pkg/api/kptfile/v1alpha2"
+	"github.com/GoogleContainerTools/kpt/pkg/kptfile/kptfileutil"
+	"github.com/GoogleContainerTools/kpt/pkg/live"
+	"github.com/stretchr/testify/assert"
+	"k8s.io/cli-runtime/pkg/genericclioptions"
+	cmdtesting "k8s.io/kubectl/pkg/cmd/testing"
+	"sigs.k8s.io/cli-utils/pkg/inventory"
+)
+
+func TestCmd(t *testing.T) {
+	testCases := map[string]struct {
+		args                []string
+		namespace           string
+		inventory           *kptfilev1alpha2.Inventory
+		destroyCallbackFunc func(*testing.T, inventory.InventoryInfo)
+		expectedErrorMsg    string
+	}{
+		"invalid inventory policy": {
+			args: []string{
+				"--inventory-policy", "noSuchPolicy",
+			},
+			namespace: "testns",
+			destroyCallbackFunc: func(t *testing.T, _ inventory.InventoryInfo) {
+				t.FailNow()
+			},
+			expectedErrorMsg: "inventory policy must be one of strict, adopt",
+		},
+		"invalid output format": {
+			args: []string{
+				"--output", "foo",
+			},
+			namespace: "testns",
+			destroyCallbackFunc: func(t *testing.T, _ inventory.InventoryInfo) {
+				t.FailNow()
+			},
+			expectedErrorMsg: "unknown output type \"foo\"",
+		},
+		"fetches the correct inventory information from the Kptfile": {
+			args: []string{
+				"--inventory-policy", "adopt",
+				"--output", "events",
+			},
+			inventory: &kptfilev1alpha2.Inventory{
+				Namespace:   "my-ns",
+				Name:        "my-name",
+				InventoryID: "my-inv-id",
+			},
+			namespace: "testns",
+			destroyCallbackFunc: func(t *testing.T, inv inventory.InventoryInfo) {
+				assert.Equal(t, "my-ns", inv.Namespace())
+				assert.Equal(t, "my-name", inv.Name())
+				assert.Equal(t, "my-inv-id", inv.ID())
+			},
+		},
+	}
+
+	for tn, tc := range testCases {
+		t.Run(tn, func(t *testing.T) {
+			tf := cmdtesting.NewTestFactory().WithNamespace(tc.namespace)
+			defer tf.Cleanup()
+			ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() //nolint:dogsled
+
+			w, clean := testutil.SetupWorkspace(t)
+			defer clean()
+			kf := kptfileutil.DefaultKptfile(filepath.Base(w.WorkspaceDirectory))
+			kf.Inventory = tc.inventory
+			testutil.AddKptfileToWorkspace(t, w, kf)
+
+			revert := testutil.Chdir(t, w.WorkspaceDirectory)
+			defer revert()
+
+			rgProvider := live.NewResourceGroupProvider(tf)
+			rgLoader := live.NewResourceGroupManifestLoader(tf)
+
+			runner := NewRunner(fake.CtxWithNilPrinter(), rgProvider, rgLoader, ioStreams)
+			runner.Command.SetArgs(tc.args)
+			runner.destroyRunner = func(_ *Runner, inv inventory.InventoryInfo) error {
+				tc.destroyCallbackFunc(t, inv)
+				return nil
+			}
+			err := runner.Command.Execute()
+
+			// Check if there should be an error
+			if tc.expectedErrorMsg != "" {
+				if !assert.Error(t, err) {
+					t.FailNow()
+				}
+				assert.Contains(t, err.Error(), tc.expectedErrorMsg)
+				return
+			}
+			assert.NoError(t, err)
+		})
+	}
+}
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()
+		}
+	}
+}