openstack
/
horizon
Code Issues Proposed changes

Allow OVA upload for images 

Bug #1641383 was not fixed for REST API call.
The two implementation of create_image_metadata are merged
into api/glance.py.

There were minor differences in field naming and handling
(min_ vs minimum_, is_public vs public vs visibility).
This commit normalizes them to simplify the logic.

Closes-Bug: #1757136
Change-Id: I41e9d94345151bc4473d704cdc040ed77870ce45
(cherry picked from commit e362626031)
This commit is contained in:
Guillaume Le Blanc 2018-03-20 14:51:46 +01:00 committed by Akihiro Motoki
parent 3982960fde
commit e41a54d4b6
7 changed files with 188 additions and 228 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

83
openstack_dashboard/api/glance.py
View File

@ -156,6 +156,18 @@ PUBLIC_TO_VISIBILITY_MAP = {
False: 'private'
}
KNOWN_PROPERTIES = [
'visibility', 'protected', 'disk_format',
'container_format', 'min_disk', 'min_ram', 'name',
'properties', 'kernel', 'ramdisk',
'tags', 'import_data', 'source', 'image_id',
'image_url', 'source_type', 'data', 'public',
'checksum', 'created_at', 'deleted', 'is_copying',
'deleted_at', 'is_public', 'virtual_size',
'status', 'size', 'owner', 'id', 'updated_at',
'kernel_id', 'ramdisk_id', 'image_file',
]
def _normalize_is_public_filter(filters):
if not filters:
@ -172,8 +184,7 @@ def _normalize_is_public_filter(filters):
filters['visibility'] = visibility
elif 'visibility' in filters:
# Glance v1: Replace 'visibility' with 'is_public'.
filters['is_public'] = (
getattr(filters, 'visibility', None) == "public")
filters['is_public'] = filters['visibility'] == "public"
del filters['visibility']
@ -415,6 +426,74 @@ class ExternallyUploadedImage(Image):
return self._token_id
def create_image_metadata(data):
"""Generate metadata dict for a new image from a given form data."""
# Default metadata
meta = {'protected': data.get('protected', False),
'disk_format': data.get('disk_format', 'raw'),
'container_format': data.get('container_format', 'bare'),
'min_disk': data.get('min_disk', 0),
'min_ram': data.get('min_ram', 0),
'name': data.get('name', '')}
# Glance does not really do anything with container_format at the
# moment. It requires it is set to the same disk_format for the three
# Amazon image types, otherwise it just treats them as 'bare.' As such
# we will just set that to be that here instead of bothering the user
# with asking them for information we can already determine.
if meta['disk_format'] in ('ami', 'aki', 'ari',):
meta['container_format'] = meta['disk_format']
elif meta['disk_format'] == 'docker':
# To support docker containers we allow the user to specify
# 'docker' as the format. In that case we really want to use
# 'raw' as the disk format and 'docker' as the container format.
meta['disk_format'] = 'raw'
meta['container_format'] = 'docker'
elif meta['disk_format'] == 'ova':
# If the user wishes to upload an OVA using Horizon, then
# 'ova' must be the container format and 'vmdk' must be the disk
# format.
meta['container_format'] = 'ova'
meta['disk_format'] = 'vmdk'
properties = {}
for prop, key in [('description', 'description'),
('kernel_id', 'kernel'),
('ramdisk_id', 'ramdisk'),
('architecture', 'architecture')]:
if data.get(key):
properties[prop] = data[key]
_handle_unknown_properties(data, properties)
if ('visibility' in data and
data['visibility'] not in ['public', 'private', 'shared']):
raise KeyError('invalid visibility option: %s' % data['visibility'])
_normalize_is_public_filter(data)
if VERSIONS.active < 2:
meta['properties'] = properties
meta['is_public'] = data.get('is_public', False)
else:
meta['visibility'] = data.get('visibility', 'private')
meta.update(properties)
return meta
def _handle_unknown_properties(data, properties):
# The Glance API takes in both known and unknown fields. Unknown fields
# are assumed as metadata. To achieve this and continue to use the
# existing horizon api wrapper, we need this function. This way, the
# client REST mirrors the Glance API.
other_props = {
k: v for (k, v) in data.items() if k not in KNOWN_PROPERTIES
}
properties.update(other_props)
@profiler.trace
def image_create(request, **kwargs):
"""Create image.

81
openstack_dashboard/api/rest/glance.py
View File

@ -76,7 +76,7 @@ class Image(generic.View):
http://localhost/api/glance/images/cc758c90-3d98-4ea1-af44-aab405c9c915
"""
meta = create_image_metadata(request.DATA)
meta = _create_image_metadata(request.DATA)
api.glance.image_update(request, image_id, **meta)
@ -171,7 +171,7 @@ class Images(generic.View):
raise rest_utils.AjaxError(500, 'Invalid request')
data = form.clean()
meta = create_image_metadata(request.DATA)
meta = _create_image_metadata(request.DATA)
meta['data'] = data['data']
image = api.glance.image_create(request, **meta)
@ -209,7 +209,7 @@ class Images(generic.View):
This returns the new image object on success.
"""
meta = create_image_metadata(request.DATA)
meta = _create_image_metadata(request.DATA)
if request.DATA.get('image_url'):
if request.DATA.get('import_data'):
@ -304,71 +304,14 @@ class MetadefsResourceTypesList(generic.View):
}
def create_image_metadata(data):
def _create_image_metadata(data):
# In Angular implementation we use 'visibility' field only and
# 'is_public' field is not used when creating/updating metadata.
# However, the previous 'is_public' value is sent in a request.
# We drop it here before passing it to create_image_metadata.
if 'is_public' in data:
del data['is_public']
try:
"""Use the given dict of image form data to generate the metadata used for
creating the image in glance.
"""
meta = {'protected': data.get('protected'),
'min_disk': data.get('min_disk', 0),
'min_ram': data.get('min_ram', 0),
'name': data.get('name'),
'disk_format': data.get('disk_format'),
'container_format': data.get('container_format')}
properties = {}
# 'architecture' will be directly mapped
# into the .properties by the handle_unknown_properties function.
# 'kernel' and 'ramdisk' need to get specifically mapped for backwards
# compatibility.
props = data.get('properties')
if props and props.get('description'):
properties['description'] = props.get('description')
if data.get('kernel'):
properties['kernel_id'] = data.get('kernel')
if data.get('ramdisk'):
properties['ramdisk_id'] = data.get('ramdisk')
handle_unknown_properties(data, properties)
if api.glance.VERSIONS.active >= 2:
meta.update(properties)
else:
meta['properties'] = properties
handle_visibility(data.get('visibility'), meta)
return api.glance.create_image_metadata(data)
except KeyError as e:
raise rest_utils.AjaxError(400,
'missing required parameter %s' % e.args[0])
return meta
def handle_unknown_properties(data, properties):
# The Glance API takes in both known and unknown fields. Unknown fields
# are assumed as metadata. To achieve this and continue to use the
# existing horizon api wrapper, we need this function. This way, the
# client REST mirrors the Glance API.
known_props = ['visibility', 'protected', 'disk_format',
'container_format', 'min_disk', 'min_ram', 'name',
'properties', 'kernel', 'ramdisk',
'tags', 'import_data', 'source',
'image_url', 'source_type', 'data',
'checksum', 'created_at', 'deleted', 'is_copying',
'deleted_at', 'is_public', 'virtual_size',
'status', 'size', 'owner', 'id', 'updated_at']
other_props = {k: v for (k, v) in data.items() if k not in known_props}
properties.update(other_props)
def handle_visibility(visibility, meta):
mapping_to_v1 = {'public': True, 'private': False, 'shared': False}
# note: presence of 'visibility' previously checked for in general call
try:
is_public = mapping_to_v1[visibility]
if api.glance.VERSIONS.active >= 2:
meta['visibility'] = visibility
else:
meta['is_public'] = is_public
except KeyError as e:
raise rest_utils.AjaxError(400,
'invalid visibility option: %s' % e.args[0])
raise rest_utils.AjaxError(400, e.args[0])

100
openstack_dashboard/dashboards/project/images/images/forms.py
View File

@ -43,64 +43,6 @@ IMAGE_FORMAT_CHOICES = IMAGE_BACKEND_SETTINGS.get('image_formats', [])
class ImageURLField(forms.URLField):
default_validators = [validators.URLValidator(schemes=["http", "https"])]
def create_image_metadata(data):
"""Generate metadata dict for a new image from a given form data."""
# Glance does not really do anything with container_format at the
# moment. It requires it is set to the same disk_format for the three
# Amazon image types, otherwise it just treats them as 'bare.' As such
# we will just set that to be that here instead of bothering the user
# with asking them for information we can already determine.
disk_format = data['disk_format']
if disk_format in ('ami', 'aki', 'ari',):
container_format = disk_format
elif disk_format == 'docker':
# To support docker containers we allow the user to specify
# 'docker' as the format. In that case we really want to use
# 'raw' as the disk format and 'docker' as the container format.
disk_format = 'raw'
container_format = 'docker'
elif disk_format == 'ova':
# If the user wishes to upload an OVA using Horizon, then
# 'ova' must be the container format and 'vmdk' must be the disk
# format.
container_format = 'ova'
disk_format = 'vmdk'
else:
container_format = 'bare'
meta = {'protected': data['protected'],
'disk_format': disk_format,
'container_format': container_format,
'min_disk': (data['minimum_disk'] or 0),
'min_ram': (data['minimum_ram'] or 0),
'name': data['name']}
is_public = data.get('is_public', data.get('public', False))
properties = {}
# NOTE(tsufiev): in V2 the way how empty non-base attributes (AKA metadata)
# are handled has changed: in V2 empty metadata is kept in image
# properties, while in V1 they were omitted. Skip empty description (which
# is metadata) to keep the same behavior between V1 and V2
if data.get('description'):
properties['description'] = data['description']
if data.get('kernel'):
properties['kernel_id'] = data['kernel']
if data.get('ramdisk'):
properties['ramdisk_id'] = data['ramdisk']
if data.get('architecture'):
properties['architecture'] = data['architecture']
if api.glance.VERSIONS.active < 2:
meta.update({'is_public': is_public, 'properties': properties})
else:
meta['visibility'] = 'public' if is_public else 'private'
meta.update(properties)
return meta
if api.glance.get_image_upload_mode() == 'direct':
FileField = forms.ExternalFileField
CreateParent = six.with_metaclass(forms.ExternalUploadMeta,
@ -172,13 +114,13 @@ class CreateImageForm(CreateParent):
label=_("Architecture"),
help_text=_('CPU architecture of the image.'),
required=False)
minimum_disk = forms.IntegerField(
min_disk = forms.IntegerField(
label=_("Minimum Disk (GB)"),
min_value=0,
help_text=_('The minimum disk size required to boot the image. '
'If unspecified, this value defaults to 0 (no minimum).'),
required=False)
minimum_ram = forms.IntegerField(
min_ram = forms.IntegerField(
label=_("Minimum RAM (MB)"),
min_value=0,
help_text=_('The minimum memory size required to boot the image. '
@ -308,7 +250,7 @@ class CreateImageForm(CreateParent):
return data
def handle(self, request, data):
meta = create_image_metadata(data)
meta = api.glance.create_image_metadata(data)
# Add image source file or URL to metadata
if (api.glance.get_image_upload_mode() != 'off' and
@ -372,23 +314,19 @@ class UpdateImageForm(forms.SelfHandlingForm):
disk_format = forms.ThemableChoiceField(
label=_("Format"),
)
minimum_disk = forms.IntegerField(label=_("Minimum Disk (GB)"),
min_value=0,
help_text=_('The minimum disk size'
' required to boot the'
' image. If unspecified,'
' this value defaults to'
' 0 (no minimum).'),
required=False)
minimum_ram = forms.IntegerField(label=_("Minimum RAM (MB)"),
min_value=0,
help_text=_('The minimum memory size'
' required to boot the'
' image. If unspecified,'
' this value defaults to'
' 0 (no minimum).'),
required=False)
public = forms.BooleanField(label=_("Public"), required=False)
min_disk = forms.IntegerField(
label=_("Minimum Disk (GB)"),
min_value=0,
help_text=_('The minimum disk size required to boot the image. '
'If unspecified, this value defaults to 0 (no minimum).'),
required=False)
min_ram = forms.IntegerField(
label=_("Minimum RAM (MB)"),
min_value=0,
help_text=_('The minimum memory size required to boot the image. '
'If unspecified, this value defaults to 0 (no minimum).'),
required=False)
is_public = forms.BooleanField(label=_("Public"), required=False)
protected = forms.BooleanField(label=_("Protected"), required=False)
def __init__(self, request, *args, **kwargs):
@ -397,15 +335,15 @@ class UpdateImageForm(forms.SelfHandlingForm):
name in IMAGE_FORMAT_CHOICES
if value]
if not policy.check((("image", "publicize_image"),), request):
self.fields['public'].widget = forms.CheckboxInput(
self.fields['is_public'].widget = forms.CheckboxInput(
attrs={'readonly': 'readonly', 'disabled': 'disabled'})
self.fields['public'].help_text = _(
self.fields['is_public'].help_text = _(
'Non admin users are not allowed to make images public.')
def handle(self, request, data):
image_id = data['image_id']
error_updating = _('Unable to update image "%s".')
meta = create_image_metadata(data)
meta = api.glance.create_image_metadata(data)
try:
image = api.glance.image_update(request, image_id, **meta)

79
openstack_dashboard/dashboards/project/images/images/tests.py
View File

@ -57,8 +57,8 @@ class CreateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
'description': u'Login with admin/admin',
'disk_format': u'qcow2',
'architecture': u'x86-64',
'minimum_disk': 15,
'minimum_ram': 512,
'min_disk': 15,
'min_ram': 512,
'is_public': 1}
files = {}
form = forms.CreateImageForm(post, files)
@ -88,53 +88,6 @@ class CreateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
self.assertNotIn('file', source_type_dict)
mock_image_list.assert_has_calls(image_calls)
@override_settings(OPENSTACK_API_VERSIONS={'image': 1})
def test_create_image_metadata_docker_v1(self):
form_data = {
'name': u'Docker image',
'description': u'Docker image test',
'source_type': u'url',
'image_url': u'/',
'disk_format': u'docker',
'architecture': u'x86-64',
'minimum_disk': 15,
'minimum_ram': 512,
'is_public': False,
'protected': False,
'is_copying': False
}
meta = forms.create_image_metadata(form_data)
self.assertEqual(meta['disk_format'], 'raw')
self.assertEqual(meta['container_format'], 'docker')
self.assertIn('properties', meta)
self.assertNotIn('description', meta)
self.assertNotIn('architecture', meta)
self.assertEqual(meta['properties']['description'],
form_data['description'])
self.assertEqual(meta['properties']['architecture'],
form_data['architecture'])
def test_create_image_metadata_docker_v2(self):
form_data = {
'name': u'Docker image',
'description': u'Docker image test',
'source_type': u'url',
'image_url': u'/',
'disk_format': u'docker',
'architecture': u'x86-64',
'minimum_disk': 15,
'minimum_ram': 512,
'is_public': False,
'protected': False,
'is_copying': False
}
meta = forms.create_image_metadata(form_data)
self.assertEqual(meta['disk_format'], 'raw')
self.assertEqual(meta['container_format'], 'docker')
self.assertNotIn('properties', meta)
self.assertEqual(meta['description'], form_data['description'])
self.assertEqual(meta['architecture'], form_data['architecture'])
class UpdateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
def test_is_format_field_editable(self):
@ -172,8 +125,8 @@ class UpdateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
u'-amd64-disk1.img',
'disk_format': u'qcow2',
'architecture': u'x86-64',
'minimum_disk': 15,
'minimum_ram': 512,
'min_disk': 15,
'min_ram': 512,
'is_public': False,
'protected': False,
'method': 'UpdateImageForm'}
@ -197,8 +150,8 @@ class UpdateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
disk_format=data['disk_format'],
container_format="bare",
name=data['name'],
min_ram=data['minimum_ram'],
min_disk=data['minimum_disk'],
min_ram=data['min_ram'],
min_disk=data['min_disk'],
properties={
'description': data['description'],
'architecture': data['architecture']})
@ -217,8 +170,8 @@ class UpdateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
u'-amd64-disk1.img',
'disk_format': u'qcow2',
'architecture': u'x86-64',
'minimum_disk': 15,
'minimum_ram': 512,
'min_disk': 15,
'min_ram': 512,
'is_public': False,
'protected': False,
'method': 'UpdateImageForm'}
@ -242,8 +195,8 @@ class UpdateImageFormTests(test.ResetImageAPIVersionMixin, test.TestCase):
disk_format=data['disk_format'],
container_format="bare",
name=data['name'],
min_ram=data['minimum_ram'],
min_disk=data['minimum_disk'],
min_ram=data['min_ram'],
min_disk=data['min_disk'],
description=data['description'],
architecture=data['architecture'])
@ -368,8 +321,8 @@ class ImageViewTests(test.ResetImageAPIVersionMixin, test.TestCase):
'description': u'Login with admin/admin',
'disk_format': u'qcow2',
'architecture': u'x86-64',
'minimum_disk': 15,
'minimum_ram': 512,
'min_disk': 15,
'min_ram': 512,
'is_public': True,
'protected': False,
'method': 'CreateImageForm'}
@ -378,8 +331,8 @@ class ImageViewTests(test.ResetImageAPIVersionMixin, test.TestCase):
api_data = {'container_format': 'bare',
'disk_format': data['disk_format'],
'protected': False,
'min_disk': data['minimum_disk'],
'min_ram': data['minimum_ram'],
'min_disk': data['min_disk'],
'min_ram': data['min_ram'],
'name': data['name']}
if api.glance.VERSIONS.active < 2:
api_data.update({'is_public': True,
@ -529,8 +482,8 @@ class ImageViewTests(test.ResetImageAPIVersionMixin, test.TestCase):
'project/images/images/_update.html')
self.assertEqual(res.context['image'].name, image.name)
# Bug 1076216 - is_public checkbox not being set correctly
self.assertContains(res, "<input type='checkbox' id='id_public'"
" name='public' checked='checked'>",
self.assertContains(res, "<input type='checkbox' id='id_is_public'"
" name='is_public' checked='checked'>",
html=True,
msg_prefix="The is_public checkbox is not checked")
mock_image_get.assert_called_once_with(test.IsHttpRequest(),

10
openstack_dashboard/dashboards/project/images/images/views.py
View File

@ -59,8 +59,8 @@ class CreateView(forms.ModalFormView):
'source_type',
'architecture',
'disk_format',
'minimum_disk',
'minimum_ram'
'min_disk',
'min_ram'
]:
tmp = self.request.GET.get(name)
if tmp:
@ -111,9 +111,9 @@ class UpdateView(forms.ModalFormView):
'kernel': properties.get('kernel_id', ''),
'ramdisk': properties.get('ramdisk_id', ''),
'architecture': properties.get('architecture', ''),
'minimum_ram': getattr(image, 'min_ram', None),
'minimum_disk': getattr(image, 'min_disk', None),
'public': getattr(image, 'is_public', None),
'min_ram': getattr(image, 'min_ram', None),
'min_disk': getattr(image, 'min_disk', None),
'is_public': getattr(image, 'is_public', None),
'protected': getattr(image, 'protected', None)}
disk_format = getattr(image, 'disk_format', None)
if (disk_format == 'raw' and

16
openstack_dashboard/test/integration_tests/pages/project/compute/imagespage.py
View File

@ -38,7 +38,7 @@ class ImagesTable(tables.TableRegion):
CREATE_IMAGE_FORM_FIELDS = (
"name", "description", "image_file", "kernel", "ramdisk",
"disk_format", "architecture", "minimum_disk", "minimum_ram",
"disk_format", "architecture", "min_disk", "min_ram",
"is_public", "protected"
)
@ -57,8 +57,8 @@ class ImagesTable(tables.TableRegion):
)
EDIT_IMAGE_FORM_FIELDS = (
"name", "description", "disk_format", "minimum_disk",
"minimum_ram", "public", "protected"
"name", "description", "disk_format", "min_disk",
"min_ram", "public", "protected"
)
@tables.bind_table_action('create')
@ -175,7 +175,7 @@ class ImagesPage(basepage.BaseNavigationPage):
return matches
def edit_image(self, name, new_name=None, description=None,
minimum_disk=None, minimum_ram=None,
min_disk=None, min_ram=None,
public=None, protected=None):
row = self._get_row_with_image_name(name)
confirm_edit_images_form = self.images_table.edit_image(row)
@ -186,11 +186,11 @@ class ImagesPage(basepage.BaseNavigationPage):
if description is not None:
confirm_edit_images_form.description.text = description
if minimum_disk is not None:
confirm_edit_images_form.minimum_disk.value = minimum_disk
if min_disk is not None:
confirm_edit_images_form.min_disk.value = min_disk
if minimum_ram is not None:
confirm_edit_images_form.minimum_ram.value = minimum_ram
if min_ram is not None:
confirm_edit_images_form.min_ram.value = min_ram
if public is True:
confirm_edit_images_form.public.mark()

47
openstack_dashboard/test/unit/api/test_glance.py
View File

@ -349,3 +349,50 @@ class GlanceApiTests(test.APIMockTestCase):
def test_image_create_v2_external_upload(self):
self._test_image_create_external_upload()
@override_settings(OPENSTACK_API_VERSIONS={'image': 1})
def test_create_image_metadata_docker_v1(self):
form_data = {
'name': u'Docker image',
'description': u'Docker image test',
'source_type': u'url',
'image_url': u'/',
'disk_format': u'docker',
'architecture': u'x86-64',
'min_disk': 15,
'min_ram': 512,
'is_public': False,
'protected': False,
'is_copying': False
}
meta = api.glance.create_image_metadata(form_data)
self.assertEqual(meta['disk_format'], 'raw')
self.assertEqual(meta['container_format'], 'docker')
self.assertIn('properties', meta)
self.assertNotIn('description', meta)
self.assertNotIn('architecture', meta)
self.assertEqual(meta['properties']['description'],
form_data['description'])
self.assertEqual(meta['properties']['architecture'],
form_data['architecture'])
def test_create_image_metadata_docker_v2(self):
form_data = {
'name': u'Docker image',
'description': u'Docker image test',
'source_type': u'url',
'image_url': u'/',
'disk_format': u'docker',
'architecture': u'x86-64',
'min_disk': 15,
'min_ram': 512,
'is_public': False,
'protected': False,
'is_copying': False
}
meta = api.glance.create_image_metadata(form_data)
self.assertEqual(meta['disk_format'], 'raw')
self.assertEqual(meta['container_format'], 'docker')
self.assertNotIn('properties', meta)
self.assertEqual(meta['description'], form_data['description'])
self.assertEqual(meta['architecture'], form_data['architecture'])