From 0f0efe2a4cb990dc60bfa8896a98bdaa67d370c0 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Mon, 15 Jun 2020 17:18:38 -0700 Subject: [PATCH] Avoid manual step for creating array setters --- .../internal/commands/cmdcreatesetter.go | 10 +- .../internal/commands/cmdcreatesetter_test.go | 19 ++- .../internal/commands/cmdlistsetters.go | 15 ++- .../internal/commands/cmdlistsetters_test.go | 114 +++++++++++++++++- kyaml/setters2/add.go | 40 +++++- kyaml/setters2/set.go | 4 + kyaml/setters2/settersutil/settercreator.go | 32 +++-- kyaml/setters2/walk.go | 9 ++ kyaml/yaml/compatibility_test.go | 2 +- 9 files changed, 219 insertions(+), 26 deletions(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index f2ea66ab3..b107adea6 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -91,7 +91,9 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { } if setterVersion == "" { - if len(args) < 2 || !c.Flag("value").Changed && len(args) < 3 { + if len(args) == 2 && r.Set.SetPartialField.Type == "array" && c.Flag("field").Changed { + setterVersion = "v2" + } else if len(args) < 2 || !c.Flag("value").Changed && len(args) < 3 { setterVersion = "v1" } else if err := initSetterVersion(c, args); err != nil { return err @@ -124,6 +126,12 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { r.CreateSetter.Description = r.Set.SetPartialField.Description r.CreateSetter.SetBy = r.Set.SetPartialField.SetBy r.CreateSetter.Type = r.Set.SetPartialField.Type + + if r.CreateSetter.Type == "array" { + // this is just a place holder value, we derive the actual value at + // later point from the field path + r.CreateSetter.FieldValue = "listValues" + } } return nil } diff --git a/cmd/config/internal/commands/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index c584230c6..461a26db5 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -130,14 +130,16 @@ spec: { name: "add replicas with schema list values", - args: []string{"list", "a", "--description", "hello world", "--set-by", "me", "--type", "array"}, - schema: `{"maxItems": 2, "type": "array", "items": {"type": "string"}}`, + args: []string{"list", "a", "--description", "hello world", "--set-by", "me", "--type", "array", "--field", "spec.list"}, + schema: `{"maxItems": 3, "type": "array", "items": {"type": "string"}}`, input: ` apiVersion: example.com/v1beta1 kind: Example spec: list: - "a" + - "b" + - "c" `, inputOpenAPI: ` apiVersion: v1alpha1 @@ -151,21 +153,26 @@ openAPI: io.k8s.cli.setters.list: items: type: string - maxItems: 2 + maxItems: 3 type: array description: hello world x-k8s-cli: setter: name: list - value: a + listValues: + - a + - b + - c setBy: me `, expectedResources: ` apiVersion: example.com/v1beta1 kind: Example spec: - list: - - "a" # {"$openapi":"list"} + list: # {"$openapi":"list"} + - "a" + - "b" + - "c" `, }, { diff --git a/cmd/config/internal/commands/cmdlistsetters.go b/cmd/config/internal/commands/cmdlistsetters.go index 273d9c671..cb84a21a3 100644 --- a/cmd/config/internal/commands/cmdlistsetters.go +++ b/cmd/config/internal/commands/cmdlistsetters.go @@ -113,17 +113,20 @@ func (r *ListSettersRunner) ListSubstitutions(c *cobra.Command, args []string) e return err } table := newTable(c.OutOrStdout(), r.Markdown) - table.SetHeader([]string{"SUBSTITUTION", "PATTERN", "SETTERS"}) + table.SetHeader([]string{"SUBSTITUTION", "PATTERN", "REFERENCES"}) for i := range r.List.Substitutions { s := r.List.Substitutions[i] - setters := "" + refs := "" for _, value := range s.Values { - setter := strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix+fieldmeta.SetterDefinitionPrefix) - setters = setters + "," + setter + // trim setter and substitution prefixes + ref := strings.TrimPrefix( + strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix+fieldmeta.SetterDefinitionPrefix), + fieldmeta.DefinitionsPrefix+fieldmeta.SubstitutionDefinitionPrefix) + refs = refs + "," + ref } - setters = fmt.Sprintf("[%s]", strings.TrimPrefix(setters, ",")) + refs = fmt.Sprintf("[%s]", strings.TrimPrefix(refs, ",")) table.Append([]string{ - s.Name, s.Pattern, setters}) + s.Name, s.Pattern, refs}) } if len(r.List.Substitutions) == 0 { return nil diff --git a/cmd/config/internal/commands/cmdlistsetters_test.go b/cmd/config/internal/commands/cmdlistsetters_test.go index 77d475a73..c870f73ec 100644 --- a/cmd/config/internal/commands/cmdlistsetters_test.go +++ b/cmd/config/internal/commands/cmdlistsetters_test.go @@ -104,7 +104,7 @@ spec: image nginx me2 hello world 2 2 replicas 3 me1 hello world 1 1 tag 1.7.9 me3 hello world 3 1 - SUBSTITUTION PATTERN SETTERS + SUBSTITUTION PATTERN REFERENCES image IMAGE:TAG [image,tag] `, }, @@ -178,7 +178,7 @@ spec: image nginx me2 hello world 2 3 replicas 3 me1 hello world 1 2 tag 1.7.9 me3 hello world 3 2 - SUBSTITUTION PATTERN SETTERS + SUBSTITUTION PATTERN REFERENCES image IMAGE:TAG [image,tag] `, }, @@ -251,8 +251,116 @@ spec: `, expected: ` NAME VALUE SET BY DESCRIPTION COUNT image nginx me2 hello world 2 3 - SUBSTITUTION PATTERN SETTERS + SUBSTITUTION PATTERN REFERENCES image IMAGE:TAG [image,tag] +`, + }, + + { + name: "list array setter", + openapi: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.list: + items: + type: string + maxItems: 3 + type: array + description: hello world + x-k8s-cli: + setter: + name: list + value: List Values + listValues: + - a + - b + - c + setBy: me + `, + input: ` +apiVersion: example.com/v1beta1 +kind: Example +metadata: + annotations: + foo: bar +spec: + list: # {"$openapi":"list"} + - "a" + - "b" + - "c" +`, + expected: ` NAME VALUE SET BY DESCRIPTION COUNT + list [a,b,c] me hello world 1 +`, + }, + + { + name: "nested substitution", + input: ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx-deployment +spec: + replicas: 3 + template: + spec: + containers: + - name: nginx + image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-nested-subst"} + - name: sidecar + image: nginx::1.7.9 # {"$openapi":"my-image-subst"} + `, + openapi: ` +apiVersion: v1alpha1 +kind: Example +openAPI: + definitions: + io.k8s.cli.setters.my-image-setter: + x-k8s-cli: + setter: + name: my-image-setter + value: nginx + io.k8s.cli.setters.my-tag-setter: + x-k8s-cli: + setter: + name: my-tag-setter + value: 1.7.9 + io.k8s.cli.substitutions.my-image-subst: + x-k8s-cli: + substitution: + name: my-image-subst + pattern: ${my-image-setter}::${my-tag-setter} + values: + - marker: ${my-image-setter} + ref: '#/definitions/io.k8s.cli.setters.my-image-setter' + - marker: ${my-tag-setter} + ref: '#/definitions/io.k8s.cli.setters.my-tag-setter' + io.k8s.cli.substitutions.my-nested-subst: + x-k8s-cli: + substitution: + name: my-nested-subst + pattern: something/${my-image-subst}/${my-other-setter} + values: + - marker: ${my-image-subst} + ref: '#/definitions/io.k8s.cli.substitutions.my-image-subst' + - marker: ${my-other-setter} + ref: '#/definitions/io.k8s.cli.setters.my-other-setter' + io.k8s.cli.setters.my-other-setter: + x-k8s-cli: + setter: + name: my-other-setter + value: nginxotherthing + `, + expected: ` NAME VALUE SET BY DESCRIPTION COUNT + my-image-setter nginx 2 + my-other-setter nginxotherthing 1 + my-tag-setter 1.7.9 2 + SUBSTITUTION PATTERN REFERENCES + my-image-subst ${my-image-setter}::${my-tag-setter} [my-image-setter,my-tag-setter] + my-nested-subst something/${my-image-subst}/${my-other-setter} [my-image-subst,my-other-setter] `, }, } diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 8d2e2c940..b7a7bf5a5 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -30,6 +30,9 @@ type Add struct { // Ref is the OpenAPI reference to set on the matching fields as a comment. Ref string + + // ListValues are the value of a list setter. + ListValues []string } // Filter implements yaml.Filter @@ -48,6 +51,37 @@ func (a *Add) visitSequence(_ *yaml.RNode, _ string, _ *openapi.ResourceSchema) return nil } +// visitMapping implements visitor +// visitMapping visits the fields in input MappingNode and adds setter/subst ref +// if the path path spec matches with input FiledName +func (a *Add) visitMapping(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error { + return object.VisitFields(func(node *yaml.MapNode) error { + if node.Value.YNode().Kind != yaml.SequenceNode { + return nil + } + + key, err := node.Key.String() + if err != nil { + return err + } + + // derive the list values for the sequence node to write it to openAPI definitions + var values []string + for _, sc := range node.Value.Content() { + values = append(values, sc.Value) + } + a.ListValues = values + + // pathToKey refers to the path address of the key node ex: metadata.annotations + // p is the path till parent node, pathToKey is obtained by appending child key + pathToKey := p + "." + strings.Trim(key, "\n") + if a.FieldName != "" && strings.HasSuffix(pathToKey, a.FieldName) { + return a.addRef(node.Key) + } + 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, _ *openapi.ResourceSchema) error { @@ -58,7 +92,11 @@ func (a *Add) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchem if a.FieldValue != "" && a.FieldValue != object.YNode().Value { return nil } + return a.addRef(object) +} +// addRef adds the setter/subst ref to the object node as a line comment +func (a *Add) addRef(object *yaml.RNode) error { // read the field metadata fm := fieldmeta.FieldMeta{} if err := fm.Read(object); err != nil { @@ -85,7 +123,7 @@ type SetterDefinition struct { Name string `yaml:"name"` // Value is the value of the setter. - Value string `yaml:"value"` + Value string `yaml:"value,omitempty"` // ListValues are the value of a list setter. ListValues []string `yaml:"listValues,omitempty"` diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index de8d85e74..ea624bfae 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -45,6 +45,10 @@ func (s *Set) isMatch(name string) bool { return s.SetAll || s.Name == name } +func (s *Set) visitMapping(object *yaml.RNode, p string, _ *openapi.ResourceSchema) error { + return nil +} + // visitSequence will perform setters for sequences func (s *Set) visitSequence(object *yaml.RNode, p string, schema *openapi.ResourceSchema) error { ext, err := getExtFromComment(schema) diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 9cc12e787..200275a52 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -60,16 +60,32 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { // Update the resources with the setter reference inout := &kio.LocalPackageReadWriter{PackagePath: resourcesPath} - return kio.Pipeline{ - Inputs: []kio.Reader{inout}, - Filters: []kio.Filter{kio.FilterAll( - &setters2.Add{ - FieldName: c.FieldName, - FieldValue: c.FieldValue, - Ref: fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name, - })}, + a := &setters2.Add{ + FieldName: c.FieldName, + FieldValue: c.FieldValue, + Ref: fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name, + } + err = kio.Pipeline{ + Inputs: []kio.Reader{inout}, + Filters: []kio.Filter{kio.FilterAll(a)}, Outputs: []kio.Writer{inout}, }.Execute() + + if err != nil { + return err + } + + // if the setter is of array type write the derived list values back to + // openAPI definitions + if len(a.ListValues) > 0 { + sd.ListValues = a.ListValues + sd.Value = "" + if err := sd.AddToFile(openAPIPath); err != nil { + return err + } + } + + return nil } // schemaFromFile reads the contents from schemaPath and returns schema diff --git a/kyaml/setters2/walk.go b/kyaml/setters2/walk.go index 786a5b815..6ca698336 100644 --- a/kyaml/setters2/walk.go +++ b/kyaml/setters2/walk.go @@ -23,6 +23,12 @@ type visitor interface { // 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 + + // visitMapping is called for each Mapping field value on a resource + // node is the mapping field value + // path is the path to the field + // oa is the OpenAPI schema for the field + visitMapping(node *yaml.RNode, path string, oa *openapi.ResourceSchema) error } // accept invokes the appropriate function on v for each field in object @@ -39,6 +45,9 @@ func acceptImpl(v visitor, object *yaml.RNode, p string, oa *openapi.ResourceSch // Traverse the child of the document return accept(v, yaml.NewRNode(object.YNode())) case yaml.MappingNode: + if err := v.visitMapping(object, p, oa); err != nil { + return err + } 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) diff --git a/kyaml/yaml/compatibility_test.go b/kyaml/yaml/compatibility_test.go index c2b9aa77f..02e83cbe7 100644 --- a/kyaml/yaml/compatibility_test.go +++ b/kyaml/yaml/compatibility_test.go @@ -23,7 +23,7 @@ func TestIsYaml1_1NonString(t *testing.T) { {val: "2", expected: true}, {val: "true", expected: true}, {val: "1.0\nhello", expected: false}, // multiline strings should always be false - {val: "", expected: false}, // empty string should be considered a string + {val: "", expected: false}, // empty string should be considered a string } for k := range valueToTagMap {