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

Author: Rakshil Modi

awscli/customizations/s3/results.py

@@ -23,6 +23,7 @@
 from awscli.customizations.s3.subscribers import OnDoneFilteredSubscriber
 from awscli.customizations.s3.utils import WarningResult, human_readable_size
 from awscli.customizations.utils import uni_print
+from awscli.customizations.s3.utils import create_warning
 
 LOGGER = logging.getLogger(__name__)
 
@@ -123,7 +124,10 @@ def _on_failure(self, future, e):
             if isinstance(e, FatalError):
                 error_result_cls = ErrorResult
             self._result_queue.put(error_result_cls(exception=e))
-        else:
+        elif self._is_precondition_failed(e):
+                error_message = f"Warning: Skipping file {self._src} as it already exists on {self._dest}"
+                LOGGER.debug(error_message)
+        else :
             self._result_queue.put(
                 FailureResult(
                     transfer_type=self._transfer_type,
@@ -132,6 +136,11 @@ def _on_failure(self, future, e):
                     exception=e,
                 )
             )
+    
+    def _is_precondition_failed(self, exception):
+        """Check if this is a PreconditionFailed error"""
+        return (hasattr(exception, 'response') and 
+            exception.response.get('Error', {}).get('Code') == 'PreconditionFailed')
 
 
 class BaseResultHandler:

awscli/customizations/s3/subcommands.py

@@ -642,6 +642,15 @@
     ),
 }
 
+NO_OVERWRITE = {
+    'name': 'no-overwrite',
+    'action': 'store_true',
+    'help_text': (
+        "This flag prevents overwriting of files at the destination. With this flag, "
+        "only the files not present at the destination will be transferred"
+    ),
+}
+
 TRANSFER_ARGS = [
     DRYRUN,
     QUIET,
@@ -1057,7 +1066,7 @@ class CpCommand(S3TransferCommand):
             }
         ]
         + TRANSFER_ARGS
-        + [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE]
+        + [METADATA, COPY_PROPS, METADATA_DIRECTIVE, EXPECTED_SIZE, RECURSIVE, NO_OVERWRITE]
     )
 
 
@@ -1081,6 +1090,7 @@ class MvCommand(S3TransferCommand):
             METADATA_DIRECTIVE,
             RECURSIVE,
             VALIDATE_SAME_S3_PATHS,
+            NO_OVERWRITE,
         ]
     )
 

awscli/customizations/s3/utils.py

@@ -489,6 +489,7 @@ def map_put_object_params(cls, request_params, cli_params):
         cls._set_sse_c_request_params(request_params, cli_params)
         cls._set_request_payer_param(request_params, cli_params)
         cls._set_checksum_algorithm_param(request_params, cli_params)
+        cls._set_no_overwrite_param(request_params, cli_params)
 
     @classmethod
     def map_get_object_params(cls, request_params, cli_params):
@@ -557,6 +558,12 @@ def map_delete_object_params(cls, request_params, cli_params):
     @classmethod
     def map_list_objects_v2_params(cls, request_params, cli_params):
         cls._set_request_payer_param(request_params, cli_params)
+    
+    @classmethod
+    def _set_no_overwrite_param(cls, request_params, cli_params):
+        """Map No overwrite header with IfNoneMatch"""
+        if cli_params.get('no_overwrite'):
+            request_params['IfNoneMatch'] = "*"
 
     @classmethod
     def _set_request_payer_param(cls, request_params, cli_params):

awscli/s3transfer/manager.py

@@ -195,6 +195,7 @@ class TransferManager:
         + [
             'ChecksumType',
             'MpuObjectSize',
+            'IfNoneMatch',
         ]
         + FULL_OBJECT_CHECKSUM_ARGS
     )

awscli/s3transfer/upload.py

@@ -515,7 +515,7 @@ class UploadSubmissionTask(SubmissionTask):
 
     PUT_OBJECT_BLOCKLIST = ["ChecksumType", "MpuObjectSize"]
 
