Enforce keystone limits for image upload
This adds enforcement of the image_size_total keystone limit for image upload and import. We simply check the quota before either of these operations and refuse to proceed further if the user is over their quota. Note that this disables checking of the global size quota if keystone quotas are enabled. Note this includes another fix to couple unit tests that do not properly pass context to the get_flow() method. Partially-implements: blueprint glance-unified-quotas Change-Id: Idf5f004b72436df1f9c77bb32d60b9be5ae77a68
This commit is contained in:
parent
f3eb5214a3
commit
76c3011a64
|
@ -152,6 +152,11 @@ def check_quota(context, image_size, db_api, image_id=None):
|
|||
:returns:
|
||||
"""
|
||||
|
||||
# NOTE(danms): If keystone quotas are enabled, those take
|
||||
# precedence and this check is a no-op.
|
||||
if CONF.use_keystone_limits:
|
||||
return
|
||||
|
||||
remaining = get_remaining_quota(context, db_api, image_id=image_id)
|
||||
|
||||
if remaining is None:
|
||||
|
|
|
@ -34,6 +34,7 @@ import glance.db
|
|||
import glance.gateway
|
||||
from glance.i18n import _, _LE, _LI
|
||||
import glance.notifier
|
||||
from glance.quota import keystone as ks_quota
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
@ -121,6 +122,12 @@ class ImageDataController(object):
|
|||
|
||||
@utils.mutating
|
||||
def upload(self, req, image_id, data, size):
|
||||
try:
|
||||
ks_quota.enforce_image_size_total(req.context, req.context.owner)
|
||||
except exception.LimitExceeded as e:
|
||||
raise webob.exc.HTTPRequestEntityTooLarge(explanation=str(e),
|
||||
request=req)
|
||||
|
||||
backend = None
|
||||
if CONF.enabled_backends:
|
||||
backend = req.headers.get('x-image-meta-store',
|
||||
|
|
|
@ -46,6 +46,7 @@ import glance.db
|
|||
import glance.gateway
|
||||
from glance.i18n import _, _LE, _LI, _LW
|
||||
import glance.notifier
|
||||
from glance.quota import keystone as ks_quota
|
||||
import glance.schema
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
@ -310,6 +311,12 @@ class ImagesController(object):
|
|||
all_stores_must_succeed = body.get('all_stores_must_succeed', True)
|
||||
stole_lock_from_task = None
|
||||
|
||||
try:
|
||||
ks_quota.enforce_image_size_total(req.context, req.context.owner)
|
||||
except exception.LimitExceeded as e:
|
||||
raise webob.exc.HTTPRequestEntityTooLarge(explanation=str(e),
|
||||
request=req)
|
||||
|
||||
try:
|
||||
image = image_repo.get(image_id)
|
||||
if image.status == 'active' and import_method != "copy-image":
|
||||
|
|
|
@ -22,6 +22,7 @@ from glance_store import exceptions as store_exceptions
|
|||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import encodeutils
|
||||
from oslo_utils import excutils
|
||||
from oslo_utils import timeutils
|
||||
from oslo_utils import units
|
||||
import six
|
||||
|
@ -38,6 +39,7 @@ from glance.common.scripts.image_import import main as image_import
|
|||
from glance.common.scripts import utils as script_utils
|
||||
from glance.common import store_utils
|
||||
from glance.i18n import _, _LE, _LI
|
||||
from glance.quota import keystone as ks_quota
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
@ -738,6 +740,27 @@ class _CompleteTask(task.Task):
|
|||
{'task_id': self.task_id, 'task_type': self.task_type})
|
||||
|
||||
|
||||
def assert_quota(context, task_repo, task_id, stores,
|
||||
action_wrapper, enforce_quota_fn,
|
||||
**enforce_kwargs):
|
||||
try:
|
||||
enforce_quota_fn(context, context.owner, **enforce_kwargs)
|
||||
except exception.LimitExceeded as e:
|
||||
with excutils.save_and_reraise_exception():
|
||||
with action_wrapper as action:
|
||||
action.remove_importing_stores(stores)
|
||||
if action.image_status == 'importing':
|
||||
action.set_image_attribute(status='queued')
|
||||
action_wrapper.drop_lock_for_task()
|
||||
task = script_utils.get_task(task_repo, task_id)
|
||||
if task is None:
|
||||
LOG.error(_LE('Failed to find task %r to update after '
|
||||
'quota failure'), task_id)
|
||||
else:
|
||||
task.fail(str(e))
|
||||
task_repo.save(task)
|
||||
|
||||
|
||||
def get_flow(**kwargs):
|
||||
"""Return task flow
|
||||
|
||||
|
@ -838,8 +861,19 @@ def get_flow(**kwargs):
|
|||
with action_wrapper as action:
|
||||
if import_method != 'copy-image':
|
||||
action.set_image_attribute(status='importing')
|
||||
image_size = (action.image_size or 0) // units.Mi
|
||||
action.add_importing_stores(stores)
|
||||
action.remove_failed_stores(stores)
|
||||
action.pop_extra_property('os_glance_stage_host')
|
||||
|
||||
# After we have marked the image as intended, check quota to make
|
||||
# sure we are not over a limit, otherwise we roll back.
|
||||
if import_method == 'glance-direct':
|
||||
# We know the size of the image in staging, so we can check
|
||||
# against available image_size_total quota.
|
||||
assert_quota(kwargs['context'], task_repo, task_id,
|
||||
stores, action_wrapper,
|
||||
ks_quota.enforce_image_size_total,
|
||||
delta=image_size)
|
||||
|
||||
return flow
|
||||
|
|
|
@ -21,6 +21,7 @@ import time
|
|||
import uuid
|
||||
|
||||
import fixtures
|
||||
from oslo_limit import exception as ol_exc
|
||||
from oslo_serialization import jsonutils
|
||||
from oslo_utils.secretutils import md5
|
||||
from oslo_utils import units
|
||||
|
@ -31,6 +32,7 @@ from six.moves import http_client as http
|
|||
from six.moves import range
|
||||
from six.moves import urllib
|
||||
|
||||
from glance.quota import keystone as ks_quota
|
||||
from glance.tests import functional
|
||||
from glance.tests.functional import ft_utils as func_utils
|
||||
from glance.tests import utils as test_utils
|
||||
|
@ -7022,3 +7024,137 @@ class TestImportProxy(functional.SynchronousAPIBase):
|
|||
self._test_import_proxy_fail_requests(
|
||||
requests.exceptions.RequestException(),
|
||||
'502 Bad Gateway')
|
||||
|
||||
|
||||
def get_enforcer_class(limits):
|
||||
class FakeEnforcer:
|
||||
def __init__(self, callback):
|
||||
self._callback = callback
|
||||
|
||||
def enforce(self, project_id, values):
|
||||
for name, delta in values.items():
|
||||
current = self._callback(project_id, values.keys())
|
||||
if current.get(name) + delta > limits.get(name, 0):
|
||||
raise ol_exc.ProjectOverLimit(
|
||||
project_id=project_id,
|
||||
over_limit_info_list=[ol_exc.OverLimitInfo(
|
||||
name, limits.get(name), current.get(name), delta)])
|
||||
|
||||
return FakeEnforcer
|
||||
|
||||
|
||||
class TestKeystoneQuotas(functional.SynchronousAPIBase):
|
||||
def setUp(self):
|
||||
super(TestKeystoneQuotas, self).setUp()
|
||||
self.config(use_keystone_limits=True)
|
||||
self.config(filesystem_store_datadir='/tmp/foo',
|
||||
group='os_glance_tasks_store')
|
||||
|
||||
self.enforcer_mock = self.useFixture(
|
||||
fixtures.MockPatchObject(ks_quota, 'limit')).mock
|
||||
|
||||
def set_limit(self, limits):
|
||||
self.enforcer_mock.Enforcer = get_enforcer_class(limits)
|
||||
|
||||
def test_upload(self):
|
||||
# Set a quota of 5MiB
|
||||
self.set_limit({'image_size_total': 5})
|
||||
self.start_server()
|
||||
|
||||
# First upload of 3MiB is good
|
||||
image_id = self._create_and_upload(
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Second upload of 3MiB is allowed to complete, but leaves us
|
||||
# over-quota
|
||||
self._create_and_upload(
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Third upload of any size fails because we are now over quota
|
||||
self._create_and_upload(expected_code=413)
|
||||
|
||||
# Delete one image, which should put us under quota
|
||||
self.api_delete('/v2/images/%s' % image_id)
|
||||
|
||||
# Upload should now succeed
|
||||
self._create_and_upload()
|
||||
|
||||
def test_import(self):
|
||||
# Set a quota of 5MiB
|
||||
self.set_limit({'image_size_total': 5})
|
||||
self.start_server()
|
||||
|
||||
# First upload of 3MiB is good
|
||||
image_id = self._create_and_upload(
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Second upload of 3MiB is allowed to complete, but leaves us
|
||||
# over-quota
|
||||
self._create_and_upload(data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Attempt to import of any size fails because we are now over quota
|
||||
self._create_and_import(stores=['store1'], expected_code=413)
|
||||
|
||||
# Delete one image, which should put us under quota
|
||||
self.api_delete('/v2/images/%s' % image_id)
|
||||
|
||||
# Import should now succeed
|
||||
self._create_and_import(stores=['store1'])
|
||||
|
||||
def test_import_would_go_over(self):
|
||||
# Set a quota limit of 5MiB
|
||||
self.set_limit({'image_size_total': 5})
|
||||
self.start_server()
|
||||
|
||||
# First upload of 3MiB is good
|
||||
image_id = self._create_and_upload(
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Stage a 3MiB image for later import
|
||||
import_id = self._create_and_stage(
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Import should fail the task because it would put us over our
|
||||
# 5MiB quota
|
||||
self._import_direct(import_id, ['store1'])
|
||||
image = self._wait_for_import(import_id)
|
||||
task = self._get_latest_task(import_id)
|
||||
self.assertEqual('failure', task['status'])
|
||||
self.assertIn(('image_size_total is over limit of 5 due to '
|
||||
'current usage 3 and delta 3'), task['message'])
|
||||
|
||||
# Delete the first image to make space
|
||||
resp = self.api_delete('/v2/images/%s' % image_id)
|
||||
self.assertEqual(204, resp.status_code)
|
||||
|
||||
# Stage a 3MiB image for later import (this must be done
|
||||
# because a failed import cannot go back to 'uploading' status)
|
||||
import_id = self._create_and_stage(
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
# Make sure the import is possible now
|
||||
resp = self._import_direct(import_id, ['store1'])
|
||||
self.assertEqual(202, resp.status_code)
|
||||
image = self._wait_for_import(import_id)
|
||||
self.assertEqual('active', image['status'])
|
||||
task = self._get_latest_task(import_id)
|
||||
self.assertEqual('success', task['status'])
|
||||
|
||||
def test_copy(self):
|
||||
# Set a quota of 5MiB
|
||||
self.set_limit({'image_size_total': 5})
|
||||
self.start_server()
|
||||
|
||||
# First import of 3MiB is good
|
||||
image_id = self._create_and_import(
|
||||
stores=['store1'],
|
||||
data_iter=test_utils.FakeData(3 * units.Mi))
|
||||
|
||||
# Second copy is allowed to complete, but leaves us us at
|
||||
# 6MiB of total usage, over quota
|
||||
req = self._import_copy(image_id, ['store2'])
|
||||
self.assertEqual(202, req.status_code)
|
||||
self._wait_for_import(image_id)
|
||||
|
||||
# Third copy should fail because we're over quota
|
||||
req = self._import_copy(image_id, ['store3'])
|
||||
self.assertEqual(413, req.status_code)
|
||||
|
|
|
@ -79,6 +79,7 @@ class TestApiImageImportTask(test_utils.BaseTestCase):
|
|||
"task_repo": self.mock_task_repo,
|
||||
"image_repo": self.mock_image_repo,
|
||||
"image_id": IMAGE_ID1,
|
||||
"context": mock.MagicMock(),
|
||||
"import_req": import_req}
|
||||
|
||||
mock_lf_init.return_value = None
|
||||
|
@ -111,12 +112,68 @@ class TestApiImageImportTask(test_utils.BaseTestCase):
|
|||
task_repo=self.mock_task_repo,
|
||||
image_repo=self.mock_image_repo,
|
||||
image_id=IMAGE_ID1,
|
||||
context=mock.MagicMock(),
|
||||
import_req=self.gd_task_input['import_req'])
|
||||
self.assertNotIn('os_glance_stage_host',
|
||||
self.mock_image.extra_properties)
|
||||
self.assertIn('os_glance_import_task',
|
||||
self.mock_image.extra_properties)
|
||||
|
||||
def test_assert_quota_no_task(self):
|
||||
ignored = mock.MagicMock()
|
||||
task_repo = mock.MagicMock()
|
||||
task_repo.get.return_value = None
|
||||
task_id = 'some-task'
|
||||
enforce_fn = mock.MagicMock()
|
||||
enforce_fn.side_effect = exception.LimitExceeded
|
||||
with mock.patch.object(import_flow, 'LOG') as mock_log:
|
||||
self.assertRaises(exception.LimitExceeded,
|
||||
import_flow.assert_quota,
|
||||
ignored, task_repo, task_id,
|
||||
[], ignored, enforce_fn)
|
||||
task_repo.get.assert_called_once_with('some-task')
|
||||
# Make sure we logged instead of crashed if no task was found
|
||||
mock_log.error.assert_called_once_with('Failed to find task %r to '
|
||||
'update after quota failure',
|
||||
'some-task')
|
||||
task_repo.save.assert_not_called()
|
||||
|
||||
def test_assert_quota(self):
|
||||
ignored = mock.MagicMock()
|
||||
task_repo = mock.MagicMock()
|
||||
task_id = 'some-task'
|
||||
enforce_fn = mock.MagicMock()
|
||||
enforce_fn.side_effect = exception.LimitExceeded
|
||||
wrapper = mock.MagicMock()
|
||||
action = wrapper.__enter__.return_value
|
||||
action.image_status = 'importing'
|
||||
self.assertRaises(exception.LimitExceeded,
|
||||
import_flow.assert_quota,
|
||||
ignored, task_repo, task_id,
|
||||
['store1'], wrapper, enforce_fn)
|
||||
action.remove_importing_stores.assert_called_once_with(['store1'])
|
||||
action.set_image_attribute.assert_called_once_with(status='queued')
|
||||
task_repo.get.assert_called_once_with('some-task')
|
||||
task_repo.save.assert_called_once_with(task_repo.get.return_value)
|
||||
|
||||
def test_assert_quota_copy(self):
|
||||
ignored = mock.MagicMock()
|
||||
task_repo = mock.MagicMock()
|
||||
task_id = 'some-task'
|
||||
enforce_fn = mock.MagicMock()
|
||||
enforce_fn.side_effect = exception.LimitExceeded
|
||||
wrapper = mock.MagicMock()
|
||||
action = wrapper.__enter__.return_value
|
||||
action.image_status = 'active'
|
||||
self.assertRaises(exception.LimitExceeded,
|
||||
import_flow.assert_quota,
|
||||
ignored, task_repo, task_id,
|
||||
['store1'], wrapper, enforce_fn)
|
||||
action.remove_importing_stores.assert_called_once_with(['store1'])
|
||||
action.set_image_attribute.assert_not_called()
|
||||
task_repo.get.assert_called_once_with('some-task')
|
||||
task_repo.save.assert_called_once_with(task_repo.get.return_value)
|
||||
|
||||
|
||||
class TestImageLock(test_utils.BaseTestCase):
|
||||
def setUp(self):
|
||||
|
|
|
@ -183,6 +183,30 @@ class TestImageQuota(test_utils.BaseTestCase):
|
|||
self._quota_exceeded_size(str(quota), data, deleted=False,
|
||||
size=quota - 1)
|
||||
|
||||
def test_quota_exceeded_keystone_quotas(self):
|
||||
# Set our global limit to a tiny ten bytes
|
||||
self.config(user_storage_quota='10B')
|
||||
context = FakeContext()
|
||||
db_api = unit_test_utils.FakeDB()
|
||||
store_api = unit_test_utils.FakeStoreAPI()
|
||||
store = unit_test_utils.FakeStoreUtils(store_api)
|
||||
base_image = FakeImage()
|
||||
base_image.image_id = 'id'
|
||||
image = glance.quota.ImageProxy(base_image, context, db_api, store)
|
||||
|
||||
# With keystone quotas disabled, a 100 byte image should fail the
|
||||
# global limit.
|
||||
data = '*' * 100
|
||||
self.assertRaises(exception.StorageQuotaFull,
|
||||
image.set_data,
|
||||
data,
|
||||
size=len(data))
|
||||
|
||||
# If we turn on keystone quotas, the global limit gets ignored
|
||||
# so the same image no longer fails.
|
||||
self.config(use_keystone_limits=True)
|
||||
image.set_data(data, size=len(data))
|
||||
|
||||
def test_append_location(self):
|
||||
new_location = {'url': 'file:///a/path', 'metadata': {},
|
||||
'status': 'active'}
|
||||
|
|
|
@ -3261,7 +3261,9 @@ class TestImagesController(base.IsolatedUnitTest):
|
|||
@mock.patch.object(glance.api.authorization.TaskFactoryProxy, 'new_task')
|
||||
@mock.patch.object(glance.domain.TaskExecutorFactory, 'new_task_executor')
|
||||
@mock.patch('glance.api.common.get_thread_pool')
|
||||
def test_image_import(self, mock_gtp, mock_nte, mock_nt, mock_spa):
|
||||
@mock.patch('glance.quota.keystone.enforce_image_size_total')
|
||||
def test_image_import(self, mock_enforce, mock_gtp, mock_nte, mock_nt,
|
||||
mock_spa):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
image = FakeImage(status='uploading')
|
||||
with mock.patch.object(
|
||||
|
@ -3272,6 +3274,10 @@ class TestImagesController(base.IsolatedUnitTest):
|
|||
|
||||
self.assertEqual(UUID4, output)
|
||||
|
||||
# Make sure we checked quota
|
||||
mock_enforce.assert_called_once_with(request.context,
|
||||
request.context.project_id)
|
||||
|
||||
# Make sure we set the lock on the image
|
||||
mock_spa.assert_called_once_with(UUID4, 'os_glance_import_task',
|
||||
mock_nt.return_value.task_id)
|
||||
|
@ -3297,6 +3303,17 @@ class TestImagesController(base.IsolatedUnitTest):
|
|||
# a task
|
||||
mock_new_task.assert_not_called()
|
||||
|
||||
@mock.patch.object(glance.api.authorization.ImageRepoProxy, 'get')
|
||||
@mock.patch('glance.quota.keystone.enforce_image_size_total')
|
||||
def test_image_import_quota_fail(self, mock_enforce, mock_get):
|
||||
request = unit_test_utils.get_fake_request()
|
||||
mock_get.return_value = FakeImage(status='uploading')
|
||||
mock_enforce.side_effect = exception.LimitExceeded('test')
|
||||
self.assertRaises(webob.exc.HTTPRequestEntityTooLarge,
|
||||
self.controller.import_image,
|
||||
request, UUID4,
|
||||
{'method': {'name': 'glance-direct'}})
|
||||
|
||||
@mock.patch('glance.db.simple.api.image_set_property_atomic')
|
||||
@mock.patch('glance.context.RequestContext.elevated')
|
||||
@mock.patch.object(glance.domain.TaskFactory, 'new_task')
|
||||
|
|
Loading…
Reference in New Issue