diff --git a/nova/api/openstack/compute/deferred_delete.py b/nova/api/openstack/compute/deferred_delete.py index 26e6e453f909..55879267ffdf 100644 --- a/nova/api/openstack/compute/deferred_delete.py +++ b/nova/api/openstack/compute/deferred_delete.py @@ -36,7 +36,7 @@ class DeferredDeleteController(wsgi.Controller): """Restore a previously deleted instance.""" context = req.environ["nova.context"] instance = common.get_instance(self.compute_api, context, id) - context.can(dd_policies.BASE_POLICY_NAME, + context.can(dd_policies.BASE_POLICY_NAME % 'restore', target={'project_id': instance.project_id}) try: self.compute_api.restore(context, instance) @@ -53,7 +53,7 @@ class DeferredDeleteController(wsgi.Controller): """Force delete of instance before deferred cleanup.""" context = req.environ["nova.context"] instance = common.get_instance(self.compute_api, context, id) - context.can(dd_policies.BASE_POLICY_NAME, + context.can(dd_policies.BASE_POLICY_NAME % 'force', target={'user_id': instance.user_id, 'project_id': instance.project_id}) try: diff --git a/nova/policies/deferred_delete.py b/nova/policies/deferred_delete.py index 3c370e2284d1..2493a7ddbdea 100644 --- a/nova/policies/deferred_delete.py +++ b/nova/policies/deferred_delete.py @@ -18,26 +18,48 @@ from oslo_policy import policy from nova.policies import base -BASE_POLICY_NAME = 'os_compute_api:os-deferred-delete' +BASE_POLICY_NAME = 'os_compute_api:os-deferred-delete:%s' +DEPRECATED_POLICY = policy.DeprecatedRule( + 'os_compute_api:os-deferred-delete', + base.RULE_ADMIN_OR_OWNER, +) + +DEPRECATED_REASON = """ +Nova API policies are introducing new default roles with scope_type +capabilities. Old policies are deprecated and silently going to be ignored +in nova 23.0.0 release. +""" deferred_delete_policies = [ policy.DocumentedRuleDefault( - name=BASE_POLICY_NAME, - check_str=base.RULE_ADMIN_OR_OWNER, - description="Restore a soft deleted server or force delete a server " - "before deferred cleanup", + name=BASE_POLICY_NAME % 'restore', + check_str=base.PROJECT_MEMBER_OR_SYSTEM_ADMIN, + description="Restore a soft deleted server", operations=[ { 'method': 'POST', 'path': '/servers/{server_id}/action (restore)' }, + ], + scope_types=['system', 'project'], + deprecated_rule=DEPRECATED_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0'), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'force', + check_str=base.PROJECT_MEMBER_OR_SYSTEM_ADMIN, + description="Force delete a server before deferred cleanup", + operations=[ { 'method': 'POST', 'path': '/servers/{server_id}/action (forceDelete)' } ], - scope_types=['system', 'project']) + scope_types=['system', 'project'], + deprecated_rule=DEPRECATED_POLICY, + deprecated_reason=DEPRECATED_REASON, + deprecated_since='21.0.0') ] diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py index 8883e92b819b..4cef1c3b6bb3 100644 --- a/nova/tests/unit/fake_policy.py +++ b/nova/tests/unit/fake_policy.py @@ -31,7 +31,8 @@ policy_data = """ "os_compute_api:os-console-output": "", "os_compute_api:os-remote-consoles": "", "os_compute_api:os-create-backup": "", - "os_compute_api:os-deferred-delete": "", + "os_compute_api:os-deferred-delete:restore": "", + "os_compute_api:os-deferred-delete:force": "", "os_compute_api:os-extended-server-attributes": "", "os_compute_api:ips:index": "", "os_compute_api:ips:show": "", diff --git a/nova/tests/unit/policies/test_deferred_delete.py b/nova/tests/unit/policies/test_deferred_delete.py index 490223241c1d..abfa728b8cdd 100644 --- a/nova/tests/unit/policies/test_deferred_delete.py +++ b/nova/tests/unit/policies/test_deferred_delete.py @@ -18,6 +18,8 @@ from oslo_utils import timeutils from nova.api.openstack.compute import deferred_delete from nova.compute import vm_states from nova import exception +from nova.policies import base as base_policy +from nova.policies import deferred_delete as dd_policies from nova.tests.unit.api.openstack import fakes from nova.tests.unit import fake_instance from nova.tests.unit.policies import base @@ -61,7 +63,7 @@ class DeferredDeletePolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.restore') def test_restore_server_policy(self, mock_restore): - rule_name = "os_compute_api:os-deferred-delete" + rule_name = dd_policies.BASE_POLICY_NAME % 'restore' self.common_policy_check(self.admin_authorized_contexts, self.admin_unauthorized_contexts, rule_name, self.controller._restore, @@ -69,7 +71,7 @@ class DeferredDeletePolicyTest(base.BasePolicyTest): body={'restore': {}}) def test_force_delete_server_policy(self): - rule_name = "os_compute_api:os-deferred-delete" + rule_name = dd_policies.BASE_POLICY_NAME % 'force' self.common_policy_check(self.admin_authorized_contexts, self.admin_unauthorized_contexts, rule_name, self.controller._force_delete, @@ -77,7 +79,7 @@ class DeferredDeletePolicyTest(base.BasePolicyTest): body={'forceDelete': {}}) def test_force_delete_server_policy_failed_with_other_user(self): - rule_name = "os_compute_api:os-deferred-delete" + rule_name = dd_policies.BASE_POLICY_NAME % 'force' # Change the user_id in request context. req = fakes.HTTPRequest.blank('') req.environ['nova.context'].user_id = 'other-user' @@ -92,7 +94,7 @@ class DeferredDeletePolicyTest(base.BasePolicyTest): @mock.patch('nova.compute.api.API.force_delete') def test_force_delete_server_policy_pass_with_same_user( self, force_delete_mock): - rule_name = "os_compute_api:os-deferred-delete" + rule_name = dd_policies.BASE_POLICY_NAME % 'force' self.policy.set_rules({rule_name: "user_id:%(user_id)s"}) self.controller._force_delete(self.req, self.instance.uuid, body={'forceDelete': {}}) @@ -114,3 +116,34 @@ class DeferredDeleteScopeTypePolicyTest(DeferredDeletePolicyTest): def setUp(self): super(DeferredDeleteScopeTypePolicyTest, self).setUp() self.flags(enforce_scope=True, group="oslo_policy") + + +class DeferredDeleteNoLegacyPolicyTest(DeferredDeletePolicyTest): + """Test Deferred Delete APIs policies with system scope enabled, + and no more deprecated rules that allow the legacy admin API to + access system_admin_or_owner APIs. + """ + without_deprecated_rules = True + rules_without_deprecation = { + dd_policies.BASE_POLICY_NAME % 'restore': + base_policy.PROJECT_MEMBER_OR_SYSTEM_ADMIN, + dd_policies.BASE_POLICY_NAME % 'force': + base_policy.PROJECT_MEMBER_OR_SYSTEM_ADMIN} + + def setUp(self): + super(DeferredDeleteNoLegacyPolicyTest, self).setUp() + self.flags(enforce_scope=True, group="oslo_policy") + + # Check that system or projct admin or owner is able to + # force delete or restore server. + self.admin_authorized_contexts = [ + self.system_admin_context, + self.project_admin_context, self.project_member_context] + # Check that non-system and non-admin/owner is not able to + # force delete or restore server. + self.admin_unauthorized_contexts = [ + self.legacy_admin_context, self.project_reader_context, + self.project_foo_context, + self.system_member_context, self.system_reader_context, + self.system_foo_context, + self.other_project_member_context] diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py index e44ce0e4cd15..9e408e608ccc 100644 --- a/nova/tests/unit/test_policy.py +++ b/nova/tests/unit/test_policy.py @@ -420,7 +420,8 @@ class RealRolePolicyTestCase(test.NoDBTestCase): "os_compute_api:os-attach-interfaces:delete", "os_compute_api:os-console-output", "os_compute_api:os-remote-consoles", -"os_compute_api:os-deferred-delete", +"os_compute_api:os-deferred-delete:restore", +"os_compute_api:os-deferred-delete:force", "os_compute_api:os-flavor-access", "os_compute_api:os-flavor-extra-specs:index", "os_compute_api:os-flavor-extra-specs:show",