Merge "s3api: return 503 if mpu complete gets 409 deleting marker"

swift/common/middleware/s3api/controllers/multi_upload.py

@@ -68,8 +68,9 @@ import time
 import six
 
 from swift.common import constraints
-from swift.common.swob import Range, bytes_to_wsgi, normalize_etag, wsgi_to_str
+from swift.common.swob import Range, bytes_to_wsgi, normalize_etag, \
-from swift.common.utils import json, public, reiterate, md5
+    wsgi_to_str
+from swift.common.utils import json, public, reiterate, md5, Timestamp
 from swift.common.db import utf8encode
 from swift.common.request_helpers import get_container_update_override_key, \
     get_param
@@ -82,7 +83,7 @@ from swift.common.middleware.s3api.s3response import InvalidArgument, \
     ErrorResponse, MalformedXML, BadDigest, KeyTooLongError, \
     InvalidPart, BucketAlreadyExists, EntityTooSmall, InvalidPartOrder, \
     InvalidRequest, HTTPOk, HTTPNoContent, NoSuchKey, NoSuchUpload, \
-    NoSuchBucket, BucketAlreadyOwnedByYou
+    NoSuchBucket, BucketAlreadyOwnedByYou, ServiceUnavailable
 from swift.common.middleware.s3api.utils import unique_id, \
     MULTIUPLOAD_SUFFIX, S3Timestamp, sysmeta_header
 from swift.common.middleware.s3api.etree import Element, SubElement, \
@@ -96,6 +97,17 @@ MAX_COMPLETE_UPLOAD_BODY_SIZE = 2048 * 1024
 
 
 def _get_upload_info(req, app, upload_id):
+    """
+    Make a HEAD request for existing upload object metadata. Tries the upload
+    marker first, and then falls back to the manifest object.
+
+    :param req: an S3Request object.
+    :param app: the wsgi app.
+    :param upload_id: the upload id.
+    :returns: a tuple of (S3Response, boolean) where the boolean is True if the
+        response is from the upload marker and False otherwise.
+    :raises: NoSuchUpload if neither the marker nor the manifest were found.
+    """
 
     container = req.container_name + MULTIUPLOAD_SUFFIX
     obj = '%s/%s' % (req.object_name, upload_id)
@@ -106,7 +118,8 @@ def _get_upload_info(req, app, upload_id):
     # it off for now...
     copy_source = req.headers.pop('X-Amz-Copy-Source', None)
     try:
-        return req.get_response(app, 'HEAD', container=container, obj=obj)
+        resp = req.get_response(app, 'HEAD', container=container, obj=obj)
+        return resp, True
     except NoSuchKey:
         # ensure consistent path and policy are logged despite manifest HEAD
         upload_marker_path = req.environ.get('s3api.backend_path')
@@ -115,7 +128,7 @@ def _get_upload_info(req, app, upload_id):
             resp = req.get_response(app, 'HEAD')
             if resp.sysmeta_headers.get(sysmeta_header(
                     'object', 'upload-id')) == upload_id:
-                return resp
+                return resp, False
         except NoSuchKey:
             pass
         finally:
