Merge "Remove deprecated ``enforce_secure_rbac`` option"

11 files changed, 77 insertions, 150 deletions

diff --git a/glance/api/policy.py b/glance/api/policy.py
index d92d4df6d..17043eb41 100644
--- a/glance/api/policy.py
+++ b/glance/api/policy.py
@@ -25,7 +25,6 @@ from oslo_policy import policy
 
 from glance.common import exception
 from glance.domain import proxy
-from glance.i18n import _LW
 from glance import policies
 
 
@@ -64,15 +63,6 @@ class Enforcer(policy.Enforcer):
         if suppress_deprecation_warnings:
             self.suppress_deprecation_warnings = True
         self.register_defaults(policies.list_rules())
-        if CONF.enforce_secure_rbac and CONF.oslo_policy.enforce_new_defaults:
-            LOG.warning(_LW(
-                "Deploying glance with secure RBAC personas enabled via "
-                "`glance-api.conf [DEFAULT] enforce_secure_rbac=True` and "
-                "`glance-api.conf [oslo_policy] enforce_new_defaults=True` "
-                "is marked as EXPERIMENTAL in Wallaby. The status of this "
-                "feature will graduate to SUPPORTED as glance adopts more "
-                "personas, specifically for system-scope."
-            ))
 
     def add_rules(self, rules):
         """Add new rules to the Rules object"""
diff --git a/glance/api/v2/image_members.py b/glance/api/v2/image_members.py
index 7c7b8d5cd..97f5708f3 100644
--- a/glance/api/v2/image_members.py
+++ b/glance/api/v2/image_members.py
@@ -82,7 +82,8 @@ class ImageMembersController(object):
         # NOTE(abhishekk): Ownership check only needs to performed while
         # adding new members to image
         owner = image.owner
-        if not CONF.enforce_secure_rbac and not context.is_admin:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope) and not context.is_admin:
             if ownership_check == 'create':
                 if owner is None or owner != context.owner:
                     message = _("You are not permitted to create image "
diff --git a/glance/api/v2/policy.py b/glance/api/v2/policy.py
index 8ad404647..bce036cb5 100644
--- a/glance/api/v2/policy.py
+++ b/glance/api/v2/policy.py
@@ -225,7 +225,8 @@ class ImageAPIPolicy(APIPolicyBase):
         self._enforce('delete_image_location')
         # TODO(danms): Remove this legacy fallback when secure RBAC
         # replaces the legacy policy.
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_is_image_mutable(self._context, self._image)
 
     def get_image_location(self):
@@ -247,7 +248,8 @@ class ImageAPIPolicy(APIPolicyBase):
                 raise
         if 'visibility' in self._target:
             self._enforce_visibility(self._target['visibility'])
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_admin_or_same_owner(self._context, self._target)
 
     def get_image(self):
@@ -260,14 +262,16 @@ class ImageAPIPolicy(APIPolicyBase):
         self._enforce('delete_image')
         # TODO(danms): Remove this legacy fallback when secure RBAC
         # replaces the legacy policy.
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_is_image_mutable(self._context, self._image)
 
     def upload_image(self):
         self._enforce('upload_image')
         # TODO(danms): Remove this legacy fallback when secure RBAC
         # replaces the legacy policy.
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_is_image_mutable(self._context, self._image)
 
     def download_image(self):
@@ -277,21 +281,24 @@ class ImageAPIPolicy(APIPolicyBase):
         self._enforce('modify_image')
         # TODO(danms): Remove this legacy fallback when secure RBAC
         # replaces the legacy policy.
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_is_image_mutable(self._context, self._image)
 
     def deactivate_image(self):
         self._enforce('deactivate')
         # TODO(danms): Remove this legacy fallback when secure RBAC
         # replaces the legacy policy.
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_is_image_mutable(self._context, self._image)
 
     def reactivate_image(self):
         self._enforce('reactivate')
         # TODO(danms): Remove this legacy fallback when secure RBAC
         # replaces the legacy policy.
-        if not CONF.enforce_secure_rbac:
+        if not (CONF.oslo_policy.enforce_new_defaults or
+                CONF.oslo_policy.enforce_scope):
             check_is_image_mutable(self._context, self._image)
 
     def copy_image(self):
diff --git a/glance/cmd/api.py b/glance/cmd/api.py
index fa5b38d1e..90511b510 100644
--- a/glance/cmd/api.py
+++ b/glance/cmd/api.py
@@ -108,14 +108,6 @@ def main():
                 host=CONF.bind_host
             )
 
