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/cmdcreatesetter_test.go b/cmd/config/internal/commands/cmdcreatesetter_test.go index ff6da4f31..001de307e 100644 --- a/cmd/config/internal/commands/cmdcreatesetter_test.go +++ b/cmd/config/internal/commands/cmdcreatesetter_test.go @@ -62,7 +62,7 @@ kind: Deployment metadata: name: nginx-deployment spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, { @@ -124,7 +124,7 @@ kind: Deployment metadata: name: nginx-deployment spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, @@ -165,7 +165,7 @@ apiVersion: example.com/v1beta1 kind: Example spec: list: - - "a" # {"$ref":"#/definitions/io.k8s.cli.setters.list"} + - "a" # {"$openapi":"list"} `, }, } 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/cmdcreatesubstitution_test.go b/cmd/config/internal/commands/cmdcreatesubstitution_test.go index 90ffc854b..505afbf39 100644 --- a/cmd/config/internal/commands/cmdcreatesubstitution_test.go +++ b/cmd/config/internal/commands/cmdcreatesubstitution_test.go @@ -99,7 +99,7 @@ spec: spec: containers: - name: nginx - image: nginx:1.7.9 # {"$ref":"#/definitions/io.k8s.cli.substitutions.my-image-subst"} + image: nginx:1.7.9 # {"$openapi":"my-image-subst"} - name: sidecar image: sidecar:1.7.9 `, @@ -181,7 +181,7 @@ spec: spec: containers: - name: nginx - image: something/nginx::1.7.9/nginxotherthing # {"$ref":"#/definitions/io.k8s.cli.substitutions.my-image-subst"} + image: something/nginx::1.7.9/nginxotherthing # {"$openapi":"my-image-subst"} - name: sidecar image: sidecar:1.7.9 `, 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..5e253573c 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..b24a792a9 100644 --- a/kyaml/fieldmeta/fieldmeta.go +++ b/kyaml/fieldmeta/fieldmeta.go @@ -5,12 +5,14 @@ package fieldmeta import ( "encoding/json" + "fmt" "reflect" "strconv" "strings" "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 +56,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 +81,55 @@ func (fm *FieldMeta) Read(n *yaml.RNode) error { return nil } +// processShortHand parses the comment for short hand ref, loads schema to fm +// and returns true if successful, returns false for any other cases and not throw +// error, as the comment might not be a setter ref +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 @@ -96,11 +150,11 @@ func (fm *FieldMeta) Write(n *yaml.RNode) error { } else { delete(fm.Schema.VendorExtensible.Extensions, "x-kustomize") } - b, err := json.Marshal(fm.Schema) - if err != nil { - return errors.Wrap(err) - } - n.YNode().LineComment = string(b) + + // Ex: {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} should be converted to + // {"openAPI":"replicas"} and added to the line comment + arr := strings.Split(fm.Schema.Ref.String(), ".") + n.YNode().LineComment = fmt.Sprintf(`{"%s":"%s"}`, shortHandRef, arr[len(arr)-1]) return nil } @@ -166,3 +220,28 @@ 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 +var shortHandRef = "$openapi" + +func SetShortHandRef(ref string) { + shortHandRef = ref +} + +func ShortHandRef() string { + return shortHandRef +} 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/add_test.go b/kyaml/setters2/add_test.go index ab5524287..9516612c3 100644 --- a/kyaml/setters2/add_test.go +++ b/kyaml/setters2/add_test.go @@ -42,7 +42,7 @@ kind: Deployment metadata: name: nginx-deployment spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, { @@ -67,9 +67,9 @@ kind: Deployment metadata: name: nginx-deployment annotations: - something: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + something: 3 # {"$openapi":"replicas"} spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, { @@ -97,7 +97,7 @@ metadata: annotations: something: 3 spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, { @@ -123,9 +123,9 @@ kind: Deployment metadata: name: nginx-deployment annotations: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, { @@ -153,7 +153,7 @@ metadata: annotations: replicas: 3 spec: - replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + replicas: 3 # {"$openapi":"replicas"} `, }, { diff --git a/kyaml/setters2/example_test.go b/kyaml/setters2/example_test.go index 10e3c918a..8bb36c485 100644 --- a/kyaml/setters2/example_test.go +++ b/kyaml/setters2/example_test.go @@ -162,7 +162,7 @@ spec: // annotations: // something: 3 // spec: - // replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + // replicas: 3 # {"$openapi":"replicas"} } // ExampleAdd demonstrates adding a setter reference to fields. @@ -196,7 +196,7 @@ spec: // metadata: // name: nginx-deployment // annotations: - // something: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + // something: 3 # {"$openapi":"replicas"} // spec: - // replicas: 3 # {"$ref":"#/definitions/io.k8s.cli.setters.replicas"} + // replicas: 3 # {"$openapi":"replicas"} } 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()