launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13978
[Merge] lp:~allenap/maas/api-uri-bug-1059645-for-1.2 into lp:maas/1.2
Gavin Panella has proposed merging lp:~allenap/maas/api-uri-bug-1059645-for-1.2 into lp:maas/1.2.
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.
Requested reviews:
Gavin Panella (allenap)
Related bugs:
Bug #1059645 in MAAS: "URI in API description wrong when accessing machine via alternative interface"
https://bugs.launchpad.net/maas/+bug/1059645
For more details, see:
https://code.launchpad.net/~allenap/maas/api-uri-bug-1059645-for-1.2/+merge/132668
--
https://code.launchpad.net/~allenap/maas/api-uri-bug-1059645-for-1.2/+merge/132668
Your team MAAS Maintainers is subscribed to branch lp:maas/1.2.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-10-30 15:15:30 +0000
+++ src/maasserver/api.py 2012-11-02 09:52:23 +0000
@@ -164,6 +164,7 @@
from maasserver.server_address import get_maas_facing_server_address
from maasserver.utils import (
absolute_reverse,
+ build_absolute_uri,
map_enum,
strip_domain,
)
@@ -967,6 +968,10 @@
get = operation(idempotent=True, exported_as='get')(get_file)
+ @classmethod
+ def resource_uri(cls, *args, **kwargs):
+ return ('files_handler', [])
+
class FilesHandler(OperationsHandler):
"""File management operations."""
@@ -1841,12 +1846,23 @@
Returns a JSON object describing the whole MAAS API.
"""
from maasserver import urls_api as urlconf
+ resources = [
+ describe_resource(resource)
+ for resource in find_api_resources(urlconf)
+ ]
+ # Make all URIs absolute. Clients - maas-cli in particular - expect that
+ # all handler URIs are absolute, not just paths. The handler URIs returned
+ # by describe_resource() are relative paths.
+ absolute = partial(build_absolute_uri, request)
+ for resource in resources:
+ for handler_type in "anon", "auth":
+ handler = resource[handler_type]
+ if handler is not None:
+ handler["uri"] = absolute(handler["path"])
+ # Package it all up.
description = {
"doc": "MAAS API",
- "resources": [
- describe_resource(resource)
- for resource in find_api_resources(urlconf)
- ],
+ "resources": resources,
}
# For backward compatibility, add "handlers" as an alias for all not-None
# anon and auth handlers in "resources".
=== modified file 'src/maasserver/apidoc.py'
--- src/maasserver/apidoc.py 2012-10-06 22:42:45 +0000
+++ src/maasserver/apidoc.py 2012-11-02 09:52:23 +0000
@@ -19,9 +19,7 @@
from inspect import getdoc
from itertools import izip_longest
-from urlparse import urljoin
-from django.conf import settings
from django.core.urlresolvers import (
get_resolver,
RegexURLPattern,
@@ -137,11 +135,8 @@
if isinstance(handler, BaseHandler):
handler = type(handler)
- uri_template = generate_doc(handler).resource_uri_template
- if uri_template is None:
- uri_template = settings.DEFAULT_MAAS_URL
- else:
- uri_template = urljoin(settings.DEFAULT_MAAS_URL, uri_template)
+ path = generate_doc(handler).resource_uri_template
+ path = "" if path is None else path
resource_uri = getattr(handler, "resource_uri", lambda: ())
view_name, uri_params, uri_kw = merge(resource_uri(), (None, (), {}))
@@ -154,7 +149,7 @@
"doc": getdoc(handler),
"name": handler.__name__,
"params": uri_params,
- "uri": uri_template,
+ "path": path,
}
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-10-30 15:15:30 +0000
+++ src/maasserver/tests/test_api.py 2012-11-02 09:52:23 +0000
@@ -27,10 +27,12 @@
import httplib
from itertools import izip
import json
+from operator import itemgetter
import os
import random
import shutil
import sys
+from urlparse import urlparse
from apiclient.maas_client import MAASClient
from celery.app import app_or_default
@@ -38,9 +40,11 @@
from django.contrib.auth.models import AnonymousUser
from django.core.urlresolvers import reverse
from django.http import QueryDict
+from django.test.client import RequestFactory
from fixtures import Fixture
from maasserver import api
from maasserver.api import (
+ describe,
DISPLAYED_NODEGROUP_FIELDS,
extract_constraints,
extract_oauth_key,
@@ -130,10 +134,15 @@
from provisioningserver.pxe import tftppath
from provisioningserver.testing.boot_images import make_boot_image_params
from testresources import FixtureResource
+from testscenarios import multiply_scenarios
from testtools.matchers import (
+ AfterPreprocessing,
AllMatch,
Contains,
Equals,
+ Is,
+ MatchesAll,
+ MatchesAny,
MatchesListwise,
MatchesStructure,
StartsWith,
@@ -4192,3 +4201,49 @@
self.assertSetEqual(
{"doc", "handlers", "resources"}, set(description))
self.assertIsInstance(description["handlers"], list)
+
+
+class TestDescribeAbsoluteURIs(AnonAPITestCase):
+ """Tests for the `describe` view's URI manipulation."""
+
+ scenarios_schemes = (
+ ("http", dict(scheme="http")),
+ ("https", dict(scheme="https")),
+ )
+
+ scenarios_paths = (
+ ("script-at-root", dict(script_name="", path_info="")),
+ ("script-below-root-1", dict(script_name="/foo/bar", path_info="")),
+ ("script-below-root-2", dict(script_name="/foo", path_info="/bar")),
+ )
+
+ scenarios = multiply_scenarios(
+ scenarios_schemes, scenarios_paths)
+
+ def test_handler_uris_are_absolute(self):
+ server = factory.make_name("server").lower()
+ extra = {
+ "PATH_INFO": self.path_info,
+ "SCRIPT_NAME": self.script_name,
+ "SERVER_NAME": server,
+ "wsgi.url_scheme": self.scheme,
+ }
+ request = RequestFactory().get(
+ "/%s/describe" % factory.make_name("path"), **extra)
+ response = describe(request)
+ self.assertEqual(httplib.OK, response.status_code, response.content)
+ description = json.loads(response.content)
+ expected_uri = AfterPreprocessing(
+ urlparse, MatchesStructure(
+ scheme=Equals(self.scheme), hostname=Equals(server),
+ # The path is always the script name followed by "api/"
+ # because all API calls are within the "api" tree.
+ path=StartsWith(self.script_name + "/api/")))
+ expected_handler = MatchesAny(
+ Is(None), AfterPreprocessing(itemgetter("uri"), expected_uri))
+ expected_resource = MatchesAll(
+ AfterPreprocessing(itemgetter("anon"), expected_handler),
+ AfterPreprocessing(itemgetter("auth"), expected_handler))
+ resources = description["resources"]
+ self.assertNotEqual([], resources)
+ self.assertThat(resources, AllMatch(expected_resource))
=== modified file 'src/maasserver/tests/test_apidoc.py'
--- src/maasserver/tests/test_apidoc.py 2012-10-08 10:23:16 +0000
+++ src/maasserver/tests/test_apidoc.py 2012-11-02 09:52:23 +0000
@@ -203,7 +203,7 @@
observed = describe_handler(ExampleHandler)
# The description contains several entries.
self.assertSetEqual(
- {"actions", "doc", "name", "params", "uri"},
+ {"actions", "doc", "name", "params", "path"},
set(observed))
self.assertEqual(ExampleHandler.__doc__, observed["doc"])
self.assertEqual(ExampleHandler.__name__, observed["name"])
@@ -231,11 +231,11 @@
}
self.assertSetEqual(expected_actions, observed_actions)
self.assertSetEqual({"system_id"}, set(description["params"]))
- # The URI is a URI Template <http://tools.ietf.org/html/rfc6570>, the
+ # The path is a URI Template <http://tools.ietf.org/html/rfc6570>, the
# components of which correspond to the parameters declared.
self.assertEqual(
- "http://example.com/api/1.0/nodes/{system_id}/";,
- description["uri"])
+ "/api/1.0/nodes/{system_id}/",
+ description["path"])
def test_describe_resource_anonymous_resource(self):
# When the resource does not require authentication, any configured
=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py 2012-10-30 15:15:30 +0000
+++ src/maasserver/utils/__init__.py 2012-11-02 09:52:23 +0000
@@ -12,6 +12,7 @@
__metaclass__ = type
__all__ = [
'absolute_reverse',
+ 'build_absolute_uri',
'get_db_state',
'ignore_unused',
'map_enum',
@@ -85,6 +86,23 @@
return url
+def build_absolute_uri(request, path):
+ """Given a full app-relative path, returns an absolute URI.
+
+ The path ordinarily starts with a forward-slash... imagine that you've
+ magically chroot'ed to your Django application; the path is the absolute
+ path to the view within that environment.
+
+ The URI returned uses the request to figure out how to make an absolute
+ URL. This means that the URI returned will use the same IP address or
+ alias that the request came in on.
+ """
+ script_name = request.META["SCRIPT_NAME"]
+ return "%s://%s%s%s" % (
+ "https" if request.is_secure() else "http",
+ request.get_host(), script_name, path)
+
+
def strip_domain(hostname):
"""Return `hostname` with the domain part removed."""
return hostname.split('.', 1)[0]
=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py 2012-10-30 15:15:30 +0000
+++ src/maasserver/utils/tests/test_utils.py 2012-11-02 09:52:23 +0000
@@ -16,11 +16,13 @@
from django.conf import settings
from django.core.urlresolvers import reverse
+from django.http import HttpRequest
from maasserver.enum import NODE_STATUS_CHOICES
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase as DjangoTestCase
from maasserver.utils import (
absolute_reverse,
+ build_absolute_uri,
get_db_state,
map_enum,
strip_domain,
@@ -107,6 +109,67 @@
self.assertEqual(status, get_db_state(node, 'status'))
+class TestBuildAbsoluteURI(TestCase):
+ """Tests for `build_absolute_uri`."""
+
+ def make_request(self, host="example.com", port=80, script_name="",
+ is_secure=False):
+ """Return a :class:`HttpRequest` with the given parameters."""
+ request = HttpRequest()
+ request.META["SERVER_NAME"] = host
+ request.META["SERVER_PORT"] = port
+ request.META["SCRIPT_NAME"] = script_name
+ request.is_secure = lambda: is_secure
+ return request
+
+ def test_simple(self):
+ request = self.make_request()
+ self.assertEqual(
+ "http://example.com/fred";,
+ build_absolute_uri(request, "/fred"))
+
+ def test_different_port(self):
+ request = self.make_request(port=1234)
+ self.assertEqual(
+ "http://example.com:1234/fred";,
+ build_absolute_uri(request, "/fred"))
+
+ def test_script_name_is_prefixed(self):
+ # The script name is always prefixed to the given path.
+ request = self.make_request(script_name="/foo/bar")
+ self.assertEqual(
+ "http://example.com/foo/bar/fred";,
+ build_absolute_uri(request, "/fred"))
+
+ def test_secure(self):
+ request = self.make_request(port=443, is_secure=True)
+ self.assertEqual(
+ "https://example.com/fred";,
+ build_absolute_uri(request, "/fred"))
+
+ def test_different_port_and_secure(self):
+ request = self.make_request(port=9443, is_secure=True)
+ self.assertEqual(
+ "https://example.com:9443/fred";,
+ build_absolute_uri(request, "/fred"))
+
+ def test_no_leading_forward_slash(self):
+ # No attempt is made to ensure that the given path is separated from
+ # the to-be-prefixed path.
+ request = self.make_request(script_name="/foo")
+ self.assertEqual(
+ "http://example.com/foobar";,
+ build_absolute_uri(request, "bar"))
+
+ def test_preserve_two_leading_slashes(self):
+ # Whilst this shouldn't ordinarily happen, two leading slashes in the
+ # path should be preserved, and not treated specially.
+ request = self.make_request(script_name="//foo")
+ self.assertEqual(
+ "http://example.com//foo/fred";,
+ build_absolute_uri(request, "/fred"))
+
+
class TestStripDomain(TestCase):
def test_strip_domain(self):
Follow ups
