launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08138
[Merge] lp:~jtv/maas/oauth-client-library into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/oauth-client-library into lp:maas with lp:~jtv/maas/apiclient-multipart as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/maas/oauth-client-library/+merge/107056
This continues work done by Julian on a MAAS HTTP API client. It's based on code extracted from Juju; eventually we'll want to move it into a separate library.
Yes, I know, it's a big chunk all at once. Not much can be done about it because of the form this was presented — a big chunk of code, with tests still to come.
The API does not model individual methods on the MAAS API. Instead, it just supports GET/POST/PUT/DELETE, with GET and POST supporting named methods (optional in the case of GET, which can also just retrieve a resource).
You may wonder why there are separate authentication and dispatch classes. In part this simply mirrors existing practice elsewhere (such as piston-mini-client) and in part it's to allow us to plug in alternatives. So far it doesn't look like we'll be doing that for the OAuth anytime soon, so the auth class may not carry its weight. The dispatcher exists to let the same code support both simple synchronous operation and Twisted-based asynchronous operation. It's just a matter of changing the dispatcher, and the return values from the HTTP methods will be Deferred objects. We decided to leave that out for now, since it's a testing pain and we don't use it, but if we're going to extract this client into a separate library then we'll want to add it.
To reduce the string interpolation we need to do to put together resource URLs, all the HTTP methods accept paths as either strings or sequences of elements. Each element in such a sequence gets represented as a string, so you can easily include numerical ids and such.
There's a lot more detail we could test, but Julian said not to go further on things like OAuth request signing; it's enough just to prove that the Authorization header is added.
Jeroen
--
https://code.launchpad.net/~jtv/maas/oauth-client-library/+merge/107056
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/oauth-client-library into lp:maas.
=== added file 'src/apiclient/maas_client.py'
--- src/apiclient/maas_client.py 1970-01-01 00:00:00 +0000
+++ src/apiclient/maas_client.py 2012-05-23 15:58:26 +0000
@@ -0,0 +1,186 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""MAAS OAuth API connection library."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ 'MAASClient',
+ 'MAASDispatcher',
+ ]
+
+from urllib import urlencode
+import urllib2
+from urlparse import (
+ urljoin,
+ urlparse,
+ )
+
+from apiclient.multipart import encode_multipart_data
+import oauth.oauth as oauth
+
+
+def _ascii_url(url):
+ """Encode `url` as ASCII if it isn't already."""
+ if isinstance(url, unicode):
+ urlparts = urlparse(url)
+ urlparts = urlparts._replace(
+ netloc=urlparts.netloc.encode("idna"))
+ url = urlparts.geturl()
+ return url.encode("ascii")
+
+
+class MAASOAuth:
+ """Helper class to OAuth-sign an HTTP request."""
+
+ def __init__(self, consumer_key, resource_token, resource_secret):
+ resource_tok_string = "oauth_token_secret=%s&oauth_token=%s" % (
+ resource_secret, resource_token)
+ self.resource_token = oauth.OAuthToken.from_string(resource_tok_string)
+ self.consumer_token = oauth.OAuthConsumer(consumer_key, "")
+
+ def sign_request(self, url, headers):
+ """Sign a request.
+
+ @param url: The URL to which the request is to be sent.
+ @param headers: The headers in the request. These will be updated
+ with the signature.
+ """
+ oauth_request = oauth.OAuthRequest.from_consumer_and_token(
+ self.consumer_token, token=self.resource_token, http_url=url)
+ oauth_request.sign_request(
+ oauth.OAuthSignatureMethod_PLAINTEXT(), self.consumer_token,
+ self.resource_token)
+ headers.update(oauth_request.to_header())
+
+
+class MAASDispatcher:
+ """Helper class to connect to a MAAS server using blocking requests.
+
+ Be careful when changing its API: this class is designed so that it
+ can be replaced with a Twisted-enabled alternative. See the MAAS
+ provider in Juju for the code this would require.
+ """
+
+ def dispatch_query(self, request_url, headers, method="GET", data=None):
+ """Synchronously dispatch an OAuth-signed request to L{request_url}.
+
+ :param request_url: The URL to which the request is to be sent.
+ :param headers: Headers to include in the request.
+ :type headers: A dict.
+ :param method: The HTTP method, e.g. C{GET}, C{POST}, etc.
+ An AssertionError is raised if trying to pass data for a GET.
+ :param data: The data to send, if any.
+ :type data: A byte string.
+
+ :return: A open file-like object that contains the response.
+ """
+ req = urllib2.Request(request_url, data, headers)
+ return urllib2.urlopen(req)
+
+
+class MAASClient:
+ """Base class for connecting to MAAS servers.
+
+ All "path" parameters can be either a string describing an absolute
+ resource path, or a sequence of items that, when represented as unicode,
+ make up the elements of the resource's path. So `['nodes', node_id]`
+ is equivalent to `"/nodes/%s" % node_id`.
+ """
+
+ def __init__(self, auth, dispatcher, base_url):
+ """Intialise the client.
+
+ :param auth: A `MAASOAuth` to sign requests.
+ :param dispatcher: An object implementing the MAASOAuthConnection
+ base class.
+ :param base_url: The base URL for the MAAS server, e.g.
+ http://my.maas.com:5240/
+ """
+ self.dispatcher = dispatcher
+ self.auth = auth
+ self.url = base_url
+
+ def _make_url(self, path):
+ """Compose an absolute URL to `path`.
+
+ :param path: Either a string giving a path to the desired resource,
+ or a sequence of items that make up the path.
+ :return: An absolute URL leading to `path`.
+ """
+ if not isinstance(path, basestring):
+ path = '/'.join(unicode(element) for element in path)
+ return urljoin(self.url, path)
+
+ def _formulate_get(self, path, params=None):
+ """Return URL and headers for a GET request.
+
+ This is similar to _formulate_change, except parameters are encoded
+ into the URL.
+
+ :param path: Path to the object to issue a GET on.
+ :param params: Optional dict of parameter values.
+ :return: A tuple: URL and headers for the request.
+ """
+ url = self._make_url(path)
+ if params is not None and len(params) > 0:
+ url += "?" + urlencode(params)
+ headers = {}
+ self.auth.sign_request(url, headers)
+ return url, headers
+
+ def _formulate_change(self, path, params):
+ """Return URL, headers, and body for a non-GET request.
+
+ This is similar to _formulate_get, except parameters are encoded as
+ a multipart form body.
+
+ :param path: Path to the object to issue a GET on.
+ :param params: A dict of parameter values.
+ :return: A tuple: URL, headers, and body for the request.
+ """
+ url = self._make_url(path)
+ body, headers = encode_multipart_data(params, {})
+ self.auth.sign_request(url, headers)
+ return url, headers, body
+
+ def get(self, path, op=None, **kwargs):
+ """Dispatch a GET.
+
+ :param op: Optional: named GET operation to invoke. If given, any
+ keyword arguments are passed to the named operation.
+ :return: The result of the dispatch_query call on the dispatcher.
+ """
+ if op is not None:
+ kwargs['op'] = op
+ url, headers = self._formulate_get(path, kwargs)
+ return self.dispatcher.dispatch_query(
+ url, method="GET", headers=headers)
+
+ def post(self, path, op, **kwargs):
+ """Dispatch POST method `op` on `path`, with the given parameters.
+
+ :return: The result of the dispatch_query call on the dispatcher.
+ """
+ kwargs['op'] = op
+ url, headers, body = self._formulate_change(path, kwargs)
+ return self.dispatcher.dispatch_query(
+ url, method="POST", headers=headers, data=body)
+
+ def put(self, path, **kwargs):
+ """Dispatch a PUT on the resource at `path`."""
+ url, headers, body = self._formulate_change(path, kwargs)
+ return self.dispatcher.dispatch_query(
+ url, method="PUT", headers=headers, data=body)
+
+ def delete(self, path):
+ """Dispatch a DELETE on the resource at `path`."""
+ url, headers, body = self._formulate_change(path, {})
+ return self.dispatcher.dispatch_query(
+ url, method="DELETE", headers=headers)
=== added file 'src/apiclient/tests/__init__.py'
=== added file 'src/apiclient/tests/test_maas_client.py'
--- src/apiclient/tests/test_maas_client.py 1970-01-01 00:00:00 +0000
+++ src/apiclient/tests/test_maas_client.py 2012-05-23 15:58:26 +0000
@@ -0,0 +1,236 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test MAAS HTTP API client."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+from email.parser import Parser
+from random import randint
+from urlparse import (
+ parse_qs,
+ urljoin,
+ urlparse,
+ )
+
+from apiclient.maas_client import (
+ _ascii_url,
+ MAASClient,
+ MAASDispatcher,
+ MAASOAuth,
+ )
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+
+
+class TestHelpers(TestCase):
+
+ def test_ascii_url_leaves_ascii_bytes_unchanged(self):
+ self.assertEqual(
+ b'http://example.com/', _ascii_url(b'http://example.com/'))
+ self.assertIsInstance(_ascii_url(b'http://example.com'), bytes)
+
+ def test_ascii_url_asciifies_unicode(self):
+ self.assertEqual(
+ b'http://example.com/', _ascii_url('http://example.com/'))
+ self.assertIsInstance(_ascii_url('http://example.com'), bytes)
+
+
+class TestMAASOAuth(TestCase):
+
+ def test_sign_request_adds_header(self):
+ headers = {}
+ auth = MAASOAuth('consumer_key', 'resource_token', 'resource_secret')
+ auth.sign_request('http://example.com/', headers)
+ self.assertIn('Authorization', headers)
+
+
+class TestMAASDispatcher(TestCase):
+
+ def test_dispatch_query_makes_direct_call(self):
+ filename = '/etc/hostname'
+ with open(filename) as f:
+ contents = f.read()
+ url = 'file://%s' % filename
+ self.assertEqual(
+ contents, MAASDispatcher().dispatch_query(url, {}).read())
+
+
+def make_url():
+ """Create an arbitrary URL."""
+ return 'http://example.com:%d/%s/' % (
+ randint(1, 65535),
+ factory.getRandomString(),
+ )
+
+
+def make_path():
+ """Create an arbitrary resource path."""
+ return '/'.join([factory.getRandomString() for counter in range(2)])
+
+
+class FakeDispatcher:
+ """Fake MAASDispatcher. Records last invocation, returns given result."""
+
+ last_call = None
+
+ def __init__(self, result=None):
+ self.result = result
+
+ def dispatch_query(self, request_url, headers, method=None, data=None):
+ self.last_call = {
+ 'request_url': request_url,
+ 'headers': headers,
+ 'method': method,
+ 'data': data,
+ }
+ return self.result
+
+
+def make_client(result=None):
+ """Create a MAASClient."""
+ root = make_url()
+ auth = MAASOAuth(
+ factory.getRandomString(), factory.getRandomString(),
+ factory.getRandomString())
+ return MAASClient(auth, FakeDispatcher(result=result), root)
+
+
+class TestMAASClient(TestCase):
+
+ def test_make_url_joins_root_and_path(self):
+ path = make_path()
+ client = make_client()
+ self.assertEqual(urljoin(client.url, path), client._make_url(path))
+
+ def test_make_url_converts_sequence_to_path(self):
+ path = [factory.getRandomString() for counter in range(3)]
+ client = make_client()
+ self.assertEqual(
+ urljoin(client.url, '/'.join(path)), client._make_url(path))
+
+ def test_make_url_represents_path_components_as_text(self):
+ number = randint(0, 100)
+ client = make_client()
+ self.assertEqual(
+ urljoin(client.url, unicode(number)), client._make_url([number]))
+
+ def test_formulate_get_makes_url(self):
+ path = make_path()
+ client = make_client()
+ url, headers = client._formulate_get(path)
+ self.assertEqual(urljoin(client.url, path), url)
+
+ def test_formulate_get_adds_parameters_to_url(self):
+ params = {
+ factory.getRandomString(): factory.getRandomString()
+ for counter in range(3)}
+ url, headers = make_client()._formulate_get(make_path(), params)
+ expectation = {key: [value] for key, value in params.items()}
+ self.assertEqual(expectation, parse_qs(urlparse(url).query))
+
+ def test_formulate_get_signs_request(self):
+ url, headers = make_client()._formulate_get(make_path())
+ self.assertIn('Authorization', headers)
+
+ def test_formulate_change_makes_url(self):
+ path = make_path()
+ client = make_client()
+ url, headers, body = client._formulate_change(path, {})
+ self.assertEqual(urljoin(client.url, path), url)
+
+ def test_formulate_change_signs_request(self):
+ url, headers, body = make_client()._formulate_change(make_path(), {})
+ self.assertIn('Authorization', headers)
+
+ def test_formulate_change_passes_parameters_in_body(self):
+ params = {factory.getRandomString(): factory.getRandomString()}
+ url, headers, body = make_client()._formulate_change(
+ make_path(), params)
+ body = Parser().parsestr(body).get_payload()
+ self.assertIn('name="%s"' % params.keys()[0], body)
+ self.assertIn('\r\n%s\r\n' % params.values()[0], body)
+
+ def test_get_dispatches_to_resource(self):
+ path = make_path()
+ client = make_client()
+ client.get(path)
+ request = client.dispatcher.last_call
+ self.assertEqual(client._make_url(path), request['request_url'])
+ self.assertIn('Authorization', request['headers'])
+ self.assertEqual('GET', request['method'])
+ self.assertIsNone(request['data'])
+
+ def test_get_without_op_gets_simple_resource(self):
+ expected_result = factory.getRandomString()
+ client = make_client(result=expected_result)
+ result = client.get(make_path())
+ self.assertEqual(expected_result, result)
+
+ def test_get_with_op_queries_resource(self):
+ path = make_path()
+ method = factory.getRandomString()
+ client = make_client()
+ client.get(path, method)
+ dispatch = client.dispatcher.last_call
+ self.assertEqual(
+ client._make_url(path) + '?op=%s' % method,
+ dispatch['request_url'])
+
+ def test_get_passes_parameters(self):
+ path = make_path()
+ param = factory.getRandomString()
+ method = factory.getRandomString()
+ client = make_client()
+ client.get(path, method, parameter=param)
+ request = client.dispatcher.last_call
+ self.assertIsNone(request['data'])
+ self.assertItemsEqual(
+ [param],
+ parse_qs(urlparse(request['request_url']).query)['parameter'])
+
+ def test_post_dispatches_to_resource(self):
+ path = make_path()
+ client = make_client()
+ client.post(path, factory.getRandomString())
+ request = client.dispatcher.last_call
+ self.assertEqual(client._make_url(path), request['request_url'])
+ self.assertIn('Authorization', request['headers'])
+ self.assertEqual('POST', request['method'])
+
+ def test_post_passes_parameters(self):
+ param = factory.getRandomString()
+ method = factory.getRandomString()
+ client = make_client()
+ client.post(make_path(), method, parameter=param)
+ request = client.dispatcher.last_call
+ body = Parser().parsestr(request['data']).get_payload()
+ self.assertIn('name="op"', body)
+ self.assertIn('\r\n%s\r\n' % method, body)
+ self.assertIn('name="parameter"', body)
+ self.assertIn('\r\n%s\r\n' % param, body)
+
+ def test_put_dispatches_to_resource(self):
+ path = make_path()
+ client = make_client()
+ client.put(path, parameter=factory.getRandomString())
+ request = client.dispatcher.last_call
+ self.assertEqual(client._make_url(path), request['request_url'])
+ self.assertIn('Authorization', request['headers'])
+ self.assertEqual('PUT', request['method'])
+
+ def test_delete_dispatches_to_resource(self):
+ path = make_path()
+ client = make_client()
+ client.delete(path)
+ request = client.dispatcher.last_call
+ self.assertEqual(client._make_url(path), request['request_url'])
+ self.assertIn('Authorization', request['headers'])
+ self.assertEqual('DELETE', request['method'])
