From 5bae01fa68ba8dd436eb6b6609f17ba7e09ee837 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Thu, 7 May 2020 20:22:17 -0700 Subject: [PATCH] UX improvements to kyaml/fn/framework --- kyaml/fn/framework/doc.go | 43 +++++++-------- kyaml/fn/framework/example/main.go | 12 +++-- kyaml/fn/framework/example_test.go | 80 ++++++++++++++++------------ kyaml/fn/framework/framework.go | 40 ++++---------- kyaml/fn/framework/framework_test.go | 7 ++- kyaml/fn/framework/types.go | 2 +- 6 files changed, 86 insertions(+), 98 deletions(-) diff --git a/kyaml/fn/framework/doc.go b/kyaml/fn/framework/doc.go index f5b82fd18..2ef5336d9 100644 --- a/kyaml/fn/framework/doc.go +++ b/kyaml/fn/framework/doc.go @@ -6,7 +6,20 @@ // // Examples // -// Example function implementation using framework.ResourceList with functionConfig +// Example function implementation using framework.Command with flag input +// +// var value string +// resourceList := &framework.ResourceList{} +// cmd := framework.Command(resourceList, func() error { +// for i := range resourceList.Items { +// // modify the items... +// } +// return nil +// }) +// cmd.Flags().StringVar(&value, "value", "", "annotation value") +// if err := cmd.Execute(); err != nil { return err } +// +// Example function implementation using framework.ResourceList with a struct input // // type Spec struct { // Value string `yaml:"value,omitempty"` @@ -24,23 +37,11 @@ // } // if err := rl.Write(); err != nil { return err } // -// Example function implementation using framework.Command with flags -// -// var value string -// cmd := framework.Command(nil, func(items []*yaml.RNode) ([]*yaml.RNode, error) { -// for i := range items { -// // modify the items... -// } -// return items, nil -// }) -// cmd.Flags().StringVar(&value, "value", "", "annotation value") -// if err := cmd.Execute(); err != nil { return err } -// // Architecture // -// Functions modify a slice of resources (ResourceList.items) which are read as input and written +// Functions modify a slice of resources (ResourceList.Items) which are read as input and written // as output. The function itself may be configured through a functionConfig -// (ResourceList.functionConfig). +// (ResourceList.FunctionConfig). // // Example Function Input: // @@ -67,7 +68,7 @@ // # run the function by creating this container and providing this // # Example as the functionConfig // config.kubernetes.io/function: | -// image: image/containing/fuction:impl +// image: image/containing/function:impl // spec: // value: foo // @@ -77,7 +78,7 @@ // // Functions may also be specified imperatively and run using: // -// config run DIR/ --image image/containing/fuction:impl -- value=foo +// config run DIR/ --image image/containing/function:impl -- value=foo // // When run imperatively, a ConfigMap is generated for the functionConfig, and the command // arguments are set as ConfigMap data entries. @@ -91,15 +92,11 @@ // // Mutator and Generator Functions // -// Functions may add, delete or modify resources by modifying the items slice. -// When using framework.Command this is done through returning the new items slice. -// When using framework.ResourceList this is done through modifying ResourceList.Items in place. +// Functions may add, delete or modify resources by modifying the ResourceList.Items slice. // // Validator Functions // -// A function may validate resources by providing a Result. -// When using framework.Command this is done through returning a framework.Result as an error. -// WHen using framework.ResourceList this is done through setting ResourceList.Result. +// A function may emit validation results by setting the ResourceList.Result // // Configuring Functions // diff --git a/kyaml/fn/framework/example/main.go b/kyaml/fn/framework/example/main.go index c84f49bbe..239b6b7c7 100644 --- a/kyaml/fn/framework/example/main.go +++ b/kyaml/fn/framework/example/main.go @@ -12,14 +12,16 @@ import ( func main() { var value string - cmd := framework.Command(nil, func(items []*yaml.RNode) ([]*yaml.RNode, error) { - for i := range items { + resourceList := framework.ResourceList{} + cmd := framework.Command(&resourceList, func() error { + for i := range resourceList.Items { // set the annotation on each resource item - if err := items[i].PipeE(yaml.SetAnnotation("value", value)); err != nil { - return nil, err + err := resourceList.Items[i].PipeE(yaml.SetAnnotation("value", value)) + if err != nil { + return err } } - return items, nil + return nil }) cmd.Flags().StringVar(&value, "value", "", "annotation value") diff --git a/kyaml/fn/framework/example_test.go b/kyaml/fn/framework/example_test.go index d791eae67..3d5b37393 100644 --- a/kyaml/fn/framework/example_test.go +++ b/kyaml/fn/framework/example_test.go @@ -89,15 +89,17 @@ functionConfig: func ExampleCommand_modify() { // configure the annotation value using a flag parsed from // ResourceList.functionConfig.data.value + resourceList := framework.ResourceList{} var value string - cmd := framework.Command(nil, func(items []*yaml.RNode) ([]*yaml.RNode, error) { - for i := range items { + cmd := framework.Command(&resourceList, func() error { + for i := range resourceList.Items { // set the annotation on each resource item - if err := items[i].PipeE(yaml.SetAnnotation("value", value)); err != nil { - return nil, err + err := resourceList.Items[i].PipeE(yaml.SetAnnotation("value", value)) + if err != nil { + return err } } - return items, nil + return nil }) cmd.Flags().StringVar(&value, "value", "", "annotation value") @@ -164,12 +166,13 @@ func ExampleCommand_generateReplace() { functionConfig := &ExampleServiceGenerator{} // function implementation -- generate a Service resource - cmd := framework.Command(functionConfig, func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + resourceList := &framework.ResourceList{FunctionConfig: functionConfig} + cmd := framework.Command(resourceList, func() error { var newNodes []*yaml.RNode - for i := range nodes { - meta, err := nodes[i].GetMeta() + for i := range resourceList.Items { + meta, err := resourceList.Items[i].GetMeta() if err != nil { - return nil, err + return err } // something we already generated, remove it from the list so we regenerate it @@ -178,7 +181,7 @@ func ExampleCommand_generateReplace() { meta.APIVersion == "v1" { continue } - newNodes = append(newNodes, nodes[i]) + newNodes = append(newNodes, resourceList.Items[i]) } // generate the resource @@ -188,9 +191,11 @@ metadata: name: %s `, functionConfig.Spec.Name)) if err != nil { - return nil, err + return err } - return append(newNodes, n), nil + newNodes = append(newNodes, n) + resourceList.Items = newNodes + return nil }) // for testing purposes only -- normally read from stdin when Executing @@ -341,12 +346,13 @@ func ExampleCommand_generateUpdate() { functionConfig := &ExampleServiceGenerator{} // function implementation -- generate or update a Service resource - cmd := framework.Command(functionConfig, func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + resourceList := &framework.ResourceList{FunctionConfig: functionConfig} + cmd := framework.Command(resourceList, func() error { var found bool - for i := range nodes { - meta, err := nodes[i].GetMeta() + for i := range resourceList.Items { + meta, err := resourceList.Items[i].GetMeta() if err != nil { - return nil, err + return err } // something we already generated, reconcile it to make sure it matches what @@ -356,9 +362,9 @@ func ExampleCommand_generateUpdate() { meta.APIVersion == "v1" { // set some values for k, v := range functionConfig.Spec.Annotations { - err := nodes[i].PipeE(yaml.SetAnnotation(k, v)) + err := resourceList.Items[i].PipeE(yaml.SetAnnotation(k, v)) if err != nil { - return nil, err + return err } } found = true @@ -366,7 +372,7 @@ func ExampleCommand_generateUpdate() { } } if found { - return nodes, nil + return nil } // generate the resource if not found @@ -375,18 +381,18 @@ kind: Service metadata: name: %s `, functionConfig.Spec.Name)) + if err != nil { + return err + } for k, v := range functionConfig.Spec.Annotations { err := n.PipeE(yaml.SetAnnotation(k, v)) if err != nil { - return nil, err + return err } } - nodes = append(nodes, n) - if err != nil { - return nil, err - } + resourceList.Items = append(resourceList.Items, n) - return nodes, nil + return nil }) // for testing purposes only -- normally read from stdin when Executing @@ -445,25 +451,26 @@ functionConfig: // If any Deployments do not contain spec.replicas, then the function will return results // which will be set on ResourceList.results func ExampleCommand_validate() { - cmd := framework.Command(nil, func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { + resourceList := &framework.ResourceList{} + cmd := framework.Command(resourceList, func() error { // validation results var validationResults []framework.Item // validate that each Deployment resource has spec.replicas set - for i := range nodes { + for i := range resourceList.Items { // only check Deployment resources - meta, err := nodes[i].GetMeta() + meta, err := resourceList.Items[i].GetMeta() if err != nil { - return nil, err + return err } if meta.Kind != "Deployment" { continue } // lookup replicas field - r, err := nodes[i].Pipe(yaml.Lookup("spec", "replicas")) + r, err := resourceList.Items[i].Pipe(yaml.Lookup("spec", "replicas")) if err != nil { - return nil, err + return err } // check replicas not specified @@ -481,11 +488,14 @@ func ExampleCommand_validate() { }) } - // framework will only consider results an error if it has at least 1 item - return nodes, framework.Result{ - Name: "replicas-validator", - Items: validationResults, + if len(validationResults) > 0 { + resourceList.Result = &framework.Result{ + Name: "replicas-validator", + Items: validationResults, + } } + + return resourceList.Result }) // for testing purposes only -- normally read from stdin when Executing diff --git a/kyaml/fn/framework/framework.go b/kyaml/fn/framework/framework.go index c89246cec..5f1dc65b8 100644 --- a/kyaml/fn/framework/framework.go +++ b/kyaml/fn/framework/framework.go @@ -163,20 +163,18 @@ func (r *ResourceList) Write() error { // Command returns a cobra.Command to run a function. // -// The cobra.Command will use a ResourceList to Read() the input, run the provided function, -// and Write() the output. -// -// If functionConfig is non-nil, the ResourceList.functionConfig will be unmarshalled into it. +// The cobra.Command will use the provided ResourceList to Read() the input, +// run the provided function, and then Write() the output. // // The returned cobra.Command will have a "gen" subcommand which can be used to generate // a Dockerfile to build the function into a container image // // go run main.go gen DIR/ -func Command(functionConfig interface{}, function Function) cobra.Command { +func Command(resourceList *ResourceList, function Function) cobra.Command { cmd := cobra.Command{} AddGenerateDockerfile(&cmd) cmd.RunE = func(cmd *cobra.Command, args []string) error { - err := execute(function, functionConfig, cmd) + err := execute(resourceList, function, cmd) if err != nil { fmt.Fprintf(cmd.ErrOrStderr(), "%v", err) } @@ -209,38 +207,20 @@ CMD ["function"] cmd.AddCommand(gen) } -func execute(function Function, functionConfig interface{}, cmd *cobra.Command) error { - rl := ResourceList{ - FunctionConfig: functionConfig, - Flags: cmd.Flags(), - Writer: cmd.OutOrStdout(), - Reader: cmd.InOrStdin(), - } +func execute(rl *ResourceList, function Function, cmd *cobra.Command) error { + rl.Reader = cmd.InOrStdin() + rl.Writer = cmd.OutOrStdout() + rl.Flags = cmd.Flags() if err := rl.Read(); err != nil { return err } - // run the function implementation - var err error - rl.Items, err = function(rl.Items) - - // set the ResourceList.results for validating functions - var result *Result - if err != nil { - if val, ok := err.(Result); ok { - rl.Result = &val - } else { - return errors.Wrap(err) - } - } + retErr := function() if err := rl.Write(); err != nil { return err } - if result != nil && result.ExitCode() != 0 { - return errors.Wrap(err) - } - return nil + return retErr } diff --git a/kyaml/fn/framework/framework_test.go b/kyaml/fn/framework/framework_test.go index 01bbd9ba3..fabb7a258 100644 --- a/kyaml/fn/framework/framework_test.go +++ b/kyaml/fn/framework/framework_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/kyaml/fn/framework" - "sigs.k8s.io/kustomize/kyaml/yaml" ) func TestCommand_dockerfile(t *testing.T) { @@ -22,9 +21,9 @@ func TestCommand_dockerfile(t *testing.T) { defer os.RemoveAll(d) // create a function - cmd := framework.Command(nil, func(nodes []*yaml.RNode) ([]*yaml.RNode, error) { - return nil, nil - }) + + resourceList := &framework.ResourceList{} + cmd := framework.Command(resourceList, func() error { return nil }) // generate the Dockerfile cmd.SetArgs([]string{"gen", d}) diff --git a/kyaml/fn/framework/types.go b/kyaml/fn/framework/types.go index 041ea2725..f734e19ba 100644 --- a/kyaml/fn/framework/types.go +++ b/kyaml/fn/framework/types.go @@ -11,7 +11,7 @@ import ( // Function defines a function which mutates or validates a collection of configuration // To create a structured validation result, return a Result as the error. -type Function func(nodes []*yaml.RNode) ([]*yaml.RNode, error) +type Function func() error // Result defines a function result which will be set on the emitted ResourceList type Result struct {