From 6f8837d84efa010c8b4ee7f457f2b9cda04e94dd Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 25 Oct 2019 00:13:13 -0400 Subject: [PATCH] Split 'action' policy into more granular controls Allow operators to specify different policies for each action, since each action is quite different in character. The previous "actions:action" rule remains and is the default for each of the new rules, so there is no effect on existing policies and no action required by the operator unless they want to take advantage of the additional flexibility. Change-Id: Ic4985e8637bc4f34ea2514075b30d2ec32f3441c Task: 37296 --- heat/api/openstack/v1/actions.py | 46 +++++++++++++------ heat/policies/actions.py | 37 +++++++++++---- heat/tests/api/openstack_v1/test_actions.py | 22 ++++----- ...anular-action-policy-b8c143bb5f203b68.yaml | 10 ++++ 4 files changed, 77 insertions(+), 38 deletions(-) create mode 100644 releasenotes/notes/granular-action-policy-b8c143bb5f203b68.yaml diff --git a/heat/api/openstack/v1/actions.py b/heat/api/openstack/v1/actions.py index 2b058e72c9..756e7ea98c 100644 --- a/heat/api/openstack/v1/actions.py +++ b/heat/api/openstack/v1/actions.py @@ -42,8 +42,9 @@ class ActionController(object): self.options = options self.rpc_client = rpc_client.EngineClient() - @util.registered_identified_stack - def action(self, req, identity, body=None): + # Don't enforce policy on this API, as potentially differing policies + # will be enforced on individual actions. + def action(self, req, tenant_id, stack_name, stack_id, body=None): """Performs a specified action on a stack. The body is expecting to contain exactly one item whose key specifies @@ -60,21 +61,36 @@ class ActionController(object): if ac not in self.ACTIONS: raise exc.HTTPBadRequest(_("Invalid action %s specified") % ac) - if ac == self.SUSPEND: - self.rpc_client.stack_suspend(req.context, identity) - elif ac == self.RESUME: - self.rpc_client.stack_resume(req.context, identity) - elif ac == self.CHECK: - self.rpc_client.stack_check(req.context, identity) - elif ac == self.CANCEL_UPDATE: - self.rpc_client.stack_cancel_update(req.context, identity, - cancel_with_rollback=True) - elif ac == self.CANCEL_WITHOUT_ROLLBACK: - self.rpc_client.stack_cancel_update(req.context, identity, - cancel_with_rollback=False) - else: + do_action = getattr(self, ac, None) + if do_action is None: raise exc.HTTPInternalServerError(_("Unexpected action %s") % ac) + do_action(req, tenant_id=tenant_id, + stack_name=stack_name, stack_id=stack_id, + body=body) + + @util.registered_identified_stack + def suspend(self, req, identity, body=None): + self.rpc_client.stack_suspend(req.context, identity) + + @util.registered_identified_stack + def resume(self, req, identity, body=None): + self.rpc_client.stack_resume(req.context, identity) + + @util.registered_identified_stack + def check(self, req, identity, body=None): + self.rpc_client.stack_check(req.context, identity) + + @util.registered_identified_stack + def cancel_update(self, req, identity, body=None): + self.rpc_client.stack_cancel_update(req.context, identity, + cancel_with_rollback=True) + + @util.registered_identified_stack + def cancel_without_rollback(self, req, identity, body=None): + self.rpc_client.stack_cancel_update(req.context, identity, + cancel_with_rollback=False) + def create_resource(options): """Actions action factory method.""" diff --git a/heat/policies/actions.py b/heat/policies/actions.py index 4dd45fcb01..78865786e2 100644 --- a/heat/policies/actions.py +++ b/heat/policies/actions.py @@ -16,20 +16,39 @@ from heat.policies import base POLICY_ROOT = 'actions:%s' + +def _action_rule(action_name, description): + return policy.DocumentedRuleDefault( + name=POLICY_ROOT % action_name, + check_str='rule:%s' % (POLICY_ROOT % 'action'), + description=description, + operations=[{ + 'path': '/v1/{tenant_id}/stacks/{stack_name}/{stack_id}/actions', + 'method': 'POST', + }] + ) + + actions_policies = [ policy.DocumentedRuleDefault( name=POLICY_ROOT % 'action', check_str=base.RULE_DENY_STACK_USER, description='Performs non-lifecycle operations on the stack ' - '(Snapshot, Resume, Cancel update, or check stack resources).', - operations=[ - { - 'path': '/v1/{tenant_id}/stacks/{stack_name}/{stack_id}/' - 'actions', - 'method': 'POST' - } - ] - ) + '(Snapshot, Resume, Cancel update, or check stack resources). ' + 'This is the default for all actions but can be overridden by more ' + 'specific policies for individual actions.', + operations=[{ + 'path': '/v1/{tenant_id}/stacks/{stack_name}/{stack_id}/actions', + 'method': 'POST', + }], + ), + _action_rule('snapshot', 'Create stack snapshot.'), + _action_rule('suspend', 'Suspend a stack.'), + _action_rule('resume', 'Resume a suspended stack.'), + _action_rule('check', 'Check stack resources.'), + _action_rule('cancel_update', 'Cancel stack operation and roll back.'), + _action_rule('cancel_without_rollback', + 'Cancel stack operation without rolling back.'), ] diff --git a/heat/tests/api/openstack_v1/test_actions.py b/heat/tests/api/openstack_v1/test_actions.py index bbce1bbb36..bca592b63d 100644 --- a/heat/tests/api/openstack_v1/test_actions.py +++ b/heat/tests/api/openstack_v1/test_actions.py @@ -45,7 +45,7 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): self.controller = actions.ActionController(options=cfgopts) def test_action_suspend(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) + self._mock_enforce_setup(mock_enforce, 'suspend', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'suspend': None} @@ -67,7 +67,7 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): ) def test_action_resume(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) + self._mock_enforce_setup(mock_enforce, 'resume', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'resume': None} @@ -89,7 +89,7 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): ) def test_action_check(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) + self._mock_enforce_setup(mock_enforce, 'check', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'check': None} @@ -111,13 +111,11 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): ) def _test_action_cancel_update(self, mock_enforce, with_rollback=True): - self._mock_enforce_setup(mock_enforce, 'action', True) + act = 'cancel_update' if with_rollback else 'cancel_without_rollback' + self._mock_enforce_setup(mock_enforce, act, True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') - if with_rollback: - body = {'cancel_update': None} - else: - body = {'cancel_without_rollback': None} + body = {act: None} req = self._post(stack_identity._tenant_path() + '/actions', data=json.dumps(body)) @@ -141,7 +139,6 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): self._test_action_cancel_update(mock_enforce, False) def test_action_badaction(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'notallowed': None} @@ -155,7 +152,6 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): body=body) def test_action_badaction_empty(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {} @@ -169,7 +165,6 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): body=body) def test_action_badaction_multiple(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'one': None, 'two': None} @@ -183,7 +178,7 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): body=body) def test_action_rmt_aterr(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) + self._mock_enforce_setup(mock_enforce, 'suspend', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'suspend': None} @@ -211,7 +206,7 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): ) def test_action_err_denied_policy(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', False) + self._mock_enforce_setup(mock_enforce, 'suspend', False) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'suspend': None} @@ -229,7 +224,6 @@ class ActionControllerTest(tools.ControllerTest, common.HeatTestCase): self.assertIn('403 Forbidden', six.text_type(resp)) def test_action_badaction_ise(self, mock_enforce): - self._mock_enforce_setup(mock_enforce, 'action', True) stack_identity = identifier.HeatIdentifier(self.tenant, 'wordpress', '1') body = {'oops': None} diff --git a/releasenotes/notes/granular-action-policy-b8c143bb5f203b68.yaml b/releasenotes/notes/granular-action-policy-b8c143bb5f203b68.yaml new file mode 100644 index 0000000000..63b1cbcb5d --- /dev/null +++ b/releasenotes/notes/granular-action-policy-b8c143bb5f203b68.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Operators can now apply different authorization policies to each action + supported by the action API (``actions:suspend`` for suspend, + ``actions:resume`` for resume, ``actions:check`` for check, + ``actions:cancel_update`` for cancel operation and roll back, and + ``actions:cancel_without_rollback`` for cancel operation without rolling + back). The default for each is to use the existing ``actions:action`` rule + that was previously the only way to specify policy for actions.