-        if CONF.enforce_secure_rbac != CONF.oslo_policy.enforce_new_defaults:
-            fail_message = (
-                "[DEFAULT] enforce_secure_rbac does not match "
-                "[oslo_policy] enforce_new_defaults. Please set both to "
-                "True to enable secure RBAC personas. Otherwise, make sure "
-                "both are False.")
-            raise exception.ServerError(fail_message)
-
         # NOTE(danms): Configure system-wide threading model to use eventlet
         glance.async_.set_threadpool_model('eventlet')
 
diff --git a/glance/common/config.py b/glance/common/config.py
index cff15ab1d..b11838005 100644
--- a/glance/common/config.py
+++ b/glance/common/config.py
@@ -593,30 +593,6 @@ Related options:
 
     Related options:
         * [DEFAULT]/node_staging_uri""")),
-    cfg.BoolOpt('enforce_secure_rbac', default=True,
-                deprecated_for_removal=True,
-                deprecated_reason=_("""
-This option has been introduced to require operators to opt into enforcing
-authorization based on common RBAC personas, which is EXPERIMENTAL as of the
-Wallaby release. This behavior will be the default and STABLE in a future
-release, allowing this option to be removed.
-"""),
-                deprecated_since='Wallaby',
-                help=_("""
-Enforce API access based on common persona definitions used across OpenStack.
-Enabling this option formalizes project-specific read/write operations, like
-creating private images or updating the status of shared image, behind the
-`member` role. It also formalizes a read-only variant useful for
-project-specific API operations, like listing private images in a project,
-behind the `reader` role.
-
-Operators should take an opportunity to understand glance's new image policies,
-audit assignments in their deployment, and update permissions using the default
-roles in keystone (e.g., `admin`, `member`, and `reader`).
-
-Related options:
-    * [oslo_policy]/enforce_new_defaults
-""")),
     cfg.StrOpt('worker_self_reference_url',
                default=None,
                help=_("""
diff --git a/glance/common/wsgi_app.py b/glance/common/wsgi_app.py
index 2a3168e0d..ab1f12170 100644
--- a/glance/common/wsgi_app.py
+++ b/glance/common/wsgi_app.py
@@ -22,7 +22,6 @@ import osprofiler.initializer
 from glance.api import common
 import glance.async_
 from glance.common import config
-from glance.common import exception
 from glance.common import store_utils
 from glance import housekeeping
 from glance.i18n import _
@@ -80,16 +79,6 @@ def _setup_os_profiler():
                                               host=CONF.bind_host)
 
 
-def _validate_policy_enforcement_configuration():
-    if CONF.enforce_secure_rbac != CONF.oslo_policy.enforce_new_defaults:
-        fail_message = (
-            "[DEFAULT] enforce_secure_rbac does not match "
-            "[oslo_policy] enforce_new_defaults. Please set both to "
-            "True to enable secure RBAC personas. Otherwise, make sure "
-            "both are False.")
-        raise exception.ServerError(fail_message)
-
-
 def drain_workers():
     # NOTE(danms): If there are any other named pools that we need to
     # drain before exit, they should be in this list.
@@ -155,5 +144,4 @@ def init_app():
     run_staging_cleanup()
 
     _setup_os_profiler()
-    _validate_policy_enforcement_configuration()
     return config.load_paste_app('glance-api')
diff --git a/glance/tests/functional/__init__.py b/glance/tests/functional/__init__.py
index a17c67c39..2573c3e22 100644
--- a/glance/tests/functional/__init__.py
+++ b/glance/tests/functional/__init__.py
@@ -408,9 +408,7 @@ class ApiServer(Server):
         self.image_location_quota = 2
         self.disable_path = None
 
-        secure_rbac = bool(os.getenv('OS_GLANCE_TEST_RBAC_DEFAULTS'))
-        self.enforce_secure_rbac = secure_rbac
-        self.enforce_new_defaults = secure_rbac
+        self.enforce_new_defaults = True
 
         self.needs_database = True
         default_sql_connection = SQLITE_CONN_TEMPLATE % self.test_dir
