Remove deprecated enforce_secure_rbac option

As per the revised SRBAC community goals, glance service is now
switching to new defaults by default 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.

Related blueprint secure-rbac

Change-Id: I273527c85d30c1c09c086c73c892aaa6d127df6b

Author: PranaliDeore

glance/api/policy.py

@@ -25,7 +25,6 @@
 
 from glance.common import exception
 from glance.domain import proxy
-from glance.i18n import _LW
 from glance import policies
 
 
@@ -64,15 +63,6 @@ def __init__(self, suppress_deprecation_warnings=False):
         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"""

glance/api/v2/image_members.py

@@ -82,7 +82,8 @@ def _check_visibility_and_ownership(self, context, image,
         # 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 "

glance/api/v2/policy.py

@@ -225,7 +225,8 @@ def delete_locations(self):
         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 @@ def add_image(self):
                 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 @@ def delete_image(self):
         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 @@ def modify_image(self):
         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):

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')
 

glance/common/config.py

@@ -593,30 +593,6 @@
 
     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=_("""

glance/common/wsgi_app.py

@@ -22,7 +22,6 @@
 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')

glance/tests/functional/__init__.py

@@ -408,9 +408,7 @@ def __init__(self, test_dir, port, policy_file, delayed_delete=False,
         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 @@ def __init__(self, test_dir, port, policy_file, delayed_delete=False,
 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 @@ def __init__(self, test_dir, port, policy_file, delayed_delete=False,
         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 @@ def __init__(self, test_dir, port, policy_file, delayed_delete=False,
 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 @@ def start_server(self, enable_cache=True):
 
         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):

glance/tests/unit/common/test_wsgi_app.py

@@ -19,7 +19,6 @@
 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 @@ def test_drain_workers_no_cache(self):
             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')

glance/tests/unit/test_policy.py

@@ -394,29 +394,6 @@ def test_ensure_context_object_is_passed_to_policy_enforcement(self):
         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):