From 572260c0f05b288fb0780797a46bd8dfd5b790a3 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Tue, 26 May 2020 17:22:04 -0700 Subject: [PATCH 1/3] change args to use flags instead, so that values starting with hyphen can be correctly parsed. --- .../internal/commands/cmdcreatesetter.go | 17 +- .../internal/commands/cmdcreatesetter_test.go | 37 +++ cmd/config/internal/commands/cmdset.go | 34 ++- cmd/config/internal/commands/cmdset_test.go | 249 +++++++++++++++++- cmd/config/internal/commands/util.go | 11 + 5 files changed, 335 insertions(+), 13 deletions(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index 2a0120aac..a15bad8f8 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -20,14 +20,16 @@ import ( func NewCreateSetterRunner(parent string) *CreateSetterRunner { r := &CreateSetterRunner{} set := &cobra.Command{ - Use: "create-setter DIR NAME VALUE", - Args: cobra.ExactArgs(3), + Use: "create-setter DIR NAME --value VALUE", + Args: cobra.RangeArgs(2, 3), Short: commands.CreateSetterShort, Long: commands.CreateSetterLong, Example: commands.CreateSetterExamples, PreRunE: r.preRunE, RunE: r.runE, } + set.Flags().StringVar(&r.Set.SetPartialField.Setter.Value, "value", "", + "optional flag, the value of the setter to be set to") set.Flags().StringVar(&r.Set.SetPartialField.SetBy, "set-by", "", "record who the field was default by.") set.Flags().StringVar(&r.Set.SetPartialField.Description, "description", "", @@ -73,18 +75,23 @@ func (r *CreateSetterRunner) runE(c *cobra.Command, args []string) error { } func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { + valueSetFromFlag := isFlagSet("value", c) var err error r.Set.SetPartialField.Setter.Name = args[1] - r.Set.SetPartialField.Setter.Value = args[2] r.CreateSetter.Name = args[1] - r.CreateSetter.FieldValue = args[2] + if valueSetFromFlag { + r.CreateSetter.FieldValue = r.Set.SetPartialField.Setter.Value + } else if len(args) > 2 { + r.Set.SetPartialField.Setter.Value = args[2] + r.CreateSetter.FieldValue = args[2] + } r.CreateSetter.FieldName, err = c.Flags().GetString("field") if err != nil { return err } if setterVersion == "" { - if len(args) < 3 { + if len(args) < 2 || !valueSetFromFlag && len(args) < 3 { setterVersion = "v1" } else if err := initSetterVersion(c, args); err != nil { return err diff --git a/cmd/config/internal/commands/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index 001de307e..c194e194c 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -166,6 +166,43 @@ kind: Example spec: list: - "a" # {"$openapi":"list"} + `, + }, + { + name: "add replicas with value set by flag", + args: []string{"replicas", "--value", "3", "--description", "hello world", "--set-by", "me"}, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + `, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +`, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} `, }, } diff --git a/cmd/config/internal/commands/cmdset.go b/cmd/config/internal/commands/cmdset.go index 6886d16ed..d5875dcbc 100644 --- a/cmd/config/internal/commands/cmdset.go +++ b/cmd/config/internal/commands/cmdset.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/cmd/config/internal/generateddocs/commands" + "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/setters" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" @@ -20,8 +21,8 @@ import ( func NewSetRunner(parent string) *SetRunner { r := &SetRunner{} c := &cobra.Command{ - Use: "set DIR NAME [VALUE]", - Args: cobra.MinimumNArgs(3), + Use: "set DIR NAME --values [VALUE]", + Args: cobra.MinimumNArgs(2), Short: commands.SetShort, Long: commands.SetLong, Example: commands.SetExamples, @@ -30,6 +31,8 @@ func NewSetRunner(parent string) *SetRunner { } fixDocs(parent, c) r.Command = c + c.Flags().StringArrayVar(&r.Values, "values", []string{}, + "optional flag, the values of the setter to be set to") c.Flags().StringVar(&r.Perform.SetBy, "set-by", "", "annotate the field with who set it") c.Flags().StringVar(&r.Perform.Description, "description", "", @@ -53,6 +56,7 @@ type SetRunner struct { Perform setters.PerformSetters Set settersutil.FieldSetter OpenAPIFile string + Values []string } func initSetterVersion(c *cobra.Command, args []string) error { @@ -75,16 +79,25 @@ func initSetterVersion(c *cobra.Command, args []string) error { } func (r *SetRunner) preRunE(c *cobra.Command, args []string) error { + valueFlagSet := isFlagSet("values", c) + + if valueFlagSet && len(args) > 2 { + return errors.Errorf("value should set either from flag or arg") + } + if len(args) > 1 { r.Perform.Name = args[1] r.Lookup.Name = args[1] } - if len(args) > 2 { + + if isFlagSet("values", c) && len(r.Values) == 1 { + r.Perform.Value = r.Values[0] + } else if len(args) > 2 { r.Perform.Value = args[2] } if setterVersion == "" { - if len(args) < 3 { + if len(args) < 2 || len(args) < 3 && !valueFlagSet { setterVersion = "v1" } else if err := initSetterVersion(c, args); err != nil { return err @@ -93,12 +106,19 @@ func (r *SetRunner) preRunE(c *cobra.Command, args []string) error { if setterVersion == "v2" { var err error r.Set.Name = args[1] - r.Set.Value = args[2] + if valueFlagSet { + r.Set.Value = r.Values[0] + } else { + r.Set.Value = args[2] + } // set remaining values as list values - if len(args) > 3 { + if valueFlagSet && len(r.Values) > 1 { + r.Set.ListValues = r.Values[1:] + } else if !valueFlagSet && len(args) > 3 { r.Set.ListValues = args[3:] } + r.Set.Description = r.Perform.Description r.Set.SetBy = r.Perform.SetBy r.OpenAPIFile, err = ext.GetOpenAPIFile(args) @@ -116,7 +136,7 @@ func (r *SetRunner) runE(c *cobra.Command, args []string) error { fmt.Fprintf(c.OutOrStdout(), "set %d fields\n", count) return handleError(c, err) } - if len(args) == 3 { + if len(args) > 2 || isFlagSet("values", c) { return handleError(c, r.perform(c, args)) } return handleError(c, lookup(r.Lookup, c, args)) diff --git a/cmd/config/internal/commands/cmdset_test.go b/cmd/config/internal/commands/cmdset_test.go index 5e253573c..427a0c557 100644 --- a/cmd/config/internal/commands/cmdset_test.go +++ b/cmd/config/internal/commands/cmdset_test.go @@ -74,6 +74,54 @@ spec: replicas: 4 # {"$openapi":"replicas"} `, }, + { + name: "validate length of argument", + args: []string{"--description", "hi there", "--set-by", "pw"}, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + `, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + `, + errMsg: "requires at least 2 arg(s), only received 1", + }, + { name: "set replicas no description", args: []string{"replicas", "4"}, @@ -121,7 +169,7 @@ spec: `, }, { - name: "set image", + name: "set image with value", args: []string{"tag", "1.8.1"}, out: "set 1 fields\n", inputOpenAPI: ` @@ -453,6 +501,205 @@ spec: `, errMsg: `list in body must be of type integer: "string" list in body must be of type integer: "boolean" +list in body should have at most 2 items`, + }, + + { + name: "set replicas with value set by flag", + args: []string{"replicas", "--values", "4", "--description", "hi there"}, + out: "set 1 fields\n", + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + `, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hi there + x-k8s-cli: + setter: + name: replicas + value: "4" + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 4 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + `, + }, + + { + name: "validate values set from either flag or arg", + args: []string{"replicas", "4", "--values", "4", "--description", "hi there"}, + inputOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + `, + expectedOpenAPI: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.replicas: + description: hello world + x-k8s-cli: + setter: + name: replicas + value: "3" + setBy: me + `, + expectedResources: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + `, + errMsg: `value should set either from flag or arg`, + }, + + { + name: "openAPI list values set by flag success", + args: []string{"list", "--values", "10", "--values", "11"}, + out: "set 1 fields\n", + inputOpenAPI: ` +kind: Kptfile +openAPI: + definitions: + io.k8s.cli.setters.list: + type: array + maxItems: 2 + items: + type: integer + x-k8s-cli: + setter: + name: list + listValues: + - 0 + `, + input: ` +apiVersion: example.com/v1beta1 +kind: Example +spec: + list: # {"$ref":"#/definitions/io.k8s.cli.setters.list"} + - 0 + `, + expectedOpenAPI: ` +kind: Kptfile +openAPI: + definitions: + io.k8s.cli.setters.list: + type: array + maxItems: 2 + items: + type: integer + x-k8s-cli: + setter: + name: list + listValues: + - "10" + - "11" + `, + expectedResources: ` +apiVersion: example.com/v1beta1 +kind: Example +spec: + list: # {"$ref":"#/definitions/io.k8s.cli.setters.list"} + - "10" + - "11" + `, + }, + + { + name: "validate openAPI list values set by flag error", + args: []string{"list", "--values", "10", "--values", "hi", "--values", "true"}, + inputOpenAPI: ` +kind: Kptfile +openAPI: + definitions: + io.k8s.cli.setters.list: + type: array + maxItems: 2 + items: + type: integer + x-k8s-cli: + setter: + name: list + listValues: + - 0 + `, + input: ` +apiVersion: example.com/v1beta1 +kind: Example +spec: + list: # {"$ref":"#/definitions/io.k8s.cli.setters.list"} + - 0 + `, + expectedOpenAPI: ` +kind: Kptfile +openAPI: + definitions: + io.k8s.cli.setters.list: + type: array + maxItems: 2 + items: + type: integer + x-k8s-cli: + setter: + name: list + listValues: + - 0 + `, + expectedResources: ` +apiVersion: example.com/v1beta1 +kind: Example +spec: + list: # {"$ref":"#/definitions/io.k8s.cli.setters.list"} + - 0 + `, + errMsg: `list in body must be of type integer: "string" +list in body must be of type integer: "boolean" list in body should have at most 2 items`, }, } diff --git a/cmd/config/internal/commands/util.go b/cmd/config/internal/commands/util.go index b201451ed..41cd100d1 100644 --- a/cmd/config/internal/commands/util.go +++ b/cmd/config/internal/commands/util.go @@ -10,6 +10,7 @@ import ( "github.com/go-errors/errors" "github.com/spf13/cobra" + flag "github.com/spf13/pflag" ) // parseFieldPath parse a flag value into a field path @@ -39,6 +40,16 @@ func parseFieldPath(path string) ([]string, error) { return newParts, nil } +func isFlagSet(name string, c *cobra.Command) bool { + set := false + c.Flags().Visit(func(f *flag.Flag) { + if f.Name == name { + set = true + } + }) + return set +} + func handleError(c *cobra.Command, err error) error { if err == nil { return nil From 4b5b0dfdceeb7942baa7ae9b9d0c3ea0d7cae584 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Fri, 29 May 2020 12:50:03 -0700 Subject: [PATCH 2/3] rebase --- cmd/config/internal/commands/cmdcreatesetter_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index c194e194c..c584230c6 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -202,7 +202,7 @@ kind: Deployment metadata: name: nginx-deployment spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, } From e63b9ef8256ebe0fdfa8ef7eff1716c164995e91 Mon Sep 17 00:00:00 2001 From: Jijie Wei Date: Fri, 29 May 2020 16:23:10 -0700 Subject: [PATCH 3/3] update on util and comments --- cmd/config/internal/commands/cmdcreatesetter.go | 8 ++++---- cmd/config/internal/commands/cmdset.go | 6 +++--- cmd/config/internal/commands/util.go | 11 ----------- 3 files changed, 7 insertions(+), 18 deletions(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index a15bad8f8..f2ea66ab3 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -20,7 +20,7 @@ import ( func NewCreateSetterRunner(parent string) *CreateSetterRunner { r := &CreateSetterRunner{} set := &cobra.Command{ - Use: "create-setter DIR NAME --value VALUE", + Use: "create-setter DIR NAME VALUE", Args: cobra.RangeArgs(2, 3), Short: commands.CreateSetterShort, Long: commands.CreateSetterLong, @@ -29,7 +29,7 @@ func NewCreateSetterRunner(parent string) *CreateSetterRunner { RunE: r.runE, } set.Flags().StringVar(&r.Set.SetPartialField.Setter.Value, "value", "", - "optional flag, the value of the setter to be set to") + "optional flag, alternative to specifying the value as an argument. e.g. used to specify values that start with '-'") set.Flags().StringVar(&r.Set.SetPartialField.SetBy, "set-by", "", "record who the field was default by.") set.Flags().StringVar(&r.Set.SetPartialField.Description, "description", "", @@ -75,7 +75,7 @@ func (r *CreateSetterRunner) runE(c *cobra.Command, args []string) error { } func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { - valueSetFromFlag := isFlagSet("value", c) + valueSetFromFlag := c.Flag("value").Changed var err error r.Set.SetPartialField.Setter.Name = args[1] r.CreateSetter.Name = args[1] @@ -91,7 +91,7 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { } if setterVersion == "" { - if len(args) < 2 || !valueSetFromFlag && len(args) < 3 { + if len(args) < 2 || !c.Flag("value").Changed && len(args) < 3 { setterVersion = "v1" } else if err := initSetterVersion(c, args); err != nil { return err diff --git a/cmd/config/internal/commands/cmdset.go b/cmd/config/internal/commands/cmdset.go index d5875dcbc..6d7143b7a 100644 --- a/cmd/config/internal/commands/cmdset.go +++ b/cmd/config/internal/commands/cmdset.go @@ -79,7 +79,7 @@ func initSetterVersion(c *cobra.Command, args []string) error { } func (r *SetRunner) preRunE(c *cobra.Command, args []string) error { - valueFlagSet := isFlagSet("values", c) + valueFlagSet := c.Flag("values").Changed if valueFlagSet && len(args) > 2 { return errors.Errorf("value should set either from flag or arg") @@ -90,7 +90,7 @@ func (r *SetRunner) preRunE(c *cobra.Command, args []string) error { r.Lookup.Name = args[1] } - if isFlagSet("values", c) && len(r.Values) == 1 { + if valueFlagSet { r.Perform.Value = r.Values[0] } else if len(args) > 2 { r.Perform.Value = args[2] @@ -136,7 +136,7 @@ func (r *SetRunner) runE(c *cobra.Command, args []string) error { fmt.Fprintf(c.OutOrStdout(), "set %d fields\n", count) return handleError(c, err) } - if len(args) > 2 || isFlagSet("values", c) { + if len(args) > 2 || c.Flag("values").Changed { return handleError(c, r.perform(c, args)) } return handleError(c, lookup(r.Lookup, c, args)) diff --git a/cmd/config/internal/commands/util.go b/cmd/config/internal/commands/util.go index 41cd100d1..b201451ed 100644 --- a/cmd/config/internal/commands/util.go +++ b/cmd/config/internal/commands/util.go @@ -10,7 +10,6 @@ import ( "github.com/go-errors/errors" "github.com/spf13/cobra" - flag "github.com/spf13/pflag" ) // parseFieldPath parse a flag value into a field path @@ -40,16 +39,6 @@ func parseFieldPath(path string) ([]string, error) { return newParts, nil } -func isFlagSet(name string, c *cobra.Command) bool { - set := false - c.Flags().Visit(func(f *flag.Flag) { - if f.Name == name { - set = true - } - }) - return set -} - func handleError(c *cobra.Command, err error) error { if err == nil { return nil