Merge "Don't auto-create shard containers"

swift/container/server.py

@@ -155,6 +155,8 @@ class ContainerController(BaseStorageServer):
                 conf['auto_create_account_prefix']
         else:
             self.auto_create_account_prefix = AUTO_CREATE_ACCOUNT_PREFIX
+        self.shards_account_prefix = (
+            self.auto_create_account_prefix + 'shards_')
         if config_true_value(conf.get('allow_versions', 'f')):
             self.save_headers.append('x-versions-location')
         if 'allow_versions' in conf:
@@ -375,14 +377,12 @@ class ContainerController(BaseStorageServer):
         # auto create accounts)
         obj_policy_index = self.get_and_validate_policy_index(req) or 0
         broker = self._get_container_broker(drive, part, account, container)
-        if account.startswith(self.auto_create_account_prefix) and obj and \
+        if obj:
-                not os.path.exists(broker.db_file):
+            self._maybe_autocreate(broker, req_timestamp, account,
-            try:
+                                   obj_policy_index, req)
-                broker.initialize(req_timestamp.internal, obj_policy_index)
+        elif not os.path.exists(broker.db_file):
-            except DatabaseAlreadyExists:
-                pass
-        if not os.path.exists(broker.db_file):
             return HTTPNotFound()
+
         if obj:     # delete object
             # redirect if a shard range exists for the object name
             redirect = self._redirect_to_shard(req, broker, obj)
@@ -449,11 +449,25 @@ class ContainerController(BaseStorageServer):
             broker.update_status_changed_at(timestamp)
         return recreated
 
+    def _should_autocreate(self, account, req):
+        auto_create_header = req.headers.get('X-Backend-Auto-Create')
+        if auto_create_header:
+            # If the caller included an explicit X-Backend-Auto-Create header,
+            # assume they know the behavior they want
+            return config_true_value(auto_create_header)
+        if account.startswith(self.shards_account_prefix):
+            # we have to specical case this subset of the
+            # auto_create_account_prefix because we don't want the updater
+            # accidently auto-creating shards; only the sharder creates
+            # shards and it will explicitly tell the server to do so
+            return False
+        return account.startswith(self.auto_create_account_prefix)
+
     def _maybe_autocreate(self, broker, req_timestamp, account,
-                          policy_index):
+                          policy_index, req):
         created = False
-        if account.startswith(self.auto_create_account_prefix) and \
+        should_autocreate = self._should_autocreate(account, req)
-                not os.path.exists(broker.db_file):
+        if should_autocreate and not os.path.exists(broker.db_file):
             if policy_index is None:
                 raise HTTPBadRequest(
                     'X-Backend-Storage-Policy-Index header is required')
@@ -506,8 +520,8 @@ class ContainerController(BaseStorageServer):
             # obj put expects the policy_index header, default is for
             # legacy support during upgrade.
             obj_policy_index = requested_policy_index or 0
-            self._maybe_autocreate(broker, req_timestamp, account,
+            self._maybe_autocreate(
-                                   obj_policy_index)
+                broker, req_timestamp, account, obj_policy_index, req)
             # redirect if a shard exists for this object name
             response = self._redirect_to_shard(req, broker, obj)
             if response:
@@ -531,8 +545,8 @@ class ContainerController(BaseStorageServer):
                                 for sr in json.loads(req.body)]
             except (ValueError, KeyError, TypeError) as err:
                 return HTTPBadRequest('Invalid body: %r' % err)
-            created = self._maybe_autocreate(broker, req_timestamp, account,
+            created = self._maybe_autocreate(
-                                             requested_policy_index)
+                broker, req_timestamp, account, requested_policy_index, req)
             self._update_metadata(req, broker, req_timestamp, 'PUT')
             if shard_ranges:
                 # TODO: consider writing the shard ranges into the pending
@@ -805,7 +819,7 @@ class ContainerController(BaseStorageServer):
         requested_policy_index = self.get_and_validate_policy_index(req)
         broker = self._get_container_broker(drive, part, account, container)
         self._maybe_autocreate(broker, req_timestamp, account,
-                               requested_policy_index)
+                               requested_policy_index, req)
         try:
             objs = json.load(req.environ['wsgi.input'])
         except ValueError as err:

