diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py index d2db66c365e..9737a212d87 100644 --- a/cinder/api/contrib/volume_actions.py +++ b/cinder/api/contrib/volume_actions.py @@ -13,6 +13,7 @@ # under the License. +from oslo_config import cfg from oslo_log import log as logging import oslo_messaging as messaging from oslo_utils import encodeutils @@ -21,6 +22,7 @@ import six import webob from cinder.api import extensions +from cinder.api.openstack import api_version_request from cinder.api.openstack import wsgi from cinder.api import xmlutil from cinder import exception @@ -30,6 +32,7 @@ from cinder import volume LOG = logging.getLogger(__name__) +CONF = cfg.CONF def authorize(context, action_name): @@ -51,6 +54,11 @@ class VolumeToImageSerializer(xmlutil.TemplateBuilder): root.set('container_format') root.set('disk_format') root.set('image_name') + root.set('protected') + if CONF.glance_api_version == 2: + root.set('visibility') + else: + root.set('is_public') return xmlutil.MasterTemplate(root, 1) @@ -62,7 +70,12 @@ class VolumeToImageDeserializer(wsgi.XMLDeserializer): action_name = action_node.tagName action_data = {} - attributes = ["force", "image_name", "container_format", "disk_format"] + attributes = ["force", "image_name", "container_format", "disk_format", + "protected"] + if CONF.glance_api_version == 2: + attributes.append('visibility') + else: + attributes.append('is_public') for attr in attributes: if action_node.hasAttribute(attr): action_data[attr] = action_node.getAttribute(attr) @@ -254,6 +267,7 @@ class VolumeActionsController(wsgi.Controller): """Uploads the specified volume to image service.""" context = req.environ['cinder.context'] params = body['os-volume_upload_image'] + req_version = req.api_version_request if not params.get("image_name"): msg = _("No image_name was specified in request.") raise webob.exc.HTTPBadRequest(explanation=msg) @@ -276,6 +290,24 @@ class VolumeActionsController(wsgi.Controller): "bare"), "disk_format": params.get("disk_format", "raw"), "name": params["image_name"]} + + if req_version >= api_version_request.APIVersionRequest('3.1'): + + image_metadata['visibility'] = params.get('visibility', 'private') + image_metadata['protected'] = params.get('protected', 'False') + + if image_metadata['visibility'] == 'public': + authorize(context, 'upload_public') + + if CONF.glance_api_version != 2: + # Replace visibility with is_public for Glance V1 + image_metadata['is_public'] = ( + image_metadata['visibility'] == 'public') + image_metadata.pop('visibility', None) + + image_metadata['protected'] = ( + utils.get_bool_param('protected', image_metadata)) + try: response = self.volume_api.copy_volume_to_image(context, volume, diff --git a/cinder/api/openstack/api_version_request.py b/cinder/api/openstack/api_version_request.py index fd242dad5a4..096ecc45556 100644 --- a/cinder/api/openstack/api_version_request.py +++ b/cinder/api/openstack/api_version_request.py @@ -46,6 +46,7 @@ REST_API_VERSION_HISTORY = """ * 3.0 - Includes all V2 APIs and extensions. V1 API is still supported. * 3.0 - Versions API updated to reflect beginning of microversions epoch. + * 3.1 - Adds visibility and protected to _volume_upload_image parameters. """ @@ -54,7 +55,7 @@ REST_API_VERSION_HISTORY = """ # the minimum version of the API supported. # Explicitly using /v1 or /v2 enpoints will still work _MIN_API_VERSION = "3.0" -_MAX_API_VERSION = "3.0" +_MAX_API_VERSION = "3.1" _LEGACY_API_VERSION1 = "1.0" _LEGACY_API_VERSION2 = "2.0" diff --git a/cinder/api/openstack/rest_api_version_history.rst b/cinder/api/openstack/rest_api_version_history.rst index 2ec94e4a373..54f7bb0c78b 100644 --- a/cinder/api/openstack/rest_api_version_history.rst +++ b/cinder/api/openstack/rest_api_version_history.rst @@ -28,3 +28,8 @@ user documentation. 3.0 and later versions and their respective /v3 endpoints. All other 3.0 APIs are functionally identical to version 2.0. + +3.1 +--- + Added the parameters ``protected`` and ``visibility`` to + _volume_upload_image requests. diff --git a/cinder/image/glance.py b/cinder/image/glance.py index bb5810fb559..f1a409d4194 100644 --- a/cinder/image/glance.py +++ b/cinder/image/glance.py @@ -507,7 +507,12 @@ def _extract_attributes(image): 'container_format', 'status', 'id', 'name', 'created_at', 'updated_at', 'deleted', 'deleted_at', 'checksum', - 'min_disk', 'min_ram', 'is_public'] + 'min_disk', 'min_ram', 'protected'] + if CONF.glance_api_version == 2: + IMAGE_ATTRIBUTES.append('visibility') + else: + IMAGE_ATTRIBUTES.append('is_public') + output = {} for attr in IMAGE_ATTRIBUTES: diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index cc0596ec920..62f0cc0e868 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -22,6 +22,7 @@ from oslo_serialization import jsonutils import webob from cinder.api.contrib import volume_actions +from cinder.api.openstack import api_version_request as api_version from cinder import context from cinder import exception from cinder.image import glance @@ -572,7 +573,6 @@ class VolumeImageActionsTest(test.TestCase): "disk_format": 'raw', "updated_at": datetime.datetime(1, 1, 1, 1, 1, 1), "image_name": 'image_name', - "is_public": False, "force": True} body = {"os-volume_upload_image": vol} @@ -591,7 +591,26 @@ class VolumeImageActionsTest(test.TestCase): 'min_ram': 0, 'checksum': None, 'min_disk': 0, - 'is_public': False, + 'deleted_at': None, + 'properties': {u'x_billing_code_license': u'246254365'}, + 'size': 0} + return ret + + def fake_image_service_create_3_1(self, *args): + ret = { + 'status': u'queued', + 'name': u'image_name', + 'deleted': False, + 'container_format': u'bare', + 'created_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'disk_format': u'raw', + 'updated_at': datetime.datetime(1, 1, 1, 1, 1, 1), + 'id': 1, + 'min_ram': 0, + 'checksum': None, + 'min_disk': 0, + 'visibility': 'public', + 'protected': True, 'deleted_at': None, 'properties': {u'x_billing_code_license': u'246254365'}, 'size': 0} @@ -815,6 +834,19 @@ class VolumeImageActionsTest(test.TestCase): self.assertDictMatch(expected_res, res_dict) + def test_copy_volume_to_image_public_not_authorized(self): + """Test unauthorized create public image from volume.""" + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + req = fakes.HTTPRequest.blank('/v3/tenant1/volumes/%s/action' % id) + req.environ['cinder.context'].is_admin = False + req.headers = {'OpenStack-API-Version': 'volume 3.1'} + req.api_version_request = api_version.APIVersionRequest('3.1') + body = self._get_os_volume_upload_image() + body['os-volume_upload_image']['visibility'] = 'public' + self.assertRaises(exception.PolicyNotAuthorized, + self.controller._volume_upload_image, + req, id, body) + def test_copy_volume_to_image_without_glance_metadata(self): """Test create image from volume if volume is created without image. @@ -1019,3 +1051,59 @@ class VolumeImageActionsTest(test.TestCase): } self.assertDictMatch(expected_res, res_dict) + + @mock.patch.object(volume_api.API, "get_volume_image_metadata") + @mock.patch.object(glance.GlanceImageService, "create") + @mock.patch.object(volume_api.API, "update") + @mock.patch.object(volume_rpcapi.VolumeAPI, "copy_volume_to_image") + def test_copy_volume_to_image_version_3_1( + self, + mock_copy_volume_to_image, + mock_update, + mock_create, + mock_get_volume_image_metadata): + """Test create image from volume with protected properties.""" + id = 'aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee' + + mock_get_volume_image_metadata.return_value = { + "volume_id": id, + "key": "x_billing_code_license", + "value": "246254365"} + mock_create.side_effect = self.fake_image_service_create_3_1 + mock_update.side_effect = stubs.stub_volume_update + mock_copy_volume_to_image.side_effect = \ + self.fake_rpc_copy_volume_to_image + + self.override_config('glance_api_version', 2) + + req = fakes.HTTPRequest.blank('/v3/tenant1/volumes/%s/action' % id) + req.environ['cinder.context'].is_admin = True + req.headers = {'OpenStack-API-Version': 'volume 3.1'} + req.api_version_request = api_version.APIVersionRequest('3.1') + body = self._get_os_volume_upload_image() + body['os-volume_upload_image']['visibility'] = 'public' + body['os-volume_upload_image']['protected'] = True + res_dict = self.controller._volume_upload_image(req, + id, + body) + expected_res = { + 'os-volume_upload_image': { + 'id': id, + 'updated_at': datetime.datetime( + 1900, 1, 1, 1, 1, 1, + tzinfo=iso8601.iso8601.Utc()), + 'status': 'uploading', + 'display_description': 'displaydesc', + 'size': 1, + 'visibility': 'public', + 'protected': True, + 'volume_type': fake_volume.fake_db_volume_type( + name='vol_type_name'), + 'image_id': 1, + 'container_format': 'bare', + 'disk_format': 'raw', + 'image_name': 'image_name' + } + } + + self.assertDictMatch(expected_res, res_dict) diff --git a/cinder/tests/unit/api/test_versions.py b/cinder/tests/unit/api/test_versions.py index c380c931e59..8dda3f3f8d6 100644 --- a/cinder/tests/unit/api/test_versions.py +++ b/cinder/tests/unit/api/test_versions.py @@ -95,10 +95,12 @@ class VersionsControllerTestCase(test.TestCase): response = req.get_response(router.APIRouter()) self.assertEqual(200, response.status_int) - @ddt.data('1.0') - def test_versions_v1(self, version): - req = self.build_request(base_url='http://localhost/v1', - header_version=version) + @ddt.data('1.0', '2.0', '3.0') + def test_versions(self, version): + req = self.build_request( + base_url='http://localhost/v{}'.format(version[0]), + header_version=version) + if version is not None: req.headers = {VERSION_HEADER_NAME: VOLUME_SERVICE + version} @@ -108,44 +110,17 @@ class VersionsControllerTestCase(test.TestCase): version_list = body['versions'] ids = [v['id'] for v in version_list] - self.assertEqual({'v1.0'}, set(ids)) + self.assertEqual({'v{}'.format(version)}, set(ids)) - self.assertEqual('', version_list[0].get('min_version')) - self.assertEqual('', version_list[0].get('version')) - - @ddt.data('2.0') - def test_versions_v2(self, version): - req = self.build_request(base_url='http://localhost/v2', - header_version=version) - - response = req.get_response(router.APIRouter()) - self.assertEqual(200, response.status_int) - body = jsonutils.loads(response.body) - version_list = body['versions'] - - ids = [v['id'] for v in version_list] - self.assertEqual({'v2.0'}, set(ids)) - - self.assertEqual('', version_list[0].get('min_version')) - self.assertEqual('', version_list[0].get('version')) - - @ddt.data('3.0', 'latest') - def test_versions_v3_0_and_latest(self, version): - req = self.build_request(header_version=version) - - response = req.get_response(router.APIRouter()) - self.assertEqual(200, response.status_int) - body = jsonutils.loads(response.body) - version_list = body['versions'] - - ids = [v['id'] for v in version_list] - self.assertEqual({'v3.0'}, set(ids)) - self.check_response(response, '3.0') - - self.assertEqual(api_version_request._MAX_API_VERSION, - version_list[0].get('version')) - self.assertEqual(api_version_request._MIN_API_VERSION, - version_list[0].get('min_version')) + if version == '3.0': + self.check_response(response, version) + self.assertEqual(api_version_request._MAX_API_VERSION, + version_list[0].get('version')) + self.assertEqual(api_version_request._MIN_API_VERSION, + version_list[0].get('min_version')) + else: + self.assertEqual('', version_list[0].get('min_version')) + self.assertEqual('', version_list[0].get('version')) def test_versions_version_latest(self): req = self.build_request(header_version='latest') diff --git a/cinder/tests/unit/glance/stubs.py b/cinder/tests/unit/glance/stubs.py index 291687c3a64..b1b18a687ac 100644 --- a/cinder/tests/unit/glance/stubs.py +++ b/cinder/tests/unit/glance/stubs.py @@ -22,7 +22,8 @@ IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', 'container_format', 'checksum', 'id', 'name', 'created_at', 'updated_at', 'deleted', 'status', - 'min_disk', 'min_ram', 'is_public'] + 'min_disk', 'min_ram', 'visibility', + 'protected'] class StubGlanceClient(object): diff --git a/cinder/tests/unit/image/fake.py b/cinder/tests/unit/image/fake.py index ee4877b75b2..c9aa4a999c7 100644 --- a/cinder/tests/unit/image/fake.py +++ b/cinder/tests/unit/image/fake.py @@ -41,7 +41,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': False, + 'visibility': 'private', + 'protected': False, 'container_format': 'raw', 'disk_format': 'raw', 'properties': {'kernel_id': 'nokernel', @@ -55,7 +56,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': True, + 'visibility': 'public', + 'protected': True, 'container_format': 'ami', 'disk_format': 'ami', 'properties': {'kernel_id': 'nokernel', @@ -68,7 +70,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': True, + 'visibility': 'public', + 'protected': True, 'container_format': None, 'disk_format': None, 'properties': {'kernel_id': 'nokernel', @@ -81,7 +84,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': True, + 'visibility': 'public', + 'protected': True, 'container_format': 'ami', 'disk_format': 'ami', 'properties': {'kernel_id': 'nokernel', @@ -94,7 +98,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': True, + 'visibility': 'public', + 'protected': True, 'container_format': 'ami', 'disk_format': 'ami', 'properties': { @@ -108,7 +113,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': False, + 'visibility': 'public', + 'protected': False, 'container_format': 'ova', 'disk_format': 'vhd', 'properties': {'kernel_id': 'nokernel', @@ -123,7 +129,8 @@ class _FakeImageService(object): 'deleted_at': None, 'deleted': False, 'status': 'active', - 'is_public': False, + 'visibility': 'public', + 'protected': False, 'container_format': 'ova', 'disk_format': 'vhd', 'properties': {'kernel_id': 'nokernel', diff --git a/cinder/tests/unit/image/test_glance.py b/cinder/tests/unit/image/test_glance.py index f078e6143b7..87fcff83aed 100644 --- a/cinder/tests/unit/image/test_glance.py +++ b/cinder/tests/unit/image/test_glance.py @@ -40,7 +40,8 @@ class NullWriter(object): class TestGlanceSerializer(test.TestCase): def test_serialize(self): metadata = {'name': 'image1', - 'is_public': True, + 'visibility': 'public', + 'protected': True, 'foo': 'bar', 'properties': { 'prop1': 'propvalue1', @@ -53,7 +54,8 @@ class TestGlanceSerializer(test.TestCase): converted_expected = { 'name': 'image1', - 'is_public': True, + 'visibility': 'public', + 'protected': True, 'foo': 'bar', 'properties': { 'prop1': 'propvalue1', @@ -114,7 +116,8 @@ class TestGlanceImageService(test.TestCase): fixture = {'name': None, 'properties': {}, 'status': None, - 'is_public': None} + 'visibility': None, + 'protected': None} fixture.update(kwargs) return fixture @@ -127,6 +130,7 @@ class TestGlanceImageService(test.TestCase): """Ensure instance_id is persisted as an image-property.""" fixture = {'name': 'test image', 'is_public': False, + 'protected': False, 'properties': {'instance_id': '42', 'user_id': 'fake'}} image_id = self.service.create(self.context, fixture)['id'] @@ -135,6 +139,7 @@ class TestGlanceImageService(test.TestCase): 'id': image_id, 'name': 'test image', 'is_public': False, + 'protected': False, 'size': None, 'min_disk': None, 'min_ram': None, @@ -161,13 +166,15 @@ class TestGlanceImageService(test.TestCase): instance_id. Public images are an example of an image not tied to an instance. """ - fixture = {'name': 'test image', 'is_public': False} + fixture = {'name': 'test image', 'is_public': False, + 'protected': False} image_id = self.service.create(self.context, fixture)['id'] expected = { 'id': image_id, 'name': 'test image', 'is_public': False, + 'protected': False, 'size': None, 'min_disk': None, 'min_ram': None, @@ -206,7 +213,8 @@ class TestGlanceImageService(test.TestCase): def test_detail_private_image(self): fixture = self._make_fixture(name='test image') - fixture['is_public'] = False + fixture['visibility'] = 'private' + fixture['protected'] = False properties = {'owner_id': 'proj1'} fixture['properties'] = properties @@ -239,6 +247,7 @@ class TestGlanceImageService(test.TestCase): 'id': ids[i], 'status': None, 'is_public': None, + 'protected': None, 'name': 'TestImage %d' % (i), 'properties': {}, 'size': None, @@ -296,6 +305,7 @@ class TestGlanceImageService(test.TestCase): 'id': ids[i], 'status': None, 'is_public': None, + 'protected': None, 'name': 'TestImage %d' % (i), 'properties': {}, 'size': None, @@ -382,6 +392,7 @@ class TestGlanceImageService(test.TestCase): 'id': image_id, 'name': 'image1', 'is_public': True, + 'protected': None, 'size': None, 'min_disk': None, 'min_ram': None, @@ -401,6 +412,7 @@ class TestGlanceImageService(test.TestCase): def test_show_raises_when_no_authtoken_in_the_context(self): fixture = self._make_fixture(name='image1', is_public=False, + protected=False, properties={'one': 'two'}) image_id = self.service.create(self.context, fixture)['id'] self.context.auth_token = False @@ -418,6 +430,7 @@ class TestGlanceImageService(test.TestCase): 'id': image_id, 'name': 'image10', 'is_public': True, + 'protected': None, 'size': None, 'min_disk': None, 'min_ram': None, @@ -590,7 +603,8 @@ class TestGlanceImageService(test.TestCase): IMAGE_ATTRIBUTES = ['size', 'disk_format', 'owner', 'container_format', 'id', 'created_at', 'updated_at', 'deleted', 'status', - 'min_disk', 'min_ram', 'is_public'] + 'min_disk', 'min_ram', 'is_public', + 'visibility', 'protected'] raw = dict.fromkeys(IMAGE_ATTRIBUTES) raw.update(metadata) self.__dict__['raw'] = raw @@ -606,6 +620,7 @@ class TestGlanceImageService(test.TestCase): 'id': 1, 'name': None, 'is_public': None, + 'protected': None, 'size': None, 'min_disk': None, 'min_ram': None, @@ -622,6 +637,46 @@ class TestGlanceImageService(test.TestCase): } self.assertEqual(expected, actual) + @mock.patch('cinder.image.glance.CONF') + def test_v2_passes_visibility_param(self, config): + + config.glance_api_version = 2 + config.glance_num_retries = 0 + + metadata = { + 'id': 1, + 'size': 2, + 'visibility': 'public', + } + + image = glance_stubs.FakeImage(metadata) + client = glance_stubs.StubGlanceClient() + + service = self._create_image_service(client) + service._image_schema = glance_stubs.FakeSchema() + + actual = service._translate_from_glance('fake_context', image) + expected = { + 'id': 1, + 'name': None, + 'visibility': 'public', + 'protected': None, + 'size': 2, + 'min_disk': None, + 'min_ram': None, + 'disk_format': None, + 'container_format': None, + 'checksum': None, + 'deleted': None, + 'status': None, + 'properties': {}, + 'owner': None, + 'created_at': None, + 'updated_at': None + } + + self.assertEqual(expected, actual) + @mock.patch('cinder.image.glance.CONF') def test_extracting_v2_boot_properties(self, config): @@ -647,7 +702,8 @@ class TestGlanceImageService(test.TestCase): expected = { 'id': 1, 'name': None, - 'is_public': None, + 'visibility': None, + 'protected': None, 'size': 2, 'min_disk': 2, 'min_ram': 2, diff --git a/cinder/tests/unit/policy.json b/cinder/tests/unit/policy.json index 2ff533cbe3d..1c4f9b554ff 100644 --- a/cinder/tests/unit/policy.json +++ b/cinder/tests/unit/policy.json @@ -47,6 +47,7 @@ "volume_extension:volume_admin_actions:migrate_volume": "rule:admin_api", "volume_extension:volume_admin_actions:migrate_volume_completion": "rule:admin_api", "volume_extension:volume_actions:upload_image": "", + "volume_extension:volume_actions:upload_public": "rule:admin_api", "volume_extension:types_manage": "", "volume_extension:types_extra_specs": "", "volume_extension:access_types_qos_specs_id": "rule:admin_api", diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 9e99c0e7ea8..52bc7ff842d 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -1187,6 +1187,12 @@ class API(base.Base): "container_format": recv_metadata['container_format'], "disk_format": recv_metadata['disk_format'], "image_name": recv_metadata.get('name', None)} + if 'protected' in recv_metadata: + response['protected'] = recv_metadata.get('protected') + if 'is_public' in recv_metadata: + response['is_public'] = recv_metadata.get('is_public') + elif 'visibility' in recv_metadata: + response['visibility'] = recv_metadata.get('visibility') LOG.info(_LI("Copy volume to image completed successfully."), resource=volume) return response diff --git a/etc/cinder/policy.json b/etc/cinder/policy.json index 02af88bdfef..29ca512d3dc 100644 --- a/etc/cinder/policy.json +++ b/etc/cinder/policy.json @@ -52,6 +52,8 @@ "volume_extension:volume_admin_actions:migrate_volume": "rule:admin_api", "volume_extension:volume_admin_actions:migrate_volume_completion": "rule:admin_api", + "volume_extension:volume_actions:upload_public": "rule:admin_api", + "volume_extension:volume_host_attribute": "rule:admin_api", "volume_extension:volume_tenant_attribute": "rule:admin_or_owner", "volume_extension:volume_mig_status_attribute": "rule:admin_api", diff --git a/releasenotes/notes/add-volume-upload-image-options-3a61a31c544fa034.yaml b/releasenotes/notes/add-volume-upload-image-options-3a61a31c544fa034.yaml new file mode 100644 index 00000000000..6c1e8325290 --- /dev/null +++ b/releasenotes/notes/add-volume-upload-image-options-3a61a31c544fa034.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Added the options ``visibility`` and ``protected`` to + the os-volume_upload_image REST API call.