s3api: Allow PUT with if-none-match: *

Swift already supports that much, at least. AWS used to not support any
conditional PUTs, but that's changed somewhat recently; see

- https://aws.amazon.com/about-aws/whats-new/2024/08/amazon-s3-conditional-writes/
- https://aws.amazon.com/about-aws/whats-new/2024/11/amazon-s3-functionality-conditional-writes/

Drive-By: Fix retry of a CompleteMultipartUpload with changed parts; it
should 404 rather than succeed in writing the new manifest.

Change-Id: I2e57dacb342b5758f16b502bb91372a2443d0182

doc/s3api/conf/ceph-known-failures-keystone.yaml

@@ -87,8 +87,6 @@ ceph_s3:
   s3tests.functional.test_s3.test_put_object_ifmatch_overwrite_existed_good: {status: KNOWN}
   s3tests.functional.test_s3.test_put_object_ifnonmatch_failed: {status: KNOWN}
   s3tests.functional.test_s3.test_put_object_ifnonmatch_good: {status: KNOWN}
-  s3tests.functional.test_s3.test_put_object_ifnonmatch_nonexisted_good: {status: KNOWN}
-  s3tests.functional.test_s3.test_put_object_ifnonmatch_overwrite_existed_failed: {status: KNOWN}
   s3tests.functional.test_s3.test_set_cors: {status: KNOWN}
   s3tests.functional.test_s3.test_stress_bucket_acls_changes: {status: KNOWN}
   s3tests.functional.test_s3.test_versioned_concurrent_object_create_concurrent_remove: {status: KNOWN}

doc/s3api/conf/ceph-known-failures-tempauth.yaml

@@ -234,8 +234,6 @@ ceph_s3:
   s3tests_boto3.functional.test_s3.test_put_object_ifmatch_overwrite_existed_good: {status: KNOWN}
   s3tests_boto3.functional.test_s3.test_put_object_ifnonmatch_failed: {status: KNOWN}
   s3tests_boto3.functional.test_s3.test_put_object_ifnonmatch_good: {status: KNOWN}
-  s3tests_boto3.functional.test_s3.test_put_object_ifnonmatch_nonexisted_good: {status: KNOWN}
-  s3tests_boto3.functional.test_s3.test_put_object_ifnonmatch_overwrite_existed_failed: {status: KNOWN}
   s3tests_boto3.functional.test_s3.test_put_tags_acl_public: {status: KNOWN}
   s3tests_boto3.functional.test_s3.test_set_cors: {status: KNOWN}
   s3tests_boto3.functional.test_s3.test_set_tagging: {status: KNOWN}

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

@@ -80,7 +80,8 @@ from swift.common.middleware.s3api.s3response import InvalidArgument, \
     ErrorResponse, MalformedXML, BadDigest, KeyTooLongError, \
     InvalidPart, BucketAlreadyExists, EntityTooSmall, InvalidPartOrder, \
     InvalidRequest, HTTPOk, HTTPNoContent, NoSuchKey, NoSuchUpload, \
-    NoSuchBucket, BucketAlreadyOwnedByYou, ServiceUnavailable
+    NoSuchBucket, BucketAlreadyOwnedByYou, ServiceUnavailable, \
+    PreconditionFailed, S3NotImplemented
 from swift.common.middleware.s3api.utils import unique_id, \
     MULTIUPLOAD_SUFFIX, S3Timestamp, sysmeta_header
 from swift.common.middleware.s3api.etree import Element, SubElement, \
