From 8b6ac08b0816c96ba7f2a40fb85413f58ae8faf7 Mon Sep 17 00:00:00 2001 From: Sulochan Acharya Date: Thu, 3 Jul 2014 21:03:53 +0000 Subject: [PATCH] Adds more policy control to cells ext Allow create,delete,update and sync_instances to have fine grained policy control. UpgradeImpact DocImpact Change-Id: I2e32ae2f37d3b599585d25535c79eecf6485b462 Closes-Bug: 1335901 --- etc/nova/policy.json | 8 ++ nova/api/openstack/compute/contrib/cells.py | 12 ++ .../api/openstack/compute/plugins/v3/cells.py | 12 ++ .../openstack/compute/contrib/test_cells.py | 105 ++++++++++++++++- .../compute/plugins/v3/test_cells.py | 107 +++++++++++++++++- nova/tests/fake_policy.py | 8 ++ 6 files changed, 243 insertions(+), 9 deletions(-) diff --git a/etc/nova/policy.json b/etc/nova/policy.json index cc5b8ea4a83d..6444f5ab3cfe 100644 --- a/etc/nova/policy.json +++ b/etc/nova/policy.json @@ -66,7 +66,15 @@ "compute_extension:v3:os-attach-interfaces:discoverable": "", "compute_extension:baremetal_nodes": "rule:admin_api", "compute_extension:cells": "rule:admin_api", + "compute_extension:cells:create": "rule:admin_api", + "compute_extension:cells:delete": "rule:admin_api", + "compute_extension:cells:update": "rule:admin_api", + "compute_extension:cells:sync_instances": "rule:admin_api", "compute_extension:v3:os-cells": "rule:admin_api", + "compute_extension:v3:os-cells:create": "rule:admin_api", + "compute_extension:v3:os-cells:delete": "rule:admin_api", + "compute_extension:v3:os-cells:update": "rule:admin_api", + "compute_extension:v3:os-cells:sync_instances": "rule:admin_api", "compute_extension:v3:os-cells:discoverable": "", "compute_extension:certificates": "", "compute_extension:v3:os-certificates:create": "", diff --git a/nova/api/openstack/compute/contrib/cells.py b/nova/api/openstack/compute/contrib/cells.py index 3937c24338fa..e2595021bacf 100644 --- a/nova/api/openstack/compute/contrib/cells.py +++ b/nova/api/openstack/compute/contrib/cells.py @@ -270,7 +270,10 @@ class Controller(object): def delete(self, req, id): """Delete a child or parent cell entry. 'id' is a cell name.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="delete") + try: num_deleted = self.cells_rpcapi.cell_delete(context, id) except exception.CellsUpdateUnsupported as e: @@ -347,7 +350,10 @@ class Controller(object): def create(self, req, body): """Create a child cell entry.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="create") + if 'cell' not in body: msg = _("No cell information in request") LOG.error(msg) @@ -371,7 +377,10 @@ class Controller(object): def update(self, req, id, body): """Update a child cell entry. 'id' is the cell name to update.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="update") + if 'cell' not in body: msg = _("No cell information in request") LOG.error(msg) @@ -403,7 +412,10 @@ class Controller(object): def sync_instances(self, req, body): """Tell all cells to sync instance info.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="sync_instances") + project_id = body.pop('project_id', None) deleted = body.pop('deleted', False) updated_since = body.pop('updated_since', None) diff --git a/nova/api/openstack/compute/plugins/v3/cells.py b/nova/api/openstack/compute/plugins/v3/cells.py index 31c3ffb13316..d542d85449d7 100644 --- a/nova/api/openstack/compute/plugins/v3/cells.py +++ b/nova/api/openstack/compute/plugins/v3/cells.py @@ -177,7 +177,10 @@ class CellsController(object): def delete(self, req, id): """Delete a child or parent cell entry. 'id' is a cell name.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="delete") + try: num_deleted = self.cells_rpcapi.cell_delete(context, id) except exception.CellsUpdateUnsupported as e: @@ -254,7 +257,10 @@ class CellsController(object): def create(self, req, body): """Create a child cell entry.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="create") + if 'cell' not in body: msg = _("No cell information in request") LOG.error(msg) @@ -277,7 +283,10 @@ class CellsController(object): def update(self, req, id, body): """Update a child cell entry. 'id' is the cell name to update.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="update") + if 'cell' not in body: msg = _("No cell information in request") LOG.error(msg) @@ -311,7 +320,10 @@ class CellsController(object): def sync_instances(self, req, body): """Tell all cells to sync instance info.""" context = req.environ['nova.context'] + authorize(context) + authorize(context, action="sync_instances") + project_id = body.pop('project_id', None) deleted = body.pop('deleted', False) updated_since = body.pop('updated_since', None) diff --git a/nova/tests/api/openstack/compute/contrib/test_cells.py b/nova/tests/api/openstack/compute/contrib/test_cells.py index 414ad33213a6..3fc6bfb5254e 100644 --- a/nova/tests/api/openstack/compute/contrib/test_cells.py +++ b/nova/tests/api/openstack/compute/contrib/test_cells.py @@ -133,7 +133,7 @@ class CellsTest(BaseCellsTest): self.assertEqual(cell['rpc_host'], 'r1.example.org') self.assertNotIn('password', cell) - def test_cell_delete(self): + def _cell_delete(self): call_info = {'delete_called': 0} def fake_cell_delete(inst, context, cell_name): @@ -143,9 +143,20 @@ class CellsTest(BaseCellsTest): self.stubs.Set(cells_rpcapi.CellsAPI, 'cell_delete', fake_cell_delete) req = self._get_request("cells/cell999") + req.environ['nova.context'] = self.context self.controller.delete(req, 'cell999') self.assertEqual(call_info['delete_called'], 1) + def test_cell_delete(self): + # Test cell delete with just cell policy + rules = {"default": "is_admin:true", + "compute_extension:cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_delete() + + def test_cell_delete_with_delete_policy(self): + self._cell_delete() + def test_delete_bogus_cell_raises(self): def fake_cell_delete(inst, context, cell_name): return 0 @@ -157,7 +168,19 @@ class CellsTest(BaseCellsTest): self.assertRaises(exc.HTTPNotFound, self.controller.delete, req, 'cell999') - def test_cell_create_parent(self): + def test_cell_delete_fails_for_invalid_policy(self): + def fake_cell_delete(inst, context, cell_name): + pass + + self.stubs.Set(cells_rpcapi.CellsAPI, 'cell_delete', fake_cell_delete) + + req = self._get_request("cells/cell999") + req.environ['nova.context'] = self.context + req.environ["nova.context"].is_admin = False + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.delete, req, 'cell999') + + def _cell_create_parent(self): body = {'cell': {'name': 'meow', 'username': 'fred', 'password': 'fubar', @@ -167,6 +190,7 @@ class CellsTest(BaseCellsTest): 'is_parent': False}} req = self._get_request("cells") + req.environ['nova.context'] = self.context res_dict = self.controller.create(req, body) cell = res_dict['cell'] @@ -177,7 +201,17 @@ class CellsTest(BaseCellsTest): self.assertNotIn('password', cell) self.assertNotIn('is_parent', cell) - def test_cell_create_child(self): + def test_cell_create_parent(self): + # Test create with just cells policy + rules = {"default": "is_admin:true", + "compute_extension:cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_create_parent() + + def test_cell_create_parent_with_create_policy(self): + self._cell_create_parent() + + def _cell_create_child(self): body = {'cell': {'name': 'meow', 'username': 'fred', 'password': 'fubar', @@ -185,6 +219,7 @@ class CellsTest(BaseCellsTest): 'type': 'child'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context res_dict = self.controller.create(req, body) cell = res_dict['cell'] @@ -195,6 +230,16 @@ class CellsTest(BaseCellsTest): self.assertNotIn('password', cell) self.assertNotIn('is_parent', cell) + def test_cell_create_child(self): + # Test create with just cells policy + rules = {"default": "is_admin:true", + "compute_extension:cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_create_child() + + def test_cell_create_child_with_create_policy(self): + self._cell_create_child() + def test_cell_create_no_name_raises(self): body = {'cell': {'username': 'moocow', 'password': 'secret', @@ -202,6 +247,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -213,6 +259,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -224,6 +271,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -235,6 +283,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -246,14 +295,24 @@ class CellsTest(BaseCellsTest): 'type': 'invalid'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) - def test_cell_update(self): + def test_cell_create_fails_for_invalid_policy(self): + body = {'cell': {'name': 'fake'}} + req = self._get_request("cells") + req.environ['nova.context'] = self.context + req.environ['nova.context'].is_admin = False + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.create, req, body) + + def _cell_update(self): body = {'cell': {'username': 'zeb', 'password': 'sneaky'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context res_dict = self.controller.update(req, 'cell1', body) cell = res_dict['cell'] @@ -262,12 +321,31 @@ class CellsTest(BaseCellsTest): self.assertEqual(cell['username'], 'zeb') self.assertNotIn('password', cell) + def test_cell_update(self): + # Test cell update with just cell policy + rules = {"default": "is_admin:true", + "compute_extension:cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_update() + + def test_cell_update_with_update_policy(self): + self._cell_update() + + def test_cell_update_fails_for_invalid_policy(self): + body = {'cell': {'name': 'got_changed'}} + req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context + req.environ['nova.context'].is_admin = False + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.create, req, body) + def test_cell_update_empty_name_raises(self): body = {'cell': {'name': '', 'username': 'zeb', 'password': 'sneaky'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.update, req, 'cell1', body) @@ -277,6 +355,7 @@ class CellsTest(BaseCellsTest): 'password': 'sneaky'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.update, req, 'cell1', body) @@ -284,6 +363,7 @@ class CellsTest(BaseCellsTest): body = {'cell': {'username': 'wingwj'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context res_dict = self.controller.update(req, 'cell1', body) cell = res_dict['cell'] @@ -297,10 +377,12 @@ class CellsTest(BaseCellsTest): body2 = {'cell': {'username': 'wingwj', 'type': 'parent'}} req1 = self._get_request("cells/cell1") + req1.environ['nova.context'] = self.context res_dict1 = self.controller.update(req1, 'cell1', body1) cell1 = res_dict1['cell'] req2 = self._get_request("cells/cell2") + req2.environ['nova.context'] = self.context res_dict2 = self.controller.update(req2, 'cell2', body2) cell2 = res_dict2['cell'] @@ -406,6 +488,7 @@ class CellsTest(BaseCellsTest): self.stubs.Set(cells_rpcapi.CellsAPI, 'sync_instances', sync_instances) req = self._get_request("cells/sync_instances") + req.environ['nova.context'] = self.context body = {} self.controller.sync_instances(req, body=body) self.assertIsNone(call_info['project_id']) @@ -455,6 +538,20 @@ class CellsTest(BaseCellsTest): self.assertRaises(exc.HTTPBadRequest, self.controller.sync_instances, req, body=body) + def test_sync_instances_fails_for_invalid_policy(self): + def sync_instances(self, context, **kwargs): + pass + + self.stubs.Set(cells_rpcapi.CellsAPI, 'sync_instances', sync_instances) + + req = self._get_request("cells/sync_instances") + req.environ['nova.context'] = self.context + req.environ['nova.context'].is_admin = False + + body = {} + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.sync_instances, req, body) + def test_cells_disabled(self): self.flags(enable=False, group='cells') diff --git a/nova/tests/api/openstack/compute/plugins/v3/test_cells.py b/nova/tests/api/openstack/compute/plugins/v3/test_cells.py index 874b1ffc085e..63cf2d96bd56 100644 --- a/nova/tests/api/openstack/compute/plugins/v3/test_cells.py +++ b/nova/tests/api/openstack/compute/plugins/v3/test_cells.py @@ -128,7 +128,7 @@ class CellsTest(BaseCellsTest): self.assertEqual(cell['rpc_host'], 'r1.example.org') self.assertNotIn('password', cell) - def test_cell_delete(self): + def _cell_delete(self): call_info = {'delete_called': 0} def fake_cell_delete(inst, context, cell_name): @@ -138,9 +138,20 @@ class CellsTest(BaseCellsTest): self.stubs.Set(cells_rpcapi.CellsAPI, 'cell_delete', fake_cell_delete) req = self._get_request("cells/cell999") + req.environ['nova.context'] = self.context self.controller.delete(req, 'cell999') self.assertEqual(call_info['delete_called'], 1) + def test_cell_delete(self): + # Test delete with just cells policy + rules = {"default": "is_admin:true", + "compute_extension:v3:os-cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_delete() + + def test_cell_delete_with_delete_policy(self): + self._cell_delete() + def test_delete_bogus_cell_raises(self): def fake_cell_delete(inst, context, cell_name): return 0 @@ -152,7 +163,19 @@ class CellsTest(BaseCellsTest): self.assertRaises(exc.HTTPNotFound, self.controller.delete, req, 'cell999') - def test_cell_create_parent(self): + def test_cell_delete_fails_for_invalid_policy(self): + def fake_cell_delete(inst, context, cell_name): + pass + + self.stubs.Set(cells_rpcapi.CellsAPI, 'cell_delete', fake_cell_delete) + + req = self._get_request("cells/cell999") + req.environ['nova.context'] = self.context + req.environ["nova.context"].is_admin = False + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.delete, req, 'cell999') + + def _cell_create_parent(self): body = {'cell': {'name': 'meow', 'username': 'fred', 'password': 'fubar', @@ -162,6 +185,7 @@ class CellsTest(BaseCellsTest): 'is_parent': False}} req = self._get_request("cells") + req.environ['nova.context'] = self.context res_dict = self.controller.create(req, body) cell = res_dict['cell'] self.assertEqual(self.controller.create.wsgi_code, 201) @@ -172,7 +196,17 @@ class CellsTest(BaseCellsTest): self.assertNotIn('password', cell) self.assertNotIn('is_parent', cell) - def test_cell_create_child(self): + def test_cell_create_parent(self): + # Test create with just cells policy + rules = {"default": "is_admin:true", + "compute_extension:v3:os-cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_create_parent() + + def test_cell_create_parent_with_create_policy(self): + self._cell_create_parent() + + def _cell_create_child(self): body = {'cell': {'name': 'meow', 'username': 'fred', 'password': 'fubar', @@ -180,6 +214,7 @@ class CellsTest(BaseCellsTest): 'type': 'child'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context res_dict = self.controller.create(req, body) cell = res_dict['cell'] self.assertEqual(self.controller.create.wsgi_code, 201) @@ -190,6 +225,16 @@ class CellsTest(BaseCellsTest): self.assertNotIn('password', cell) self.assertNotIn('is_parent', cell) + def test_cell_create_child(self): + # Test create child with just cells policy + rules = {"default": "is_admin:true", + "compute_extension:v3:os-cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_create_child() + + def test_cell_create_child_with_create_policy(self): + self._cell_create_child() + def test_cell_create_no_name_raises(self): body = {'cell': {'username': 'moocow', 'password': 'secret', @@ -197,6 +242,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -208,6 +254,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -219,6 +266,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -230,6 +278,7 @@ class CellsTest(BaseCellsTest): 'type': 'parent'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) @@ -241,14 +290,24 @@ class CellsTest(BaseCellsTest): 'type': 'invalid'}} req = self._get_request("cells") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.create, req, body) - def test_cell_update(self): + def test_cell_create_fails_for_invalid_policy(self): + body = {'cell': {'name': 'fake'}} + req = self._get_request("cells") + req.environ['nova.context'] = self.context + req.environ['nova.context'].is_admin = False + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.create, req, body) + + def _cell_update(self): body = {'cell': {'username': 'zeb', 'password': 'sneaky'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context res_dict = self.controller.update(req, 'cell1', body) cell = res_dict['cell'] @@ -257,12 +316,31 @@ class CellsTest(BaseCellsTest): self.assertEqual(cell['username'], 'zeb') self.assertNotIn('password', cell) + def test_cell_update(self): + # Test update with just cells policy + rules = {"default": "is_admin:true", + "compute_extension:v3:os-cells": "is_admin:true"} + self.policy.set_rules(rules) + self._cell_update() + + def test_cell_update_with_update_policy(self): + self._cell_update() + + def test_cell_update_fails_for_invalid_policy(self): + body = {'cell': {'name': 'got_changed'}} + req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context + req.environ['nova.context'].is_admin = False + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.create, req, body) + def test_cell_update_empty_name_raises(self): body = {'cell': {'name': '', 'username': 'zeb', 'password': 'sneaky'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.update, req, 'cell1', body) @@ -272,6 +350,7 @@ class CellsTest(BaseCellsTest): 'password': 'sneaky'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context self.assertRaises(exc.HTTPBadRequest, self.controller.update, req, 'cell1', body) @@ -279,6 +358,7 @@ class CellsTest(BaseCellsTest): body = {'cell': {'username': 'wingwj'}} req = self._get_request("cells/cell1") + req.environ['nova.context'] = self.context res_dict = self.controller.update(req, 'cell1', body) cell = res_dict['cell'] @@ -292,10 +372,12 @@ class CellsTest(BaseCellsTest): body2 = {'cell': {'username': 'wingwj', 'type': 'parent'}} req1 = self._get_request("cells/cell1") + req1.environ['nova.context'] = self.context res_dict1 = self.controller.update(req1, 'cell1', body1) cell1 = res_dict1['cell'] req2 = self._get_request("cells/cell2") + req2.environ['nova.context'] = self.context res_dict2 = self.controller.update(req2, 'cell2', body2) cell2 = res_dict2['cell'] @@ -343,7 +425,7 @@ class CellsTest(BaseCellsTest): self.assertEqual(response, res_dict['cell']['capacities']) def test_show_capacity_fails_with_non_admin_context(self): - rules = {"compute_extension:cells": "is_admin:true"} + rules = {"compute_extension:v3:os-cells": "is_admin:true"} self.policy.set_rules(rules) self.mox.ReplayAll() @@ -397,6 +479,7 @@ class CellsTest(BaseCellsTest): self.stubs.Set(cells_rpcapi.CellsAPI, 'sync_instances', sync_instances) req = self._get_request("cells/sync_instances") + req.environ['nova.context'] = self.context body = {} self.controller.sync_instances(req, body=body) self.assertIsNone(call_info['project_id']) @@ -446,6 +529,20 @@ class CellsTest(BaseCellsTest): self.assertRaises(exc.HTTPBadRequest, self.controller.sync_instances, req, body=body) + def test_sync_instances_fails_for_invalid_policy(self): + def sync_instances(self, context, **kwargs): + pass + + self.stubs.Set(cells_rpcapi.CellsAPI, 'sync_instances', sync_instances) + + req = self._get_request("cells/sync_instances") + req.environ['nova.context'] = self.context + req.environ['nova.context'].is_admin = False + + body = {} + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.sync_instances, req, body) + def test_cells_disabled(self): self.flags(enable=False, group='cells') diff --git a/nova/tests/fake_policy.py b/nova/tests/fake_policy.py index 38d9cd0584f0..181ad7a43867 100644 --- a/nova/tests/fake_policy.py +++ b/nova/tests/fake_policy.py @@ -134,7 +134,15 @@ policy_data = """ "compute_extension:v3:os-attach-interfaces": "", "compute_extension:baremetal_nodes": "", "compute_extension:cells": "", + "compute_extension:cells:create": "rule:admin_api", + "compute_extension:cells:delete": "rule:admin_api", + "compute_extension:cells:update": "rule:admin_api", + "compute_extension:cells:sync_instances": "rule:admin_api", "compute_extension:v3:os-cells": "", + "compute_extension:v3:os-cells:create": "rule:admin_api", + "compute_extension:v3:os-cells:delete": "rule:admin_api", + "compute_extension:v3:os-cells:update": "rule:admin_api", + "compute_extension:v3:os-cells:sync_instances": "rule:admin_api", "compute_extension:certificates": "", "compute_extension:v3:os-certificates:create": "", "compute_extension:v3:os-certificates:show": "",