@@ -655,7 +668,14 @@ class UploadController(Controller):
         Handles Complete Multipart Upload.
         """
         upload_id = get_param(req, 'uploadId')
-        resp = _get_upload_info(req, self.app, upload_id)
+        resp, is_marker = _get_upload_info(req, self.app, upload_id)
+        if (is_marker and
+                resp.sw_headers.get('X-Backend-Timestamp') >= Timestamp.now()):
+            # Somehow the marker was created in the future w.r.t. this thread's
+            # clock. The manifest PUT may succeed but the subsequent marker
+            # DELETE will fail, so don't attempt either.
+            raise ServiceUnavailable
+
         headers = {'Accept': 'application/json',
                    sysmeta_header('object', 'upload-id'): upload_id}
         for key, val in resp.headers.items():

swift/common/middleware/s3api/s3request.py

@@ -1432,8 +1432,10 @@ class S3Request(swob.Request):
                 raise SlowDown(status='429 Slow Down')
             raise SlowDown()
         if resp.status_int == HTTP_CONFLICT:
-            # TODO: validate that this actually came up out of SLO
+            if self.method == 'GET':
                 raise BrokenMPU()
+            else:
+                raise ServiceUnavailable()
 
         raise InternalError('unexpected status code %d' % status)
 

test/unit/common/middleware/s3api/test_multi_upload.py

@@ -24,7 +24,7 @@ from six.moves.urllib.parse import parse_qs, quote, quote_plus
 
 from swift.common import swob
 from swift.common.swob import Request
-from swift.common.utils import json, md5
+from swift.common.utils import json, md5, Timestamp
 
 from test.unit import FakeMemcache, patch_policies
 from test.unit.common.middleware.s3api import S3ApiTestCase
@@ -1649,6 +1649,64 @@ class TestS3ApiMultiUpload(S3ApiTestCase):
         self.assertEqual(headers.get('X-Object-Meta-Foo'), 'bar')
         self.assertEqual(headers.get('Content-Type'), 'baz/quux')
 
+    def _do_test_object_multipart_upload_complete_marker_in_future(
+            self, marker_timestamp, now_timestamp):
+        # verify that clock skew is detected before manifest is created and
+        # results in a 503
+        segment_bucket = '/v1/AUTH_test/bucket+segments'
+        self.swift.register('HEAD', segment_bucket + '/object/X',
+                            swob.HTTPOk,
+                            {'x-object-meta-foo': 'bar',
+                             'content-type': 'application/directory',
+                             'x-object-sysmeta-s3api-has-content-type': 'yes',
+                             'x-object-sysmeta-s3api-content-type':
+                                 'baz/quux',
+                             'X-Backend-Timestamp': marker_timestamp.internal},
+                            None)
+        req = Request.blank('/bucket/object?uploadId=X',
+                            environ={'REQUEST_METHOD': 'POST'},
+                            headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Date': self.get_date_header(skew=100), },
+                            body=XML)
+
+        # marker created in the future
+        with patch('swift.common.middleware.s3api.controllers.multi_upload.'
+                   'Timestamp.now', return_value=now_timestamp):
+            status, headers, body = self.call_s3api(req)
+        self.assertEqual(status.split()[0], '503')
+        self.assertEqual('ServiceUnavailable', self._get_error_code(body))
+        self.assertEqual(self.swift.calls, [
+            ('HEAD', '/v1/AUTH_test'),
+            ('HEAD', '/v1/AUTH_test/bucket'),
+            ('HEAD', '/v1/AUTH_test/bucket+segments/object/X')])
+
+    def test_object_multipart_upload_complete_marker_ts_now(self):
+        marker_timestamp = now_timestamp = Timestamp.now()
+        self._do_test_object_multipart_upload_complete_marker_in_future(
+            marker_timestamp, now_timestamp)
+
+    def test_object_multipart_upload_complete_marker_ts_in_future(self):
+        marker_timestamp = Timestamp.now()
+        now_timestamp = Timestamp(float(marker_timestamp) - 1)
+        self._do_test_object_multipart_upload_complete_marker_in_future(
+            marker_timestamp, now_timestamp)
+
+    def test_object_multipart_upload_complete_409_on_marker_delete(self):
+        # verify that clock skew preventing an upload marker DELETE results in
+        # a 503 (this would be unexpected because there's a check for clock
+        # skew before the manifest PUT and marker DELETE)
+        segment_bucket = '/v1/AUTH_test/bucket+segments'
+        self.swift.register('DELETE', segment_bucket + '/object/X',
+                            swob.HTTPConflict, {}, None)
+        req = Request.blank('/bucket/object?uploadId=X',
+                            environ={'REQUEST_METHOD': 'POST'},
+                            headers={'Authorization': 'AWS test:tester:hmac',
+                                     'Date': self.get_date_header(), },
+                            body=XML)
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status.split()[0], '503')
+        self.assertEqual('ServiceUnavailable', self._get_error_code(body))
+
     def test_object_multipart_upload_complete_old_content_type(self):
         self.swift.register_unconditionally(
             'HEAD', '/v1/AUTH_test/bucket+segments/object/X',