From 9fe72de4b690bc5c964c12715581128830c667d5 Mon Sep 17 00:00:00 2001
From: Sean McGinnis <sean.mcginnis@huawei.com>
Date: Fri, 1 Dec 2017 15:34:32 -0600
Subject: [PATCH] Add cg policies and clean up old policy handling

This completes the "policy-in-code" work for Cinder by removing
the old policy.json handling. It also adds new in-code checks to
the legacy consistency group code for completeness.

Change-Id: I810b6cb6bba2d95cc5bb477d6e2968ac1734c96b
Depends-on: I364e401227fe43e2bacf8a799e10286ee445f835
Implements: bp policy-in-code
---
 cinder/api/contrib/consistencygroups.py       | 19 ++--
 cinder/api/v3/consistencygroups.py            |  5 +-
 cinder/policy.py                              | 93 +++++++++----------
 cinder/test.py                                |  5 +-
 cinder/tests/unit/policy.json                 | 12 ---
 etc/cinder/policy.json                        | 14 ---
 .../policy-in-code-226f71562ab28195.yaml      | 11 +++
 setup.cfg                                     |  1 -
 8 files changed, 70 insertions(+), 90 deletions(-)
 delete mode 100644 etc/cinder/policy.json
 create mode 100644 releasenotes/notes/policy-in-code-226f71562ab28195.yaml

diff --git a/cinder/api/contrib/consistencygroups.py b/cinder/api/contrib/consistencygroups.py
index a17e89acb32..02be5fac3c9 100644
--- a/cinder/api/contrib/consistencygroups.py
+++ b/cinder/api/contrib/consistencygroups.py
@@ -25,10 +25,11 @@ from cinder.api import common
 from cinder.api import extensions
 from cinder.api.openstack import wsgi
 from cinder.api.views import consistencygroups as consistencygroup_views
-from cinder.consistencygroup import api as consistencygroup_api
 from cinder import exception
 from cinder import group as group_api
 from cinder.i18n import _
+from cinder.policies import group_actions as gp_action_policy
+from cinder.policies import groups as group_policy
 from cinder.volume import group_types
 
 LOG = logging.getLogger(__name__)
@@ -73,7 +74,7 @@ class ConsistencyGroupsController(wsgi.Controller):
 
         try:
             group = self._get(context, id)
-            consistencygroup_api.check_policy(context, 'delete')
+            context.authorize(gp_action_policy.DELETE_POLICY, target_obj=group)
             self.group_api.delete(context, group, force)
         # Not found exception will be handled at the wsgi level
         except exception.InvalidConsistencyGroup as error:
@@ -106,6 +107,7 @@ class ConsistencyGroupsController(wsgi.Controller):
     def _get_consistencygroups(self, req, is_detail):
         """Returns a list of consistency groups through view builder."""
         context = req.environ['cinder.context']
+        context.authorize(group_policy.GET_ALL_POLICY)
         filters = req.params.copy()
 
         # make another copy of filters, since it is being modified in
@@ -131,6 +133,7 @@ class ConsistencyGroupsController(wsgi.Controller):
         self.assert_valid_body(body, 'consistencygroup')
 
         context = req.environ['cinder.context']
+        context.authorize(group_policy.CREATE_POLICY)
         consistencygroup = body['consistencygroup']
         self.validate_name_and_description(consistencygroup)
         name = consistencygroup.get('name', None)
@@ -153,7 +156,6 @@ class ConsistencyGroupsController(wsgi.Controller):
                  {'name': name})
 
         try:
-            consistencygroup_api.check_policy(context, 'create')
             new_consistencygroup = self.group_api.create(
                 context, name, description, group_type['id'], volume_types,
                 availability_zone=availability_zone)
@@ -181,6 +183,7 @@ class ConsistencyGroupsController(wsgi.Controller):
         self.assert_valid_body(body, 'consistencygroup-from-src')
 
         context = req.environ['cinder.context']
+        context.authorize(group_policy.CREATE_POLICY)
         consistencygroup = body['consistencygroup-from-src']
         self.validate_name_and_description(consistencygroup)
         name = consistencygroup.get('name', None)
@@ -213,7 +216,6 @@ class ConsistencyGroupsController(wsgi.Controller):
                 self._get(context, source_cgid)
             if cgsnapshot_id:
                 self._get_cgsnapshot(context, cgsnapshot_id)
