Merge lp:~allenap/maas/api-uri-bug-1059645-for-1.2 into lp:maas/1.2

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 1281
Proposed branch: lp:~allenap/maas/api-uri-bug-1059645-for-1.2
Merge into: lp:maas/1.2
Diff against target: 332 lines (+163/-16)
6 files modified
src/maasserver/api.py (+20/-4)
src/maasserver/apidoc.py (+3/-8)
src/maasserver/tests/test_api.py (+55/-0)
src/maasserver/tests/test_apidoc.py (+4/-4)
src/maasserver/utils/__init__.py (+18/-0)
src/maasserver/utils/tests/test_utils.py (+63/-0)
To merge this branch: bzr merge lp:~allenap/maas/api-uri-bug-1059645-for-1.2
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+132668@code.launchpad.net

Commit message

Backport of lp:maas revision 1324:

In the API description, make handler URIs absolute with respect to the request.

Previously, DEFAULT_MAAS_URL was always used.

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api.py'
2--- src/maasserver/api.py 2012-10-30 15:15:30 +0000
3+++ src/maasserver/api.py 2012-11-02 09:52:23 +0000
4@@ -164,6 +164,7 @@
5 from maasserver.server_address import get_maas_facing_server_address
6 from maasserver.utils import (
7 absolute_reverse,
8+ build_absolute_uri,
9 map_enum,
10 strip_domain,
11 )
12@@ -967,6 +968,10 @@
13
14 get = operation(idempotent=True, exported_as='get')(get_file)
15
16+ @classmethod
17+ def resource_uri(cls, *args, **kwargs):
18+ return ('files_handler', [])
19+
20
21 class FilesHandler(OperationsHandler):
22 """File management operations."""
23@@ -1841,12 +1846,23 @@
24 Returns a JSON object describing the whole MAAS API.
25 """
26 from maasserver import urls_api as urlconf
27+ resources = [
28+ describe_resource(resource)
29+ for resource in find_api_resources(urlconf)
30+ ]
31+ # Make all URIs absolute. Clients - maas-cli in particular - expect that
32+ # all handler URIs are absolute, not just paths. The handler URIs returned
33+ # by describe_resource() are relative paths.
34+ absolute = partial(build_absolute_uri, request)
35+ for resource in resources:
36+ for handler_type in "anon", "auth":
37+ handler = resource[handler_type]
38+ if handler is not None:
39+ handler["uri"] = absolute(handler["path"])
40+ # Package it all up.
41 description = {
42 "doc": "MAAS API",
43- "resources": [
44- describe_resource(resource)
45- for resource in find_api_resources(urlconf)
46- ],
47+ "resources": resources,
48 }
49 # For backward compatibility, add "handlers" as an alias for all not-None
50 # anon and auth handlers in "resources".
51
52=== modified file 'src/maasserver/apidoc.py'
53--- src/maasserver/apidoc.py 2012-10-06 22:42:45 +0000
54+++ src/maasserver/apidoc.py 2012-11-02 09:52:23 +0000
55@@ -19,9 +19,7 @@
56
57 from inspect import getdoc
58 from itertools import izip_longest
59-from urlparse import urljoin
60
61-from django.conf import settings
62 from django.core.urlresolvers import (
63 get_resolver,
64 RegexURLPattern,
65@@ -137,11 +135,8 @@
66 if isinstance(handler, BaseHandler):
67 handler = type(handler)
68
69- uri_template = generate_doc(handler).resource_uri_template
70- if uri_template is None:
71- uri_template = settings.DEFAULT_MAAS_URL
72- else:
73- uri_template = urljoin(settings.DEFAULT_MAAS_URL, uri_template)
74+ path = generate_doc(handler).resource_uri_template
75+ path = "" if path is None else path
76
77 resource_uri = getattr(handler, "resource_uri", lambda: ())
78 view_name, uri_params, uri_kw = merge(resource_uri(), (None, (), {}))
79@@ -154,7 +149,7 @@
80 "doc": getdoc(handler),
81 "name": handler.__name__,
82 "params": uri_params,
83- "uri": uri_template,
84+ "path": path,
85 }
86
87
88
89=== modified file 'src/maasserver/tests/test_api.py'
90--- src/maasserver/tests/test_api.py 2012-10-30 15:15:30 +0000
91+++ src/maasserver/tests/test_api.py 2012-11-02 09:52:23 +0000
92@@ -27,10 +27,12 @@
93 import httplib
94 from itertools import izip
95 import json
96+from operator import itemgetter
97 import os
98 import random
99 import shutil
100 import sys
101+from urlparse import urlparse
102
103 from apiclient.maas_client import MAASClient
104 from celery.app import app_or_default
105@@ -38,9 +40,11 @@
106 from django.contrib.auth.models import AnonymousUser
107 from django.core.urlresolvers import reverse
108 from django.http import QueryDict
109+from django.test.client import RequestFactory
110 from fixtures import Fixture
111 from maasserver import api
112 from maasserver.api import (
113+ describe,
114 DISPLAYED_NODEGROUP_FIELDS,
115 extract_constraints,
116 extract_oauth_key,
117@@ -130,10 +134,15 @@
118 from provisioningserver.pxe import tftppath
119 from provisioningserver.testing.boot_images import make_boot_image_params
120 from testresources import FixtureResource
121+from testscenarios import multiply_scenarios
122 from testtools.matchers import (
123+ AfterPreprocessing,
124 AllMatch,
125 Contains,
126 Equals,
127+ Is,
128+ MatchesAll,
129+ MatchesAny,
130 MatchesListwise,
131 MatchesStructure,
132 StartsWith,
133@@ -4192,3 +4201,49 @@
134 self.assertSetEqual(
135 {"doc", "handlers", "resources"}, set(description))
136 self.assertIsInstance(description["handlers"], list)
137+
138+
139+class TestDescribeAbsoluteURIs(AnonAPITestCase):
140+ """Tests for the `describe` view's URI manipulation."""
141+
142+ scenarios_schemes = (
143+ ("http", dict(scheme="http")),
144+ ("https", dict(scheme="https")),
145+ )
146+
147+ scenarios_paths = (
148+ ("script-at-root", dict(script_name="", path_info="")),
149+ ("script-below-root-1", dict(script_name="/foo/bar", path_info="")),
150+ ("script-below-root-2", dict(script_name="/foo", path_info="/bar")),
151+ )
152+
153+ scenarios = multiply_scenarios(
154+ scenarios_schemes, scenarios_paths)
155+
156+ def test_handler_uris_are_absolute(self):
157+ server = factory.make_name("server").lower()
158+ extra = {
159+ "PATH_INFO": self.path_info,
160+ "SCRIPT_NAME": self.script_name,
161+ "SERVER_NAME": server,
162+ "wsgi.url_scheme": self.scheme,
163+ }
164+ request = RequestFactory().get(
165+ "/%s/describe" % factory.make_name("path"), **extra)
166+ response = describe(request)
167+ self.assertEqual(httplib.OK, response.status_code, response.content)
168+ description = json.loads(response.content)
169+ expected_uri = AfterPreprocessing(
170+ urlparse, MatchesStructure(
171+ scheme=Equals(self.scheme), hostname=Equals(server),
172+ # The path is always the script name followed by "api/"
173+ # because all API calls are within the "api" tree.
174+ path=StartsWith(self.script_name + "/api/")))
175+ expected_handler = MatchesAny(
176+ Is(None), AfterPreprocessing(itemgetter("uri"), expected_uri))
177+ expected_resource = MatchesAll(
178+ AfterPreprocessing(itemgetter("anon"), expected_handler),
179+ AfterPreprocessing(itemgetter("auth"), expected_handler))
180+ resources = description["resources"]
181+ self.assertNotEqual([], resources)
182+ self.assertThat(resources, AllMatch(expected_resource))
183
184=== modified file 'src/maasserver/tests/test_apidoc.py'
185--- src/maasserver/tests/test_apidoc.py 2012-10-08 10:23:16 +0000
186+++ src/maasserver/tests/test_apidoc.py 2012-11-02 09:52:23 +0000
187@@ -203,7 +203,7 @@
188 observed = describe_handler(ExampleHandler)
189 # The description contains several entries.
190 self.assertSetEqual(
191- {"actions", "doc", "name", "params", "uri"},
192+ {"actions", "doc", "name", "params", "path"},
193 set(observed))
194 self.assertEqual(ExampleHandler.__doc__, observed["doc"])
195 self.assertEqual(ExampleHandler.__name__, observed["name"])
196@@ -231,11 +231,11 @@
197 }
198 self.assertSetEqual(expected_actions, observed_actions)
199 self.assertSetEqual({"system_id"}, set(description["params"]))
200- # The URI is a URI Template <http://tools.ietf.org/html/rfc6570>, the
201+ # The path is a URI Template <http://tools.ietf.org/html/rfc6570>, the
202 # components of which correspond to the parameters declared.
203 self.assertEqual(
204- "http://example.com/api/1.0/nodes/{system_id}/",
205- description["uri"])
206+ "/api/1.0/nodes/{system_id}/",
207+ description["path"])
208
209 def test_describe_resource_anonymous_resource(self):
210 # When the resource does not require authentication, any configured
211
212=== modified file 'src/maasserver/utils/__init__.py'
213--- src/maasserver/utils/__init__.py 2012-10-30 15:15:30 +0000
214+++ src/maasserver/utils/__init__.py 2012-11-02 09:52:23 +0000
215@@ -12,6 +12,7 @@
216 __metaclass__ = type
217 __all__ = [
218 'absolute_reverse',
219+ 'build_absolute_uri',
220 'get_db_state',
221 'ignore_unused',
222 'map_enum',
223@@ -85,6 +86,23 @@
224 return url
225
226
227+def build_absolute_uri(request, path):
228+ """Given a full app-relative path, returns an absolute URI.
229+
230+ The path ordinarily starts with a forward-slash... imagine that you've
231+ magically chroot'ed to your Django application; the path is the absolute
232+ path to the view within that environment.
233+
234+ The URI returned uses the request to figure out how to make an absolute
235+ URL. This means that the URI returned will use the same IP address or
236+ alias that the request came in on.
237+ """
238+ script_name = request.META["SCRIPT_NAME"]
239+ return "%s://%s%s%s" % (
240+ "https" if request.is_secure() else "http",
241+ request.get_host(), script_name, path)
242+
243+
244 def strip_domain(hostname):
245 """Return `hostname` with the domain part removed."""
246 return hostname.split('.', 1)[0]
247
248=== modified file 'src/maasserver/utils/tests/test_utils.py'
249--- src/maasserver/utils/tests/test_utils.py 2012-10-30 15:15:30 +0000
250+++ src/maasserver/utils/tests/test_utils.py 2012-11-02 09:52:23 +0000
251@@ -16,11 +16,13 @@
252
253 from django.conf import settings
254 from django.core.urlresolvers import reverse
255+from django.http import HttpRequest
256 from maasserver.enum import NODE_STATUS_CHOICES
257 from maasserver.testing.factory import factory
258 from maasserver.testing.testcase import TestCase as DjangoTestCase
259 from maasserver.utils import (
260 absolute_reverse,
261+ build_absolute_uri,
262 get_db_state,
263 map_enum,
264 strip_domain,
265@@ -107,6 +109,67 @@
266 self.assertEqual(status, get_db_state(node, 'status'))
267
268
269+class TestBuildAbsoluteURI(TestCase):
270+ """Tests for `build_absolute_uri`."""
271+
272+ def make_request(self, host="example.com", port=80, script_name="",
273+ is_secure=False):
274+ """Return a :class:`HttpRequest` with the given parameters."""
275+ request = HttpRequest()
276+ request.META["SERVER_NAME"] = host
277+ request.META["SERVER_PORT"] = port
278+ request.META["SCRIPT_NAME"] = script_name
279+ request.is_secure = lambda: is_secure
280+ return request
281+
282+ def test_simple(self):
283+ request = self.make_request()
284+ self.assertEqual(
285+ "http://example.com/fred",
286+ build_absolute_uri(request, "/fred"))
287+
288+ def test_different_port(self):
289+ request = self.make_request(port=1234)
290+ self.assertEqual(
291+ "http://example.com:1234/fred",
292+ build_absolute_uri(request, "/fred"))
293+
294+ def test_script_name_is_prefixed(self):
295+ # The script name is always prefixed to the given path.
296+ request = self.make_request(script_name="/foo/bar")
297+ self.assertEqual(
298+ "http://example.com/foo/bar/fred",
299+ build_absolute_uri(request, "/fred"))
300+
301+ def test_secure(self):
302+ request = self.make_request(port=443, is_secure=True)
303+ self.assertEqual(
304+ "https://example.com/fred",
305+ build_absolute_uri(request, "/fred"))
306+
307+ def test_different_port_and_secure(self):
308+ request = self.make_request(port=9443, is_secure=True)
309+ self.assertEqual(
310+ "https://example.com:9443/fred",
311+ build_absolute_uri(request, "/fred"))
312+
313+ def test_no_leading_forward_slash(self):
314+ # No attempt is made to ensure that the given path is separated from
315+ # the to-be-prefixed path.
316+ request = self.make_request(script_name="/foo")
317+ self.assertEqual(
318+ "http://example.com/foobar",
319+ build_absolute_uri(request, "bar"))
320+
321+ def test_preserve_two_leading_slashes(self):
322+ # Whilst this shouldn't ordinarily happen, two leading slashes in the
323+ # path should be preserved, and not treated specially.
324+ request = self.make_request(script_name="//foo")
325+ self.assertEqual(
326+ "http://example.com//foo/fred",
327+ build_absolute_uri(request, "/fred"))
328+
329+
330 class TestStripDomain(TestCase):
331
332 def test_strip_domain(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: