Merge "Rework private-request-method interface"

swift/cli/container_deleter.py

@@ -113,7 +113,7 @@ def mark_for_deletion(swift, account, container, marker, end_marker,
             swift.make_request(
                 'UPDATE',
                 swift.make_path('.expiring_objects', str(int(timestamp))),
-                headers={'X-Backend-Allow-Method': 'UPDATE',
+                headers={'X-Backend-Allow-Private-Methods': 'True',
                          'X-Backend-Storage-Policy-Index': '0',
                          'X-Timestamp': timestamp.internal},
                 acceptable_statuses=(2,),

swift/common/utils.py

@@ -3740,6 +3740,17 @@ def public(func):
     return func
 
 
+def private(func):
+    """
+    Decorator to declare which methods are privately accessible as HTTP
+    requests with an ``X-Backend-Allow-Private-Methods: True`` override
+
+    :param func: function to make private
+    """
+    func.privately_accessible = True
+    return func
+
+
 def majority_size(n):
     return (n // 2) + 1
 

swift/proxy/controllers/base.py

@@ -1517,6 +1517,7 @@ class Controller(object):
         self.app = app
         self.trans_id = '-'
         self._allowed_methods = None
+        self._private_methods = None
 
     @property
     def allowed_methods(self):
@@ -1528,6 +1529,16 @@ class Controller(object):
                     self._allowed_methods.add(name)
         return self._allowed_methods
 
+    @property
+    def private_methods(self):
+        if self._private_methods is None:
+            self._private_methods = set()
+            all_methods = inspect.getmembers(self, predicate=inspect.ismethod)
+            for name, m in all_methods:
+                if getattr(m, 'privately_accessible', False):
+                    self._private_methods.add(name)
+        return self._private_methods
+
     def _x_remove_headers(self):
         """
         Returns a list of headers that must not be sent to the backend

swift/proxy/controllers/container.py

@@ -18,7 +18,7 @@ import json
 
 from six.moves.urllib.parse import unquote
 
-from swift.common.utils import public, csv_append, Timestamp, \
+from swift.common.utils import public, private, csv_append, Timestamp, \
     config_true_value, ShardRange
 from swift.common.constraints import check_metadata, CONTAINER_LISTING_LIMIT
 from swift.common.http import HTTP_ACCEPTED, is_success
@@ -356,6 +356,7 @@ class ContainerController(Controller):
             return HTTPNotFound(request=req)
         return resp
 
+    @private
     def UPDATE(self, req):
         """HTTP UPDATE request handler.
 

swift/proxy/server.py

@@ -507,13 +507,14 @@ class Application(object):
             controller.trans_id = req.environ['swift.trans_id']
             self.logger.client_ip = get_remote_client(req)
 
-            allowed_methods = set(controller.allowed_methods)
+            allowed_methods = controller.allowed_methods
-            if 'X-Backend-Allow-Method' in req.headers:
+            if config_true_value(req.headers.get(
-                allowed_methods.add(req.headers['X-Backend-Allow-Method'])
+                    'X-Backend-Allow-Private-Methods', False)):
+                allowed_methods = set(allowed_methods).union(
+                    controller.private_methods)
             if req.method not in allowed_methods:
                 return HTTPMethodNotAllowed(request=req, headers={
-                    # Only advertise the *controller's* allowed_methods
+                    'Allow': ', '.join(allowed_methods)})
-                    'Allow': ', '.join(controller.allowed_methods)})
             handler = getattr(controller, req.method)
 
             old_authorize = None

test/unit/cli/test_container_deleter.py

@@ -90,7 +90,7 @@ class TestContainerDeleter(unittest.TestCase):
         uacct = acct = u'acct-\U0001f334'
         ucont = cont = u'cont-\N{SNOWMAN}'
         uobj1 = obj1 = u'obj-\N{GREEK CAPITAL LETTER ALPHA}'
-        uobj2 = obj2 = u'obj-\N{GREEK CAPITAL LETTER OMEGA}'
+        uobj2 = obj2 = u'/obj-\N{GREEK CAPITAL LETTER OMEGA}'
         if six.PY2:
             acct = acct.encode('utf8')
             cont = cont.encode('utf8')
@@ -184,7 +184,7 @@ class TestContainerDeleter(unittest.TestCase):
         ts = '1558463777.42739'
         with FakeInternalClient([
             swob.Response(json.dumps([
-                {'name': 'obj1'},
+                {'name': '/obj1'},
                 {'name': 'obj2'},
                 {'name': 'obj3'},
             ])),
@@ -208,14 +208,14 @@ class TestContainerDeleter(unittest.TestCase):
                 ('GET', '/v1/account/container',
                  'format=json&marker=obj3&end_marker=&prefix=', {}, None),
                 ('UPDATE', '/v1/.expiring_objects/' + ts.split('.')[0], '', {
-                    'X-Backend-Allow-Method': 'UPDATE',
+                    'X-Backend-Allow-Private-Methods': 'True',
                     'X-Backend-Storage-Policy-Index': '0',
                     'X-Timestamp': ts}, mock.ANY),
             ])
             self.assertEqual(
                 json.loads(swift.calls[-1].body),
                 container_deleter.make_delete_jobs(
-                    'account', 'container', ['obj1', 'obj2', 'obj3'],
+                    'account', 'container', ['/obj1', 'obj2', 'obj3'],
                     utils.Timestamp(ts)
                 )
             )
@@ -241,22 +241,23 @@ class TestContainerDeleter(unittest.TestCase):
                 'account',
                 'container',
                 '',
-                '',
+                'end',
-                '',
+                'pre',
                 timestamp=utils.Timestamp(ts),
                 yield_time=0,
             )), [(5, 'obj5'), (6, 'obj6'), (6, None)])
             self.assertEqual(swift.calls, [
                 ('GET', '/v1/account/container',
-                 'format=json&marker=&end_marker=&prefix=', {}, None),
+                 'format=json&marker=&end_marker=end&prefix=pre', {}, None),
                 ('UPDATE', '/v1/.expiring_objects/' + ts.split('.')[0], '', {
-                    'X-Backend-Allow-Method': 'UPDATE',
+                    'X-Backend-Allow-Private-Methods': 'True',
                     'X-Backend-Storage-Policy-Index': '0',
                     'X-Timestamp': ts}, mock.ANY),
                 ('GET', '/v1/account/container',
-                 'format=json&marker=obj6&end_marker=&prefix=', {}, None),
+                 'format=json&marker=obj6&end_marker=end&prefix=pre',
+                 {}, None),
                 ('UPDATE', '/v1/.expiring_objects/' + ts.split('.')[0], '', {
-                    'X-Backend-Allow-Method': 'UPDATE',
+                    'X-Backend-Allow-Private-Methods': 'True',
                     'X-Backend-Storage-Policy-Index': '0',
                     'X-Timestamp': ts}, mock.ANY),
             ])

test/unit/proxy/test_server.py

@@ -684,11 +684,19 @@ class TestProxyServer(unittest.TestCase):
         # But with appropriate (internal-only) overrides, you can still use it
         resp = baseapp.handle_request(
             Request.blank('/v1/a/c', environ={'REQUEST_METHOD': 'UPDATE'},
-                          headers={'X-Backend-Allow-Method': 'UPDATE',
+                          headers={'X-Backend-Allow-Private-Methods': 'True',
                                    'X-Backend-Storage-Policy-Index': '0'}))
         # Now we actually make the requests, but there aren't any nodes
         self.assertEqual(resp.status, '503 Service Unavailable')
 
+        # Bad method with overrides advertises private methods
+        resp = baseapp.handle_request(
+            Request.blank('/v1/a/c', environ={'REQUEST_METHOD': 'BOGUS'},
+                          headers={'X-Backend-Allow-Private-Methods': '1'}))
+        self.assertEqual(resp.status, '405 Method Not Allowed')
+        self.assertEqual(sorted(resp.headers['Allow'].split(', ')), [
+            'DELETE', 'GET', 'HEAD', 'OPTIONS', 'POST', 'PUT', 'UPDATE'])
+
     def test_calls_authorize_allow(self):
         called = [False]