@@ -646,6 +647,14 @@ class UploadController(Controller):
         Handles Complete Multipart Upload.
         """
         upload_id = get_param(req, 'uploadId')
+        # Check for conditional requests before getting upload info so the
+        # headers can't bleed into the HEAD
+        if req.headers.get('If-None-Match', '*') != '*' or any(
+                h in req.headers for h in (
+                    'If-Match', 'If-Modified-Since', 'If-Unmodified-Since')):
+            raise S3NotImplemented(
+                'Conditional uploads are not supported.')
+
         resp, is_marker = _get_upload_info(req, self.app, upload_id)
         if (is_marker and
                 resp.sw_headers.get('X-Backend-Timestamp') >= Timestamp.now()):
@@ -735,13 +744,17 @@ class UploadController(Controller):
 
         s3_etag = '%s-%d' % (s3_etag_hasher.hexdigest(), len(manifest))
         s3_etag_header = sysmeta_header('object', 'etag')
-        if resp.sysmeta_headers.get(s3_etag_header) == s3_etag:
+        # This header should only already be present if the upload marker
-            # This header should only already be present if the upload marker
+        # has been cleaned up and the current target uses the same upload-id
-            # has been cleaned up and the current target uses the same
+        already_uploaded_s3_etag = resp.sysmeta_headers.get(s3_etag_header)
-            # upload-id; assuming the segments to use haven't changed, the work
+        if already_uploaded_s3_etag == s3_etag:
-            # is already done
+            # If the segments to use haven't changed, the work is already done
             return HTTPOk(body=_make_complete_body(req, s3_etag, False),
                           content_type='application/xml')
+        elif already_uploaded_s3_etag:
+            # If the header's present but *doesn't* match, upload-id is
+            # no longer valid
+            raise NoSuchUpload(upload_id=upload_id)
         headers[s3_etag_header] = s3_etag
         # Leave base header value blank; SLO will populate
         c_etag = '; s3_etag=%s' % s3_etag
@@ -796,7 +809,10 @@ class UploadController(Controller):
                                 continue
                             body.append(chunk)
                         body = json.loads(b''.join(body))
-                        if body['Response Status'] != '201 Created':
+                        if body['Response Status'] == \
+                                '412 Precondition Failed':
+                            raise PreconditionFailed
+                        elif body['Response Status'] != '201 Created':
                             for seg, err in body['Errors']:
                                 if err == too_small_message:
                                     raise EntityTooSmall()

swift/common/middleware/s3api/s3request.py

@@ -918,9 +918,10 @@ class S3Request(swob.Request):
                 raise InvalidArgument('Content-Length',
                                       self.environ['CONTENT_LENGTH'])
 
-        if self.method == 'PUT' and any(h in self.headers for h in (
+        if self.method == 'PUT' and (
-                'If-Match', 'If-None-Match',
+                any(h in self.headers for h in (
-                'If-Modified-Since', 'If-Unmodified-Since')):
+                    'If-Match', 'If-Modified-Since', 'If-Unmodified-Since'))
+                or self.headers.get('If-None-Match', '*') != '*'):
             raise S3NotImplemented(
                 'Conditional object PUTs are not supported.')
 

test/functional/s3api/test_multi_upload.py

@@ -729,6 +729,66 @@ class TestS3ApiMultiUpload(S3ApiBase):
                                    query=query)
         self.assertEqual(get_error_code(body), 'InvalidPart')
 
+    def test_complete_multi_upload_conditional(self):
+        bucket = 'bucket'
+        key = 'obj'
+        self.conn.make_request('PUT', bucket)
+        query = 'uploads'
+        status, headers, body = \
+            self.conn.make_request('POST', bucket, key, query=query)
+        elem = fromstring(body, 'InitiateMultipartUploadResult')
+        upload_id = elem.find('UploadId').text
+
+        query = 'partNumber=1&uploadId=%s' % upload_id
+        status, headers, body = \
+            self.conn.make_request('PUT', bucket, key, query=query)
+        part_etag = headers['etag']
+        xml = self._gen_comp_xml([part_etag])
+
+        for headers in [
+            {'If-Match': part_etag},
+            {'If-Match': '*'},
+            {'If-None-Match': part_etag},
+            {'If-Modified-Since': 'Wed, 21 Oct 2015 07:28:00 GMT'},
+            {'If-Unmodified-Since': 'Wed, 21 Oct 2015 07:28:00 GMT'},
+        ]:
+            with self.subTest(headers=headers):
+                query = 'uploadId=%s' % upload_id
+                status, _, body = self.conn.make_request(
+                    'POST', bucket, key, body=xml,
+                    query=query, headers=headers)
+                self.assertEqual(status, 501)
+                self.assertEqual(get_error_code(body), 'NotImplemented')
+
+        # Can do basic existence checks, though
+        headers = {'If-None-Match': '*'}
+        query = 'uploadId=%s' % upload_id
+        status, _, body = self.conn.make_request(
+            'POST', bucket, key, body=xml,
+            query=query, headers=headers)
+        self.assertEqual(status, 200)
+
+        # And it'll prevent overwrites
+        query = 'uploads'
+        status, headers, body = \
+            self.conn.make_request('POST', bucket, key, query=query)
+        elem = fromstring(body, 'InitiateMultipartUploadResult')
+        upload_id = elem.find('UploadId').text
+
+        query = 'partNumber=1&uploadId=%s' % upload_id
+        status, headers, body = \
+            self.conn.make_request('PUT', bucket, key, query=query)
+        part_etag = headers['etag']
+        xml = self._gen_comp_xml([part_etag])
+
+        headers = {'If-None-Match': '*'}
+        query = 'uploadId=%s' % upload_id
+        status, _, body = self.conn.make_request(
+            'POST', bucket, key, body=xml,
+            query=query, headers=headers)
+        self.assertEqual(status, 412)
+        self.assertEqual(get_error_code(body), 'PreconditionFailed')
+
     def test_complete_upload_min_segment_size(self):
         bucket = 'bucket'
         key = 'obj'

test/functional/s3api/test_object.py

@@ -310,7 +310,7 @@ class TestS3ApiObject(S3ApiBase):
     def test_put_object_conditional_requests(self):
         obj = 'object'
         content = b'abcdefghij'
-        headers = {'If-None-Match': '*'}
+        headers = {'If-None-Match': 'asdf'}
         status, headers, body = \
             self.conn.make_request('PUT', self.bucket, obj, headers, content)
         self.assertEqual(status, 501)
@@ -335,6 +335,18 @@ class TestS3ApiObject(S3ApiBase):
             self.conn.make_request('HEAD', self.bucket, obj, {}, '')
         self.assertEqual(status, 404)
 
+        # But this will
+        headers = {'If-None-Match': '*'}
+        status, headers, body = \
+            self.conn.make_request('PUT', self.bucket, obj, headers, content)
+        self.assertEqual(status, 200)
+
+        # And the if-none-match prevents overwrites
+        headers = {'If-None-Match': '*'}
+        status, headers, body = \
+            self.conn.make_request('PUT', self.bucket, obj, headers, content)
+        self.assertEqual(status, 412)
+
     def test_put_object_expect(self):
         obj = 'object'
         content = b'abcdefghij'

test/s3api/test_conditional_writes.py

@@ -0,0 +1,142 @@
+# Copyright (c) 2025 Nvidia
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+# implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from test.s3api import BaseS3TestCaseWithBucket, status_from_error, \
+    code_from_error
+from botocore.exceptions import ClientError
+
+
+class TestConditionalWrites(BaseS3TestCaseWithBucket):
+    def test_if_none_match_star_simple_put(self):
+        client = self.get_s3_client(1)
+        key_name = self.create_name('if-none-match-simple')
+        # Can create new object fine
+        resp = client.put_object(
+            Bucket=self.bucket_name,
+            Key=key_name,
+            IfNoneMatch='*',
+            Body=b'',
+        )
+        self.assertEqual(200, resp['ResponseMetadata']['HTTPStatusCode'])
+        # But overwrite is blocked
+        with self.assertRaises(ClientError) as caught:
+            client.put_object(
+                Bucket=self.bucket_name,
+                Key=key_name,
+                IfNoneMatch='*',
+                Body=b'',
+            )
+        self.assertEqual(412, status_from_error(caught.exception))
+        self.assertEqual('PreconditionFailed',
+                         code_from_error(caught.exception))
+
+    def test_if_none_match_star_mpu(self):
+        client = self.get_s3_client(1)
+        key_name = self.create_name('if-none-match-mpu')
+
+        create_mpu_resp = client.create_multipart_upload(
+            Bucket=self.bucket_name, Key=key_name)
+        self.assertEqual(200, create_mpu_resp[
+            'ResponseMetadata']['HTTPStatusCode'])
+        upload_id = create_mpu_resp['UploadId']
+        parts = []
+        for part_num in range(1, 4):
+            part_resp = client.upload_part(
+                Body=b'x' * 5 * 1024 * 1024,
+                Bucket=self.bucket_name, Key=key_name,
+                PartNumber=part_num, UploadId=upload_id)
+            self.assertEqual(200, part_resp[
+                'ResponseMetadata']['HTTPStatusCode'])
+            parts.append({
+                'ETag': part_resp['ETag'],
+                'PartNumber': part_num,
+            })
+
+        # Nothing there, so complete succeeds
+        complete_mpu_resp = client.complete_multipart_upload(
+            Bucket=self.bucket_name,
+            Key=key_name,
+            MultipartUpload={'Parts': parts[:2]},
+            UploadId=upload_id,
+            IfNoneMatch='*',
+        )
+        self.assertEqual(200, complete_mpu_resp[
+            'ResponseMetadata']['HTTPStatusCode'])
+
+        # Retrying with more parts fails
+        with self.assertRaises(ClientError) as caught:
+            client.complete_multipart_upload(
+                Bucket=self.bucket_name,
+                Key=key_name,
+                MultipartUpload={'Parts': parts},
+                UploadId=upload_id,
+                IfNoneMatch='*',
+            )
+        self.assertEqual(404, status_from_error(caught.exception))
+        self.assertEqual('NoSuchUpload',
+                         code_from_error(caught.exception))
+
+        # Ditto fewer
+        with self.assertRaises(ClientError) as caught:
+            client.complete_multipart_upload(
+                Bucket=self.bucket_name,
+                Key=key_name,
+                MultipartUpload={'Parts': parts[:1]},
+                UploadId=upload_id,
+                IfNoneMatch='*',
+            )
+        self.assertEqual(404, status_from_error(caught.exception))
+        self.assertEqual('NoSuchUpload',
+                         code_from_error(caught.exception))
+
+        # Can retry with all the same parts and 200 though
+        complete_mpu_resp = client.complete_multipart_upload(
+            Bucket=self.bucket_name,
+            Key=key_name,
+            MultipartUpload={'Parts': parts[:2]},
+            UploadId=upload_id,
+            IfNoneMatch='*',
+        )
+        self.assertEqual(200, complete_mpu_resp[
+            'ResponseMetadata']['HTTPStatusCode'])
+
+        # Can still start a new upload
+        create_mpu_resp = client.create_multipart_upload(
+            Bucket=self.bucket_name, Key=key_name)
+        self.assertEqual(200, create_mpu_resp[
+            'ResponseMetadata']['HTTPStatusCode'])
+        upload_id = create_mpu_resp['UploadId']
+        # And upload parts
+        part_resp = client.upload_part(
+            Body=b'', Bucket=self.bucket_name, Key=key_name,
+            PartNumber=1, UploadId=upload_id)
+        self.assertEqual(200, part_resp[
+            'ResponseMetadata']['HTTPStatusCode'])
+        parts = [{
+            'ETag': part_resp['ETag'],
+            'PartNumber': 1,
+        }]
+        # But completion will be blocked
+        with self.assertRaises(ClientError) as caught:
+            client.complete_multipart_upload(
+                Bucket=self.bucket_name,
+                Key=key_name,
+                MultipartUpload={'Parts': parts},
+                UploadId=upload_id,
+                IfNoneMatch='*',
+            )
+        self.assertEqual(412, status_from_error(caught.exception))
+        self.assertEqual('PreconditionFailed',
+                         code_from_error(caught.exception))

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

@@ -1589,10 +1589,9 @@ class TestS3ApiMultiUpload(BaseS3ApiMultiUpload, S3ApiTestCase):
                                      'Content-MD5': content_md5, },
                             body=XML)
         status, headers, body = self.call_s3api(req)
-        elem = fromstring(body, 'CompleteMultipartUploadResult')
+        elem = fromstring(body, 'Error')
-        self.assertNotIn('Etag', headers)
+        self.assertEqual(elem.find('Code').text, 'NoSuchUpload')
-        self.assertEqual(elem.find('ETag').text, S3_ETAG)
+        self.assertEqual(status.split()[0], '404')
-        self.assertEqual(status.split()[0], '200')
 
         self.assertEqual(self.swift.calls, [
             # Bucket exists
@@ -1602,24 +1601,11 @@ class TestS3ApiMultiUpload(BaseS3ApiMultiUpload, S3ApiTestCase):
             ('HEAD', '/v1/AUTH_test/bucket+segments/object/X'),
             # But the object does, and with the same upload ID
             ('HEAD', '/v1/AUTH_test/bucket/object'),
-            # Create the SLO
+            # And then we bail
-            ('PUT', '/v1/AUTH_test/bucket/object'
-                    '?heartbeat=on&multipart-manifest=put'),
-            # Retry deleting the marker for the sake of completeness
-            ('DELETE', '/v1/AUTH_test/bucket+segments/object/X')
         ])
         self.assertEqual(req.environ['swift.backend_path'],
                          '/v1/AUTH_test/bucket+segments/object/X')
 
-        _, _, headers = self.swift.calls_with_headers[-2]
-        self.assertEqual(headers.get('X-Object-Meta-Foo'), 'bar')
-        self.assertEqual(headers.get('Content-Type'), 'baz/quux')
-        # SLO will provide a base value
-        override_etag = '; s3_etag=%s' % S3_ETAG.strip('"')
-        h = 'X-Object-Sysmeta-Container-Update-Override-Etag'
-        self.assertEqual(headers.get(h), override_etag)
-        self.assertEqual(headers.get('X-Object-Sysmeta-S3Api-Upload-Id'), 'X')
-
     def test_object_multipart_upload_retry_complete_upload_id_mismatch(self):
         content_md5 = base64.b64encode(md5(
             XML.encode('ascii'), usedforsecurity=False).digest())