added cloudwatch module #184
Conversation
terraform/modules/cloudwatch/main.tf
Outdated
| } | ||
| } | ||
|
|
||
| resource "aws_sns_topic" "slack_notifications" { |
There was a problem hiding this comment.
SNS should be split out into a separate module. the topic can be created there and passed to the events module
terraform/modules/cloudwatch/main.tf
Outdated
| endpoint = var.slack_notification_endpoint | ||
| } | ||
|
|
||
| resource "aws_iam_role" "cloudwatch_events_role" { |
There was a problem hiding this comment.
can the IAM resources be split out into an "iam.tf" file - this makes it easier to review these if we are asked for this by cloudone
terraform/modules/cloudwatch/main.tf
Outdated
| schedule_expression = var.cron_expression | ||
| } | ||
|
|
||
| resource "aws_cloudwatch_event_target" "ecs_event_target" { |
There was a problem hiding this comment.
for the event target we should be setting this as a conditional step, right now we only have ecs as the target but in the future this could change. I think there should be some variable "target_type" that will control whether this block gets run. if the target type is ecs you would run this, something like:
for_each = var.target_type == "ecs" ? 1 : 0
| @@ -0,0 +1,30 @@ | |||
| variable "resource_prefix" { | |||
There was a problem hiding this comment.
from previous comment we should have something to determine the target type like this:
variable "target_type" {
type = string
description = "Select a target type for the cloudwatch event"
sensitive = false
validation {
condition = contains(["ecs"], var.target_type)
error_message = "The variable target_type must be one of: ecs"
}
}
| type = string | ||
| } | ||
|
|
||
| variable "ecs_cluster_name" { |
There was a problem hiding this comment.
these should have default null values and can be validated in locals like:
locals {
validate_ecs_cluster = (var.target_type == "ecs" && var.ecs_cluster_name == null) ? tobool("Please pass in the ecs cluster name if you are using ecs as the target.") : true
}
this way there will be an error if the target is set to ecs and these vars don't have values passed in
No description provided.