Skip to content

Commit 0dcab51

Browse files
author
Rakshil Modi
committed
Implemented No-overwrite for uploads using cp and mv command
Customizing error message test cases for no_overwrite for upload operations using cp command Code cleanup Test cases for move command Removed unwanted code changes code quality improvement Adding test cases Updating help text Removing unwanted comments fixed identation
1 parent df9bac5 commit 0dcab51

File tree

9 files changed

+330
-4
lines changed

9 files changed

+330
-4
lines changed

awscli/customizations/s3/results.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from awscli.customizations.s3.subscribers import OnDoneFilteredSubscriber
2424
from awscli.customizations.s3.utils import WarningResult, human_readable_size
2525
from awscli.customizations.utils import uni_print
26+
from awscli.customizations.s3.utils import create_warning
2627

2728
LOGGER = logging.getLogger(__name__)
2829

@@ -123,7 +124,13 @@ def _on_failure(self, future, e):
123124
if isinstance(e, FatalError):
124125
error_result_cls = ErrorResult
125126
self._result_queue.put(error_result_cls(exception=e))
126-
else:
127+
elif self._is_precondition_failed(e):
128+
LOGGER.debug(
129+
"Warning: Skipping file %s as it already exists on %s",
130+
self._src,
131+
self._dest,
132+
)
133+
else :
127134
self._result_queue.put(
128135
FailureResult(
129136
transfer_type=self._transfer_type,
@@ -132,6 +139,11 @@ def _on_failure(self, future, e):
132139
exception=e,
133140
)
134141
)
142+
143+
def _is_precondition_failed(self, exception):
144+
"""Check if this is a PreconditionFailed error"""
145+
return (hasattr(exception, 'response') and
146+
exception.response.get('Error', {}).get('Code') == 'PreconditionFailed')
135147

136148

137149
class BaseResultHandler:

awscli/customizations/s3/subcommands.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,15 @@
642642
),
643643
}
644644

645+
NO_OVERWRITE = {
646+
'name': 'no-overwrite',
647+
'action': 'store_true',
648+
'help_text': (
649+
"This flag prevents overwriting of files at the destination. With this flag, "
650+
"only files not present at the destination will be transferred."
651+
),
652+
}
653+
645654
TRANSFER_ARGS = [
646655
DRYRUN,
647656
QUIET,
@@ -1057,7 +1066,7 @@ class CpCommand(S3TransferCommand):
10571066
}
10581067
]
10591068
+ TRANSFER_ARGS
1060-
+ [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE]
1069+
+ [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE, NO_OVERWRITE]
10611070
)
10621071

10631072

@@ -1081,6 +1090,7 @@ class MvCommand(S3TransferCommand):
10811090
METADATA_DIRECTIVE,
10821091
RECURSIVE,
10831092
VALIDATE_SAME_S3_PATHS,
1093+
NO_OVERWRITE,
10841094
]
10851095
)
10861096

awscli/customizations/s3/utils.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ def map_put_object_params(cls, request_params, cli_params):
489489
cls._set_sse_c_request_params(request_params, cli_params)
490490
cls._set_request_payer_param(request_params, cli_params)
491491
cls._set_checksum_algorithm_param(request_params, cli_params)
492+
cls._set_no_overwrite_param(request_params, cli_params)
492493

493494
@classmethod
494495
def map_get_object_params(cls, request_params, cli_params):
@@ -557,6 +558,12 @@ def map_delete_object_params(cls, request_params, cli_params):
557558
@classmethod
558559
def map_list_objects_v2_params(cls, request_params, cli_params):
559560
cls._set_request_payer_param(request_params, cli_params)
561+
562+
@classmethod
563+
def _set_no_overwrite_param(cls, request_params, cli_params):
564+
"""Map No overwrite header with IfNoneMatch"""
565+
if cli_params.get('no_overwrite'):
566+
request_params['IfNoneMatch'] = "*"
560567

561568
@classmethod
562569
def _set_request_payer_param(cls, request_params, cli_params):

awscli/s3transfer/manager.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ class TransferManager:
195195
+ [
196196
'ChecksumType',
197197
'MpuObjectSize',
198+
'IfNoneMatch',
198199
]
199200
+ FULL_OBJECT_CHECKSUM_ARGS
200201
)

awscli/s3transfer/upload.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ class UploadSubmissionTask(SubmissionTask):
515515

