From 4dcc02a9273c8a002837a1fa89856d3ad0e25cd8 Mon Sep 17 00:00:00 2001 From: Benjamin Yolken Date: Mon, 22 Feb 2021 17:47:06 -0800 Subject: [PATCH 1/3] Bump version --- pkg/util/template.go | 45 +++++++++++++++++++++++++++++++++++---- pkg/util/template_test.go | 41 +++++++++++++++++++++++++++++++++++ pkg/version/version.go | 2 +- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/pkg/util/template.go b/pkg/util/template.go index 68c4656..b50a09b 100644 --- a/pkg/util/template.go +++ b/pkg/util/template.go @@ -8,6 +8,7 @@ import ( "net/url" "os" "path/filepath" + "reflect" "strings" "text/template" @@ -15,10 +16,15 @@ import ( "github.com/ghodss/yaml" ) -var extraTemplateFuncs = template.FuncMap{ - "urlEncode": url.QueryEscape, - "toYaml": toYaml, -} +var ( + extraTemplateFuncs = template.FuncMap{ + "lookup": lookup, + "toYaml": toYaml, + "urlEncode": url.QueryEscape, + } + + zeroValue = reflect.Value{} +) // ApplyTemplate runs golang templating on all files in the provided path, // replacing them in-place with their templated versions. @@ -214,6 +220,37 @@ func configMapEntriesGenerator( } } +// lookup does a dot-separated path lookup on the input struct or map. If the input or any of +// its children on the targeted path are not a map or struct, it returns nil. +func lookup(path string, input interface{}) interface{} { + obj := reflect.ValueOf(input) + + components := strings.Split(path, ".") + for i := 0; i < len(components); { + component := components[i] + + switch obj.Kind() { + case reflect.Struct: + obj = obj.FieldByName(component) + i++ + case reflect.Map: + obj = obj.MapIndex(reflect.ValueOf(component)) + i++ + case reflect.Ptr, reflect.Interface: + // Get the thing being pointed to or interfaced, don't advance index + obj = obj.Elem() + default: + obj = zeroValue + break + } + } + + if obj == zeroValue { + return nil + } + return obj.Interface() +} + func toYaml(input interface{}) (string, error) { bytes, err := yaml.Marshal(input) if err != nil { diff --git a/pkg/util/template_test.go b/pkg/util/template_test.go index 6ef50d0..ed0ece0 100644 --- a/pkg/util/template_test.go +++ b/pkg/util/template_test.go @@ -136,3 +136,44 @@ func fileContents(t *testing.T, path string) string { return string(contents) } + +type TestStruct struct { + Key string + Inner TestStructInner + Inner2 *TestStructInner +} +type TestStructInner struct { + Map map[string]interface{} +} + +func TestLookup(t *testing.T) { + s := TestStruct{ + Key: "value0", + Inner: TestStructInner{ + Map: map[string]interface{}{ + "key1": "value1", + "key2": map[string]interface{}{ + "key3": map[string]interface{}{ + "key4": "value4", + }, + "key5": 1234, + }, + }, + }, + Inner2: &TestStructInner{ + Map: map[string]interface{}{ + "key6": "value6", + }, + }, + } + assert.Equal(t, "value0", lookup("Key", s)) + assert.Equal(t, nil, lookup("bad-key", s)) + assert.Equal(t, nil, lookup("", s)) + assert.Equal(t, "value1", lookup("Inner.Map.key1", s)) + assert.Equal(t, "value1", lookup("Inner.Map.key1", &s)) + assert.Equal(t, "value4", lookup("Inner.Map.key2.key3.key4", s)) + assert.Equal(t, 1234, lookup("Inner.Map.key2.key5", s)) + assert.Equal(t, nil, lookup("Inner.Map.non-existent-key", s)) + assert.Equal(t, nil, lookup("Inner.Map.key2.non-existent-key", s)) + assert.Equal(t, "value6", lookup("Inner2.Map.key6", s)) +} diff --git a/pkg/version/version.go b/pkg/version/version.go index ea85fdf..af9a452 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -1,4 +1,4 @@ package version // Version stores the current kubeapply version. -const Version = "0.0.27" +const Version = "0.0.28" From f32921336f8a0e23cdb10bc54059f1eda77ae4c9 Mon Sep 17 00:00:00 2001 From: Benjamin Yolken Date: Mon, 22 Feb 2021 17:53:04 -0800 Subject: [PATCH 2/3] Misc. fixes --- pkg/util/template.go | 5 +++-- pkg/util/template_test.go | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/util/template.go b/pkg/util/template.go index b50a09b..b853f08 100644 --- a/pkg/util/template.go +++ b/pkg/util/template.go @@ -226,6 +226,7 @@ func lookup(path string, input interface{}) interface{} { obj := reflect.ValueOf(input) components := strings.Split(path, ".") + for i := 0; i < len(components); { component := components[i] @@ -240,8 +241,8 @@ func lookup(path string, input interface{}) interface{} { // Get the thing being pointed to or interfaced, don't advance index obj = obj.Elem() default: - obj = zeroValue - break + // Got an unexpected type + return nil } } diff --git a/pkg/util/template_test.go b/pkg/util/template_test.go index ed0ece0..c77bf0a 100644 --- a/pkg/util/template_test.go +++ b/pkg/util/template_test.go @@ -169,6 +169,7 @@ func TestLookup(t *testing.T) { assert.Equal(t, "value0", lookup("Key", s)) assert.Equal(t, nil, lookup("bad-key", s)) assert.Equal(t, nil, lookup("", s)) + assert.Equal(t, nil, lookup("key", "not a map")) assert.Equal(t, "value1", lookup("Inner.Map.key1", s)) assert.Equal(t, "value1", lookup("Inner.Map.key1", &s)) assert.Equal(t, "value4", lookup("Inner.Map.key2.key3.key4", s)) From b0cfcc37dc84e6d87bcc166e610088578b87451a Mon Sep 17 00:00:00 2001 From: Benjamin Yolken Date: Tue, 23 Feb 2021 12:10:23 -0800 Subject: [PATCH 3/3] Updates in response to code review feedback --- pkg/util/template.go | 52 +++++++++++------- pkg/util/template_test.go | 113 ++++++++++++++++++++++++++------------ 2 files changed, 110 insertions(+), 55 deletions(-) diff --git a/pkg/util/template.go b/pkg/util/template.go index b853f08..e9958c5 100644 --- a/pkg/util/template.go +++ b/pkg/util/template.go @@ -18,12 +18,11 @@ import ( var ( extraTemplateFuncs = template.FuncMap{ - "lookup": lookup, - "toYaml": toYaml, - "urlEncode": url.QueryEscape, + "lookup": lookup, + "pathLookup": pathLookup, + "toYaml": toYaml, + "urlEncode": url.QueryEscape, } - - zeroValue = reflect.Value{} ) // ApplyTemplate runs golang templating on all files in the provided path, @@ -220,36 +219,49 @@ func configMapEntriesGenerator( } } -// lookup does a dot-separated path lookup on the input struct or map. If the input or any of -// its children on the targeted path are not a map or struct, it returns nil. -func lookup(path string, input interface{}) interface{} { +// lookup does a dot-separated path lookup on the input map. If a key on the path is +// not found, it returns nil. If the input or any of its children on the lookup path is not +// a map, it returns an error. +func lookup(input interface{}, path string) (interface{}, error) { obj := reflect.ValueOf(input) - components := strings.Split(path, ".") for i := 0; i < len(components); { - component := components[i] - switch obj.Kind() { - case reflect.Struct: - obj = obj.FieldByName(component) - i++ case reflect.Map: - obj = obj.MapIndex(reflect.ValueOf(component)) + obj = obj.MapIndex(reflect.ValueOf(components[i])) i++ case reflect.Ptr, reflect.Interface: + if obj.IsNil() { + return nil, nil + } + // Get the thing being pointed to or interfaced, don't advance index obj = obj.Elem() default: - // Got an unexpected type - return nil + if obj.IsValid() { + // An object was found, but it's not a map. Return an error. + return nil, fmt.Errorf( + "Tried to traverse a value that's not a map (kind=%s)", + obj.Kind(), + ) + } + + // An intermediate key wasn't found + return nil, nil } } - if obj == zeroValue { - return nil + if !obj.IsValid() { + // The last key wasn't found + return nil, nil } - return obj.Interface() + return obj.Interface(), nil +} + +// pathLookup is the same as lookup, but with the arguments flipped. +func pathLookup(path string, input interface{}) (interface{}, error) { + return lookup(input, path) } func toYaml(input interface{}) (string, error) { diff --git a/pkg/util/template_test.go b/pkg/util/template_test.go index c77bf0a..302055a 100644 --- a/pkg/util/template_test.go +++ b/pkg/util/template_test.go @@ -137,44 +137,87 @@ func fileContents(t *testing.T, path string) string { return string(contents) } -type TestStruct struct { - Key string - Inner TestStructInner - Inner2 *TestStructInner -} -type TestStructInner struct { - Map map[string]interface{} -} - func TestLookup(t *testing.T) { - s := TestStruct{ - Key: "value0", - Inner: TestStructInner{ - Map: map[string]interface{}{ - "key1": "value1", - "key2": map[string]interface{}{ - "key3": map[string]interface{}{ - "key4": "value4", - }, - "key5": 1234, - }, + m := map[string]interface{}{ + "key1": "value1", + "key2": map[string]interface{}{ + "key3": map[string]interface{}{ + "key4": "value4", }, + "key5": 1234, }, - Inner2: &TestStructInner{ - Map: map[string]interface{}{ - "key6": "value6", - }, + "key6": nil, + } + + type testCase struct { + input interface{} + path string + expectedResult interface{} + expectErr bool + } + + testCases := []testCase{ + { + input: m, + path: "bad-key", + expectedResult: nil, + }, + { + input: m, + path: "", + expectedResult: nil, + }, + { + input: nil, + path: "key1", + expectedResult: nil, }, + { + input: "not a map", + path: "key1", + expectedResult: nil, + expectErr: true, + }, + { + input: m, + path: "key1", + expectedResult: "value1", + }, + { + input: &m, + path: "key1", + expectedResult: "value1", + }, + { + input: m, + path: "key1.not-a-map", + expectedResult: nil, + expectErr: true, + }, + { + input: m, + path: "key2.key3.key4", + expectedResult: "value4", + }, + { + input: m, + path: "key2.key5", + expectedResult: 1234, + }, + { + input: m, + path: "key6.nil-key", + expectedResult: nil, + }, + } + + for index, tc := range testCases { + result, err := lookup(tc.input, tc.path) + assert.Equal(t, tc.expectedResult, result, "Unexpected result for case %d", index) + if tc.expectErr { + assert.Error(t, err, "Did not get expected error in case %d", index) + } else { + assert.NoError(t, err, "Got unexpected error in case %d", index) + } } - assert.Equal(t, "value0", lookup("Key", s)) - assert.Equal(t, nil, lookup("bad-key", s)) - assert.Equal(t, nil, lookup("", s)) - assert.Equal(t, nil, lookup("key", "not a map")) - assert.Equal(t, "value1", lookup("Inner.Map.key1", s)) - assert.Equal(t, "value1", lookup("Inner.Map.key1", &s)) - assert.Equal(t, "value4", lookup("Inner.Map.key2.key3.key4", s)) - assert.Equal(t, 1234, lookup("Inner.Map.key2.key5", s)) - assert.Equal(t, nil, lookup("Inner.Map.non-existent-key", s)) - assert.Equal(t, nil, lookup("Inner.Map.key2.non-existent-key", s)) - assert.Equal(t, "value6", lookup("Inner2.Map.key6", s)) }