FakeSwift: use HTTPMethodNotAllowed not HTTPNotImplemented

If a method is not allowed, real swift proxy server app will return an
HTTPMethodNotAllowed response, whereas FakeSwift would previously
*raise* HTTPNotImplemented. S3Api deliberately sends requests with
method 'TEST' which is not allowed/implemented. To workaround the
difference in real and fake swift behaviour, FakeSwift was configured
to allow the 'TEST' method, and then in some tests an
HTTPMethodNotAllowed response was registered for 'TEST' requests!

This patch modifies FakeSwift to return an HTTPMethodNotAllowed
response to the incoming request when the request method is not
allowed.

It is no longer necessary for FakeSwift to support extending the
default list of allowed methods.

Change-Id: I550d0174e14a5d5a05d26e5cbe9d3353f5da4e8a

test/unit/common/middleware/helpers.py

@@ -23,7 +23,7 @@ from swift.common.request_helpers import is_user_meta, \
     is_object_transient_sysmeta, resolve_etag_is_at_header, \
     resolve_ignore_range_header
 from swift.common.storage_policy import POLICIES
-from swift.common.swob import HTTPNotImplemented
+from swift.common.swob import HTTPMethodNotAllowed
 from swift.common.utils import split_path, md5
 
 from test.debug_logger import debug_logger