516516
PUT_OBJECT_BLOCKLIST = ["ChecksumType", "MpuObjectSize"]
517517

518-
CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize"]
518+
CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize", "IfNoneMatch"]
519519

520520
UPLOAD_PART_ARGS = [
521521
'ChecksumAlgorithm',
@@ -534,6 +534,7 @@ class UploadSubmissionTask(SubmissionTask):
534534
'ExpectedBucketOwner',
535535
'ChecksumType',
536536
'MpuObjectSize',
537+
'IfNoneMatch',
537538
] + FULL_OBJECT_CHECKSUM_ARGS
538539

539540
def _get_upload_input_manager_cls(self, transfer_future):

tests/functional/s3/test_cp_command.py

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,98 @@ def test_operations_used_in_recursive_download(self):
295295
len(self.operations_called), 1, self.operations_called
296296
)
297297
self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
298+
299+
def test_for_no_overwrite_flag_when_object_not_exists_on_target(self):
300+
"""Testing when object with different key is successsfully uploaded using no-overwrite"""
301+
full_path = self.files.create_file('foo.txt', 'mycontent')
302+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
303+
self.parsed_responses = [
304+
{'ETag': '"c8afdb36c52cf4727836669019e69222"'}
305+
]
306+
self.run_cmd(cmdline, expected_rc=0)
307+
# Verify putObject was called
308+
self.assertEqual(len(self.operations_called), 1)
309+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
310+
# Verify the IfNoneMatch condition was set in the request
311+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
312+
313+
def test_for_no_overwrite_flag_when_object_exists_on_target(self):
314+
"""Testing when object already exists using no-overwrite"""
315+
full_path = self.files.create_file('foo.txt', 'mycontent')
316+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
317+
# Set up the response to simulate a PreconditionFailed error
318+
self.http_response.status_code = 412
319+
self.parsed_responses = [
320+
{
321+
'Error': {
322+
'Code': 'PreconditionFailed',
323+
'Message': 'At least one of the pre-conditions you specified did not hold',
324+
'Condition': 'If-None-match'
325+
}
326+
}
327+
]
328+
self.run_cmd(cmdline, expected_rc=0)
329+
# Verify PutObject was attempted with IfNoneMatch
330+
self.assertEqual(len(self.operations_called), 1)
331+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
332+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
298333

334+
def test_for_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
335+
"""Testing multipart upload with no-overwrite flag when object doesn't exist"""
336+
# Create a large file that will trigger multipart upload
337+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
338+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
339+
340+
# Set up responses for multipart upload
341+
self.parsed_responses = [
342+
{'UploadId': 'foo'}, # CreateMultipartUpload response
343+
{'ETag': '"foo-1"'}, # UploadPart response
344+
{'ETag': '"foo-2"'}, # UploadPart response
345+
{} # CompleteMultipartUpload response
346+
]
347+
self.run_cmd(cmdline, expected_rc=0)
348+
# Verify all multipart operations were called
349+
self.assertEqual(len(self.operations_called), 4)
350+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
351+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
352+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
353+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
354+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
355+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
356+
357+
def test_for_no_overwrite_flag_multipart_upload_when_object_exists_on_target(self):
358+
"""Testing multipart upload with no-overwrite flag when object already exist"""
359+
# Create a large file that will trigger multipart upload
360+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
361+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
362+
# Set up responses for multipart upload
363+
self.parsed_responses = [
364+
{'UploadId': 'foo'}, # CreateMultipartUpload response
365+
{'ETag': '"foo-1"'}, # UploadPart response
366+
{'ETag': '"foo-2"'}, # UploadPart response
367+
{
368+
'Error': {
369+
'Code': 'PreconditionFailed',
370+
'Message': 'At least one of the pre-conditions you specified did not hold',
371+
'Condition': 'If-None-match'
372+
}
373+
}, # PreconditionFailed error for CompleteMultipart Upload
374+
{} # AbortMultipartUpload response
375+
]
376+
# Checking for success as file is skipped
377+
self.run_cmd(cmdline, expected_rc=0)
378+
# Set up the response to simulate a PreconditionFailed error
379+
self.http_response.status_code = 412
380+
# Verify all multipart operations were called
381+
self.assertEqual(len(self.operations_called), 5)
382+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
383+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
384+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
385+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
386+
self.assertEqual(self.operations_called[4][0].name, 'AbortMultipartUpload')
387+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
388+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
389+
299390
def test_dryrun_download(self):
300391
self.parsed_responses = [self.head_object_response()]
301392
target = self.files.full_path('file.txt')

