From 172a9b369f8e19d1dd6526a10787e79e4309e74e Mon Sep 17 00:00:00 2001 From: David Goetz Date: Fri, 3 Oct 2014 12:11:06 -0700 Subject: [PATCH] Change black/white-listing to use sysmeta. The way we do this now involves a conf change and a proxy reload which is a pain. You can now just set these: X-Account-Sysmeta-Global-Write-Ratelimit: WHITELIST or X-Account-Sysmeta-Global-Write-Ratelimit: BLACKLIST NOTE: The existing proxy config settings: account_whitelist and account_blacklist will continue to work. Change-Id: I532663f1d2c75d03170c5fdb9b330416822fbc88 --- doc/manpages/proxy-server.conf.5 | 5 - doc/source/ratelimit.rst | 20 ++- etc/proxy-server.conf-sample | 3 + swift/common/middleware/ratelimit.py | 37 +++-- swift/proxy/controllers/base.py | 1 + test/unit/common/middleware/test_ratelimit.py | 156 ++++++++++-------- 6 files changed, 135 insertions(+), 87 deletions(-) diff --git a/doc/manpages/proxy-server.conf.5 b/doc/manpages/proxy-server.conf.5 index 8ebf62f202..17197453ac 100644 --- a/doc/manpages/proxy-server.conf.5 +++ b/doc/manpages/proxy-server.conf.5 @@ -265,11 +265,6 @@ rate but better average accuracy. The default is 5. .IP \fBaccount_ratelimit\fR If set, will limit PUT and DELETE requests to /account_name/container_name. Number is in requests per second. If set to 0 means disabled. The default is 0. -.IP \fBaccount_whitelist\fR -Comma separated lists of account names that will not be rate limited. The default is ''. -.IP \fBaccount_blacklist\fR -Comma separated lists of account names that will not be allowed. Returns a 497 response. -The default is ''. .IP \fBcontainer_ratelimit_size\fR When set with container_limit_x = r: for containers of size x, limit requests per second to r. Will limit PUT, DELETE, and POST requests to /a/c/o. The default is ''. diff --git a/doc/source/ratelimit.rst b/doc/source/ratelimit.rst index 0fee63f9ed..a54bb5bbf3 100644 --- a/doc/source/ratelimit.rst +++ b/doc/source/ratelimit.rst @@ -42,11 +42,6 @@ account_ratelimit 0 If set, will limit PUT and DELETE requests to /account_name/container_name. Number is in requests per second. -account_whitelist '' Comma separated lists of account names - that will not be rate limited. -account_blacklist '' Comma separated lists of account names - that will not be allowed. Returns a - 497 response. container_ratelimit_size '' When set with container_ratelimit_x = r: for containers of size x, limit requests per second to r. Will limit @@ -85,6 +80,7 @@ Container Size Rate Limit Account Specific Ratelimiting ----------------------------- + The above ratelimiting is to prevent the "many writes to a single container" bottleneck from causing a problem. There could also be a problem where a single account is just using too much of the cluster's resources. In this case, the @@ -98,3 +94,17 @@ to the applicable account/container limits from above. This header will be hidden from the user, because of the gatekeeper middleware, and can only be set using a direct client to the account nodes. It accepts a float value and will only limit requests if the value is > 0. + +------------------- +Black/White-listing +------------------- + +To blacklist or whitelist an account set: + +X-Account-Sysmeta-Global-Write-Ratelimit: BLACKLIST + +or + +X-Account-Sysmeta-Global-Write-Ratelimit: WHITELIST + +in the account headers. diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index e17f0a7703..8ac10dd27c 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -361,6 +361,9 @@ use = egg:swift#ratelimit # account_ratelimit of 0 means disabled # account_ratelimit = 0 +# DEPRECATED- these will continue to work but will be replaced +# by the X-Account-Sysmeta-Global-Write-Ratelimit flag. +# Please see ratelimiting docs for details. # these are comma separated lists of account names # account_whitelist = a,b # account_blacklist = c,d diff --git a/swift/common/middleware/ratelimit.py b/swift/common/middleware/ratelimit.py index 06823cd775..c38cc3a697 100644 --- a/swift/common/middleware/ratelimit.py +++ b/swift/common/middleware/ratelimit.py @@ -127,7 +127,8 @@ class RateLimitMiddleware(object): return rv def get_ratelimitable_key_tuples(self, req, account_name, - container_name=None, obj_name=None): + container_name=None, obj_name=None, + global_ratelimit=None): """ Returns a list of key (used in memcache), ratelimit tuples. Keys should be checked in order. @@ -136,9 +137,12 @@ class RateLimitMiddleware(object): :param account_name: account name from path :param container_name: container name from path :param obj_name: object name from path + :param global_ratelimit: this account has an account wide + ratelimit on all writes combined """ keys = [] # COPYs are not limited + if self.account_ratelimit and \ account_name and container_name and not obj_name and \ req.method in ('PUT', 'DELETE'): @@ -166,16 +170,13 @@ class RateLimitMiddleware(object): container_rate)) if account_name and req.method in ('PUT', 'DELETE', 'POST', 'COPY'): - account_info = get_account_info(req.environ, self.app) - account_global_ratelimit = \ - account_info.get('sysmeta', {}).get('global-write-ratelimit') - if account_global_ratelimit: + if global_ratelimit: try: - account_global_ratelimit = float(account_global_ratelimit) - if account_global_ratelimit > 0: + global_ratelimit = float(global_ratelimit) + if global_ratelimit > 0: keys.append(( "ratelimit/global-write/%s" % account_name, - account_global_ratelimit)) + global_ratelimit)) except ValueError: pass @@ -229,18 +230,30 @@ class RateLimitMiddleware(object): ''' if not self.memcache_client: return None - if account_name in self.ratelimit_blacklist: + + try: + account_info = get_account_info(req.environ, self.app) + account_global_ratelimit = \ + account_info.get('sysmeta', {}).get('global-write-ratelimit') + except ValueError: + account_global_ratelimit = None + + if account_name in self.ratelimit_whitelist or \ + account_global_ratelimit == 'WHITELIST': + return None + + if account_name in self.ratelimit_blacklist or \ + account_global_ratelimit == 'BLACKLIST': self.logger.error(_('Returning 497 because of blacklisting: %s'), account_name) eventlet.sleep(self.BLACK_LIST_SLEEP) return Response(status='497 Blacklisted', body='Your account has been blacklisted', request=req) - if account_name in self.ratelimit_whitelist: - return None + for key, max_rate in self.get_ratelimitable_key_tuples( req, account_name, container_name=container_name, - obj_name=obj_name): + obj_name=obj_name, global_ratelimit=account_global_ratelimit): try: need_to_sleep = self._get_sleep_time(key, max_rate) if self.log_sleep_time_seconds and \ diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index 728ec594de..e5e3f25800 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -310,6 +310,7 @@ def get_account_info(env, app, swift_source=None): This call bypasses auth. Success does not imply that the request has authorization to the account. + :raises ValueError: when path can't be split(path, 2, 4) """ (version, account, _junk, _junk) = \ split_path(env['PATH_INFO'], 2, 4, True) diff --git a/test/unit/common/middleware/test_ratelimit.py b/test/unit/common/middleware/test_ratelimit.py index 12940eac85..1f9bfc3994 100644 --- a/test/unit/common/middleware/test_ratelimit.py +++ b/test/unit/common/middleware/test_ratelimit.py @@ -208,25 +208,16 @@ class TestRateLimit(unittest.TestCase): self.assertEquals(len(the_app.get_ratelimitable_key_tuples( req, 'a', 'c', 'o')), 1) - def get_fake_ratelimit(*args, **kwargs): - return {'sysmeta': {'global-write-ratelimit': 10}} + req.method = 'PUT' + self.assertEquals(len(the_app.get_ratelimitable_key_tuples( + req, 'a', 'c', None, global_ratelimit=10)), 2) + self.assertEquals(the_app.get_ratelimitable_key_tuples( + req, 'a', 'c', None, global_ratelimit=10)[1], + ('ratelimit/global-write/a', 10)) - with mock.patch('swift.common.middleware.ratelimit.get_account_info', - get_fake_ratelimit): - req.method = 'PUT' - self.assertEquals(len(the_app.get_ratelimitable_key_tuples( - req, 'a', 'c', None)), 2) - self.assertEquals(the_app.get_ratelimitable_key_tuples( - req, 'a', 'c', None)[1], ('ratelimit/global-write/a', 10)) - - def get_fake_ratelimit(*args, **kwargs): - return {'sysmeta': {'global-write-ratelimit': 'notafloat'}} - - with mock.patch('swift.common.middleware.ratelimit.get_account_info', - get_fake_ratelimit): - req.method = 'PUT' - self.assertEquals(len(the_app.get_ratelimitable_key_tuples( - req, 'a', 'c', None)), 1) + req.method = 'PUT' + self.assertEquals(len(the_app.get_ratelimitable_key_tuples( + req, 'a', 'c', None, global_ratelimit='notafloat')), 1) def test_memcached_container_info_dict(self): mdict = headers_to_container_info({'x-container-object-count': '45'}) @@ -291,7 +282,26 @@ class TestRateLimit(unittest.TestCase): self._run(make_app_call, num_calls, current_rate, check_time=False) self.assertEquals(round(time.time() - begin, 1), 9.8) - def test_ratelimit_whitelist(self): + def test_ratelimit_old_white_black_list(self): + global time_ticker + current_rate = 2 + conf_dict = {'account_ratelimit': current_rate, + 'max_sleep_time_seconds': 2, + 'account_whitelist': 'a', + 'account_blacklist': 'b'} + self.test_ratelimit = ratelimit.filter_factory(conf_dict)(FakeApp()) + req = Request.blank('/') + with mock.patch.object(self.test_ratelimit, + 'memcache_client', FakeMemcache()): + self.assertEqual( + self.test_ratelimit.handle_ratelimit(req, 'a', 'c', 'o'), + None) + self.assertEqual( + self.test_ratelimit.handle_ratelimit( + req, 'b', 'c', 'o').status_int, + 497) + + def test_ratelimit_whitelist_sysmeta(self): global time_ticker current_rate = 2 conf_dict = {'account_ratelimit': current_rate, @@ -312,18 +322,25 @@ class TestRateLimit(unittest.TestCase): def run(self): self.result = self.parent.test_ratelimit(req.environ, start_response) - nt = 5 - threads = [] - for i in range(nt): - rc = rate_caller(self) - rc.start() - threads.append(rc) - for thread in threads: - thread.join() - the_498s = [ - t for t in threads if ''.join(t.result).startswith('Slow down')] - self.assertEquals(len(the_498s), 0) - self.assertEquals(time_ticker, 0) + + def get_fake_ratelimit(*args, **kwargs): + return {'sysmeta': {'global-write-ratelimit': 'WHITELIST'}} + + with mock.patch('swift.common.middleware.ratelimit.get_account_info', + get_fake_ratelimit): + nt = 5 + threads = [] + for i in range(nt): + rc = rate_caller(self) + rc.start() + threads.append(rc) + for thread in threads: + thread.join() + the_498s = [ + t for t in threads + if ''.join(t.result).startswith('Slow down')] + self.assertEquals(len(the_498s), 0) + self.assertEquals(time_ticker, 0) def test_ratelimit_blacklist(self): global time_ticker @@ -348,18 +365,25 @@ class TestRateLimit(unittest.TestCase): def run(self): self.result = self.parent.test_ratelimit(req.environ, start_response) - nt = 5 - threads = [] - for i in range(nt): - rc = rate_caller(self) - rc.start() - threads.append(rc) - for thread in threads: - thread.join() - the_497s = [ - t for t in threads if ''.join(t.result).startswith('Your account')] - self.assertEquals(len(the_497s), 5) - self.assertEquals(time_ticker, 0) + + def get_fake_ratelimit(*args, **kwargs): + return {'sysmeta': {'global-write-ratelimit': 'BLACKLIST'}} + + with mock.patch('swift.common.middleware.ratelimit.get_account_info', + get_fake_ratelimit): + nt = 5 + threads = [] + for i in range(nt): + rc = rate_caller(self) + rc.start() + threads.append(rc) + for thread in threads: + thread.join() + the_497s = [ + t for t in threads + if ''.join(t.result).startswith('Your account')] + self.assertEquals(len(the_497s), 5) + self.assertEquals(time_ticker, 0) def test_ratelimit_max_rate_double(self): global time_ticker @@ -443,28 +467,30 @@ class TestRateLimit(unittest.TestCase): get_container_memcache_key('a', 'c'), {'container_size': 1}) - time_override = [0, 0, 0, 0, None] - # simulates 4 requests coming in at same time, then sleeping - r = self.test_ratelimit(req.environ, start_response) - mock_sleep(.1) - r = self.test_ratelimit(req.environ, start_response) - mock_sleep(.1) - r = self.test_ratelimit(req.environ, start_response) - self.assertEquals(r[0], 'Slow down') - mock_sleep(.1) - r = self.test_ratelimit(req.environ, start_response) - self.assertEquals(r[0], 'Slow down') - mock_sleep(.1) - r = self.test_ratelimit(req.environ, start_response) - self.assertEquals(r[0], '204 No Content') - mc = self.test_ratelimit.memcache_client - try: - self.test_ratelimit.memcache_client = None - self.assertEquals( - self.test_ratelimit.handle_ratelimit(req, 'n', 'c', None), - None) - finally: - self.test_ratelimit.memcache_client = mc + with mock.patch('swift.common.middleware.ratelimit.get_account_info', + lambda *args, **kwargs: {}): + time_override = [0, 0, 0, 0, None] + # simulates 4 requests coming in at same time, then sleeping + r = self.test_ratelimit(req.environ, start_response) + mock_sleep(.1) + r = self.test_ratelimit(req.environ, start_response) + mock_sleep(.1) + r = self.test_ratelimit(req.environ, start_response) + self.assertEquals(r[0], 'Slow down') + mock_sleep(.1) + r = self.test_ratelimit(req.environ, start_response) + self.assertEquals(r[0], 'Slow down') + mock_sleep(.1) + r = self.test_ratelimit(req.environ, start_response) + self.assertEquals(r[0], '204 No Content') + mc = self.test_ratelimit.memcache_client + try: + self.test_ratelimit.memcache_client = None + self.assertEquals( + self.test_ratelimit.handle_ratelimit(req, 'n', 'c', None), + None) + finally: + self.test_ratelimit.memcache_client = mc def test_ratelimit_max_rate_multiple_acc(self): num_calls = 4