launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/gomaasapi-listnodes into lp:~maas-maintainers/maas/gomaasapi.

Commit message:
Implement server.ListNodes.

- Add an example file so that we can manually test the library against a live MAAS server.
- Create our own oauth.go (without tests!) (we will consider using the code in lp:usso when it will stabilize).

Requested reviews:
  Jeroen T. Vermeulen (jtv)

For more details, see:
https://code.launchpad.net/~rvb/maas/gomaasapi-listnodes/+merge/144654
-- 
https://code.launchpad.net/~rvb/maas/gomaasapi-listnodes/+merge/144654
Your team MAAS Maintainers is subscribed to branch lp:~maas-maintainers/maas/gomaasapi.

=== modified file 'client.go'
--- client.go	2013-01-23 10:14:43 +0000
+++ client.go	2013-01-24 09:29:23 +0000
@@ -4,90 +4,85 @@
 package gomaasapi
 
 import (
+	"errors"
 	"io/ioutil"
-	"log"
 	"net/http"
 	"net/url"
+	"strings"
 )
 
-type Client interface {
-	Get(URL string, parameters url.Values) (response []byte, err error)
-	Post(URL string, parameters url.Values) (response []byte, err error)
-	Put(URL string, parameters url.Values) (response []byte, err error)
-	Delete(URL string, parameters url.Values) error
+type Client struct {
+	Signer OAuthSigner
 }
 
-type genericClient struct{}
-
-func (client *genericClient) Get(URL string, parameters url.Values) ([]byte, error) {
-	// TODO: do a proper url.join here.
-	queryUrl := URL + parameters.Encode()
-	request, err := http.NewRequest("GET", queryUrl, nil)
-	if err != nil {
-		log.Println(err)
-		return nil, err
-	}
-	client.Sign(request)
+func (client Client) dispatchRequest(request *http.Request) ([]byte, error) {
+	client.Signer.OAuthSign(request)
 	httpClient := http.Client{}
-	response, reqErr := httpClient.Do(request)
-	if reqErr != nil {
-		log.Println(reqErr)
-		return nil, reqErr
-	}
-
-	body, parseErr := ioutil.ReadAll(response.Body)
-	if parseErr != nil {
-		log.Println(parseErr)
-		return nil, parseErr
+	response, err := httpClient.Do(request)
+	if err != nil {
+		return nil, err
+	}
+	body, err := ioutil.ReadAll(response.Body)
+	if err != nil {
+		return nil, err
+	}
+	if response.StatusCode/100 != 2 {
+		return body, errors.New("Error requesting the MAAS server: " + response.Status + ".")
 	}
 	return body, nil
 }
 
-func (client *genericClient) Post(URL string, parameters url.Values) ([]byte, error) {
-	// Not implemented.
-	return []byte{}, nil
-}
-func (client *genericClient) Put(URL string, parameters url.Values) ([]byte, error) {
-	// Not implemented.
-	return []byte{}, nil
-}
-func (client *genericClient) Delete(URL string, parameters url.Values) error {
-	// Not implemented.
-	return nil
-}
-
-// Trick to ensure *genericClient implements the Client interface.
-var _ Client = (*genericClient)(nil)
-
-// Sign does not do anything but is here to let children implement it.
-func (client *genericClient) Sign(request *http.Request) error {
-	return nil
-}
-
-// AnonymousClient implements a client which performs non-authenticated
-// requests.
-type AnonymousClient struct {
-	genericClient
-}
-
-// AuthenticatedClient implements a client which performs OAuth-authenticated
-// requests.
-type AuthenticatedClient struct {
-	genericClient
-	consumerKey    string
-	consumerSecret string
-	tokenKey       string
-	tokenSecret    string
-}
-
-func NewAuthenticatedClient(apiKey string) *AuthenticatedClient {
-	// Parse MAAS API key and create an OAuthClient.
-	// Not implemented.
-	return nil
-}
-
-func (client *AuthenticatedClient) Sign(request *http.Request) error {
-	// Sign the request with OAuth signature.
-	// Not implemented.
-	return nil
+func (client Client) Get(URL string, parameters url.Values) ([]byte, error) {
+	queryUrl := URL + "?" + parameters.Encode()
+	request, err := http.NewRequest("GET", queryUrl, nil)
+	if err != nil {
+		return nil, err
+	}
+	return client.dispatchRequest(request)
+}
+
+func (client Client) Post(URL string, parameters url.Values) ([]byte, error) {
+	// Not implemented.
+	return []byte{}, nil
+}
+func (client Client) Put(URL string, parameters url.Values) ([]byte, error) {
+	// Not implemented.
+	return []byte{}, nil
+}
+func (client Client) Delete(URL string, parameters url.Values) error {
+	// Not implemented.
+	return nil
+}
+
+type anonSigner struct{}
+
+func (signer anonSigner) OAuthSign(request *http.Request) error {
+	return nil
+}
+
+// Trick to ensure *anonSigner implements the OAuthSigner interface.
+var _ OAuthSigner = (*anonSigner)(nil)
+
+// NewAnonymousClient creates a client that issues anonymous requests.
+func NewAnonymousClient() (*Client, error) {
+	return &Client{Signer: &anonSigner{}}, nil
+}
+
+// NewAuthenticatedClient parses the given MAAS API key into the individual
+// OAuth tokens and creates an Client that will use these tokens to sign the
+// requests it issues.
+func NewAuthenticatedClient(apiKey string) (*Client, error) {
+	elements := strings.Split(apiKey, ":")
+	if len(elements) != 3 {
+		errString := "Invalid API key. The format of the key must be \"<consumer secret>:<token key>:<token secret>\"."
+		err := errors.New(errString)
+		return nil, err
+	}
+	// The consumer secret is the empty string in MAAS' authentication.
+	token := &OAuthToken{ConsumerKey: elements[0], ConsumerSecret: "", TokenKey: elements[1], TokenSecret: elements[2]}
+	signer, err := NewPLAINTEXTOAuthSigner(token, "MAAS API")
+	if err != nil {
+		return nil, err
+	}
+	return &Client{Signer: signer}, nil
 }

=== modified file 'client_test.go'
--- client_test.go	2013-01-22 17:00:40 +0000
+++ client_test.go	2013-01-24 09:29:23 +0000
@@ -5,8 +5,78 @@
 
 import (
 	. "launchpad.net/gocheck"
+	"net/http"
+	"net/url"
+	"strings"
 )
 
-func (suite *GomaasapiTestSuite) TestAnonymousClientGetPerformsGetRequest(c *C) {
-	// Not implemented.
+func (suite *GomaasapiTestSuite) TestClientdispatchRequestReturnsError(c *C) {
+	URI := "/some/url/?param1=test"
+	expectedResult := "expected:result"
+	client, _ := NewAnonymousClient()
+	server := newSingleServingServer(URI, expectedResult, http.StatusBadRequest)
+	defer server.Close()
+	request, err := http.NewRequest("GET", server.URL+URI, nil)
+
+	result, err := client.dispatchRequest(request)
+
+	c.Assert(err, ErrorMatches, "Error requesting the MAAS server: 400 Bad Request.*")
+	c.Assert(string(result), Equals, expectedResult)
+}
+
+func (suite *GomaasapiTestSuite) TestClientdispatchRequestSignsRequest(c *C) {
+	URI := "/some/url/?param1=test"
+	expectedResult := "expected:result"
+	client, _ := NewAuthenticatedClient("the:api:key")
+	server := newSingleServingServer(URI, expectedResult, http.StatusOK)
+	defer server.Close()
+	request, err := http.NewRequest("GET", server.URL+URI, nil)
+
+	result, err := client.dispatchRequest(request)
+
+	c.Assert(err, IsNil)
+	c.Assert(string(result), Equals, expectedResult)
+	c.Assert((*server.requestHeader)["Authorization"][0], Matches, "^OAuth .*")
+}
+
+func (suite *GomaasapiTestSuite) TestClientGetFormatsGetParameters(c *C) {
+	URI := "/some/url"
+	expectedResult := "expected:result"
+	client, _ := NewAnonymousClient()
+	params := url.Values{}
+	params.Add("op", "list")
+	fullURI := URI + "?op=list"
+	server := newSingleServingServer(fullURI, expectedResult, http.StatusOK)
+	defer server.Close()
+
+	result, err := client.Get(server.URL+URI, params)
+
+	c.Assert(err, IsNil)
+	c.Assert(string(result), Equals, expectedResult)
+}
+
+func (suite *GomaasapiTestSuite) TestNewAuthenticatedClientParsesApiKey(c *C) {
+	// NewAuthenticatedClient returns a pLAINTEXTOAuthSigner configured
+	// to use the given API key.
+	consumerKey := "consumerKey"
+	tokenKey := "tokenKey"
+	tokenSecret := "tokenSecret"
+	keyElements := []string{consumerKey, tokenKey, tokenSecret}
+	apiKey := strings.Join(keyElements, ":")
+
+	client, err := NewAuthenticatedClient(apiKey)
+
+	c.Assert(err, IsNil)
+	signer := client.Signer.(pLAINTEXTOAuthSigner)
+	c.Assert(signer.token.ConsumerKey, Equals, consumerKey)
+	c.Assert(signer.token.TokenKey, Equals, tokenKey)
+	c.Assert(signer.token.TokenSecret, Equals, tokenSecret)
+}
+
+func (suite *GomaasapiTestSuite) TestNewAuthenticatedClientFailsIfInvalidKey(c *C) {
+	client, err := NewAuthenticatedClient("invalid-key")
+
+	c.Assert(err, ErrorMatches, "Invalid API key.*")
+	c.Assert(client, IsNil)
+
 }

=== added directory 'example'
=== added file 'example/live_example.go'
--- example/live_example.go	1970-01-01 00:00:00 +0000
+++ example/live_example.go	2013-01-24 09:29:23 +0000
@@ -0,0 +1,28 @@
+package main
+
+import (
+	"fmt"
+	"launchpad.net/gomaasapi"
+)
+
+var apiKey string
+var apiURL string
+
+func init() {
+	fmt.Print("Enter apiKey: ")
+	fmt.Scanf("%s", &apiKey)
+	fmt.Print("Enter apiURL: ")
+	fmt.Scanf("%s", &apiURL)
+}
+
+func main() {
+	authClient, err := gomaasapi.NewAuthenticatedClient(apiKey)
+	if err != nil {
+		panic(err)
+	}
+
+	server := gomaasapi.Server{apiURL, authClient}
+	fmt.Println("Fetching list of nodes...")
+	listNodes, _ := server.ListNodes()
+	fmt.Printf("Got list of nodes: %s\n", listNodes)
+}

=== added file 'oauth.go'
--- oauth.go	1970-01-01 00:00:00 +0000
+++ oauth.go	2013-01-24 09:29:23 +0000
@@ -0,0 +1,75 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gomaasapi
+
+import (
+	"fmt"
+	"math/rand"
+	"net/http"
+	"net/url"
+	"strconv"
+	"strings"
+	"time"
+)
+
+func init() {
+	// Initialize the random generator.
+	rand.Seed(time.Now().UTC().UnixNano())
+}
+
+func generateNonce() string {
+	return strconv.Itoa(rand.Intn(100000000))
+}
+
+func generateTimestamp() string {
+	return strconv.Itoa(int(time.Now().Unix()))
+}
+
+type OAuthSigner interface {
+	OAuthSign(request *http.Request) error
+}
+
+type OAuthToken struct {
+	ConsumerKey    string
+	ConsumerSecret string
+	TokenKey       string
+	TokenSecret    string
+}
+
+// Trick to ensure *pLAINTEXTOAuthSigner implements the OAuthSigner interface.
+var _ OAuthSigner = (*pLAINTEXTOAuthSigner)(nil)
+
+type pLAINTEXTOAuthSigner struct {
+	token *OAuthToken
+	realm string
+}
+
+func NewPLAINTEXTOAuthSigner(token *OAuthToken, realm string) (OAuthSigner, error) {
+	return pLAINTEXTOAuthSigner{token, realm}, nil
+}
+
+// OAuthSignPLAINTEXT signs the provided request using the OAuth PLAINTEXT
+// method: http://oauth.net/core/1.0/#anchor22.
+func (signer pLAINTEXTOAuthSigner) OAuthSign(request *http.Request) error {
+
+	signature := signer.token.ConsumerSecret + `&` + signer.token.TokenSecret
+	authData := map[string]string{
+		"realm":                  signer.realm,
+		"oauth_consumer_key":     signer.token.ConsumerKey,
+		"oauth_token":            signer.token.TokenKey,
+		"oauth_signature_method": "PLAINTEXT",
+		"oauth_signature":        signature,
+		"oauth_timestamp":        generateTimestamp(),
+		"oauth_nonce":            generateNonce(),
+		"oauth_version":          "1.0",
+	}
+	// Build OAuth header.
+	authHeader := []string{}
+	for key, value := range authData {
+		authHeader = append(authHeader, fmt.Sprintf(`%s="%s"`, key, url.QueryEscape(value)))
+	}
+	strHeader := "OAuth " + strings.Join(authHeader, ", ")
+	request.Header.Add("Authorization", strHeader)
+	return nil
+}

=== modified file 'server.go'
--- server.go	2013-01-23 10:10:06 +0000
+++ server.go	2013-01-24 09:29:23 +0000
@@ -4,26 +4,25 @@
 package gomaasapi
 
 import (
-	"log"
+	"net/url"
 )
 
 type Server struct {
 	URL    string
-	client *Client
+	Client *Client
 }
 
-func (server *Server) listNodes() ([]*MAASObject, error) {
-	// Do something like (warning, completely untested code):
-	listURL := server.URL + "nodes/"
-	result, err := (*server.client).Get(listURL, nil)
+func (server *Server) ListNodes() ([]*MAASObject, error) {
+	listURL := server.URL + "/nodes/"
+	params := url.Values{}
+	params.Add("op", "list")
+	result, err := server.Client.Get(listURL, params)
 	if err != nil {
-		log.Println(err)
 		return nil, err
 	}
-	list, errJson := NewMAASObjectList(result)
-	if errJson != nil {
-		log.Println(errJson)
-		return nil, errJson
+	list, err := NewMAASObjectList(result)
+	if err != nil {
+		return nil, err
 	}
 	return list, nil
 }

=== added file 'server_test.go'
--- server_test.go	1970-01-01 00:00:00 +0000
+++ server_test.go	2013-01-24 09:29:23 +0000
@@ -0,0 +1,37 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gomaasapi
+
+import (
+	. "launchpad.net/gocheck"
+	"net/http"
+)
+
+func (suite *GomaasapiTestSuite) TestServerListNodesReturnsMAASObject(c *C) {
+	URL := "/nodes/?op=list"
+	expectedResult := "expected:result"
+	testServer := newSingleServingServer(URL, expectedResult, http.StatusOK)
+	defer testServer.Close()
+	client, _ := NewAnonymousClient()
+	server := Server{testServer.URL, client}
+
+	result, err := server.ListNodes()
+
+	c.Assert(err, IsNil)
+	// TODO: Really test result.
+	c.Assert(result, Not(IsNil))
+}
+
+func (suite *GomaasapiTestSuite) TestServerListNodesReturnsServerError(c *C) {
+	URL := "/nodes/?op=list"
+	expectedResult := "expected:result"
+	testServer := newSingleServingServer(URL, expectedResult, http.StatusBadRequest)
+	defer testServer.Close()
+	client, _ := NewAnonymousClient()
+	server := Server{testServer.URL, client}
+
+	_, err := server.ListNodes()
+
+	c.Assert(err, ErrorMatches, "Error requesting the MAAS server: 400 Bad Request.*")
+}

=== added file 'testing.go'
--- testing.go	1970-01-01 00:00:00 +0000
+++ testing.go	2013-01-24 09:29:23 +0000
@@ -0,0 +1,45 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gomaasapi
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+)
+
+type singleServingServer struct {
+	*httptest.Server
+	requestContent *string
+	requestHeader  *http.Header
+}
+
+// newSingleServingServer create a single-serving test http server which will
+// return only one response as defined by the passed arguments.
+func newSingleServingServer(uri string, response string, code int) *singleServingServer {
+	var requestContent string
+	var requestHeader http.Header
+	var requested bool
+	handler := func(writer http.ResponseWriter, request *http.Request) {
+		if requested {
+			http.Error(writer, "Already requested", http.StatusServiceUnavailable)
+		}
+		res, err := ioutil.ReadAll(request.Body)
+		if err != nil {
+			panic(err)
+		}
+		requestContent = string(res)
+		requestHeader = request.Header
+		if request.URL.String() != uri {
+			http.Error(writer, "404 page not found", http.StatusNotFound)
+		} else {
+			writer.WriteHeader(code)
+			fmt.Fprint(writer, response)
+		}
+		requested = true
+	}
+	server := httptest.NewServer(http.HandlerFunc(handler))
+	return &singleServingServer{server, &requestContent, &requestHeader}
+}