swift/container/sharder.py

@@ -1148,7 +1148,8 @@ class ContainerSharder(ContainerReplicator):
                 'X-Backend-Storage-Policy-Index': broker.storage_policy_index,
                 'X-Container-Sysmeta-Shard-Quoted-Root': quote(
                     broker.root_path),
-                'X-Container-Sysmeta-Sharding': True}
+                'X-Container-Sysmeta-Sharding': 'True',
+                'X-Backend-Auto-Create': 'True'}
             # NB: we *used* to send along
             #    'X-Container-Sysmeta-Shard-Root': broker.root_path
             # but that isn't safe for container names with nulls or newlines

test/unit/container/test_server.py

@@ -2380,15 +2380,17 @@ class TestContainerController(unittest.TestCase):
                    'X-Container-Sysmeta-Test': 'set',
                    'X-Container-Meta-Test': 'persisted'}
 
-        # PUT shard range to non-existent container with non-autocreate prefix
+        # PUT shard range to non-existent container without autocreate flag
-        req = Request.blank('/sda1/p/a/c', method='PUT', headers=headers,
+        req = Request.blank(
-                            body=json.dumps([dict(shard_range)]))
+            '/sda1/p/.shards_a/shard_c', method='PUT', headers=headers,
+            body=json.dumps([dict(shard_range)]))
         resp = req.get_response(self.controller)
         self.assertEqual(404, resp.status_int)
 
-        # PUT shard range to non-existent container with autocreate prefix,
+        # PUT shard range to non-existent container with autocreate flag,
         # missing storage policy
         headers['X-Timestamp'] = next(ts_iter).internal
+        headers['X-Backend-Auto-Create'] = 't'
         req = Request.blank(
             '/sda1/p/.shards_a/shard_c', method='PUT', headers=headers,
             body=json.dumps([dict(shard_range)]))
@@ -2397,7 +2399,7 @@ class TestContainerController(unittest.TestCase):
         self.assertIn(b'X-Backend-Storage-Policy-Index header is required',
                       resp.body)
 
-        # PUT shard range to non-existent container with autocreate prefix
+        # PUT shard range to non-existent container with autocreate flag
         headers['X-Timestamp'] = next(ts_iter).internal
         policy_index = random.choice(POLICIES).idx
         headers['X-Backend-Storage-Policy-Index'] = str(policy_index)
@@ -2407,7 +2409,7 @@ class TestContainerController(unittest.TestCase):
         resp = req.get_response(self.controller)
         self.assertEqual(201, resp.status_int)
 
