From 4099ed85848eef0e878f76dd55ff5e1ec62b3f55 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Sat, 17 Mar 2018 12:10:17 -0700 Subject: [PATCH] Fix proxy extension for neutron RBAC Neutron is looking for 'tenant_id' in the response from the proxy plugin. This patch makes sure the 'project_id' returned from octavia is copied over to the 'tenant_id' in the response to neutron or assigns the one from the context. This patch also adapts Octavia's 403 Forbidden errors to neutron's 404 [1]. [1] https://docs.openstack.org/neutron/pike/contributor/internals/ \ policy.html#request-authorization Neutron is also returning a 404 instead of a status "DELETED". This adjusts that. Neutron/tempest will ask for the tenant_id field - replace this with project_id. Handle 403 Forbidden Fixes bug to not pass on empty string to Octavia Story: 2002602 Task: 22220 Change-Id: I840b5e2e8a95a4b3e166db9f9aa44590441aa7b6 --- .../services/loadbalancer/proxy_plugin.py | 59 +++++++++++++++++-- .../unit/db/loadbalancer/test_proxy_plugin.py | 53 +++++++++++++++-- 2 files changed, 103 insertions(+), 9 deletions(-) diff --git a/neutron_lbaas/services/loadbalancer/proxy_plugin.py b/neutron_lbaas/services/loadbalancer/proxy_plugin.py index ebd7606a8..c8b5674d0 100644 --- a/neutron_lbaas/services/loadbalancer/proxy_plugin.py +++ b/neutron_lbaas/services/loadbalancer/proxy_plugin.py @@ -144,6 +144,10 @@ class LoadBalancerProxyPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): e = lib_exc.NotAuthorized() e.msg = str(r.content) raise e + elif r.status_code == 403: + e = lib_exc.AdminRequired() + e.msg = str(r.content) + raise e elif r.status_code == 404: e = lib_exc.NotFound() e.msg = str(r.content) @@ -167,7 +171,7 @@ class LoadBalancerProxyPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): res = {} for k in map: if k not in keys: - if map[k]: + if map[k] or map[k] == '' or isinstance(map[k], bool): res[k] = map[k] if 'tenant_id' in res: res['project_id'] = res.pop('tenant_id') @@ -194,7 +198,13 @@ class LoadBalancerProxyPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): r = self._filter(FILTER, res[resource_]) r = self.post(self._path(resource, sub_resource, resource_id), context.auth_token, {resource_: r}) - return r[resource_] + response = r[resource_] + # neutron is looking for tenant_id in the response for RBAC + if 'project_id' in response: + response['tenant_id'] = response['project_id'] + else: + response['tenant_id'] = context.tenant_id + return response def _get_resources(self, resource, context, filters=None, fields=None, sub_resource=None, resource_id=None, @@ -207,22 +217,54 @@ class LoadBalancerProxyPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): filters['project_id'] = filters.pop('tenant_id') args['filters'] = filters if fields: + if 'tenant_id' in fields: + fields.remove('tenant_id') + if 'project_id' not in fields: + fields.append('project_id') args['fields'] = fields + + LOG.debug("context-tenant-id %s" % context.tenant_id) + res = self.get(self._path(resource, sub_resource, resource_id), context.auth_token, args) - return res[self.pluralize(resource_)] if not pass_through else res + response = res[self.pluralize(resource_)] if not pass_through else res + # neutron is looking for tenant_id in the response for RBAC + if isinstance(response, (list,)): + for e in response: + e['tenant_id'] = e.get('project_id', context.tenant_id) + else: + if 'project_id' in response: + response['tenant_id'] = response.get( + 'project_id', context.tenant_id) + return response def _get_resource(self, resource, context, id, fields=None, sub_resource=None, resource_id=None): # not sure how to test that or if we even support sorting/filtering? args = {} if fields: + if 'tenant_id' in fields: + fields.remove('tenant_id') + if 'project_id' not in fields: + fields.append('project_id') args['fields'] = fields resource_ = resource if not sub_resource else sub_resource + + LOG.debug("-get_resource context-tenant-id %s" % context.tenant_id) + res = self.get('{}/{}'.format( self._path(resource, sub_resource, resource_id), id), context.auth_token, args) - return res[resource_] + response = res[resource_] + if ('provisioning_status' in response) and ( + response['provisioning_status'] == 'DELETED'): + raise lib_exc.NotFound() + # neutron is looking for tenant_id in the response for RBAC + if 'project_id' in response: + response['tenant_id'] = response['project_id'] + else: + response['tenant_id'] = context.tenant_id + return response def _update_resource(self, resource, context, id, res, sub_resource=None, resource_id=None): @@ -233,7 +275,13 @@ class LoadBalancerProxyPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): resource, sub_resource, resource_id), id), context.auth_token, {resource_: r}) - return res[resource_] + response = res[resource_] + # neutron is looking for tenant_id in the response for RBAC + if 'project_id' in response: + response['tenant_id'] = response['project_id'] + else: + response['tenant_id'] = context.tenant_id + return response def _delete_resource(self, resource, context, id, sub_resource=None, resource_id=None): @@ -330,6 +378,7 @@ class LoadBalancerProxyPluginv2(loadbalancerv2.LoadBalancerPluginBaseV2): pass def statuses(self, context, loadbalancer_id): + LOG.debug("Statuses called!") return self._get_resources(LOADBALANCER, context, sub_resource=STATUS, resource_id=loadbalancer_id, pass_through=True) diff --git a/neutron_lbaas/tests/unit/db/loadbalancer/test_proxy_plugin.py b/neutron_lbaas/tests/unit/db/loadbalancer/test_proxy_plugin.py index 9fe668315..9e42b3f37 100644 --- a/neutron_lbaas/tests/unit/db/loadbalancer/test_proxy_plugin.py +++ b/neutron_lbaas/tests/unit/db/loadbalancer/test_proxy_plugin.py @@ -134,6 +134,7 @@ class LbaasLoadBalancerTests(TestLbaasProxyPluginDbTestCase): 'provisioning_status': n_constants.ACTIVE, 'operating_status': lb_const.ONLINE, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'listeners': [], 'pools': [], 'provider': 'lbaas' @@ -248,6 +249,25 @@ class LbaasLoadBalancerTests(TestLbaasProxyPluginDbTestCase): self.assertEqual(expected_values[k], body['loadbalancer'][k]) + @requests_mock.mock() + def test_show_loadbalancer_deleted(self, m): + name = 'lb_show' + description = 'lb_show description' + lb_id = "testid" + expected_values = {'name': name, + 'description': description, + 'vip_address': '10.0.0.10', + 'admin_state_up': True, + 'provisioning_status': 'DELETED', + 'operating_status': lb_const.ONLINE, + 'listeners': [], + 'provider': 'lbaas', + 'id': lb_id} + m.get("{}/{}".format(self.url, lb_id), + json={'loadbalancer': expected_values}) + resp, body = self._get_loadbalancer_api(lb_id) + self.assertEqual(404, resp.status_int) + @requests_mock.mock() def test_update_loadbalancer(self, m): loadbalancer_id = "test_uuid" @@ -349,6 +369,7 @@ class LbaasListenerTests(ListenerTestBase): 'protocol_port': 80, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'default_pool_id': None, 'loadbalancers': [{'id': self.lb_id}], 'id': '123' @@ -407,7 +428,6 @@ class LbaasListenerTests(ListenerTestBase): 'protocol': 'HTTP', 'connection_limit': 100, 'admin_state_up': False, - 'tenant_id': self._tenant_id, 'loadbalancers': [{'id': self.lb_id}]} listener_id = uuidutils.generate_uuid() @@ -449,13 +469,14 @@ class LbaasListenerTests(ListenerTestBase): 'connection_limit': -1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'default_pool_id': None, 'loadbalancers': [{'id': self.lb_id}]} listener_id = uuidutils.generate_uuid() m.get("{}/{}".format(self.url, listener_id), json={'listener': expected_values}) resp, body = self._get_listener_api(listener_id) - for k in expected_values: + for k in body['listener']: self.assertEqual(expected_values[k], body['listener'][k]) @requests_mock.mock() @@ -467,6 +488,7 @@ class LbaasListenerTests(ListenerTestBase): 'connection_limit': -1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'loadbalancers': [{'id': self.lb_id}]} listener_id = uuidutils.generate_uuid() @@ -499,6 +521,7 @@ class LbaasL7Tests(ListenerTestBase): 'redirect_pool_id': None, 'redirect_url': None, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, } expected.update(extras) listener_id = uuidutils.generate_uuid() @@ -519,6 +542,7 @@ class LbaasL7Tests(ListenerTestBase): 'redirect_pool_id': None, 'redirect_url': None, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, } expected.update(extras) listener_id = uuidutils.generate_uuid() @@ -546,6 +570,7 @@ class LbaasL7Tests(ListenerTestBase): 'redirect_pool_id': None, 'redirect_url': 'redirect_url', 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'position': 1, } expected.update(extras) @@ -589,6 +614,7 @@ class LbaasL7Tests(ListenerTestBase): 'redirect_pool_id': None, 'redirect_url': None, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'listener_id': listener_id, 'id': l7policy_id, } @@ -644,7 +670,9 @@ class LbaasL7Tests(ListenerTestBase): 'type': lb_const.L7_RULE_TYPE_HOST_NAME, 'compare_type': lb_const.L7_RULE_COMPARE_TYPE_EQUAL_TO, 'key': None, - 'value': 'value1' + 'value': 'value1', + 'project_id': self._tenant_id, + 'tenant_id': self._tenant_id, } m.post(self._rules(l7_policy_id), json={'rule': expected}) @@ -773,6 +801,7 @@ class LbaasPoolTests(PoolTestBase): 'lb_algorithm': 'ROUND_ROBIN', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'healthmonitor_id': None, 'members': [], 'id': uuidutils.generate_uuid() @@ -808,6 +837,7 @@ class LbaasPoolTests(PoolTestBase): 'lb_algorithm': 'ROUND_ROBIN', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'listeners': [{'id': self.listener_id}], 'healthmonitor_id': None, 'members': [] @@ -834,6 +864,7 @@ class LbaasPoolTests(PoolTestBase): 'lb_algorithm': 'LEAST_CONNECTIONS', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'listeners': [{'id': self.listener_id}], 'healthmonitor_id': None, 'members': [] @@ -883,7 +914,8 @@ class LbaasPoolTests(PoolTestBase): 'protocol': 'BLANK', 'lb_algorithm': 'LEAST_CONNECTIONS', 'admin_state_up': True, - 'tenant_id': self._tenant_id + 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, }} m.get(self.url, json={'pools': [data]}) m.post(self.url, status_code=webob.exc.HTTPBadRequest.code) @@ -899,6 +931,7 @@ class LbaasPoolTests(PoolTestBase): 'lb_algorithm': 'ROUND_ROBIN', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'session_persistence': {'cookie_name': None, 'type': 'HTTP_COOKIE'}, 'loadbalancers': [{'id': self.lb_id}], @@ -972,6 +1005,7 @@ class LbaasMemberTests(MemberTestBase): 'weight': 1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'subnet_id': '', 'name': 'member1', 'id': uuidutils.generate_uuid() @@ -1001,6 +1035,7 @@ class LbaasMemberTests(MemberTestBase): 'weight': 1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'subnet_id': '', 'name': 'member1', 'id': uuidutils.generate_uuid() @@ -1037,6 +1072,7 @@ class LbaasMemberTests(MemberTestBase): 'weight': 1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'subnet_id': '', 'name': 'member1', 'id': uuidutils.generate_uuid() @@ -1074,6 +1110,7 @@ class LbaasMemberTests(MemberTestBase): 'weight': 1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'subnet_id': self.test_subnet_id}} m.post(self._members(pool_id='WRONG_POOL_ID'), status_code=404) resp, body = self._create_member_api('WRONG_POOL_ID', data) @@ -1087,6 +1124,7 @@ class LbaasMemberTests(MemberTestBase): 'weight': 1, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'subnet_id': self.test_subnet_id, 'name': 123}} resp, body = self._create_member_api('POOL_ID', data) @@ -1149,6 +1187,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'expected_codes': '200', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pools': [{'id': self.pool_id}], 'name': 'monitor1', 'id': self.hm_id @@ -1182,6 +1221,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'expected_codes': '200', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pools': [{'id': self.pool_id}], 'name': 'monitor1' } @@ -1210,6 +1250,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'expected_codes': '200,404', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pools': [{'id': self.pool_id}], 'name': 'monitor2' } @@ -1249,6 +1290,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'max_retries': 2, 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pool_id': self.pool_id}} resp, body = self._create_healthmonitor_api(data) self.assertEqual(webob.exc.HTTPBadRequest.code, resp.status_int) @@ -1261,6 +1303,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'timeout': 1, 'max_retries': 1, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pool_id': uuidutils.generate_uuid()}} resp, body = self._create_healthmonitor_api(data) self.assertEqual(webob.exc.HTTPNotFound.code, resp.status_int) @@ -1273,6 +1316,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'timeout': 1, 'max_retries': 1, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pool_id': self.pool_id}} resp, body = self._create_healthmonitor_api(data) self.assertEqual(webob.exc.HTTPConflict.code, resp.status_int) @@ -1289,6 +1333,7 @@ class TestLbaasHealthMonitorTests(HealthMonitorTestBase): 'expected_codes': '200', 'admin_state_up': True, 'tenant_id': self._tenant_id, + 'project_id': self._tenant_id, 'pools': [{'id': self.pool_id}], 'name': '', 'max_retries_down': 3,