Merge pull request #2509 from phanimarupaka/RefChange

Short hand notation for setters/subst

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
 		}

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"}
  `,
 		},
 	}

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,

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
  `,

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, ","))

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
 `,

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
+}

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 {

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"}
  `,
 		},
 		{

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"}
 }

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
 		}

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

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()

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()