tests/functional/s3/test_mv_command.py

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,67 @@ def test_download_with_checksum_mode_crc32(self):
315315
self.assertEqual(
316316
self.operations_called[1][1]['ChecksumMode'], 'ENABLED'
317317
)
318-
318+
319+
def test_mv_no_overwrite_flag_when_object_not_exists_on_target(self):
320+
"""Testing when object doesnt exist using no-overwrite"""
321+
full_path = self.files.create_file('foo.txt', 'contents')
322+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
323+
self.run_cmd(cmdline, expected_rc=0)
324+
# Verify putObject was called
325+
self.assertEqual(len(self.operations_called),1)
326+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
327+
# Verify the IfNoneMatch condition was set in the request
328+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
329+
# Verify source file was deleted (move operation)
330+
self.assertFalse(os.path.exists(full_path))
331+
332+
def test_mv_no_overwrite_flag_when_object_exists_on_target(self):
333+
"""Testing when object already exists using no-overwrite"""
334+
full_path = self.files.create_file('foo.txt', 'mycontent')
335+
cmdline = f'{self.prefix} {full_path} s3://bucket/foo.txt --no-overwrite'
336+
# Set up the response to simulate a PreconditionFailed error
337+
self.http_response.status_code = 412
338+
self.parsed_responses = [
339+
{
340+
'Error': {
341+
'Code': 'PreconditionFailed',
342+
'Message': 'At least one of the pre-conditions you specified did not hold',
343+
'Condition': 'If-None-match'
344+
}
345+
}
346+
]
347+
stdout, stderr, rc = self.run_cmd(cmdline, expected_rc=0)
348+
# Verify PutObject was attempted with IfNoneMatch
349+
self.assertEqual(len(self.operations_called), 1)
350+
self.assertEqual(self.operations_called[0][0].name, 'PutObject')
351+
self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
352+
# Verify source file was not deleted
353+
self.assertTrue(os.path.exists(full_path))
354+
355+
def test_mv_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
356+
"""Testing multipart upload with no-overwrite flag when object doesn't exist"""
357+
# Create a large file that will trigger multipart upload
358+
full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
359+
cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
360+
361+
# Set up responses for multipart upload
362+
self.parsed_responses = [
363+
{'UploadId': 'foo'}, # CreateMultipartUpload response
364+
{'ETag': '"foo-1"'}, # UploadPart response
365+
{'ETag': '"foo-2"'}, # UploadPart response
366+
{} # CompleteMultipartUpload response
367+
]
368+
self.run_cmd(cmdline, expected_rc=0)
369+
# Verify all multipart operations were called
370+
self.assertEqual(len(self.operations_called), 4)
371+
self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
372+
self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
373+
self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
374+
self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
375+
# Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
376+
self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
377+
# Verify source file was deleted (successful move operation)
378+
self.assertFalse(os.path.exists(full_path))
319379

320380
class TestMvWithCRTClient(BaseCRTTransferClientTest):
321381
def test_upload_move_using_crt_client(self):

tests/functional/s3transfer/test_upload.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,6 +367,44 @@ def test_raise_exception_on_s3_object_lambda_resource(self):
367367
with self.assertRaisesRegex(ValueError, 'methods do not support'):
368368
self.manager.upload(self.filename, s3_object_lambda_arn, self.key)
369369

