Skip to content

fix: Remove unused variables regarding listener ARNs count #292

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Remove unused variables regarding listener ARNs count #292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 0 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ module "default_backend_web_app" {
name = "appname"
vpc_id = module.vpc.vpc_id
alb_ingress_unauthenticated_listener_arns = module.alb.listener_arns
alb_ingress_unauthenticated_listener_arns_count = 1
aws_logs_region = "us-east-2"
ecs_cluster_arn = aws_ecs_cluster.default.arn
ecs_cluster_name = aws_ecs_cluster.default.name
Expand Down Expand Up @@ -151,7 +150,6 @@ module "default_backend_web_app" {
| <a name="input_alb_container_name"></a> [alb\_container\_name](#input\_alb\_container\_name) | The name of the container to associate with the ALB. If not provided, the generated container will be used | `string` | `null` | no |
| <a name="input_alb_ingress_authenticated_hosts"></a> [alb\_ingress\_authenticated\_hosts](#input\_alb\_ingress\_authenticated\_hosts) | Authenticated hosts to match in Hosts header | `list(string)` | `[]` | no |
| <a name="input_alb_ingress_authenticated_listener_arns"></a> [alb\_ingress\_authenticated\_listener\_arns](#input\_alb\_ingress\_authenticated\_listener\_arns) | A list of authenticated ALB listener ARNs to attach ALB listener rules to | `list(string)` | `[]` | no |
| <a name="input_alb_ingress_authenticated_listener_arns_count"></a> [alb\_ingress\_authenticated\_listener\_arns\_count](#input\_alb\_ingress\_authenticated\_listener\_arns\_count) | The number of authenticated ARNs in `alb_ingress_authenticated_listener_arns`. This is necessary to work around a limitation in Terraform where counts cannot be computed | `number` | `0` | no |
| <a name="input_alb_ingress_authenticated_paths"></a> [alb\_ingress\_authenticated\_paths](#input\_alb\_ingress\_authenticated\_paths) | Authenticated path pattern to match (a maximum of 1 can be defined) | `list(string)` | `[]` | no |
| <a name="input_alb_ingress_enable_default_target_group"></a> [alb\_ingress\_enable\_default\_target\_group](#input\_alb\_ingress\_enable\_default\_target\_group) | If true, create a default target group for the ALB ingress | `bool` | `true` | no |
| <a name="input_alb_ingress_health_check_healthy_threshold"></a> [alb\_ingress\_health\_check\_healthy\_threshold](#input\_alb\_ingress\_health\_check\_healthy\_threshold) | The number of consecutive health checks successes required before healthy | `number` | `2` | no |
Expand All @@ -170,7 +168,6 @@ module "default_backend_web_app" {
| <a name="input_alb_ingress_target_type"></a> [alb\_ingress\_target\_type](#input\_alb\_ingress\_target\_type) | Target type for the ALB ingress. One of `ip`, `instance`, `lambda` or `container`. Defaults to `ip`, for bridge networking mode should be `instance` | `string` | `"ip"` | no |
| <a name="input_alb_ingress_unauthenticated_hosts"></a> [alb\_ingress\_unauthenticated\_hosts](#input\_alb\_ingress\_unauthenticated\_hosts) | Unauthenticated hosts to match in Hosts header | `list(string)` | `[]` | no |
| <a name="input_alb_ingress_unauthenticated_listener_arns"></a> [alb\_ingress\_unauthenticated\_listener\_arns](#input\_alb\_ingress\_unauthenticated\_listener\_arns) | A list of unauthenticated ALB listener ARNs to attach ALB listener rules to | `list(string)` | `[]` | no |
| <a name="input_alb_ingress_unauthenticated_listener_arns_count"></a> [alb\_ingress\_unauthenticated\_listener\_arns\_count](#input\_alb\_ingress\_unauthenticated\_listener\_arns\_count) | The number of unauthenticated ARNs in `alb_ingress_unauthenticated_listener_arns`. This is necessary to work around a limitation in Terraform where counts cannot be computed | `number` | `0` | no |
| <a name="input_alb_ingress_unauthenticated_paths"></a> [alb\_ingress\_unauthenticated\_paths](#input\_alb\_ingress\_unauthenticated\_paths) | Unauthenticated path pattern to match (a maximum of 1 can be defined) | `list(string)` | `[]` | no |
| <a name="input_alb_security_group"></a> [alb\_security\_group](#input\_alb\_security\_group) | Security group of the ALB | `string` | n/a | yes |
| <a name="input_alb_stickiness_cookie_duration"></a> [alb\_stickiness\_cookie\_duration](#input\_alb\_stickiness\_cookie\_duration) | The time period, in seconds, during which requests from a client should be routed to the same target. After this time period expires, the load balancer-generated cookie is considered stale. The range is 1 second to 1 week (604800 seconds). The default value is 1 day (86400 seconds) | `number` | `86400` | no |
Expand Down
1 change: 0 additions & 1 deletion README.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ usage: |-
name = "appname"
vpc_id = module.vpc.vpc_id
alb_ingress_unauthenticated_listener_arns = module.alb.listener_arns
alb_ingress_unauthenticated_listener_arns_count = 1
aws_logs_region = "us-east-2"
ecs_cluster_arn = aws_ecs_cluster.default.arn
ecs_cluster_name = aws_ecs_cluster.default.name
Expand Down
9 changes: 4 additions & 5 deletions examples/complete/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,10 @@ module "ecs_web_app" {
container_port = var.container_port

# ALB
alb_arn_suffix = module.alb.alb_arn_suffix
alb_security_group = module.alb.security_group_id
alb_ingress_unauthenticated_listener_arns = [module.alb.http_listener_arn]
alb_ingress_unauthenticated_listener_arns_count = 1
alb_ingress_healthcheck_path = var.alb_ingress_healthcheck_path
alb_arn_suffix = module.alb.alb_arn_suffix
alb_security_group = module.alb.security_group_id
alb_ingress_unauthenticated_listener_arns = [module.alb.http_listener_arn]
alb_ingress_healthcheck_path = var.alb_ingress_healthcheck_path

# CodePipeline
codepipeline_enabled = var.codepipeline_enabled
Expand Down
3 changes: 1 addition & 2 deletions examples/with_cognito_authentication/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,7 @@ module "web_app" {
alb_ingress_healthcheck_path = "/"

# NOTE: Cognito and OIDC authentication only supported on HTTPS endpoints; here we provide `https_listener_arn` from ALB
alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn
alb_ingress_authenticated_listener_arns_count = 1
alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Wrap single ARN in a list
The variable alb_ingress_authenticated_listener_arns expects a list of strings but is being assigned a single string. Wrap it in a list to satisfy the variable type.

Apply this diff:

-  alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn
+  alb_ingress_authenticated_listener_arns = [module.alb.https_listener_arn]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn
alb_ingress_authenticated_listener_arns = [module.alb.https_listener_arn]
🤖 Prompt for AI Agents 
In examples/with_cognito_authentication/main.tf at line 129, the variable
alb_ingress_authenticated_listener_arns expects a list of strings but is
currently assigned a single string. Fix this by wrapping
module.alb.https_listener_arn in square brackets to create a list containing
that single ARN.


# Unauthenticated paths (with higher priority than the authenticated paths)
alb_ingress_unauthenticated_paths = ["/events"]
Expand Down
3 changes: 1 addition & 2 deletions examples/with_google_oidc_authentication/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ module "web_app" {
alb_ingress_healthcheck_path = "/"

# NOTE: Cognito and OIDC authentication only supported on HTTPS endpoints; here we provide `https_listener_arn` from ALB
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Wrap single ARN in a list
The variable alb_ingress_authenticated_listener_arns is defined as a list of strings, but here it's assigned a single string (module.alb.https_listener_arn), causing a type mismatch. Wrap the ARN in a list.

Apply this diff:

-  alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn
+  alb_ingress_authenticated_listener_arns = [module.alb.https_listener_arn]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# NOTE: Cognito and OIDC authentication only supported on HTTPS endpoints; here we provide `https_listener_arn` from ALB
# NOTE: Cognito and OIDC authentication only supported on HTTPS endpoints; here we provide `https_listener_arn` from ALB
alb_ingress_authenticated_listener_arns = [module.alb.https_listener_arn]
🤖 Prompt for AI Agents 
In examples/with_google_oidc_authentication/main.tf at line 129, the variable
alb_ingress_authenticated_listener_arns expects a list of strings but is
assigned a single string module.alb.https_listener_arn. Fix this by wrapping
module.alb.https_listener_arn in square brackets to make it a single-element
list.

alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn
alb_ingress_authenticated_listener_arns_count = 1
alb_ingress_authenticated_listener_arns = module.alb.https_listener_arn

# Unauthenticated paths (with higher priority than the authenticated paths)
alb_ingress_unauthenticated_paths = ["/events"]
Expand Down
3 changes: 1 addition & 2 deletions examples/without_authentication/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ module "web_app" {
alb_ingress_healthcheck_path = "/"

# Without authentication, both HTTP and HTTPS endpoints are supported
alb_ingress_unauthenticated_listener_arns = module.alb.listener_arns
alb_ingress_unauthenticated_listener_arns_count = 2
alb_ingress_unauthenticated_listener_arns = module.alb.listener_arns

# All paths are unauthenticated
alb_ingress_unauthenticated_paths = ["/*"]
Expand Down
12 changes: 0 additions & 12 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -935,24 +935,12 @@ variable "alb_ingress_unauthenticated_listener_arns" {
default = []
}

variable "alb_ingress_unauthenticated_listener_arns_count" {
type = number
description = "The number of unauthenticated ARNs in `alb_ingress_unauthenticated_listener_arns`. This is necessary to work around a limitation in Terraform where counts cannot be computed"
default = 0
}

variable "alb_ingress_authenticated_listener_arns" {
type = list(string)
description = "A list of authenticated ALB listener ARNs to attach ALB listener rules to"
default = []
}

variable "alb_ingress_authenticated_listener_arns_count" {
type = number
description = "The number of authenticated ARNs in `alb_ingress_authenticated_listener_arns`. This is necessary to work around a limitation in Terraform where counts cannot be computed"
default = 0
}

variable "authentication_type" {
type = string
description = "Authentication type. Supported values are `COGNITO` and `OIDC`"
Expand Down