From 084212711672ccc9bc1a6c0baa32a5a3e3112442 Mon Sep 17 00:00:00 2001 From: Alan Bishop Date: Thu, 2 Sep 2021 09:16:13 -0700 Subject: [PATCH] Implement user visible extra specs The following volume type extra specs are now visible to regular users (non-administrators): - RESKEY:availability_zones - multiattach - replication_enabled The list is defined in the code, and is not configurable. Regular users may view these user visible specs, and use them as a filter when listing volume types, but access is essentially read-only. Only cloud administrators are authorized to create or modify extra specs, including the user visible ones. The feature works by relaxing a few policies that were previously admin-only, and adds a new volume_extension:types_extra_specs:read_sensitive policy that limits access to all other (non-user visible) specs so that only cloud administrators can see them. DocImpact Implements: bp expose-user-visible-extra-specs Change-Id: I5434ea4199cce8158b75771fb6127be001baf328 --- cinder/api/contrib/types_extra_specs.py | 11 +- cinder/api/v3/types.py | 13 + cinder/api/v3/views/types.py | 17 +- cinder/policies/type_extra_specs.py | 38 ++- cinder/policies/volume_type.py | 2 +- .../api/contrib/test_types_extra_specs.py | 74 +++-- cinder/tests/unit/api/v3/test_types.py | 56 ++-- cinder/tests/unit/api/v3/test_types_orig.py | 263 +++++------------- .../admin/blockstorage-manage-volumes.rst | 1 + .../blockstorage-user-visible-extra-specs.rst | 162 +++++++++++ doc/source/admin/generalized_filters.rst | 2 +- doc/source/admin/index.rst | 1 + etc/cinder/resource_filters.json | 2 +- ...-visible-extra-specs-6cf7e49c6be57a01.yaml | 16 ++ 14 files changed, 397 insertions(+), 261 deletions(-) create mode 100644 doc/source/admin/blockstorage-user-visible-extra-specs.rst create mode 100644 releasenotes/notes/user-visible-extra-specs-6cf7e49c6be57a01.yaml diff --git a/cinder/api/contrib/types_extra_specs.py b/cinder/api/contrib/types_extra_specs.py index 45233e5dda2..a7790ee10c8 100644 --- a/cinder/api/contrib/types_extra_specs.py +++ b/cinder/api/contrib/types_extra_specs.py @@ -38,9 +38,14 @@ class VolumeTypeExtraSpecsController(wsgi.Controller): def _get_extra_specs(self, context, type_id): extra_specs = db.volume_type_extra_specs_get(context, type_id) - specs_dict = {} - for key, value in extra_specs.items(): - specs_dict[key] = value + if context.authorize(policy.READ_SENSITIVE_POLICY, fatal=False): + specs_dict = extra_specs + else: + # Limit the response to contain only user visible specs. + specs_dict = {} + for uv_spec in policy.USER_VISIBLE_EXTRA_SPECS: + if uv_spec in extra_specs: + specs_dict[uv_spec] = extra_specs[uv_spec] return dict(extra_specs=specs_dict) def _check_type(self, context, type_id): diff --git a/cinder/api/v3/types.py b/cinder/api/v3/types.py index d6579dd9d30..abd160e56e4 100644 --- a/cinder/api/v3/types.py +++ b/cinder/api/v3/types.py @@ -26,6 +26,7 @@ from cinder.api.openstack import wsgi from cinder.api.v3.views import types as views_types from cinder import exception from cinder.i18n import _ +from cinder.policies import type_extra_specs as extra_specs_policy from cinder.policies import volume_type as type_policy from cinder.volume import volume_types @@ -100,6 +101,18 @@ class VolumeTypesController(wsgi.Controller): except (ValueError, SyntaxError): LOG.debug('Could not evaluate "extra_specs" %s, assuming ' 'dictionary string.', filters['extra_specs']) + + # Do not allow sensitive extra specs to be used in a filter if + # the context only allows access to user visible extra specs. + # Removing the filter would yield inaccurate results, so an + # empty result is returned because as far as an unauthorized + # user goes, the list of volume-types meeting their filtering + # criteria is empty. + if not context.authorize(extra_specs_policy.READ_SENSITIVE_POLICY, + fatal=False): + for k in filters['extra_specs'].keys(): + if k not in extra_specs_policy.USER_VISIBLE_EXTRA_SPECS: + return [] limited_types = volume_types.get_all_types(context, filters=filters, marker=marker, limit=limit, diff --git a/cinder/api/v3/views/types.py b/cinder/api/v3/views/types.py index e9064b7b686..2d1c80cc5ff 100644 --- a/cinder/api/v3/views/types.py +++ b/cinder/api/v3/views/types.py @@ -15,6 +15,7 @@ # under the License. from cinder.api import common +from cinder.policies import type_extra_specs as extra_specs_policy from cinder.policies import volume_type as policy @@ -27,10 +28,24 @@ class ViewBuilder(common.ViewBuilder): name=volume_type.get('name'), is_public=volume_type.get('is_public'), description=volume_type.get('description')) + if context.authorize(policy.EXTRA_SPEC_POLICY, fatal=False): - trimmed['extra_specs'] = volume_type.get('extra_specs') + extra_specs = volume_type.get('extra_specs', {}) + if context.authorize(extra_specs_policy.READ_SENSITIVE_POLICY, + fatal=False): + trimmed_specs = extra_specs + else: + # Limit the response to contain only user visible specs. + trimmed_specs = {} + for uv_spec in extra_specs_policy.USER_VISIBLE_EXTRA_SPECS: + if uv_spec in extra_specs: + trimmed_specs[uv_spec] = extra_specs[uv_spec] + + trimmed['extra_specs'] = trimmed_specs + if context.authorize(policy.QOS_POLICY, fatal=False): trimmed['qos_specs_id'] = volume_type.get('qos_specs_id') + return trimmed if brief else dict(volume_type=trimmed) def index(self, request, volume_types): diff --git a/cinder/policies/type_extra_specs.py b/cinder/policies/type_extra_specs.py index a9eacdcd21f..206d20c1710 100644 --- a/cinder/policies/type_extra_specs.py +++ b/cinder/policies/type_extra_specs.py @@ -18,17 +18,23 @@ from oslo_policy import policy from cinder.policies import base +USER_VISIBLE_EXTRA_SPECS = ( + "RESKEY:availability_zones", + "multiattach", + "replication_enabled", +) + CREATE_POLICY = "volume_extension:types_extra_specs:create" DELETE_POLICY = "volume_extension:types_extra_specs:delete" GET_ALL_POLICY = "volume_extension:types_extra_specs:index" GET_POLICY = "volume_extension:types_extra_specs:show" +READ_SENSITIVE_POLICY = "volume_extension:types_extra_specs:read_sensitive" UPDATE_POLICY = "volume_extension:types_extra_specs:update" - type_extra_specs_policies = [ policy.DocumentedRuleDefault( name=GET_ALL_POLICY, - check_str=base.RULE_ADMIN_API, + check_str="", description="List type extra specs.", operations=[ { @@ -48,7 +54,7 @@ type_extra_specs_policies = [ ]), policy.DocumentedRuleDefault( name=GET_POLICY, - check_str=base.RULE_ADMIN_API, + check_str="", description="Show one specified type extra specs.", operations=[ { @@ -56,6 +62,32 @@ type_extra_specs_policies = [ 'path': '/types/{type_id}/extra_specs/{extra_spec_key}' } ]), + policy.DocumentedRuleDefault( + name=READ_SENSITIVE_POLICY, + check_str=base.RULE_ADMIN_API, + description=("Include extra_specs fields that may reveal sensitive " + "information about the deployment that should not be " + "exposed to end users in various volume-type responses " + "that show extra_specs. The ability to make these calls " + "is governed by other policies."), + operations=[ + { + 'method': 'GET', + 'path': '/types' + }, + { + 'method': 'GET', + 'path': '/types/{type_id}' + }, + { + 'method': 'GET', + 'path': '/types/{type_id}/extra_specs' + }, + { + 'method': 'GET', + 'path': '/types/{type_id}/extra_specs/{extra_spec_key}' + } + ]), policy.DocumentedRuleDefault( name=UPDATE_POLICY, check_str=base.RULE_ADMIN_API, diff --git a/cinder/policies/volume_type.py b/cinder/policies/volume_type.py index f4454ff8453..f7c1b5ea3b3 100644 --- a/cinder/policies/volume_type.py +++ b/cinder/policies/volume_type.py @@ -147,7 +147,7 @@ volume_type_policies = [ ]), policy.DocumentedRuleDefault( name=EXTRA_SPEC_POLICY, - check_str=base.RULE_ADMIN_API, + check_str="", description="List or show volume type with access type extra " "specs attribute.", operations=[ diff --git a/cinder/tests/unit/api/contrib/test_types_extra_specs.py b/cinder/tests/unit/api/contrib/test_types_extra_specs.py index 755fce70573..bcb43df392a 100644 --- a/cinder/tests/unit/api/contrib/test_types_extra_specs.py +++ b/cinder/tests/unit/api/contrib/test_types_extra_specs.py @@ -18,54 +18,51 @@ from unittest import mock import ddt -from oslo_config import cfg from oslo_utils import timeutils import webob from cinder.api.contrib import types_extra_specs from cinder import exception from cinder.image import glance as image_store +from cinder.policies import type_extra_specs as extra_specs_policy from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import test import cinder.wsgi -CONF = cfg.CONF +user_visible_extra_specs = { + k: '%s_value' % k for k in extra_specs_policy.USER_VISIBLE_EXTRA_SPECS +} + +volume_type_extra_specs = { + **user_visible_extra_specs, + "key1": "value1", + "key2": "value2", + "key3": "value3", + "key4": "value4", + "key5": "value5", +} def return_create_volume_type_extra_specs(context, volume_type_id, extra_specs): - return fake_volume_type_extra_specs() + return volume_type_extra_specs def return_volume_type_extra_specs(context, volume_type_id): - return fake_volume_type_extra_specs() + return volume_type_extra_specs def return_volume_type(context, volume_type_id, expected_fields=None): - specs = {"key1": "value1", - "key2": "value2", - "key3": "value3", - "key4": "value4", - "key5": "value5"} return dict(id=id, name='vol_type_%s' % id, description='vol_type_desc_%s' % id, - extra_specs=specs, + extra_specs=volume_type_extra_specs, created_at=timeutils.utcnow(), updated_at=timeutils.utcnow(), deleted_at=timeutils.utcnow()) -def fake_volume_type_extra_specs(): - specs = {"key1": "value1", - "key2": "value2", - "key3": "value3", - "key4": "value4", - "key5": "value5"} - return specs - - @ddt.ddt class VolumeTypesExtraSpecsTest(test.TestCase): @@ -77,34 +74,51 @@ class VolumeTypesExtraSpecsTest(test.TestCase): fake.PROJECT_ID, fake.VOLUME_TYPE_ID) self.controller = types_extra_specs.VolumeTypeExtraSpecsController() - """to reset notifier drivers left over from other api/contrib tests""" - - def test_index(self): + @ddt.data( + {'is_admin': True, 'visible_specs': volume_type_extra_specs}, + {'is_admin': False, 'visible_specs': user_visible_extra_specs}, + ) + @ddt.unpack + def test_index(self, is_admin, visible_specs): self.mock_object(cinder.db, 'volume_type_extra_specs_get', return_volume_type_extra_specs) - req = fakes.HTTPRequest.blank(self.api_path) + req = fakes.HTTPRequest.blank(self.api_path, + use_admin_context=is_admin) res_dict = self.controller.index(req, fake.VOLUME_TYPE_ID) - self.assertEqual('value1', res_dict['extra_specs']['key1']) + self.assertEqual(visible_specs, res_dict['extra_specs']) def test_index_no_data(self): self.mock_object(cinder.db, 'volume_type_extra_specs_get', return_value={}) - req = fakes.HTTPRequest.blank(self.api_path) + req = fakes.HTTPRequest.blank(self.api_path, + use_admin_context=True) res_dict = self.controller.index(req, fake.VOLUME_TYPE_ID) self.assertEqual(0, len(res_dict['extra_specs'])) - def test_show(self): + @ddt.data( + {'is_admin': True, 'spec': 'key5', 'is_sensitive': True}, + {'is_admin': False, 'spec': 'key5', 'is_sensitive': True}, + # multiattach is a user visible extra spec (not sensitve) + {'is_admin': True, 'spec': 'multiattach', 'is_sensitive': False}, + {'is_admin': False, 'spec': 'multiattach', 'is_sensitive': False}, + ) + @ddt.unpack + def test_show(self, is_admin, spec, is_sensitive): self.mock_object(cinder.db, 'volume_type_extra_specs_get', return_volume_type_extra_specs) - req = fakes.HTTPRequest.blank(self.api_path + '/key5') - res_dict = self.controller.show(req, fake.VOLUME_TYPE_ID, 'key5') - - self.assertEqual('value5', res_dict['key5']) + req = fakes.HTTPRequest.blank(self.api_path + '/' + spec, + use_admin_context=is_admin) + if is_sensitive and not is_admin: + self.assertRaises(exception.VolumeTypeExtraSpecsNotFound, + self.controller.show, req, fake.VOLUME_ID, spec) + else: + res_dict = self.controller.show(req, fake.VOLUME_TYPE_ID, spec) + self.assertEqual(volume_type_extra_specs[spec], res_dict[spec]) def test_show_spec_not_found(self): self.mock_object(cinder.db, 'volume_type_extra_specs_get', diff --git a/cinder/tests/unit/api/v3/test_types.py b/cinder/tests/unit/api/v3/test_types.py index 0fd58957787..7d18cdfe282 100644 --- a/cinder/tests/unit/api/v3/test_types.py +++ b/cinder/tests/unit/api/v3/test_types.py @@ -58,40 +58,50 @@ class VolumeTypesApiTest(test.TestCase): self.type3.destroy() def test_volume_types_index_with_extra_specs(self): - req = fakes.HTTPRequest.blank( - '/v3/%s/types?extra_specs={"key1":"value1"}' % fake.PROJECT_ID, - use_admin_context=False) - req.api_version_request = mv.get_api_version(mv.get_prior_version( - mv.SUPPORT_VOLUME_TYPE_FILTER)) - res_dict = self.controller.index(req) + def _get_volume_types(extra_specs, + use_admin_context=True, + microversion=mv.SUPPORT_VOLUME_TYPE_FILTER): + req = fakes.HTTPRequest.blank( + '/v3/%s/types?extra_specs=%s' % (fake.PROJECT_ID, extra_specs), + use_admin_context=use_admin_context) + req.api_version_request = mv.get_api_version(microversion) + res_dict = self.controller.index(req) + return res_dict['volume_types'] # since __DEFAULT__ type always exists, total number of volume types # is total_types_created + 1. In this case it's 4 - self.assertEqual(4, len(res_dict['volume_types'])) + volume_types = _get_volume_types('{"key1":"value1"}', + use_admin_context=False, + microversion=mv.get_prior_version( + mv.SUPPORT_VOLUME_TYPE_FILTER)) + self.assertEqual(4, len(volume_types)) # Test filter volume type with extra specs - req = fakes.HTTPRequest.blank( - '/v3/%s/types?extra_specs={"key1":"value1"}' % fake.PROJECT_ID, - use_admin_context=True) - req.api_version_request = mv.get_api_version( - mv.SUPPORT_VOLUME_TYPE_FILTER) - res_dict = self.controller.index(req) - self.assertEqual(1, len(res_dict['volume_types'])) + volume_types = _get_volume_types('{"key1":"value1"}') + self.assertEqual(1, len(volume_types)) self.assertDictEqual({'key1': 'value1', 'RESKEY:availability_zones': 'az1,az2'}, - res_dict['volume_types'][0]['extra_specs']) + volume_types[0]['extra_specs']) # Test filter volume type with 'availability_zones' - req = fakes.HTTPRequest.blank( - '/v3/%s/types?extra_specs={"RESKEY:availability_zones":"az1"}' - % fake.PROJECT_ID, use_admin_context=True) - req.api_version_request = mv.get_api_version( - mv.SUPPORT_VOLUME_TYPE_FILTER) - res_dict = self.controller.index(req) - self.assertEqual(2, len(res_dict['volume_types'])) + volume_types = _get_volume_types('{"RESKEY:availability_zones":"az1"}') + self.assertEqual(2, len(volume_types)) self.assertEqual( ['volume_type1', 'volume_type2'], - sorted([az['name'] for az in res_dict['volume_types']])) + sorted([az['name'] for az in volume_types])) + + # Test ability for non-admin to filter with user visible extra specs + volume_types = _get_volume_types('{"RESKEY:availability_zones":"az1"}', + use_admin_context=False) + self.assertEqual(2, len(volume_types)) + self.assertEqual( + ['volume_type1', 'volume_type2'], + sorted([az['name'] for az in volume_types])) + + # Test inability for non-admin to filter with sensitive extra specs + volume_types = _get_volume_types('{"key1":"value1"}', + use_admin_context=False) + self.assertEqual(0, len(volume_types)) def test_delete_non_project_default_type(self): type = self._create_volume_type(self.ctxt, 'type1') diff --git a/cinder/tests/unit/api/v3/test_types_orig.py b/cinder/tests/unit/api/v3/test_types_orig.py index 1a1f4fa81b1..0baaf585e5e 100644 --- a/cinder/tests/unit/api/v3/test_types_orig.py +++ b/cinder/tests/unit/api/v3/test_types_orig.py @@ -15,6 +15,7 @@ from unittest import mock +import ddt from oslo_utils import timeutils import webob @@ -22,6 +23,7 @@ from cinder.api.v3 import types from cinder.api.v3.views import types as views_types from cinder import context from cinder import exception +from cinder.policies import type_extra_specs as extra_specs_policy from cinder.policies import volume_type as type_policy from cinder.tests.unit.api import fakes from cinder.tests.unit import fake_constants as fake @@ -77,6 +79,7 @@ def return_volume_types_get_default(context): return fake_volume_type(1) +@ddt.ddt class VolumeTypesApiTest(test.TestCase): def _create_volume_type(self, volume_type_name, extra_specs=None, @@ -288,9 +291,50 @@ class VolumeTypesApiTest(test.TestCase): self.assertRaises(exception.VolumeTypeNotFound, self.controller.show, req, 'default') - def test_view_builder_show(self): + @ddt.data( + { + 'extra_spec_policy': False, + 'read_sensitive_policy': False, + 'qos_policy': False, + }, + { + 'extra_spec_policy': True, + 'read_sensitive_policy': False, + 'qos_policy': False, + }, + { + 'extra_spec_policy': True, + 'read_sensitive_policy': True, + 'qos_policy': False, + }, + { + 'extra_spec_policy': False, + 'read_sensitive_policy': False, + 'qos_policy': True, + }, + { + 'extra_spec_policy': True, + 'read_sensitive_policy': True, + 'qos_policy': True, + }, + ) + @ddt.unpack + def test_view_builder_show(self, + extra_spec_policy, + read_sensitive_policy, + qos_policy): + # This function returns the authorization result supplied by the + # DDT data for the associated policy. + def authorize(policy, fatal): + policy_data = { + type_policy.EXTRA_SPEC_POLICY: extra_spec_policy, + extra_specs_policy.READ_SENSITIVE_POLICY: ( + read_sensitive_policy), + type_policy.QOS_POLICY: qos_policy, + } + return policy_data[policy] + view_builder = views_types.ViewBuilder() - self.mock_authorize.return_value = False now = timeutils.utcnow().isoformat() raw_volume_type = dict( name='new_type', @@ -300,13 +344,15 @@ class VolumeTypesApiTest(test.TestCase): deleted=False, created_at=now, updated_at=now, - extra_specs={}, + extra_specs={'multiattach': True, 'sensitive': 'secret'}, deleted_at=None, id=42, ) request = fakes.HTTPRequest.blank("/v3") - output = view_builder.show(request, raw_volume_type) + with mock.patch('cinder.context.RequestContext.authorize', + side_effect=authorize): + output = view_builder.show(request, raw_volume_type) self.assertIn('volume_type', output) expected_volume_type = dict( @@ -315,165 +361,19 @@ class VolumeTypesApiTest(test.TestCase): is_public=True, id=42, ) + if extra_spec_policy: + expected_volume_type['extra_specs'] = {'multiattach': True} + if read_sensitive_policy: + expected_volume_type['extra_specs']['sensitive'] = 'secret' + if qos_policy: + expected_volume_type['qos_specs_id'] = 'new_id' + self.assertDictEqual(expected_volume_type, output['volume_type']) - def test_view_builder_show_admin(self): + @ddt.data(False, True) + def test_view_builder_list(self, is_admin): view_builder = views_types.ViewBuilder() - self.mock_authorize.return_value = True - now = timeutils.utcnow().isoformat() - raw_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - deleted=False, - created_at=now, - updated_at=now, - extra_specs={}, - deleted_at=None, - id=42, - ) - - request = fakes.HTTPRequest.blank("/v3", use_admin_context=True) - output = view_builder.show(request, raw_volume_type) - - self.assertIn('volume_type', output) - expected_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - extra_specs={}, - id=42, - ) - self.assertDictEqual(expected_volume_type, output['volume_type']) - - def test_view_builder_show_qos_specs_id_policy(self): - with mock.patch('cinder.context.RequestContext.authorize', - side_effect=[False, True]): - view_builder = views_types.ViewBuilder() - now = timeutils.utcnow().isoformat() - raw_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - deleted=False, - created_at=now, - updated_at=now, - extra_specs={}, - deleted_at=None, - id=42, - ) - - request = fakes.HTTPRequest.blank("/v3") - output = view_builder.show(request, raw_volume_type) - - self.assertIn('volume_type', output) - expected_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - id=42, - ) - self.assertDictEqual(expected_volume_type, output['volume_type']) - - def test_view_builder_show_extra_specs_policy(self): - with mock.patch('cinder.context.RequestContext.authorize', - side_effect=[True, False]): - view_builder = views_types.ViewBuilder() - now = timeutils.utcnow().isoformat() - raw_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - deleted=False, - created_at=now, - updated_at=now, - extra_specs={}, - deleted_at=None, - id=42, - ) - - request = fakes.HTTPRequest.blank("/v3") - output = view_builder.show(request, raw_volume_type) - - self.assertIn('volume_type', output) - expected_volume_type = dict( - name='new_type', - description='new_type_desc', - extra_specs={}, - is_public=True, - id=42, - ) - self.assertDictEqual(expected_volume_type, output['volume_type']) - - with mock.patch('cinder.context.RequestContext.authorize', - side_effect=[False, False]): - view_builder = views_types.ViewBuilder() - now = timeutils.utcnow().isoformat() - raw_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - deleted=False, - created_at=now, - updated_at=now, - extra_specs={}, - deleted_at=None, - id=42, - ) - - request = fakes.HTTPRequest.blank("/v3") - output = view_builder.show(request, raw_volume_type) - - self.assertIn('volume_type', output) - expected_volume_type = dict( - name='new_type', - description='new_type_desc', - is_public=True, - id=42, - ) - self.assertDictEqual(expected_volume_type, output['volume_type']) - - def test_view_builder_show_pass_all_policy(self): - with mock.patch('cinder.context.RequestContext.authorize', - side_effect=[True, True]): - view_builder = views_types.ViewBuilder() - now = timeutils.utcnow().isoformat() - raw_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - deleted=False, - created_at=now, - updated_at=now, - extra_specs={}, - deleted_at=None, - id=42, - ) - - request = fakes.HTTPRequest.blank("/v3") - output = view_builder.show(request, raw_volume_type) - - self.assertIn('volume_type', output) - expected_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - extra_specs={}, - is_public=True, - id=42, - ) - self.assertDictEqual(expected_volume_type, output['volume_type']) - - def test_view_builder_list(self): - view_builder = views_types.ViewBuilder() - self.mock_authorize.return_value = False + self.mock_authorize.return_value = is_admin now = timeutils.utcnow().isoformat() raw_volume_types = [] for i in range(0, 10): @@ -486,7 +386,7 @@ class VolumeTypesApiTest(test.TestCase): deleted=False, created_at=now, updated_at=now, - extra_specs={}, + extra_specs={'multiattach': True, 'sensitive': 'secret'}, deleted_at=None, id=42 + i ) @@ -503,42 +403,9 @@ class VolumeTypesApiTest(test.TestCase): is_public=True, id=42 + i ) - self.assertDictEqual(expected_volume_type, - output['volume_types'][i]) - - def test_view_builder_list_admin(self): - view_builder = views_types.ViewBuilder() - - now = timeutils.utcnow().isoformat() - raw_volume_types = [] - for i in range(0, 10): - raw_volume_types.append( - dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - deleted=False, - created_at=now, - updated_at=now, - extra_specs={}, - deleted_at=None, - id=42 + i - ) - ) - - request = fakes.HTTPRequest.blank("/v3", use_admin_context=True) - output = view_builder.index(request, raw_volume_types) - - self.assertIn('volume_types', output) - for i in range(0, 10): - expected_volume_type = dict( - name='new_type', - description='new_type_desc', - qos_specs_id='new_id', - is_public=True, - extra_specs={}, - id=42 + i - ) + if is_admin: + expected_volume_type['qos_specs_id'] = 'new_id' + expected_volume_type['extra_specs'] = {'multiattach': True, + 'sensitive': 'secret'} self.assertDictEqual(expected_volume_type, output['volume_types'][i]) diff --git a/doc/source/admin/blockstorage-manage-volumes.rst b/doc/source/admin/blockstorage-manage-volumes.rst index 49d3aaa2491..0913bd3079e 100644 --- a/doc/source/admin/blockstorage-manage-volumes.rst +++ b/doc/source/admin/blockstorage-manage-volumes.rst @@ -67,6 +67,7 @@ troubleshoot your installation and back up your Compute volumes. blockstorage-image-volume-cache.rst blockstorage-volume-backed-image.rst blockstorage-get-capabilities.rst + blockstorage-user-visible-extra-specs.rst blockstorage-groups.rst .. note:: diff --git a/doc/source/admin/blockstorage-user-visible-extra-specs.rst b/doc/source/admin/blockstorage-user-visible-extra-specs.rst new file mode 100644 index 00000000000..aeb5d8e8560 --- /dev/null +++ b/doc/source/admin/blockstorage-user-visible-extra-specs.rst @@ -0,0 +1,162 @@ +.. _user_visible_extra_specs: + +======================== +User visible extra specs +======================== + +Starting in Xena, certain volume type ``extra specs`` (i.e. properties) are +considered user visible, meaning their visibility is not restricted to only +cloud administrators. This feature provides regular users with more +information about the volume types available to them, and lets them make more +informed decisions on which volume type to choose when creating volumes. + +The following ``extra spec`` keys are treated as user visible: + +- ``RESKEY:availability_zones`` +- ``multiattach`` +- ``replication_enabled`` + +.. note:: + + * The set of user visible ``extra specs`` is a fixed list that is not + configurable. + + * The feature is entirely policy based, and does not require a new + microversion. + +Behavior using openstack client +------------------------------- + +Consider the following volume type, as viewed from an administrator's +perspective. In this example, ``multiattach`` is a user visible ``extra spec`` +and ``volume_backend_name`` is not. + +.. code-block:: console + + # Administrator behavior + [admin@host]$ openstack volume type show vol_type + +--------------------+-------------------------------------------------------+ + | Field | Value | + +--------------------+-------------------------------------------------------+ + | access_project_ids | None | + | description | None | + | id | d03a0f33-e695-4f5c-b712-7d92abbf72be | + | is_public | True | + | name | vol_type | + | properties | multiattach=' True', volume_backend_name='secret' | + | qos_specs_id | None | + +--------------------+-------------------------------------------------------+ + +Here is the output when a regular user executes the same command. Notice only +the user visible ``multiattach`` property is listed. + +.. code-block:: console + + # Regular user behavior + [user@host]$ openstack volume type show vol_type + +--------------------+--------------------------------------+ + | Field | Value | + +--------------------+--------------------------------------+ + | access_project_ids | None | + | description | None | + | id | d03a0f33-e695-4f5c-b712-7d92abbf72be | + | is_public | True | + | name | vol_type | + | properties | multiattach=' True' | + +--------------------+--------------------------------------+ + +The behavior for listing volume types is similar. Administrators will see all +``extra specs`` but regular users will see only user visible ``extra specs``. + +.. code-block:: console + + # Administrator behavior + [admin@host]$ openstack volume type list --long + +--------------------------------------+-------------+-----------+---------------------+-------------------------------------------------------+ + | ID | Name | Is Public | Description | Properties | + +--------------------------------------+-------------+-----------+---------------------+-------------------------------------------------------+ + | d03a0f33-e695-4f5c-b712-7d92abbf72be | vol_type | True | None | multiattach=' True', volume_backend_name='secret' | + | 80f38273-f4b9-4862-a4e6-87692eb66a96 | __DEFAULT__ | True | Default Volume Type | | + +--------------------------------------+-------------+-----------+---------------------+-------------------------------------------------------+ + + # Regular user behavior + [user@host]$ openstack volume type list --long + +--------------------------------------+-------------+-----------+---------------------+-------------------------+ + | ID | Name | Is Public | Description | Properties | + +--------------------------------------+-------------+-----------+---------------------+-------------------------+ + | d03a0f33-e695-4f5c-b712-7d92abbf72be | vol_type | True | None | multiattach=' True' | + | 80f38273-f4b9-4862-a4e6-87692eb66a96 | __DEFAULT__ | True | Default Volume Type | | + +--------------------------------------+-------------+-----------+---------------------+-------------------------+ + +Regular users may view these properties, but they may not modify them. Attempts +to modify a user visible property by a non-administrator will fail. + +.. code-block:: console + + [user@host]$ openstack volume type set --property multiattach=' False' vol_type + Failed to set volume type property: Policy doesn't allow + volume_extension:types_extra_specs:create to be performed. (HTTP 403) + +Filtering with extra specs +-------------------------- + +API microversion 3.52 adds support for using ``extra specs`` to filter the +list of volume types. Regular users are able to use that feature to filter for +user visible ``extra specs``. If a regular user attempts to filter on a +non-user visible ``extra spec`` then an empty list is returned. + +.. code-block:: console + + # Administrator behavior + [admin@host]$ cinder --os-volume-api-version 3.52 type-list \ + > --filters extra_specs={"multiattach":" True"} + +--------------------------------------+----------+-------------+-----------+ + | ID | Name | Description | Is_Public | + +--------------------------------------+----------+-------------+-----------+ + | d03a0f33-e695-4f5c-b712-7d92abbf72be | vol_type | - | True | + +--------------------------------------+----------+-------------+-----------+ + + [admin@host]$ cinder --os-volume-api-version 3.52 type-list \ + > --filters extra_specs={"volume_backend_name":"secret"} + +--------------------------------------+----------+-------------+-----------+ + | ID | Name | Description | Is_Public | + +--------------------------------------+----------+-------------+-----------+ + | d03a0f33-e695-4f5c-b712-7d92abbf72be | vol_type | - | True | + +--------------------------------------+----------+-------------+-----------+ + + # Regular user behavior + [user@host]$ cinder --os-volume-api-version 3.52 type-list \ + > --filters extra_specs={"multiattach":" True"} + +--------------------------------------+----------+-------------+-----------+ + | ID | Name | Description | Is_Public | + +--------------------------------------+----------+-------------+-----------+ + | d03a0f33-e695-4f5c-b712-7d92abbf72be | vol_type | - | True | + +--------------------------------------+----------+-------------+-----------+ + + [user@host]$ cinder --os-volume-api-version 3.52 type-list \ + > --filters extra_specs={"volume_backend_name":"secret"} + +----+------+-------------+-----------+ + | ID | Name | Description | Is_Public | + +----+------+-------------+-----------+ + +----+------+-------------+-----------+ + +Security considerations +----------------------- + +Cloud administrators who do not wish to expose any ``extra specs`` to regular +users may restore the previous behavior by setting the following policies to +their pre-Xena default values. + +.. code-block:: console + + "volume_extension:access_types_extra_specs": "rule:admin_api" + "volume_extension:types_extra_specs:index": "rule:admin_api" + "volume_extension:types_extra_specs:show": "rule:admin_api" + +To restrict regular users from using ``extra specs`` to filter the list of +volume types, modify /etc/cinder/resource_filters.json to restore the +*"volume_type"* entry to its pre-Xena default value. + +.. code-block:: console + + "volume_type": ["is_public"] diff --git a/doc/source/admin/generalized_filters.rst b/doc/source/admin/generalized_filters.rst index 20b9da6f2b2..0f8412a86fd 100644 --- a/doc/source/admin/generalized_filters.rst +++ b/doc/source/admin/generalized_filters.rst @@ -79,5 +79,5 @@ in the table. project_id * - get pools - name, volume_type - * - list types (3.51) + * - list types (3.52) - is_public, extra_specs diff --git a/doc/source/admin/index.rst b/doc/source/admin/index.rst index fa6520c32c1..6acec85c54f 100644 --- a/doc/source/admin/index.rst +++ b/doc/source/admin/index.rst @@ -38,6 +38,7 @@ Amazon EC2 Elastic Block Storage (EBS) offering. blockstorage-consistency-groups.rst blockstorage-driver-filter-weighing.rst blockstorage-get-capabilities.rst + blockstorage-user-visible-extra-specs.rst blockstorage-groups.rst blockstorage-image-volume-cache.rst blockstorage-lio-iscsi-support.rst diff --git a/etc/cinder/resource_filters.json b/etc/cinder/resource_filters.json index 967361e692d..61720e7c6a6 100644 --- a/etc/cinder/resource_filters.json +++ b/etc/cinder/resource_filters.json @@ -11,5 +11,5 @@ "message": ["resource_uuid", "resource_type", "event_id", "request_id", "message_level"], "pool": ["name", "volume_type"], - "volume_type": ["is_public"] + "volume_type": ["is_public", "extra_specs"] } diff --git a/releasenotes/notes/user-visible-extra-specs-6cf7e49c6be57a01.yaml b/releasenotes/notes/user-visible-extra-specs-6cf7e49c6be57a01.yaml new file mode 100644 index 00000000000..09048e781a3 --- /dev/null +++ b/releasenotes/notes/user-visible-extra-specs-6cf7e49c6be57a01.yaml @@ -0,0 +1,16 @@ +--- +features: + - | + A small list volume type extra specs are now visible to regular users, and + not just to cloud administrators. This allows users to see non-senstive + extra specs, which may help them choose a particular volume type when + creating volumes. Sensitive extra specs are still only visible to cloud + administrators. See the ``User visible extra specs`` section in the Cinder + Administration guide for more information. +security: + - | + A small list volume type extra specs are now visible to regular users, and + not just to cloud administrators. Cloud administrators that wish to opt + out of this feature should consult the ``Security considerations`` + portion of the ``User visible extra specs`` section in the Cinder + Administration guide.