@@ -119,16 +119,13 @@ class FakeSwift(object):
       * received ``POST /v1/a/c/o?x=y``, if it matches a registered ``POST``,
         will update uploaded ``/v1/a/c/o``
     """
-    DEFAULT_ALLOWED_METHODS = [
+    ALLOWED_METHODS = [
         'PUT', 'POST', 'DELETE', 'GET', 'HEAD', 'OPTIONS', 'REPLICATE',
         'SSYNC', 'UPDATE']
     container_existence_skip_cache = 0.0
     account_existence_skip_cache = 0.0
 
-    def __init__(self, allowed_methods=None):
+    def __init__(self):
-        self.allowed_methods = set(self.DEFAULT_ALLOWED_METHODS)
-        if allowed_methods:
-            self.allowed_methods.update(allowed_methods)
         self._calls = []
         self.req_bodies = []
         self._unclosed_req_keys = defaultdict(int)
@@ -237,8 +234,8 @@ class FakeSwift(object):
 
     def __call__(self, env, start_response):
         method = env['REQUEST_METHOD']
-        if method not in self.allowed_methods:
+        if method not in self.ALLOWED_METHODS:
-            raise HTTPNotImplemented()
+            return HTTPMethodNotAllowed()(env, start_response)
 
         path, acc, cont, obj = self._parse_path(env)
 

test/unit/common/middleware/s3api/__init__.py

@@ -39,7 +39,7 @@ class FakeApp(object):
     def __init__(self):
         self.remote_user = 'authorized'
         self._pipeline_final_app = self
-        self.swift = FakeSwift(allowed_methods=['TEST'])
+        self.swift = FakeSwift()
         self.logger = debug_logger()
 
     def _update_s3_path_info(self, env):
@@ -281,10 +281,6 @@ class S3ApiTestCaseAcl(S3ApiTestCase):
             self.swift.register('HEAD', path, swob.HTTPNoContent, {}, None),
             self.swift.register('GET', path, swob.HTTPOk, {}, json.dumps([])),
 
-        for account in ('AUTH_test', 'AUTH_X'):
-            self.swift.register('TEST', '/v1/' + account,
-                                swob.HTTPMethodNotAllowed, {}, None)
-
         # setup sticky ACL headers...
         grants = [_gen_grant(perm) for perm in PERMISSIONS]
         self.default_owner = Owner('test:tester', 'test:tester')

test/unit/common/middleware/s3api/test_s3api.py

@@ -1440,7 +1440,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
             self.s3api.logger.logger.statsd_client.get_increment_counts())
 
     def test_s3api_with_only_s3_token(self):
-        self.swift = FakeSwift(allowed_methods=['TEST'])
+        self.swift = FakeSwift()
         self.keystone_auth = KeystoneAuth(
             self.swift, {'operator_roles': 'swift-user'})
         self.s3_token = S3Token(
@@ -1470,7 +1470,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
                              req.environ['swift.backend_path'])
 
     def test_s3api_with_only_s3_token_v3(self):
-        self.swift = FakeSwift(allowed_methods=['TEST'])
+        self.swift = FakeSwift()
         self.keystone_auth = KeystoneAuth(
             self.swift, {'operator_roles': 'swift-user'})
         self.s3_token = S3Token(
@@ -1500,7 +1500,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
                              req.environ['swift.backend_path'])
 
     def test_s3api_with_s3_token_and_auth_token(self):
-        self.swift = FakeSwift(allowed_methods=['TEST'])
+        self.swift = FakeSwift()
         self.keystone_auth = KeystoneAuth(
             self.swift, {'operator_roles': 'swift-user'})
         self.auth_token = AuthProtocol(
@@ -1555,7 +1555,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
                     statsd_client.get_increment_counts())
 
     def test_s3api_with_only_s3_token_in_s3acl(self):
-        self.swift = FakeSwift(allowed_methods=['TEST'])
+        self.swift = FakeSwift()
         self.keystone_auth = KeystoneAuth(
             self.swift, {'operator_roles': 'swift-user'})
         self.s3_token = S3Token(
@@ -1575,8 +1575,6 @@ class TestS3ApiMiddleware(S3ApiTestCase):
         # after PUT container so we need to register the resposne here
         self.swift.register('POST', '/v1/AUTH_TENANT_ID/bucket',
                             swob.HTTPNoContent, {}, None)
-        self.swift.register('TEST', '/v1/AUTH_TENANT_ID',
-                            swob.HTTPMethodNotAllowed, {}, None)
         with patch.object(self.s3_token, '_json_request') as mock_req:
             mock_resp = requests.Response()
             mock_resp._content = json.dumps(GOOD_RESPONSE_V2).encode('ascii')

test/unit/common/middleware/test_helpers.py

@@ -17,7 +17,7 @@ import unittest
 
 from swift.common.storage_policy import POLICIES
 from swift.common.swob import Request, HTTPOk, HTTPNotFound, \
-    HTTPCreated, HeaderKeyDict, HTTPException
+    HTTPCreated, HeaderKeyDict
 from swift.common import request_helpers as rh
 from swift.common.middleware.s3api.utils import sysmeta_header
 from test.unit.common.middleware.helpers import FakeSwift
@@ -26,29 +26,19 @@ from test.unit.common.middleware.helpers import FakeSwift
 class TestFakeSwift(unittest.TestCase):
     def test_allowed_methods(self):
 
-        def assert_allowed(swift, method):
+        def do_test(swift, method, exp_status):
             path = '/v1/a/c/o'
             swift.register(method, path, HTTPOk, {}, None)
             req = Request.blank(path)
             req.method = method
-            self.assertEqual(200, req.get_response(swift).status_int)
+            self.assertEqual(exp_status, req.get_response(swift).status_int)
-
-        def assert_disallowed(swift, method):
-            path = '/v1/a/c/o'
-            swift.register(method, path, HTTPOk, {}, None)
-            req = Request.blank(path)
-            req.method = method
-            with self.assertRaises(HTTPException) as cm:
-                req.get_response(swift)
-                self.assertEqual(501, cm.exception.status_int)
 
         for method in ('PUT', 'POST', 'DELETE', 'GET', 'HEAD', 'OPTIONS',
                        'REPLICATE', 'SSYNC', 'UPDATE'):
-            assert_allowed(FakeSwift(), method)
+            do_test(FakeSwift(), method, 200)
-            assert_allowed(FakeSwift(allowed_methods=['TEST']), 'TEST')
 
-        assert_disallowed(FakeSwift(), 'TEST')
+        do_test(FakeSwift(), 'TEST', 405)
-        assert_allowed(FakeSwift(allowed_methods=['TEST']), 'TEST')
+        do_test(FakeSwift(), 'get', 405)
 
     def test_not_registered(self):
         swift = FakeSwift()