Skip to content
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

Jump to bottom

feat(terraform): adds ignore_default_action input variable #140

Closed
wants to merge 2 commits into from
Closed

feat(terraform): adds ignore_default_action input variable #140

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
4ead18f
feat(terraform): adds ignore_default_action input variable
jbrt May 23, 2023
39b9377
Merge branch 'main' into feat/adds_ignore_default_action
jbrt Oct 4, 2023
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
4 changes: 4 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,11 @@ Available targets:
|------|------|
| [aws_lb.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb) | resource |
| [aws_lb_listener.http_forward](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.http_forward_ignore_default_action](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.http_redirect](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.http_redirect_ignore_default_action](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.https](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.https_ignore_default_action](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener_certificate.https_sni](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener_certificate) | resource |
| [aws_lb_target_group.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_target_group) | resource |
| [aws_security_group.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource |
Expand Down Expand Up @@ -282,6 +285,7 @@ Available targets:
| <a name="input_https_ssl_policy"></a> [https\_ssl\_policy](#input\_https\_ssl\_policy) | The name of the SSL Policy for the listener | `string` | `"ELBSecurityPolicy-2015-05"` | no |
| <a name="input_id_length_limit"></a> [id\_length\_limit](#input\_id\_length\_limit) | Limit `id` to this many characters (minimum 6).<br>Set to `0` for unlimited length.<br>Set to `null` for keep the existing setting, which defaults to `0`.<br>Does not affect `id_full`. | `number` | `null` | no |
| <a name="input_idle_timeout"></a> [idle\_timeout](#input\_idle\_timeout) | The time in seconds that the connection is allowed to be idle | `number` | `60` | no |
| <a name="input_ignore_default_action_changes"></a> [ignore\_default\_action\_changes](#input\_ignore\_default\_action\_changes) | Whether to ignore changes for Load balancer default action (useful when using CodeDeploy Blue/Green deployments to avoid drifts) | `bool` | `false` | no |
| <a name="input_internal"></a> [internal](#input\_internal) | A boolean flag to determine whether the ALB should be internal | `bool` | `false` | no |
| <a name="input_ip_address_type"></a> [ip\_address\_type](#input\_ip\_address\_type) | The type of IP addresses used by the subnets for your load balancer. The possible values are `ipv4` and `dualstack`. | `string` | `"ipv4"` | no |
| <a name="input_label_key_case"></a> [label\_key\_case](#input\_label\_key\_case) | Controls the letter case of the `tags` keys (label names) for tags generated by this module.<br>Does not affect keys of tags passed in via the `tags` input.<br>Possible values: `lower`, `title`, `upper`.<br>Default value: `title`. | `string` | `null` | no |
Expand Down
4 changes: 4 additions & 0 deletions docs/terraform.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,11 @@
|------|------|
| [aws_lb.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb) | resource |
| [aws_lb_listener.http_forward](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.http_forward_ignore_default_action](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.http_redirect](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.http_redirect_ignore_default_action](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.https](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener.https_ignore_default_action](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener) | resource |
| [aws_lb_listener_certificate.https_sni](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener_certificate) | resource |
| [aws_lb_target_group.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_target_group) | resource |
| [aws_security_group.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource |
Expand Down Expand Up @@ -82,6 +85,7 @@
| <a name="input_https_ssl_policy"></a> [https\_ssl\_policy](#input\_https\_ssl\_policy) | The name of the SSL Policy for the listener | `string` | `"ELBSecurityPolicy-2015-05"` | no |
| <a name="input_id_length_limit"></a> [id\_length\_limit](#input\_id\_length\_limit) | Limit `id` to this many characters (minimum 6).<br>Set to `0` for unlimited length.<br>Set to `null` for keep the existing setting, which defaults to `0`.<br>Does not affect `id_full`. | `number` | `null` | no |
| <a name="input_idle_timeout"></a> [idle\_timeout](#input\_idle\_timeout) | The time in seconds that the connection is allowed to be idle | `number` | `60` | no |
| <a name="input_ignore_default_action_changes"></a> [ignore\_default\_action\_changes](#input\_ignore\_default\_action\_changes) | Whether to ignore changes for Load balancer default action (useful when using CodeDeploy Blue/Green deployments to avoid drifts) | `bool` | `false` | no |
| <a name="input_internal"></a> [internal](#input\_internal) | A boolean flag to determine whether the ALB should be internal | `bool` | `false` | no |
| <a name="input_ip_address_type"></a> [ip\_address\_type](#input\_ip\_address\_type) | The type of IP addresses used by the subnets for your load balancer. The possible values are `ipv4` and `dualstack`. | `string` | `"ipv4"` | no |
| <a name="input_label_key_case"></a> [label\_key\_case](#input\_label\_key\_case) | Controls the letter case of the `tags` keys (label names) for tags generated by this module.<br>Does not affect keys of tags passed in via the `tags` input.<br>Possible values: `lower`, `title`, `upper`.<br>Default value: `title`. | `string` | `null` | no |
Expand Down
86 changes: 83 additions & 3 deletions main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@
resource "aws_lb_listener" "http_forward" {
#bridgecrew:skip=BC_AWS_GENERAL_43 - Skipping Ensure that load balancer is using TLS 1.2.
#bridgecrew:skip=BC_AWS_NETWORKING_29 - Skipping Ensure ALB Protocol is HTTPS
count = module.this.enabled && var.http_enabled && var.http_redirect != true ? 1 : 0
count = module.this.enabled && var.http_enabled && !var.ignore_default_action_changes && var.http_redirect != true ? 1 : 0
load_balancer_arn = one(aws_lb.default[*].arn)
port = var.http_port
protocol = "HTTP"
Expand All @@ -171,8 +171,36 @@
}
}

resource "aws_lb_listener" "http_forward_ignore_default_action" {
Copy link
Member

Choose a reason for hiding this comment

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

Huh this is beyond the scope of your PR, but noting for myself and any fellow contributors: I'm not even sure why we need to have two of these resources for http_forward + http_redirect. It seems we could just do the conditional logic In the default action itself without requiring two resources.

@joe-niland do you happen to know the history behind why it was done this way?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

@Gowiem sorry I missed this one! I don't know why but it dates back to the initial implementation I think. I agree it could be refactored as you described.

Copy link
Member

Choose a reason for hiding this comment

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

@joe-niland I'll create an issue and we can see if a passer by wants to pick it up!

#bridgecrew:skip=BC_AWS_GENERAL_43 - Skipping Ensure that load balancer is using TLS 1.2.
#bridgecrew:skip=BC_AWS_NETWORKING_29 - Skipping Ensure ALB Protocol is HTTPS
count = module.this.enabled && var.http_enabled && var.ignore_default_action_changes && var.http_redirect != true ? 1 : 0
load_balancer_arn = join("", aws_lb.default.*.arn)

Check warning on line 178 in main.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
main.tf:178:46: warning: List items should be accessed using square brackets ()
port = var.http_port
protocol = "HTTP"
tags = merge(module.this.tags, var.listener_additional_tags)

default_action {
target_group_arn = var.listener_http_fixed_response != null ? null : join("", aws_lb_target_group.default.*.arn)

Check warning on line 184 in main.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
main.tf:184:110: warning: List items should be accessed using square brackets ()
type = var.listener_http_fixed_response != null ? "fixed-response" : "forward"

dynamic "fixed_response" {
for_each = var.listener_http_fixed_response != null ? [var.listener_http_fixed_response] : []
content {
content_type = fixed_response.value["content_type"]
message_body = fixed_response.value["message_body"]
status_code = fixed_response.value["status_code"]
}
}
}

lifecycle {
ignore_changes = [default_action]
}
}

resource "aws_lb_listener" "http_redirect" {
count = module.this.enabled && var.http_enabled && var.http_redirect == true ? 1 : 0
count = module.this.enabled && var.http_enabled && !var.ignore_default_action_changes && var.http_redirect == true ? 1 : 0
load_balancer_arn = one(aws_lb.default[*].arn)
port = var.http_port
protocol = "HTTP"
Expand All @@ -190,9 +218,32 @@
}
}

resource "aws_lb_listener" "http_redirect_ignore_default_action" {
count = module.this.enabled && var.http_enabled && var.ignore_default_action_changes && var.http_redirect == true ? 1 : 0
load_balancer_arn = join("", aws_lb.default.*.arn)

Check warning on line 223 in main.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
main.tf:223:46: warning: List items should be accessed using square brackets ()
port = var.http_port
protocol = "HTTP"
tags = merge(module.this.tags, var.listener_additional_tags)

default_action {
target_group_arn = join("", aws_lb_target_group.default.*.arn)

Check warning on line 229 in main.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
main.tf:229:60: warning: List items should be accessed using square brackets ()
type = "redirect"

redirect {
port = "443"
protocol = "HTTPS"
status_code = "HTTP_301"
}
}

lifecycle {
ignore_changes = [default_action]
}
}

resource "aws_lb_listener" "https" {
#bridgecrew:skip=BC_AWS_GENERAL_43 - Skipping Ensure that load balancer is using TLS 1.2.
count = module.this.enabled && var.https_enabled ? 1 : 0
count = module.this.enabled && !var.ignore_default_action_changes && var.https_enabled ? 1 : 0
load_balancer_arn = one(aws_lb.default[*].arn)

port = var.https_port
Expand All @@ -216,6 +267,35 @@
}
}

resource "aws_lb_listener" "https_ignore_default_action" {
#bridgecrew:skip=BC_AWS_GENERAL_43 - Skipping Ensure that load balancer is using TLS 1.2.
count = module.this.enabled && var.ignore_default_action_changes && var.https_enabled ? 1 : 0
load_balancer_arn = join("", aws_lb.default.*.arn)

Check warning on line 273 in main.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
main.tf:273:46: warning: List items should be accessed using square brackets ()

port = var.https_port
protocol = "HTTPS"
ssl_policy = var.https_ssl_policy
certificate_arn = var.certificate_arn
tags = merge(module.this.tags, var.listener_additional_tags)

default_action {
target_group_arn = var.listener_https_fixed_response != null ? null : join("", aws_lb_target_group.default.*.arn)

Check warning on line 282 in main.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
main.tf:282:111: warning: List items should be accessed using square brackets ()
type = var.listener_https_fixed_response != null ? "fixed-response" : "forward"

dynamic "fixed_response" {
for_each = var.listener_https_fixed_response != null ? [var.listener_https_fixed_response] : []
content {
content_type = fixed_response.value["content_type"]
message_body = fixed_response.value["message_body"]
status_code = fixed_response.value["status_code"]
}
}
}
lifecycle {
ignore_changes = [default_action]
}
}

resource "aws_lb_listener_certificate" "https_sni" {
count = module.this.enabled && var.https_enabled && length(var.additional_certs) > 0 ? length(var.additional_certs) : 0
listener_arn = one(aws_lb_listener.https[*].arn)
Expand Down
8 changes: 4 additions & 4 deletions outputs.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,23 @@

output "http_listener_arn" {
description = "The ARN of the HTTP forwarding listener"
value = one(aws_lb_listener.http_forward[*].arn)
value = join("", aws_lb_listener.http_forward.*.arn, aws_lb_listener.http_forward_ignore_default_action.*.arn)

Check warning on line 43 in outputs.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
outputs.tf:43:54: warning: List items should be accessed using square brackets ()

Check warning on line 43 in outputs.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
outputs.tf:43:112: warning: List items should be accessed using square brackets ()
Copy link
Member

Choose a reason for hiding this comment

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

@jbrt for all of these lint errors, you do want to use brackets instead of .*. as that is the preferred way of using that operator.

}

output "http_redirect_listener_arn" {
description = "The ARN of the HTTP to HTTPS redirect listener"
value = one(aws_lb_listener.http_redirect[*].arn)
value = join("", aws_lb_listener.http_redirect.*.arn, aws_lb_listener.http_redirect_ignore_default_action.*.arn)

Check warning on line 48 in outputs.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
outputs.tf:48:55: warning: List items should be accessed using square brackets ()

Check warning on line 48 in outputs.tf

View workflow job for this annotation

GitHub Actions / terraform-module / ci-terraform / lint (.) 

[tflint] reported by reviewdog 🐶
List items should be accessed using square brackets

Raw Output:
outputs.tf:48:114: warning: List items should be accessed using square brackets ()
}

output "https_listener_arn" {
description = "The ARN of the HTTPS listener"
value = one(aws_lb_listener.https[*].arn)
value = join("", aws_lb_listener.https.*.arn, aws_lb_listener.https_ignore_default_action.*.arn)
}

output "listener_arns" {
description = "A list of all the listener ARNs"
value = compact(
concat(aws_lb_listener.http_forward[*].arn, aws_lb_listener.http_redirect[*].arn, aws_lb_listener.https[*].arn)
concat(aws_lb_listener.http_forward.*.arn, aws_lb_listener.http_redirect.*.arn, aws_lb_listener.https.*.arn, aws_lb_listener.http_forward_ignore_default_action.*.arn, aws_lb_listener.http_redirect_ignore_default_action.*.arn, aws_lb_listener.https_ignore_default_action.*.arn)
)
}

Expand Down
6 changes: 6 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,12 @@ variable "health_check_matcher" {
description = "The HTTP response codes to indicate a healthy check"
}

variable "ignore_default_action_changes" {
description = "Whether to ignore changes for Load balancer default action (useful when using CodeDeploy Blue/Green deployments to avoid drifts)"
type = bool
default = false
}

variable "alb_access_logs_s3_bucket_force_destroy" {
type = bool
default = false
Expand Down
Loading