diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 19a551c9a7..855e33a991 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -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': { diff --git a/glance/api/v2/policy.py b/glance/api/v2/policy.py index bce036cb51..67e1d5041c 100644 --- a/glance/api/v2/policy.py +++ b/glance/api/v2/policy.py @@ -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') diff --git a/glance/api/v2/router.py b/glance/api/v2/router.py index d8ebdf398a..b2773f62fe 100644 --- a/glance/api/v2/router.py +++ b/glance/api/v2/router.py @@ -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, diff --git a/glance/common/utils.py b/glance/common/utils.py index feb2c81565..fc9f816ab1 100644 --- a/glance/common/utils.py +++ b/glance/common/utils.py @@ -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 diff --git a/glance/policies/base.py b/glance/policies/base.py index 0e74974f8b..9a7ad5ff08 100644 --- a/glance/policies/base.py +++ b/glance/policies/base.py @@ -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.'), ] diff --git a/glance/policies/image.py b/glance/policies/image.py index b4ba6f4b03..ef096a6d4b 100644 --- a/glance/policies/image.py +++ b/glance/policies/image.py @@ -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, diff --git a/glance/tests/functional/v2/test_schemas.py b/glance/tests/functional/v2/test_schemas.py index 3c2d27eff0..22b4cd0886 100644 --- a/glance/tests/functional/v2/test_schemas.py +++ b/glance/tests/functional/v2/test_schemas.py @@ -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())) diff --git a/glance/tests/unit/common/test_utils.py b/glance/tests/unit/common/test_utils.py index 817ea05bb4..ae676171bf 100644 --- a/glance/tests/unit/common/test_utils.py +++ b/glance/tests/unit/common/test_utils.py @@ -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): diff --git a/glance/tests/unit/test_policy.py b/glance/tests/unit/test_policy.py index a78f6b315f..b77aef308b 100644 --- a/glance/tests/unit/test_policy.py +++ b/glance/tests/unit/test_policy.py @@ -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): diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 834fd01126..77e1e0b1ea 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -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): diff --git a/glance/tests/unit/v2/test_schemas_resource.py b/glance/tests/unit/v2/test_schemas_resource.py index e1494ad146..206b0e8802 100644 --- a/glance/tests/unit/v2/test_schemas_resource.py +++ b/glance/tests/unit/v2/test_schemas_resource.py @@ -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): diff --git a/releasenotes/notes/add-new-add-location-api-acd459299976b4a5.yaml b/releasenotes/notes/add-new-add-location-api-acd459299976b4a5.yaml new file mode 100644 index 0000000000..b14f66583d --- /dev/null +++ b/releasenotes/notes/add-new-add-location-api-acd459299976b4a5.yaml @@ -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.