@@ -456,7 +454,6 @@ image_location_quota=%(image_location_quota)s
 location_strategy=%(location_strategy)s
 allow_additional_image_properties = True
 node_staging_uri=%(node_staging_uri)s
-enforce_secure_rbac=%(enforce_secure_rbac)s
 [oslo_policy]
 policy_file = %(policy_file)s
 policy_default_rule = %(policy_default_rule)s
@@ -585,7 +582,6 @@ class ApiServerForMultipleBackend(Server):
         self.image_location_quota = 2
         self.disable_path = None
 
-        self.enforce_secure_rbac = True
         self.enforce_new_defaults = True
 
         self.needs_database = True
@@ -629,7 +625,6 @@ image_location_quota=%(image_location_quota)s
 location_strategy=%(location_strategy)s
 allow_additional_image_properties = True
 enabled_backends=file1:file,file2:file,file3:file
-enforce_secure_rbac=%(enforce_secure_rbac)s
 [oslo_policy]
 policy_file = %(policy_file)s
 policy_default_rule = %(policy_default_rule)s
@@ -1620,9 +1615,9 @@ class SynchronousAPIBase(test_utils.BaseTestCase):
 
         self.api = config.load_paste_app(root_app,
                                          conf_file=self.paste_config)
-        secure_rbac = bool(os.getenv('OS_GLANCE_TEST_RBAC_DEFAULTS'))
-        self.config(enforce_secure_rbac=secure_rbac)
-        self.config(enforce_new_defaults=secure_rbac,
+        self.config(enforce_new_defaults=True,
+                    group='oslo_policy')
+        self.config(enforce_scope=True,
                     group='oslo_policy')
 
     def _headers(self, custom_headers=None):
diff --git a/glance/tests/unit/common/test_wsgi_app.py b/glance/tests/unit/common/test_wsgi_app.py
index 5b7cb8212..87afc40d7 100644
--- a/glance/tests/unit/common/test_wsgi_app.py
+++ b/glance/tests/unit/common/test_wsgi_app.py
@@ -19,7 +19,6 @@ from unittest import mock
 from glance.api import common
 from glance.api.v2 import cached_images
 import glance.async_
-from glance.common import exception
 from glance.common import wsgi_app
 from glance.tests import utils as test_utils
 
@@ -91,40 +90,6 @@ class TestWsgiAppInit(test_utils.BaseTestCase):
             wsgi_app.drain_workers()
             self.assertIsNone(cached_images.WORKER)
 
-    @mock.patch('glance.common.config.load_paste_app')
-    @mock.patch('glance.async_.set_threadpool_model')
-    @mock.patch('glance.common.wsgi_app._get_config_files')
-    def test_policy_enforcement_kills_service_if_misconfigured(
-            self, mock_load_app, mock_set, mock_config_files):
-        self.config(enforce_new_defaults=True, group='oslo_policy')
-        self.config(enforce_secure_rbac=False)
-        self.assertRaises(exception.ServerError, wsgi_app.init_app)
-
-        self.config(enforce_new_defaults=False, group='oslo_policy')
-        self.config(enforce_secure_rbac=True)
-        self.assertRaises(exception.ServerError, wsgi_app.init_app)
-
-    @mock.patch('glance.common.config.load_paste_app')
-    @mock.patch('glance.async_.set_threadpool_model')
-    @mock.patch('glance.common.wsgi_app._get_config_files')
-    def test_policy_enforcement_valid_truthy_configuration(
-            self, mock_load_app, mock_set, mock_config_files):
-        self.config(enforce_new_defaults=True, group='oslo_policy')
-        self.config(enforce_secure_rbac=True)
-        self.assertTrue(wsgi_app.init_app())
-
-    @mock.patch('glance.common.config.load_paste_app')
-    @mock.patch('glance.async_.set_threadpool_model')
-    @mock.patch('glance.common.wsgi_app._get_config_files')
-    def test_policy_enforcement_valid_falsy_configuration(
-            self, mock_load_app, mock_set, mock_config_files):
-        # This is effectively testing the default values, but we're doing that
-        # to make sure nothing bad happens at runtime in the default case when
-        # validating policy enforcement configuration.
-        self.config(enforce_new_defaults=False, group='oslo_policy')
-        self.config(enforce_secure_rbac=False)
-        self.assertTrue(wsgi_app.init_app())
-
     @mock.patch('glance.async_._THREADPOOL_MODEL', new=None)
     @mock.patch('glance.common.config.load_paste_app')
     @mock.patch('glance.common.wsgi_app._get_config_files')
diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py
index 9f26d0e29..b37268592 100644
--- a/glance/tests/unit/test_policy.py
+++ b/glance/tests/unit/test_policy.py
@@ -394,29 +394,6 @@ class TestPolicyEnforcer(base.IsolatedUnitTest):
         enforcer.check(context, 'foo', {})
         mock_enforcer.assert_called_once_with('foo', {}, context)
 
-    def test_ensure_experimental_warning_is_logged_for_secure_rbac(self):
-        self.config(enforce_new_defaults=True, group='oslo_policy')
-        self.config(enforce_secure_rbac=True)
-        expected_log_string = (
-            "Deploying glance with secure RBAC personas enabled via "
-            "`glance-api.conf [DEFAULT] enforce_secure_rbac=True` and "
-            "`glance-api.conf [oslo_policy] enforce_new_defaults=True` "
-            "is marked as EXPERIMENTAL in Wallaby. The status of this "
-            "feature will graduate to SUPPORTED as glance adopts more "
-            "personas, specifically for system-scope."
-        )
-        with mock.patch.object(glance.api.policy, 'LOG') as mock_log:
-            glance.api.policy.Enforcer(
-                suppress_deprecation_warnings=True)
-            mock_log.warning.assert_called_once_with(expected_log_string)
-
-    def test_ensure_experimental_warning_is_not_logged_for_legacy_rbac(self):
-        self.config(enforce_new_defaults=False, group='oslo_policy')
-        with mock.patch.object(glance.api.policy, 'LOG') as mock_log:
-            glance.api.policy.Enforcer(
-                suppress_deprecation_warnings=True)
-            mock_log.warning.assert_not_called()
-
 
 class TestPolicyEnforcerNoFile(base.IsolatedUnitTest):
 
diff --git a/glance/tests/unit/v2/test_v2_policy.py b/glance/tests/unit/v2/test_v2_policy.py
index ae04c7de8..18b65e66c 100644
--- a/glance/tests/unit/v2/test_v2_policy.py
+++ b/glance/tests/unit/v2/test_v2_policy.py
@@ -142,7 +142,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       mock.ANY)
 
     def test_delete_locations_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
 
         # As admin, image is mutable even if owner does not match
         self.context.is_admin = True