-        # repeat PUT of shard range to autocreated container - 204 response
+        # repeat PUT of shard range to autocreated container - 202 response
         headers['X-Timestamp'] = next(ts_iter).internal
         headers.pop('X-Backend-Storage-Policy-Index')  # no longer required
         req = Request.blank(
@@ -2416,7 +2418,7 @@ class TestContainerController(unittest.TestCase):
         resp = req.get_response(self.controller)
         self.assertEqual(202, resp.status_int)
 
-        # regular PUT to autocreated container - 204 response
+        # regular PUT to autocreated container - 202 response
         headers['X-Timestamp'] = next(ts_iter).internal
         req = Request.blank(
             '/sda1/p/.shards_a/shard_c', method='PUT',
@@ -4649,61 +4651,53 @@ class TestContainerController(unittest.TestCase):
                              "%d on param %s" % (resp.status_int, param))
 
     def test_put_auto_create(self):
-        headers = {'x-timestamp': Timestamp(1).internal,
+        def do_test(expected_status, path, extra_headers=None, body=None):
-                   'x-size': '0',
+            headers = {'x-timestamp': Timestamp(1).internal,
-                   'x-content-type': 'text/plain',
+                       'x-size': '0',
-                   'x-etag': 'd41d8cd98f00b204e9800998ecf8427e'}
+                       'x-content-type': 'text/plain',
+                       'x-etag': 'd41d8cd98f00b204e9800998ecf8427e'}
+            if extra_headers:
+                headers.update(extra_headers)
+            req = Request.blank('/sda1/p/' + path,
+                                environ={'REQUEST_METHOD': 'PUT'},
+                                headers=headers, body=body)
+            resp = req.get_response(self.controller)
+            self.assertEqual(resp.status_int, expected_status)
 
-        req = Request.blank('/sda1/p/a/c/o',
+        do_test(404, 'a/c/o')
-                            environ={'REQUEST_METHOD': 'PUT'},
+        do_test(404, '.a/c/o', {'X-Backend-Auto-Create': 'no'})
-                            headers=dict(headers))
+        do_test(201, '.a/c/o')
-        resp = req.get_response(self.controller)
+        do_test(404, 'a/.c/o')
-        self.assertEqual(resp.status_int, 404)
+        do_test(404, 'a/c/.o')
+        do_test(201, 'a/c/o', {'X-Backend-Auto-Create': 'yes'})
 
-        req = Request.blank('/sda1/p/.a/c/o',
+        do_test(404, '.shards_a/c/o')
-                            environ={'REQUEST_METHOD': 'PUT'},
+        create_shard_headers = {
-                            headers=dict(headers))
+            'X-Backend-Record-Type': 'shard',
-        resp = req.get_response(self.controller)
+            'X-Backend-Storage-Policy-Index': '0'}
-        self.assertEqual(resp.status_int, 201)
+        do_test(404, '.shards_a/c', create_shard_headers, '[]')
-
+        create_shard_headers['X-Backend-Auto-Create'] = 't'
-        req = Request.blank('/sda1/p/a/.c/o',
+        do_test(201, '.shards_a/c', create_shard_headers, '[]')
-                            environ={'REQUEST_METHOD': 'PUT'},
-                            headers=dict(headers))
-        resp = req.get_response(self.controller)
-        self.assertEqual(resp.status_int, 404)
-
-        req = Request.blank('/sda1/p/a/c/.o',
-                            environ={'REQUEST_METHOD': 'PUT'},
-                            headers=dict(headers))
-        resp = req.get_response(self.controller)
-        self.assertEqual(resp.status_int, 404)
 
     def test_delete_auto_create(self):
-        headers = {'x-timestamp': Timestamp(1).internal}
+        def do_test(expected_status, path, extra_headers=None):
+            headers = {'x-timestamp': Timestamp(1).internal}
+            if extra_headers:
+                headers.update(extra_headers)
+            req = Request.blank('/sda1/p/' + path,
+                                environ={'REQUEST_METHOD': 'DELETE'},
+                                headers=headers)
+            resp = req.get_response(self.controller)
+            self.assertEqual(resp.status_int, expected_status)
 
-        req = Request.blank('/sda1/p/a/c/o',
+        do_test(404, 'a/c/o')
-                            environ={'REQUEST_METHOD': 'DELETE'},
+        do_test(404, '.a/c/o', {'X-Backend-Auto-Create': 'false'})
-                            headers=dict(headers))
+        do_test(204, '.a/c/o')
-        resp = req.get_response(self.controller)
+        do_test(404, 'a/.c/o')
-        self.assertEqual(resp.status_int, 404)
+        do_test(404, 'a/.c/.o')
-
+        do_test(404, '.shards_a/c/o')
-        req = Request.blank('/sda1/p/.a/c/o',
+        do_test(204, 'a/c/o', {'X-Backend-Auto-Create': 'true'})
-                            environ={'REQUEST_METHOD': 'DELETE'},
+        do_test(204, '.shards_a/c/o', {'X-Backend-Auto-Create': 'true'})
-                            headers=dict(headers))
-        resp = req.get_response(self.controller)
-        self.assertEqual(resp.status_int, 204)
-
-        req = Request.blank('/sda1/p/a/.c/o',
-                            environ={'REQUEST_METHOD': 'DELETE'},
-                            headers=dict(headers))
-        resp = req.get_response(self.controller)
-        self.assertEqual(resp.status_int, 404)
-
-        req = Request.blank('/sda1/p/a/.c/.o',
-                            environ={'REQUEST_METHOD': 'DELETE'},
-                            headers=dict(headers))
-        resp = req.get_response(self.controller)
-        self.assertEqual(resp.status_int, 404)
 
     def test_content_type_on_HEAD(self):
         Request.blank('/sda1/p/a/o',