From 8d22cbdccaf139e9a36d09ef54d3762669c5f55d Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Mon, 13 Apr 2020 11:53:29 -0700 Subject: [PATCH] Support writing results from container filter --- cmd/config/internal/commands/run-fns.go | 5 + cmd/config/internal/commands/run_test.go | 56 ++- kyaml/kio/filters/container.go | 64 +++- kyaml/kio/filters/container_test.go | 438 +++++++++++++++++++++-- kyaml/runfn/runfn.go | 18 + 5 files changed, 516 insertions(+), 65 deletions(-) diff --git a/cmd/config/internal/commands/run-fns.go b/cmd/config/internal/commands/run-fns.go index 12558f044..260808561 100644 --- a/cmd/config/internal/commands/run-fns.go +++ b/cmd/config/internal/commands/run-fns.go @@ -51,6 +51,9 @@ func GetRunFnRunner(name string) *RunFnRunner { &r.StarName, "star-name", "", "name of starlark program.") r.Command.Flags().MarkHidden("star-name") + r.Command.Flags().StringVar( + &r.ResultsDir, "results-dir", "", "write function results to this dir") + r.Command.Flags().BoolVar( &r.Network, "network", false, "enable network access for functions that declare it") r.Command.Flags().StringVar( @@ -77,6 +80,7 @@ type RunFnRunner struct { StarPath string StarName string RunFns runfn.RunFns + ResultsDir string Network bool NetworkName string Mounts []string @@ -267,6 +271,7 @@ func (r *RunFnRunner) preRunE(c *cobra.Command, args []string) error { NetworkName: r.NetworkName, EnableStarlark: r.EnableStar, StorageMounts: storageMounts, + ResultsDir: r.ResultsDir, } // don't consider args for the function diff --git a/cmd/config/internal/commands/run_test.go b/cmd/config/internal/commands/run_test.go index 33a400df6..4c3c6d4fc 100644 --- a/cmd/config/internal/commands/run_test.go +++ b/cmd/config/internal/commands/run_test.go @@ -11,23 +11,25 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/kyaml/runfn" ) // TestRunFnCommand_preRunE verifies that preRunE correctly parses the commandline // flags and arguments into the RunFns structure to be executed. func TestRunFnCommand_preRunE(t *testing.T) { tests := []struct { - name string - args []string - expected string - err string - path string - input io.Reader - output io.Writer - functionPaths []string - network bool - networkName string - mount []string + name string + args []string + expected string + expectedStruct *runfn.RunFns + err string + path string + input io.Reader + output io.Writer + functionPaths []string + network bool + networkName string + mount []string }{ { name: "config map", @@ -234,6 +236,26 @@ metadata: data: {g: h, i: j=k} kind: Foo apiVersion: v1 +`, + }, + { + name: "results_dir", + args: []string{"run", "dir", "--results-dir", "foo/", "--image", "foo:bar", "--", "a=b", "c=d", "e=f"}, + path: "dir", + expectedStruct: &runfn.RunFns{ + Path: "dir", + NetworkName: "bridge", + ResultsDir: "foo/", + }, + expected: ` +metadata: + name: function-input + annotations: + config.kubernetes.io/function: | + container: {image: 'foo:bar'} +data: {a: b, c: d, e: f} +kind: ConfigMap +apiVersion: v1 `, }, { @@ -324,6 +346,10 @@ apiVersion: v1 t.FailNow() } + if !assert.Equal(t, r.RunFns, r.RunFns) { + t.FailNow() + } + if !assert.Equal(t, toStorageMounts(tt.mount), r.RunFns.StorageMounts) { t.FailNow() } @@ -339,6 +365,14 @@ apiVersion: v1 } } + if tt.expectedStruct != nil { + r.RunFns.Functions = nil + tt.expectedStruct.FunctionPaths = tt.functionPaths + if !assert.Equal(t, *tt.expectedStruct, r.RunFns) { + t.FailNow() + } + } + }) } diff --git a/kyaml/kio/filters/container.go b/kyaml/kio/filters/container.go index f4294d3b8..0beb62f21 100644 --- a/kyaml/kio/filters/container.go +++ b/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. diff --git a/kyaml/kio/filters/container_test.go b/kyaml/kio/filters/container_test.go index 319a1a535..d043e9cba 100644 --- a/kyaml/kio/filters/container_test.go +++ b/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 diff --git a/kyaml/runfn/runfn.go b/kyaml/runfn/runfn.go index 0b3adfa0f..0d6ec195f 100644 --- a/kyaml/runfn/runfn.go +++ b/kyaml/runfn/runfn.go @@ -4,12 +4,14 @@ package runfn import ( + "fmt" "io" "os" "path" "path/filepath" "sort" "strings" + "sync/atomic" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" @@ -64,6 +66,12 @@ type RunFns struct { // DisableContainers will disable functions run as containers DisableContainers bool + // ResultsDir is where to write each functions results + ResultsDir string + + // resultsCount is used to generate the results filename for each container + resultsCount uint32 + // functionFilterProvider provides a filter to perform the function. // this is a variable so it can be mocked in tests functionFilterProvider func( @@ -219,6 +227,7 @@ func (r RunFns) getFunctionFilters(global bool, fns ...*yaml.RNode) ( if err != nil { return nil, err } + if c == nil { continue } @@ -304,12 +313,21 @@ func (r *RunFns) init() { // ffp provides function filters func (r *RunFns) ffp(spec filters.FunctionSpec, api *yaml.RNode) (kio.Filter, error) { if !r.DisableContainers && spec.Container.Image != "" { + var resultsFile string + // TODO: Add a test for this behavior + + if r.ResultsDir != "" { + resultsFile = filepath.Join(r.ResultsDir, fmt.Sprintf( + "results-%v.yaml", r.resultsCount)) + atomic.AddUint32(&r.resultsCount, 1) + } return &filters.ContainerFilter{ Image: spec.Container.Image, Config: api, Network: spec.Network, StorageMounts: r.StorageMounts, GlobalScope: r.GlobalScope, + ResultsFile: resultsFile, }, nil } if r.EnableStarlark && spec.Starlark.Path != "" {