Fix issue with configservice subscribe command

When checking to see if a bucket exists, we assumed that an exception
meant that the bucket did not exist when using HeadBucket. In reality,
the error could be a 403. Also expanded the output to inform users
if a new bucket/topic is being used or an existing one.

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):

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):