Allow plugins to mutate image extra_properties
This adds a set_image_extra_properties() method to the action wrapper. It specifically disallows setting os_glance_* properties, for two reasons. First, several (such as the task lock and the store lists) need special handling to be atomic. Second, setting os_glance_* properties from image metadata injection is almost definitely destined for failure, now or later, as it would muck with our internals. As discussed during the work to formally reserve that namespace from the API, we also need to make sure operators do not set these keys during injection. This makes us drop any such keys, with appropriate logging. The next patch will make the metadata injection task use this and is the point at which we will actually change that behavior. Change-Id: I0574ee3daf08d59b4547e353c921451e756e09f6
This commit is contained in:
parent
d2bb1252e6
commit
e54a1a52ec
@ -29,6 +29,7 @@ from taskflow.patterns import linear_flow as lf
|
||||
from taskflow import retry
|
||||
from taskflow import task
|
||||
|
||||
from glance.api import common as api_common
|
||||
import glance.async_.flows._internal_plugins as internal_plugins
|
||||
import glance.async_.flows.plugins as import_plugins
|
||||
from glance.common import exception
|
||||
@ -301,6 +302,24 @@ class _ImportActions(object):
|
||||
raise AttributeError('Setting %s is not allowed' % attr)
|
||||
setattr(self._image, attr, value)
|
||||
|
||||
def set_image_extra_properties(self, properties):
|
||||
"""Merge values into image extra_properties.
|
||||
|
||||
This allows a plugin to set additional properties on the image,
|
||||
as long as those are outside the reserved namespace. Any keys
|
||||
in the internal namespace will be dropped (and logged).
|
||||
|
||||
:param properties: A dict of properties to be merged in
|
||||
"""
|
||||
for key, value in properties.items():
|
||||
if key.startswith(api_common.GLANCE_RESERVED_NS):
|
||||
LOG.warning(('Dropping %(key)s=%(val)s during metadata '
|
||||
'injection for %(image)s'),
|
||||
{'key': key, 'val': value,
|
||||
'image': self.image_id})
|
||||
else:
|
||||
self._image.extra_properties[key] = value
|
||||
|
||||
def remove_location_for_store(self, backend):
|
||||
"""Remove a location from an image given a backend store.
|
||||
|
||||
|
@ -689,6 +689,47 @@ class TestImportActionWrapper(test_utils.BaseTestCase):
|
||||
self.assertRaises(AttributeError,
|
||||
action.set_image_attribute, id='foo')
|
||||
|
||||
@mock.patch.object(import_flow, 'LOG')
|
||||
def test_set_image_extra_properties(self, mock_log):
|
||||
mock_repo = mock.MagicMock()
|
||||
mock_image = mock_repo.get.return_value
|
||||
mock_image.image_id = IMAGE_ID1
|
||||
mock_image.extra_properties = {'os_glance_import_task': TASK_ID1}
|
||||
mock_image.status = 'bar'
|
||||
wrapper = import_flow.ImportActionWrapper(mock_repo, IMAGE_ID1,
|
||||
TASK_ID1)
|
||||
# One banned property
|
||||
with wrapper as action:
|
||||
action.set_image_extra_properties({'os_glance_foo': 'bar'})
|
||||
self.assertEqual({'os_glance_import_task': TASK_ID1},
|
||||
mock_image.extra_properties)
|
||||
|
||||
mock_log.warning.assert_called()
|
||||
mock_log.warning.reset_mock()
|
||||
|
||||
# Two banned properties
|
||||
with wrapper as action:
|
||||
action.set_image_extra_properties({'os_glance_foo': 'bar',
|
||||
'os_glance_baz': 'bat'})
|
||||
self.assertEqual({'os_glance_import_task': TASK_ID1},
|
||||
mock_image.extra_properties)
|
||||
|
||||
mock_log.warning.assert_called()
|
||||
mock_log.warning.reset_mock()
|
||||
|
||||
# One banned and one allowed property
|
||||
with wrapper as action:
|
||||
action.set_image_extra_properties({'foo': 'bar',
|
||||
'os_glance_foo': 'baz'})
|
||||
self.assertEqual({'foo': 'bar',
|
||||
'os_glance_import_task': TASK_ID1},
|
||||
mock_image.extra_properties)
|
||||
|
||||
mock_log.warning.assert_called_once_with(
|
||||
'Dropping %(key)s=%(val)s during metadata injection for %(image)s',
|
||||
{'key': 'os_glance_foo', 'val': 'baz',
|
||||
'image': IMAGE_ID1})
|
||||
|
||||
def test_drop_lock_for_task(self):
|
||||
mock_repo = mock.MagicMock()
|
||||
mock_repo.get.return_value.extra_properties = {
|
||||
|
Loading…
Reference in New Issue
Block a user