Merge "Add new default roles in os-aggregates policies"

2020-04-13 22:08:16 +00:00
parent 2e299a6e72 92ddc0ca98
commit 0daa2a4f63
4 changed files with 45 additions and 59 deletions
--- 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=[
            {
--- 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")
--- 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": "",
--- 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
+        ]