From de65503177337f9f3bf807f7ca3b9e3198db26aa Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Fri, 8 Jun 2018 15:07:57 -0700 Subject: [PATCH] Handle arb length paths in GetFieldName --- pkg/app/application.go | 8 +++--- pkg/resource/resource.go | 29 ++++++++++----------- pkg/resource/resource_test.go | 48 +++++++++++++++++++++-------------- 3 files changed, 46 insertions(+), 39 deletions(-) diff --git a/pkg/app/application.go b/pkg/app/application.go index 1a5b7065e..09f55a152 100644 --- a/pkg/app/application.go +++ b/pkg/app/application.go @@ -22,7 +22,6 @@ import ( "bytes" "encoding/json" "fmt" - "strings" "github.com/ghodss/yaml" "github.com/golang/glog" @@ -316,10 +315,9 @@ func (a *applicationImpl) resolveRefVars(resources resmap.ResMap) (map[string]st } for _, refvar := range vars { - refGVKN := resource.NewResId(refvar.ObjRef.GroupVersionKind(), refvar.ObjRef.Name) - if r, found := resources[refGVKN]; found { - s, err := resource.GetFieldValue( - r.UnstructuredContent(), strings.Split(refvar.FieldRef.FieldPath, ".")) + id := resource.NewResId(refvar.ObjRef.GroupVersionKind(), refvar.ObjRef.Name) + if r, found := resources[id]; found { + s, err := r.GetFieldValue(refvar.FieldRef.FieldPath) if err != nil { return nil, fmt.Errorf("failed to resolve referred var: %+v", refvar) } diff --git a/pkg/resource/resource.go b/pkg/resource/resource.go index aa3e5b5ec..e10b1432f 100644 --- a/pkg/resource/resource.go +++ b/pkg/resource/resource.go @@ -21,6 +21,8 @@ import ( "encoding/json" "fmt" + "strings" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" ) @@ -49,6 +51,13 @@ func NewBehaviorlessResource(u *unstructured.Unstructured) *Resource { return &Resource{Unstructured: *u, b: BehaviorUnspecified} } +// NewResource returns a new instance of Resource. +func NewResource(m map[string]interface{}) *Resource { + return &Resource{ + Unstructured: unstructured.Unstructured{Object: m}, + b: BehaviorUnspecified} +} + // Behavior returns the behavior for the resource. func (r *Resource) Behavior() GenerationBehavior { return r.b @@ -94,21 +103,14 @@ func mergeStringMaps(maps ...map[string]string) map[string]string { return result } -func newUnstructuredFromObject(in runtime.Object) (*unstructured.Unstructured, error) { - marshaled, err := json.Marshal(in) - if err != nil { - return nil, err - } - var out unstructured.Unstructured - err = out.UnmarshalJSON(marshaled) - return &out, err +func (r *Resource) GetFieldValue(fieldPath string) (string, error) { + return getFieldValue(r.UnstructuredContent(), strings.Split(fieldPath, ".")) } -func GetFieldValue(m map[string]interface{}, pathToField []string) (string, error) { +func getFieldValue(m map[string]interface{}, pathToField []string) (string, error) { if len(pathToField) == 0 { return "", fmt.Errorf("Field not found") } - if len(pathToField) == 1 { if v, found := m[pathToField[0]]; found { if s, ok := v.(string); ok { @@ -118,13 +120,10 @@ func GetFieldValue(m map[string]interface{}, pathToField []string) (string, erro } return "", fmt.Errorf("field at given fieldpath does not exist") } - - curr, rest := pathToField[0], pathToField[1] - - v := m[curr] + v := m[pathToField[0]] switch typedV := v.(type) { case map[string]interface{}: - return GetFieldValue(typedV, []string{rest}) + return getFieldValue(typedV, pathToField[1:]) default: return "", fmt.Errorf("%#v is not expected to be a primitive type", typedV) } diff --git a/pkg/resource/resource_test.go b/pkg/resource/resource_test.go index 4ccf9184e..a76bee20a 100644 --- a/pkg/resource/resource_test.go +++ b/pkg/resource/resource_test.go @@ -20,8 +20,8 @@ import ( "testing" ) -func TestGetFieldAsString(t *testing.T) { - m := map[string]interface{}{ +func TestGetFieldValue(t *testing.T) { + res := NewResource(map[string]interface{}{ "Kind": "Service", "metadata": map[string]interface{}{ "labels": map[string]string{ @@ -29,37 +29,47 @@ func TestGetFieldAsString(t *testing.T) { }, "name": "service-name", }, - } + "spec": map[string]interface{}{ + "ports": map[string]interface{}{ + "port": "80", + }, + }, + }) tests := []struct { - pathToField []string - expectedName string - expectedError bool + pathToField string + expectedValue string + errorExpected bool }{ { - pathToField: []string{"Kind"}, - expectedName: "Service", - expectedError: false, + pathToField: "Kind", + expectedValue: "Service", + errorExpected: false, }, { - pathToField: []string{"metadata", "name"}, - expectedName: "service-name", - expectedError: false, + pathToField: "metadata.name", + expectedValue: "service-name", + errorExpected: false, }, { - pathToField: []string{"metadata", "non-existing-field"}, - expectedName: "", - expectedError: true, + pathToField: "metadata.non-existing-field", + expectedValue: "", + errorExpected: true, + }, + { + pathToField: "spec.ports.port", + expectedValue: "80", + errorExpected: false, }, } for _, test := range tests { - s, err := GetFieldValue(m, test.pathToField) - if test.expectedError && err == nil { + s, err := res.GetFieldValue(test.pathToField) + if test.errorExpected && err == nil { t.Fatalf("should return error, but no error returned") } else { - if test.expectedName != s { - t.Fatalf("Got:%s expected:%s", s, test.expectedName) + if test.expectedValue != s { + t.Fatalf("Got:%s expected:%s", s, test.expectedValue) } } }