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

@@ -42,8 +42,9 @@ class ActionController(object):
         self.options = options
         self.rpc_client = rpc_client.EngineClient()
 
-    @util.registered_identified_stack
+    # Don't enforce policy on this API, as potentially differing policies
-    def action(self, req, identity, body=None):
+    # 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:
+        do_action = getattr(self, ac, None)
-            self.rpc_client.stack_suspend(req.context, identity)
+        if do_action is None:
-        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:
             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."""

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).',
+        '(Snapshot, Resume, Cancel update, or check stack resources). '
-        operations=[
+        'This is the default for all actions but can be overridden by more '
-            {
+        'specific policies for individual actions.',
-                'path': '/v1/{tenant_id}/stacks/{stack_name}/{stack_id}/'
+        operations=[{
-                'actions',
+            'path': '/v1/{tenant_id}/stacks/{stack_name}/{stack_id}/actions',
-                'method': 'POST'
+            '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.'),
 ]
 
 

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 = {act: None}
-            body = {'cancel_update': None}
-        else:
-            body = {'cancel_without_rollback': 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}

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.