370+
def test_upload_with_no_overwrite_flag_when_object_exists(self):
371+
"""Test that upload fails when object exists at the destination"""
372+
self.extra_args['IfNoneMatch'] = '*'
373+
# Mocking a precondition Error
374+
self.stubber.add_client_error(
375+
'put_object',
376+
http_status_code=412,
377+
service_error_code='PreconditionFailed',
378+
expected_params={
379+
'Body': ANY,
380+
'Bucket': self.bucket,
381+
'Key': self.key,
382+
'IfNoneMatch': '*',
383+
'ChecksumAlgorithm': DEFAULT_CHECKSUM_ALGORITHM
384+
}
385+
)
386+
with self.assertRaises(ClientError) as context:
387+
future = self.manager.upload(
388+
self.filename, self.bucket, self.key, self.extra_args
389+
)
390+
future.result()
391+
# Verify the error is a PreconditionFailed error
392+
self.assertEqual(context.exception.response['Error']['Code'], 'PreconditionFailed')
393+
self.stubber.assert_no_pending_responses()
394+
395+
def test_upload_with_no_overwrite_flag_when_object_does_not_exists(self):
396+
"""Test that upload succeed when object does not exists at the destination"""
397+
self.extra_args['IfNoneMatch'] = '*'
398+
self.add_put_object_response_with_default_expected_params(
399+
extra_expected_params={'IfNoneMatch': '*'}
400+
)
401+
future = self.manager.upload(
402+
self.filename, self.bucket, self.key, self.extra_args
403+
)
404+
future.result()
405+
self.assert_expected_client_calls_were_correct()
406+
self.assert_put_object_body_was_correct()
407+
370408

371409
class TestMultipartUpload(BaseUploadTest):
372410
__test__ = True
@@ -848,3 +886,70 @@ def test_multipart_upload_with_default_checksum_when_required(self):
848886
)
849887
future.result()
850888
self.assert_expected_client_calls_were_correct()
889+
890+
def test_multipart_upload_with_no_overwrite_flag_when_object_exists(self):
891+
"""Test that multipart upload fails when object exists"""
892+
self.extra_args['IfNoneMatch'] = '*'
893+
# Add response for create_multipart_upload (no IfNoneMatch here)
894+
self.add_create_multipart_response_with_default_expected_params()
895+
# Add responses for upload_part
896+
self.add_upload_part_responses_with_default_expected_params()
897+
# Mock a PreconditionFailed error response when trying to complete the multipart upload
898+
self.stubber.add_client_error(
899+
method='complete_multipart_upload',
900+
service_error_code='PreconditionFailed',
901+
service_message='The condition specified in the conditional header(s) was not met',
902+
http_status_code=412,
903+
expected_params={
904+
'Bucket': self.bucket,
905+
'Key': self.key,
906+
'UploadId': self.multipart_id,
907+
'MultipartUpload': {
908+
'Parts': [
909+
{'ETag': 'etag-1', 'PartNumber': 1, 'ChecksumCRC64NVME': 'sum1=='},
910+
{'ETag': 'etag-2', 'PartNumber': 2, 'ChecksumCRC64NVME': 'sum2=='},
911+
{'ETag': 'etag-3', 'PartNumber': 3, 'ChecksumCRC64NVME': 'sum3=='}
912+
]
913+
},
914+
'IfNoneMatch': '*'
915+
}
916+
)
917+
# Add response for abort_multipart_upload that should be called after the error
918+
self.stubber.add_response(
919+
method='abort_multipart_upload',
920+
service_response={},
921+
expected_params={
922+
'Bucket': self.bucket,
923+
'Key': self.key,
924+
'UploadId': self.multipart_id,
925+
},
926+
)
927+
# Execute the upload and verify it raises the expected exception
928+
with self.assertRaises(ClientError) as context:
929+
future = self.manager.upload(
930+
self.filename, self.bucket, self.key, self.extra_args
931+
)
932+
future.result()
933+
# Verify the error is a PreconditionFailed error
934+
self.assertEqual(context.exception.response['Error']['Code'], 'PreconditionFailed')
935+
self.stubber.assert_no_pending_responses()
936+
937+
def test_multipart_upload_with_no_overwrite_flag_when_object_does_not_exist(self):
938+
"""Test that multipart upload succeeds when object doesn't exist"""
939+
self.extra_args['IfNoneMatch'] = '*'
940+
# Add response for create_multipart_upload (no IfNoneMatch here)
941+
self.add_create_multipart_response_with_default_expected_params()
942+
# Add responses for upload_part
943+
self.add_upload_part_responses_with_default_expected_params()
944+
# Add response for complete_multipart_upload with IfNoneMatch
945+
self.add_complete_multipart_response_with_default_expected_params(
946+
extra_expected_params={'IfNoneMatch': '*'}
947+
)
948+
# Execute the upload
949+
future = self.manager.upload(
950+
self.filename, self.bucket, self.key, self.extra_args
951+
)
952+
future.result()
953+
# Verify the upload was successful
954+
self.assert_expected_client_calls_were_correct()
955+
self.assert_upload_part_bodies_were_correct()

0 commit comments

Comments
 (0)