Skip to content

Commit

Permalink
Use CloudTrail's regionalized policy templates
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
danielgtaylor committed Feb 19, 2015
1 parent b9c2900 commit 99ca9a6
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 11 deletions.
21 changes: 12 additions & 9 deletions awscli/customizations/cloudtrail.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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('<BucketName>', bucket)\
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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>', region)\
Expand Down
25 changes: 23 additions & 2 deletions tests/unit/customizations/test_cloudtrail.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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')

Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 99ca9a6

Please sign in to comment.