diff --git a/cinder/policies/attachments.py b/cinder/policies/attachments.py index 3dc13168d72..1378e4477fc 100644 --- a/cinder/policies/attachments.py +++ b/cinder/policies/attachments.py @@ -21,6 +21,7 @@ from cinder.policies import base CREATE_POLICY = 'volume:attachment_create' UPDATE_POLICY = 'volume:attachment_update' DELETE_POLICY = 'volume:attachment_delete' +MULTIATTACH_BOOTABLE_VOLUME_POLICY = 'volume:multiattach_bootable_volume' attachments_policies = [ policy.DocumentedRuleDefault( @@ -53,6 +54,16 @@ attachments_policies = [ 'path': '/attachments/{attachment_id}' } ]), + policy.DocumentedRuleDefault( + name=MULTIATTACH_BOOTABLE_VOLUME_POLICY, + check_str=base.RULE_ADMIN_OR_OWNER, + description="Allow multiattach of bootable volumes.", + operations=[ + { + 'method': 'POST', + 'path': '/attachments' + } + ]), ] diff --git a/cinder/policies/volumes.py b/cinder/policies/volumes.py index 15b9245be11..d9d8d024bd7 100644 --- a/cinder/policies/volumes.py +++ b/cinder/policies/volumes.py @@ -29,6 +29,7 @@ HOST_ATTRIBUTE_POLICY = "volume_extension:volume_host_attribute" TENANT_ATTRIBUTE_POLICY = "volume_extension:volume_tenant_attribute" MIG_ATTRIBUTE_POLICY = "volume_extension:volume_mig_status_attribute" ENCRYPTION_METADATA_POLICY = "volume_extension:volume_encryption_metadata" +MULTIATTACH_POLICY = "volume:multiattach" volumes_policies = [ policy.DocumentedRuleDefault( @@ -161,6 +162,16 @@ volumes_policies = [ 'path': '/volumes/{volume_id}/encryption/{encryption_key}' } ]), + policy.DocumentedRuleDefault( + name=MULTIATTACH_POLICY, + check_str=base.RULE_ADMIN_OR_OWNER, + description="Create multiattach capable volume.", + operations=[ + { + 'method': 'POST', + 'path': '/volumes' + } + ]), ] diff --git a/cinder/tests/unit/attachments/test_attachments_api.py b/cinder/tests/unit/attachments/test_attachments_api.py index 0bcf15b6ded..fbfc6bd9eeb 100644 --- a/cinder/tests/unit/attachments/test_attachments_api.py +++ b/cinder/tests/unit/attachments/test_attachments_api.py @@ -12,11 +12,15 @@ import mock from oslo_config import cfg +from oslo_policy import policy as oslo_policy from cinder import context from cinder import db from cinder import exception from cinder import objects +from cinder.policies import attachments as attachment_policy +from cinder.policies import base as base_policy +from cinder import policy from cinder import test from cinder.tests.unit import fake_constants as fake from cinder.tests.unit import utils as tests_utils @@ -38,6 +42,9 @@ class AttachmentManagerTestCase(test.TestCase): self.project_id = fake.PROJECT3_ID self.context.project_id = self.project_id self.volume_api = volume_api.API() + self.user_context = context.RequestContext( + user_id=fake.USER_ID, + project_id=fake.PROJECT3_ID) def test_attachment_create_no_connector(self): """Test attachment_create no connector.""" @@ -237,3 +244,23 @@ class AttachmentManagerTestCase(test.TestCase): vref.id) self.assertEqual('reserved', vref.status) self.assertEqual(1, len(vref.volume_attachment)) + + def test_attachment_create_bootable_multiattach_policy(self): + """Test attachment_create no connector.""" + volume_params = {'status': 'available'} + + vref = tests_utils.create_volume(self.context, **volume_params) + vref.multiattach = True + vref.bootable = True + vref.status = 'in-use' + + rules = { + attachment_policy.MULTIATTACH_BOOTABLE_VOLUME_POLICY: base_policy.RULE_ADMIN_API # noqa + } + policy.set_rules(oslo_policy.Rules.from_dict(rules)) + self.addCleanup(policy.reset) + self.assertRaises(exception.PolicyNotAuthorized, + self.volume_api.attachment_create, + self.user_context, + vref, + fake.UUID2) diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index d8649ec9ff7..ce4df54988e 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -54,6 +54,7 @@ from cinder.volume import driver from cinder.volume import manager as vol_manager from cinder.volume import rpcapi as volume_rpcapi import cinder.volume.targets.tgt +from cinder.volume import volume_types QUOTAS = quota.QUOTAS @@ -606,6 +607,26 @@ class VolumeTestCase(base.BaseVolumeTestCase): volume_type=db_vol_type) self.assertEqual(db_vol_type.get('id'), volume['volume_type_id']) + def test_create_volume_with_multiattach_volume_type(self): + """Test volume creation with multiattach volume type.""" + elevated = context.get_admin_context() + volume_api = cinder.volume.api.API() + + especs = dict(multiattach=" True") + volume_types.create(elevated, + "multiattach-type", + especs, + description="test-multiattach") + foo = objects.VolumeType.get_by_name_or_id(elevated, + "multiattach-type") + + vol = volume_api.create(self.context, + 1, + 'admin-vol', + 'description', + volume_type=foo) + self.assertEqual(foo['id'], vol['volume_type_id']) + @mock.patch.object(key_manager, 'API', fake_keymgr.fake_api) def test_create_volume_with_encrypted_volume_type_aes(self): ctxt = context.get_admin_context() diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 79bc7b10d83..fdf84705263 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -193,6 +193,10 @@ class API(base.Base): return (volume['migration_status'] not in self.AVAILABLE_MIGRATION_STATUS) + def _is_multiattach(self, volume_type): + specs = getattr(volume_type, 'extra_specs', {}) + return specs.get('multiattach', 'False') == ' True' + def create(self, context, size, name, description, snapshot=None, image_id=None, volume_type=None, metadata=None, availability_zone=None, source_volume=None, @@ -285,6 +289,9 @@ class API(base.Base): utils.check_metadata_properties(metadata) + if (volume_type and self._is_multiattach(volume_type)): + context.authorize(vol_policy.MULTIATTACH_POLICY) + create_what = { 'context': context, 'raw_size': size, @@ -1622,6 +1629,30 @@ class API(base.Base): vol_type_id = vol_type['id'] + # NOTE(jdg): We check here if multiattach is involved in either side + # of the retype, we can't change multiattach on an in-use volume + # because there's things the hypervisor needs when attaching, so + # we just disallow retype of in-use volumes in this case. You still + # have to get through scheduling if all the conditions are met, we + # should consider an up front capabilities check to give fast feedback + # rather than "No hosts found" and error status + + src_is_multiattach = volume.multiattach + tgt_is_multiattach = False + if (vol_type and + self._is_multiattach(vol_type)): + tgt_is_multiattach = True + + if src_is_multiattach != tgt_is_multiattach: + if volume.status != "available": + msg = _('Invalid volume_type passed, retypes affecting ' + 'multiattach are only allowed on available volumes, ' + 'the specified volume however currently has a status ' + 'of: %s.') % volume.status + LOG.info(msg) + raise exception.InvalidInput(reason=msg) + context.authorize(vol_policy.MULTIATTACH_POLICY) + # We're checking here in so that we can report any quota issues as # early as possible, but won't commit until we change the type. We # pass the reservations onward in case we need to roll back. @@ -2021,6 +2052,14 @@ class API(base.Base): # creation of other new reserves/attachments while in this state # so we avoid contention issues with shared connections + # Multiattach of bootable volumes is a special case with it's own + # policy, check that here right off the bat + if (vref.get('multiattach', False) and + vref.status == 'in-use' and + vref.bootable): + context.authorize( + attachment_policy.MULTIATTACH_BOOTABLE_VOLUME_POLICY) + # FIXME(JDG): We want to be able to do things here like reserve a # volume for Nova to do BFV WHILE the volume may be in the process of # downloading image, we add downloading here; that's easy enough but @@ -2032,6 +2071,7 @@ class API(base.Base): 'status': (('available', 'in-use', 'downloading') if vref.multiattach else ('available', 'downloading'))} + result = vref.conditional_update({'status': 'reserved'}, expected) if not result: diff --git a/releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml b/releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml new file mode 100644 index 00000000000..162cc610d2a --- /dev/null +++ b/releasenotes/notes/add_multiattach_policies-8e0b22505ed6cbd8.yaml @@ -0,0 +1,27 @@ +--- +features: + - Added policies to disallow multiattach operations. This includes two + policies, the first being a general policy to allow the creation or + retyping of multiattach volumes is a volume create policy with the name + ``MULTIATTACH_POLICY``. + + The second policy is specifically for disallowing the ability to create + multiple attachments on a volume that is marked as bootable, and is an + attachment policy with the name ``MULTIATTACH_BOOTABLE_VOLUME_POLICY``. + + The default for these new policies is ``ADMIN_OR_OWNER``; be aware that + if you wish to disable either of these policies for your users you will + need to modify the default policy settings. +upgrade: + - Added policies to disallow multiattach operations. This includes two + policies, the first being a general policy to allow the creation or + retyping of multiattach volumes is a volume create policy with the name + ``MULTIATTACH_POLICY``. + + The second policy is specifically for disallowing the ability to create + multiple attachments on a volume that is marked as bootable, and is an + attachment policy with the name ``MULTIATTACH_BOOTABLE_VOLUME_POLICY``. + + The default for these new policies is ``ADMIN_OR_OWNER``; be aware that + if you wish to disable either of these policies for your users you will + need to modify the default policy settings.