From 99ca9a6ad0e0e89364261c51c9d065ac390700ad Mon Sep 17 00:00:00 2001 From: "Daniel G. Taylor" Date: Thu, 19 Feb 2015 13:32:51 -0800 Subject: [PATCH] Use CloudTrail's regionalized policy templates This change switches away from always using an S3 bucket in `us-west-2` to fetch CloudTrail policy information for S3 and SNS when setting up new buckets/topics and instead uses the `awscloudtrail-policy-REGION` regionalized buckets. It also updates the policy version to use the most recent release. Tests are updated and a couple new ones make sure the regionalized buckets are used. --- awscli/customizations/cloudtrail.py | 21 +++++++++------- tests/unit/customizations/test_cloudtrail.py | 25 ++++++++++++++++++-- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/awscli/customizations/cloudtrail.py b/awscli/customizations/cloudtrail.py index f99489301e2f..307410054e56 100644 --- a/awscli/customizations/cloudtrail.py +++ b/awscli/customizations/cloudtrail.py @@ -20,8 +20,8 @@ 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' def initialize(cli): @@ -100,6 +100,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: @@ -204,8 +206,9 @@ 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) + data = self.s3.GetObject( + bucket='awscloudtrail-policy-' + self.region_name, + key=S3_POLICY_TEMPLATE) policy = data['Body'].read().decode('utf-8') policy = policy.replace('', bucket)\ @@ -233,10 +236,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,8 +284,9 @@ 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) + data = self.s3.GetObject( + bucket='awscloudtrail-policy-' + self.region_name, + key=SNS_POLICY_TEMPLATE) policy = data['Body'].read().decode('utf-8') policy = policy.replace('', region)\ diff --git a/tests/unit/customizations/test_cloudtrail.py b/tests/unit/customizations/test_cloudtrail.py index eb3a31600172..de29cdfdc866 100644 --- a/tests/unit/customizations/test_cloudtrail.py +++ b/tests/unit/customizations/test_cloudtrail.py @@ -18,7 +18,7 @@ from awscli.customizations.cloudtrail import 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 +28,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 +94,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 +116,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') @@ -166,6 +179,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')