diff --git a/awscli/customizations/configservice/subscribe.py b/awscli/customizations/configservice/subscribe.py index 9075ade6e49b..2ee74e809269 100644 --- a/awscli/customizations/configservice/subscribe.py +++ b/awscli/customizations/configservice/subscribe.py @@ -13,6 +13,8 @@ import json import sys +from botocore.exceptions import ClientError + from awscli.customizations.commands import BasicCommand from awscli.customizations.s3.utils import find_bucket_key @@ -138,16 +140,25 @@ def prepare_bucket(self, s3_path): bucket_exists = self._check_bucket_exists(bucket) if not bucket_exists: self._create_bucket(bucket) + sys.stdout.write('Using new S3 bucket: %s\n' % bucket) + else: + sys.stdout.write('Using existing S3 bucket: %s\n' % bucket) return bucket, key def _check_bucket_exists(self, bucket): bucket_exists = True + self._s3_client.meta.events.unregister( + 'after-call', + unique_id='awscli-error-handler') try: # See if the bucket exists by running a head bucket self._s3_client.head_bucket(Bucket=bucket) - except Exception: - # If a client error is thrown than the bucket does not exist. - bucket_exists = False + except ClientError as e: + # If a client error is thrown. Check that it was a 404 error. + # If it was a 404 error, than the bucket does not exist. + error_code = int(e.response['Error']['Code']) + if error_code == 404: + bucket_exists = False return bucket_exists def _create_bucket(self, bucket): @@ -171,6 +182,9 @@ def prepare_topic(self, sns_topic): if not self._check_is_arn(sns_topic): response = self._sns_client.create_topic(Name=sns_topic) sns_topic_arn = response['TopicArn'] + sys.stdout.write('Using new SNS topic: %s\n' % sns_topic_arn) + else: + sys.stdout.write('Using existing SNS topic: %s\n' % sns_topic_arn) return sns_topic_arn def _check_is_arn(self, sns_topic): diff --git a/tests/unit/customizations/configservice/test_subscribe.py b/tests/unit/customizations/configservice/test_subscribe.py index 98fab14fc893..da7d4bf4e62b 100644 --- a/tests/unit/customizations/configservice/test_subscribe.py +++ b/tests/unit/customizations/configservice/test_subscribe.py @@ -11,8 +11,10 @@ # ANY KIND, either express or implied. See the License for the specific # language governing permissions and limitations under the License. import mock +from botocore.exceptions import ClientError from awscli.testutils import unittest +from awscli.compat import StringIO from awscli.customizations.configservice.subscribe import SubscribeCommand, \ S3BucketHelper, SNSTopicHelper @@ -21,6 +23,16 @@ class TestS3BucketHelper(unittest.TestCase): def setUp(self): self.s3_client = mock.Mock() self.helper = S3BucketHelper(self.s3_client) + self.error_response = { + 'Error': { + 'Code': '404', + 'Message': 'Not Found' + } + } + self.bucket_no_exists_error = ClientError( + self.error_response, + 'HeadBucket' + ) def test_correct_prefix_returned(self): name = 'MyBucket/MyPrefix' @@ -40,7 +52,7 @@ def test_bucket_exists(self): def test_bucket_no_exist(self): name = 'MyBucket/MyPrefix' - self.s3_client.head_bucket.side_effect = Exception() + self.s3_client.head_bucket.side_effect = self.bucket_no_exists_error self.s3_client._endpoint.region_name = 'us-east-1' bucket, prefix = self.helper.prepare_bucket(name) # Ensure that the create bucket was called with the proper args. @@ -53,7 +65,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 = Exception() + self.s3_client.head_bucket.side_effect = self.bucket_no_exists_error self.s3_client._endpoint.region_name = 'us-west-2' bucket, prefix = self.helper.prepare_bucket(name) # Ensure that the create bucket was called with the proper args. @@ -65,6 +77,39 @@ def test_bucket_no_exist_with_location_constraint(self): self.assertEqual(bucket, 'MyBucket') self.assertEqual(prefix, 'MyPrefix') + def test_bucket_client_exception_non_404(self): + name = 'MyBucket/MyPrefix' + self.error_response['Error']['Code'] = '403' + self.error_response['Error']['Message'] = 'Forbidden' + forbidden_error = ClientError(self.error_response, 'HeadBucket') + self.s3_client.head_bucket.side_effect = forbidden_error + self.s3_client._endpoint.region_name = 'us-east-1' + bucket, prefix = self.helper.prepare_bucket(name) + # A new bucket should not have been created because a 404 error + # was not thrown + self.assertFalse(self.s3_client.create_bucket.called) + # Ensure the returned bucket and key are as expected + self.assertEqual(bucket, 'MyBucket') + self.assertEqual(prefix, 'MyPrefix') + + def test_output_use_existing_bucket(self): + name = 'MyBucket/MyPrefix' + with mock.patch('sys.stdout', StringIO()) as mock_stdout: + self.helper.prepare_bucket(name) + self.assertIn( + 'Using existing S3 bucket: MyBucket', + mock_stdout.getvalue()) + + def test_output_create_bucket(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' + with mock.patch('sys.stdout', StringIO()) as mock_stdout: + self.helper.prepare_bucket(name) + self.assertIn( + 'Using new S3 bucket: MyBucket', + mock_stdout.getvalue()) + class TestSNSTopicHelper(unittest.TestCase): def setUp(self): @@ -86,6 +131,23 @@ def test_sns_topic_by_arn(self): self.assertFalse(self.sns_client.create_topic.called) self.assertEqual(sns_arn, name) + def test_output_existing_topic(self): + name = 'mysnstopic' + self.sns_client.create_topic.return_value = {'TopicArn': 'myARN'} + with mock.patch('sys.stdout', StringIO()) as mock_stdout: + self.helper.prepare_topic(name) + self.assertIn( + 'Using new SNS topic: myARN', + mock_stdout.getvalue()) + + def test_output_new_topic(self): + name = 'arn:aws:sns:us-east-1:934212987125:config' + with mock.patch('sys.stdout', StringIO()) as mock_stdout: + self.helper.prepare_topic(name) + self.assertIn( + 'Using existing SNS topic: %s' % name, + mock_stdout.getvalue()) + class TestSubscribeCommand(unittest.TestCase): def setUp(self):