-    CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize"]
+    CREATE_MULTIPART_BLOCKLIST = FULL_OBJECT_CHECKSUM_ARGS + ["MpuObjectSize", "IfNoneMatch"]
 
     UPLOAD_PART_ARGS = [
         'ChecksumAlgorithm',
@@ -534,6 +534,7 @@ class UploadSubmissionTask(SubmissionTask):
         'ExpectedBucketOwner',
         'ChecksumType',
         'MpuObjectSize',
+        'IfNoneMatch',
     ] + FULL_OBJECT_CHECKSUM_ARGS
 
     def _get_upload_input_manager_cls(self, transfer_future):

tests/functional/s3/test_cp_command.py

@@ -295,7 +295,99 @@ def test_operations_used_in_recursive_download(self):
             len(self.operations_called), 1, self.operations_called
         )
         self.assertEqual(self.operations_called[0][0].name, 'ListObjectsV2')
+    
+    def test_for_no_overwrite_flag_when_object_not_exists_on_target(self):
+        """Testing when object with different key is successsfully uploaded using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'mycontent')
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+        self.parsed_responses = [
+            {'ETag': '"c8afdb36c52cf4727836669019e69222"'}
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        # Verify putObject was called
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        # Verify the IfNoneMatch condition was set in the request
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+    
+    def test_for_no_overwrite_flag_when_object_exists_on_target(self):
+        """Testing when object already exists using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'mycontent')
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+        # Set up the response to simulate a PreconditionFailed error
+        self.http_response.status_code = 412
+        self.parsed_responses = [
+            {
+                'Error': {
+                    'Code': 'PreconditionFailed',
+                    'Message': 'At least one of the pre-conditions you specified did not hold',
+                    'Condition': 'If-None-match'
+                }
+            }
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        # Verify PutObject was attempted with IfNoneMatch
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
 
+    def test_for_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
+        """Testing multipart upload with no-overwrite flag when object doesn't exist"""
+        # Create a large file that will trigger multipart upload
+        full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+    
+        # Set up responses for multipart upload
+        self.parsed_responses = [
+            {'UploadId': 'foo'},  # CreateMultipartUpload response
+            {'ETag': '"foo-1"'},  # UploadPart response
+            {'ETag': '"foo-2"'},  # UploadPart response
+            {}                    # CompleteMultipartUpload response
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        # Verify all multipart operations were called
+        self.assertEqual(len(self.operations_called), 4)
+        self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
+        self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
+        # Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
+        self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
+
+    def test_for_no_overwrite_flag_multipart_upload_when_object_exists_on_target(self):
+        """Testing multipart upload with no-overwrite flag when object already exist"""
+        # Create a large file that will trigger multipart upload
+        full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+        # Set up responses for multipart upload
+        self.parsed_responses = [
+            {'UploadId': 'foo'},  # CreateMultipartUpload response
+            {'ETag': '"foo-1"'},  # UploadPart response
+            {'ETag': '"foo-2"'},  # UploadPart response
+            {
+                'Error': {
+                    'Code': 'PreconditionFailed',
+                    'Message': 'At least one of the pre-conditions you specified did not hold',
+                    'Condition': 'If-None-match'
+                }
+            },  # PreconditionFailed error for CompleteMultipart Upload
+            {}  # AbortMultipartUpload response                
+        ]
+        # Checking for success as file is skipped
+        self.run_cmd(cmdline, expected_rc=0)
+        # Set up the response to simulate a PreconditionFailed error
+        self.http_response.status_code = 412
+        # Verify all multipart operations were called
+        self.assertEqual(len(self.operations_called), 5)
+        self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
+        self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
+        self.assertEqual(self.operations_called[4][0].name, 'AbortMultipartUpload')
+        # Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
+        self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
+    
+    
     def test_dryrun_download(self):
         self.parsed_responses = [self.head_object_response()]
         target = self.files.full_path('file.txt')

tests/functional/s3/test_mv_command.py

@@ -315,7 +315,67 @@ def test_download_with_checksum_mode_crc32(self):
         self.assertEqual(
             self.operations_called[1][1]['ChecksumMode'], 'ENABLED'
         )
-
+    
+    def test_mv_no_overwrite_flag_when_object_not_exists_on_target(self):
+        """Testing when object doesnt exist using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'contents')
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+        self.run_cmd(cmdline, expected_rc=0)
+        # Verify putObject was called
+        self.assertEqual(len(self.operations_called),1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        # Verify the IfNoneMatch condition was set in the request
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+        # Verify source file was deleted (move operation)
+        self.assertFalse(os.path.exists(full_path))
+    
+    def test_mv_no_overwrite_flag_when_object_exists_on_target(self):
+        """Testing when object already exists using no-overwrite"""
+        full_path = self.files.create_file('foo.txt', 'mycontent')
+        cmdline = f'{self.prefix} {full_path} s3://bucket/foo.txt --no-overwrite'
+        # Set up the response to simulate a PreconditionFailed error
+        self.http_response.status_code = 412
+        self.parsed_responses = [
+            {
+                'Error': {
+                    'Code': 'PreconditionFailed',
+                    'Message': 'At least one of the pre-conditions you specified did not hold',
+                    'Condition': 'If-None-match'
+                }
+            }
+        ]
+        stdout, stderr, rc = self.run_cmd(cmdline, expected_rc=0)
+        # Verify PutObject was attempted with IfNoneMatch
+        self.assertEqual(len(self.operations_called), 1)
+        self.assertEqual(self.operations_called[0][0].name, 'PutObject')
+        self.assertEqual(self.operations_called[0][1]['IfNoneMatch'], '*')
+        # Verify source file was not deleted
+        self.assertTrue(os.path.exists(full_path))
+
+    def test_mv_no_overwrite_flag_multipart_upload_when_object_not_exists_on_target(self):
+        """Testing multipart upload with no-overwrite flag when object doesn't exist"""
+        # Create a large file that will trigger multipart upload
+        full_path = self.files.create_file('foo.txt', 'a' * 10 * (1024**2))
+        cmdline = f'{self.prefix} {full_path} s3://bucket --no-overwrite'
+        
+        # Set up responses for multipart upload
+        self.parsed_responses = [
+            {'UploadId': 'foo'},  # CreateMultipartUpload response
+            {'ETag': '"foo-1"'},  # UploadPart response
+            {'ETag': '"foo-2"'},  # UploadPart response
+            {}                    # CompleteMultipartUpload response
+        ]
+        self.run_cmd(cmdline, expected_rc=0)
+        # Verify all multipart operations were called
+        self.assertEqual(len(self.operations_called), 4)
+        self.assertEqual(self.operations_called[0][0].name, 'CreateMultipartUpload')
+        self.assertEqual(self.operations_called[1][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[2][0].name, 'UploadPart')
+        self.assertEqual(self.operations_called[3][0].name, 'CompleteMultipartUpload')
+        # Verify the IfNoneMatch condition was set in the CompleteMultipartUpload request
+        self.assertEqual(self.operations_called[3][1]['IfNoneMatch'], '*')
+        # Verify source file was deleted (successful move operation)
+        self.assertFalse(os.path.exists(full_path))
 
 class TestMvWithCRTClient(BaseCRTTransferClientTest):
     def test_upload_move_using_crt_client(self):

tests/functional/s3transfer/test_upload.py

@@ -367,6 +367,45 @@ def test_raise_exception_on_s3_object_lambda_resource(self):
         with self.assertRaisesRegex(ValueError, 'methods do not support'):
             self.manager.upload(self.filename, s3_object_lambda_arn, self.key)
 
+    # Adding new tests
+    def test_upload_with_no_overwrite_flag_when_object_exists(self):
+        """Test that upload fails when object exists at the destination"""
+        self.extra_args['IfNoneMatch'] = '*'
+        # Mocking a precondition Error
+        self.stubber.add_client_error(
+            'put_object',
+            http_status_code=412,
+            service_error_code='PreconditionFailed',
+            expected_params={
+                'Body': ANY,
+                'Bucket': self.bucket,
+                'Key': self.key,
+                'IfNoneMatch': '*',
+                'ChecksumAlgorithm': DEFAULT_CHECKSUM_ALGORITHM
+            }
+        )
+        with self.assertRaises(ClientError) as context:
+            future = self.manager.upload(
+                self.filename, self.bucket, self.key, self.extra_args
+            )
+            future.result()
+        # Verify the error is a PreconditionFailed error
+        self.assertEqual(context.exception.response['Error']['Code'], 'PreconditionFailed')
+        self.stubber.assert_no_pending_responses()
+
+    def test_upload_with_no_overwrite_flag_when_object_does_not_exists(self):
+        """Test that upload succeed when object does not exists at the destination"""
+        self.extra_args['IfNoneMatch'] = '*'
+        self.add_put_object_response_with_default_expected_params(
+            extra_expected_params={'IfNoneMatch': '*'}
+        )
+        future = self.manager.upload(
+            self.filename, self.bucket, self.key, self.extra_args
+        )
+        future.result()
+        self.assert_expected_client_calls_were_correct()
+        self.assert_put_object_body_was_correct()
+
 
 class TestMultipartUpload(BaseUploadTest):
     __test__ = True
@@ -848,3 +887,71 @@ def test_multipart_upload_with_default_checksum_when_required(self):
         )
         future.result()
         self.assert_expected_client_calls_were_correct()
+
+    # Adding new tests
+    def test_multipart_upload_with_no_overwrite_flag_when_object_exists(self):
+        """Test that multipart upload fails when object exists"""
+        self.extra_args['IfNoneMatch'] = '*'
+        # Add response for create_multipart_upload (no IfNoneMatch here)
+        self.add_create_multipart_response_with_default_expected_params()
+        # Add responses for upload_part
+        self.add_upload_part_responses_with_default_expected_params()
+        # Mock a PreconditionFailed error response when trying to complete the multipart upload
+        self.stubber.add_client_error(
+            method='complete_multipart_upload',
+            service_error_code='PreconditionFailed',
+            service_message='The condition specified in the conditional header(s) was not met',
+            http_status_code=412,
+            expected_params={
+                'Bucket': self.bucket,
+                'Key': self.key,
+                'UploadId': self.multipart_id,
+                'MultipartUpload': {
+                    'Parts': [
+                        {'ETag': 'etag-1', 'PartNumber': 1, 'ChecksumCRC64NVME': 'sum1=='},
+                        {'ETag': 'etag-2', 'PartNumber': 2, 'ChecksumCRC64NVME': 'sum2=='},
+                        {'ETag': 'etag-3', 'PartNumber': 3, 'ChecksumCRC64NVME': 'sum3=='}
+                    ]
+                },
+                'IfNoneMatch': '*'
+            }
+        )
+        # Add response for abort_multipart_upload that should be called after the error
+        self.stubber.add_response(
+            method='abort_multipart_upload',
+            service_response={},
+            expected_params={
+                'Bucket': self.bucket,
+                'Key': self.key,
+                'UploadId': self.multipart_id,
+            },
+        )
+        # Execute the upload and verify it raises the expected exception
+        with self.assertRaises(ClientError) as context:
+            future = self.manager.upload(
+                self.filename, self.bucket, self.key, self.extra_args
+            )
+            future.result()
+        # Verify the error is a PreconditionFailed error
+        self.assertEqual(context.exception.response['Error']['Code'], 'PreconditionFailed')
+        self.stubber.assert_no_pending_responses()
+
+    def test_multipart_upload_with_no_overwrite_flag_when_object_does_not_exist(self):
+        """Test that multipart upload succeeds when object doesn't exist"""
+        self.extra_args['IfNoneMatch'] = '*'
+        # Add response for create_multipart_upload (no IfNoneMatch here)
+        self.add_create_multipart_response_with_default_expected_params()
+        # Add responses for upload_part
+        self.add_upload_part_responses_with_default_expected_params()
+        # Add response for complete_multipart_upload with IfNoneMatch
+        self.add_complete_multipart_response_with_default_expected_params(
+            extra_expected_params={'IfNoneMatch': '*'}
+        )
+        # Execute the upload
+        future = self.manager.upload(
+            self.filename, self.bucket, self.key, self.extra_args
+        )
+        future.result()
+        # Verify the upload was successful
+        self.assert_expected_client_calls_were_correct()
+        self.assert_upload_part_bodies_were_correct()