diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 434ed9c627..d0bc1caae7 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -635,6 +635,11 @@ use = egg:swift#s3api # the allowed origins must be set cluster-wide. (default: blank; all # preflight requests will be denied) # cors_preflight_allow_origin = +# +# AWS will return a 503 Slow Down when clients are making too many requests, +# but that can make client logs confusing if they only log/give metrics on +# status ints. Turn this on to return 429 instead. +# ratelimit_as_client_error = false # You can override the default log routing for this filter here: # log_name = s3api diff --git a/swift/common/middleware/s3api/s3api.py b/swift/common/middleware/s3api/s3api.py index 7b9ad07784..1d0bc28545 100644 --- a/swift/common/middleware/s3api/s3api.py +++ b/swift/common/middleware/s3api/s3api.py @@ -283,6 +283,8 @@ class S3ApiMiddleware(object): len(self.conf.cors_preflight_allow_origin) > 1: raise ValueError('if cors_preflight_allow_origin should include ' 'all domains, * must be the only entry') + self.conf.ratelimit_as_client_error = config_true_value( + wsgi_conf.get('ratelimit_as_client_error', False)) self.logger = get_logger( wsgi_conf, log_route=wsgi_conf.get('log_name', 's3api')) diff --git a/swift/common/middleware/s3api/s3request.py b/swift/common/middleware/s3api/s3request.py index 17d50a6969..37ab4e8e61 100644 --- a/swift/common/middleware/s3api/s3request.py +++ b/swift/common/middleware/s3api/s3request.py @@ -1400,6 +1400,8 @@ class S3Request(swob.Request): if status == HTTP_SERVICE_UNAVAILABLE: raise ServiceUnavailable() if status in (HTTP_RATE_LIMITED, HTTP_TOO_MANY_REQUESTS): + if self.conf.ratelimit_as_client_error: + raise SlowDown(status='429 Slow Down') raise SlowDown() raise InternalError('unexpected status code %d' % status) diff --git a/swift/common/middleware/s3api/utils.py b/swift/common/middleware/s3api/utils.py index 8946c9e306..41f89192f9 100644 --- a/swift/common/middleware/s3api/utils.py +++ b/swift/common/middleware/s3api/utils.py @@ -160,6 +160,7 @@ class Config(dict): 'allow_multipart_uploads': True, 'allow_no_owner': False, 'allowable_clock_skew': 900, + 'ratelimit_as_client_error': False, } def __init__(self, base=None): diff --git a/test/unit/common/middleware/s3api/__init__.py b/test/unit/common/middleware/s3api/__init__.py index 403291239a..6dadba2e47 100644 --- a/test/unit/common/middleware/s3api/__init__.py +++ b/test/unit/common/middleware/s3api/__init__.py @@ -135,7 +135,8 @@ class S3ApiTestCase(unittest.TestCase): return elem.find('./Message').text def _test_method_error(self, method, path, response_class, headers={}, - env={}, expected_xml_tags=None): + env={}, expected_xml_tags=None, + expected_status=None): if not path.startswith('/'): path = '/' + path # add a missing slash before the path @@ -149,6 +150,8 @@ class S3ApiTestCase(unittest.TestCase): env.update({'REQUEST_METHOD': method}) req = swob.Request.blank(path, environ=env, headers=headers) status, headers, body = self.call_s3api(req) + if expected_status is not None: + self.assertEqual(status, expected_status) if expected_xml_tags is not None: elem = fromstring(body, 'Error') self.assertEqual(set(expected_xml_tags), diff --git a/test/unit/common/middleware/s3api/test_obj.py b/test/unit/common/middleware/s3api/test_obj.py index 6b6d0ce767..a2c886836d 100644 --- a/test/unit/common/middleware/s3api/test_obj.py +++ b/test/unit/common/middleware/s3api/test_obj.py @@ -16,6 +16,7 @@ import binascii import unittest from datetime import datetime +import functools from hashlib import sha256 import os from os.path import join @@ -307,6 +308,19 @@ class TestS3ApiObj(S3ApiTestCase): swob.HTTPServiceUnavailable) self.assertEqual(code, 'ServiceUnavailable') + code = self._test_method_error( + 'GET', '/bucket/object', + functools.partial(swob.Response, status='498 Rate Limited'), + expected_status='503 Slow Down') + self.assertEqual(code, 'SlowDown') + + with patch.object(self.s3api.conf, 'ratelimit_as_client_error', True): + code = self._test_method_error( + 'GET', '/bucket/object', + functools.partial(swob.Response, status='498 Rate Limited'), + expected_status='429 Slow Down') + self.assertEqual(code, 'SlowDown') + @s3acl def test_object_GET(self): self._test_object_GETorHEAD('GET') diff --git a/test/unit/common/middleware/s3api/test_s3api.py b/test/unit/common/middleware/s3api/test_s3api.py index fb9b0f4d85..43c9bd8254 100644 --- a/test/unit/common/middleware/s3api/test_s3api.py +++ b/test/unit/common/middleware/s3api/test_s3api.py @@ -119,6 +119,7 @@ class TestS3ApiMiddleware(S3ApiTestCase): 'multi_delete_concurrency': 2, 's3_acl': False, 'cors_preflight_allow_origin': [], + 'ratelimit_as_client_error': False, }) s3api = S3ApiMiddleware(None, {}) self.assertEqual(expected, s3api.conf) @@ -142,6 +143,7 @@ class TestS3ApiMiddleware(S3ApiTestCase): 'multi_delete_concurrency': 1, 's3_acl': True, 'cors_preflight_allow_origin': 'foo.example.com,bar.example.com', + 'ratelimit_as_client_error': True, } s3api = S3ApiMiddleware(None, conf) conf['cors_preflight_allow_origin'] = \ diff --git a/test/unit/common/middleware/s3api/test_utils.py b/test/unit/common/middleware/s3api/test_utils.py index 0ae78552a7..3b7cb959e2 100644 --- a/test/unit/common/middleware/s3api/test_utils.py +++ b/test/unit/common/middleware/s3api/test_utils.py @@ -140,6 +140,7 @@ class TestConfig(unittest.TestCase): self.assertTrue(conf.allow_multipart_uploads) self.assertFalse(conf.allow_no_owner) self.assertEqual(900, conf.allowable_clock_skew) + self.assertFalse(conf.ratelimit_as_client_error) def test_defaults(self): # deliberately brittle so new defaults will need to be added to test @@ -152,6 +153,7 @@ class TestConfig(unittest.TestCase): del conf.allow_multipart_uploads del conf.allow_no_owner del conf.allowable_clock_skew + del conf.ratelimit_as_client_error self.assertEqual({}, conf) def test_update(self):