@@ -166,8 +167,10 @@ class APIImagePolicy(APIPolicyBase):
             self.policy.delete_locations()
             m.assert_called_once_with(self.context, self.image)
 
-        # Make sure we are not checking it if enforce_secure_rbac=True
-        self.config(enforce_secure_rbac=True)
+        # Make sure we are not checking it if enforce_new_defaults=True and
+        # enforce_scope=True
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_is_image_mutable') as m:
             self.policy.delete_locations()
             self.assertFalse(m.called)
@@ -235,7 +238,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       generic_target)
 
     def test_add_image_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
 
         self.context.is_admin = False
         self.policy = policy.ImageAPIPolicy(self.context, {'owner': 'else'},
@@ -252,7 +256,8 @@ class APIImagePolicy(APIPolicyBase):
         # Make sure we are not calling the legacy handler if
         # secure_rbac is being used. We won't fail the check because
         # our enforcer is a mock, just make sure we don't call that handler.
-        self.config(enforce_secure_rbac=True)
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_admin_or_same_owner') as m:
             self.policy.add_image()
             m.assert_not_called()
@@ -287,8 +292,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       mock.ANY)
 
     def test_delete_image_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
-
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
         # As admin, image is mutable even if owner does not match
         self.context.is_admin = True
         self.context.owner = 'someuser'
@@ -311,8 +316,10 @@ class APIImagePolicy(APIPolicyBase):
             self.policy.delete_image()
             m.assert_called_once_with(self.context, self.image)
 
-        # Make sure we are not checking it if enforce_secure_rbac=True
-        self.config(enforce_secure_rbac=True)
+        # Make sure we are not checking it if enforce_new_defaults=True and
+        # enforce_scope=True
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_is_image_mutable') as m:
             self.policy.delete_image()
             self.assertFalse(m.called)
