Support writing results from container filter

kyaml/kio/filters/container.go

@@ -6,6 +6,7 @@ package filters
 import (
 	"bytes"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"path"
@@ -146,6 +147,13 @@ type ContainerFilter struct {
 	// nodes instead of only nodes scoped under the function.
 	GlobalScope bool
 
+	ResultsFile string
+
+	Results *yaml.RNode
+
+	// SetFlowStyleForConfig sets the style for config to Flow when serializing it
+	SetFlowStyleForConfig bool
+
 	// args may be specified by tests to override how a container is spawned
 	args []string
 
@@ -257,10 +265,7 @@ func (c *ContainerFilter) scope(dir string, nodes []*yaml.RNode) ([]*yaml.RNode,
 // GrepFilter implements kio.GrepFilter
 func (c *ContainerFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 	// get the command to filter the Resources
-	cmd, err := c.getCommand()
-	if err != nil {
-		return nil, err
-	}
+	cmd := c.getCommand()
 
 	in := &bytes.Buffer{}
 	out := &bytes.Buffer{}
@@ -296,7 +301,16 @@ func (c *ContainerFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 	cmd.Stdin = in
 	cmd.Stdout = out
 	if err := cmd.Run(); err != nil {
-		return nil, err
+		// write the results file on failure
+		results, e := r.Read()
+		if e != nil {
+			return nil, e
+		}
+		if e = c.doResults(r); e != nil {
+			return nil, e
+		}
+		// return the results from the function even on failure
+		return results, err
 	}
 
 	output, err := r.Read()
@@ -304,6 +318,10 @@ func (c *ContainerFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 		return nil, err
 	}
 
+	if err := c.doResults(r); err != nil {
+		return nil, err
+	}
+
 	// annotate any generated Resources with a path and index if they don't already have one
 	if err := kioutil.DefaultPathAnnotation(functionDir, output); err != nil {
 		return nil, err
@@ -314,6 +332,25 @@ func (c *ContainerFilter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 	return append(output, saved...), nil
 }
 
+func (c *ContainerFilter) doResults(r *kio.ByteReader) error {
+	// Write the results to a file if configured to do so
+	if c.ResultsFile != "" && r.Results != nil {
+		results, err := r.Results.String()
+		if err != nil {
+			return err
+		}
+		err = ioutil.WriteFile(c.ResultsFile, []byte(results), 0600)
+		if err != nil {
+			return err
+		}
+	}
+
+	if r.Results != nil {
+		c.Results = r.Results
+	}
+	return nil
+}
+
 // getArgs returns the command + args to run to spawn the container
 func (c *ContainerFilter) getArgs() []string {
 	// run the container using docker.  this is simpler than using the docker
@@ -341,6 +378,9 @@ func (c *ContainerFilter) getArgs() []string {
 		args = append(args, "--mount", storageMount.String())
 	}
 
+	// tell functions to write error messages to stderr as well as results
+	os.Setenv("LOG_TO_STDERR", "true")
+
 	// export the local environment vars to the container
 	for _, pair := range os.Environ() {
 		tokens := strings.Split(pair, "=")
@@ -353,17 +393,9 @@ func (c *ContainerFilter) getArgs() []string {
 }
 
 // getCommand returns a command which will apply the Filter using the container image
-func (c *ContainerFilter) getCommand() (*exec.Cmd, error) {
-	// encode the filter command API configuration
-	cfg := &bytes.Buffer{}
-	if err := func() error {
-		e := yaml.NewEncoder(cfg)
-		defer e.Close()
-		// make it fit on a single line
+func (c *ContainerFilter) getCommand() *exec.Cmd {
+	if c.SetFlowStyleForConfig {
 		c.Config.YNode().Style = yaml.FlowStyle
-		return e.Encode(c.Config.YNode())
-	}(); err != nil {
-		return nil, err
 	}
 
 	if len(c.args) == 0 {
@@ -375,7 +407,7 @@ func (c *ContainerFilter) getCommand() (*exec.Cmd, error) {
 	cmd.Env = os.Environ()
 
 	// set stderr for err messaging
-	return cmd, nil
+	return cmd
 }
 
 // IsReconcilerFilter filters Resources based on whether or not they are Reconciler Resource.

kyaml/kio/filters/container_test.go

@@ -6,6 +6,7 @@ package filters
 import (
 	"bytes"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"strings"
 	"testing"
@@ -15,6 +16,367 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/yaml"
 )
 
+func TestContainerFilter_Filter(t *testing.T) {
+	var tests = []struct {
+		name              string
+		input             []string
+		expectedOutput    []string
+		expectedError     string
+		expectedResults   string
+		noMakeResultsFile bool
+		instance          ContainerFilter
+	}{
+		{
+			name: "add_path_annotation",
+			instance: ContainerFilter{args: []string{
+				"echo", `
+apiVersion: config.kubernetes.io/v1alpha1
+kind: ResourceList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-foo
+- apiVersion: v1
+  kind: Service
+  metadata:
+    name: service-foo
+`,
+			},
+			},
+			expectedOutput: []string{
+				`
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: deployment-foo
+  annotations:
+    config.kubernetes.io/path: 'deployment_deployment-foo.yaml'
+`,
+				`
+apiVersion: v1
+kind: Service
+metadata:
+  name: service-foo
+  annotations:
+    config.kubernetes.io/path: 'service_service-foo.yaml'
+`,
+			},
+		},
+
+		{
+			name: "write_results",
+			instance: ContainerFilter{args: []string{
+				"echo", `
+apiVersion: config.kubernetes.io/v1alpha1
+kind: ResourceList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-foo
+- apiVersion: v1
+  kind: Service
+  metadata:
+    name: service-foo
+results:
+- apiVersion: config.k8s.io/v1alpha1
+  kind: ObjectError
+  name: "some-validator"
+  items:  
+  - type: error
+    message: "some message"
+    resourceRef:
+      apiVersion: apps/v1
+      kind: Deployment
+      name: foo
+      namespace: bar
+    file:
+      path: deploy.yaml
+      index: 0
+    field:
+      path: "spec.template.spec.containers[3].resources.limits.cpu"
+      currentValue: "200"
+      suggestedValue: "2"
+`,
+			},
+			},
+			expectedOutput: []string{
+				`
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: deployment-foo
+  annotations:
+    config.kubernetes.io/path: 'deployment_deployment-foo.yaml'
+`,
+				`
+apiVersion: v1
+kind: Service
+metadata:
+  name: service-foo
+  annotations:
+    config.kubernetes.io/path: 'service_service-foo.yaml'
+`,
+			},
+			expectedResults: `
+- apiVersion: config.k8s.io/v1alpha1
+  kind: ObjectError
+  name: "some-validator"
+  items:
+  - type: error
+    message: "some message"
+    resourceRef:
+      apiVersion: apps/v1
+      kind: Deployment
+      name: foo
+      namespace: bar
+    file:
+      path: deploy.yaml
+      index: 0
+    field:
+      path: "spec.template.spec.containers[3].resources.limits.cpu"
+      currentValue: "200"
+      suggestedValue: "2"
+`,
+		},
+
+		{
+			name:          "write_results_non_0_exit",
+			expectedError: "exit status 1",
+			instance: ContainerFilter{args: []string{"sh", "-c",
+				`echo '
+apiVersion: config.kubernetes.io/v1alpha1
+kind: ResourceList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-foo
+- apiVersion: v1
+  kind: Service
+  metadata:
+    name: service-foo
+results:
+- apiVersion: config.k8s.io/v1alpha1
+  kind: ObjectError
+  name: "some-validator"
+  items:  
+  - type: error
+    message: "some message"
+    resourceRef:
+      apiVersion: apps/v1
+      kind: Deployment
+      name: foo
+      namespace: bar
+    file:
+      path: deploy.yaml
+      index: 0
+    field:
+      path: "spec.template.spec.containers[3].resources.limits.cpu"
+      currentValue: "200"
+      suggestedValue: "2"
+' && cat not-real-dir
+`,
+			},
+			},
+			expectedOutput: []string{
+				`
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: deployment-foo
+  annotations:
+    config.kubernetes.io/path: 'deployment_deployment-foo.yaml'
+`,
+				`
+apiVersion: v1
+kind: Service
+metadata:
+  name: service-foo
+  annotations:
+    config.kubernetes.io/path: 'service_service-foo.yaml'
+`,
+			},
+			expectedResults: `
+- apiVersion: config.k8s.io/v1alpha1
+  kind: ObjectError
+  name: "some-validator"
+  items:
+  - type: error
+    message: "some message"
+    resourceRef:
+      apiVersion: apps/v1
+      kind: Deployment
+      name: foo
+      namespace: bar
+    file:
+      path: deploy.yaml
+      index: 0
+    field:
+      path: "spec.template.spec.containers[3].resources.limits.cpu"
+      currentValue: "200"
+      suggestedValue: "2"
+`,
+		},
+
+		{
+			name:              "write_results_non_0_exit_missing_file",
+			expectedError:     "open /not/real/file: no such file or directory",
+			noMakeResultsFile: true,
+			instance: ContainerFilter{args: []string{"sh", "-c",
+				`echo '
+apiVersion: config.kubernetes.io/v1alpha1
+kind: ResourceList
+items:
+- apiVersion: apps/v1
+  kind: Deployment
+  metadata:
+    name: deployment-foo
+- apiVersion: v1
+  kind: Service
+  metadata:
+    name: service-foo
+results:
+- apiVersion: config.k8s.io/v1alpha1
+  kind: ObjectError
+  name: "some-validator"
+  items:  
+  - type: error
+    message: "some message"
+    resourceRef:
+      apiVersion: apps/v1
+      kind: Deployment
+      name: foo
+      namespace: bar
+    file:
+      path: deploy.yaml
+      index: 0
+    field:
+      path: "spec.template.spec.containers[3].resources.limits.cpu"
+      currentValue: "200"
+      suggestedValue: "2"
+' && cat not-real-dir
+`,
+			},
+			},
+			expectedOutput: []string{
+				`
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: deployment-foo
+  annotations:
+    config.kubernetes.io/path: 'deployment_deployment-foo.yaml'
+`,
+				`
+apiVersion: v1
+kind: Service
+metadata:
+  name: service-foo
+  annotations:
+    config.kubernetes.io/path: 'service_service-foo.yaml'
+`,
+			},
+			expectedResults: `
+- apiVersion: config.k8s.io/v1alpha1
+  kind: ObjectError
+  name: "some-validator"
+  items:
+  - type: error
+    message: "some message"
+    resourceRef:
+      apiVersion: apps/v1
+      kind: Deployment
+      name: foo
+      namespace: bar
+    file:
+      path: deploy.yaml
+      index: 0
+    field:
+      path: "spec.template.spec.containers[3].resources.limits.cpu"
+      currentValue: "200"
+      suggestedValue: "2"
+`,
+		},
+	}
+
+	for i := range tests {
+		tt := tests[i]
+		t.Run(tt.name, func(t *testing.T) {
+			if len(tt.expectedResults) > 0 && !tt.noMakeResultsFile {
+				f, err := ioutil.TempFile("", "test-kyaml-*.yaml")
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				defer os.RemoveAll(f.Name())
+				tt.instance.ResultsFile = f.Name()
+			} else if len(tt.expectedResults) > 0 {
+				tt.instance.ResultsFile = "/not/real/file"
+			}
+
+			var inputs []*yaml.RNode
+			for i := range tt.input {
+				node, err := yaml.Parse(tt.input[i])
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				inputs = append(inputs, node)
+			}
+
+			output, err := tt.instance.Filter(inputs)
+			if tt.expectedError != "" {
+				if !assert.EqualError(t, err, tt.expectedError) {
+					t.FailNow()
+				}
+				return
+			}
+
+			if !assert.NoError(t, err) {
+				t.FailNow()
+			}
+
+			var actual []string
+			for i := range output {
+				s, err := output[i].String()
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				actual = append(actual, strings.TrimSpace(s))
+			}
+			var expected []string
+			for i := range tt.expectedOutput {
+				expected = append(expected, strings.TrimSpace(tt.expectedOutput[i]))
+			}
+
+			if !assert.Equal(t, expected, actual) {
+				t.FailNow()
+			}
+
+			if len(tt.instance.ResultsFile) > 0 {
+				tt.expectedResults = strings.TrimSpace(tt.expectedResults)
+
+				results, err := tt.instance.Results.String()
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				if !assert.Equal(t, tt.expectedResults, strings.TrimSpace(results)) {
+					t.FailNow()
+				}
+
+				b, err := ioutil.ReadFile(tt.instance.ResultsFile)
+				writtenResults := strings.TrimSpace(string(b))
+				if !assert.NoError(t, err) {
+					t.FailNow()
+				}
+				if !assert.Equal(t, tt.expectedResults, writtenResults) {
+					t.FailNow()
+				}
+			}
+		})
+	}
+}
+
 func TestFilter_command(t *testing.T) {
 	cfg, err := yaml.Parse(`apiVersion: apps/v1
 kind: Deployment
@@ -29,10 +391,7 @@ metadata:
 		Config: cfg,
 	}
 	os.Setenv("KYAML_TEST", "FOO")
-	cmd, err := instance.getCommand()
-	if !assert.NoError(t, err) {
-		return
-	}
+	cmd := instance.getCommand()
 
 	expected := []string{
 		"docker", "run",
@@ -78,10 +437,7 @@ metadata:
 		Config:        cfg,
 		StorageMounts: []StorageMount{bindMount, localVol, tmpfs},
 	}
-	cmd, err := instance.getCommand()
-	if !assert.NoError(t, err) {
-		return
-	}
+	cmd := instance.getCommand()
 
 	expected := []string{
 		"docker", "run",
@@ -116,10 +472,7 @@ metadata:
 		Network: "test-net",
 		Config:  cfg,
 	}
-	cmd, err := instance.getCommand()
-	if !assert.NoError(t, err) {
-		return
-	}
+	cmd := instance.getCommand()
 
 	expected := []string{
 		"docker", "run",
@@ -168,9 +521,10 @@ metadata:
 
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
-		args:   []string{"sed", "s/Deployment/StatefulSet/g"},
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sed", "s/Deployment/StatefulSet/g"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
@@ -252,9 +606,10 @@ metadata:
 
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
-		args:   []string{"sh", "-c", "cat <&0"},
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sh", "-c", "cat <&0"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
@@ -597,8 +952,9 @@ metadata:
 
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
 		args: []string{"echo", `apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -671,8 +1027,9 @@ metadata:
 
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
 		args: []string{"echo", `apiVersion: apps/v1
 kind: Deployment
 metadata:
@@ -756,9 +1113,10 @@ metadata:
 	// no resources match the scope
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
-		args:   []string{"sed", "s/Deployment/StatefulSet/g"},
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sed", "s/Deployment/StatefulSet/g"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
@@ -831,10 +1189,11 @@ metadata:
 	// no resources match the scope
 	called := false
 	result, err := (&ContainerFilter{
-		GlobalScope: true,
-		Image:       "example.com:version",
-		Config:      cfg,
-		args:        []string{"sed", "s/Deployment/StatefulSet/g"},
+		SetFlowStyleForConfig: true,
+		GlobalScope:           true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sed", "s/Deployment/StatefulSet/g"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
@@ -926,9 +1285,10 @@ metadata:
 	// no resources match the scope
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
-		args:   []string{"sed", "s/Deployment/StatefulSet/g"},
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sed", "s/Deployment/StatefulSet/g"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
@@ -1022,9 +1382,10 @@ metadata:
 	// no resources match the scope
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
-		args:   []string{"sed", "s/Deployment/StatefulSet/g"},
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sed", "s/Deployment/StatefulSet/g"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1
@@ -1118,9 +1479,10 @@ metadata:
 	// no resources match the scope
 	called := false
 	result, err := (&ContainerFilter{
-		Image:  "example.com:version",
-		Config: cfg,
-		args:   []string{"sed", "s/Deployment/StatefulSet/g"},
+		SetFlowStyleForConfig: true,
+		Image:                 "example.com:version",
+		Config:                cfg,
+		args:                  []string{"sed", "s/Deployment/StatefulSet/g"},
 		checkInput: func(s string) {
 			called = true
 			if !assert.Equal(t, `apiVersion: config.kubernetes.io/v1alpha1