From 2c6de2ae523818a7d28165656336c2dfa3cf1c34 Mon Sep 17 00:00:00 2001 From: gholt Date: Wed, 14 Mar 2012 17:30:02 +0000 Subject: [PATCH] Added optional max_containers_per_account restr... Added optional max_containers_per_account restriction. If set to a positive value and if a client tries to perform a container PUT when at or above the max_containers_per_acount cap, a 403 Forbidden will be returned with an explanatory message. This only restricts the proxy server, not any of the background processes that might need to create containers (replication, for instance). Also, the container count is cached for the proxy's recheck_account_existence number of seconds. For these reasons, a given account could exceed this cap before the 403 Forbidden responses kick in and therefore this feature should be considered a "soft" limit. You may also add accounts to the proxy's max_containers_whitelist setting to have accounts that ignore this cap. Change-Id: I74e8fb152de5e78d070ed30006ad4e53f82c8376 --- bin/swift | 13 +++++- doc/source/deployment_guide.rst | 14 +++++++ etc/proxy-server.conf-sample | 8 ++++ swift/proxy/server.py | 64 ++++++++++++++++++++--------- test/unit/proxy/test_server.py | 72 +++++++++++++++++++++++++-------- 5 files changed, 133 insertions(+), 38 deletions(-) diff --git a/bin/swift b/bin/swift index 0a626655a1..f3b5530118 100755 --- a/bin/swift +++ b/bin/swift @@ -1804,8 +1804,17 @@ def st_upload(options, args, print_queue, error_queue): conn.put_container(args[0]) if options.segment_size is not None: conn.put_container(args[0] + '_segments') - except Exception: - pass + except ClientException, err: + msg = ' '.join(str(x) for x in (err.http_status, err.http_reason)) + if err.http_response_content: + if msg: + msg += ': ' + msg += err.http_response_content[:60] + error_queue.put( + 'Error trying to create container %r: %s' % (args[0], msg)) + except Exception, err: + error_queue.put( + 'Error trying to create container %r: %s' % (args[0], err)) try: for arg in args[1:]: if isdir(arg): diff --git a/doc/source/deployment_guide.rst b/doc/source/deployment_guide.rst index cc1047354d..12eab8a1f2 100644 --- a/doc/source/deployment_guide.rst +++ b/doc/source/deployment_guide.rst @@ -561,6 +561,20 @@ account_autocreate false If set to 'true' authorized accounts that do not yet exist within the Swift cluster will be automatically created. +max_containers_per_account 0 If set to a positive value, + trying to create a container + when the account already has at + least this maximum containers + will result in a 403 Forbidden. + Note: This is a soft limit, + meaning a user might exceed the + cap for + recheck_account_existence before + the 403s kick in. +max_containers_whitelist This is a comma separated list + of account hashes that ignore + the max_containers_per_account + cap. ============================ =============== ============================= [tempauth] diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index e0138db210..148616bd3c 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -49,6 +49,14 @@ use = egg:swift#proxy # If set to 'true' authorized accounts that do not yet exist within the Swift # cluster will be automatically created. # account_autocreate = false +# If set to a positive value, trying to create a container when the account +# already has at least this maximum containers will result in a 403 Forbidden. +# Note: This is a soft limit, meaning a user might exceed the cap for +# recheck_account_existence before the 403s kick in. +# max_containers_per_account = 0 +# This is a comma separated list of account hashes that ignore the +# max_containers_per_account cap. +# max_containers_whitelist = [filter:tempauth] use = egg:swift#tempauth diff --git a/swift/proxy/server.py b/swift/proxy/server.py index a1de162fc8..dc8f3eff89 100644 --- a/swift/proxy/server.py +++ b/swift/proxy/server.py @@ -45,11 +45,10 @@ from random import shuffle from eventlet import sleep, spawn_n, GreenPile, Timeout from eventlet.queue import Queue, Empty, Full from eventlet.timeout import Timeout -from webob.exc import HTTPAccepted, HTTPBadRequest, HTTPMethodNotAllowed, \ - HTTPNotFound, HTTPPreconditionFailed, \ - HTTPRequestTimeout, HTTPServiceUnavailable, \ - HTTPUnprocessableEntity, HTTPRequestEntityTooLarge, HTTPServerError, \ - status_map +from webob.exc import HTTPAccepted, HTTPBadRequest, HTTPForbidden, \ + HTTPMethodNotAllowed, HTTPNotFound, HTTPPreconditionFailed, \ + HTTPRequestEntityTooLarge, HTTPRequestTimeout, HTTPServerError, \ + HTTPServiceUnavailable, HTTPUnprocessableEntity, status_map from webob import Request, Response from swift.common.ring import Ring @@ -389,19 +388,26 @@ class Controller(object): Get account information, and also verify that the account exists. :param account: name of the account to get the info for - :returns: tuple of (account partition, account nodes) or (None, None) - if it does not exist + :returns: tuple of (account partition, account nodes, container_count) + or (None, None, None) if it does not exist """ partition, nodes = self.app.account_ring.get_nodes(account) # 0 = no responses, 200 = found, 404 = not found, -1 = mixed responses if self.app.memcache: cache_key = get_account_memcache_key(account) - result_code = self.app.memcache.get(cache_key) + cache_value = self.app.memcache.get(cache_key) + if not isinstance(cache_value, dict): + result_code = cache_value + container_count = 0 + else: + result_code = cache_value['status'] + container_count = cache_value['container_count'] if result_code == 200: - return partition, nodes + return partition, nodes, container_count elif result_code == 404 and not autocreate: - return None, None + return None, None, None result_code = 0 + container_count = 0 attempts_left = self.app.account_ring.replica_count path = '/%s' % account headers = {'x-trans-id': self.trans_id, 'Connection': 'close'} @@ -415,6 +421,8 @@ class Controller(object): body = resp.read() if 200 <= resp.status <= 299: result_code = 200 + container_count = int( + resp.getheader('x-account-container-count') or 0) break elif resp.status == 404: if result_code == 0: @@ -434,7 +442,7 @@ class Controller(object): _('Trying to get account info for %s') % path) if result_code == 404 and autocreate: if len(account) > MAX_ACCOUNT_NAME_LENGTH: - return None, None + return None, None, None headers = {'X-Timestamp': normalize_timestamp(time.time()), 'X-Trans-Id': self.trans_id, 'Connection': 'close'} @@ -449,11 +457,12 @@ class Controller(object): cache_timeout = self.app.recheck_account_existence else: cache_timeout = self.app.recheck_account_existence * 0.1 - self.app.memcache.set(cache_key, result_code, - timeout=cache_timeout) + self.app.memcache.set(cache_key, + {'status': result_code, 'container_count': container_count}, + timeout=cache_timeout) if result_code == 200: - return partition, nodes - return None, None + return partition, nodes, container_count + return None, None, None def container_info(self, account, container, account_autocreate=False): """ @@ -1503,8 +1512,16 @@ class ContainerController(Controller): resp.body = 'Container name length of %d longer than %d' % \ (len(self.container_name), MAX_CONTAINER_NAME_LENGTH) return resp - account_partition, accounts = self.account_info(self.account_name, - autocreate=self.app.account_autocreate) + account_partition, accounts, container_count = \ + self.account_info(self.account_name, + autocreate=self.app.account_autocreate) + if self.app.max_containers_per_account > 0 and \ + container_count >= self.app.max_containers_per_account and \ + self.account_name not in self.app.max_containers_whitelist: + resp = HTTPForbidden(request=req) + resp.body = 'Reached container limit of %s' % \ + self.app.max_containers_per_account + return resp if not accounts: return HTTPNotFound(request=req) container_partition, containers = self.app.container_ring.get_nodes( @@ -1533,8 +1550,9 @@ class ContainerController(Controller): self.clean_acls(req) or check_metadata(req, 'container') if error_response: return error_response - account_partition, accounts = self.account_info(self.account_name, - autocreate=self.app.account_autocreate) + account_partition, accounts, container_count = \ + self.account_info(self.account_name, + autocreate=self.app.account_autocreate) if not accounts: return HTTPNotFound(request=req) container_partition, containers = self.app.container_ring.get_nodes( @@ -1554,7 +1572,8 @@ class ContainerController(Controller): @public def DELETE(self, req): """HTTP DELETE request handler.""" - account_partition, accounts = self.account_info(self.account_name) + account_partition, accounts, container_count = \ + self.account_info(self.account_name) if not accounts: return HTTPNotFound(request=req) container_partition, containers = self.app.container_ring.get_nodes( @@ -1742,6 +1761,11 @@ class BaseApplication(object): 'expiring_objects' self.expiring_objects_container_divisor = \ int(conf.get('expiring_objects_container_divisor') or 86400) + self.max_containers_per_account = \ + int(conf.get('max_containers_per_account') or 0) + self.max_containers_whitelist = [a.strip() + for a in conf.get('max_containers_whitelist', '').split(',') + if a.strip()] def get_controller(self, path): """ diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 82cf9f144e..68dd9ea2ca 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -180,7 +180,7 @@ def fake_http_connect(*code_iter, **kwargs): 'etag': self.etag or '"68b329da9893e34099c7d8ad5cb9c940"', 'x-works': 'yes', - } + 'x-account-container-count': 12345} if not self.timestamp: del headers['x-timestamp'] try: @@ -353,7 +353,8 @@ class TestController(unittest.TestCase): def test_make_requests(self): with save_globals(): proxy_server.http_connect = fake_http_connect(200) - partition, nodes = self.controller.account_info(self.account) + partition, nodes, count = \ + self.controller.account_info(self.account) proxy_server.http_connect = fake_http_connect(201, raise_timeout_exc=True) self.controller._make_request(nodes, partition, 'POST', @@ -363,37 +364,49 @@ class TestController(unittest.TestCase): def test_account_info_200(self): with save_globals(): proxy_server.http_connect = fake_http_connect(200) - partition, nodes = self.controller.account_info(self.account) + partition, nodes, count = \ + self.controller.account_info(self.account) self.check_account_info_return(partition, nodes) + self.assertEquals(count, 12345) cache_key = proxy_server.get_account_memcache_key(self.account) - self.assertEquals(200, self.memcache.get(cache_key)) + self.assertEquals({'status': 200, 'container_count': 12345}, + self.memcache.get(cache_key)) proxy_server.http_connect = fake_http_connect() - partition, nodes = self.controller.account_info(self.account) + partition, nodes, count = \ + self.controller.account_info(self.account) self.check_account_info_return(partition, nodes) + self.assertEquals(count, 12345) # tests if 404 is cached and used def test_account_info_404(self): with save_globals(): proxy_server.http_connect = fake_http_connect(404, 404, 404) - partition, nodes = self.controller.account_info(self.account) + partition, nodes, count = \ + self.controller.account_info(self.account) self.check_account_info_return(partition, nodes, True) + self.assertEquals(count, None) cache_key = proxy_server.get_account_memcache_key(self.account) - self.assertEquals(404, self.memcache.get(cache_key)) + self.assertEquals({'status': 404, 'container_count': 0}, + self.memcache.get(cache_key)) proxy_server.http_connect = fake_http_connect() - partition, nodes = self.controller.account_info(self.account) + partition, nodes, count = \ + self.controller.account_info(self.account) self.check_account_info_return(partition, nodes, True) + self.assertEquals(count, None) # tests if some http status codes are not cached def test_account_info_no_cache(self): def test(*status_list): proxy_server.http_connect = fake_http_connect(*status_list) - partition, nodes = self.controller.account_info(self.account) + partition, nodes, count = \ + self.controller.account_info(self.account) self.assertEqual(len(self.memcache.keys()), 0) self.check_account_info_return(partition, nodes, True) + self.assertEquals(count, None) with save_globals(): test(503, 404, 404) @@ -406,37 +419,41 @@ class TestController(unittest.TestCase): self.memcache.store = {} proxy_server.http_connect = \ fake_http_connect(404, 404, 404, 201, 201, 201) - partition, nodes = \ + partition, nodes, count = \ self.controller.account_info(self.account, autocreate=False) self.check_account_info_return(partition, nodes, is_none=True) + self.assertEquals(count, None) self.memcache.store = {} proxy_server.http_connect = \ fake_http_connect(404, 404, 404, 201, 201, 201) - partition, nodes = \ + partition, nodes, count = \ self.controller.account_info(self.account) self.check_account_info_return(partition, nodes, is_none=True) + self.assertEquals(count, None) self.memcache.store = {} proxy_server.http_connect = \ fake_http_connect(404, 404, 404, 201, 201, 201) - partition, nodes = \ + partition, nodes, count = \ self.controller.account_info(self.account, autocreate=True) self.check_account_info_return(partition, nodes) + self.assertEquals(count, 0) self.memcache.store = {} proxy_server.http_connect = \ fake_http_connect(404, 404, 404, 503, 201, 201) - partition, nodes = \ + partition, nodes, count = \ self.controller.account_info(self.account, autocreate=True) self.check_account_info_return(partition, nodes) + self.assertEquals(count, 0) self.memcache.store = {} proxy_server.http_connect = \ fake_http_connect(404, 404, 404, 503, 201, 503) exc = None try: - partition, nodes = \ + partition, nodes, count = \ self.controller.account_info(self.account, autocreate=True) except Exception, err: exc = err @@ -468,7 +485,7 @@ class TestController(unittest.TestCase): # tests if 200 is cached and used def test_container_info_200(self): def account_info(self, account, autocreate=False): - return True, True + return True, True, 0 with save_globals(): headers = {'x-container-read': self.read_acl, @@ -494,7 +511,7 @@ class TestController(unittest.TestCase): # tests if 404 is cached and used def test_container_info_404(self): def account_info(self, account, autocreate=False): - return True, True + return True, True, 0 with save_globals(): proxy_server.Controller.account_info = account_info @@ -3164,6 +3181,29 @@ class TestContainerController(unittest.TestCase): test_status_map((200, 204, 404, 404), 404, missing_container=True) test_status_map((200, 204, 500, 404), 503, missing_container=True) + def test_PUT_max_containers_per_account(self): + with save_globals(): + self.app.max_containers_per_account = 12346 + controller = proxy_server.ContainerController(self.app, 'account', + 'container') + self.assert_status_map(controller.PUT, + (200, 200, 200, 201, 201, 201), 201, + missing_container=True) + + self.app.max_containers_per_account = 12345 + controller = proxy_server.ContainerController(self.app, 'account', + 'container') + self.assert_status_map(controller.PUT, (201, 201, 201), 403, + missing_container=True) + + self.app.max_containers_per_account = 12345 + self.app.max_containers_whitelist = ['account'] + controller = proxy_server.ContainerController(self.app, 'account', + 'container') + self.assert_status_map(controller.PUT, + (200, 200, 200, 201, 201, 201), 201, + missing_container=True) + def test_PUT_max_container_name_length(self): with save_globals(): controller = proxy_server.ContainerController(self.app, 'account',