Merge pull request #1593 from kyleknap/subscribe-fix

Fix regression from client switch over

CHANGELOG.rst

@@ -13,6 +13,9 @@ Next Release (TBD)
   respectively.  The old arguments are still supported for backwards
   compatibility, but are no longer documented.
   (`issue 1599 <https://github.com/aws/aws-cli/pull/1599>`__)
+* bugfix:``aws configservice subscribe``: Fix an issue when creating a
+  new S3 bucket
+  (`issue 1593 <https://github.com/aws/aws-cli/pull/1593>`__)
 
 
 1.9.1

awscli/customizations/configservice/subscribe.py

@@ -151,7 +151,7 @@ def _check_bucket_exists(self, bucket):
         return s3_bucket_exists(self._s3_client, bucket)
 
     def _create_bucket(self, bucket):
-        region_name = self._s3_client._endpoint.region_name
+        region_name = self._s3_client.meta.region_name
         params = {
             'Bucket': bucket
         }

awscli/testutils.py

@@ -301,6 +301,7 @@ def tearDown(self):
         self.environ_patch.stop()
         if self.make_request_is_patched:
             self.make_request_patch.stop()
+            self.make_request_is_patched = False
 
     def before_call(self, params, **kwargs):
         self._store_params(params)
@@ -310,6 +311,13 @@ def _store_params(self, params):
         self.last_params = params['body']
 
     def patch_make_request(self):
+        # If you do not stop a previously started patch,
+        # it can never be stopped if you call start() again on the same
+        # patch again...
+        # So stop the current patch before calling start() on it again.
+        if self.make_request_is_patched:
+            self.make_request_patch.stop()
+            self.make_request_is_patched = False
         make_request_patch = self.make_request_patch.start()
         if self.parsed_responses is not None:
             make_request_patch.side_effect = lambda *args, **kwargs: \

tests/functional/configservice/test_subscribe.py

@@ -0,0 +1,139 @@
+# Copyright 2015 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.
+import mock
+
+from awscli.testutils import BaseAWSCommandParamsTest
+from awscli.customizations.configservice.subscribe import S3BucketHelper
+
+
+class TestSubscribe(BaseAWSCommandParamsTest):
+    prefix = 'configservice subscribe'
+
+    def setUp(self):
+        super(TestSubscribe, self).setUp()
+        self.parsed_responses = [
+            {},  # S3 HeadBucket
+            {'TopicArn': 'my-topic-arn'},  # SNS CreateTopic
+            {},  # PutConfigurationRecorder
+            {},  # PutDeliveryChannel
+            {},  # StartConfigurationRecorder
+            {'ConfigurationRecorders': {}},  # DescribeConfigurationRecorders
+            {'DeliveryChannels': {}}  # DescribeDeliveryChannels
+        ]
+
+    def test_subscribe_when_bucket_exists_and_new_sns_topic(self):
+        self.prefix += ' --s3-bucket mybucket --sns-topic mytopic'
+        self.prefix += ' --iam-role myrole'
+        self.run_cmd(self.prefix)
+
+        self.assertEqual(len(self.operations_called), 7)
+        list_of_operation_names_called = []
+        list_of_parameters_called = []
+        for operation_called in self.operations_called:
+            list_of_operation_names_called.append(operation_called[0].name)
+            list_of_parameters_called.append(operation_called[1])
+
+        self.assertEqual(
+            list_of_operation_names_called, [
+                'HeadBucket',
+                'CreateTopic',
+                'PutConfigurationRecorder',
+                'PutDeliveryChannel',
+                'StartConfigurationRecorder',
+                'DescribeConfigurationRecorders',
+                'DescribeDeliveryChannels'
+            ]
+        )
+        self.assertEqual(
+            list_of_parameters_called, [
+                {'Bucket': 'mybucket'},  # S3 HeadBucket
+                {'Name': 'mytopic'},  # SNS CreateTopic
+                {'ConfigurationRecorder': {  # PutConfigurationRecorder
+                    'name': 'default', 'roleARN': 'myrole'}},
+                {'DeliveryChannel': {  # PutDeliveryChannel
+                    'name': 'default',
+                    's3BucketName': 'mybucket',
+                    'snsTopicARN': 'my-topic-arn'}},
+                # StartConfigurationRecorder
+                {'ConfigurationRecorderName': 'default'},
+                {},  # DescribeConfigurationRecorders
+                {}  # DescribeDeliveryChannels
+            ]
+        )
+
+    def test_subscribe_when_bucket_exists_and_sns_topic_arn_provided(self):
+        self.parsed_responses.pop(1)
+        self.prefix += ' --s3-bucket mybucket --sns-topic arn:mytopic'
+        self.prefix += ' --iam-role myrole'
+        self.run_cmd(self.prefix)
+
+        self.assertEqual(len(self.operations_called), 6)
+        list_of_operation_names_called = []
+        list_of_parameters_called = []
+        for operation_called in self.operations_called:
+            list_of_operation_names_called.append(operation_called[0].name)
+            list_of_parameters_called.append(operation_called[1])
+
+        self.assertEqual(
+            list_of_operation_names_called, [
+                'HeadBucket',
+                'PutConfigurationRecorder',
+                'PutDeliveryChannel',
+                'StartConfigurationRecorder',
+                'DescribeConfigurationRecorders',
+                'DescribeDeliveryChannels'
+            ]
+        )
+        self.assertEqual(
+            list_of_parameters_called, [
+                {'Bucket': 'mybucket'},  # S3 HeadBucket
+                {'ConfigurationRecorder': {  # PutConfigurationRecorder
+                    'name': 'default', 'roleARN': 'myrole'}},
+                {'DeliveryChannel': {  # PutDeliveryChannel
+                    'name': 'default',
+                    's3BucketName': 'mybucket',
+                    'snsTopicARN': 'arn:mytopic'}},
+                # StartConfigurationRecorder
+                {'ConfigurationRecorderName': 'default'},
+                {},  # DescribeConfigurationRecorders
+                {}  # DescribeDeliveryChannels
+            ]
+        )
+
+    def test_subscribe_when_bucket_needs_to_be_created(self):
+        with mock.patch('botocore.endpoint.Session.send') as \
+                http_session_send_patch:
+            # Mock for HeadBucket request
+            head_bucket_response = mock.Mock()
+            head_bucket_response.status_code = 404
+            head_bucket_response.content = b''
+            head_bucket_response.headers = {}
+
+            # Mock for CreateBucket request
+            create_bucket_response = mock.Mock()
+            create_bucket_response.status_code = 200
+            create_bucket_response.content = b''
+            create_bucket_response.headers = {}
+
+            http_session_send_patch.side_effect = [
+                head_bucket_response, create_bucket_response
+            ]
+
+            s3_client = self.driver.session.create_client('s3')
+            bucket_helper = S3BucketHelper(s3_client)
+            bucket_helper.prepare_bucket('mybucket')
+            send_call_list = http_session_send_patch.call_args_list
+            self.assertEqual(send_call_list[0][0][0].method, 'HEAD')
+            # Since the HeadObject fails with 404, the CreateBucket which is
+            # is a PUT request should be made.
+            self.assertEqual(send_call_list[1][0][0].method, 'PUT')

