-
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
Create re-usable Lambda Terraform module #596
Conversation
48e0565
to
c456343
Compare
Update: Due to hashicorp/terraform-provider-aws#443, this is going to have to be re-worked so that there are 2 different Lambda resources (VPC and non-VPC). It works the first time, and then subsequent deploys with no VPC always fail. 👎 Not ready for review yet - apologies |
@jacknagz @ryandeivert this is now ready for review. Thanks! |
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.
One quick comment, and then another general question: How will this module be used with the SA CLI? Are those changes coming after this?
@@ -0,0 +1,132 @@ | |||
// Note: We use this variable because terraform does not support "count" for module resources | |||
// https://github.com/hashicorp/terraform/issues/953 | |||
variable "enabled" { |
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.
Has there been an example where this would be used? I'm trying to contrast this approach with just deleting the module altogether
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.
Yes - the threat intel downloader is optional. Once the downloader uses this module (instead of building the Lambda resource directly), it needs a way to indicate whether or not the function should be enabled. This is, for example, how the optional BinaryAlert CarbonBlack downloader works
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.
I see, so your vision is to pre-allocate the Terraform code, and then just flip the switch as needed. Generally with the SA CLI, we would just not marshal out the Terraform code to generate the Lambda. Either approach works though.
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.
Ah, I see. Yeah, having the feature just adds flexibility - whichever way works best
@jacknagz this PR just provides a new module for future use, it does not change the existing infrastructure or workflow in any way After this merges, we will create a global alert processor, at which point the CLI, config files, and Terraform generation process will need to be changed accordingly. |
Sounds good, thanks! @austinbyers |
to: @ryandeivert and @jacknagz
cc: @airbnb/streamalert-maintainers
size: medium
Background
The number of different Lambda functions has grown over time and will continue to do so this quarter. To simplify the process of managing Terraform resources related to each Lambda function, this creates a generic re-usable Lambda module. BinaryAlert has a similar module which provided inspiration for this implementation.
Changes
tf_lambda
. The alert processor (and future alert dispatcher) will use this soon and other Lambda functions will be migrated to this over time. The module contains the following components:Providing a re-usable module like this ensures that all Lambda functions have the same base permissions, metrics, and tagging. For example, right now tagging is not enabled for CloudWatch log groups in most (all?) of StreamAlert's Lambda functions.
This PR simply creates the new module, it does not use it any way. The existing infrastructure remains unchanged.
Testing
main.tf.json
to create a global alert processor like so:"enable_iterator_age_alarm": true
- iterator age alarm was created"enable_metric_alarms": false
- metric alarms were destroyed"enabled": false
- all resources were destroyed