kpt fn render ensures validators don't mutate resources (#1896)

* kpt fn render ensures validators don't mutate resources

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
-    configPath: starlark-httpbin-gen.yaml
+    - image: gcr.io/kpt-fn/starlark:unstable # generates httpbin deployment
+      configPath: starlark-httpbin-gen.yaml
   validators:
     - image: gcr.io/kpt-fn/starlark:unstable # validates httpbin deployment exists
       configPath: starlark-httpbin-val.yaml

e2e/testdata/fn-render/validator-mutates-resources/.krmignore

@@ -0,0 +1 @@
+.expected

e2e/testdata/fn-render/validator-mutates-resources/Kptfile

@@ -0,0 +1,10 @@
+apiVersion: kpt.dev/v1alpha2
+kind: Kptfile
+metadata:
+  name: app
+pipeline:
+  validators:
+# mutating function is being used as validator
+    - image: gcr.io/kpt-fn/set-label:unstable
+      configMap:
+        tier: backend

e2e/testdata/fn-render/validator-mutates-resources/resources.yaml

@@ -0,0 +1,26 @@
+# 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: apps/v1
+kind: Deployment
+metadata:
+  name: nginx-deployment
+spec:
+  replicas: 3
+---
+apiVersion: custom.io/v1
+kind: Custom
+metadata:
+  name: custom
+spec:
+  image: nginx:1.2.3

internal/cmdrender/executor.go

@@ -286,30 +286,97 @@ func (pn *pkgNode) runPipeline(ctx context.Context, hctx *hydrationContext, inpu
 		return input, nil
 	}
 
-	fnChain, err := fnChain(ctx, pl, pn.pkg.UniquePath)
+	mutatedResources, err := pn.runMutators(ctx, hctx, input)
 	if err != nil {
 		return nil, errors.E(op, pn.pkg.UniquePath, err)
 	}
 
+	if err = pn.runValidators(ctx, hctx, mutatedResources); err != nil {
+		return nil, errors.E(op, pn.pkg.UniquePath, err)
+	}
+	// print a new line after a pipeline running
+	pr.Printf("\n")
+	return mutatedResources, nil
+}
+
+// runMutators runs a set of mutators functions on given input resources.
+func (pn *pkgNode) runMutators(ctx context.Context, hctx *hydrationContext, input []*yaml.RNode) ([]*yaml.RNode, error) {
+	if len(input) == 0 {
+		return input, nil
+	}
+
+	pl, err := pn.pkg.Pipeline()
+	if err != nil {
+		return nil, err
+	}
+
+	if len(pl.Mutators) == 0 {
+		return input, nil
+	}
+
+	mutators, err := fnChain(ctx, pn.pkg.UniquePath, pl.Mutators)
+	if err != nil {
+		return nil, err
+	}
+
 	output := &kio.PackageBuffer{}
 	// create a kio pipeline from kyaml library to execute the function chains
-	kioPipeline := kio.Pipeline{
+	mutation := kio.Pipeline{
 		Inputs: []kio.Reader{
 			&kio.PackageBuffer{Nodes: input},
 		},
-		Filters: fnChain,
+		Filters: mutators,
 		Outputs: []kio.Writer{output},
 	}
-	err = kioPipeline.Execute()
+	err = mutation.Execute()
 	if err != nil {
-		return nil, errors.E(op, pn.pkg.UniquePath, err)
+		return nil, err
 	}
-	hctx.executedFunctionCnt += len(fnChain)
-	// print a new line after a pipeline running
-	pr.Printf("\n")
+	hctx.executedFunctionCnt += len(mutators)
 	return output.Nodes, nil
 }
 
+// runValidators runs a set of validator functions on input resources.
+// We bail out on first validation failure today, but the logic can be
+// improved to report multiple failures. Reporting multiple failures
+// will require changes to the way we print errors
+func (pn *pkgNode) runValidators(ctx context.Context, hctx *hydrationContext, input []*yaml.RNode) error {
+	if len(input) == 0 {
+		return nil
+	}
+
+	pl, err := pn.pkg.Pipeline()
+	if err != nil {
+		return err
+	}
+
+	if len(pl.Validators) == 0 {
+		return nil
+	}
+
+	for i := range pl.Validators {
+		fn := pl.Validators[i]
+		validator, err := newFnRunner(ctx, &fn, pn.pkg.UniquePath)
+		if err != nil {
+			return err
+		}
+		// validators are run on a copy of mutated resources to ensure
+		// resources are not mutated.
+		if _, err = validator.Filter(cloneResources(input)); err != nil {
+			return err
+		}
+		hctx.executedFunctionCnt++
+	}
+	return nil
+}
+
+func cloneResources(input []*yaml.RNode) (output []*yaml.RNode) {
+	for _, resource := range input {
+		output = append(output, resource.Copy())
+	}
+	return
+}
+
 // path (location) of a KRM resources is tracked in a special key in
 // metadata.annotation field. adjustRelPath updates that path annotation by prepending
 // the given relPath to the current path annotation if it doesn't exist already.
@@ -338,13 +405,8 @@ func adjustRelPath(resources []*yaml.RNode, relPath string) ([]*yaml.RNode, erro
 	return resources, nil
 }
 
-// fnChain returns a slice of function runners from the
-// functions and configs defined in pipeline.
-func fnChain(ctx context.Context, pl *kptfilev1alpha2.Pipeline, pkgPath types.UniquePath) ([]kio.Filter, error) {
-	fns := []kptfilev1alpha2.Function{}
-	fns = append(fns, pl.Mutators...)
-	// TODO: Validators cannot modify resources.
-	fns = append(fns, pl.Validators...)
+// fnChain returns a slice of function runners given a list of functions defined in pipeline.
+func fnChain(ctx context.Context, pkgPath types.UniquePath, fns []kptfilev1alpha2.Function) ([]kio.Filter, error) {
 	var runners []kio.Filter
 	for i := range fns {
 		fn := fns[i]