diff --git a/cmd/config/go.sum b/cmd/config/go.sum index a767e7439..a499dd06c 100644 --- a/cmd/config/go.sum +++ b/cmd/config/go.sum @@ -156,8 +156,10 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d h1:LCPbGQ34PMrwad11aMZ+dbz5SAsq/0ySjRwQ8I9Qwd8= -gopkg.in/yaml.v3 v3.0.0-20191026110619-0b21df46bc1d/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v2 v2.2.7 h1:VUgggvou5XRW9mHwD/yXxIYSMtY0zoKQf/v226p2nyo= +gopkg.in/yaml.v2 v2.2.7/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20191120175047-4206685974f2 h1:XZx7nhd5GMaZpmDaEHFVafUZC7ya0fuo7cSJ3UCKYmM= +gopkg.in/yaml.v3 v3.0.0-20191120175047-4206685974f2/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/apimachinery v0.17.0 h1:xRBnuie9rXcPxUkDizUsGvPf1cnlZCFu210op7J7LJo= k8s.io/apimachinery v0.17.0/go.mod h1:b9qmWdKlLuU9EBh+06BtLcSf/Mu89rWL33naRxs1uZg= k8s.io/gengo v0.0.0-20190128074634-0689ccc1d7d6/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0= diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index 9bcbe03d0..ec4297eb3 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -38,8 +38,7 @@ func NewCreateSetterRunner(parent string) *CreateSetterRunner { "kind of the Resource on which to create the setter.") set.Flags().MarkHidden("kind") set.Flags().StringVar(&r.Set.SetPartialField.Type, "type", "", - "valid OpenAPI field type -- e.g. integer,boolean,string.") - set.Flags().MarkHidden("type") + "OpenAPI field type for the setter -- e.g. integer,boolean,string.") set.Flags().BoolVar(&r.Set.SetPartialField.Partial, "partial", false, "create a partial setter for only part of the field value.") set.Flags().MarkHidden("partial") @@ -89,6 +88,7 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { r.OpenAPIFile, err = ext.GetOpenAPIFile(args) r.CreateSetter.Description = r.Set.SetPartialField.Description r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy + r.CreateSetter.Type = r.Set.SetPartialField.Type if err != nil { return err } diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index d082b5d58..83575c070 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -105,6 +105,9 @@ type SetterDefinition struct { // Count is the number of fields set by this setter. Count int `yaml:"count,omitempty"` + // Type is the type of the setter value. + Type string `yaml:"type,omitempty"` + // EnumValues is a map of possible setter values to actual field values. // If EnumValues is specified, then the value set the by user 1) MUST // be present in the enumValues map as a key, and 2) the map entry value @@ -135,6 +138,15 @@ func (sd SetterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { sd.Description = "" } + if sd.Type != "" { + err = def.PipeE(yaml.FieldSetter{Name: "type", StringValue: sd.Type}) + if err != nil { + return nil, err + } + // don't write the type to the extension + sd.Type = "" + } + ext, err := def.Pipe(yaml.LookupCreate(yaml.MappingNode, K8sCliExtensionKey)) if err != nil { return nil, err diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index fdff66dfe..b75f61d56 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -30,7 +30,7 @@ func (s *Set) Filter(object *yaml.RNode) (*yaml.RNode, error) { // visitScalar func (s *Set) visitScalar(object *yaml.RNode, p string) error { // get the openAPI for this field describing how to apply the setter - ext, err := getExtFromComment(object) + ext, sch, err := getExtFromComment(object) if err != nil { return err } @@ -39,13 +39,13 @@ func (s *Set) visitScalar(object *yaml.RNode, p string) error { } // perform a direct set of the field if it matches - if s.set(object, ext) { + if s.set(object, ext, sch) { s.Count++ return nil } // perform a substitution of the field if it matches - sub, err := s.substitute(object, ext) + sub, err := s.substitute(object, ext, sch) if err != nil { return err } @@ -57,7 +57,7 @@ func (s *Set) visitScalar(object *yaml.RNode, p string) error { // substitute updates the value of field from ext if ext contains a substitution that // depends on a setter whose name matches s.Name. -func (s *Set) substitute(field *yaml.RNode, ext *cliExtension) (bool, error) { +func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) (bool, error) { nameMatch := false // check partial setters to see if they contain the setter as part of a @@ -109,11 +109,15 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension) (bool, error) { // TODO(pwittrock): validate the field value field.YNode().Value = p + + // substitutions are always strings + field.YNode().Tag = "!!str" + return true, nil } // set applies the value from ext to field if its name matches s.Name -func (s *Set) set(field *yaml.RNode, ext *cliExtension) bool { +func (s *Set) set(field *yaml.RNode, ext *cliExtension, sch *spec.Schema) bool { // check full setter if ext.Setter == nil || ext.Setter.Name != s.Name { return false @@ -130,6 +134,9 @@ func (s *Set) set(field *yaml.RNode, ext *cliExtension) bool { // this has a full setter, set its value field.YNode().Value = ext.Setter.Value + + // format the node so it is quoted if it is a string + yaml.FormatNonStringStyle(field.YNode(), *sch) return true } @@ -191,7 +198,14 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { } } - if err := def.PipeE(&yaml.FieldSetter{Name: "value", StringValue: s.Value}); err != nil { + v := yaml.NewScalarRNode(s.Value) + // values are always represented as strings the OpenAPI + // since the are unmarshalled into strings. Use double quote style to + // ensure this consistently. + v.YNode().Tag = "!!str" + v.YNode().Style = yaml.DoubleQuotedStyle + + if err := def.PipeE(&yaml.FieldSetter{Name: "value", Value: v}); err != nil { return nil, err } diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index 705df8c01..3414cf10d 100644 --- a/kyaml/setters2/set_test.go +++ b/kyaml/setters2/set_test.go @@ -15,11 +15,12 @@ import ( func TestSet_Filter(t *testing.T) { var tests = []struct { - name string - setter string - openapi string - input string - expected string + name string + description string + setter string + openapi string + input string + expected string }{ { name: "set-replicas", @@ -58,6 +59,98 @@ metadata: name: nginx-deployment spec: replicas: 4 # {"$ref": "#/definitions/io.k8s.cli.setters.replicas"} + `, + }, + { + name: "set-foo-type", + description: "if a type is specified for a setter, ensure the field is properly quoted", + setter: "foo", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.foo: + x-k8s-cli: + setter: + name: foo + value: "4" + type: string + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + foo: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"} + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + foo: "4" # {"$ref": "#/definitions/io.k8s.cli.setters.foo"} + `, + }, + { + name: "set-foo-type-wrong", + description: "if a type is specified for a setter, for the field to the type", + setter: "foo", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.foo: + x-k8s-cli: + setter: + name: foo + value: "4" + type: boolean + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + foo: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"} + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + foo: !!bool 4 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"} + `, + }, + { + name: "set-foo-no-type", + description: "if a type is not specified for a setter, keep the existing quoting", + setter: "foo", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.foo: + x-k8s-cli: + setter: + name: foo + value: "4" + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + foo: 3 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"} + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment + annotations: + foo: 4 # {"$ref": "#/definitions/io.k8s.cli.setters.foo"} `, }, { @@ -632,6 +725,29 @@ openAPI: setter: name: no-match-2 value: "2" +`, + }, + { + name: "set-annotation-quoted", + setter: "replicas", + value: "3", + input: ` +openAPI: + definitions: + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: 4 + `, + expected: ` +openAPI: + definitions: + io.k8s.cli.setters.replicas: + x-k8s-cli: + setter: + name: replicas + value: "3" `, }, { diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 121be30ce..8b6221335 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -19,6 +19,8 @@ type SetterCreator struct { SetBy string + Type string + // FieldName if set will add the OpenAPI reference to fields with this name or path // FieldName may be the full name of the field, full path to the field, or the path suffix. // e.g. all of the following would match spec.template.spec.containers.image -- @@ -35,7 +37,9 @@ type SetterCreator struct { func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { // Update the OpenAPI definitions to hace the setter sd := setters2.SetterDefinition{ - Name: c.Name, Value: c.FieldValue, Description: c.Description, SetBy: c.SetBy} + Name: c.Name, Value: c.FieldValue, Description: c.Description, SetBy: c.SetBy, + Type: c.Type, + } if err := sd.AddToFile(openAPIPath); err != nil { return err } diff --git a/kyaml/setters2/types.go b/kyaml/setters2/types.go index 933c5f58e..b896a2cd7 100644 --- a/kyaml/setters2/types.go +++ b/kyaml/setters2/types.go @@ -57,32 +57,32 @@ func getExtFromSchema(schema *spec.Schema) (*cliExtension, error) { // getExtFromComment returns the cliExtension openAPI extension if it is present as // a comment on the field. -func getExtFromComment(object *yaml.RNode) (*cliExtension, error) { +func getExtFromComment(object *yaml.RNode) (*cliExtension, *spec.Schema, error) { // TODO(pwittrock): also use path to the field to get openapi, not just comments // parse comment containing the extended openapi for this field fm := fieldmeta.FieldMeta{} if err := fm.Read(object); err != nil { - return nil, errors.Wrap(err) + return nil, nil, errors.Wrap(err) } if fm.Schema.Ref.String() == "" { - return nil, nil + return nil, nil, nil } // resolve the comment reference to the extended openapi definitions r, err := openapi.Resolve(&fm.Schema.Ref) if err != nil { - return nil, errors.Wrap(err) + return nil, nil, errors.Wrap(err) } if r == nil { // no schema found // TODO(pwittrock): should this be an error if it doesn't resolve? - return nil, nil + return nil, nil, nil } // get the cli extension from the openapi (contains setter information) ext, err := getExtFromSchema(r) if err != nil { - return nil, errors.Wrap(err) + return nil, nil, errors.Wrap(err) } - return ext, nil + return ext, r, nil }