Drop items query param in gatekeeper

Maybe this is a sign we should be using a X-Backend-* header instead?

Change-Id: I81b14acee6d479e96f451fd18de2e54ca032b9c7

swift/common/middleware/gatekeeper.py

@@ -31,6 +31,7 @@ automatically inserted close to the start of the pipeline by the proxy server.
 """
 
 
+from swift.common.constraints import valid_api_version
 from swift.common.swob import Request
 from swift.common.utils import get_logger, config_true_value
 from swift.common.request_helpers import (
@@ -78,6 +79,21 @@ class GatekeeperMiddleware(object):
 
     def __call__(self, env, start_response):
         req = Request(env)
+
+        # Remove sharding-specific container-server query param.
+        # External clients shouldn't need to worry about shards.
+        try:
+            ver, _acct, _cont = req.split_path(3, 3)
+            if not valid_api_version(ver):
+                raise ValueError
+        except ValueError:
+            pass
+        else:
+            params = req.params
+            if params.get('items') in ('shard', 'all'):
+                del params['items']
+                req.params = params
+
         removed = remove_items(req.headers, self.inbound_condition)
         if removed:
             self.logger.debug('removed request headers: %s' % removed)

swift/proxy/controllers/container.py

@@ -115,8 +115,10 @@ class ContainerController(Controller):
         sharding_state = \
             int(resp.headers.get('X-Backend-Sharding-State',
                                  DB_STATE_UNSHARDED))
-        if req.method == "GET" and sharding_state in (DB_STATE_SHARDING,
-                                                      DB_STATE_SHARDED):
+        if req.method == "GET" and params.get('items') != 'shard' and \
+                sharding_state in (DB_STATE_SHARDING, DB_STATE_SHARDED):
+            # TODO: think about whether this actually does what we want for
+            # items=all -- and whether we should even support that here
             new_resp = self._get_sharded(req, resp, sharding_state)
             if new_resp:
                 resp = new_resp
@@ -125,8 +127,11 @@ class ContainerController(Controller):
         # up-to-date information for the container.
         resp.headers['X-Backend-Recheck-Container-Existence'] = str(
             self.app.recheck_container_existence)
-        set_info_cache(self.app, req.environ, self.account_name,
-                       self.container_name, resp)
+        # TODO: seems like a good idea to not set cache for shard/all
+        # requests, but revisit this at some point
+        if params.get('items') not in ('shard', 'all'):
+            set_info_cache(self.app, req.environ, self.account_name,
+                           self.container_name, resp)
         if 'swift.authorize' in req.environ:
             req.acl = resp.headers.get('x-container-read')
             aresp = req.environ['swift.authorize'](req)

test/unit/common/middleware/test_gatekeeper.py

@@ -245,6 +245,32 @@ class TestGatekeeper(unittest.TestCase):
         self._test_location_header('/v/a/c/o2?query=path#test')
         self._test_location_header('/v/a/c/o2;whatisparam?query=path#test')
 
+    def test_sharding_items_query_param(self):
+        def capturing_app(env, start_response):
+            req = Request(env)
+            capturing_app.params.append(req.params)
+            start_response('200 OK', [])
+
+        app = self.get_app(capturing_app, {})
+
+        def do_test(request_path, should_be_removed):
+            capturing_app.params = []
+            resp = Request.blank(request_path).get_response(app)
+            self.assertEqual('200 OK', resp.status)
+            self.assertEqual(1, len(capturing_app.params),
+                             'Expected one param dict, got %r' % (
+                                 capturing_app.params, ))
+            if should_be_removed:
+                self.assertNotIn('items', capturing_app.params[0])
+            else:
+                self.assertIn('items', capturing_app.params[0])
+
+        do_test('/v1/a/c?items=shard', True)
+        do_test('/v1.0/a/c/?items=all', True)
+        do_test('/v1/a/c?items=foo', False)
+        do_test('/v1/a?items=shard', False)
+        do_test('/auth/a/c?items=shard', False)
+        do_test('/v1/a/c/o?items=shard', False)
 
 if __name__ == '__main__':
     unittest.main()