From 375fd9140bcb785b9495de64f5511a99a762e691 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Fri, 9 Oct 2015 16:15:05 +0100 Subject: [PATCH] review feedback * Opted to remove json path traversal, as it added more code than it saved. --- proxy/create_container_interceptor.go | 19 ++++------- proxy/json.go | 19 ++++------- proxy/json_test.go | 46 +++++++++++++-------------- 3 files changed, 35 insertions(+), 49 deletions(-) diff --git a/proxy/create_container_interceptor.go b/proxy/create_container_interceptor.go index 5f154631..381c3bf2 100644 --- a/proxy/create_container_interceptor.go +++ b/proxy/create_container_interceptor.go @@ -82,17 +82,7 @@ func (i *createContainerInterceptor) setWeaveWaitEntrypoint(container jsonObject var entrypoint []string entrypoint, err := container.StringArray("Entrypoint") if err != nil { - err, ok := err.(*UnmarshalWrongTypeError) - if !ok { - return err - } - - got, ok := err.Got.(string) - if !ok { - return err - } - - entrypoint = []string{got} + return err } cmd, err := container.StringArray("Cmd") @@ -146,7 +136,6 @@ func (i *createContainerInterceptor) setHostname(container jsonObject, name, dns if len(name)+1+len(trimmedDNSDomain) > MaxDockerHostname { Log.Warningf("Container name [%s] too long to be used as hostname", name) } else { - hostname = name container["Hostname"] = name container["Domainname"] = trimmedDNSDomain } @@ -169,7 +158,11 @@ func (i *createContainerInterceptor) containerHostname(r *http.Request, containe } func (i *createContainerInterceptor) hostnameFromLabel(hostname string, container jsonObject) (string, error) { - label, err := container.String("Labels", i.proxy.Config.HostnameFromLabel) + labels, err := container.Object("Labels") + if err != nil { + return "", err + } + label, err := labels.String(i.proxy.Config.HostnameFromLabel) if err != nil { return "", err } diff --git a/proxy/json.go b/proxy/json.go index 8e333612..f736a77b 100644 --- a/proxy/json.go +++ b/proxy/json.go @@ -31,16 +31,7 @@ func (j jsonObject) Object(key string) (jsonObject, error) { return jsonObject(result), nil } -// For parsing json objects. Look up (or create) a string in the given map. -func (j jsonObject) String(key string, ks ...string) (string, error) { - if len(ks) > 0 { - j, err := j.Object(key) - if err != nil { - return "", err - } - return j.String(ks[0], ks[1:]...) - } - +func (j jsonObject) String(key string) (string, error) { iface, ok := j[key] if !ok || iface == nil { return "", nil @@ -54,7 +45,6 @@ func (j jsonObject) String(key string, ks ...string) (string, error) { return result, nil } -// For parsing json objects. Look up (or create) a string array in the given map. func (j jsonObject) StringArray(key string) ([]string, error) { iface, ok := j[key] if !ok || iface == nil { @@ -62,6 +52,8 @@ func (j jsonObject) StringArray(key string) ([]string, error) { } switch o := iface.(type) { + case string: + return []string{o}, nil case []string: return o, nil case []interface{}: @@ -70,10 +62,11 @@ func (j jsonObject) StringArray(key string) ([]string, error) { if s, ok := s.(string); ok { result = append(result, s) } else { - return nil, &UnmarshalWrongTypeError{key, "array of strings", iface} + return nil, &UnmarshalWrongTypeError{key, "string or array of strings", iface} } } return result, nil } - return nil, &UnmarshalWrongTypeError{key, "array of strings", iface} + + return nil, &UnmarshalWrongTypeError{key, "string or array of strings", iface} } diff --git a/proxy/json_test.go b/proxy/json_test.go index 0083bebc..d57d07dd 100644 --- a/proxy/json_test.go +++ b/proxy/json_test.go @@ -36,49 +36,43 @@ func TestLookupObject(t *testing.T) { for _, test := range tests { gotResult, gotErr := test.root.Object(test.key) msg := fmt.Sprintf("%q.Object(%q) => %q, %q", test.root, test.key, gotResult, gotErr) - assert.Equal(t, gotResult, test.result, msg) - assert.Equal(t, gotErr, test.err, msg) + assert.Equal(t, test.result, gotResult, msg) + assert.Equal(t, test.err, gotErr, msg) } } func TestLookupString(t *testing.T) { tests := []struct { root jsonObject - path []string + key string result string err error }{ { jsonObject{}, - []string{"a", "b"}, + "a", "", nil, }, { jsonObject{"nonString": int(1)}, - []string{"nonString"}, + "nonString", "", &UnmarshalWrongTypeError{Field: "nonString", Expected: "string", Got: 1}, }, - { - jsonObject{"nonObject": int(1)}, - []string{"nonObject", "b"}, - "", - &UnmarshalWrongTypeError{Field: "nonObject", Expected: "object", Got: 1}, - }, } for _, test := range tests { - gotResult, gotErr := test.root.String(test.path[0], test.path[1:]...) - msg := fmt.Sprintf("%q.String(%q) => %q, %q", test.root, test.path, gotResult, gotErr) - assert.Equal(t, gotResult, test.result, msg) - assert.Equal(t, gotErr, test.err, msg) + gotResult, gotErr := test.root.String(test.key) + msg := fmt.Sprintf("%q.String(%q) => %q, %q", test.root, test.key, gotResult, gotErr) + assert.Equal(t, test.result, gotResult, msg) + assert.Equal(t, test.err, gotErr, msg) } } func TestLookupStringArray(t *testing.T) { tests := []struct { root jsonObject - path string + key string result []string err error }{ @@ -101,16 +95,22 @@ func TestLookupStringArray(t *testing.T) { nil, }, { - jsonObject{"string": "foo"}, - "string", + jsonObject{"a": "foo"}, + "a", + []string{"foo"}, nil, - &UnmarshalWrongTypeError{Field: "string", Expected: "array of strings", Got: "foo"}, + }, + { + jsonObject{"int": 5}, + "int", + nil, + &UnmarshalWrongTypeError{Field: "int", Expected: "string or array of strings", Got: 5}, }, } for _, test := range tests { - gotResult, gotErr := test.root.StringArray(test.path) - msg := fmt.Sprintf("%q.String(%q) => %q, %q", test.root, test.path, gotResult, gotErr) - assert.Equal(t, gotResult, test.result, msg) - assert.Equal(t, gotErr, test.err, msg) + gotResult, gotErr := test.root.StringArray(test.key) + msg := fmt.Sprintf("%q.String(%q) => %q, %q", test.root, test.key, gotResult, gotErr) + assert.Equal(t, test.result, gotResult, msg) + assert.Equal(t, test.err, gotErr, msg) } }