Skip to content

fix: define container_insights as an enum #69

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 8, 2025
Merged

fix: define container_insights as an enum #69

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50690a4
fix: define container_insights as an enum
leeserpa Sep 5, 2025
664f216
terraform-docs: automated update action
leeserpa Sep 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ module "ecs_apps" {
| certificate\_arn | n/a | `any` | n/a | yes |
| certificate\_internal\_arn | certificate arn for internal ALB. | `string` | `""` | no |
| code\_deploy | Enables CodeDeploy role to be used for deployment | `bool` | `true` | no |
| container\_insights | Enables CloudWatch Container Insights for a cluster. | `bool` | `false` | no |
| container\_insights | Enables CloudWatch Container Insights for a cluster. | `string` | `"disabled"` | no |
| create\_efs | Enables creation of EFS volume for cluster | `bool` | `true` | no |
| create\_iam\_service\_linked\_role | Create iam\_service\_linked\_role for ECS or not. | `bool` | `false` | no |
| ebs\_key\_arn | ARN of a KMS Key to use on EBS volumes | `string` | `""` | no |
Expand Down
8 changes: 6 additions & 2 deletions _variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,9 +368,13 @@ variable "extra_task_policies_arn" {
}

variable "container_insights" {
type = bool
default = false
type = string
Comment on lines 369 to +371
Copy link

@amazon-q-developer amazon-q-developer bot Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good improvement to the container insights configuration. Using an enum with validation ensures that only valid values are provided and adds support for the "enhanced" option. Consider adding a description of what each value means to help users choose the appropriate setting.

Suggested change
variable "container_insights" {
type = bool
default = false
type = string
variable "container_insights" {
type = string
default = "disabled"
description = <<-EOT
Enables CloudWatch Container Insights for a cluster.
Valid values:
- "enhanced": Enables enhanced container insights with additional metrics
- "enabled": Enables basic container insights
- "disabled": Disables container insights
EOT
validation {
condition = contains(["enhanced", "enabled", "disabled"], var.container_insights)
error_message = "Container Insights must be one of: enhanced, enabled, disabled."
}
}

default = "disabled"
description = "Enables CloudWatch Container Insights for a cluster."
validation {
condition = contains(["enhanced", "enabled", "disabled"], var.container_insights)
error_message = "Container Insights must be one of: enhanced, enabled, disabled."
}
}

variable "alb_test_listener" {
Expand Down
4 changes: 2 additions & 2 deletions ecs.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ resource "aws_ecs_cluster" "ecs" {

setting {
name = "containerInsights"
value = var.container_insights ? "enabled" : "disabled"
value = var.container_insights
}

tags = merge(
Expand All @@ -13,7 +13,7 @@ resource "aws_ecs_cluster" "ecs" {
},
)
lifecycle {
ignore_changes = []
ignore_changes = [service_connect_defaults]

Comment on lines 15 to 17
Copy link

@amazon-q-developer amazon-q-developer bot Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good addition of service_connect_defaults to ignore_changes. This prevents unwanted updates when service connect configurations are modified outside of Terraform. Consider adding a comment explaining why this change is needed for better maintainability.

Suggested change
lifecycle {
ignore_changes = []
ignore_changes = [service_connect_defaults]
lifecycle {
# Ignore changes to service connect defaults as they may be modified outside of Terraform
ignore_changes = [service_connect_defaults]
}

}
}
Expand Down