Merge pull request #443 from /issues/439

Fixes #439 - unhanelded exception when organization CloudTrail trails are present
jantman · Oct 31, 2019 · 2737fad · 2737fad
2 parents d65e897 + 5f6be78
commit 2737fad
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 10 deletions.
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -18,6 +18,7 @@ Unreleased Changes
 * `Issue #429 <https://github.com/jantman/awslimitchecker/issues/429>`_ - add 87 missing EC2 instance types. This will now only impact ``cn-*`` and ``us-gov-*`` regions.
 * `Issue #433 <https://github.com/jantman/awslimitchecker/issues/433>`_ - Fix broken links in the docs; waffle.io and landscape.io are both gone, sadly.
 * `Issue #441 <https://github.com/jantman/awslimitchecker/issues/441>`_ - Fix critical bug where awslimitchecker would die with an unhandled ``botocore.exceptions.ParamValidationError`` exception in accounts that have Trusted Advisor but do not have a "Service Limits" check in the "performance" category.
+* `Issue #439 <https://github.com/jantman/awslimitchecker/issues/439>`_ - Fix unhandled exception in CloudTrail service when attempting to call ``GetEventSelectors`` on an Organization trail. When calling ``DescribeTrails``, we will now pass ``includeShadowTrails`` as False, to not include replications of trails in different regions or organization trails in member accounts (relevant `API documentation <https://docs.aws.amazon.com/awscloudtrail/latest/APIReference/API_DescribeTrails.html>`_).
 
 New EC2 vCPU Limits
 +++++++++++++++++++

diff --git a/awslimitchecker/services/cloudtrail.py b/awslimitchecker/services/cloudtrail.py
@@ -71,15 +71,24 @@ def find_usage(self):
     def _find_usage_cloudtrail(self):
         """Calculate current usage for CloudTrail related metrics"""
 
-        trail_list = self.conn.describe_trails()['trailList']
+        trail_list = self.conn.describe_trails(
+            includeShadowTrails=False
+        )['trailList']
         trail_count = len(trail_list) if trail_list else 0
 
         for trail in trail_list:
             data_resource_count = 0
             if self.conn._client_config.region_name == trail['HomeRegion']:
-                response = self.conn.get_event_selectors(
-                    TrailName=trail['Name']
-                )
+                try:
+                    response = self.conn.get_event_selectors(
+                        TrailName=trail['TrailARN']
+                    )
+                except Exception as ex:
+                    logger.debug(
+                        'Unable to call GetEventSelectors on CloudTrail trail '
+                        '%s: %s', trail, ex
+                    )
+                    continue
                 event_selectors = response['EventSelectors']
                 for event_selector in event_selectors:
                     data_resource_count += len(

diff --git a/awslimitchecker/tests/services/result_fixtures.py b/awslimitchecker/tests/services/result_fixtures.py
@@ -3949,7 +3949,7 @@ class CloudTrail(object):
                     'IncludeGlobalServiceEvents': True,
                     'IsMultiRegionTrail': True,
                     'HomeRegion': 'thisregion',
-                    'TrailARN': 'string',
+                    'TrailARN': 'trailarn1',
                     'LogFileValidationEnabled': True,
                     'CloudWatchLogsLogGroupArn': 'string',
                     'CloudWatchLogsRoleArn': 'string',
@@ -3965,7 +3965,7 @@ class CloudTrail(object):
                     'IncludeGlobalServiceEvents': True,
                     'IsMultiRegionTrail': True,
                     'HomeRegion': 'thisregion',
-                    'TrailARN': 'string',
+                    'TrailARN': 'trailarn2',
                     'LogFileValidationEnabled': True,
                     'CloudWatchLogsLogGroupArn': 'string',
                     'CloudWatchLogsRoleArn': 'string',
@@ -3981,12 +3981,17 @@ class CloudTrail(object):
                     'IncludeGlobalServiceEvents': True,
                     'IsMultiRegionTrail': True,
                     'HomeRegion': 'otherRegion',
-                    'TrailARN': 'string',
+                    'TrailARN': 'trailarn3',
                     'LogFileValidationEnabled': True,
                     'CloudWatchLogsLogGroupArn': 'string',
                     'CloudWatchLogsRoleArn': 'string',
                     'KmsKeyId': 'string',
                     'HasCustomEventSelectors': True
+                },
+                {
+                    'Name': 'trail4',
+                    'TrailARN': 'trailarn4',
+                    'HomeRegion': 'thisregion'
                 }
             ],
     }

diff --git a/awslimitchecker/tests/services/test_cloudtrail.py b/awslimitchecker/tests/services/test_cloudtrail.py
@@ -113,10 +113,12 @@ def test_find_usage(self):
             result_fixtures.CloudTrail.mock_describe_trails
 
         def se_selectors(*args, **kwargs):
-            if kwargs['TrailName'] == 'trail2':
+            if kwargs['TrailName'] == 'trailarn2':
                 return result_fixtures.CloudTrail.mock_get_event_selectors
+            if kwargs['TrailName'] == 'trailarn4':
+                raise RuntimeError('foo')
             return {
-                'TrailArn': 'arn:%s' % kwargs['TrailName'],
+                'TrailArn': kwargs['TrailName'],
                 'EventSelectors': []
             }
 
@@ -133,7 +135,7 @@ def se_selectors(*args, **kwargs):
 
         usage = cls.limits['Trails Per Region'].get_current_usage()
         assert len(usage) == 1
-        assert usage[0].get_value() == 3
+        assert usage[0].get_value() == 4
 
         usage = cls.limits['Event Selectors Per Trail'].get_current_usage()
         assert len(usage) == 2