openstack/glance
Code Issues Proposed changes

Remove deprecated `enforce_secure_rbac` option

Browse Source 
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
This commit is contained in:
Pranali Deore 2023-02-10 13:48:13 +00:00
parent 8c04d19e88
commit b20cc91e6f
11 changed files with 77 additions and 150 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
glance/api/policy.py
View File

@ -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"""

3
glance/api/v2/image_members.py
View File

@ -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 "

21
glance/api/v2/policy.py
View File

@ -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):

8
glance/cmd/api.py
View File

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

24
glance/common/config.py
View File

@ -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=_("""

12
glance/common/wsgi_app.py
View File

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

13
glance/tests/functional/__init__.py
View File

@ -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):

35
glance/tests/unit/common/test_wsgi_app.py
View File

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

23
glance/tests/unit/test_policy.py
View File

@ -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):

61
glance/tests/unit/v2/test_v2_policy.py
View File

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

17
releasenotes/notes/remove-enforce-secure-rbac-ec9a0249870460c2.yaml Normal file
View File

@ -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