Skip to content

Commit 301ecd9

Browse files
author
iru
authored
fix: sanitized cluster name when using existing (#160)
due to #159 , a bug was introduced for the usage of existing ecs_cluster name. renamed field to `sanitized_cluster_name` for clarity, and protected it to fit both use-cases. more in depth explanation - input-variable format is `foo` - while data format for the cluster_name is `arn:aws:ecs:eu-west-3:425287181461:cluster/foo`. for the ECS autoscale, we just need `foo`. not sure about why data is so non-useful ¯\_(ツ)_/¯ ``` $ terraform state show module.aws_cloudvision_single_account.module.cloud_connector.data.aws_ecs_cluster.this data "aws_ecs_cluster" "this" { arn = "arn:aws:ecs:eu-west-3:425287181461:cluster/foo" cluster_name = "arn:aws:ecs:eu-west-3:425287181461:cluster/foo" id = "arn:aws:ecs:eu-west-3:425287181461:cluster/foo" ... } ```
1 parent 0ce09a9 commit 301ecd9

File tree

4 files changed

+13
-11
lines changed

4 files changed

+13
-11
lines changed

modules/services/cloud-connector-ecs/ecs-service-autoscaling.tf

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ resource "aws_appautoscaling_target" "ecs_target" {
33

44
max_capacity = var.autoscaling_config.max_replicas
55
min_capacity = var.autoscaling_config.min_replicas
6-
resource_id = "service/${local.cluster_name}/${var.name}"
6+
resource_id = "service/${local.sanitized_cluster_name}/${var.name}"
77
scalable_dimension = "ecs:service:DesiredCount"
88
service_namespace = "ecs"
99

@@ -36,7 +36,7 @@ resource "aws_appautoscaling_policy" "ecs_memory_above" {
3636
resource "aws_cloudwatch_metric_alarm" "ecs_memory_above" {
3737
count = var.enable_autoscaling ? 1 : 0
3838

39-
alarm_name = "Step-Scaling-Alarm-Upscale-ECS:service/${local.cluster_name}/${aws_ecs_service.service.name}"
39+
alarm_name = "Step-Scaling-Alarm-Upscale-ECS:service/${local.sanitized_cluster_name}/${aws_ecs_service.service.name}"
4040
alarm_description = "ECS cloud-connector service is above memory utilization threshold"
4141

4242
metric_name = "MemoryUtilization"
@@ -51,7 +51,7 @@ resource "aws_cloudwatch_metric_alarm" "ecs_memory_above" {
5151
alarm_actions = [aws_appautoscaling_policy.ecs_memory_above[0].arn]
5252

5353
dimensions = {
54-
ClusterName = local.cluster_name,
54+
ClusterName = local.sanitized_cluster_name,
5555
ServiceName = aws_ecs_service.service.name
5656
}
5757

@@ -91,7 +91,7 @@ resource "aws_appautoscaling_policy" "ecs_memory_below" {
9191
resource "aws_cloudwatch_metric_alarm" "ecs_memory_below" {
9292
count = var.enable_autoscaling ? 1 : 0
9393

94-
alarm_name = "Step-Scaling-Alarm-Dowscale-ECS:service/${local.cluster_name}/${aws_ecs_service.service.name}"
94+
alarm_name = "Step-Scaling-Alarm-Dowscale-ECS:service/${local.sanitized_cluster_name}/${aws_ecs_service.service.name}"
9595
alarm_description = "ECS cloud-connector service is below memory utilization threshold"
9696

9797
metric_name = "MemoryUtilization"
@@ -106,7 +106,7 @@ resource "aws_cloudwatch_metric_alarm" "ecs_memory_below" {
106106
alarm_actions = [aws_appautoscaling_policy.ecs_memory_below[0].arn]
107107

108108
dimensions = {
109-
ClusterName = local.cluster_name,
109+
ClusterName = local.sanitized_cluster_name,
110110
ServiceName = aws_ecs_service.service.name
111111
}
112112

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
11
locals {
22
deploy_image_scanning = var.deploy_image_scanning_ecs || var.deploy_image_scanning_ecr || var.deploy_beta_image_scanning_ecr
33
deploy_image_scanning_with_codebuild = (var.deploy_image_scanning_ecs || var.deploy_image_scanning_ecr) && !var.deploy_beta_image_scanning_ecr
4+
5+
verify_ssl = var.verify_ssl == "auto" ? length(regexall("https://.*?\\.sysdig(cloud)?.com/?", data.sysdig_secure_connection.current.secure_url)) == 1 : var.verify_ssl == "true"
6+
7+
# this must satisfy input provided existing cluster ARNs and just created cluster format
8+
# ex. input var: 'foo'
9+
# ex. just created cluster name, gathered through tf data : 'arn:aws:ecs:eu-west-3:425287181461:cluster/foo'
10+
sanitized_cluster_name = coalesce(split("/", join("/", [var.ecs_cluster_name, ""]))[1], var.ecs_cluster_name)
411
}
Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1 @@
11
data "aws_region" "current" {}
2-
3-
locals {
4-
verify_ssl = var.verify_ssl == "auto" ? length(regexall("https://.*?\\.sysdig(cloud)?.com/?", data.sysdig_secure_connection.current.secure_url)) == 1 : var.verify_ssl == "true"
5-
cluster_name = coalesce(split("/", var.ecs_cluster_name)[1], var.ecs_cluster_name)
6-
}

test/trigger-events/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ $ terraform apply
4949

5050
| Name | Version |
5151
|------|---------|
52-
| <a name="provider_aws"></a> [aws](#provider\_aws) | 4.38.0 |
52+
| <a name="provider_aws"></a> [aws](#provider\_aws) | >= 4.0.0 |
5353

5454
## Modules
5555

0 commit comments

Comments
 (0)