Merge "s3api: Add config option to return 429s on ratelimit"

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

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'))

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)

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):

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),

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')

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'] = \

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):