diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 602c84a1160c..048c3878cdb8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -2,6 +2,13 @@ CHANGELOG ========= +Next Release (TBD) +================== + +* feature:``aws cloudtrail``: Add support for regionalized policy templates + for the ``create-subscription`` and ``update-subscription`` commands. + (`issue 1167 `__) + 1.7.12 ====== diff --git a/awscli/customizations/cloudtrail.py b/awscli/customizations/cloudtrail.py index f99489301e2f..0bc4c1d557ce 100644 --- a/awscli/customizations/cloudtrail.py +++ b/awscli/customizations/cloudtrail.py @@ -20,8 +20,12 @@ LOG = logging.getLogger(__name__) -S3_POLICY_TEMPLATE = 'policy/S3/AWSCloudTrail-S3BucketPolicy-2013-11-01.json' -SNS_POLICY_TEMPLATE = 'policy/SNS/AWSCloudTrail-SnsTopicPolicy-2013-11-01.json' +S3_POLICY_TEMPLATE = 'policy/S3/AWSCloudTrail-S3BucketPolicy-2014-12-17.json' +SNS_POLICY_TEMPLATE = 'policy/SNS/AWSCloudTrail-SnsTopicPolicy-2014-12-17.json' + + +class CloudTrailError(Exception): + pass def initialize(cli): @@ -100,6 +104,8 @@ def setup_services(self, args, parsed_globals): self.sns = Service('sns', endpoint_args=endpoint_args, session=self._session) + self.region_name = self.s3.endpoint.region_name + # If the endpoint is specified, it is designated for the cloudtrail # service. Not all of the other services will use it. if 'endpoint_url' in parsed_globals: @@ -184,6 +190,17 @@ def _call(self, options, parsed_globals): 'Logs will be delivered to {bucket}:{prefix}\n'.format( bucket=bucket, prefix=options.s3_prefix or '')) + def _get_policy(self, key_name): + try: + data = self.s3.GetObject( + bucket='awscloudtrail-policy-' + self.region_name, + key=key_name) + return data['Body'].read().decode('utf-8') + except Exception as e: + raise CloudTrailError( + 'Unable to get regional policy template for' + ' region %s: %s. Error: %s', self.region_name, key_name, e) + def setup_new_bucket(self, bucket, prefix, policy_url=None): """ Creates a new S3 bucket with an appropriate policy to let CloudTrail @@ -204,9 +221,7 @@ def setup_new_bucket(self, bucket, prefix, policy_url=None): if policy_url: policy = requests.get(policy_url).text else: - data = self.s3.GetObject(bucket='awscloudtrail', - key=S3_POLICY_TEMPLATE) - policy = data['Body'].read().decode('utf-8') + policy = self._get_policy(S3_POLICY_TEMPLATE) policy = policy.replace('', bucket)\ .replace('', account_id) @@ -233,10 +248,9 @@ def setup_new_bucket(self, bucket, prefix, policy_url=None): # If we are not using the us-east-1 region, then we must set # a location constraint on the new bucket. - region_name = self.s3.endpoint.region_name params = {'bucket': bucket} - if region_name != 'us-east-1': - bucket_config = {'LocationConstraint': region_name} + if self.region_name != 'us-east-1': + bucket_config = {'LocationConstraint': self.region_name} params['create_bucket_configuration'] = bucket_config data = self.s3.CreateBucket(**params) @@ -282,9 +296,7 @@ def setup_new_topic(self, topic, policy_url=None): if policy_url: policy = requests.get(policy_url).text else: - data = self.s3.GetObject(bucket='awscloudtrail', - key=SNS_POLICY_TEMPLATE) - policy = data['Body'].read().decode('utf-8') + policy = self._get_policy(SNS_POLICY_TEMPLATE) policy = policy.replace('', region)\ .replace('', account_id)\ diff --git a/tests/unit/customizations/test_cloudtrail.py b/tests/unit/customizations/test_cloudtrail.py index eb3a31600172..8e74902d8b50 100644 --- a/tests/unit/customizations/test_cloudtrail.py +++ b/tests/unit/customizations/test_cloudtrail.py @@ -16,9 +16,10 @@ import os from awscli.compat import six -from awscli.customizations.cloudtrail import CloudTrailSubscribe +from awscli.customizations.cloudtrail import CloudTrailError, \ + CloudTrailSubscribe from awscli.customizations.service import Service -from mock import Mock, patch +from mock import ANY, Mock, patch from awscli.testutils import BaseAWSCommandParamsTest from tests.unit.test_clidriver import FakeSession from awscli.testutils import unittest @@ -28,6 +29,7 @@ class TestCloudTrail(unittest.TestCase): def setUp(self): self.session = FakeSession({'config_file': 'myconfigfile'}) self.subscribe = CloudTrailSubscribe(self.session) + self.subscribe.region_name = 'us-east-1' self.subscribe.iam = Mock() self.subscribe.iam.GetUser = Mock( @@ -93,6 +95,10 @@ def test_setup_services(self): ref_args['endpoint_url'] = parsed_globals.endpoint_url self.assertEqual(endpoint_call_args[3][1], ref_args) + # Ensure a region name was set on the command class + self.assertEqual(self.subscribe.region_name, + fake_service.get_endpoint.return_value.region_name) + def test_s3_create(self): iam = self.subscribe.iam s3 = self.subscribe.s3 @@ -111,11 +117,19 @@ def test_s3_create(self): args, kwargs = s3.CreateBucket.call_args self.assertNotIn('create_bucket_configuration', kwargs) + def test_s3_uses_regionalized_policy(self): + s3 = self.subscribe.s3 + + self.subscribe.setup_new_bucket('test', 'logs') + + s3.GetObject.assert_called_with( + bucket='awscloudtrail-policy-us-east-1', key=ANY) + def test_s3_create_non_us_east_1(self): # Because this is outside of us-east-1, it should create # a bucket configuration with a location constraint. s3 = self.subscribe.s3 - s3.endpoint.region_name = 'us-west-2' + self.subscribe.region_name = 'us-west-2' self.subscribe.setup_new_bucket('test', 'logs') @@ -144,15 +158,33 @@ def test_s3_create_set_policy_fail(self): s3.PutBucketPolicy = orig - def test_get_policy_fail(self): - orig = self.subscribe.s3.GetObject + def test_s3_get_policy_fail(self): + self.subscribe.s3.GetObject = Mock(side_effect=Exception('Foo!')) + + with self.assertRaises(CloudTrailError) as cm: + self.subscribe.setup_new_bucket('test', 'logs') + + # Exception should contain its custom message, the region + # where there is an issue, and the original exception message. + self.assertIn('us-east-1', str(cm.exception)) + self.assertIn('Foo!', str(cm.exception)) + + def test_get_policy_read_timeout(self): + response = { + 'Body': Mock() + } + response['Body'].read.side_effect = Exception('Error!') + self.subscribe.s3.GetObject.return_value = response + + with self.assertRaises(CloudTrailError): + self.subscribe.setup_new_bucket('test', 'logs') + + def test_sns_get_policy_fail(self): self.subscribe.s3.GetObject = Mock(side_effect=Exception('Error!')) with self.assertRaises(Exception): self.subscribe.setup_new_bucket('test', 'logs') - self.subscribe.s3.GetObject = orig - def test_sns_create(self): s3 = self.subscribe.s3 sns = self.subscribe.sns @@ -166,6 +198,14 @@ def test_sns_create(self): sns.DeleteTopic.assert_not_called() + def test_sns_uses_regionalized_policy(self): + s3 = self.subscribe.s3 + + self.subscribe.setup_new_topic('test') + + s3.GetObject.assert_called_with( + bucket='awscloudtrail-policy-us-east-1', key=ANY) + def test_sns_create_already_exists(self): with self.assertRaises(Exception): self.subscribe.setup_new_topic('test2')