Fix DELETE Object to delete segments when it is multipart object

When deleting an object created via Multipart Upload, delete both the
manifest file and the segments by adding "multipart-manifest=delete"
to the query string.

This requires an additional HEAD before each DELETE, which adds a good
bit of overhead. There is a new config option, "allow_multipart_uploads"
which operators may turn off to avoid this overhead if they know their
use-case does not require Multipart Uploads.

Co-Authored-By: Tim Burke <tim.burke@gmail.com>
Change-Id: Ie1889750b0e6fbe48af0da40596f09ed504b9099
Closes-Bug: #1420144

etc/proxy-server.conf-sample

@@ -87,6 +87,12 @@ use = egg:swift3#swift3
 # middlewares in order to use other 3rd party (or your proprietary) authenticate middleware.
 # auth_pipeline_check = True
 #
+# Enable multi-part uploads. (default: true)
+# This is required to store files larger than Swift's max_file_size (by default, 5GiB).
+# Note that has performance implications when deleting objects, as we now have to
+# check for whether there are also segments to delete.
+# allow_multipart_uploads = True
+#
 # Set the maximum number of parts for Upload Part operation.(default: 1000)
 # When setting it to be larger than the default value in order to match the
 # specification of S3, set to be larger max_manifest_segments for slo

swift3/acl_handlers.py

@@ -178,6 +178,11 @@ class ObjectAclHandler(BaseAclHandler):
     """
     ObjectAclHandler: Handler for ObjectController
     """
+    def HEAD(self, app):
+        # No check object permission needed at DELETE Object
+        if self.method != 'DELETE':
+            return self._handle_acl(app, 'HEAD')
+
     def PUT(self, app):
         b_resp = self._handle_acl(app, 'HEAD', obj='')
         req_acl = ACL.from_headers(self.req.headers,

swift3/cfg.py

@@ -63,4 +63,5 @@ CONF = Config({
     'max_upload_part_num': 1000,
     'check_bucket_owner': False,
     'force_swift_request_proxy_log': False,
+    'allow_multipart_uploads': True,
 })

swift3/controllers/obj.py

@@ -15,7 +15,7 @@
 
 import sys
 
-from swift.common.http import HTTP_OK, HTTP_PARTIAL_CONTENT
+from swift.common.http import HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_NO_CONTENT
 from swift.common.swob import Range, content_range_header_value
 
 from swift3.controllers.base import Controller
@@ -113,7 +113,13 @@ class ObjectController(Controller):
         Handle DELETE Object request
         """
         try:
-            resp = req.get_response(self.app)
+            query = req.gen_multipart_manifest_delete_query(self.app)
+            resp = req.get_response(self.app, query=query)
+            if query and resp.status_int == HTTP_OK:
+                for chunk in resp.app_iter:
+                    pass  # drain the bulk-deleter response
+                resp.status = HTTP_NO_CONTENT
+                resp.body = ''
         except NoSuchKey:
             # expect to raise NoSuchBucket when the bucket doesn't exist
             exc_type, exc_value, exc_traceback = sys.exc_info()

swift3/middleware.py

@@ -69,7 +69,7 @@ class Swift3Middleware(object):
     """Swift3 S3 compatibility midleware"""
     def __init__(self, app, conf, *args, **kwargs):
         self.app = app
-        self.slo_enabled = True
+        self.slo_enabled = conf['allow_multipart_uploads']
         self.check_pipeline(conf)
 
     def __call__(self, env, start_response):
@@ -126,9 +126,9 @@ class Swift3Middleware(object):
                                      pipeline.index('proxy-server')]
 
             # Check SLO middleware
-            if 'slo' not in auth_pipeline:
+            if self.slo_enabled and 'slo' not in auth_pipeline:
                 self.slo_enabled = False
-                LOGGER.warning('swift3 middleware is required SLO middleware '
+                LOGGER.warning('swift3 middleware requires SLO middleware '
                                'to support multi-part upload, please add it '
                                'in pipline')
 
@@ -181,7 +181,8 @@ def filter_factory(global_conf, **local_conf):
         max_bucket_listing=CONF['max_bucket_listing'],
         max_parts_listing=CONF['max_parts_listing'],
         max_upload_part_num=CONF['max_upload_part_num'],
-        max_multi_delete_objects=CONF['max_multi_delete_objects']
+        max_multi_delete_objects=CONF['max_multi_delete_objects'],
+        allow_multipart_uploads=CONF['allow_multipart_uploads'],
     )
 
     def swift3_filter(app):

swift3/request.py

@@ -539,6 +539,7 @@ class Request(swob.Request):
                     HTTP_ACCEPTED,
                 ],
                 'DELETE': [
+                    HTTP_OK,
                     HTTP_NO_CONTENT,
                 ],
             }
@@ -737,6 +738,13 @@ class Request(swob.Request):
             return headers_to_container_info(
                 resp.sw_headers, resp.status_int)  # pylint: disable-msg=E1101
 
+    def gen_multipart_manifest_delete_query(self, app):
+        if not CONF.allow_multipart_uploads:
+            return None
+        query = {'multipart-manifest': 'delete'}
+        resp = self.get_response(app, 'HEAD')
+        return query if resp.is_slo else None
+
 
 class S3AclRequest(Request):
     """

swift3/response.py

@@ -84,6 +84,7 @@ class Response(ResponseBase, swob.Response):
         sw_sysmeta_headers = swob.HeaderKeyDict()
         sw_headers = swob.HeaderKeyDict()
         headers = HeaderKeyDict()
+        self.is_slo = False
 
         for key, val in self.headers.iteritems():
             _key = key.lower()
@@ -103,6 +104,9 @@ class Response(ResponseBase, swob.Response):
                           'content-range', 'content-encoding',
                           'etag', 'last-modified'):
                 headers[key] = val