@@ -324,7 +331,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       mock.ANY)
 
     def test_upload_image_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
 
         # As admin, image is mutable even if owner does not match
         self.context.is_admin = True
@@ -348,8 +356,10 @@ class APIImagePolicy(APIPolicyBase):
             self.policy.upload_image()
             m.assert_called_once_with(self.context, self.image)
 
-        # Make sure we are not checking it if enforce_secure_rbac=True
-        self.config(enforce_secure_rbac=True)
+        # Make sure we are not checking it if enforce_new_defaults=True and
+        # enforce_scope=True
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_is_image_mutable') as m:
             self.policy.upload_image()
             self.assertFalse(m.called)
@@ -367,7 +377,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       mock.ANY)
 
     def test_modify_image_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
 
         # As admin, image is mutable even if owner does not match
         self.context.is_admin = True
@@ -391,8 +402,10 @@ class APIImagePolicy(APIPolicyBase):
             self.policy.modify_image()
             m.assert_called_once_with(self.context, self.image)
 
-        # Make sure we are not checking it if enforce_secure_rbac=True
-        self.config(enforce_secure_rbac=True)
+        # Make sure we are not checking it if enforce_new_defaults=True and
+        # enforce_scope=True
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_is_image_mutable') as m:
             self.policy.modify_image()
             self.assertFalse(m.called)
@@ -404,7 +417,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       mock.ANY)
 
     def test_deactivate_image_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
 
         # As admin, image is mutable even if owner does not match
         self.context.is_admin = True
@@ -428,8 +442,10 @@ class APIImagePolicy(APIPolicyBase):
             self.policy.deactivate_image()
             m.assert_called_once_with(self.context, self.image)
 
-        # Make sure we are not checking it if enforce_secure_rbac=True
-        self.config(enforce_secure_rbac=True)
+        # Make sure we are not checking it if enforce_new_defaults=Truei and
+        # enforce_scope=True
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_is_image_mutable') as m:
             self.policy.deactivate_image()
             self.assertFalse(m.called)
@@ -441,7 +457,8 @@ class APIImagePolicy(APIPolicyBase):
                                                       mock.ANY)
 
     def test_reactivate_image_falls_back_to_legacy(self):
-        self.config(enforce_secure_rbac=False)
+        self.config(enforce_new_defaults=False, group='oslo_policy')
+        self.config(enforce_scope=False, group='oslo_policy')
 
         # As admin, image is mutable even if owner does not match
         self.context.is_admin = True
@@ -465,8 +482,10 @@ class APIImagePolicy(APIPolicyBase):
             self.policy.reactivate_image()
             m.assert_called_once_with(self.context, self.image)
 
-        # Make sure we are not checking it if enforce_secure_rbac=True
-        self.config(enforce_secure_rbac=True)
+        # Make sure we are not checking it if enforce_new_defaults=True and
+        # enforce_scope=True
+        self.config(enforce_new_defaults=True, group='oslo_policy')
+        self.config(enforce_scope=True, group='oslo_policy')
         with mock.patch('glance.api.v2.policy.check_is_image_mutable') as m:
             self.policy.reactivate_image()
             self.assertFalse(m.called)
diff --git a/releasenotes/notes/remove-enforce-secure-rbac-ec9a0249870460c2.yaml b/releasenotes/notes/remove-enforce-secure-rbac-ec9a0249870460c2.yaml
new file mode 100644
index 000000000..f40f46ed5
--- /dev/null
+++ b/releasenotes/notes/remove-enforce-secure-rbac-ec9a0249870460c2.yaml
@@ -0,0 +1,17 @@
+---
+upgrade:
+  - |
+    As per the revised SRBAC community goals, glance service is switching to
+    new defaults by default in Antelope cycle, hence removing the deprecated
+    ``enforce_secure_rbac`` option which is no longer needed.
+    The ``enforce_secure_rbac`` option was introduced EXPERIMENTAL in Wallaby
+    release for operators to opt into enforcing authorization based on common
+    RBAC personas.
+
+    Now operator can control the scope and new defaults flag with the below
+    config options in
+    ``glance-api.conf`` file::
+
+      [oslo_policy]
+      enforce_new_defaults=True
+      enforce_scope=True
\ No newline at end of file