From 15fbfdef386d421258ee78647b613dd07c09aee3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Ortiz=20Garc=C3=ADa?= <goodwine@apple.com>
Date: Wed, 6 Oct 2021 18:49:54 -0700
Subject: [PATCH] Handle kyaml Filter errors type Result as non-breaking errors
 and store in ResourceList

This is a breaking change:
- Result can only count as error when passed as pointer, this makes easy use of "errors.As"
- Existing Filter() implementations that return Result as an error will no longer break the pipeline
---
 kyaml/fn/framework/framework.go      | 14 ++++-
 kyaml/fn/framework/framework_test.go | 86 +---------------------------
 kyaml/fn/framework/result.go         |  2 +-
 3 files changed, 17 insertions(+), 85 deletions(-)

diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go
index 12c30a2c221..f7bde1ddf9a 100644
--- a/kyaml/fn/framework/framework.go
+++ b/kyaml/fn/framework/framework.go
@@ -4,6 +4,7 @@
 package framework
 
 import (
+	goerrors "errors"
 	"os"
 
 	"sigs.k8s.io/kustomize/kyaml/errors"
@@ -139,8 +140,19 @@ func Execute(p ResourceListProcessor, rlSource *kio.ByteReadWriter) error {
 
 // Filter executes the given kio.Filter and replaces the ResourceList's items with the result.
 // This can be used to help implement ResourceListProcessors. See SimpleProcessor for example.
+//
+// Filters that return a Result as error will store the result in the ResourceList instead of
+// and continue processing instead of erroring out.
 func (rl *ResourceList) Filter(api kio.Filter) error {
 	var err error
 	rl.Items, err = api.Filter(rl.Items)
-	return errors.Wrap(err)
+	if err != nil {
+		var r *Result
+		if goerrors.As(err, &r) {
+			rl.Result = r
+			return nil
+		}
+		return errors.Wrap(err)
+	}
+	return nil
 }
diff --git a/kyaml/fn/framework/framework_test.go b/kyaml/fn/framework/framework_test.go
index d893a35948c..9286bf2d858 100644
--- a/kyaml/fn/framework/framework_test.go
+++ b/kyaml/fn/framework/framework_test.go
@@ -15,85 +15,6 @@ import (
 )
 
 func TestExecute_Result(t *testing.T) {
-	p := framework.ResourceListProcessorFunc(func(rl *framework.ResourceList) error {
-		err := &framework.Result{
-			Name: "Incompatible config",
-			Items: []framework.ResultItem{
-				{
-					Message:  "bad value for replicas",
-					Severity: framework.Error,
-					ResourceRef: yaml.ResourceIdentifier{
-						TypeMeta: yaml.TypeMeta{APIVersion: "v1", Kind: "Deployment"},
-						NameMeta: yaml.NameMeta{Name: "tester", Namespace: "default"},
-					},
-					Field: framework.Field{
-						Path:           ".spec.Replicas",
-						CurrentValue:   "0",
-						SuggestedValue: "3",
-					},
-					File: framework.File{
-						Path:  "/path/to/deployment.yaml",
-						Index: 0,
-					},
-				},
-				{
-					Message:  "some error",
-					Severity: framework.Error,
-				},
-			},
-		}
-		rl.Result = err
-		return err
-	})
-	out := new(bytes.Buffer)
-	source := &kio.ByteReadWriter{Reader: bytes.NewBufferString(`
-kind: ResourceList
-apiVersion: config.kubernetes.io/v1alpha1
-items:
-- kind: Deployment
-  apiVersion: v1
-  metadata:
-    name: tester
-    namespace: default
-  spec:
-    replicas: 0
-`), Writer: out}
-	err := framework.Execute(p, source)
-	assert.EqualError(t, err, `[error] v1/Deployment/default/tester .spec.Replicas: bad value for replicas
-
-[error] : some error`)
-	assert.Equal(t, 1, err.(*framework.Result).ExitCode())
-	assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
-kind: ResourceList
-items:
-- kind: Deployment
-  apiVersion: v1
-  metadata:
-    name: tester
-    namespace: default
-  spec:
-    replicas: 0
-results:
-  name: Incompatible config
-  items:
-  - message: bad value for replicas
-    severity: error
-    resourceRef:
-      apiVersion: v1
-      kind: Deployment
-      name: tester
-      namespace: default
-    field:
-      path: .spec.Replicas
-      currentValue: "0"
-      suggestedValue: "3"
-    file:
-      path: /path/to/deployment.yaml
-  - message: some error
-    severity: error`, strings.TrimSpace(out.String()))
-}
-
-func TestExecute_NoErrorResult(t *testing.T) {
 	singleDeployment := `kind: Deployment
 apiVersion: v1
 metadata:
@@ -201,8 +122,8 @@ results:
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			p := framework.ResourceListProcessorFunc(func(rl *framework.ResourceList) error {
-				rl.Result = &framework.Result{
+			p := framework.SimpleProcessor{Filter: kio.FilterFunc(func(in []*yaml.RNode) ([]*yaml.RNode, error) {
+				return in, &framework.Result{
 					Name: "Incompatible config",
 					Items: []framework.ResultItem{
 						{
@@ -228,8 +149,7 @@ results:
 						},
 					},
 				}
-				return nil
-			})
+			})}
 			got := new(bytes.Buffer)
 			source := &kio.ByteReadWriter{
 				Reader:             bytes.NewBufferString(tc.input),
diff --git a/kyaml/fn/framework/result.go b/kyaml/fn/framework/result.go
index 94e43ee3e3e..3965a3db16e 100644
--- a/kyaml/fn/framework/result.go
+++ b/kyaml/fn/framework/result.go
@@ -95,7 +95,7 @@ type Result struct {
 }
 
 // Error enables a Result to be returned as an error
-func (e Result) Error() string {
+func (e *Result) Error() string {
 	var msgs []string
 	for _, i := range e.Items {
 		msgs = append(msgs, i.String())