Add new add location api
This change adds a new location api POST /v2/images/{image_id}/locations to add the new location to the image when in `queued` state only. This operation will be service user & image owner only, and validated by the new policy rule `add_image_location`. Implements: blueprint new-location-apis Change-Id: I238c21efd09de296e8928d8fa68bac4b41f81694
This commit is contained in:
parent
a0b7650c4b
commit
4281558dff
@ -928,6 +928,27 @@ class ImagesController(object):
|
||||
except exception.NotAuthenticated as e:
|
||||
raise webob.exc.HTTPUnauthorized(explanation=e.msg)
|
||||
|
||||
def _validate_hashing_data(self, val_data):
|
||||
if 'os_hash_value' in val_data:
|
||||
try:
|
||||
hashval = bytearray.fromhex(val_data['os_hash_value'])
|
||||
except ValueError:
|
||||
msg = (_("os_hash_value (%s) is not a valid hexadecimal"
|
||||
" value") % (val_data['os_hash_value']))
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
hash_algo = val_data.get('os_hash_algo',
|
||||
CONF['hashing_algorithm'])
|
||||
want_size = hashlib.new(hash_algo).digest_size
|
||||
if len(hashval) != want_size:
|
||||
msg = (_("os_hash_value: (%(value)s) is not the correct "
|
||||
"size for (%(algo)s) "
|
||||
"(should be (%(want)d) bytes)") %
|
||||
{'value': val_data['os_hash_value'],
|
||||
'algo': hash_algo,
|
||||
'want': want_size})
|
||||
raise webob.exc.HTTPBadRequest(explanation=msg)
|
||||
|
||||
def _validate_validation_data(self, image, locations):
|
||||
val_data = {}
|
||||
for loc in locations:
|
||||
@ -1108,6 +1129,94 @@ class ImagesController(object):
|
||||
raise webob.exc.HTTPInternalServerError(
|
||||
explanation=encodeutils.exception_to_unicode(e))
|
||||
|
||||
def add_location(self, req, image_id, body):
|
||||
url = body.get('url')
|
||||
validation_data = body.get('validation_data', {})
|
||||
image_repo = self.gateway.get_repo(req.context)
|
||||
ctxt = req.context
|
||||
stole_lock_from_task = None
|
||||
task_factory = self.gateway.get_task_factory(ctxt)
|
||||
task_repo = self.gateway.get_task_repo(ctxt)
|
||||
try:
|
||||
image = image_repo.get(image_id)
|
||||
if image.status != 'queued':
|
||||
msg = _("It's not allowed to add locations if image status is "
|
||||
"%s.") % image.status
|
||||
raise webob.exc.HTTPConflict(explanation=msg)
|
||||
|
||||
api_pol = api_policy.ImageAPIPolicy(req.context, image,
|
||||
self.policy)
|
||||
api_pol.add_location()
|
||||
|
||||
roles = list(set(req.context.roles + req.context.service_roles))
|
||||
if 'service' not in roles:
|
||||
# NOTE(pdeore): Add location API is disabled for other stores
|
||||
# than http
|
||||
if not utils.is_http_store_configured(url):
|
||||
msg = _("http store must be enabled to use location API"
|
||||
" by normal user.")
|
||||
raise exception.Forbidden(msg)
|
||||
|
||||
if validation_data is not None:
|
||||
self._validate_hashing_data(validation_data)
|
||||
|
||||
if 'os_glance_import_task' in image.extra_properties:
|
||||
# NOTE(pdeore): This will raise exception.Conflict if the
|
||||
# lock is present and valid, or return if absent or invalid.
|
||||
stole_lock_from_task = self._enforce_import_lock(req, image)
|
||||
|
||||
task_input = {'image_id': image_id,
|
||||
'loc_url': url,
|
||||
'validation_data': validation_data}
|
||||
|
||||
executor_factory = self.gateway.get_task_executor_factory(
|
||||
ctxt)
|
||||
add_location_task = task_factory.new_task(
|
||||
task_type='location_import',
|
||||
owner=ctxt.owner,
|
||||
task_input=task_input,
|
||||
image_id=image_id,
|
||||
user_id=ctxt.user_id,
|
||||
request_id=ctxt.request_id)
|
||||
|
||||
try:
|
||||
# NOTE(pdeore): Try to grab the lock for this task
|
||||
image_repo.set_property_atomic(image, 'os_glance_import_task',
|
||||
add_location_task.task_id)
|
||||
except exception.Duplicate:
|
||||
msg = (_("New operation on image '%s' is not "
|
||||
"permitted as prior operation is still "
|
||||
"in progress") % image_id)
|
||||
raise exception.Conflict(msg)
|
||||
|
||||
# NOTE(pdeore): We now have the import lock on this image.
|
||||
# If we busted the lock above and have a reference to that
|
||||
# task, try to clean up the import status information left
|
||||
# over from that execution.
|
||||
if stole_lock_from_task:
|
||||
self._cleanup_stale_task_progress(image_repo, image,
|
||||
stole_lock_from_task)
|
||||
|
||||
task_repo.add(add_location_task)
|
||||
task_executor = executor_factory.new_task_executor(ctxt)
|
||||
pool = common.get_thread_pool("tasks_pool")
|
||||
pool.spawn(add_location_task.run, task_executor)
|
||||
except exception.Conflict as e:
|
||||
raise webob.exc.HTTPConflict(explanation=e.msg)
|
||||
except exception.NotFound as e:
|
||||
raise webob.exc.HTTPNotFound(explanation=e.msg)
|
||||
except exception.Forbidden as e:
|
||||
LOG.debug("User not permitted to add location to image '%s'",
|
||||
image_id)
|
||||
raise webob.exc.HTTPForbidden(explanation=e.msg)
|
||||
except exception.NotAuthenticated as e:
|
||||
raise webob.exc.HTTPUnauthorized(explanation=e.msg)
|
||||
except ValueError as e:
|
||||
raise webob.exc.HTTPBadRequest(
|
||||
explanation=encodeutils.exception_to_unicode(e))
|
||||
|
||||
return image_id
|
||||
|
||||
|
||||
class RequestDeserializer(wsgi.JSONRequestDeserializer):
|
||||
|
||||
@ -1550,6 +1659,16 @@ class RequestDeserializer(wsgi.JSONRequestDeserializer):
|
||||
self._validate_import_body(body)
|
||||
return {'body': body}
|
||||
|
||||
def add_location(self, request):
|
||||
body = self._get_request_body(request)
|
||||
values = {'add_location': body}
|
||||
try:
|
||||
self.schema.validate(values)
|
||||
except exception.InvalidObject as e:
|
||||
raise webob.exc.HTTPBadRequest(explanation=e.msg)
|
||||
|
||||
return {'body': body}
|
||||
|
||||
|
||||
class ResponseSerializer(wsgi.JSONResponseSerializer):
|
||||
# These properties will be filtered out from the response and not
|
||||
@ -1692,6 +1811,9 @@ class ResponseSerializer(wsgi.JSONResponseSerializer):
|
||||
def import_image(self, response, result):
|
||||
response.status_int = http.ACCEPTED
|
||||
|
||||
def add_location(self, response, result):
|
||||
response.status_int = http.ACCEPTED
|
||||
|
||||
|
||||
def get_base_properties():
|
||||
return {
|
||||
@ -1834,6 +1956,48 @@ def get_base_properties():
|
||||
'readOnly': True,
|
||||
'description': _('An image schema url'),
|
||||
},
|
||||
'add_location': {
|
||||
'type': 'object',
|
||||
'description': _('Values of location url, do_secure_hash and '
|
||||
'validation_data for new add location API'),
|
||||
'properties': {
|
||||
'url': {
|
||||
'type': 'string',
|
||||
'readOnly': True,
|
||||
'description': _('The URL of the new location to be '
|
||||
'added in the image.')
|
||||
},
|
||||
'validation_data': {
|
||||
'description': _('Values to be used to populate the '
|
||||
'corresponding image properties.'
|
||||
'do_secure_hash is not True then '
|
||||
'image checksum and hash will not be '
|
||||
'calculated so it is the responsibility'
|
||||
' of the consumer of location ADD API '
|
||||
'to provide the correct values in the '
|
||||
'validation_data parameter'),
|
||||
'type': 'object',
|
||||
'writeOnly': True,
|
||||
'additionalProperties': False,
|
||||
'properties': {
|
||||
'os_hash_algo': {
|
||||
'type': 'string',
|
||||
'maxLength': 64,
|
||||
'enum': ['sha1', 'sha256', 'sha512', 'md5'],
|
||||
},
|
||||
'os_hash_value': {
|
||||
'type': 'string',
|
||||
'maxLength': 128,
|
||||
},
|
||||
},
|
||||
'dependentRequired': {
|
||||
"os_hash_value": ["os_hash_algo"],
|
||||
"os_hash_algo": ["os_hash_value"]
|
||||
},
|
||||
},
|
||||
},
|
||||
'required': ['url'],
|
||||
},
|
||||
'locations': {
|
||||
'type': 'array',
|
||||
'items': {
|
||||
|
@ -232,6 +232,9 @@ class ImageAPIPolicy(APIPolicyBase):
|
||||
def get_image_location(self):
|
||||
self._enforce('get_image_location')
|
||||
|
||||
def add_location(self):
|
||||
self._enforce('add_image_location')
|
||||
|
||||
def add_image(self):
|
||||
try:
|
||||
self._enforce('add_image')
|
||||
|
@ -492,6 +492,17 @@ class API(wsgi.Router):
|
||||
action='reject',
|
||||
allowed_methods='PUT')
|
||||
|
||||
# Location APIs
|
||||
image_actions_resource = image_actions.create_resource()
|
||||
mapper.connect('/images/{image_id}/locations',
|
||||
controller=images_resource,
|
||||
action='add_location',
|
||||
conditions={'method': ['POST']})
|
||||
mapper.connect('/images/{image_id}/locations',
|
||||
controller=reject_method_resource,
|
||||
action='reject',
|
||||
allowed_methods='POST')
|
||||
|
||||
image_tags_resource = image_tags.create_resource()
|
||||
mapper.connect('/images/{image_id}/tags/{tag_value}',
|
||||
controller=image_tags_resource,
|
||||
|
@ -735,3 +735,14 @@ def sort_image_locations(locations):
|
||||
sorted_locations = sorted(locations, key=get_store_weight, reverse=True)
|
||||
LOG.debug(('Sorted locations: %s'), sorted_locations)
|
||||
return sorted_locations
|
||||
|
||||
|
||||
def is_http_store_configured(url):
|
||||
if not url.startswith("http"):
|
||||
return False
|
||||
|
||||
enabled_backends = CONF.enabled_backends
|
||||
if enabled_backends:
|
||||
return 'http' in enabled_backends.values()
|
||||
else:
|
||||
return 'http' in CONF.glance_store.stores
|
||||
|
@ -84,6 +84,9 @@ ADMIN_OR_PROJECT_READER_OR_SHARED_MEMBER = (
|
||||
f'{ADMIN} or '
|
||||
f'role:reader and (project_id:%(project_id)s or {IMAGE_MEMBER_CHECK})'
|
||||
)
|
||||
SERVICE_OR_PROJECT_MEMBER = (
|
||||
f'rule:service_api or ({PROJECT_MEMBER} and project_id:%(owner)s)'
|
||||
)
|
||||
|
||||
rules = [
|
||||
policy.RuleDefault(name='default', check_str='',
|
||||
@ -106,6 +109,9 @@ rules = [
|
||||
policy.RuleDefault(name='context_is_admin', check_str='role:admin',
|
||||
description='Defines the rule for the is_admin:True '
|
||||
'check.'),
|
||||
policy.RuleDefault(name='service_api', check_str='role:service',
|
||||
description='Default rule for the service-to-service '
|
||||
'API.'),
|
||||
]
|
||||
|
||||
|
||||
|
@ -188,6 +188,17 @@ image_policies = [
|
||||
deprecated_since=versionutils.deprecated.WALLABY),
|
||||
),
|
||||
|
||||
policy.DocumentedRuleDefault(
|
||||
name="add_image_location",
|
||||
check_str=base.SERVICE_OR_PROJECT_MEMBER,
|
||||
scope_types=['project'],
|
||||
description='Add location URI to given image',
|
||||
operations=[
|
||||
{'path': '/v2/images/{image_id}/locations',
|
||||
'method': 'POST'}
|
||||
],
|
||||
),
|
||||
|
||||
policy.DocumentedRuleDefault(
|
||||
name="add_member",
|
||||
check_str=base.ADMIN_OR_PROJECT_MEMBER,
|
||||
|
@ -59,7 +59,8 @@ class TestSchemas(functional.FunctionalTest):
|
||||
'min_disk',
|
||||
'protected',
|
||||
'os_hidden',
|
||||
'stores'
|
||||
'stores',
|
||||
'add_location',
|
||||
])
|
||||
self.assertEqual(expected, set(image_schema['properties'].keys()))
|
||||
|
||||
|
@ -28,10 +28,13 @@ from glance.common import exception
|
||||
from glance.common import store_utils
|
||||
from glance.common import utils
|
||||
from glance.tests.unit import base
|
||||
import glance.tests.unit.utils as unit_test_utils
|
||||
from glance.tests import utils as test_utils
|
||||
|
||||
CONF = cfg.CONF
|
||||
|
||||
BASE_URI = unit_test_utils.BASE_URI
|
||||
|
||||
|
||||
class TestStoreUtils(test_utils.BaseTestCase):
|
||||
"""Test glance.common.store_utils module"""
|
||||
@ -792,6 +795,56 @@ class TestUtils(test_utils.BaseTestCase):
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
utils.get_stores_from_request, req, body)
|
||||
|
||||
def test_single_store_http_enabled_and_http_not_in_url(self):
|
||||
self.config(stores="http,file", group="glance_store")
|
||||
loc_url = "rbd://aaaaaaaa/images/id"
|
||||
self.assertFalse(utils.is_http_store_configured(loc_url))
|
||||
|
||||
def test_single_store_http_disabled_and_http_in_url(self):
|
||||
self.config(stores="rbd,file", group="glance_store")
|
||||
loc_url = BASE_URI
|
||||
self.assertFalse(utils.is_http_store_configured(loc_url))
|
||||
|
||||
def test_single_store_http_enabled_and_http_in_url(self):
|
||||
self.config(stores="http,file", group="glance_store")
|
||||
loc_url = BASE_URI
|
||||
self.assertTrue(utils.is_http_store_configured(loc_url))
|
||||
|
||||
def test_multiple_store_http_enabled_and_http_not_in_url(self):
|
||||
enabled_backends = {
|
||||
"ceph1": "rbd",
|
||||
"ceph2": "rbd",
|
||||
"http": "http"
|
||||
}
|
||||
self.config(enabled_backends=enabled_backends)
|
||||
store.register_store_opts(CONF)
|
||||
self.config(default_backend="http", group="glance_store")
|
||||
loc_url = "rbd://aaaaaaaa/images/id"
|
||||
self.assertFalse(utils.is_http_store_configured(loc_url))
|
||||
|
||||
def test_multiple_store_http_disabled_and_http_in_url(self):
|
||||
enabled_backends = {
|
||||
"ceph1": "rbd",
|
||||
"ceph2": "rbd",
|
||||
}
|
||||
self.config(enabled_backends=enabled_backends)
|
||||
store.register_store_opts(CONF)
|
||||
self.config(default_backend="ceph1", group="glance_store")
|
||||
loc_url = BASE_URI
|
||||
self.assertFalse(utils.is_http_store_configured(loc_url))
|
||||
|
||||
def test_multiple_store_http_enabled_and_http_in_url(self):
|
||||
enabled_backends = {
|
||||
"ceph1": "rbd",
|
||||
"ceph2": "rbd",
|
||||
"http": "http"
|
||||
}
|
||||
self.config(enabled_backends=enabled_backends)
|
||||
store.register_store_opts(CONF)
|
||||
self.config(default_backend="http", group="glance_store")
|
||||
loc_url = BASE_URI
|
||||
self.assertTrue(utils.is_http_store_configured(loc_url))
|
||||
|
||||
|
||||
class SplitFilterOpTestCase(test_utils.BaseTestCase):
|
||||
|
||||
|
@ -523,6 +523,13 @@ class TestDefaultPolicyCheckStrings(base.IsolatedUnitTest):
|
||||
PROJECT_READER_OR_IMAGE_MEMBER_OR_COMMUNITY_OR_PUBLIC_OR_SHARED
|
||||
)
|
||||
|
||||
def test_service_or_member_check_string(self):
|
||||
expected = (
|
||||
'rule:service_api or (role:member and project_id:%(project_id)s'
|
||||
' and project_id:%(owner)s)'
|
||||
)
|
||||
self.assertEqual(expected, base_policy.SERVICE_OR_PROJECT_MEMBER)
|
||||
|
||||
|
||||
class TestImageTarget(base.IsolatedUnitTest):
|
||||
def test_image_target_ignores_locations(self):
|
||||
|
@ -3933,6 +3933,319 @@ class TestImagesController(base.IsolatedUnitTest):
|
||||
self.assertTrue(image['deleted'])
|
||||
self.assertEqual('deleted', image['status'])
|
||||
|
||||
@mock.patch.object(glance.notifier.TaskFactoryProxy, 'new_task')
|
||||
def test_add_location(self, mock_task):
|
||||
# Test add location without service role but with http store
|
||||
self.config(do_secure_hash=True)
|
||||
self.config(default_store='http', group='glance_store')
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request()
|
||||
url = '%s/fake_location_1' % BASE_URI
|
||||
task_input = {
|
||||
"image_id": image_id,
|
||||
"loc_url": url,
|
||||
"validation_data": {}
|
||||
}
|
||||
request = unit_test_utils.get_fake_request()
|
||||
req_body = {'url': url}
|
||||
self.controller.add_location(request, image_id, req_body)
|
||||
mock_task.assert_called_with(task_type='location_import',
|
||||
owner=TENANT1,
|
||||
task_input=task_input,
|
||||
image_id=image_id,
|
||||
user_id=request.context.user_id,
|
||||
request_id=request.context.request_id)
|
||||
|
||||
@mock.patch.object(glance.notifier.TaskFactoryProxy, 'new_task')
|
||||
def test_add_location_with_service_role(self, mock_task):
|
||||
# Need to make sure 'http' store is not enabled
|
||||
self.config(stores='file', group='glance_store')
|
||||
self.config(do_secure_hash=True)
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request(roles=['service'])
|
||||
url = '%s/fake_location_1' % BASE_URI
|
||||
task_input = {
|
||||
"image_id": image_id,
|
||||
"loc_url": url,
|
||||
"validation_data": {}
|
||||
}
|
||||
req_body = {'url': url}
|
||||
self.controller.add_location(request, image_id, req_body)
|
||||
mock_task.assert_called_with(task_type='location_import',
|
||||
owner=TENANT1,
|
||||
task_input=task_input,
|
||||
image_id=image_id,
|
||||
user_id=request.context.user_id,
|
||||
request_id=request.context.request_id)
|
||||
|
||||
@mock.patch.object(glance.notifier.ImageRepoProxy, 'get')
|
||||
def test_add_location_locked(self, mock_get):
|
||||
task = test_tasks_resource._db_fixture(test_tasks_resource.UUID1,
|
||||
status='pending')
|
||||
self.db.task_create(None, task)
|
||||
image = FakeImage(status='queued')
|
||||
# Image is locked with a valid task that has not aged out, so
|
||||
# the lock will not be busted.
|
||||
image.extra_properties['os_glance_import_task'] = task['id']
|
||||
mock_get.return_value = image
|
||||
|
||||
request = unit_test_utils.get_fake_request(tenant=TENANT1)
|
||||
url = '%s/fake_location_1' % BASE_URI
|
||||
req_body = {'url': url}
|
||||
|
||||
exc = self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller.add_location,
|
||||
request, UUID1, req_body)
|
||||
self.assertEqual('Image has active task', str(exc))
|
||||
|
||||
@mock.patch.object(glance.notifier.ImageRepoProxy, 'save')
|
||||
@mock.patch('glance.db.simple.api.image_set_property_atomic')
|
||||
@mock.patch('glance.db.simple.api.image_delete_property_atomic')
|
||||
@mock.patch.object(glance.notifier.TaskFactoryProxy, 'new_task')
|
||||
@mock.patch.object(glance.notifier.ImageRepoProxy, 'get')
|
||||
def test_add_location_locked_by_bustable_task(self, mock_get, mock_nt,
|
||||
mock_dpi, mock_spi,
|
||||
mock_save,
|
||||
task_status='processing'):
|
||||
if task_status == 'processing':
|
||||
# NOTE(danms): Only set task_input on one of the tested
|
||||
# states to make sure we don't choke on a task without
|
||||
# some of the data set yet.
|
||||
task_input = {'backend': ['store2']}
|
||||
else:
|
||||
task_input = {}
|
||||
task = test_tasks_resource._db_fixture(
|
||||
test_tasks_resource.UUID1,
|
||||
status=task_status,
|
||||
input=task_input)
|
||||
self.db.task_create(None, task)
|
||||
image = FakeImage(status='queued')
|
||||
# Image is locked by a task in 'processing' state
|
||||
image.extra_properties['os_glance_import_task'] = task['id']
|
||||
image.extra_properties['os_glance_importing_to_stores'] = 'store2'
|
||||
mock_get.return_value = image
|
||||
|
||||
request = unit_test_utils.get_fake_request(tenant=TENANT1)
|
||||
url = '%s/fake_location_1' % BASE_URI
|
||||
req_body = {'url': url}
|
||||
|
||||
# Task has only been running for ten minutes
|
||||
time_fixture = fixture.TimeFixture(task['updated_at'] +
|
||||
datetime.timedelta(minutes=10))
|
||||
self.useFixture(time_fixture)
|
||||
|
||||
mock_nt.return_value.task_id = 'mytask'
|
||||
|
||||
# Task holds the lock, API refuses to bust it
|
||||
self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller.add_location,
|
||||
request, UUID1, req_body)
|
||||
mock_dpi.assert_not_called()
|
||||
mock_spi.assert_not_called()
|
||||
mock_nt.assert_not_called()
|
||||
|
||||
# Fast forward to 90 minutes from now
|
||||
time_fixture.advance_time_delta(datetime.timedelta(minutes=90))
|
||||
self.controller.add_location(request, UUID1, req_body)
|
||||
|
||||
# API deleted the other task's lock and locked it for us
|
||||
mock_dpi.assert_called_once_with(image.id, 'os_glance_import_task',
|
||||
task['id'])
|
||||
mock_spi.assert_called_once_with(image.id, 'os_glance_import_task',
|
||||
'mytask')
|
||||
|
||||
# If previous operation is still in processing, new operation
|
||||
# is not allowed
|
||||
mock_nt.return_value.task_id = 'mytask1'
|
||||
mock_spi.side_effect = exception.Duplicate
|
||||
request = unit_test_utils.get_fake_request(tenant=TENANT2)
|
||||
url = '%s/fake_location_2' % BASE_URI
|
||||
req_body = {'url': url}
|
||||
self.assertRaises(webob.exc.HTTPConflict,
|
||||
self.controller.add_location,
|
||||
request, UUID1, req_body)
|
||||
# If we stored task_input with information about the stores
|
||||
# and thus triggered the cleanup code, make sure that cleanup
|
||||
# happened here.
|
||||
if task_status == 'processing':
|
||||
self.assertNotIn('store2',
|
||||
image.extra_properties[
|
||||
'os_glance_importing_to_stores'])
|
||||
|
||||
def test_add_location_locked_by_bustable_terminal_task_failure(self):
|
||||
# Make sure we don't fail with a task status transition error
|
||||
self.test_add_location_locked_by_bustable_task(task_status='failure')
|
||||
|
||||
def test_add_location_locked_by_bustable_terminal_task_success(self):
|
||||
# Make sure we don't fail with a task status transition error
|
||||
self.test_add_location_locked_by_bustable_task(task_status='success')
|
||||
|
||||
def test_add_location_by_non_owner(self):
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request()
|
||||
enforcer = unit_test_utils.enforcer_from_rules({
|
||||
"get_image": "",
|
||||
"add_location": "'{0}':%(owner)s".format(TENANT2)
|
||||
})
|
||||
self.controller.policy = enforcer
|
||||
req_body = {'url': '%s/fake_location_1' % BASE_URI}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPForbidden,
|
||||
'You are not authorized to complete add_image_location action.',
|
||||
self.controller.add_location,
|
||||
request, image_id, req_body)
|
||||
|
||||
def test_add_location_without_service_role(self):
|
||||
# Need to make sure 'http' store is not enabled
|
||||
self.config(stores='file', group='glance_store')
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request(roles=['admin', 'member'])
|
||||
req_body = {'url': 'file://%s/%s' % (self.test_dir, UUID7)}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPForbidden,
|
||||
'http store must be enabled to use location API by normal user.',
|
||||
self.controller.add_location, request, image_id, req_body)
|
||||
|
||||
def test_add_location_to_invalid_image(self):
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request()
|
||||
req_body = {'url': '%s/fake_location_1' % BASE_URI}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPNotFound,
|
||||
'No image found with ID .*',
|
||||
self.controller.add_location,
|
||||
request, str(uuid.uuid4()), req_body)
|
||||
|
||||
def test_add_location_to_active_image(self):
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='active'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request()
|
||||
req_body = {'url': '%s/fake_location_1' % BASE_URI}
|
||||
self.assertRaises(
|
||||
webob.exc.HTTPConflict,
|
||||
self.controller.add_location,
|
||||
request, image_id, req_body)
|
||||
|
||||
@mock.patch.object(store, 'get_size_from_backend')
|
||||
def test_add_location_with_invalid_validation_data(
|
||||
self, mock_get_size):
|
||||
mock_get_size.return_value = 1
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
checksum=None,
|
||||
os_hash_algo=None,
|
||||
os_hash_value=None,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued'),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
request = unit_test_utils.get_fake_request()
|
||||
validation_data = {
|
||||
'os_hash_algo': 'sha256',
|
||||
'os_hash_value': MULTIHASH1,
|
||||
}
|
||||
req_body = {
|
||||
'url': '%s/fake_location_1' % BASE_URI,
|
||||
'validation_data': validation_data
|
||||
}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'os_hash_value: .* is not the correct size',
|
||||
self.controller.add_location,
|
||||
request, image_id, req_body)
|
||||
|
||||
validation_data = {
|
||||
'os_hash_algo': 'sha123',
|
||||
'os_hash_value': MULTIHASH1,
|
||||
}
|
||||
req_body = {
|
||||
'url': '%s/fake_location_1' % BASE_URI,
|
||||
'validation_data': validation_data
|
||||
}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'unsupported hash type .*',
|
||||
self.controller.add_location,
|
||||
request, image_id, req_body)
|
||||
|
||||
validation_data = {
|
||||
'os_hash_algo': 'sha512',
|
||||
'os_hash_value': 'not a hex value',
|
||||
}
|
||||
req_body = {
|
||||
'url': '%s/fake_location_1' % BASE_URI,
|
||||
'validation_data': validation_data
|
||||
}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'os_hash_value .* is not a valid hexadecimal value',
|
||||
self.controller.add_location,
|
||||
request, image_id, req_body)
|
||||
|
||||
validation_data = {
|
||||
'os_hash_algo': 'sha512',
|
||||
'os_hash_value': '0123456789abcdef',
|
||||
}
|
||||
req_body = {
|
||||
'url': '%s/fake_location_1' % BASE_URI,
|
||||
'validation_data': validation_data
|
||||
}
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'os_hash_value: .* is not the correct size',
|
||||
self.controller.add_location,
|
||||
request, image_id, req_body)
|
||||
|
||||
|
||||
class TestImagesControllerPolicies(base.IsolatedUnitTest):
|
||||
|
||||
@ -4071,6 +4384,14 @@ class TestImagesControllerPolicies(base.IsolatedUnitTest):
|
||||
self.assertRaises(webob.exc.HTTPForbidden, self.controller.delete,
|
||||
request, UUID1)
|
||||
|
||||
def test_add_location_unauthorized(self):
|
||||
rules = {"add_image_location": False}
|
||||
self.policy.set_rules(rules)
|
||||
body = {'url': '%s/fake_location' % BASE_URI}
|
||||
request = unit_test_utils.get_fake_request()
|
||||
self.assertRaises(webob.exc.HTTPForbidden,
|
||||
self.controller.add_location, request, UUID1, body)
|
||||
|
||||
|
||||
class TestImagesDeserializer(test_utils.BaseTestCase):
|
||||
|
||||
@ -4961,6 +5282,106 @@ class TestImagesDeserializer(test_utils.BaseTestCase):
|
||||
self.deserializer.import_image,
|
||||
request)
|
||||
|
||||
def test_add_location_no_body(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.deserializer.add_location,
|
||||
request)
|
||||
|
||||
def test_add_location(self):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
body = {
|
||||
'url': 'scheme1://path1',
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
output = self.deserializer.add_location(request)
|
||||
expected = {"body": body}
|
||||
self.assertEqual(expected, output)
|
||||
|
||||
def test_add_location_with_invalid_body(self):
|
||||
request = self._get_fake_patch_request()
|
||||
body = {
|
||||
'do_secure_hash': True
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'Provided object does not match schema',
|
||||
self.deserializer.add_location, request)
|
||||
|
||||
body = {
|
||||
'url': 'scheme1://path2',
|
||||
'validation_data': {
|
||||
'os_hash_algo': 'sha123',
|
||||
'os_hash_value': MULTIHASH1,
|
||||
}
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'Provided object does not match schema',
|
||||
self.deserializer.add_location, request)
|
||||
|
||||
body = {
|
||||
'url': 'scheme1://path2',
|
||||
'validation_data': {
|
||||
'os_hash_algo': 'sha123',
|
||||
}
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'Provided object does not match schema',
|
||||
self.deserializer.add_location, request)
|
||||
|
||||
body = {
|
||||
'url': 'scheme1://path2',
|
||||
'validation_data': {
|
||||
'os_hash_value': MULTIHASH1,
|
||||
}
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'Provided object does not match schema',
|
||||
self.deserializer.add_location, request)
|
||||
|
||||
body = {
|
||||
'url': 'scheme1://path2',
|
||||
'validation_data': {
|
||||
'os_hash_algo': 'sha512',
|
||||
'os_hash_value': MULTIHASH1,
|
||||
'bogus_value': 'test'
|
||||
}
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'Provided object does not match schema',
|
||||
self.deserializer.add_location, request)
|
||||
|
||||
body = {
|
||||
'url': 'scheme1://path2',
|
||||
'validation_data': {
|
||||
'bogus_value': 'test'
|
||||
}
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.assertRaisesRegex(
|
||||
webob.exc.HTTPBadRequest,
|
||||
'Provided object does not match schema',
|
||||
self.deserializer.add_location, request)
|
||||
|
||||
body = {
|
||||
'url': 'scheme1://path2',
|
||||
'validation_data': {
|
||||
'os_hash_algo': 'sha512',
|
||||
'os_hash_value': MULTIHASH1,
|
||||
}
|
||||
}
|
||||
request.body = jsonutils.dump_as_bytes(body)
|
||||
self.deserializer.add_location(request)
|
||||
|
||||
|
||||
class TestImagesDeserializerWithExtendedSchema(test_utils.BaseTestCase):
|
||||
|
||||
@ -5391,6 +5812,12 @@ class TestImagesSerializer(test_utils.BaseTestCase):
|
||||
self.assertIn('foo', actual)
|
||||
self.assertNotIn('os_glance_stage_host', actual)
|
||||
|
||||
def test_add_location(self):
|
||||
response = webob.Response()
|
||||
self.serializer.add_location(response, {})
|
||||
self.assertEqual(http.ACCEPTED, response.status_int)
|
||||
self.assertEqual('0', response.headers['Content-Length'])
|
||||
|
||||
|
||||
class TestImagesSerializerWithUnicode(test_utils.BaseTestCase):
|
||||
|
||||
@ -6201,6 +6628,48 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest):
|
||||
|
||||
self.assertEqual(UUID7, output)
|
||||
|
||||
@mock.patch.object(glance.notifier.TaskFactoryProxy, 'new_task')
|
||||
@mock.patch.object(glance.quota, '_calc_required_size')
|
||||
@mock.patch.object(glance.location, '_check_image_location')
|
||||
@mock.patch.object(glance.location.ImageRepoProxy, '_set_acls')
|
||||
@mock.patch.object(store, 'get_size_from_uri_and_backend')
|
||||
def test_add_location(self,
|
||||
mock_get_size_uri,
|
||||
mock_set_acls,
|
||||
mock_check_loc,
|
||||
mock_calc,
|
||||
mock_task):
|
||||
self.config(do_secure_hash=True)
|
||||
mock_calc.return_value = 1
|
||||
mock_get_size_uri.return_value = 1
|
||||
image_id = str(uuid.uuid4())
|
||||
self.images = [
|
||||
_db_fixture(image_id, owner=TENANT1,
|
||||
name='1',
|
||||
disk_format='raw',
|
||||
container_format='bare',
|
||||
status='queued',
|
||||
checksum=None,
|
||||
os_hash_algo=None,
|
||||
os_hash_value=None),
|
||||
]
|
||||
self.db.image_create(None, self.images[0])
|
||||
url = 'file://%s/%s' % (self.test_dir, UUID7)
|
||||
task_input = {
|
||||
"image_id": image_id,
|
||||
"loc_url": url,
|
||||
"validation_data": {}
|
||||
}
|
||||
request = unit_test_utils.get_fake_request(roles=['service'])
|
||||
req_body = {'url': url}
|
||||
self.controller.add_location(request, image_id, req_body)
|
||||
mock_task.assert_called_with(task_type='location_import',
|
||||
owner=TENANT1,
|
||||
task_input=task_input,
|
||||
image_id=image_id,
|
||||
user_id=request.context.user_id,
|
||||
request_id=request.context.request_id)
|
||||
|
||||
|
||||
class TestProxyHelpers(base.IsolatedUnitTest):
|
||||
def test_proxy_response_error(self):
|
||||
|
@ -34,7 +34,8 @@ class TestSchemasController(test_utils.BaseTestCase):
|
||||
'file', 'container_format', 'schema', 'id', 'size',
|
||||
'direct_url', 'min_ram', 'min_disk', 'protected',
|
||||
'locations', 'owner', 'virtual_size', 'os_hidden',
|
||||
'os_hash_algo', 'os_hash_value', 'stores'])
|
||||
'os_hash_algo', 'os_hash_value', 'stores',
|
||||
'add_location'])
|
||||
self.assertEqual(expected, set(output['properties'].keys()))
|
||||
|
||||
def test_image_has_correct_statuses(self):
|
||||
|
@ -0,0 +1,21 @@
|
||||
---
|
||||
features:
|
||||
- |
|
||||
This release brings the additional functionality of adding new
|
||||
location to a ``queued`` state image which will replace the
|
||||
image-update mechanism for consumers like cinder and nova to
|
||||
address OSSN-0090 and OSSN-0065.
|
||||
issues:
|
||||
- |
|
||||
In case of ``http`` store if bad value is passed for ``os_hash_value``
|
||||
in validation data then task fails which is expected but it stores
|
||||
location of the image which is wrong, that needs to be popped out.
|
||||
The location doesn't get deleted because deletion of locatio is not
|
||||
allowed for ``http`` store. Here image needs to be deleted as it is
|
||||
of no use.
|
||||
- |
|
||||
During validation of hashing data when do_secure_hash is `false`,
|
||||
we can just validate length expected for hash_algo and not actual
|
||||
expected hash value. If garbage hash_value with expected size has
|
||||
been provided, image becomes active after adding location but it will
|
||||
be of no use as download or boot will fail with corrupt image error.
|
Loading…
Reference in New Issue
Block a user