From eec96136be1e8d87d2a07c7d4306de52cbc3e7c8 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 15 Sep 2015 13:58:30 +0100 Subject: [PATCH] Add config option to override url for links The versions url returns the wrong data when Ironic API is behind a proxy. This adds a new config option called "public_endpoint" so it can be set properly. Closes-Bug: #1384379 Change-Id: I6d1b59db3ce09aba7bca5a71edcf97eb79f0b17b --- etc/ironic/ironic.conf.sample | 8 ++++++++ ironic/api/__init__.py | 8 ++++++++ ironic/api/app.py | 3 ++- ironic/api/controllers/link.py | 2 +- ironic/api/controllers/root.py | 2 +- ironic/api/controllers/v1/__init__.py | 18 ++++++++--------- ironic/api/controllers/v1/chassis.py | 2 +- ironic/api/controllers/v1/collection.py | 2 +- ironic/api/controllers/v1/driver.py | 4 ++-- ironic/api/controllers/v1/node.py | 2 +- ironic/api/controllers/v1/port.py | 2 +- ironic/api/hooks.py | 13 +++++++++++++ ironic/tests/api/test_hooks.py | 20 +++++++++++++++++++ ironic/tests/api/v1/test_chassis.py | 17 +++++++++++++++- ironic/tests/api/v1/test_drivers.py | 26 +++++++++++++++++++++++++ ironic/tests/api/v1/test_nodes.py | 17 +++++++++++++++- ironic/tests/api/v1/test_ports.py | 17 +++++++++++++++- 17 files changed, 142 insertions(+), 21 deletions(-) diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index 122fd48db7..9124ed7c8e 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -447,6 +447,14 @@ # from a collection resource. (integer value) #max_limit=1000 +# Public URL to use when building the links to the API +# resources (for example, "https://ironic.rocks:6384"). If +# None the links will be built using the request's host URL. +# If the API is operating behind a proxy, you will want to +# change this to represent the proxy's URL. Defaults to None. +# (string value) +#public_endpoint= + [cisco_ucs] diff --git a/ironic/api/__init__.py b/ironic/api/__init__.py index ff738fce79..8349ed7ba8 100644 --- a/ironic/api/__init__.py +++ b/ironic/api/__init__.py @@ -29,6 +29,14 @@ API_SERVICE_OPTS = [ default=1000, help=_('The maximum number of items returned in a single ' 'response from a collection resource.')), + cfg.StrOpt('public_endpoint', + default=None, + help=_("Public URL to use when building the links to the API " + "resources (for example, \"https://ironic.rocks:6384\")." + " If None the links will be built using the request's " + "host URL. If the API is operating behind a proxy, you " + "will want to change this to represent the proxy's URL. " + "Defaults to None.")), ] CONF = cfg.CONF diff --git a/ironic/api/app.py b/ironic/api/app.py index a9e57ba362..93220447b9 100644 --- a/ironic/api/app.py +++ b/ironic/api/app.py @@ -54,7 +54,8 @@ def setup_app(pecan_config=None, extra_hooks=None): hooks.DBHook(), hooks.ContextHook(pecan_config.app.acl_public_routes), hooks.RPCHook(), - hooks.NoExceptionTracebackHook()] + hooks.NoExceptionTracebackHook(), + hooks.PublicUrlHook()] if extra_hooks: app_hooks.extend(extra_hooks) diff --git a/ironic/api/controllers/link.py b/ironic/api/controllers/link.py index 8f9b891473..9d3a9096e7 100644 --- a/ironic/api/controllers/link.py +++ b/ironic/api/controllers/link.py @@ -21,7 +21,7 @@ from ironic.api.controllers import base def build_url(resource, resource_args, bookmark=False, base_url=None): if base_url is None: - base_url = pecan.request.host_url + base_url = pecan.request.public_url template = '%(url)s/%(res)s' if bookmark else '%(url)s/v1/%(res)s' # FIXME(lucasagomes): I'm getting a 404 when doing a GET on diff --git a/ironic/api/controllers/root.py b/ironic/api/controllers/root.py index 70fb594ca7..5485fd3c8c 100644 --- a/ironic/api/controllers/root.py +++ b/ironic/api/controllers/root.py @@ -37,7 +37,7 @@ class Version(base.APIBase): def convert(id): version = Version() version.id = id - version.links = [link.Link.make_link('self', pecan.request.host_url, + version.links = [link.Link.make_link('self', pecan.request.public_url, id, '', bookmark=True)] return version diff --git a/ironic/api/controllers/v1/__init__.py b/ironic/api/controllers/v1/__init__.py index 49ea101a83..a108e49e22 100644 --- a/ironic/api/controllers/v1/__init__.py +++ b/ironic/api/controllers/v1/__init__.py @@ -86,7 +86,7 @@ class V1(base.APIBase): def convert(): v1 = V1() v1.id = "v1" - v1.links = [link.Link.make_link('self', pecan.request.host_url, + v1.links = [link.Link.make_link('self', pecan.request.public_url, 'v1', '', bookmark=True), link.Link.make_link('describedby', 'http://docs.openstack.org', @@ -96,31 +96,31 @@ class V1(base.APIBase): ] v1.media_types = [MediaType('application/json', 'application/vnd.openstack.ironic.v1+json')] - v1.chassis = [link.Link.make_link('self', pecan.request.host_url, + v1.chassis = [link.Link.make_link('self', pecan.request.public_url, 'chassis', ''), link.Link.make_link('bookmark', - pecan.request.host_url, + pecan.request.public_url, 'chassis', '', bookmark=True) ] - v1.nodes = [link.Link.make_link('self', pecan.request.host_url, + v1.nodes = [link.Link.make_link('self', pecan.request.public_url, 'nodes', ''), link.Link.make_link('bookmark', - pecan.request.host_url, + pecan.request.public_url, 'nodes', '', bookmark=True) ] - v1.ports = [link.Link.make_link('self', pecan.request.host_url, + v1.ports = [link.Link.make_link('self', pecan.request.public_url, 'ports', ''), link.Link.make_link('bookmark', - pecan.request.host_url, + pecan.request.public_url, 'ports', '', bookmark=True) ] - v1.drivers = [link.Link.make_link('self', pecan.request.host_url, + v1.drivers = [link.Link.make_link('self', pecan.request.public_url, 'drivers', ''), link.Link.make_link('bookmark', - pecan.request.host_url, + pecan.request.public_url, 'drivers', '', bookmark=True) ] diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index 93ea09c681..89e7933ca7 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -108,7 +108,7 @@ class Chassis(base.APIBase): if fields is not None: api_utils.check_for_invalid_fields(fields, chassis.as_dict()) - return cls._convert_with_links(chassis, pecan.request.host_url, + return cls._convert_with_links(chassis, pecan.request.public_url, fields) @classmethod diff --git a/ironic/api/controllers/v1/collection.py b/ironic/api/controllers/v1/collection.py index 4ddadb0154..9f82bd1bfe 100644 --- a/ironic/api/controllers/v1/collection.py +++ b/ironic/api/controllers/v1/collection.py @@ -44,5 +44,5 @@ class Collection(base.APIBase): 'args': q_args, 'limit': limit, 'marker': self.collection[-1].uuid} - return link.Link.make_link('next', pecan.request.host_url, + return link.Link.make_link('next', pecan.request.public_url, resource_url, next_args).href diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index d28376db90..74299028ba 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -77,10 +77,10 @@ class Driver(base.APIBase): driver.hosts = hosts driver.links = [ link.Link.make_link('self', - pecan.request.host_url, + pecan.request.public_url, 'drivers', name), link.Link.make_link('bookmark', - pecan.request.host_url, + pecan.request.public_url, 'drivers', name, bookmark=True) ] diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 9cc9e8a84c..aa826c979c 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -684,7 +684,7 @@ class Node(base.APIBase): assert_juno_provision_state_name(node) hide_fields_in_newer_versions(node) show_password = pecan.request.context.show_password - return cls._convert_with_links(node, pecan.request.host_url, + return cls._convert_with_links(node, pecan.request.public_url, fields=fields, show_password=show_password) diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 6e7db2a5ff..16b9ab5025 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -137,7 +137,7 @@ class Port(base.APIBase): if fields is not None: api_utils.check_for_invalid_fields(fields, port.as_dict()) - return cls._convert_with_links(port, pecan.request.host_url, + return cls._convert_with_links(port, pecan.request.public_url, fields=fields) @classmethod diff --git a/ironic/api/hooks.py b/ironic/api/hooks.py index 69adad359d..73d50466ba 100644 --- a/ironic/api/hooks.py +++ b/ironic/api/hooks.py @@ -153,3 +153,16 @@ class NoExceptionTracebackHook(hooks.PecanHook): # Replace the whole json. Cannot change original one beacause it's # generated on the fly. state.response.json = json_body + + +class PublicUrlHook(hooks.PecanHook): + """Attach the right public_url to the request. + + Attach the right public_url to the request so resources can create + links even when the API service is behind a proxy or SSL terminator. + + """ + + def before(self, state): + state.request.public_url = (cfg.CONF.api.public_endpoint or + state.request.host_url) diff --git a/ironic/tests/api/test_hooks.py b/ironic/tests/api/test_hooks.py index 6233d93ed2..e1b3e5723e 100644 --- a/ironic/tests/api/test_hooks.py +++ b/ironic/tests/api/test_hooks.py @@ -36,6 +36,7 @@ class FakeRequest(object): self.context = context self.environ = environ or {} self.version = (1, 0) + self.host_url = 'http://127.0.0.1:6385' class FakeRequestState(object): @@ -281,3 +282,22 @@ class TestTrustedCallHookCompatJuno(TestTrustedCallHook): def test_trusted_call_hook_public_api(self): self.skipTest('no public_api trusted call policy in juno') + + +class TestPublicUrlHook(base.FunctionalTest): + + def test_before_host_url(self): + headers = fake_headers() + reqstate = FakeRequestState(headers=headers) + trusted_call_hook = hooks.PublicUrlHook() + trusted_call_hook.before(reqstate) + self.assertEqual(reqstate.request.host_url, + reqstate.request.public_url) + + def test_before_public_endpoint(self): + cfg.CONF.set_override('public_endpoint', 'http://foo', 'api') + headers = fake_headers() + reqstate = FakeRequestState(headers=headers) + trusted_call_hook = hooks.PublicUrlHook() + trusted_call_hook.before(reqstate) + self.assertEqual('http://foo', reqstate.request.public_url) diff --git a/ironic/tests/api/v1/test_chassis.py b/ironic/tests/api/v1/test_chassis.py index 7cb9a590a8..a4d1772778 100644 --- a/ironic/tests/api/v1/test_chassis.py +++ b/ironic/tests/api/v1/test_chassis.py @@ -132,7 +132,8 @@ class TestListChassis(test_api_base.FunctionalTest): uuids = [n['uuid'] for n in data['chassis']] six.assertCountEqual(self, ch_list, uuids) - def test_links(self): + def _test_links(self, public_url=None): + cfg.CONF.set_override('public_endpoint', public_url, 'api') uuid = uuidutils.generate_uuid() obj_utils.create_test_chassis(self.context, uuid=uuid) data = self.get_json('/chassis/%s' % uuid) @@ -143,6 +144,20 @@ class TestListChassis(test_api_base.FunctionalTest): bookmark = l['rel'] == 'bookmark' self.assertTrue(self.validate_link(l['href'], bookmark=bookmark)) + if public_url is not None: + expected = [{'href': '%s/v1/chassis/%s' % (public_url, uuid), + 'rel': 'self'}, + {'href': '%s/chassis/%s' % (public_url, uuid), + 'rel': 'bookmark'}] + for i in expected: + self.assertIn(i, data['links']) + + def test_links(self): + self._test_links() + + def test_links_public_url(self): + self._test_links(public_url='http://foo') + def test_collection_links(self): for id in range(5): obj_utils.create_test_chassis(self.context, diff --git a/ironic/tests/api/v1/test_drivers.py b/ironic/tests/api/v1/test_drivers.py index 1c47feb827..989c6a71a5 100644 --- a/ironic/tests/api/v1/test_drivers.py +++ b/ironic/tests/api/v1/test_drivers.py @@ -16,6 +16,7 @@ import json import mock +from oslo_config import cfg from six.moves import http_client from testtools.matchers import HasLength @@ -75,6 +76,31 @@ class TestListDrivers(base.FunctionalTest): response = self.get_json('/drivers/%s' % self.d1, expect_errors=True) self.assertEqual(http_client.NOT_FOUND, response.status_int) + def _test_links(self, public_url=None): + cfg.CONF.set_override('public_endpoint', public_url, 'api') + self.register_fake_conductors() + data = self.get_json('/drivers/%s' % self.d1) + self.assertIn('links', data.keys()) + self.assertEqual(2, len(data['links'])) + self.assertIn(self.d1, data['links'][0]['href']) + for l in data['links']: + bookmark = l['rel'] == 'bookmark' + self.assertTrue(self.validate_link(l['href'], bookmark=bookmark)) + + if public_url is not None: + expected = [{'href': '%s/v1/drivers/%s' % (public_url, self.d1), + 'rel': 'self'}, + {'href': '%s/drivers/%s' % (public_url, self.d1), + 'rel': 'bookmark'}] + for i in expected: + self.assertIn(i, data['links']) + + def test_links(self): + self._test_links() + + def test_links_public_url(self): + self._test_links(public_url='http://foo') + @mock.patch.object(rpcapi.ConductorAPI, 'driver_vendor_passthru') def test_driver_vendor_passthru_sync(self, mocked_driver_vendor_passthru): self.register_fake_conductors() diff --git a/ironic/tests/api/v1/test_nodes.py b/ironic/tests/api/v1/test_nodes.py index c6e3aab219..f99e807fda 100644 --- a/ironic/tests/api/v1/test_nodes.py +++ b/ironic/tests/api/v1/test_nodes.py @@ -322,7 +322,8 @@ class TestListNodes(test_api_base.FunctionalTest): self.assertEqual(len(nodes), len(data['nodes'])) self.assertEqual(sorted(node_names), sorted(names)) - def test_links(self): + def _test_links(self, public_url=None): + cfg.CONF.set_override('public_endpoint', public_url, 'api') uuid = uuidutils.generate_uuid() obj_utils.create_test_node(self.context, uuid=uuid) data = self.get_json('/nodes/%s' % uuid) @@ -333,6 +334,20 @@ class TestListNodes(test_api_base.FunctionalTest): bookmark = l['rel'] == 'bookmark' self.assertTrue(self.validate_link(l['href'], bookmark=bookmark)) + if public_url is not None: + expected = [{'href': '%s/v1/nodes/%s' % (public_url, uuid), + 'rel': 'self'}, + {'href': '%s/nodes/%s' % (public_url, uuid), + 'rel': 'bookmark'}] + for i in expected: + self.assertIn(i, data['links']) + + def test_links(self): + self._test_links() + + def test_links_public_url(self): + self._test_links(public_url='http://foo') + def test_collection_links(self): nodes = [] for id in range(5): diff --git a/ironic/tests/api/v1/test_ports.py b/ironic/tests/api/v1/test_ports.py index 712a2ac8d7..bff8be3b06 100644 --- a/ironic/tests/api/v1/test_ports.py +++ b/ironic/tests/api/v1/test_ports.py @@ -161,7 +161,8 @@ class TestListPorts(test_api_base.FunctionalTest): uuids = [n['uuid'] for n in data['ports']] six.assertCountEqual(self, ports, uuids) - def test_links(self): + def _test_links(self, public_url=None): + cfg.CONF.set_override('public_endpoint', public_url, 'api') uuid = uuidutils.generate_uuid() obj_utils.create_test_port(self.context, uuid=uuid, @@ -174,6 +175,20 @@ class TestListPorts(test_api_base.FunctionalTest): bookmark = l['rel'] == 'bookmark' self.assertTrue(self.validate_link(l['href'], bookmark=bookmark)) + if public_url is not None: + expected = [{'href': '%s/v1/ports/%s' % (public_url, uuid), + 'rel': 'self'}, + {'href': '%s/ports/%s' % (public_url, uuid), + 'rel': 'bookmark'}] + for i in expected: + self.assertIn(i, data['links']) + + def test_links(self): + self._test_links() + + def test_links_public_url(self): + self._test_links(public_url='http://foo') + def test_collection_links(self): ports = [] for id_ in range(5):