+            elif _key == 'x-static-large-object':
+                # for delete slo
+                self.is_slo = val
 
         self.headers = headers
         # Used for pure swift header handling at the request layer

swift3/test/unit/test_obj.py

@@ -602,13 +602,64 @@ class TestSwift3Obj(Swift3TestCase):
             self.assertEquals(code, 'NoSuchBucket')
 
     @s3acl
-    def test_object_DELETE(self):
+    @patch('swift3.cfg.CONF.allow_multipart_uploads', False)
+    def test_object_DELETE_no_multipart(self):
         req = Request.blank('/bucket/object',
                             environ={'REQUEST_METHOD': 'DELETE'},
                             headers={'Authorization': 'AWS test:tester:hmac'})
         status, headers, body = self.call_swift3(req)
         self.assertEquals(status.split()[0], '204')
 
+        self.assertNotIn(('HEAD', '/v1/AUTH_test/bucket/object'),
+                         self.swift.calls)
+        self.assertIn(('DELETE', '/v1/AUTH_test/bucket/object'),
+                      self.swift.calls)
+        _, path = self.swift.calls[-1]
+        self.assertEquals(path.count('?'), 0)
+
+    @s3acl
+    def test_object_DELETE_multipart(self):
+        req = Request.blank('/bucket/object',
+                            environ={'REQUEST_METHOD': 'DELETE'},
+                            headers={'Authorization': 'AWS test:tester:hmac'})
+        status, headers, body = self.call_swift3(req)
+        self.assertEquals(status.split()[0], '204')
+
+        self.assertIn(('HEAD', '/v1/AUTH_test/bucket/object'),
+                      self.swift.calls)
+        self.assertIn(('DELETE', '/v1/AUTH_test/bucket/object'),
+                      self.swift.calls)
+        _, path = self.swift.calls[-1]
+        self.assertEquals(path.count('?'), 0)
+
+    @s3acl
+    def test_slo_object_DELETE(self):
+        self.swift.register('HEAD', '/v1/AUTH_test/bucket/object',
+                            swob.HTTPOk,
+                            {'x-static-large-object': 'True'},
+                            None)
+        self.swift.register('DELETE', '/v1/AUTH_test/bucket/object',
+                            swob.HTTPOk, {}, '<SLO delete results>')
+        req = Request.blank('/bucket/object',
+                            environ={'REQUEST_METHOD': 'DELETE'},
+                            headers={'Authorization': 'AWS test:tester:hmac'})
+        status, headers, body = self.call_swift3(req)
+        self.assertEqual(status.split()[0], '204')
+        self.assertEqual(body, '')
+
+        self.assertIn(('HEAD', '/v1/AUTH_test/bucket/object'),
+                      self.swift.calls)
+        self.assertIn(('DELETE', '/v1/AUTH_test/bucket/object'
+                                 '?multipart-manifest=delete'),
+                      self.swift.calls)
+        _, path = self.swift.calls[-1]
+        path, query_string = path.split('?', 1)
+        query = {}
+        for q in query_string.split('&'):
+            key, arg = q.split('=')
+            query[key] = arg
+        self.assertEquals(query['multipart-manifest'], 'delete')
+
     def _test_object_for_s3acl(self, method, account):
         req = Request.blank('/bucket/object',
                             environ={'REQUEST_METHOD': method},

swift3/test/unit/test_s3_acl.py

@@ -70,12 +70,14 @@ def s3acl(func=None, s3acl_only=False):
                 message += failing_point
                 raise exc_type(message)
 
+        instance = args[0]
+
         if not s3acl_only:
             call_func()
+            instance.swift._calls = []
 
         with patch('swift3.cfg.CONF.s3_acl', True):
             owner = Owner('test:tester', 'test:tester')
-            instance = args[0]
             generate_s3acl_environ('test', instance.swift, owner)
             call_func(' (fail at s3_acl)')