Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix regression from client switch over #1593

Merged
merged 7 commits into from
Oct 29, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion awscli/customizations/configservice/subscribe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
8 changes: 8 additions & 0 deletions awscli/testutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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: \
Expand Down
139 changes: 139 additions & 0 deletions tests/functional/configservice/test_subscribe.py
Original file line number Diff line number Diff line change
@@ -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')
23 changes: 15 additions & 8 deletions tests/unit/customizations/configservice/test_subscribe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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': {
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 3 additions & 6 deletions tests/unit/output/test_text_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down