Merge pull request aws#2667 from kyleknap/rm-no-head-obj

Remove unnecessary HeadObject for single object rm

.changes/next-release/bugfix-s3rm-54186.json

@@ -0,0 +1,5 @@
+{
+  "category": "``s3 rm``", 
+  "type": "bugfix", 
+  "description": "Remove unnecessary HeadObject call for non-recursive ``s3 rm``. This caused errors when a remote S3 object was encrypted with SSE-C as HeadObject call requires the SSE-C key but DeleteObject does not (`#2494 <https://github.com/aws/aws-cli/issues/2494>`__)"
+}

awscli/customizations/s3/filegenerator.py

@@ -340,6 +340,12 @@ def _list_single_object(self, s3_path):
         # a ListObjects operation (which causes concern for anyone setting
         # IAM policies with the smallest set of permissions needed) and
         # instead use a HeadObject request.
+        if self.operation_name == 'delete':
+            # If the operation is just a single remote delete, there is
+            # no need to run HeadObject on the S3 object as none of the
+            # information gained from HeadObject is required to delete the
+            # object.
+            return s3_path, {'Size': None, 'LastModified': None}
         bucket, key = find_bucket_key(s3_path)
         try:
             params = {'Bucket': bucket, 'Key': key}

tests/functional/s3/test_rm_command.py

@@ -0,0 +1,34 @@
+# Copyright 2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file 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 awscli.testutils import BaseAWSCommandParamsTest, FileCreator
+
+
+class TestRmCommand(BaseAWSCommandParamsTest):
+
+    prefix = 's3 rm'
+
+    def setUp(self):
+        super(TestRmCommand, self).setUp()
+        self.files = FileCreator()
+
+    def tearDown(self):
+        super(TestRmCommand, self).tearDown()
+        self.files.remove_all()
+
+    def test_operations_used_in_upload(self):
+        cmdline = '%s s3://bucket/key.txt' % self.prefix
+        self.run_cmd(cmdline, expected_rc=0)
+        # The only operation we should have called is DeleteObject.
+        self.assertEqual(
+            len(self.operations_called), 1, self.operations_called)
+        self.assertEqual(self.operations_called[0][0].name, 'DeleteObject')

tests/integration/customizations/s3/test_plugin.py

@@ -2039,6 +2039,20 @@ def test_sse_c_upload_and_download(self):
         self.download_and_assert_sse_c_object_integrity(
             self.bucket, key, self.encrypt_key, contents)
 
+    def test_can_delete_single_sse_c_object(self):
+        key = 'foo.txt'
+        contents = 'contents'
+        self.put_object(
+            self.bucket, key, contents,
+            extra_args={
+                'SSECustomerKey': self.encrypt_key,
+                'SSECustomerAlgorithm': 'AES256'
+            }
+        )
+        p = aws('s3 rm s3://%s/%s' % (self.bucket, key))
+        self.assert_no_errors(p)
+        self.assertFalse(self.key_exists(self.bucket, key))
+
     def test_sse_c_upload_and_download_large_file(self):
         key = 'foo.txt'
         contents = 'a' * (10 * (1024 * 1024))

tests/unit/customizations/s3/test_filegenerator.py

@@ -534,6 +534,25 @@ def test_s3_single_file_404(self):
         with self.assertRaisesRegexp(ClientError, '404.*text1.txt'):
             list(files)
 
+    def test_s3_single_file_delete(self):
+        input_s3_file = {'src': {'path': self.file1, 'type': 's3'},
+                         'dest': {'path': '', 'type': 'local'},
+                         'dir_op': False, 'use_src_name': True}
+        self.client = mock.Mock()
+        file_gen = FileGenerator(self.client, 'delete')
+        result_list = list(file_gen.call(input_s3_file))
+        self.assertEqual(len(result_list), 1)
+        compare_files(
+            self,
+            result_list[0],
+            FileStat(src=self.file1, dest='text1.txt',
+                     compare_key='text1.txt',
+                     size=None, last_update=None,
+                     src_type='s3', dest_type='local',
+                     operation_name='delete')
+        )
+        self.client.head_object.assert_not_called()
+
     def test_s3_directory(self):
         """
         Generates s3 files under a common prefix. Also it ensures that