Merge pull request #1111 from keleustes/master

Improvment var and varRefs handling

pkg/accumulator/resaccumulator.go

@@ -91,7 +91,7 @@ func (ra *ResAccumulator) MergeAccumulator(other *ResAccumulator) (err error) {
 	return ra.varSet.MergeSet(other.varSet)
 }
 
-func (ra *ResAccumulator) findValueFromResources(v types.Var) (string, error) {
+func (ra *ResAccumulator) findVarValueFromResources(v types.Var) (interface{}, error) {
 	for _, res := range ra.resMap.Resources() {
 		for _, varName := range res.GetRefVarNames() {
 			if varName == v.Name {
@@ -115,10 +115,10 @@ func (ra *ResAccumulator) findValueFromResources(v types.Var) (string, error) {
 // makeVarReplacementMap returns a map of Var names to
 // their final values. The values are strings intended
 // for substitution wherever the $(var.Name) occurs.
-func (ra *ResAccumulator) makeVarReplacementMap() (map[string]string, error) {
-	result := map[string]string{}
+func (ra *ResAccumulator) makeVarReplacementMap() (map[string]interface{}, error) {
+	result := map[string]interface{}{}
 	for _, v := range ra.Vars() {
-		s, err := ra.findValueFromResources(v)
+		s, err := ra.findVarValueFromResources(v)
 		if err != nil {
 			return nil, err
 		}

pkg/accumulator/resaccumulator_test.go

@@ -326,5 +326,11 @@ func getCommand(r *resource.Resource) string {
 	m, _ = m["spec"].(map[string]interface{})
 	c, _ = m["containers"].([]interface{})
 	m, _ = c[0].(map[string]interface{})
-	return strings.Join(m["command"].([]string), " ")
+
+	cmd, _ := m["command"].([]interface{})
+	n := make([]string, len(cmd))
+	for i, v := range cmd {
+		n[i] = v.(string)
+	}
+	return strings.Join(n, " ")
 }

pkg/expansion/expand.go

@@ -19,6 +19,7 @@ package expansion
 
 import (
 	"bytes"
+	"fmt"
 )
 
 const (
@@ -38,13 +39,18 @@ func syntaxWrap(input string) string {
 // for the input is found.
 func MappingFuncFor(
 	counts map[string]int,
-	context ...map[string]string) func(string) string {
-	return func(input string) string {
+	context ...map[string]interface{}) func(string) interface{} {
+	return func(input string) interface{} {
 		for _, vars := range context {
 			val, ok := vars[input]
 			if ok {
 				counts[input]++
-				return val
+				switch typedV := val.(type) {
+				case string, int64, float64, bool:
+					return typedV
+				default:
+					return syntaxWrap(input)
+				}
 			}
 		}
 		return syntaxWrap(input)
@@ -54,7 +60,7 @@ func MappingFuncFor(
 // Expand replaces variable references in the input string according to
 // the expansion spec using the given mapping function to resolve the
 // values of variables.
-func Expand(input string, mapping func(string) string) string {
+func Expand(input string, mapping func(string) interface{}) interface{} {
 	var buf bytes.Buffer
 	checkpoint := 0
 	for cursor := 0; cursor < len(input); cursor++ {
@@ -71,7 +77,14 @@ func Expand(input string, mapping func(string) string) string {
 				// We were able to read a variable name correctly;
 				// apply the mapping to the variable name and copy the
 				// bytes into the buffer
-				buf.WriteString(mapping(read))
+				mapped := mapping(read)
+				if input == syntaxWrap(read) {
+					// Preserve the type of variable
+					return mapped
+				}
+
+				// Variable is used in a middle of a string
+				buf.WriteString(fmt.Sprintf("%v", mapped))
 			} else {
 				// Not a variable name; copy the read bytes into the buffer
 				buf.WriteString(read)

pkg/expansion/expand_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package expansion_test
 
 import (
+	"fmt"
 	"testing"
 
 	. "sigs.k8s.io/kustomize/pkg/expansion"
@@ -30,7 +31,7 @@ type expected struct {
 func TestMapReference(t *testing.T) {
 	type env struct {
 		Name  string
-		Value string
+		Value interface{}
 	}
 	envs := []env{
 		{
@@ -45,25 +46,49 @@ func TestMapReference(t *testing.T) {
 			Name:  "BLU",
 			Value: "$(ZOO)-2",
 		},
+		{
+			Name:  "INT",
+			Value: 2,
+		},
+		{
+			Name:  "ZINT",
+			Value: "$(INT)",
+		},
+		{
+			Name:  "BOOL",
+			Value: true,
+		},
+		{
+			Name:  "ZBOOL",
+			Value: "$(BOOL)",
+		},
 	}
 
-	declaredEnv := map[string]string{
-		"FOO": "bar",
-		"ZOO": "$(FOO)-1",
-		"BLU": "$(ZOO)-2",
+	declaredEnv := map[string]interface{}{
+		"FOO":   "bar",
+		"ZOO":   "$(FOO)-1",
+		"BLU":   "$(ZOO)-2",
+		"INT":   "2",
+		"ZINT":  "$(INT)",
+		"BOOL":  "true",
+		"ZBOOL": "$(BOOL)",
 	}
 
 	counts := make(map[string]int)
 	mapping := MappingFuncFor(counts, declaredEnv)
 
 	for _, env := range envs {
-		declaredEnv[env.Name] = Expand(env.Value, mapping)
+		declaredEnv[env.Name] = Expand(fmt.Sprintf("%v", env.Value), mapping)
 	}
 
 	expectedEnv := map[string]expected{
-		"FOO": {count: 1, edited: "bar"},
-		"ZOO": {count: 1, edited: "bar-1"},
-		"BLU": {count: 0, edited: "bar-1-2"},
+		"FOO":   {count: 1, edited: "bar"},
+		"ZOO":   {count: 1, edited: "bar-1"},
+		"BLU":   {count: 0, edited: "bar-1-2"},
+		"INT":   {count: 1, edited: "2"},
+		"ZINT":  {count: 0, edited: "2"},
+		"BOOL":  {count: 1, edited: "true"},
+		"ZBOOL": {count: 0, edited: "true"},
 	}
 
 	for k, v := range expectedEnv {
@@ -81,7 +106,7 @@ func TestMapReference(t *testing.T) {
 }
 
 func TestMapping(t *testing.T) {
-	context := map[string]string{
+	context := map[string]interface{}{
 		"VAR_A":     "A",
 		"VAR_B":     "B",
 		"VAR_C":     "C",
@@ -92,11 +117,11 @@ func TestMapping(t *testing.T) {
 }
 
 func TestMappingDual(t *testing.T) {
-	context := map[string]string{
+	context := map[string]interface{}{
 		"VAR_A":     "A",
 		"VAR_EMPTY": "",
 	}
-	context2 := map[string]string{
+	context2 := map[string]interface{}{
 		"VAR_B":   "B",
 		"VAR_C":   "C",
 		"VAR_REF": "$(VAR_A)",
@@ -105,7 +130,7 @@ func TestMappingDual(t *testing.T) {
 	doExpansionTest(t, context, context2)
 }
 
-func doExpansionTest(t *testing.T, context ...map[string]string) {
+func doExpansionTest(t *testing.T, context ...map[string]interface{}) {
 	cases := []struct {
 		name     string
 		input    string
@@ -325,7 +350,7 @@ func doExpansionTest(t *testing.T, context ...map[string]string) {
 	for _, tc := range cases {
 		counts := make(map[string]int)
 		mapping := MappingFuncFor(counts, context...)
-		expanded := Expand(tc.input, mapping)
+		expanded := Expand(fmt.Sprintf("%v", tc.input), mapping)
 		if e, a := tc.expected, expanded; e != a {
 			t.Errorf("%v: expected %q, got %q", tc.name, e, a)
 		}

pkg/ifc/ifc.go

@@ -40,8 +40,15 @@ type Kunstructured interface {
 	Map() map[string]interface{}
 	SetMap(map[string]interface{})
 	Copy() Kunstructured
-	GetFieldValue(string) (string, error)
+	GetFieldValue(string) (interface{}, error)
+	GetString(string) (string, error)
 	GetStringSlice(string) ([]string, error)
+	GetBool(path string) (bool, error)
+	GetFloat64(path string) (float64, error)
+	GetInt64(path string) (int64, error)
+	GetSlice(path string) ([]interface{}, error)
+	GetStringMap(path string) (map[string]string, error)
+	GetMap(path string) (map[string]interface{}, error)
 	MarshalJSON() ([]byte, error)
 	UnmarshalJSON([]byte) error
 	GetGvk() gvk.Gvk

pkg/resource/resource.go

@@ -182,7 +182,7 @@ func (r *Resource) NeedHashSuffix() bool {
 
 // GetNamespace returns the namespace the resource thinks it's in.
 func (r *Resource) GetNamespace() string {
-	namespace, _ := r.GetFieldValue("metadata.namespace")
+	namespace, _ := r.GetString("metadata.namespace")
 	// if err, namespace is empty, so no need to check.
 	return namespace
 }

pkg/transformers/mutatefield_test.go

@@ -79,7 +79,7 @@ func makeTestDeployment() ifc.Kunstructured {
 }
 
 func getFieldValue(t *testing.T, obj ifc.Kunstructured, fieldName string) string {
-	v, err := obj.GetFieldValue(fieldName)
+	v, err := obj.GetString(fieldName)
 	if err != nil {
 		t.Fatalf("unexpected field error: %v", err)
 	}

pkg/transformers/refvars.go

@@ -24,17 +24,17 @@ import (
 )
 
 type RefVarTransformer struct {
-	varMap            map[string]string
+	varMap            map[string]interface{}
 	replacementCounts map[string]int
 	fieldSpecs        []config.FieldSpec
-	mappingFunc       func(string) string
+	mappingFunc       func(string) interface{}
 }
 
 // NewRefVarTransformer returns a new RefVarTransformer
 // that replaces $(VAR) style variables with values.
 // The fieldSpecs are the places to look for occurrences of $(VAR).
 func NewRefVarTransformer(
-	varMap map[string]string, fs []config.FieldSpec) *RefVarTransformer {
+	varMap map[string]interface{}, fs []config.FieldSpec) *RefVarTransformer {
 	return &RefVarTransformer{
 		varMap:     varMap,
 		fieldSpecs: fs,
@@ -48,7 +48,7 @@ func NewRefVarTransformer(
 func (rv *RefVarTransformer) replaceVars(in interface{}) (interface{}, error) {
 	switch vt := in.(type) {
 	case []interface{}:
-		var xs []string
+		var xs []interface{}
 		for _, a := range in.([]interface{}) {
 			xs = append(xs, expansion.Expand(a.(string), rv.mappingFunc))
 		}
@@ -59,16 +59,26 @@ func (rv *RefVarTransformer) replaceVars(in interface{}) (interface{}, error) {
 		for k, v := range inMap {
 			s, ok := v.(string)
 			if !ok {
-				return nil, fmt.Errorf("%#v is expected to be %T", v, s)
+				// This field not contain a $(VAR) since it is not
+				// of string type. For instance .spec.replicas: 3 in
+				// a Deployment object
+				xs[k] = v
+			} else {
+				// This field can potentially contains a $(VAR) since it is
+				// of string type. For instance .spec.replicas: $(REPLICAS)
+				// in a Deployment object
+				xs[k] = expansion.Expand(s, rv.mappingFunc)
 			}
-			xs[k] = expansion.Expand(s, rv.mappingFunc)
 		}
 		return xs, nil
 	case interface{}:
 		s, ok := in.(string)
 		if !ok {
-			return nil, fmt.Errorf("%#v is expected to be %T", in, s)
+			// This field not contain a $(VAR) since it is not of string type.
+			return in, nil
 		}
+		// This field can potentially contain a $(VAR) since it is
+		// of string type.
 		return expansion.Expand(s, rv.mappingFunc), nil
 	case nil:
 		return nil, nil

pkg/transformers/refvars_test.go

@@ -15,7 +15,7 @@ import (
 
 func TestVarRef(t *testing.T) {
 	type given struct {
-		varMap map[string]string
+		varMap map[string]interface{}
 		fs     []config.FieldSpec
 		res    resmap.ResMap
 	}
@@ -31,9 +31,11 @@ func TestVarRef(t *testing.T) {
 		{
 			description: "var replacement in map[string]",
 			given: given{
-				varMap: map[string]string{
+				varMap: map[string]interface{}{
 					"FOO": "replacementForFoo",
 					"BAR": "replacementForBar",
+					"BAZ": int64(5),
+					"BOO": true,
 				},
 				fs: []config.FieldSpec{
 					{Gvk: gvk.Gvk{Version: "v1", Kind: "ConfigMap"}, Path: "data"},
@@ -48,6 +50,10 @@ func TestVarRef(t *testing.T) {
 						"data": map[string]interface{}{
 							"item1": "$(FOO)",
 							"item2": "bla",
+							"item3": "$(BAZ)",
+							"item4": "$(BAZ)+$(BAZ)",
+							"item5": "$(BOO)",
+							"item6": "if $(BOO)",
 						},
 					}).ResMap(),
 			},
@@ -62,6 +68,10 @@ func TestVarRef(t *testing.T) {
 						"data": map[string]interface{}{
 							"item1": "replacementForFoo",
 							"item2": "bla",
+							"item3": int64(5),
+							"item4": "5+5",
+							"item5": true,
+							"item6": "if true",
 						}}).ResMap(),
 				unused: []string{"BAR"},
 			},

pkg/types/kustomization.go

@@ -84,10 +84,10 @@ type Kustomization struct {
 	Replicas []Replica `json:"replicas,omitempty" yaml:"replicas,omitempty"`
 
 	// Vars allow things modified by kustomize to be injected into a
-	// container specification. A var is a name (e.g. FOO) associated
+	// kubernetes object specification. A var is a name (e.g. FOO) associated
 	// with a field in a specific resource instance.  The field must
-	// contain a value of type string, and defaults to the name field
-	// of the instance.  Any appearance of "$(FOO)" in the container
+	// contain a value of type string/bool/int/float, and defaults to the name field
+	// of the instance.  Any appearance of "$(FOO)" in the object
 	// spec will be replaced at kustomize build time, after the final
 	// value of the specified field has been determined.
 	Vars []Var `json:"vars,omitempty" yaml:"vars,omitempty"`