diff --git a/nova/policies/aggregates.py b/nova/policies/aggregates.py index 5b6f7a51ea32..ea629a5db153 100644 --- a/nova/policies/aggregates.py +++ b/nova/policies/aggregates.py @@ -25,7 +25,7 @@ NEW_POLICY_ROOT = 'compute:aggregates:%s' aggregates_policies = [ policy.DocumentedRuleDefault( name=POLICY_ROOT % 'set_metadata', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Create or replace metadata for an aggregate", operations=[ { @@ -36,7 +36,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'add_host', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Add a host to an aggregate", operations=[ { @@ -47,7 +47,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'create', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Create an aggregate", operations=[ { @@ -58,7 +58,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'remove_host', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Remove a host from an aggregate", operations=[ { @@ -69,7 +69,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'update', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Update name and/or availability zone for an aggregate", operations=[ { @@ -80,7 +80,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'index', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_READER, description="List all aggregates", operations=[ { @@ -91,7 +91,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'delete', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Delete an aggregate", operations=[ { @@ -102,7 +102,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=POLICY_ROOT % 'show', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_READER, description="Show details for an aggregate", operations=[ { @@ -113,7 +113,7 @@ aggregates_policies = [ scope_types=['system']), policy.DocumentedRuleDefault( name=NEW_POLICY_ROOT % 'images', - check_str=base.RULE_ADMIN_API, + check_str=base.SYSTEM_ADMIN, description="Request image caching for an aggregate", operations=[ { diff --git a/nova/tests/unit/api/openstack/compute/test_aggregates.py b/nova/tests/unit/api/openstack/compute/test_aggregates.py index 34e34ba85b7f..fb096861eb75 100644 --- a/nova/tests/unit/api/openstack/compute/test_aggregates.py +++ b/nova/tests/unit/api/openstack/compute/test_aggregates.py @@ -116,11 +116,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): self._assert_agg_data(AGGREGATE_LIST, _make_agg_list(result)) self.assertTrue(mock_list.called) - def test_index_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - self.controller.index, - self.user_req) - def test_create(self): with mock.patch.object(self.controller.api, 'create_aggregate', return_value=AGGREGATE) as mock_create: @@ -131,12 +126,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): self._assert_agg_data(FORMATTED_AGGREGATE, _make_agg_obj(result)) mock_create.assert_called_once_with(self.context, 'test', 'nova1') - def test_create_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - self.controller.create, self.user_req, - body={"aggregate": {"name": "test", - "availability_zone": "nova1"}}) - def test_create_with_duplicate_aggregate_name(self): side_effect = exception.AggregateNameExists(aggregate_name="test") with mock.patch.object(self.controller.api, 'create_aggregate', @@ -294,11 +283,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): self._assert_agg_data(AGGREGATE, _make_agg_obj(aggregate)) mock_get.assert_called_once_with(self.context, '1') - def test_show_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - self.controller.show, - self.user_req, "1") - def test_show_with_bad_aggregate(self): side_effect = exception.AggregateNotFound(aggregate_id='2') with mock.patch.object(self.controller.api, 'get_aggregate', @@ -323,12 +307,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): mock_update.assert_called_once_with(self.context, '1', body["aggregate"]) - def test_update_no_admin(self): - body = {"aggregate": {"availability_zone": "nova"}} - self.assertRaises(exception.PolicyNotAuthorized, - self.controller.update, - self.user_req, "1", body=body) - def test_update_with_only_name(self): body = {"aggregate": {"name": "new_name"}} with mock.patch.object(self.controller.api, 'update_aggregate', @@ -459,12 +437,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): self._assert_agg_data(AGGREGATE, _make_agg_obj(aggregate)) mock_add.assert_called_once_with(self.context, "1", "host1") - def test_add_host_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - eval(self.add_host), - self.user_req, "1", - body={"add_host": {"host": "host1"}}) - def test_add_host_with_already_added_host(self): side_effect = exception.AggregateHostExists(aggregate_id="1", host="host1") @@ -541,12 +513,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): body={"remove_host": {"host": "host1"}}) mock_rem.assert_called_once_with(self.context, "1", "host1") - def test_remove_host_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - eval(self.remove_host), - self.user_req, "1", - body={"remove_host": {"host": "host1"}}) - def test_remove_host_with_bad_aggregate(self): side_effect = exception.AggregateNotFound( aggregate_id="2") @@ -650,13 +616,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): mocked.assert_called_once_with(self.context, "1", body["set_metadata"]["metadata"]) - def test_set_metadata_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - eval(self.set_metadata), - self.user_req, "1", - body={"set_metadata": {"metadata": - {"foo": "bar"}}}) - def test_set_metadata_with_bad_aggregate(self): body = {"set_metadata": {"metadata": {"foo": "bar"}}} side_effect = exception.AggregateNotFound(aggregate_id="2") @@ -715,11 +674,6 @@ class AggregateTestCaseV21(test.NoDBTestCase): self.controller.delete(self.req, "1") mock_del.assert_called_once_with(self.context, "1") - def test_delete_aggregate_no_admin(self): - self.assertRaises(exception.PolicyNotAuthorized, - self.controller.delete, - self.user_req, "1") - def test_delete_aggregate_with_bad_aggregate(self): side_effect = exception.AggregateNotFound( aggregate_id="2") diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index abea3bdcc764..5aff60d21d56 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -32,6 +32,15 @@ policy_data = """ "os_compute_api:os-agents:create": "", "os_compute_api:os-agents:update": "", "os_compute_api:os-agents:delete": "", + "os_compute_api:os-aggregates:set_metadata": "", + "os_compute_api:os-aggregates:remove_host": "", + "os_compute_api:os-aggregates:add_host": "", + "os_compute_api:os-aggregates:create": "", + "os_compute_api:os-aggregates:index": "", + "os_compute_api:os-aggregates:update": "", + "os_compute_api:os-aggregates:delete": "", + "os_compute_api:os-aggregates:show": "", + "compute:aggregates:images": "", "os_compute_api:os-attach-interfaces:list": "", "os_compute_api:os-attach-interfaces:show": "", "os_compute_api:os-attach-interfaces:create": "", diff --git a/nova/tests/unit/policies/test_aggregates.py b/nova/tests/unit/policies/test_aggregates.py index 2bcfa516c9b5..215320f10d1c 100644 --- a/nova/tests/unit/policies/test_aggregates.py +++ b/nova/tests/unit/policies/test_aggregates.py @@ -43,11 +43,23 @@ class AggregatesPolicyTest(base.BasePolicyTest): self.project_foo_context, self.project_reader_context ] + # Check that system reader is able to get Aggregate + self.system_reader_authorized_contexts = [ + self.legacy_admin_context, self.system_admin_context, + self.project_admin_context, self.system_member_context, + self.system_reader_context] + # Check that non-admin is not able to get Aggregate + self.system_reader_unauthorized_contexts = [ + self.system_foo_context, self.project_member_context, + self.other_project_member_context, + self.project_foo_context, self.project_reader_context + ] + @mock.patch('nova.compute.api.AggregateAPI.get_aggregate_list') def test_list_aggregate_policy(self, mock_list): rule_name = "os_compute_api:os-aggregates:index" - self.common_policy_check(self.admin_authorized_contexts, - self.admin_unauthorized_contexts, + self.common_policy_check(self.system_reader_authorized_contexts, + self.system_reader_unauthorized_contexts, rule_name, self.controller.index, self.req) @@ -87,8 +99,8 @@ class AggregatesPolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.AggregateAPI.get_aggregate') def test_show_aggregate_policy(self, mock_show): rule_name = "os_compute_api:os-aggregates:show" - self.common_policy_check(self.admin_authorized_contexts, - self.admin_unauthorized_contexts, + self.common_policy_check(self.system_reader_authorized_contexts, + self.system_reader_unauthorized_contexts, rule_name, self.controller.show, self.req, 1) @@ -162,3 +174,14 @@ class AggregatesScopeTypePolicyTest(AggregatesPolicyTest): self.other_project_member_context, self.project_foo_context, self.project_reader_context ] + # Check that system reader is able to get Aggregate + self.system_reader_authorized_contexts = [ + self.system_admin_context, self.system_member_context, + self.system_reader_context] + # Check that non-admin is not able to get Aggregate + self.system_reader_unauthorized_contexts = [ + self.legacy_admin_context, self.project_admin_context, + self.system_foo_context, self.project_member_context, + self.other_project_member_context, + self.project_foo_context, self.project_reader_context + ]