diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c3b316693968..a030098203dc 100644 --- a/CHANGELOG.rst +++ b/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 `__) +* bugfix:``aws configservice subscribe``: Fix an issue when creating a + new S3 bucket + (`issue 1593 `__) 1.9.1 diff --git a/awscli/customizations/configservice/subscribe.py b/awscli/customizations/configservice/subscribe.py index c19b95e18011..16f2081fd18c 100644 --- a/awscli/customizations/configservice/subscribe.py +++ b/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 } diff --git a/awscli/testutils.py b/awscli/testutils.py index 13d684600ee1..bcf4c1dcb8b9 100644 --- a/awscli/testutils.py +++ b/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: \ diff --git a/tests/functional/configservice/test_subscribe.py b/tests/functional/configservice/test_subscribe.py new file mode 100644 index 000000000000..3c5188ef3c2c --- /dev/null +++ b/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') diff --git a/tests/unit/customizations/configservice/test_subscribe.py b/tests/unit/customizations/configservice/test_subscribe.py index da7d4bf4e62b..91e24154c11a 100644 --- a/tests/unit/customizations/configservice/test_subscribe.py +++ b/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, diff --git a/tests/unit/output/test_text_output.py b/tests/unit/output/test_text_output.py index 267e6778edba..d19a1f173098 100644 --- a/tests/unit/output/test_text_output.py +++ b/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]