From 4556eb3a0cd31d327d6d2e359332c8c3169031a5 Mon Sep 17 00:00:00 2001 From: Jerome Brette Date: Tue, 18 Jun 2019 17:29:55 -0500 Subject: [PATCH] Improve robutness of helper code As per request, changed usage of pointer to int into plain int. Use -1 value where nil use to be used. --- k8sdeps/kunstruct/helper.go | 12 ++++++++---- k8sdeps/kunstruct/helper_test.go | 29 ++++++++++++----------------- k8sdeps/kunstruct/kunstruct.go | 14 +++++++------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/k8sdeps/kunstruct/helper.go b/k8sdeps/kunstruct/helper.go index 79c03ae0c..bebd4435c 100644 --- a/k8sdeps/kunstruct/helper.go +++ b/k8sdeps/kunstruct/helper.go @@ -31,7 +31,11 @@ import ( // field of the first item in the foo list type PathSection struct { fields []string - idx *int + idx int +} + +func newPathSection() PathSection { + return PathSection{idx: -1} } func appendNonEmpty(section *PathSection, field string) { @@ -41,7 +45,7 @@ func appendNonEmpty(section *PathSection, field string) { } func parseFields(path string) ([]PathSection, error) { - section := PathSection{} + section := newPathSection() sectionset := []PathSection{} if !strings.Contains(path, "[") { section.fields = strings.Split(path, ".") @@ -75,9 +79,9 @@ func parseFields(path string) ([]PathSection, error) { if err != nil { return nil, fmt.Errorf("invalid index %s", path) } - section.idx = &tmpIdx + section.idx = tmpIdx sectionset = append(sectionset, section) - section = PathSection{} + section = newPathSection() start = i + 1 insideParentheses = false diff --git a/k8sdeps/kunstruct/helper_test.go b/k8sdeps/kunstruct/helper_test.go index 4d5b3c5a3..46e5011be 100644 --- a/k8sdeps/kunstruct/helper_test.go +++ b/k8sdeps/kunstruct/helper_test.go @@ -23,20 +23,15 @@ import ( type PathSectionSlice []PathSection -func buildPath(idx *int, fields ...string) PathSectionSlice { +func buildPath(idx int, fields ...string) PathSectionSlice { return PathSectionSlice{PathSection{fields: fields, idx: idx}} } -func (a PathSectionSlice) addSection(idx *int, fields ...string) PathSectionSlice { +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 @@ -47,55 +42,55 @@ func TestParseField(t *testing.T) { { name: "oneField", pathToField: "Kind", - expectedValue: buildPath(nil, "Kind"), + expectedValue: buildPath(-1, "Kind"), errorExpected: false, }, { name: "twoFields", pathToField: "metadata.name", - expectedValue: buildPath(nil, "metadata", "name"), + expectedValue: buildPath(-1, "metadata", "name"), errorExpected: false, }, { name: "threeFields", pathToField: "spec.ports.port", - expectedValue: buildPath(nil, "spec", "ports", "port"), + expectedValue: buildPath(-1, "spec", "ports", "port"), errorExpected: false, }, { name: "empty", pathToField: "", - expectedValue: buildPath(nil, ""), + expectedValue: buildPath(-1, ""), errorExpected: false, }, { name: "validStringIndex", pathToField: "that[1]", - expectedValue: buildPath(one, "that"), + expectedValue: buildPath(1, "that"), errorExpected: false, }, { name: "sliceInSlice", pathToField: "that[1][0]", - expectedValue: buildPath(one, "that").addSection(zero), + expectedValue: buildPath(1, "that").addSection(0), errorExpected: false, }, { name: "validStructSubField", pathToField: "those[1].field2", - expectedValue: buildPath(one, "those").addSection(nil, "field2"), + expectedValue: buildPath(1, "those").addSection(-1, "field2"), errorExpected: false, }, { name: "validStructSubFieldIndex", pathToField: "these[1].field2[0]", - expectedValue: buildPath(one, "these").addSection(zero, "field2"), + expectedValue: buildPath(1, "these").addSection(0, "field2"), errorExpected: false, }, { name: "validStructSubFieldIndexSubfield", pathToField: "complextree[1].field2[1].stringsubfield", - expectedValue: buildPath(one, "complextree").addSection(one, "field2").addSection(nil, "stringsubfield"), + expectedValue: buildPath(1, "complextree").addSection(1, "field2").addSection(-1, "stringsubfield"), errorExpected: false, }, { @@ -119,7 +114,7 @@ func TestParseField(t *testing.T) { { name: "validFieldsWithQuotes", pathToField: "'complextree'[1].field2[1].'stringsubfield'", - expectedValue: buildPath(one, "complextree").addSection(one, "field2").addSection(nil, "stringsubfield"), + expectedValue: buildPath(1, "complextree").addSection(1, "field2").addSection(-1, "stringsubfield"), errorExpected: false, }, } diff --git a/k8sdeps/kunstruct/kunstruct.go b/k8sdeps/kunstruct/kunstruct.go index 163b3b892..cb39c1dea 100644 --- a/k8sdeps/kunstruct/kunstruct.go +++ b/k8sdeps/kunstruct/kunstruct.go @@ -92,7 +92,7 @@ func (fs *UnstructAdapter) selectSubtree(path string) (map[string]interface{}, [ idx := sections[sectionIdx].idx fields := sections[sectionIdx].fields - if idx == nil { + if idx == -1 { // This section has no index return content, fields, true, nil } @@ -107,24 +107,24 @@ func (fs *UnstructAdapter) selectSubtree(path string) (map[string]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 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]} + 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{}) + 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]) + fmt.Errorf("%#v is expected to be of type map[string]interface{}", s[idx]) } content = newContent }