s3api: Delete multipart uploads via multi-delete

tipabu · tipabu · commit b35fc41184ed · 2019-01-07T23:17:11.000Z
We have code that's *supposed* to do it, but we weren't reading the
result of the bulk-delete, so we never actually deleted anything!

Change-Id: I5c972749cadf903161456f34371a6f83ebc05eb9
Closes-Bug: 1810567
diff --git a/swift/common/middleware/s3api/controllers/multi_delete.py b/swift/common/middleware/s3api/controllers/multi_delete.py
@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import copy
+import json
 
 from swift.common.constraints import MAX_OBJECT_NAME_LENGTH
 from swift.common.utils import public, StreamingPile
@@ -115,7 +116,30 @@ def do_delete(base_req, key, version):
 
             try:
                 query = req.gen_multipart_manifest_delete_query(self.app)
-                req.get_response(self.app, method='DELETE', query=query)
+                resp = req.get_response(self.app, method='DELETE', query=query,
+                                        headers={'Accept': 'application/json'})
+                # Have to read the response to actually do the SLO delete
+                if query:
+                    try:
+                        delete_result = json.loads(resp.body)
+                        if delete_result['Errors']:
+                            # NB: bulk includes 404s in "Number Not Found",
+                            # not "Errors"
+                            msg_parts = [delete_result['Response Status']]
+                            msg_parts.extend(
+                                '%s: %s' % (obj, status)
+                                for obj, status in delete_result['Errors'])
+                            return key, {'code': 'SLODeleteError',
+                                         'message': '\n'.join(msg_parts)}
+                        # else, all good
+                    except (ValueError, TypeError, KeyError):
+                        # Logs get all the gory details
+                        self.logger.exception(
+                            'Could not parse SLO delete response: %r',
+                            resp.body)
+                        # Client gets something more generic
+                        return key, {'code': 'SLODeleteError',
+                                     'message': 'Unexpected swift response'}
             except NoSuchKey:
                 pass
             except ErrorResponse as e:
diff --git a/test/functional/s3api/test_multi_upload.py b/test/functional/s3api/test_multi_upload.py
@@ -32,7 +32,8 @@
 
 from test.functional.s3api import S3ApiBase
 from test.functional.s3api.s3_test_client import Connection
-from test.functional.s3api.utils import get_error_code, get_error_msg
+from test.functional.s3api.utils import get_error_code, get_error_msg, \
+    calculate_md5
 
 
 def setUpModule():
@@ -907,6 +908,27 @@ def test_delete_bucket_multi_upload_object_exisiting(self):
         self.assertEqual(status, 200)  # sanity
         self.assertEqual(content, body)  # sanity
 
+        # Can delete it with DeleteMultipleObjects request
+        elem = Element('Delete')
+        SubElement(elem, 'Quiet').text = 'true'
+        obj_elem = SubElement(elem, 'Object')
+        SubElement(obj_elem, 'Key').text = key
+        body = tostring(elem, use_s3ns=False)
+
+        status, headers, body = self.conn.make_request(
+            'POST', bucket, body=body, query='delete',
+            headers={'Content-MD5': calculate_md5(body)})
+        self.assertEqual(status, 200)
+        self.assertCommonResponseHeaders(headers)
+
+        status, headers, body = \
+            self.conn.make_request('GET', bucket, key)
+        self.assertEqual(status, 404)  # sanity
+
+        # Now we can delete
+        status, headers, body = \
+            self.conn.make_request('DELETE', bucket)
+        self.assertEqual(status, 204)  # sanity
 
 if __name__ == '__main__':
     unittest2.main()
diff --git a/test/unit/common/middleware/s3api/test_multi_delete.py b/test/unit/common/middleware/s3api/test_multi_delete.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import json
 import unittest
 from datetime import datetime
 from hashlib import md5
@@ -64,8 +65,15 @@ def test_object_multi_DELETE(self):
                             swob.HTTPNoContent, {}, None)
         self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key2',
                             swob.HTTPNotFound, {}, None)
+        slo_delete_resp = {
+            'Number Not Found': 0,
+            'Response Status': '200 OK',
+            'Errors': [],
+            'Response Body': '',
+            'Number Deleted': 8
+        }
         self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key3',
-                            swob.HTTPOk, {}, None)
+                            swob.HTTPOk, {}, json.dumps(slo_delete_resp))
 
         elem = Element('Delete')
         for key in ['Key1', 'Key2', 'Key3']:
@@ -97,15 +105,31 @@ def test_object_multi_DELETE(self):
 
     @s3acl
     def test_object_multi_DELETE_with_error(self):
-        self.swift.register('HEAD', '/v1/AUTH_test/bucket/Key3',
-                            swob.HTTPForbidden, {}, None)
         self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key1',
                             swob.HTTPNoContent, {}, None)
         self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key2',
                             swob.HTTPNotFound, {}, None)
+        self.swift.register('HEAD', '/v1/AUTH_test/bucket/Key3',
+                            swob.HTTPForbidden, {}, None)
+        self.swift.register('HEAD', '/v1/AUTH_test/bucket/Key4',
+                            swob.HTTPOk,
+                            {'x-static-large-object': 'True'},
+                            None)
+        slo_delete_resp = {
+            'Number Not Found': 0,
+            'Response Status': '400 Bad Request',
+            'Errors': [
+                ["/bucket+segments/obj1", "403 Forbidden"],
+                ["/bucket+segments/obj2", "403 Forbidden"]
+            ],
+            'Response Body': '',
+            'Number Deleted': 8
+        }
+        self.swift.register('DELETE', '/v1/AUTH_test/bucket/Key4',
+                            swob.HTTPOk, {}, json.dumps(slo_delete_resp))
 
         elem = Element('Delete')
-        for key in ['Key1', 'Key2', 'Key3']:
+        for key in ['Key1', 'Key2', 'Key3', 'Key4']:
             obj = SubElement(elem, 'Object')
             SubElement(obj, 'Key').text = key
         body = tostring(elem, use_s3ns=False)
@@ -123,13 +147,24 @@ def test_object_multi_DELETE_with_error(self):
 
         elem = fromstring(body)
         self.assertEqual(len(elem.findall('Deleted')), 2)
-        self.assertEqual(len(elem.findall('Error')), 1)
+        self.assertEqual(len(elem.findall('Error')), 2)
+        self.assertEqual(
+            [(el.find('Code').text, el.find('Message').text)
+             for el in elem.findall('Error')],
+            [('AccessDenied', 'Access Denied.'),
+             ('SLODeleteError', '\n'.join([
+                 '400 Bad Request',
+                 '/bucket+segments/obj1: 403 Forbidden',
+                 '/bucket+segments/obj2: 403 Forbidden']))]
+        )
         self.assertEqual(self.swift.calls, [
             ('HEAD', '/v1/AUTH_test/bucket'),
             ('HEAD', '/v1/AUTH_test/bucket/Key1'),
             ('DELETE', '/v1/AUTH_test/bucket/Key1'),
             ('HEAD', '/v1/AUTH_test/bucket/Key2'),
             ('HEAD', '/v1/AUTH_test/bucket/Key3'),
+            ('HEAD', '/v1/AUTH_test/bucket/Key4'),
+            ('DELETE', '/v1/AUTH_test/bucket/Key4?multipart-manifest=delete'),
         ])
 
     @s3acl
