Merge pull request #85 from SimonRichardson/update-port

Allow overloading of headers.

client/client.go

@@ -93,8 +93,29 @@ type AuthenticatingClient interface {
 	IdentityAuthOptions() (identity.AuthOptions, error)
 }
 
-// A single http client is shared between all Goose clients.
-var sharedHttpClient = goosehttp.New()
+// Option allows the adaptation of a client given new options.
+// Both client.Client and http.Client have Options. To allow isolation between
+// layers, we have separate options. If client.Client and http.Client want
+// different options they can do so, without causing conflict.
+type Option func(*options)
+
+type options struct {
+	httpHeadersFunc goosehttp.HeadersFunc
+}
+
+// WithHTTPHeadersFunc allows passing in a new HTTP headers func for the client
+// to execute for each request.
+func WithHTTPHeadersFunc(httpHeadersFunc goosehttp.HeadersFunc) Option {
+	return func(options *options) {
+		options.httpHeadersFunc = httpHeadersFunc
+	}
+}
+
+func newOptions() *options {
+	return &options{
+		httpHeadersFunc: goosehttp.DefaultHeaders,
+	}
+}
 
 // This client sends requests without authenticating.
 type client struct {
@@ -142,19 +163,72 @@ func (c *authenticatingClient) EndpointsForRegion(region string) identity.Servic
 
 var _ AuthenticatingClient = (*authenticatingClient)(nil)
 
-func NewPublicClient(baseURL string, logger logging.CompatLogger) Client {
-	client := client{baseURL: baseURL, logger: logger, httpClient: sharedHttpClient}
-	return &client
+// TODO (stickupkid): The needs some clean up.
+// All the following New constructor methods should actually be placed into
+// one factory method with a given configuration so that there is only one
+// place that a client can be constructed.
+
+// NewPublicClient creates a new Client that validates against TLS.
+func NewPublicClient(baseURL string, logger logging.CompatLogger, options ...Option) Client {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
+	return &client{
+		baseURL:    baseURL,
+		logger:     logger,
+		httpClient: goosehttp.New(goosehttp.WithHeadersFunc(opts.httpHeadersFunc)),
+	}
 }
 
-func NewNonValidatingPublicClient(baseURL string, logger logging.CompatLogger) Client {
+// NewNonValidatingPublicClient creates a new Client that doesn't validate
+// against TLS.
+func NewNonValidatingPublicClient(baseURL string, logger logging.CompatLogger, options ...Option) Client {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
 	return &client{
 		baseURL:    baseURL,
 		logger:     logger,
-		httpClient: goosehttp.NewNonSSLValidating(),
+		httpClient: goosehttp.NewNonSSLValidating(goosehttp.WithHeadersFunc(opts.httpHeadersFunc)),
 	}
 }
 
+// NewClient creates a new authenticated client.
+func NewClient(creds *identity.Credentials, authMethod identity.AuthMode, logger logging.CompatLogger, options ...Option) AuthenticatingClient {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
+	return newClient(creds, authMethod, goosehttp.New(goosehttp.WithHeadersFunc(opts.httpHeadersFunc)), logger)
+}
+
+// NewNonValidatingClient creates a new authenticated client that doesn't
+// validate against TLS.
+func NewNonValidatingClient(creds *identity.Credentials, authMethod identity.AuthMode, logger logging.CompatLogger, options ...Option) AuthenticatingClient {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
+	return newClient(creds, authMethod, goosehttp.NewNonSSLValidating(goosehttp.WithHeadersFunc(opts.httpHeadersFunc)), logger)
+}
+
+// NewClientTLSConfig creates a new authenticated client that allows passing
+// in a new TLS config.
+func NewClientTLSConfig(creds *identity.Credentials, authMethod identity.AuthMode, logger logging.CompatLogger, config *tls.Config, options ...Option) AuthenticatingClient {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
+	return newClient(creds, authMethod, goosehttp.NewWithTLSConfig(config, goosehttp.WithHeadersFunc(opts.httpHeadersFunc)), logger)
+}
+
 var defaultRequiredServiceTypes = []string{"compute", "object-store"}
 
 func newClient(creds *identity.Credentials, auth_method identity.AuthMode, httpClient goosehttp.HttpClient, logger logging.CompatLogger) AuthenticatingClient {
@@ -169,28 +243,19 @@ func newClient(creds *identity.Credentials, auth_method identity.AuthMode, httpC
 		client_creds.URL = client_creds.URL + apiTokens
 	}
 	client := authenticatingClient{
-		creds:                       &client_creds,
-		requiredServiceTypes:        defaultRequiredServiceTypes,
-		client:                      client{logger: logger, httpClient: httpClient},
+		creds:                &client_creds,
+		requiredServiceTypes: defaultRequiredServiceTypes,
+		client: client{
+			logger:     logger,
+			httpClient: httpClient,
+		},
 		apiVersionDiscoveryDisabled: set.NewStrings(),
 	}
 	client.auth = &client
 	client.authMode = identity.NewAuthenticator(auth_method, httpClient)
 	return &client
 }
 
-func NewClient(creds *identity.Credentials, authMethod identity.AuthMode, logger logging.CompatLogger) AuthenticatingClient {
-	return newClient(creds, authMethod, sharedHttpClient, logger)
-}
-
-func NewNonValidatingClient(creds *identity.Credentials, authMethod identity.AuthMode, logger logging.CompatLogger) AuthenticatingClient {
-	return newClient(creds, authMethod, goosehttp.NewNonSSLValidating(), logger)
-}
-
-func NewClientTLSConfig(creds *identity.Credentials, authMethod identity.AuthMode, logger logging.CompatLogger, config *tls.Config) AuthenticatingClient {
-	return newClient(creds, authMethod, goosehttp.NewWithTLSConfig(config), logger)
-}
-
 func (c *client) sendRequest(method, url, token string, requestData *goosehttp.RequestData) (err error) {
 	if requestData.ReqValue != nil || requestData.RespValue != nil {
 		err = c.httpClient.JsonRequest(method, url, token, requestData, c.logger)

http/client.go

@@ -37,8 +37,33 @@ type HttpClient interface {
 	PostForm(url string, data url.Values) (resp *http.Response, err error)
 }
 
+// Option allows the adaptation of a http client given new options.
+// Both client.Client and http.Client have Options. To allow isolation between
+// layers, we have separate options. If client.Client and http.Client want
+// different options they can do so, without causing conflict.
+type Option func(*options)
+
+type options struct {
+	headersFunc HeadersFunc
+}
+
+// WithHeadersFunc allows passing in a new headers func for the http.Client
+// to execute for each request.
+func WithHeadersFunc(headersFunc HeadersFunc) Option {
+	return func(options *options) {
+		options.headersFunc = headersFunc
+	}
+}
+
+func newOptions() *options {
+	return &options{
+		headersFunc: DefaultHeaders,
+	}
+}
+
 type Client struct {
 	http.Client
+	headersFunc     HeadersFunc
 	maxSendAttempts int
 }
 
@@ -112,11 +137,26 @@ var insecureClient *http.Client
 var insecureClientMutex sync.Mutex
 
 // New returns a new goose http *Client using the default net/http client.
-func New() *Client {
-	return &Client{*http.DefaultClient, MaxSendAttempts}
+func New(options ...Option) *Client {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
+	return &Client{
+		Client:          *http.DefaultClient,
+		headersFunc:     opts.headersFunc,
+		maxSendAttempts: MaxSendAttempts,
+	}
 }
 
-func NewNonSSLValidating() *Client {
+// NewNonSSLValidating creates a new goose http *Client skipping SSL validation.
+func NewNonSSLValidating(options ...Option) *Client {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
 	insecureClientMutex.Lock()
 	httpClient := insecureClient
 	if httpClient == nil {
@@ -126,41 +166,38 @@ func NewNonSSLValidating() *Client {
 		httpClient = insecureClient
 	}
 	insecureClientMutex.Unlock()
-	return &Client{*httpClient, MaxSendAttempts}
+
+	return &Client{
+		Client:          *httpClient,
+		headersFunc:     opts.headersFunc,
+		maxSendAttempts: MaxSendAttempts,
+	}
 }
 
-func NewWithTLSConfig(tlsConfig *tls.Config) *Client {
+// NewWithTLSConfig creates a new goose http *Client with a TLS config.
+func NewWithTLSConfig(tlsConfig *tls.Config, options ...Option) *Client {
+	opts := newOptions()
+	for _, option := range options {
+		option(opts)
+	}
+
 	defaultClient := *http.DefaultClient
 	defaultClient.Transport = &http.Transport{
 		TLSClientConfig: tlsConfig,
 	}
-	return &Client{defaultClient, MaxSendAttempts}
+
+	return &Client{
+		Client:          defaultClient,
+		headersFunc:     opts.headersFunc,
+		maxSendAttempts: MaxSendAttempts,
+	}
 }
 
+// gooseAgent returns the current client goose agent version.
 func gooseAgent() string {
 	return fmt.Sprintf("goose (%s)", goose.Version)
 }
 
-func createHeaders(extraHeaders http.Header, contentType, authToken string, payloadExists bool) http.Header {
-	headers := make(http.Header)
-	if extraHeaders != nil {
-		for header, values := range extraHeaders {
-			for _, value := range values {
-				headers.Add(header, value)
-			}
-		}
-	}
-	if authToken != "" {
-		headers.Set("X-Auth-Token", authToken)
-	}
-	if payloadExists {
-		headers.Add("Content-Type", contentType)
-	}
-	headers.Add("Accept", contentType)
-	headers.Add("User-Agent", gooseAgent())
-	return headers
-}
-
 // JsonRequest JSON encodes and sends the object in reqData.ReqValue (if any) to the specified URL.
 // Optional method arguments are passed using the RequestData object.
 // Relevant RequestData fields:
@@ -186,7 +223,7 @@ func (c *Client) JsonRequest(method, url, token string, reqData *RequestData, lo
 		}
 		length = int64(len(data))
 	}
-	headers := createHeaders(reqData.ReqHeaders, contentTypeJSON, token, reqData.ReqValue != nil)
+	headers := c.headersFunc(method, reqData.ReqHeaders, contentTypeJSON, token, reqData.ReqValue != nil)
 	resp, err := c.sendRequest(
 		method,
 		url,
@@ -231,7 +268,7 @@ func (c *Client) BinaryRequest(method, url, token string, reqData *RequestData,
 	if reqData.Params != nil {
 		url += "?" + reqData.Params.Encode()
 	}
-	headers := createHeaders(reqData.ReqHeaders, contentTypeOctetStream, token, reqData.ReqLength != 0)
+	headers := c.headersFunc(method, reqData.ReqHeaders, contentTypeOctetStream, token, reqData.ReqLength != 0)
 	resp, err := c.sendRequest(
 		method,
 		url,

http/client_test.go

@@ -46,7 +46,7 @@ var _ = gc.Suite(&HTTPClientTestSuite{})
 
 func (s *HTTPClientTestSuite) assertHeaderValues(c *gc.C, token string) {
 	emptyHeaders := http.Header{}
-	headers := createHeaders(emptyHeaders, "content-type", token, true)
+	headers := DefaultHeaders("GET", emptyHeaders, "content-type", token, true)
 	contentTypes := []string{"content-type"}
 	headerData := map[string][]string{
 		"Content-Type": contentTypes, "Accept": contentTypes, "User-Agent": {gooseAgent()}}
@@ -71,7 +71,7 @@ func (s *HTTPClientTestSuite) TestCreateHeadersCopiesSupplied(c *gc.C) {
 	initialHeaders["Foo"] = []string{"Bar"}
 	contentType := contentTypeJSON
 	contentTypes := []string{contentType}
-	headers := createHeaders(initialHeaders, contentType, "", true)
+	headers := DefaultHeaders("GET", initialHeaders, contentType, "", true)
 	// it should not change the headers passed in
 	c.Assert(initialHeaders, gc.DeepEquals, http.Header{"Foo": []string{"Bar"}})
 	// The initial headers should be in the output

http/headers.go

@@ -0,0 +1,40 @@
+package http
+
+import "net/http"
+
+// HeadersFunc is type for aligning the creation of a series of client headers.
+type HeadersFunc = func(method string, headers http.Header, contentType, authToken string, hasPayload bool) http.Header
+
+// DefaultHeaders creates a set of http.Headers from the given arguments passed
+// in.
+// In this case it applies the headers passed in first, then sets the following:
+//  - X-Auth-Token
+//  - Content-Type
+//  - Accept
+//  - User-Agent
+//
+func DefaultHeaders(method string, extraHeaders http.Header, contentType, authToken string, payloadExists bool) http.Header {
+	headers := BasicHeaders()
+	if extraHeaders != nil {
+		for header, values := range extraHeaders {
+			for _, value := range values {
+				headers.Add(header, value)
+			}
+		}
+	}
+	if authToken != "" {
+		headers.Set("X-Auth-Token", authToken)
+	}
+	if payloadExists {
+		headers.Add("Content-Type", contentType)
+	}
+	headers.Add("Accept", contentType)
+	return headers
+}
+
+// BasicHeaders constructs basic http.Headers with expected default values.
+func BasicHeaders() http.Header {
+	headers := make(http.Header)
+	headers.Add("User-Agent", gooseAgent())
+	return headers
+}

identity/legacy.go

@@ -17,39 +17,45 @@ func (l *Legacy) Auth(creds *Credentials) (*AuthDetails, error) {
 	if l.client == nil {
 		l.client = goosehttp.New()
 	}
+
 	request, err := http.NewRequest("GET", creds.URL, nil)
 	if err != nil {
 		return nil, err
 	}
 	request.Header.Set("X-Auth-User", creds.User)
 	request.Header.Set("X-Auth-Key", creds.Secrets)
+
 	response, err := l.client.Do(request)
 	if err != nil {
 		return nil, err
 	}
 	defer response.Body.Close()
+
 	if response.StatusCode != http.StatusNoContent && response.StatusCode != http.StatusOK {
 		content, _ := ioutil.ReadAll(response.Body)
 		return nil, fmt.Errorf("Failed to Authenticate (code %d %s): %s",
 			response.StatusCode, response.Status, content)
 	}
+
 	details := &AuthDetails{}
 	details.Token = response.Header.Get("X-Auth-Token")
 	if details.Token == "" {
 		return nil, gooseerrors.NewUnauthorisedf(nil, "", "Did not get valid Token from auth request")
 	}
 	details.RegionServiceURLs = make(map[string]ServiceURLs)
+
 	serviceURLs := make(ServiceURLs)
+
 	// Legacy authentication doesn't require a region so use "".
 	details.RegionServiceURLs[""] = serviceURLs
-	nova_url := response.Header.Get("X-Server-Management-Url")
-	serviceURLs["compute"] = nova_url
+	novaURL := response.Header.Get("X-Server-Management-Url")
+	serviceURLs["compute"] = novaURL
 
-	swift_url := response.Header.Get("X-Storage-Url")
-	if swift_url == "" {
+	swiftURL := response.Header.Get("X-Storage-Url")
+	if swiftURL == "" {
 		return nil, fmt.Errorf("Did not get valid swift management URL from auth request")
 	}
-	serviceURLs["object-store"] = swift_url
+	serviceURLs["object-store"] = swiftURL
 
 	return details, nil
 }