Skip to content

Commit

Permalink
fix: sanitized cluster name when using existing (#160)
Browse files Browse the repository at this point in the history
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"
...
}
```
  • Loading branch information
iru authored Jan 31, 2023
1 parent 0ce09a9 commit 301ecd9
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 11 deletions.
10 changes: 5 additions & 5 deletions modules/services/cloud-connector-ecs/ecs-service-autoscaling.tf
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ resource "aws_appautoscaling_target" "ecs_target" {

max_capacity = var.autoscaling_config.max_replicas
min_capacity = var.autoscaling_config.min_replicas
resource_id = "service/${local.cluster_name}/${var.name}"
resource_id = "service/${local.sanitized_cluster_name}/${var.name}"
scalable_dimension = "ecs:service:DesiredCount"
service_namespace = "ecs"

Expand Down Expand Up @@ -36,7 +36,7 @@ resource "aws_appautoscaling_policy" "ecs_memory_above" {
resource "aws_cloudwatch_metric_alarm" "ecs_memory_above" {
count = var.enable_autoscaling ? 1 : 0

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

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

dimensions = {
ClusterName = local.cluster_name,
ClusterName = local.sanitized_cluster_name,
ServiceName = aws_ecs_service.service.name
}

Expand Down Expand Up @@ -91,7 +91,7 @@ resource "aws_appautoscaling_policy" "ecs_memory_below" {
resource "aws_cloudwatch_metric_alarm" "ecs_memory_below" {
count = var.enable_autoscaling ? 1 : 0

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

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

dimensions = {
ClusterName = local.cluster_name,
ClusterName = local.sanitized_cluster_name,
ServiceName = aws_ecs_service.service.name
}

Expand Down
7 changes: 7 additions & 0 deletions modules/services/cloud-connector-ecs/locals.tf
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
locals {
deploy_image_scanning = var.deploy_image_scanning_ecs || var.deploy_image_scanning_ecr || var.deploy_beta_image_scanning_ecr
deploy_image_scanning_with_codebuild = (var.deploy_image_scanning_ecs || var.deploy_image_scanning_ecr) && !var.deploy_beta_image_scanning_ecr

verify_ssl = var.verify_ssl == "auto" ? length(regexall("https://.*?\\.sysdig(cloud)?.com/?", data.sysdig_secure_connection.current.secure_url)) == 1 : var.verify_ssl == "true"

# this must satisfy input provided existing cluster ARNs and just created cluster format
# ex. input var: 'foo'
# ex. just created cluster name, gathered through tf data : 'arn:aws:ecs:eu-west-3:425287181461:cluster/foo'
sanitized_cluster_name = coalesce(split("/", join("/", [var.ecs_cluster_name, ""]))[1], var.ecs_cluster_name)
}
5 changes: 0 additions & 5 deletions modules/services/cloud-connector-ecs/main.tf
Original file line number Diff line number Diff line change
@@ -1,6 +1 @@
data "aws_region" "current" {}

locals {
verify_ssl = var.verify_ssl == "auto" ? length(regexall("https://.*?\\.sysdig(cloud)?.com/?", data.sysdig_secure_connection.current.secure_url)) == 1 : var.verify_ssl == "true"
cluster_name = coalesce(split("/", var.ecs_cluster_name)[1], var.ecs_cluster_name)
}
2 changes: 1 addition & 1 deletion test/trigger-events/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ $ terraform apply

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

## Modules

Expand Down

0 comments on commit 301ecd9

Please sign in to comment.