From 6fd1680b9f0a453084b9c7baafd2f9672616b6bf Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Sat, 6 Feb 2016 15:43:16 -0800 Subject: [PATCH 1/2] ref(client): refactor url parsing --- cmd/helm/list.go | 3 +- dm/client.go | 77 ++++++++++++++++++++++++++++++++++++++++++----- dm/client_test.go | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 dm/client_test.go diff --git a/cmd/helm/list.go b/cmd/helm/list.go index eadb25757..afc26c090 100644 --- a/cmd/helm/list.go +++ b/cmd/helm/list.go @@ -22,7 +22,6 @@ func listCmd() cli.Command { } func list(host string) error { - client := dm.NewClient(host) - client.Protocol = "http" + client := dm.NewClient(host).SetDebug(true) return client.ListDeployments() } diff --git a/dm/client.go b/dm/client.go index 301d1f564..74dcf594c 100644 --- a/dm/client.go +++ b/dm/client.go @@ -6,8 +6,10 @@ import ( "io" "io/ioutil" "net/http" + "net/url" "os" "path/filepath" + "strings" "time" "github.com/ghodss/yaml" @@ -16,6 +18,9 @@ import ( // The default HTTP timeout var DefaultHTTPTimeout = time.Second * 10 +// The default HTTP Protocol +var DefaultHTTPProtocol = "http" + // Client is a DM client. type Client struct { // Timeout on HTTP connections. @@ -26,29 +31,61 @@ type Client struct { Protocol string // Transport Transport http.RoundTripper + // Debug enables http logging + Debug bool + + // Base URL for remote service + baseURL *url.URL } // NewClient creates a new DM client. Host name is required. func NewClient(host string) *Client { + url, _ := DefaultServerURL(host) + return &Client{ HTTPTimeout: DefaultHTTPTimeout, - Protocol: "https", - Host: host, - Transport: NewDebugTransport(nil), + baseURL: url, + Transport: http.DefaultTransport, + } +} + +// SetDebug enables debug mode which logs http +func (c *Client) SetDebug(enable bool) *Client { + c.Debug = enable + return c +} + +// transport wraps client transport if debug is enabled +func (c *Client) transport() http.RoundTripper { + if c.Debug { + return NewDebugTransport(c.Transport) } + return c.Transport +} + +// SetTransport sets a custom Transport. Defaults to http.DefaultTransport +func (c *Client) SetTransport(tr http.RoundTripper) *Client { + c.Transport = tr + return c } // url constructs the URL. -func (c *Client) url(path string) string { - // TODO: Switch to net.URL - return c.Protocol + "://" + c.Host + "/" + path +func (c *Client) url(rawurl string) (string, error) { + u, err := url.Parse(rawurl) + if err != nil { + return "", err + } + return c.baseURL.ResolveReference(u).String(), nil } // CallService is a low-level function for making an API call. // // This calls the service and then unmarshals the returned data into dest. func (c *Client) CallService(path, method, action string, dest interface{}, reader io.ReadCloser) error { - u := c.url(path) + u, err := c.url(path) + if err != nil { + return err + } resp, err := c.callHTTP(u, method, action, reader) if err != nil { @@ -76,7 +113,7 @@ func (c *Client) callHTTP(path, method, action string, reader io.ReadCloser) (st client := http.Client{ Timeout: time.Duration(time.Duration(DefaultHTTPTimeout) * time.Second), - Transport: c.Transport, + Transport: c.transport(), } response, err := client.Do(request) @@ -155,3 +192,27 @@ func (c *Client) DeployChart(filename, deployname string) error { return nil } + +// DefaultServerURL converts a host, host:port, or URL string to the default base server API path +// to use with a Client +func DefaultServerURL(host string) (*url.URL, error) { + if host == "" { + return nil, fmt.Errorf("host must be a URL or a host:port pair") + } + base := host + hostURL, err := url.Parse(base) + if err != nil { + return nil, err + } + if hostURL.Scheme == "" { + hostURL, err = url.Parse(DefaultHTTPProtocol + "://" + base) + if err != nil { + return nil, err + } + } + if len(hostURL.Path) > 0 && !strings.HasSuffix(hostURL.Path, "/") { + hostURL.Path = hostURL.Path + "/" + } + + return hostURL, nil +} diff --git a/dm/client_test.go b/dm/client_test.go new file mode 100644 index 000000000..f6b1d0dfe --- /dev/null +++ b/dm/client_test.go @@ -0,0 +1,60 @@ +package dm + +import ( + "testing" +) + +func TestDefaultServerURL(t *testing.T) { + tt := []struct { + host string + url string + }{ + {"127.0.0.1", "http://127.0.0.1"}, + {"127.0.0.1:8080", "http://127.0.0.1:8080"}, + {"foo.bar.com", "http://foo.bar.com"}, + {"foo.bar.com/prefix", "http://foo.bar.com/prefix/"}, + {"http://host/prefix", "http://host/prefix/"}, + {"https://host/prefix", "https://host/prefix/"}, + {"http://host", "http://host"}, + {"http://host/other", "http://host/other/"}, + } + + for _, tc := range tt { + u, err := DefaultServerURL(tc.host) + if err != nil { + t.Fatal(err) + } + + if tc.url != u.String() { + t.Errorf("%s, expected host %s, got %s", tc.host, tc.url, u.String()) + } + } +} + +func TestURL(t *testing.T) { + tt := []struct { + host string + path string + url string + }{ + {"127.0.0.1", "foo", "http://127.0.0.1/foo"}, + {"127.0.0.1:8080", "foo", "http://127.0.0.1:8080/foo"}, + {"foo.bar.com", "foo", "http://foo.bar.com/foo"}, + {"foo.bar.com/prefix", "foo", "http://foo.bar.com/prefix/foo"}, + {"http://host/prefix", "foo", "http://host/prefix/foo"}, + {"http://host", "foo", "http://host/foo"}, + {"http://host/other", "/foo", "http://host/foo"}, + } + + for _, tc := range tt { + c := NewClient(tc.host) + p, err := c.url(tc.path) + if err != nil { + t.Fatal(err) + } + + if tc.url != p { + t.Errorf("expected %s, got %s", tc.url, p) + } + } +} From bb9c5fcad63846896a048c3e1e671777c13ce0a6 Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Sat, 6 Feb 2016 15:53:22 -0800 Subject: [PATCH 2/2] fix(glide): use master branch of deployment-manager --- glide.lock | 17 +++++++++++------ glide.yaml | 3 --- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/glide.lock b/glide.lock index d34eae154..7efa51c8a 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: fce0581223b80f7a04fbb4ad4bd7ff8fa3d12e879dba894ba448770933731887 -updated: 2016-02-02T17:30:13.283644703-07:00 +hash: 71e9a5a2d1f27a0ecfa70595154f87fee9f5519ba6bdea4d01834bab5aa29074 +updated: 2016-02-06T17:26:31.257079434-08:00 imports: - name: github.com/codegangsta/cli version: cf1f63a7274872768d4037305d572b70b1199397 @@ -13,8 +13,6 @@ imports: version: 45bba206dd5270d96bac4942dcfe515726613249 - name: github.com/google/go-github version: b8b4ac742977310ff6e75140a403a38dab109977 - subpackages: - - /github - name: github.com/google/go-querystring version: 2a60fc2ba6c19de80291203597d752e9ba58e4c0 - name: github.com/gorilla/context @@ -27,8 +25,6 @@ imports: version: 14c555599c2a4f493c1e13fd1ea6fdf721739028 - name: github.com/kubernetes/deployment-manager version: "" - repo: https://github.com/technosophos/deployment-manager - vcs: git subpackages: - /common - name: github.com/Masterminds/semver @@ -43,6 +39,15 @@ imports: version: 8a57ed94ffd43444c0879fe75701732a38afc985 - name: golang.org/x/text version: 5aaa1a807bf8a2f763540b140e7805973476eb88 +- name: google.golang.com/appengine/datastore + version: "" + repo: https://google.golang.com/appengine/datastore +- name: google.golang.com/appengine/memcache + version: "" + repo: https://google.golang.com/appengine/memcache +- name: google.golang.com/appengine/user + version: "" + repo: https://google.golang.com/appengine/user - name: google.golang.org/api version: 8fa1015948e6fc21c025050624e4c4e2f4f405c4 - name: google.golang.org/appengine diff --git a/glide.yaml b/glide.yaml index dd229737b..6a6b34b16 100644 --- a/glide.yaml +++ b/glide.yaml @@ -4,9 +4,6 @@ ignore: import: - package: github.com/codegangsta/cli - package: github.com/kubernetes/deployment-manager - version: feat/chartfile - repo: https://github.com/technosophos/deployment-manager - vcs: git subpackages: - /common - package: github.com/ghodss/yaml