-            consistencygroup_api.check_policy(context, 'create')
             new_group = self.group_api.create_from_src(
                 context, name, description, cgsnapshot_id, source_cgid)
         except exception.NotFound:
@@ -232,19 +234,18 @@ class ConsistencyGroupsController(wsgi.Controller):
                     "can not be all empty in the request body.")
             raise exc.HTTPBadRequest(explanation=msg)
 
-    def _update(self, context, id, name, description, add_volumes,
+    def _update(self, context, group, name, description, add_volumes,
                 remove_volumes,
                 allow_empty=False):
         LOG.info("Updating consistency group %(id)s with name %(name)s "
                  "description: %(description)s add_volumes: "
                  "%(add_volumes)s remove_volumes: %(remove_volumes)s.",
-                 {'id': id,
+                 {'id': group.id,
                   'name': name,
                   'description': description,
                   'add_volumes': add_volumes,
                   'remove_volumes': remove_volumes})
 
-        group = self._get(context, id)
         self.group_api.update(context, group, name, description,
                               add_volumes, remove_volumes)
 
@@ -273,6 +274,8 @@ class ConsistencyGroupsController(wsgi.Controller):
 
         self.assert_valid_body(body, 'consistencygroup')
         context = req.environ['cinder.context']
+        group = self._get(context, id)
+        context.authorize(group_policy.UPDATE_POLICY, target_obj=group)
         consistencygroup = body.get('consistencygroup', None)
         self.validate_name_and_description(consistencygroup)
         name = consistencygroup.get('name', None)
@@ -282,7 +285,7 @@ class ConsistencyGroupsController(wsgi.Controller):
 
         self._check_update_parameters(name, description, add_volumes,
                                       remove_volumes)
-        self._update(context, id, name, description, add_volumes,
+        self._update(context, group, name, description, add_volumes,
                      remove_volumes)
         return webob.Response(status_int=http_client.ACCEPTED)
 
diff --git a/cinder/api/v3/consistencygroups.py b/cinder/api/v3/consistencygroups.py
index 6ff3efba88c..a076c181f81 100644
--- a/cinder/api/v3/consistencygroups.py
+++ b/cinder/api/v3/consistencygroups.py
@@ -22,6 +22,7 @@ from cinder.api.contrib import consistencygroups as cg_v2
 from cinder.api import microversions as mv
 from cinder.api.openstack import wsgi
 from cinder.i18n import _
+from cinder.policies import groups as group_policy
 
 LOG = logging.getLogger(__name__)
 
@@ -72,6 +73,8 @@ class ConsistencyGroupsController(cg_v2.ConsistencyGroupsController):
 
         self.assert_valid_body(body, 'consistencygroup')
         context = req.environ['cinder.context']
+        group = self._get(context, id)
+        context.authorize(group_policy.UPDATE_POLICY, target_obj=group)
         consistencygroup = body.get('consistencygroup', None)
         self.validate_name_and_description(consistencygroup)
         name = consistencygroup.get('name', None)
@@ -83,7 +86,7 @@ class ConsistencyGroupsController(cg_v2.ConsistencyGroupsController):
                                                        description,
                                                        add_volumes,
                                                        remove_volumes)
-        self._update(context, id, name, description, add_volumes,
+        self._update(context, group, name, description, add_volumes,
                      remove_volumes, allow_empty)
         return webob.Response(status_int=http_client.ACCEPTED)
 
diff --git a/cinder/policy.py b/cinder/policy.py
index 06e8ff8ec06..a25df1e03eb 100644
--- a/cinder/policy.py
+++ b/cinder/policy.py
@@ -40,25 +40,17 @@ def reset():
         _ENFORCER = None
 
 
-def init(policy_file=None, rules=None, default_rule=None, use_conf=True):
+def init(use_conf=True):
     """Init an Enforcer class.
 
-        :param policy_file: Custom policy file to use, if none is specified,
-                          `CONF.policy_file` will be used.
-        :param rules: Default dictionary / Rules to use. It will be
-                    considered just in the first instantiation.
-        :param default_rule: Default rule to use, CONF.default_rule will
-                           be used if none is specified.
-        :param use_conf: Whether to load rules from config file.
+    :param use_conf: Whether to load rules from config file.
     """
 
     global _ENFORCER
     if not _ENFORCER:
-        _ENFORCER = policy.Enforcer(CONF,
-                                    policy_file=policy_file,
-                                    rules=rules,
-                                    default_rule=default_rule,
-                                    use_conf=use_conf)
+        _ENFORCER = policy.Enforcer(
+            CONF,
+            use_conf=use_conf)
         register_rules(_ENFORCER)
         _ENFORCER.load_rules()
 
@@ -66,19 +58,19 @@ def init(policy_file=None, rules=None, default_rule=None, use_conf=True):
 def enforce(context, action, target):
     """Verifies that the action is valid on the target in this context.
 
-       :param context: cinder context
-       :param action: string representing the action to be checked
-           this should be colon separated for clarity.
-           i.e. ``compute:create_instance``,
-           ``compute:attach_volume``,
-           ``volume:attach_volume``
+    :param context: cinder context
+    :param action: string representing the action to be checked
+                   this should be colon separated for clarity.
+                   i.e. ``compute:create_instance``,
+                   ``compute:attach_volume``,
+                   ``volume:attach_volume``
 
-       :param object: dictionary representing the object of the action
-           for object creation this should be a dictionary representing the
-           location of the object e.g. ``{'project_id': context.project_id}``
-
-       :raises PolicyNotAuthorized: if verification fails.
+    :param target: dictionary representing the object of the action for object
+                   creation this should be a dictionary representing the
+                   location of the object e.g.
+                   ``{'project_id': context.project_id}``
 
+    :raises PolicyNotAuthorized: if verification fails.
     """
     init()
 
@@ -93,10 +85,10 @@ def enforce(context, action, target):
 def set_rules(rules, overwrite=True, use_conf=False):
     """Set rules based on the provided dict of rules.
 
-       :param rules: New rules to use. It should be an instance of dict.
-       :param overwrite: Whether to overwrite current rules or update them
-                         with the new rules.
-       :param use_conf: Whether to reload rules from config file.
+    :param rules: New rules to use. It should be an instance of dict.
+    :param overwrite: Whether to overwrite current rules or update them
+                      with the new rules.
+    :param use_conf: Whether to reload rules from config file.
     """
 
     init(use_conf=False)
@@ -135,30 +127,31 @@ def get_enforcer():
 def authorize(context, action, target, do_raise=True, exc=None):
     """Verifies that the action is valid on the target in this context.
 
-       :param context: cinder context
-       :param action: string representing the action to be checked
-           this should be colon separated for clarity.
-           i.e. ``compute:create_instance``,
-           ``compute:attach_volume``,
-           ``volume:attach_volume``
-       :param target: dictionary representing the object of the action
-           for object creation this should be a dictionary representing the
-           location of the object e.g. ``{'project_id': context.project_id}``
-       :param do_raise: if True (the default), raises PolicyNotAuthorized;
-           if False, returns False
-       :param exc: Class of the exception to raise if the check fails.
-                   Any remaining arguments passed to :meth:`authorize` (both
-                   positional and keyword arguments) will be passed to
-                   the exception class. If not specified,
-                   :class:`PolicyNotAuthorized` will be used.
+    :param context: cinder context
+    :param action: string representing the action to be checked
+                   this should be colon separated for clarity.
+                   i.e. ``compute:create_instance``,
+                   ``compute:attach_volume``,
+                   ``volume:attach_volume``
+    :param target: dictionary representing the object of the action for object
+                   creation this should be a dictionary representing the
+                   location of the object e.g.
+                   ``{'project_id': context.project_id}``
+    :param do_raise: if True (the default), raises PolicyNotAuthorized;
+                     if False, returns False
+    :param exc: Class of the exception to raise if the check fails.
+                Any remaining arguments passed to :meth:`authorize` (both
+                positional and keyword arguments) will be passed to
+                the exception class. If not specified,
+                :class:`PolicyNotAuthorized` will be used.
 
-       :raises cinder.exception.PolicyNotAuthorized: if verification fails
+    :raises cinder.exception.PolicyNotAuthorized: if verification fails
            and do_raise is True. Or if 'exc' is specified it will raise an
            exception of that type.
 
-       :return: returns a non-False value (not necessarily "True") if
-           authorized, and the exact value False if not authorized and
-           do_raise is False.
+    :return: returns a non-False value (not necessarily "True") if
+             authorized, and the exact value False if not authorized and
+             do_raise is False.
     """
     init()
     credentials = context.to_policy_values()
@@ -179,9 +172,7 @@ def authorize(context, action, target, do_raise=True, exc=None):
 
 
 def check_is_admin(context):
-    """Whether or not user is admin according to policy setting.
-
-    """
+    """Whether or not user is admin according to policy setting."""
     init()
     # the target is user-self
     credentials = context.to_policy_values()
diff --git a/cinder/test.py b/cinder/test.py
index bf388ce8f7d..80505ed34fa 100644
--- a/cinder/test.py
+++ b/cinder/test.py
@@ -28,7 +28,6 @@ import uuid
 import fixtures
 import mock
 from oslo_concurrency import lockutils
-from oslo_config import cfg
 from oslo_config import fixture as config_fixture
 from oslo_log.fixture import logging_error as log_fixture
 import oslo_messaging
@@ -39,7 +38,7 @@ from oslo_utils import timeutils
 import six
 import testtools
 
-from cinder.common import config  # noqa Need to register global_opts
+from cinder.common import config
 from cinder import context
 from cinder import coordination
 from cinder.db import migration
@@ -54,7 +53,7 @@ from cinder.tests.unit import fake_notifier
 from cinder.volume import utils
 
 
-CONF = cfg.CONF
+CONF = config.CONF
 
 _DB_CACHE = None
 SESSION_CONFIGURED = False
diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json
index be9c5a5c070..8c1e0905fa1 100644
--- a/cinder/tests/unit/policy.json
+++ b/cinder/tests/unit/policy.json
@@ -61,18 +61,6 @@
     "backup:get_all": "",
     "backup:restore": "",
 
-    "consistencygroup:create" : "",
-    "consistencygroup:delete": "",
-    "consistencygroup:update": "",
-    "consistencygroup:get": "",
-    "consistencygroup:get_all": "",
-
-    "consistencygroup:create_cgsnapshot" : "",
-    "consistencygroup:delete_cgsnapshot": "",
-    "consistencygroup:get_cgsnapshot": "",
-    "consistencygroup:get_all_cgsnapshots": "",
-
-
     "group:delete": "",
     "group:update": "",
     "group:get": "",
diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json
deleted file mode 100644
index c1f95d4d768..00000000000
--- a/etc/cinder/policy.json
+++ /dev/null
@@ -1,14 +0,0 @@
-{
-
-    "consistencygroup:create" : "group:nobody",
-    "consistencygroup:delete": "group:nobody",
-    "consistencygroup:update": "group:nobody",
-    "consistencygroup:get": "group:nobody",
-    "consistencygroup:get_all": "group:nobody",
-
-    "consistencygroup:create_cgsnapshot" : "group:nobody",
-    "consistencygroup:delete_cgsnapshot": "group:nobody",
-    "consistencygroup:get_cgsnapshot": "group:nobody",
-    "consistencygroup:get_all_cgsnapshots": "group:nobody"
-
-}
diff --git a/releasenotes/notes/policy-in-code-226f71562ab28195.yaml b/releasenotes/notes/policy-in-code-226f71562ab28195.yaml
new file mode 100644
index 00000000000..b9382249822
--- /dev/null
+++ b/releasenotes/notes/policy-in-code-226f71562ab28195.yaml
@@ -0,0 +1,11 @@
+---
+features:
+  - |
+    Cinder now support policy in code, which means if users don't need to
+    modify any of the default policy rules, they do not need a policy file.
+    Users can modify/generate a `policy.yaml` file which will override specific
+    policy rules from their defaults.
+other:
+  - |
+    Default `policy.json` file is now removed as Cinder now uses default
+    policies. A policy file is only needed if overriding one of the defaults.
diff --git a/setup.cfg b/setup.cfg
index 62941d960e9..79efd5f622d 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -24,7 +24,6 @@ setup-hooks =
 data_files =
     etc/cinder =
         etc/cinder/api-paste.ini
-        etc/cinder/policy.json
         etc/cinder/rootwrap.conf
     etc/cinder/rootwrap.d = etc/cinder/rootwrap.d/*
 packages =