Added more options while uploading volume as image

Added support for is_public/visibility, and protected
while uploading volume as image to image service.
is_public/visibility is used to make the image publicly accessible
and protected is used to prevent the image from being deleted.

DocImpact
APIImpact

Change-Id: I6e6b2276af22b7809ea88289427c6873211b3faf
Closes-Bug: #1288131

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,

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"
 

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.

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:

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)

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')
+    @ddt.data('1.0', '2.0', '3.0')
-    def test_versions_v1(self, version):
+    def test_versions(self, version):
-        req = self.build_request(base_url='http://localhost/v1',
+        req = self.build_request(
-                                 header_version=version)
+            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'))
+        if version == '3.0':
-        self.assertEqual('', version_list[0].get('version'))
+            self.check_response(response, version)
-
+            self.assertEqual(api_version_request._MAX_API_VERSION,
-    @ddt.data('2.0')
+                             version_list[0].get('version'))
-    def test_versions_v2(self, version):
+            self.assertEqual(api_version_request._MIN_API_VERSION,
-        req = self.build_request(base_url='http://localhost/v2',
+                             version_list[0].get('min_version'))
-                                 header_version=version)
+        else:
-
+            self.assertEqual('', version_list[0].get('min_version'))
-        response = req.get_response(router.APIRouter())
+            self.assertEqual('', version_list[0].get('version'))
-        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'))
 
     def test_versions_version_latest(self):
         req = self.build_request(header_version='latest')

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

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

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,

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

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

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

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.