diff --git a/cmd/config/internal/commands/cmdlistsetters.go b/cmd/config/internal/commands/cmdlistsetters.go index e51f84fff..b43681fa4 100644 --- a/cmd/config/internal/commands/cmdlistsetters.go +++ b/cmd/config/internal/commands/cmdlistsetters.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "strings" "github.com/olekukonko/tablewriter" "github.com/spf13/cobra" @@ -67,8 +68,15 @@ func (r *ListSettersRunner) runE(c *cobra.Command, args []string) error { table.SetHeader([]string{"NAME", "VALUE", "SET BY", "DESCRIPTION", "COUNT"}) for i := range r.List.Setters { s := r.List.Setters[i] + v := s.Value + + // if the setter is for a list, populate the values + if len(s.ListValues) > 0 { + v = strings.Join(s.ListValues, ",") + v = fmt.Sprintf("[%s]", v) + } table.Append([]string{ - s.Name, s.Value, s.SetBy, s.Description, fmt.Sprintf("%d", s.Count)}) + s.Name, v, s.SetBy, s.Description, fmt.Sprintf("%d", s.Count)}) } table.Render() diff --git a/cmd/config/internal/commands/cmdset.go b/cmd/config/internal/commands/cmdset.go index d549a616b..6886d16ed 100644 --- a/cmd/config/internal/commands/cmdset.go +++ b/cmd/config/internal/commands/cmdset.go @@ -21,7 +21,7 @@ func NewSetRunner(parent string) *SetRunner { r := &SetRunner{} c := &cobra.Command{ Use: "set DIR NAME [VALUE]", - Args: cobra.RangeArgs(1, 3), + Args: cobra.MinimumNArgs(3), Short: commands.SetShort, Long: commands.SetLong, Example: commands.SetExamples, @@ -94,6 +94,11 @@ func (r *SetRunner) preRunE(c *cobra.Command, args []string) error { var err error r.Set.Name = args[1] r.Set.Value = args[2] + + // set remaining values as list values + if 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) diff --git a/kyaml/fieldmeta/fieldmeta.go b/kyaml/fieldmeta/fieldmeta.go index 6006f4466..a6a34cccc 100644 --- a/kyaml/fieldmeta/fieldmeta.go +++ b/kyaml/fieldmeta/fieldmeta.go @@ -140,11 +140,11 @@ func (it FieldValueType) Validate(value string) error { func (it FieldValueType) Tag() string { switch it { case String: - return "!!str" + return yaml.StringTag case Bool: - return "!!bool" + return yaml.BoolTag case Int: - return "!!int" + return yaml.IntTag } return "" } @@ -152,17 +152,17 @@ func (it FieldValueType) Tag() string { func (it FieldValueType) TagForValue(value string) string { switch it { case String: - return "!!str" + return yaml.StringTag case Bool: if _, err := strconv.ParseBool(string(it)); err != nil { return "" } - return "!!bool" + return yaml.BoolTag case Int: if _, err := strconv.ParseInt(string(it), 0, 32); err != nil { return "" } - return "!!int" + return yaml.IntTag } return "" } diff --git a/kyaml/openapi/openapi.go b/kyaml/openapi/openapi.go index 11c9d2422..5ff9507d3 100644 --- a/kyaml/openapi/openapi.go +++ b/kyaml/openapi/openapi.go @@ -210,6 +210,10 @@ func (rs *ResourceSchema) Elements() *ResourceSchema { // either not an array, or array has multiple types return nil } + if rs == nil || rs.Schema == nil || rs.Schema.Items == nil { + // no-scheme for the items + return nil + } s := *rs.Schema.Items.Schema for s.Ref.String() != "" { sc, e := Resolve(&s.Ref) diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 83575c070..93bc47f5f 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -43,9 +43,14 @@ func (a *Add) Filter(object *yaml.RNode) (*yaml.RNode, error) { return object, accept(a, object) } +func (a *Add) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema) error { + // no-op + return nil +} + // visitScalar implements visitor // visitScalar will set the field metadata on each scalar field whose name + value match -func (a *Add) visitScalar(object *yaml.RNode, p string) error { +func (a *Add) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error { // check if the field matches if a.FieldName != "" && !strings.HasSuffix(p, a.FieldName) { return nil @@ -96,6 +101,9 @@ type SetterDefinition struct { // Value is the value of the setter. Value string `yaml:"value"` + // ListValues are the value of a list setter. + ListValues []string `yaml:"listValues,omitempty"` + // SetBy is the person or role that last set the value. SetBy string `yaml:"setBy,omitempty"` diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index b75f61d56..f5d163015 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -27,10 +27,36 @@ func (s *Set) Filter(object *yaml.RNode) (*yaml.RNode, error) { return object, accept(s, object) } +// visitSequence will perform setters for sequences +func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error { + ext, err := getExtFromComment(schema) + if err != nil { + return err + } + if ext == nil || ext.Setter == nil || ext.Setter.Name != s.Name || + len(ext.Setter.ListValues) == 0 { + // setter was not invoked for this sequence + return nil + } + s.Count++ + + // set the values on the sequences + var elements []*yaml.Node + for i := range ext.Setter.ListValues { + v := ext.Setter.ListValues[i] + n := yaml.NewScalarRNode(v).YNode() + n.Style = yaml.DoubleQuotedStyle + elements = append(elements, n) + } + object.YNode().Content = elements + object.YNode().Style = yaml.FoldedStyle + return nil +} + // visitScalar -func (s *Set) visitScalar(object *yaml.RNode, p string) error { +func (s *Set) visitScalar(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error { // get the openAPI for this field describing how to apply the setter - ext, sch, err := getExtFromComment(object) + ext, err := getExtFromComment(schema) if err != nil { return err } @@ -39,13 +65,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, sch) { + if s.set(object, ext, schema.Schema) { s.Count++ return nil } // perform a substitution of the field if it matches - sub, err := s.substitute(object, ext, sch) + sub, err := s.substitute(object, ext, schema.Schema) if err != nil { return err } @@ -111,7 +137,7 @@ func (s *Set) substitute(field *yaml.RNode, ext *cliExtension, _ *spec.Schema) ( field.YNode().Value = p // substitutions are always strings - field.YNode().Tag = "!!str" + field.YNode().Tag = yaml.StringTag return true, nil } @@ -147,6 +173,9 @@ type SetOpenAPI struct { // Value is the current value of the setter Value string `yaml:"value"` + // ListValue is the current value for a list of items + ListValues []string `yaml:"listValue"` + Description string `yaml:"description"` SetBy string `yaml:"setBy"` @@ -159,8 +188,14 @@ func (s SetOpenAPI) UpdateFile(path string) error { func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { key := SetterDefinitionPrefix + s.Name - def, err := object.Pipe(yaml.Lookup( - "openAPI", "definitions", key, "x-k8s-cli", "setter")) + oa, err := object.Pipe(yaml.Lookup("openAPI", "definitions", key)) + if err != nil { + return nil, err + } + if oa == nil { + return nil, errors.Errorf("no setter %s found", s.Name) + } + def, err := oa.Pipe(yaml.Lookup("x-k8s-cli", "setter")) if err != nil { return nil, err } @@ -168,9 +203,16 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { return nil, errors.Errorf("no setter %s found", s.Name) } + // record the OpenAPI type for the setter + var t string + if n := oa.Field("type"); n != nil { + t = n.Value.YNode().Value + } + // if the setter contains an enumValues map, then ensure the set value appears // as a key in the map - if values, err := def.Pipe(yaml.Lookup("enumValues")); err != nil { + if values, err := def.Pipe( + yaml.Lookup("enumValues")); err != nil { // error looking up the enumValues return nil, err } else if values != nil { @@ -202,11 +244,40 @@ func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { // 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().Tag = yaml.StringTag v.YNode().Style = yaml.DoubleQuotedStyle - if err := def.PipeE(&yaml.FieldSetter{Name: "value", Value: v}); err != nil { - return nil, err + if t != "array" { + // set a scalar value + if err := def.PipeE(&yaml.FieldSetter{Name: "value", Value: v}); err != nil { + return nil, err + } + } else { + // set a list value + if err := def.PipeE(&yaml.FieldClearer{Name: "value"}); err != nil { + return nil, err + } + // create the list values + var elements []*yaml.Node + n := yaml.NewScalarRNode(s.Value).YNode() + n.Tag = yaml.StringTag + n.Style = yaml.DoubleQuotedStyle + elements = append(elements, n) + for i := range s.ListValues { + v := s.ListValues[i] + n := yaml.NewScalarRNode(v).YNode() + n.Style = yaml.DoubleQuotedStyle + elements = append(elements, n) + } + l := yaml.NewRNode(&yaml.Node{ + Kind: yaml.SequenceNode, + Content: elements, + }) + + def.YNode().Style = yaml.FoldedStyle + if err := def.PipeE(&yaml.FieldSetter{Name: "listValues", Value: l}); err != nil { + return nil, err + } } if err := def.PipeE(&yaml.FieldSetter{Name: "setBy", StringValue: s.SetBy}); err != nil { diff --git a/kyaml/setters2/set_test.go b/kyaml/setters2/set_test.go index 3414cf10d..64610ca9e 100644 --- a/kyaml/setters2/set_test.go +++ b/kyaml/setters2/set_test.go @@ -595,6 +595,76 @@ spec: containers: - name: nginx image: nginx:1.8.1 # {"$ref": "#/definitions/io.k8s.cli.substitutions.image"} + `, + }, + { + name: "set-args-list", + setter: "args", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.args: + x-k8s-cli: + type: array + setter: + name: args + listValues: ["1", "2", "3"] + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + # {"$ref": "#/definitions/io.k8s.cli.setters.args"} + replicas: [] + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + # {"$ref": "#/definitions/io.k8s.cli.setters.args"} + replicas: + - "1" + - "2" + - "3" + `, + }, + { + name: "set-args-list-replace", + setter: "args", + openapi: ` +openAPI: + definitions: + io.k8s.cli.setters.args: + x-k8s-cli: + type: array + setter: + name: args + listValues: ["1", "2", "3"] + `, + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + # {"$ref": "#/definitions/io.k8s.cli.setters.args"} + replicas: ["4", "5"] + `, + expected: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + # {"$ref": "#/definitions/io.k8s.cli.setters.args"} + replicas: + - "1" + - "2" + - "3" `, }, } @@ -678,6 +748,7 @@ func TestSetOpenAPI_Filter(t *testing.T) { name string setter string value string + values []string input string expected string description string @@ -1004,6 +1075,33 @@ openAPI: value: "2" `, }, + + { + name: "set-args-list", + setter: "args", + value: "2", + values: []string{"3", "4"}, + input: ` +openAPI: + definitions: + io.k8s.cli.setters.args: + type: array + x-k8s-cli: + setter: + name: args + listValues: ["1"] + `, + expected: ` +openAPI: + definitions: + io.k8s.cli.setters.args: + type: array + x-k8s-cli: + setter: + name: args + listValues: ["2", "3", "4"] +`, + }, } for i := range tests { test := tests[i] @@ -1015,7 +1113,7 @@ openAPI: // invoke the setter instance := &SetOpenAPI{ - Name: test.setter, Value: test.value, + Name: test.setter, Value: test.value, ListValues: test.values, SetBy: test.setBy, Description: test.description} result, err := instance.Filter(in) if test.err != "" { diff --git a/kyaml/setters2/settersutil/fieldsetter.go b/kyaml/setters2/settersutil/fieldsetter.go index bf2b44fa6..92881931b 100644 --- a/kyaml/setters2/settersutil/fieldsetter.go +++ b/kyaml/setters2/settersutil/fieldsetter.go @@ -17,6 +17,9 @@ type FieldSetter struct { // Value is the value to set Value string + // ListValues contains a list of values to set on a Sequence + ListValues []string + Description string SetBy string @@ -26,7 +29,12 @@ type FieldSetter struct { func (fs FieldSetter) Set(openAPIPath, resourcesPath string) (int, error) { // Update the OpenAPI definitions soa := setters2.SetOpenAPI{ - Name: fs.Name, Value: fs.Value, Description: fs.Description, SetBy: fs.SetBy} + Name: fs.Name, + Value: fs.Value, + ListValues: fs.ListValues, + Description: fs.Description, + SetBy: fs.SetBy, + } if err := soa.UpdateFile(openAPIPath); err != nil { return 0, err } diff --git a/kyaml/setters2/types.go b/kyaml/setters2/types.go index b896a2cd7..5b489bcb1 100644 --- a/kyaml/setters2/types.go +++ b/kyaml/setters2/types.go @@ -8,9 +8,7 @@ import ( "github.com/go-openapi/spec" "sigs.k8s.io/kustomize/kyaml/errors" - "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/openapi" - "sigs.k8s.io/kustomize/kyaml/yaml" ) type cliExtension struct { @@ -21,6 +19,7 @@ type cliExtension struct { type setter struct { Name string `yaml:"name,omitempty" json:"name,omitempty"` Value string `yaml:"value,omitempty" json:"value,omitempty"` + ListValues []string `yaml:"listValues,omitempty" json:"listValues,omitempty"` EnumValues map[string]string `yaml:"enumValues,omitempty" json:"enumValues,omitempty"` } @@ -57,32 +56,17 @@ 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, *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, nil, errors.Wrap(err) - } - if fm.Schema.Ref.String() == "" { - 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, nil, errors.Wrap(err) - } - if r == nil { +func getExtFromComment(schema *openapi.ResourceSchema) (*cliExtension, error) { + if schema == nil { // no schema found // TODO(pwittrock): should this be an error if it doesn't resolve? - return nil, nil, nil + return nil, nil } // get the cli extension from the openapi (contains setter information) - ext, err := getExtFromSchema(r) + ext, err := getExtFromSchema(schema.Schema) if err != nil { - return nil, nil, errors.Wrap(err) + return nil, errors.Wrap(err) } - return ext, r, nil + return ext, nil } diff --git a/kyaml/setters2/walk.go b/kyaml/setters2/walk.go index d084842ce..786a5b815 100644 --- a/kyaml/setters2/walk.go +++ b/kyaml/setters2/walk.go @@ -4,6 +4,8 @@ package setters2 import ( + "sigs.k8s.io/kustomize/kyaml/fieldmeta" + "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -13,33 +15,100 @@ type visitor interface { // visitScalar is called for each scalar field value on a resource // node is the scalar field value // path is the path to the field; path elements are separated by '.' - visitScalar(node *yaml.RNode, path string) error + // oa is the OpenAPI schema for the field + visitScalar(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error + + // visitSequence is called for each sequence field value on a resource + // node is the sequence field value + // path is the path to the field + // oa is the OpenAPI schema for the field + visitSequence(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error } // accept invokes the appropriate function on v for each field in object func accept(v visitor, object *yaml.RNode) error { - return acceptImpl(v, object, "") + // get the OpenAPI for the type if it exists + oa := getSchema(object, nil, "") + return acceptImpl(v, object, "", oa) } // acceptImpl implements accept using recursion -func acceptImpl(v visitor, object *yaml.RNode, p string) error { +func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSchema) error { switch object.YNode().Kind { case yaml.DocumentNode: // Traverse the child of the document return accept(v, yaml.NewRNode(object.YNode())) case yaml.MappingNode: return object.VisitFields(func(node *yaml.MapNode) error { + // get the schema for the field and propagate it + oa = getSchema(node.Key, oa, node.Key.YNode().Value) // Traverse each field value - return acceptImpl(v, node.Value, p+"."+node.Key.YNode().Value) + return acceptImpl(v, node.Value, p+"."+node.Key.YNode().Value, oa) }) case yaml.SequenceNode: + // get the schema for the sequence node, use the schema provided if not present + // on the field + if err := v.visitSequence(object, p, oa); err != nil { + return err + } + // get the schema for the elements + oa = getSchema(object, oa, "") return object.VisitElements(func(node *yaml.RNode) error { // Traverse each list element - return acceptImpl(v, node, p) + return acceptImpl(v, node, p, oa) }) case yaml.ScalarNode: // Visit the scalar field - return v.visitScalar(object, p) + oa = getSchema(object, oa, "") + return v.visitScalar(object, p, oa) } return nil } + +// getSchema returns OpenAPI schema for an RNode or field of the +// RNode. It will overriding the provide schema with field specific values +// if they are found +// r is the Node to get the Schema for +// s is the provided schema for the field if known +// field is the name of the field +func getSchema(r *yaml.RNode, s *openapi.ResourceSchema, field string) *openapi.ResourceSchema { + // get the override schema if it exists on the field + fm := fieldmeta.FieldMeta{} + if err := fm.Read(r); err == nil && !fm.IsEmpty() { + // per-field schema, this is fine + if fm.Schema.Ref.String() != "" { + // resolve the reference + s, err := openapi.Resolve(&fm.Schema.Ref) + if err == nil && s != nil { + fm.Schema = *s + } + } + return &openapi.ResourceSchema{Schema: &fm.Schema} + } + + // get the schema for a field of the node if the field is provided + if s != nil && field != "" { + return s.Field(field) + } + + // get the schema for the elements if this is a list + if s != nil && r.YNode().Kind == yaml.SequenceNode { + return s.Elements() + } + + // use the provided schema if present + if s != nil { + return s + } + + if yaml.IsEmpty(r) { + return nil + } + + // lookup the schema for the type + m, _ := r.GetMeta() + if m.Kind == "" || m.APIVersion == "" { + return nil + } + return openapi.SchemaForResourceType(yaml.TypeMeta{Kind: m.Kind, APIVersion: m.APIVersion}) +} diff --git a/kyaml/yaml/compatibility.go b/kyaml/yaml/compatibility.go index ff64f6f06..628feb0ad 100644 --- a/kyaml/yaml/compatibility.go +++ b/kyaml/yaml/compatibility.go @@ -14,9 +14,9 @@ import ( // typeToTag maps OpenAPI schema types to yaml 1.2 tags var typeToTag = map[string]string{ - "string": "!!str", - "integer": "!!int", - "boolean": "!!bool", + "string": StringTag, + "integer": IntTag, + "boolean": BoolTag, "number": "!!float", } diff --git a/kyaml/yaml/kfns.go b/kyaml/yaml/kfns.go index 84d3f2af7..229c9c4ca 100644 --- a/kyaml/yaml/kfns.go +++ b/kyaml/yaml/kfns.go @@ -3,7 +3,9 @@ package yaml -import "gopkg.in/yaml.v3" +import ( + "gopkg.in/yaml.v3" +) // AnnotationClearer removes an annotation at metadata.annotations. // Returns nil if the annotation or field does not exist. @@ -33,7 +35,7 @@ type AnnotationSetter struct { func (s AnnotationSetter) Filter(rn *RNode) (*RNode, error) { // some tools get confused about the type if annotations are not quoted v := NewScalarRNode(s.Value) - v.YNode().Tag = "!!str" + v.YNode().Tag = StringTag v.YNode().Style = yaml.SingleQuotedStyle return rn.Pipe( PathGetter{Path: []string{"metadata", "annotations"}, Create: yaml.MappingNode}, @@ -80,7 +82,7 @@ type LabelSetter struct { func (s LabelSetter) Filter(rn *RNode) (*RNode, error) { // some tools get confused about the type if labels are not quoted v := NewScalarRNode(s.Value) - v.YNode().Tag = "!!str" + v.YNode().Tag = StringTag v.YNode().Style = yaml.SingleQuotedStyle return rn.Pipe( PathGetter{Path: []string{"metadata", "labels"}, Create: yaml.MappingNode}, diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index cedf1f878..1a2af996f 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -588,6 +588,12 @@ func (rn *RNode) VisitFields(fn func(node *MapNode) error) error { return nil } +const ( + StringTag = "!!str" + BoolTag = "!!bool" + IntTag = "!!int" +) + // Elements returns the list of elements in the RNode. // Returns an error for non-SequenceNodes. func (rn *RNode) Elements() ([]*RNode, error) {