tests/unit/customizations/configservice/test_subscribe.py

@@ -11,6 +11,7 @@
 # ANY KIND, either express or implied. See the License for the specific
 # language governing permissions and limitations under the License.
 import mock
+import botocore.session
 from botocore.exceptions import ClientError
 
 from awscli.testutils import unittest
@@ -21,7 +22,8 @@
 
 class TestS3BucketHelper(unittest.TestCase):
     def setUp(self):
-        self.s3_client = mock.Mock()
+        self.session = botocore.session.get_session()
+        self.s3_client = mock.Mock(self.session.create_client('s3'))
         self.helper = S3BucketHelper(self.s3_client)
         self.error_response = {
             'Error': {
@@ -53,7 +55,7 @@ def test_bucket_exists(self):
     def test_bucket_no_exist(self):
         name = 'MyBucket/MyPrefix'
         self.s3_client.head_bucket.side_effect = self.bucket_no_exists_error
-        self.s3_client._endpoint.region_name = 'us-east-1'
+        self.s3_client.meta.region_name = 'us-east-1'
         bucket, prefix = self.helper.prepare_bucket(name)
         # Ensure that the create bucket was called with the proper args.
         self.s3_client.create_bucket.assert_called_with(
@@ -66,7 +68,7 @@ def test_bucket_no_exist(self):
     def test_bucket_no_exist_with_location_constraint(self):
         name = 'MyBucket/MyPrefix'
         self.s3_client.head_bucket.side_effect = self.bucket_no_exists_error
-        self.s3_client._endpoint.region_name = 'us-west-2'
+        self.s3_client.meta.region_name = 'us-west-2'
         bucket, prefix = self.helper.prepare_bucket(name)
         # Ensure that the create bucket was called with the proper args.
         self.s3_client.create_bucket.assert_called_with(
@@ -113,7 +115,9 @@ def test_output_create_bucket(self):
 
 class TestSNSTopicHelper(unittest.TestCase):
     def setUp(self):
-        self.sns_client = mock.Mock()
+        self.session = botocore.session.get_session()
+        self.sns_client = mock.Mock(self.session.create_client(
+            'sns', 'us-east-1'))
         self.helper = SNSTopicHelper(self.sns_client)
 
     def test_sns_topic_by_name(self):
@@ -151,17 +155,20 @@ def test_output_new_topic(self):
 
 class TestSubscribeCommand(unittest.TestCase):
     def setUp(self):
-        self.session = mock.Mock()
+        self.session = botocore.session.get_session()
 
         # Set up the client mocks.
-        self.s3_client = mock.Mock()
-        self.sns_client = mock.Mock()
-        self.config_client = mock.Mock()
+        self.s3_client = mock.Mock(self.session.create_client('s3'))
+        self.sns_client = mock.Mock(self.session.create_client(
+            'sns', 'us-east-1'))
+        self.config_client = mock.Mock(self.session.create_client(
+            'config', 'us-east-1'))
         self.config_client.describe_configuration_recorders.return_value = \
             {'ConfigurationRecorders': []}
         self.config_client.describe_delivery_channels.return_value = \
             {'DeliveryChannels': []}
 
+        self.session = mock.Mock(self.session)
         self.session.create_client.side_effect = [
             self.s3_client,
             self.sns_client,

tests/unit/output/test_text_output.py

@@ -55,13 +55,10 @@ def setUp(self):
             'Groups': []
         }
 
-    def patch_make_request(self):
-        make_request_patch = self.make_request_patch.start()
-        make_request_patch.side_effect = [
-            (self.http_response, self.first_parsed_response),
-            (self.http_response, self.second_parsed_response),
+        self.parsed_responses = [
+            self.first_parsed_response,
+            self.second_parsed_response
         ]
-        self.make_request_is_patched = True
 
     def test_text_response(self):
         output = self.run_cmd('iam list-users --output text', expected_rc=0)[0]