-
Notifications
You must be signed in to change notification settings - Fork 334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug Fixes (Initialization, AWS Perms, Config), CloudWatch Event Patterns, and More #205
Changes from 8 commits
9b521a2
2ecf848
7865f2e
9cce9d8
dc17bbe
291f629
6dadb70
7098ac1
3c74456
61d5a13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -53,7 +53,7 @@ def generate_s3_bucket(**kwargs): | |
'target_bucket': logging_bucket, | ||
'target_prefix': '{}/'.format(bucket_name) | ||
} | ||
force_destroy = kwargs.get('force_destroy', False) | ||
force_destroy = kwargs.get('force_destroy', True) | ||
versioning = kwargs.get('versioning', True) | ||
lifecycle_rule = kwargs.get('lifecycle_rule') | ||
|
||
|
@@ -352,6 +352,32 @@ def generate_cloudtrail(cluster_name, cluster_dict, config): | |
""" | ||
modules = config['clusters'][cluster_name]['modules'] | ||
cloudtrail_enabled = bool(modules['cloudtrail']['enabled']) | ||
existing_trail_default = False | ||
existing_trail = modules['cloudtrail'].get('existing_trail', existing_trail_default) | ||
is_global_trail_default = True | ||
is_global_trail = modules['cloudtrail'].get('is_global_trail', is_global_trail_default) | ||
event_pattern_default = { | ||
'account': [config['global']['account']['aws_account_id']] | ||
} | ||
event_pattern = modules['cloudtrail'].get('event_pattern', event_pattern_default) | ||
|
||
# From here: | ||
# http://docs.aws.amazon.com/AmazonCloudWatch/latest/events/CloudWatchEventsandEventPatterns.html | ||
valid_event_pattern_keys = { | ||
'version', | ||
'id', | ||
'detail-type', | ||
'source', | ||
'account', | ||
'time', | ||
'region', | ||
'resources', | ||
'detail' | ||
} | ||
if not set(event_pattern.keys()).issubset(valid_event_pattern_keys): | ||
LOGGER_CLI.error('Invalid CloudWatch Event Pattern!') | ||
sys.exit(1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably start returning a value in places like this (maybe just a simple bool == False, or a more comprehensive solution would be returning a non-zero error number) that we could check up the call stack and exit on error. It would avoid having There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, I'll make some changes |
||
|
||
cluster_dict['module']['cloudtrail_{}'.format(cluster_name)] = { | ||
'account_id': config['global']['account']['aws_account_id'], | ||
'cluster': cluster_name, | ||
|
@@ -360,7 +386,11 @@ def generate_cloudtrail(cluster_name, cluster_dict, config): | |
'enable_logging': cloudtrail_enabled, | ||
'source': 'modules/tf_stream_alert_cloudtrail', | ||
's3_logging_bucket': '{}.streamalert.s3-logging'.format( | ||
config['global']['account']['prefix'])} | ||
config['global']['account']['prefix']), | ||
'existing_trail': existing_trail, | ||
'is_global_trail': is_global_trail, | ||
'event_pattern': json.dumps(event_pattern) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we want There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
|
||
def generate_flow_logs(cluster_name, cluster_dict, config): | ||
|
@@ -373,11 +403,17 @@ def generate_flow_logs(cluster_name, cluster_dict, config): | |
config [dict]: The loaded config from the 'conf/' directory | ||
""" | ||
modules = config['clusters'][cluster_name]['modules'] | ||
flow_log_group_name_default = '{}_{}_streamalert_flow_logs'.format( | ||
config['global']['account']['prefix'], | ||
cluster_name | ||
) | ||
flow_log_group_name = modules['flow_logs'].get('log_group_name', flow_log_group_name_default) | ||
|
||
if modules['flow_logs']['enabled']: | ||
cluster_dict['module']['flow_logs_{}'.format(cluster_name)] = { | ||
'source': 'modules/tf_stream_alert_flow_logs', | ||
'destination_stream_arn': '${{module.kinesis_{}.arn}}'.format(cluster_name), | ||
'flow_log_group_name': modules['flow_logs']['log_group_name']} | ||
'flow_log_group_name': flow_log_group_name} | ||
for flow_log_input in ('vpcs', 'subnets', 'enis'): | ||
input_data = modules['flow_logs'].get(flow_log_input) | ||
if input_data: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'm just not seeing it elsewhere, but it looks like
existing_trail_default
andis_global_trail_default
are just booleans and only used to hold default values for the.get()
calls. Can we just place the boolean value directly in the get for brevity?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, they're just booleans. I defined them as variables to be really explicit about what the defaults are for these options.