From b3f5874978d4a5af80220ca2f6ed082a1db5af1c Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Wed, 20 May 2020 16:46:27 -0700 Subject: [PATCH] Short hand notation for set --- .../internal/commands/cmdcreatesetter.go | 4 +- .../commands/cmdcreatesubstitution.go | 5 +- .../internal/commands/cmdlistsetters.go | 3 +- cmd/config/internal/commands/cmdset_test.go | 20 ++--- kyaml/fieldmeta/fieldmeta.go | 82 +++++++++++++++++-- kyaml/setters2/add.go | 18 +--- kyaml/setters2/list.go | 5 +- kyaml/setters2/set.go | 3 +- kyaml/setters2/settersutil/settercreator.go | 3 +- .../settersutil/substitutioncreator.go | 3 +- 10 files changed, 103 insertions(+), 43 deletions(-) diff --git a/cmd/config/internal/commands/cmdcreatesetter.go b/cmd/config/internal/commands/cmdcreatesetter.go index dd3174c74..2a0120aac 100644 --- a/cmd/config/internal/commands/cmdcreatesetter.go +++ b/cmd/config/internal/commands/cmdcreatesetter.go @@ -9,10 +9,10 @@ import ( "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/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/setters" - "sigs.k8s.io/kustomize/kyaml/setters2" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" ) @@ -102,7 +102,7 @@ func (r *CreateSetterRunner) preRunE(c *cobra.Command, args []string) error { } // check if substitution with same name exists and throw error - ref, err := spec.NewRef(setters2.DefinitionsPrefix + setters2.SubstitutionDefinitionPrefix + r.CreateSetter.Name) + ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + r.CreateSetter.Name) if err != nil { return err } diff --git a/cmd/config/internal/commands/cmdcreatesubstitution.go b/cmd/config/internal/commands/cmdcreatesubstitution.go index b4cb47988..e0a12ae24 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" "sigs.k8s.io/kustomize/cmd/config/ext" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/setters2" "sigs.k8s.io/kustomize/kyaml/setters2/settersutil" @@ -71,7 +72,7 @@ func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) erro } // check if setter with same name exists and throw error - ref, err := spec.NewRef(setters2.DefinitionsPrefix + setters2.SetterDefinitionPrefix + r.CreateSubstitution.Name) + ref, err := spec.NewRef(fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + r.CreateSubstitution.Name) if err != nil { return err } @@ -92,7 +93,7 @@ func (r *CreateSubstitutionRunner) preRunE(c *cobra.Command, args []string) erro } for _, marker := range markers { - ref := setters2.DefinitionsPrefix + setters2.SetterDefinitionPrefix + + ref := fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + strings.TrimSuffix(strings.TrimPrefix(string(marker), "${"), "}") r.CreateSubstitution.Values = append( r.CreateSubstitution.Values, diff --git a/cmd/config/internal/commands/cmdlistsetters.go b/cmd/config/internal/commands/cmdlistsetters.go index d88a4802d..d5fc93610 100644 --- a/cmd/config/internal/commands/cmdlistsetters.go +++ b/cmd/config/internal/commands/cmdlistsetters.go @@ -13,6 +13,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/fieldmeta" "sigs.k8s.io/kustomize/kyaml/setters" "sigs.k8s.io/kustomize/kyaml/setters2" ) @@ -117,7 +118,7 @@ func (r *ListSettersRunner) ListSubstitutions(c *cobra.Command, args []string) e s := r.List.Substitutions[i] setters := "" for _, value := range s.Values { - setter := strings.TrimPrefix(value.Ref, setters2.DefinitionsPrefix+setters2.SetterDefinitionPrefix) + setter := strings.TrimPrefix(value.Ref, fieldmeta.DefinitionsPrefix+fieldmeta.SetterDefinitionPrefix) setters = setters + "," + setter } setters = fmt.Sprintf("[%s]", strings.TrimPrefix(setters, ",")) diff --git a/cmd/config/internal/commands/cmdset_test.go b/cmd/config/internal/commands/cmdset_test.go index 7bf532263..c10edcf08 100644 --- a/cmd/config/internal/commands/cmdset_test.go +++ b/cmd/config/internal/commands/cmdset_test.go @@ -50,7 +50,7 @@ kind: Deployment metadata: name: nginx-deployment spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openAPI":"replicas"} `, expectedOpenAPI: ` apiVersion: v1alpha1 @@ -71,7 +71,7 @@ kind: Deployment metadata: name: nginx-deployment spec: - replicas: 4 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 4 # {"$openAPI":"replicas"} `, }, { @@ -129,10 +129,10 @@ apiVersion: v1alpha1 kind: Example openAPI: definitions: - io.k8s.cli.setters.image: + io.k8s.cli.setters.image-setter: x-k8s-cli: setter: - name: image + name: image-setter value: "nginx" io.k8s.cli.setters.tag: x-k8s-cli: @@ -146,7 +146,7 @@ openAPI: pattern: IMAGE:TAG values: - marker: IMAGE - ref: '#/definitions/io.k8s.cli.setters.image' + ref: '#/definitions/io.k8s.cli.setters.image-setter' - marker: TAG ref: '#/definitions/io.k8s.cli.setters.tag' `, @@ -161,7 +161,7 @@ spec: spec: containers: - name: nginx - image: nginx:1.7.9 # {"$ref":"#/definitions/io.k8s.cli.substitutions.image"} + image: nginx:1.7.9 # {"$openAPI":"image"} - name: sidecar image: sidecar:1.7.9 `, @@ -170,10 +170,10 @@ apiVersion: v1alpha1 kind: Example openAPI: definitions: - io.k8s.cli.setters.image: + io.k8s.cli.setters.image-setter: x-k8s-cli: setter: - name: image + name: image-setter value: "nginx" io.k8s.cli.setters.tag: x-k8s-cli: @@ -187,7 +187,7 @@ openAPI: pattern: IMAGE:TAG values: - marker: IMAGE - ref: '#/definitions/io.k8s.cli.setters.image' + ref: '#/definitions/io.k8s.cli.setters.image-setter' - marker: TAG ref: '#/definitions/io.k8s.cli.setters.tag' @@ -203,7 +203,7 @@ spec: spec: containers: - name: nginx - image: nginx:1.8.1 # {"$ref":"#/definitions/io.k8s.cli.substitutions.image"} + image: nginx:1.8.1 # {"$openAPI":"image"} - name: sidecar image: sidecar:1.7.9 `, diff --git a/kyaml/fieldmeta/fieldmeta.go b/kyaml/fieldmeta/fieldmeta.go index a6a34cccc..663832efc 100644 --- a/kyaml/fieldmeta/fieldmeta.go +++ b/kyaml/fieldmeta/fieldmeta.go @@ -11,6 +11,7 @@ import ( "github.com/go-openapi/spec" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/yaml" ) @@ -54,14 +55,17 @@ func (fm *FieldMeta) Read(n *yaml.RNode) error { continue } c := strings.TrimLeft(c, "#") - // if it doesn't Unmarshal that is fine, it means there is no metadata - // other comments are valid, they just don't parse - // TODO: consider more sophisticated parsing techniques similar to what is used - // for go struct tags. - if err := fm.Schema.UnmarshalJSON([]byte(c)); err != nil { - // note: don't return an error if the comment isn't a fieldmeta struct - return nil + // check for new short hand notation or fall back to openAPI ref format + if !fm.processShortHand(c) { + // if it doesn't Unmarshal that is fine, it means there is no metadata + // other comments are valid, they just don't parse + // TODO: consider more sophisticated parsing techniques similar to what is used + // for go struct tags. + if err := fm.Schema.UnmarshalJSON([]byte(c)); err != nil { + // note: don't return an error if the comment isn't a fieldmeta struct + return nil + } } fe := fm.Schema.VendorExtensible.Extensions["x-kustomize"] if fe == nil { @@ -76,6 +80,53 @@ func (fm *FieldMeta) Read(n *yaml.RNode) error { return nil } +// processShortHand parses the comment for short hand ref and loads schema to fm +func (fm *FieldMeta) processShortHand(comment string) bool { + input := map[string]string{} + err := json.Unmarshal([]byte(comment), &input) + if err != nil { + return false + } + name := input[ShortHandRef] + if name == "" { + return false + } + + // check if setter with the name exists, else check for a substitution + // setter and substitution can't have same name in shorthand + + setterRef, err := spec.NewRef(DefinitionsPrefix + SetterDefinitionPrefix + name) + if err != nil { + return false + } + + setterRefBytes, err := setterRef.MarshalJSON() + if err != nil { + return false + } + + if _, err := openapi.Resolve(&setterRef); err == nil { + setterErr := fm.Schema.UnmarshalJSON(setterRefBytes) + return setterErr == nil + } + + substRef, err := spec.NewRef(DefinitionsPrefix + SubstitutionDefinitionPrefix + name) + if err != nil { + return false + } + + substRefBytes, err := substRef.MarshalJSON() + if err != nil { + return false + } + + if _, err := openapi.Resolve(&substRef); err == nil { + substErr := fm.Schema.UnmarshalJSON(substRefBytes) + return substErr == nil + } + return false +} + func isExtensionEmpty(x XKustomize) bool { if x.FieldSetter != nil { return false @@ -166,3 +217,20 @@ func (it FieldValueType) TagForValue(value string) string { } return "" } + +const ( + // CLIDefinitionsPrefix is the prefix for cli definition keys. + CLIDefinitionsPrefix = "io.k8s.cli." + + // SetterDefinitionPrefix is the prefix for setter definition keys. + SetterDefinitionPrefix = CLIDefinitionsPrefix + "setters." + + // SubstitutionDefinitionPrefix is the prefix for substitution definition keys. + SubstitutionDefinitionPrefix = CLIDefinitionsPrefix + "substitutions." + + // DefinitionsPrefix is the prefix used to reference definitions in the OpenAPI + DefinitionsPrefix = "#/definitions/" + + // ShortHandRef is the shorthand reference to setters and substitutions + ShortHandRef = "$openAPI" +) diff --git a/kyaml/setters2/add.go b/kyaml/setters2/add.go index 8b2946eed..8d2e2c940 100644 --- a/kyaml/setters2/add.go +++ b/kyaml/setters2/add.go @@ -79,20 +79,6 @@ func (a *Add) visitScalar(object *yaml.RNode, p string, _ *openapi.ResourceSchem return nil } -const ( - // CLIDefinitionsPrefix is the prefix for cli definition keys. - CLIDefinitionsPrefix = "io.k8s.cli." - - // SetterDefinitionPrefix is the prefix for setter definition keys. - SetterDefinitionPrefix = CLIDefinitionsPrefix + "setters." - - // SubstitutionDefinitionPrefix is the prefix for substitution definition keys. - SubstitutionDefinitionPrefix = CLIDefinitionsPrefix + "substitutions." - - // DefinitionsPrefix is the prefix used to reference definitions in the OpenAPI - DefinitionsPrefix = "#/definitions/" -) - // SetterDefinition may be used to update a files OpenAPI definitions with a new setter. type SetterDefinition struct { // Name is the name of the setter to create or update. @@ -133,7 +119,7 @@ func (sd SetterDefinition) AddToFile(path string) error { } func (sd SetterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) { - key := SetterDefinitionPrefix + sd.Name + key := fieldmeta.SetterDefinitionPrefix + sd.Name definitions, err := object.Pipe(yaml.LookupCreate( yaml.MappingNode, openapi.SupplementaryOpenAPIFieldName, "definitions")) @@ -235,7 +221,7 @@ func (sd SubstitutionDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) } // lookup or create the definition for the substitution - defKey := SubstitutionDefinitionPrefix + sd.Name + defKey := fieldmeta.SubstitutionDefinitionPrefix + sd.Name def, err := object.Pipe(yaml.LookupCreate( yaml.MappingNode, openapi.SupplementaryOpenAPIFieldName, "definitions", defKey, "x-k8s-cli")) if err != nil { diff --git a/kyaml/setters2/list.go b/kyaml/setters2/list.go index ca00f944c..0a15ded99 100644 --- a/kyaml/setters2/list.go +++ b/kyaml/setters2/list.go @@ -8,6 +8,7 @@ import ( "strings" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -63,7 +64,7 @@ func (l *List) listSetters(object *yaml.RNode, resourcePath string) error { // the definition key -- contains the setter name key := node.Key.YNode().Value - if !strings.HasPrefix(key, SetterDefinitionPrefix) { + if !strings.HasPrefix(key, fieldmeta.SetterDefinitionPrefix) { // not a setter -- doesn't have the right prefix return nil } @@ -136,7 +137,7 @@ func (l *List) listSubst(object *yaml.RNode) error { // the definition key -- contains the substitution name key := node.Key.YNode().Value - if !strings.HasPrefix(key, SubstitutionDefinitionPrefix) { + if !strings.HasPrefix(key, fieldmeta.SubstitutionDefinitionPrefix) { // not a substitution -- doesn't have the right prefix return nil } diff --git a/kyaml/setters2/set.go b/kyaml/setters2/set.go index e7daedb6c..39a63a5a5 100644 --- a/kyaml/setters2/set.go +++ b/kyaml/setters2/set.go @@ -12,6 +12,7 @@ import ( "github.com/go-openapi/strfmt" "github.com/go-openapi/validate" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/openapi" @@ -268,7 +269,7 @@ func (s SetOpenAPI) UpdateFile(path string) error { } func (s SetOpenAPI) Filter(object *yaml.RNode) (*yaml.RNode, error) { - key := SetterDefinitionPrefix + s.Name + key := fieldmeta.SetterDefinitionPrefix + s.Name oa, err := object.Pipe(yaml.Lookup("openAPI", "definitions", key)) if err != nil { return nil, err diff --git a/kyaml/setters2/settersutil/settercreator.go b/kyaml/setters2/settersutil/settercreator.go index 03094fb53..9cc12e787 100644 --- a/kyaml/setters2/settersutil/settercreator.go +++ b/kyaml/setters2/settersutil/settercreator.go @@ -6,6 +6,7 @@ package settersutil import ( "io/ioutil" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/setters2" @@ -65,7 +66,7 @@ func (c SetterCreator) Create(openAPIPath, resourcesPath string) error { &setters2.Add{ FieldName: c.FieldName, FieldValue: c.FieldValue, - Ref: setters2.DefinitionsPrefix + setters2.SetterDefinitionPrefix + c.Name, + Ref: fieldmeta.DefinitionsPrefix + fieldmeta.SetterDefinitionPrefix + c.Name, })}, Outputs: []kio.Writer{inout}, }.Execute() diff --git a/kyaml/setters2/settersutil/substitutioncreator.go b/kyaml/setters2/settersutil/substitutioncreator.go index e31cb8dd9..e654c7037 100644 --- a/kyaml/setters2/settersutil/substitutioncreator.go +++ b/kyaml/setters2/settersutil/substitutioncreator.go @@ -8,6 +8,7 @@ import ( "strings" "sigs.k8s.io/kustomize/kyaml/errors" + "sigs.k8s.io/kustomize/kyaml/fieldmeta" "sigs.k8s.io/kustomize/kyaml/kio" "sigs.k8s.io/kustomize/kyaml/openapi" "sigs.k8s.io/kustomize/kyaml/setters2" @@ -68,7 +69,7 @@ func (c SubstitutionCreator) Create(openAPIPath, resourcesPath string) error { &setters2.Add{ FieldName: c.FieldName, FieldValue: c.FieldValue, - Ref: setters2.DefinitionsPrefix + setters2.SubstitutionDefinitionPrefix + c.Name, + Ref: fieldmeta.DefinitionsPrefix + fieldmeta.SubstitutionDefinitionPrefix + c.Name, })}, Outputs: []kio.Writer{inout}, }.Execute()