diff --git a/k8sdeps/kunstruct/helper.go b/k8sdeps/kunstruct/helper.go index 0675b961d..79c03ae0c 100644 --- a/k8sdeps/kunstruct/helper.go +++ b/k8sdeps/kunstruct/helper.go @@ -19,39 +19,66 @@ package kunstruct import ( "fmt" + "strconv" "strings" ) -func parseFields(path string) ([]string, error) { +// A PathSection contains a list of nested fields, which may end with an +// indexable value. For instance, foo.bar resolves to a PathSection with 2 +// fields and no index, while foo[0].bar resolves to two path sections, the +// first containing the field foo and the index 0, and the second containing +// the field bar, with no index. The latter PathSection references the bar +// field of the first item in the foo list +type PathSection struct { + fields []string + idx *int +} + +func appendNonEmpty(section *PathSection, field string) { + if len(field) != 0 { + section.fields = append(section.fields, field) + } +} + +func parseFields(path string) ([]PathSection, error) { + section := PathSection{} + sectionset := []PathSection{} if !strings.Contains(path, "[") { - return strings.Split(path, "."), nil + section.fields = strings.Split(path, ".") + sectionset = append(sectionset, section) + return sectionset, nil } - var fields []string start := 0 insideParentheses := false - for i := range path { - switch path[i] { + for i, c := range path { + switch c { case '.': if !insideParentheses { - fields = append(fields, path[start:i]) + appendNonEmpty(§ion, path[start:i]) start = i + 1 } case '[': if !insideParentheses { - if i == start { - start = i + 1 - } else { - fields = append(fields, path[start:i]) - start = i + 1 - } + appendNonEmpty(§ion, path[start:i]) + start = i + 1 insideParentheses = true } else { return nil, fmt.Errorf("nested parentheses are not allowed: %s", path) } case ']': if insideParentheses { - fields = append(fields, path[start:i]) + // Assign this index to the current + // PathSection, save it to the set, then begin + // a new PathSection + tmpIdx, err := strconv.Atoi(path[start:i]) + if err != nil { + return nil, fmt.Errorf("invalid index %s", path) + } + section.idx = &tmpIdx + sectionset = append(sectionset, section) + section = PathSection{} + start = i + 1 insideParentheses = false } else { @@ -60,12 +87,16 @@ func parseFields(path string) ([]string, error) { } } if start < len(path)-1 { - fields = append(fields, path[start:]) + appendNonEmpty(§ion, path[start:]) + sectionset = append(sectionset, section) } - for i, f := range fields { - if strings.HasPrefix(f, "\"") || strings.HasPrefix(f, "'") { - fields[i] = strings.Trim(f, "\"'") + + for _, section := range sectionset { + for i, f := range section.fields { + if strings.HasPrefix(f, "\"") || strings.HasPrefix(f, "'") { + section.fields[i] = strings.Trim(f, "\"'") + } } } - return fields, nil + return sectionset, nil } diff --git a/k8sdeps/kunstruct/helper_test.go b/k8sdeps/kunstruct/helper_test.go new file mode 100644 index 000000000..4d5b3c5a3 --- /dev/null +++ b/k8sdeps/kunstruct/helper_test.go @@ -0,0 +1,170 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kunstruct + +import ( + "reflect" + "testing" +) + +type PathSectionSlice []PathSection + +func buildPath(idx *int, fields ...string) PathSectionSlice { + return PathSectionSlice{PathSection{fields: fields, idx: idx}} +} + +func (a PathSectionSlice) addSection(idx *int, fields ...string) PathSectionSlice { + return append(a, PathSection{fields: fields, idx: idx}) +} + +func TestParseField(t *testing.T) { + i := 1 + var one *int = &i + j := 0 + var zero *int = &j + + tests := []struct { + name string + pathToField string + expectedValue []PathSection + errorExpected bool + errorMsg string + }{ + { + name: "oneField", + pathToField: "Kind", + expectedValue: buildPath(nil, "Kind"), + errorExpected: false, + }, + { + name: "twoFields", + pathToField: "metadata.name", + expectedValue: buildPath(nil, "metadata", "name"), + errorExpected: false, + }, + { + name: "threeFields", + pathToField: "spec.ports.port", + expectedValue: buildPath(nil, "spec", "ports", "port"), + errorExpected: false, + }, + { + name: "empty", + pathToField: "", + expectedValue: buildPath(nil, ""), + errorExpected: false, + }, + { + name: "validStringIndex", + pathToField: "that[1]", + expectedValue: buildPath(one, "that"), + errorExpected: false, + }, + { + name: "sliceInSlice", + pathToField: "that[1][0]", + expectedValue: buildPath(one, "that").addSection(zero), + errorExpected: false, + }, + { + name: "validStructSubField", + pathToField: "those[1].field2", + expectedValue: buildPath(one, "those").addSection(nil, "field2"), + errorExpected: false, + }, + { + name: "validStructSubFieldIndex", + pathToField: "these[1].field2[0]", + expectedValue: buildPath(one, "these").addSection(zero, "field2"), + errorExpected: false, + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1].stringsubfield", + expectedValue: buildPath(one, "complextree").addSection(one, "field2").addSection(nil, "stringsubfield"), + errorExpected: false, + }, + { + name: "validStructSubFieldNoneIntIndex", + pathToField: "complextree[thisisnotanint]", + errorExpected: true, + errorMsg: "invalid index complextree[thisisnotanint]", + }, + { + name: "invalidIndexInIndex", + pathToField: "complextree[1[0]]", + errorExpected: true, + errorMsg: "nested parentheses are not allowed: complextree[1[0]]", + }, + { + name: "invalidClosingBrackets", + pathToField: "complextree[1]]", + errorExpected: true, + errorMsg: "invalid field path complextree[1]]", + }, + { + name: "validFieldsWithQuotes", + pathToField: "'complextree'[1].field2[1].'stringsubfield'", + expectedValue: buildPath(one, "complextree").addSection(one, "field2").addSection(nil, "stringsubfield"), + errorExpected: false, + }, + } + + for _, test := range tests { + s, err := parseFields(test.pathToField) + if test.errorExpected { + compareExpectedParserError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedParserError(t, test.name, test.pathToField, err) + } + compareParserValues(t, test.name, test.pathToField, test.expectedValue, s) + } +} + +// unExpectedError function handles unexpected error +func unExpectedParserError(t *testing.T, name string, pathToField string, err error) { + t.Fatalf("%q; path %q - unexpected error %v", name, pathToField, err) +} + +// compareExpectedError compares the expectedError and the actualError return by parseFields +func compareExpectedParserError(t *testing.T, name string, pathToField string, err error, errorMsg string) { + if err == nil { + t.Fatalf("%q; path %q - should return error, but no error returned", + name, pathToField) + } + + if errorMsg != err.Error() { + t.Fatalf("%q; path %q - expected error: \"%s\", got error: \"%v\"", + name, pathToField, errorMsg, err.Error()) + } +} + +// compareValues compares the expectedValue and actualValue returned by parseFields +func compareParserValues(t *testing.T, name string, pathToField string, expectedValue PathSectionSlice, actualValue []PathSection) { + t.Helper() + if len(expectedValue) != len(actualValue) { + t.Fatalf("%q; Path: %s Got: %v Expected: %v", name, pathToField, actualValue, expectedValue) + } + + for idx, expected := range expectedValue { + if !reflect.DeepEqual(expected, actualValue[idx]) { + t.Fatalf("%q; Path: %s idx: %v Fields Got: %v Expected: %v", name, pathToField, idx, actualValue[idx], expected) + } + } +} diff --git a/k8sdeps/kunstruct/kunstruct.go b/k8sdeps/kunstruct/kunstruct.go index c074311b3..163b3b892 100644 --- a/k8sdeps/kunstruct/kunstruct.go +++ b/k8sdeps/kunstruct/kunstruct.go @@ -19,6 +19,7 @@ package kunstruct import ( "encoding/json" + "fmt" "sigs.k8s.io/kustomize/pkg/types" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -77,14 +78,85 @@ func (fs *UnstructAdapter) SetMap(m map[string]interface{}) { fs.Object = m } -// GetFieldValue returns value at the given fieldpath. -func (fs *UnstructAdapter) GetFieldValue(path string) (string, error) { - fields, err := parseFields(path) - if err != nil { - return "", err +func (fs *UnstructAdapter) selectSubtree(path string) (map[string]interface{}, []string, bool, error) { + sections, err := parseFields(path) + if len(sections) == 0 || (err != nil) { + return nil, nil, false, err } + + content := fs.UnstructuredContent() + lastSectionIdx := len(sections) + + // There are multiple sections to walk + for sectionIdx := 0; sectionIdx < lastSectionIdx; sectionIdx++ { + idx := sections[sectionIdx].idx + fields := sections[sectionIdx].fields + + if idx == nil { + // This section has no index + return content, fields, true, nil + } + + // This section is terminated by an indexed field. + // Let's extract the slice first + indexedField, found, err := unstructured.NestedFieldNoCopy(content, fields...) + if !found || err != nil { + return content, fields, found, err + } + s, ok := indexedField.([]interface{}) + if !ok { + return content, fields, false, fmt.Errorf("%v is of the type %T, expected []interface{}", indexedField, indexedField) + } + if *idx >= len(s) { + return content, fields, false, fmt.Errorf("index %d is out of bounds", *idx) + } + + if sectionIdx == lastSectionIdx-1 { + // This is the last section. Let's build a fake map + // to let the rest of the field extraction to work. + idxstring := fmt.Sprintf("[%v]", *idx) + newContent := map[string]interface{}{idxstring: s[*idx]} + newFields := []string{idxstring} + return newContent, newFields, true, nil + } + + newContent, ok := s[*idx].(map[string]interface{}) + if !ok { + // Only map are supported here + return content, fields, false, + fmt.Errorf("%#v is expected to be of type map[string]interface{}", s[*idx]) + } + content = newContent + } + + // It seems to be an invalid path + return nil, []string{}, false, nil +} + +// GetFieldValue returns the value at the given fieldpath. +func (fs *UnstructAdapter) GetFieldValue(path string) (interface{}, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return nil, types.NoFieldError{Field: path} + } + + s, found, err := unstructured.NestedFieldNoCopy( + content, fields...) + if found || err != nil { + return s, err + } + return nil, types.NoFieldError{Field: path} +} + +// GetString returns value at the given fieldpath. +func (fs *UnstructAdapter) GetString(path string) (string, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return "", types.NoFieldError{Field: path} + } + s, found, err := unstructured.NestedString( - fs.UnstructuredContent(), fields...) + content, fields...) if found || err != nil { return s, err } @@ -93,14 +165,105 @@ func (fs *UnstructAdapter) GetFieldValue(path string) (string, error) { // GetStringSlice returns value at the given fieldpath. func (fs *UnstructAdapter) GetStringSlice(path string) ([]string, error) { - fields, err := parseFields(path) - if err != nil { - return []string{}, err + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return []string{}, types.NoFieldError{Field: path} } + s, found, err := unstructured.NestedStringSlice( - fs.UnstructuredContent(), fields...) + content, fields...) if found || err != nil { return s, err } return []string{}, types.NoFieldError{Field: path} } + +// GetBool returns value at the given fieldpath. +func (fs *UnstructAdapter) GetBool(path string) (bool, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return false, types.NoFieldError{Field: path} + } + + s, found, err := unstructured.NestedBool( + content, fields...) + if found || err != nil { + return s, err + } + return false, types.NoFieldError{Field: path} +} + +// GetFloat64 returns value at the given fieldpath. +func (fs *UnstructAdapter) GetFloat64(path string) (float64, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return 0, err + } + + s, found, err := unstructured.NestedFloat64( + content, fields...) + if found || err != nil { + return s, err + } + return 0, types.NoFieldError{Field: path} +} + +// GetInt64 returns value at the given fieldpath. +func (fs *UnstructAdapter) GetInt64(path string) (int64, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return 0, types.NoFieldError{Field: path} + } + + s, found, err := unstructured.NestedInt64( + content, fields...) + if found || err != nil { + return s, err + } + return 0, types.NoFieldError{Field: path} +} + +// GetSlice returns value at the given fieldpath. +func (fs *UnstructAdapter) GetSlice(path string) ([]interface{}, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return nil, types.NoFieldError{Field: path} + } + + s, found, err := unstructured.NestedSlice( + content, fields...) + if found || err != nil { + return s, err + } + return nil, types.NoFieldError{Field: path} +} + +// GetStringMap returns value at the given fieldpath. +func (fs *UnstructAdapter) GetStringMap(path string) (map[string]string, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return nil, types.NoFieldError{Field: path} + } + + s, found, err := unstructured.NestedStringMap( + content, fields...) + if found || err != nil { + return s, err + } + return nil, types.NoFieldError{Field: path} +} + +// GetMap returns value at the given fieldpath. +func (fs *UnstructAdapter) GetMap(path string) (map[string]interface{}, error) { + content, fields, found, err := fs.selectSubtree(path) + if !found || err != nil { + return nil, types.NoFieldError{Field: path} + } + + s, found, err := unstructured.NestedMap( + content, fields...) + if found || err != nil { + return s, err + } + return nil, types.NoFieldError{Field: path} +} diff --git a/k8sdeps/kunstruct/kunstruct_test.go b/k8sdeps/kunstruct/kunstruct_test.go index dc7eb8bb2..c0a675705 100644 --- a/k8sdeps/kunstruct/kunstruct_test.go +++ b/k8sdeps/kunstruct/kunstruct_test.go @@ -17,40 +17,139 @@ limitations under the License. package kunstruct import ( + "reflect" "testing" ) -func TestGetFieldValue(t *testing.T) { - factory := NewKunstructuredFactoryImpl() - kunstructured := factory.FromMap(map[string]interface{}{ - "Kind": "Service", - "metadata": map[string]interface{}{ - "labels": map[string]string{ - "app": "application-name", - }, - "name": "service-name", +var kunstructured = NewKunstructuredFactoryImpl().FromMap(map[string]interface{}{ + "Kind": "Service", + "metadata": map[string]interface{}{ + "labels": map[string]string{ + "app": "application-name", }, - "spec": map[string]interface{}{ - "ports": map[string]interface{}{ - "port": "80", + "name": "service-name", + }, + "spec": map[string]interface{}{ + "ports": map[string]interface{}{ + "port": "80", + }, + }, + "this": map[string]interface{}{ + "is": map[string]interface{}{ + "aNumber": int64(1000), + "aFloat": float64(1.001), + "aNilValue": nil, + "aBool": true, + "anEmptyMap": map[string]interface{}{}, + "anEmptySlice": []interface{}{}, + "unrecognizable": testing.InternalExample{ + Name: "fooBar", }, }, - "this": map[string]interface{}{ - "is": map[string]interface{}{ - "aNumber": 1000, - "aNilValue": nil, - "anEmptyMap": map[string]interface{}{}, - "unrecognizable": testing.InternalExample{ - Name: "fooBar", + }, + "that": []interface{}{ + "idx0", + "idx1", + "idx2", + "idx3", + }, + "those": []interface{}{ + map[string]interface{}{ + "field1": "idx0foo", + "field2": "idx0bar", + }, + map[string]interface{}{ + "field1": "idx1foo", + "field2": "idx1bar", + }, + map[string]interface{}{ + "field1": "idx2foo", + "field2": "idx2bar", + }, + }, + "these": []interface{}{ + map[string]interface{}{ + "field1": []interface{}{"idx010", "idx011"}, + "field2": []interface{}{"idx020", "idx021"}, + }, + map[string]interface{}{ + "field1": []interface{}{"idx110", "idx111"}, + "field2": []interface{}{"idx120", "idx121"}, + }, + map[string]interface{}{ + "field1": []interface{}{"idx210", "idx211"}, + "field2": []interface{}{"idx220", "idx221"}, + }, + }, + "complextree": []interface{}{ + map[string]interface{}{ + "field1": []interface{}{ + map[string]interface{}{ + "stringsubfield": "idx1010", + "intsubfield": int64(1010), + "floatsubfield": float64(1.010), + "boolfield": true, + }, + map[string]interface{}{ + "stringsubfield": "idx1011", + "intsubfield": int64(1011), + "floatsubfield": float64(1.011), + "boolfield": false, + }, + }, + "field2": []interface{}{ + map[string]interface{}{ + "stringsubfield": "idx1020", + "intsubfield": int64(1020), + "floatsubfield": float64(1.020), + "boolfield": true, + }, + map[string]interface{}{ + "stringsubfield": "idx1021", + "intsubfield": int64(1021), + "floatsubfield": float64(1.021), + "boolfield": false, }, }, }, - }) + map[string]interface{}{ + "field1": []interface{}{ + map[string]interface{}{ + "stringsubfield": "idx1110", + "intsubfield": int64(1110), + "floatsubfield": float64(1.110), + "boolfield": true, + }, + map[string]interface{}{ + "stringsubfield": "idx1111", + "intsubfield": int64(1111), + "floatsubfield": float64(1.111), + "boolfield": false, + }, + }, + "field2": []interface{}{ + map[string]interface{}{ + "stringsubfield": "idx1120", + "intsubfield": int64(1120), + "floatsubfield": float64(1.1120), + "boolfield": true, + }, + map[string]interface{}{ + "stringsubfield": "idx1121", + "intsubfield": int64(1121), + "floatsubfield": float64(1.1121), + "boolfield": false, + }, + }, + }, + }, +}) +func TestGetFieldValue(t *testing.T) { tests := []struct { name string pathToField string - expectedValue string + expectedValue interface{} errorExpected bool errorMsg string }{ @@ -96,17 +195,202 @@ func TestGetFieldValue(t *testing.T) { errorExpected: true, errorMsg: "no field named 'this.is.aDeep.field.that.does.not.exist'", }, + { + name: "emptyMap", + pathToField: "this.is.anEmptyMap", + errorExpected: false, + expectedValue: map[string]interface{}{}, + }, + { + name: "emptySlice", + pathToField: "this.is.anEmptySlice", + errorExpected: false, + expectedValue: []interface{}{}, + }, + { + name: "numberAsValue", + pathToField: "this.is.aNumber", + errorExpected: false, + expectedValue: int64(1000), + }, + { + name: "floatAsValue", + pathToField: "this.is.aFloat", + errorExpected: false, + expectedValue: float64(1.001), + }, + { + name: "boolAsValue", + pathToField: "this.is.aBool", + errorExpected: false, + expectedValue: true, + }, + { + name: "nilAsValue", + pathToField: "this.is.aNilValue", + errorExpected: false, + expectedValue: nil, + }, + { + name: "unrecognizable", + pathToField: "this.is.unrecognizable.Name", + errorExpected: true, + errorMsg: ".this.is.unrecognizable.Name accessor error: {fooBar false} is of the type testing.InternalExample, expected map[string]interface{}", + }, + { + name: "validStringIndex", + pathToField: "that[2]", + expectedValue: "idx2", + errorExpected: false, + }, + { + name: "outOfBoundIndex", + pathToField: "that[99]", + errorMsg: "no field named 'that[99]'", + errorExpected: true, + }, + { + name: "unknownSlice", + pathToField: "unknown[0]", + errorMsg: "no field named 'unknown[0]'", + errorExpected: true, + }, + { + name: "sliceInSlice", + pathToField: "that[2][0]", + errorExpected: true, + errorMsg: "no field named 'that[2][0]'", + }, + { + name: "validStructIndex", + pathToField: "those[1]", + errorExpected: false, + expectedValue: map[string]interface{}{"field1": "idx1foo", "field2": "idx1bar"}, + }, + { + name: "validStructSubField", + pathToField: "those[1].field2", + errorExpected: false, + expectedValue: "idx1bar", + }, + { + name: "validStructSubFieldIndex", + pathToField: "these[1].field2[1]", + errorExpected: false, + expectedValue: "idx121", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1].stringsubfield", + errorExpected: false, + expectedValue: "idx1121", + }, + { + name: "validStructSubFieldIndexInvalidName", + pathToField: "complextree[1].field2[1].invalidsubfield", + errorExpected: true, + errorMsg: "no field named 'complextree[1].field2[1].invalidsubfield'", + }, + { + name: "validStructSubFieldNoneIntIndex", + pathToField: "complextree[thisisnotanint]", + errorExpected: true, + errorMsg: "no field named 'complextree[thisisnotanint]'", + }, + { + name: "invalidNoneIntIndex", + pathToField: "complextree[thisisnotanint]", + errorExpected: true, + errorMsg: "no field named 'complextree[thisisnotanint]'", + }, + { + name: "invalidIndexInIndex", + pathToField: "complextree[1[0]]", + errorExpected: true, + errorMsg: "no field named 'complextree[1[0]]'", + }, + { + name: "invalidClosingBrackets", + pathToField: "complextree[1]]", + errorExpected: true, + errorMsg: "no field named 'complextree[1]]'", + }, + { + name: "validFieldsWithQuotes", + pathToField: "'complextree'[1].field2[1].'stringsubfield'", + errorExpected: false, + expectedValue: "idx1121", + }, + } + + for _, test := range tests { + s, err := kunstructured.GetFieldValue(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + compareValues(t, test.name, test.pathToField, test.expectedValue, s) + } +} + +func TestGetString(t *testing.T) { + tests := []struct { + name string + pathToField string + expectedValue string + errorExpected bool + errorMsg string + }{ + { + name: "oneField", + pathToField: "Kind", + expectedValue: "Service", + errorExpected: false, + }, + { + name: "twoFields", + pathToField: "metadata.name", + expectedValue: "service-name", + errorExpected: false, + }, + { + name: "threeFields", + pathToField: "spec.ports.port", + expectedValue: "80", + errorExpected: false, + }, { name: "emptyMap", pathToField: "this.is.anEmptyMap", errorExpected: true, errorMsg: ".this.is.anEmptyMap accessor error: map[] is of the type map[string]interface {}, expected string", }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "emptySlice", + pathToField: "this.is.anEmptySlice", + errorExpected: true, + errorMsg: ".this.is.anEmptySlice accessor error: [] is of the type []interface {}, expected string", + }, { name: "numberAsValue", pathToField: "this.is.aNumber", errorExpected: true, - errorMsg: ".this.is.aNumber accessor error: 1000 is of the type int, expected string", + errorMsg: ".this.is.aNumber accessor error: 1000 is of the type int64, expected string", }, { name: "nilAsValue", @@ -115,34 +399,410 @@ func TestGetFieldValue(t *testing.T) { errorMsg: ".this.is.aNilValue accessor error: is of the type , expected string", }, { - name: "unrecognizable", - pathToField: "this.is.unrecognizable.Name", + name: "validStringIndex", + pathToField: "that[2]", + expectedValue: "idx2", + errorExpected: false, + }, + { + name: "validStructIndex", + pathToField: "those[1]", errorExpected: true, - errorMsg: ".this.is.unrecognizable.Name accessor error: {fooBar false} is of the type testing.InternalExample, expected map[string]interface{}", + errorMsg: ".[1] accessor error: map[field1:idx1foo field2:idx1bar] is of the type map[string]interface {}, expected string", + }, + { + name: "validStructSubField", + pathToField: "those[1].field2", + errorExpected: false, + expectedValue: "idx1bar", + }, + { + name: "validStructSubFieldIndex", + pathToField: "these[1].field2[1]", + errorExpected: false, + expectedValue: "idx121", + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1].stringsubfield", + errorExpected: false, + expectedValue: "idx1121", + }, + { + name: "invalidIndexInMap", + pathToField: "this.is[1]", + errorExpected: true, + errorMsg: "no field named 'this.is[1]'", + }, + { + name: "anotherInvalidIndexInMap", + pathToField: "this.is[1].aString", + errorExpected: true, + errorMsg: "no field named 'this.is[1].aString'", }, } for _, test := range tests { - s, err := kunstructured.GetFieldValue(test.pathToField) + s, err := kunstructured.GetString(test.pathToField) if test.errorExpected { - if err == nil { - t.Fatalf("%q; path %q - should return error, but no error returned", - test.name, test.pathToField) - } - if test.errorMsg != err.Error() { - t.Fatalf("%q; path %q - expected error: \"%s\", got error: \"%v\"", - test.name, test.pathToField, test.errorMsg, err.Error()) - } + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) continue } if err != nil { - t.Fatalf("%q; path %q - unexpected error %v", - test.name, test.pathToField, err) + unExpectedError(t, test.name, test.pathToField, err) } - if test.expectedValue != s { - t.Fatalf("%q; Got: %s expected: %s", - test.name, s, test.expectedValue) - } - + compareValues(t, test.name, test.pathToField, test.expectedValue, s) + } +} + +func TestGetInt64(t *testing.T) { + tests := []struct { + name string + pathToField string + expectedValue int64 + errorExpected bool + errorMsg string + }{ + { + name: "numberAsValue", + pathToField: "this.is.aNumber", + errorExpected: false, + expectedValue: int64(1000), + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1].intsubfield", + errorExpected: false, + expectedValue: int64(1121), + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + } + + for _, test := range tests { + s, err := kunstructured.GetInt64(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + compareValues(t, test.name, test.pathToField, test.expectedValue, s) + } +} + +func TestGetFloat64(t *testing.T) { + tests := []struct { + name string + pathToField string + expectedValue float64 + errorExpected bool + errorMsg string + }{ + { + name: "floatAsValue", + pathToField: "this.is.aFloat", + errorExpected: false, + expectedValue: float64(1.001), + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1].floatsubfield", + errorExpected: false, + expectedValue: float64(1.1121), + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "index 99 is out of bounds", + }, + } + + for _, test := range tests { + s, err := kunstructured.GetFloat64(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + compareValues(t, test.name, test.pathToField, test.expectedValue, s) + } +} + +func TestGetBool(t *testing.T) { + tests := []struct { + name string + pathToField string + expectedValue bool + errorExpected bool + errorMsg string + }{ + { + name: "boolAsValue", + pathToField: "this.is.aBool", + errorExpected: false, + expectedValue: true, + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1].boolfield", + errorExpected: false, + expectedValue: false, + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + } + + for _, test := range tests { + s, err := kunstructured.GetBool(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + compareValues(t, test.name, test.pathToField, test.expectedValue, s) + } +} + +func TestGetStringMap(t *testing.T) { + tests := []struct { + name string + pathToField string + errorExpected bool + errorMsg string + }{ + { + name: "validStringMap", + pathToField: "those[2]", + errorExpected: false, + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + } + + for _, test := range tests { + _, err := kunstructured.GetStringMap(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + } +} + +func TestGetMap(t *testing.T) { + tests := []struct { + name string + pathToField string + errorExpected bool + errorMsg string + }{ + { + name: "validMap", + pathToField: "those[2]", + errorExpected: false, + }, + { + name: "validStructSubFieldIndexSubfield", + pathToField: "complextree[1].field2[1]", + errorExpected: false, + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + } + + for _, test := range tests { + _, err := kunstructured.GetMap(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + } +} + +func TestGetStringSlice(t *testing.T) { + tests := []struct { + name string + pathToField string + errorExpected bool + errorMsg string + }{ + { + name: "validStringSlice", + pathToField: "that", + errorExpected: false, + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + } + + for _, test := range tests { + _, err := kunstructured.GetStringSlice(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + } +} + +func TestGetSlice(t *testing.T) { + tests := []struct { + name string + pathToField string + errorExpected bool + errorMsg string + }{ + { + name: "validSlice1", + pathToField: "that", + errorExpected: false, + }, + { + name: "validSlice2", + pathToField: "those", + errorExpected: false, + }, + { + name: "validSlice3", + pathToField: "these", + errorExpected: false, + }, + { + name: "validSlice4", + pathToField: "complextree", + errorExpected: false, + }, + { + name: "twoFieldsOneMissing", + pathToField: "metadata.banana", + errorExpected: true, + errorMsg: "no field named 'metadata.banana'", + }, + { + name: "validStructSubFieldOutOfBoundIndex", + pathToField: "these[1].field2[99]", + errorExpected: true, + errorMsg: "no field named 'these[1].field2[99]'", + }, + } + + for _, test := range tests { + _, err := kunstructured.GetSlice(test.pathToField) + if test.errorExpected { + compareExpectedError(t, test.name, test.pathToField, err, test.errorMsg) + continue + } + if err != nil { + unExpectedError(t, test.name, test.pathToField, err) + } + } +} + +// unExpectedError function handles unexpected error +func unExpectedError(t *testing.T, name string, pathToField string, err error) { + t.Fatalf("%q; path %q - unexpected error %v", name, pathToField, err) +} + +// compareExpectedError compares the expectedError and the actualError return by GetFieldValue +func compareExpectedError(t *testing.T, name string, pathToField string, err error, errorMsg string) { + if err == nil { + t.Fatalf("%q; path %q - should return error, but no error returned", + name, pathToField) + } + + if errorMsg != err.Error() { + t.Fatalf("%q; path %q - expected error: \"%s\", got error: \"%v\"", + name, pathToField, errorMsg, err.Error()) + } +} + +// compareValues compares the expectedValue and actualValue returned by GetFieldValue +func compareValues(t *testing.T, name string, pathToField string, expectedValue interface{}, actualValue interface{}) { + t.Helper() + switch typedV := expectedValue.(type) { + case nil, string, bool, float64, int, int64: + if expectedValue != actualValue { + t.Fatalf("%q; Got: %v Expected: %v", name, actualValue, expectedValue) + } + case map[string]interface{}: + if !reflect.DeepEqual(expectedValue, actualValue) { + t.Fatalf("%q; Got: %v Expected: %v", name, actualValue, expectedValue) + } + case []interface{}: + if !reflect.DeepEqual(expectedValue, actualValue) { + t.Fatalf("%q; Got: %v Expected: %v", name, actualValue, expectedValue) + } + default: + t.Logf("%T value at `%s`", typedV, pathToField) } } diff --git a/pkg/accumulator/resaccumulator.go b/pkg/accumulator/resaccumulator.go index 66e4f0874..8fffe43e2 100644 --- a/pkg/accumulator/resaccumulator.go +++ b/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 } diff --git a/pkg/accumulator/resaccumulator_test.go b/pkg/accumulator/resaccumulator_test.go index 4cf5cd3a6..1e5915143 100644 --- a/pkg/accumulator/resaccumulator_test.go +++ b/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, " ") } diff --git a/pkg/expansion/expand.go b/pkg/expansion/expand.go index de55e4614..fb15f2e13 100644 --- a/pkg/expansion/expand.go +++ b/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) diff --git a/pkg/expansion/expand_test.go b/pkg/expansion/expand_test.go index 2a8a13125..fbec78a1d 100644 --- a/pkg/expansion/expand_test.go +++ b/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) } diff --git a/pkg/ifc/ifc.go b/pkg/ifc/ifc.go index e26059537..ac61f03f4 100644 --- a/pkg/ifc/ifc.go +++ b/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 diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index 5f1b9b1c5..a059ef6f3 100644 --- a/pkg/resource/resource.go +++ b/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 } diff --git a/pkg/transformers/mutatefield_test.go b/pkg/transformers/mutatefield_test.go index 31fd8d643..528c0cb34 100644 --- a/pkg/transformers/mutatefield_test.go +++ b/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) } diff --git a/pkg/transformers/refvars.go b/pkg/transformers/refvars.go index 794e87438..57cb71507 100644 --- a/pkg/transformers/refvars.go +++ b/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 diff --git a/pkg/transformers/refvars_test.go b/pkg/transformers/refvars_test.go index 18169ad90..f82597023 100644 --- a/pkg/transformers/refvars_test.go +++ b/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"}, }, diff --git a/pkg/types/kustomization.go b/pkg/types/kustomization.go index fae15dcc1..cb693a2ff 100644 --- a/pkg/types/kustomization.go +++ b/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"`