feat: redirect default action for https listener

[oycyc]

what

• Adds the ability for a HTTPS listener to have an default action of "redirect" in addition to the current two existing "fixed-response" and "target group".

• Create a new variable to support this with default of null.

• Ran make readme

why

There are use cases when the default action to be redirect that we want for an ALB listener if it doesn't match any rules to redirect. See image below for the action in AWS console.

This would be good to have in the module, otherwise when there is a case that this needs to be configured, this specific resource has to be stripped out.

[gberenice]

/terratest

[gberenice]

@oycyc looks good, I added a couple of suggestions.
Also, could you please fix the test following this example cloudposse/terraform-aws-ecs-alb-service-task#252?
Thanks in advance 🙌

[gberenice]

/terratest

[gberenice]

/terratest

[gberenice]

Thanks, @oycyc 👍

[github-actions]

These changes were released in v1.11.2.

[oycyc]

Awesome, thanks!

[mschfh]

JFYI, it seems the readme was not updated after the latest commit that added the defaults:

-| <a name="input_listener_https_redirect"></a> [listener\_https\_redirect](#input\_listener\_https\_redirect) | Have the HTTPS listener return a redirect response for the default action. | <pre>object({<br/>    host        = optional(string)<br/>    path        = optional(string)<br/>    port        = optional(string)<br/>    protocol    = optional(string)<br/>    query       = optional(string)<br/>    status_code = optional(string)<br/>  })</pre> | `null` | no |
+| <a name="input_listener_https_redirect"></a> [listener\_https\_redirect](#input\_listener\_https\_redirect) | Have the HTTPS listener return a redirect response for the default action. | <pre>object({<br/>    host        = optional(string)<br/>    path        = optional(string)<br/>    port        = optional(string)<br/>    protocol    = optional(string)<br/>    query       = optional(string)<br/>    status_code = string<br/>  })</pre> | <pre>{<br/>  "host": null,<br/>  "path": null,<br/>  "port": null,<br/>  "protocol": null,<br/>  "query": null,<br/>  "status_code": "HTTP_301"<br/>}</pre> | no |

IIRC there was a CI check for this in the past, was it removed?

[oycyc]

JFYI, it seems the readme was not updated after the latest commit that added the defaults:

-| <a name="input_listener_https_redirect"></a> [listener\_https\_redirect](#input\_listener\_https\_redirect) | Have the HTTPS listener return a redirect response for the default action. | <pre>object({<br/>    host        = optional(string)<br/>    path        = optional(string)<br/>    port        = optional(string)<br/>    protocol    = optional(string)<br/>    query       = optional(string)<br/>    status_code = optional(string)<br/>  })</pre> | `null` | no |
+| <a name="input_listener_https_redirect"></a> [listener\_https\_redirect](#input\_listener\_https\_redirect) | Have the HTTPS listener return a redirect response for the default action. | <pre>object({<br/>    host        = optional(string)<br/>    path        = optional(string)<br/>    port        = optional(string)<br/>    protocol    = optional(string)<br/>    query       = optional(string)<br/>    status_code = string<br/>  })</pre> | <pre>{<br/>  "host": null,<br/>  "path": null,<br/>  "port": null,<br/>  "protocol": null,<br/>  "query": null,<br/>  "status_code": "HTTP_301"<br/>}</pre> | no |

IIRC there was a CI check for this in the past, was it removed?

Thanks! Somehow got missed. I'm creating a new PR just to update the README. #188

[vonZeppelin]

Looks like #184 can be closed now, thank you for implementing this!

[oycyc]

Looks like #184 can be closed now, thank you for implementing this!

Ah didn't see this issue. Thanks for linking this!

[mschfh]

It seems this adds a redirect by default, as the default for the variable is an object, not null.

terraform-aws-alb/main.tf

Lines 231 to 232 in cb8fa65

	dynamic "redirect" {
	for_each = var.listener_https_redirect != null ? [var.listener_https_redirect] : []

terraform-aws-alb/variables.tf

Lines 292 to 311 in cb8fa65

	variable "listener_https_redirect" {
	description = "Have the HTTPS listener return a redirect response for the default action."
	# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener#status_code-2
	type = object({
	host = optional(string)
	path = optional(string)
	port = optional(string)
	protocol = optional(string)
	query = optional(string)
	status_code = string
	})
	default = {
	host = null
	path = null
	port = null
	protocol = null
	query = null
	status_code = "HTTP_301"
	}
	}

Plan:

      ~ default_action {
          ~ type             = "forward" -> "redirect"
            # (2 unchanged attributes hidden)

          + redirect {
              + host        = "#{host}"
              + path        = "/#{path}"
              + port        = "#{port}"
              + protocol    = "#{protocol}"
              + query       = "#{query}"
              + status_code = "HTTP_301"
            }
        }

Explicitly passing listener_https_redirect = null to the module does prevent this change, please update the default or adjust the for_each.

[oycyc]

It seems this adds a redirect by default, as the default for the variable is an object, not null.

terraform-aws-alb/main.tf

Lines 231 to 232 in cb8fa65

	dynamic "redirect" {
	for_each = var.listener_https_redirect != null ? [var.listener_https_redirect] : []

terraform-aws-alb/variables.tf

Lines 292 to 311 in cb8fa65

	variable "listener_https_redirect" {
	description = "Have the HTTPS listener return a redirect response for the default action."
	# https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener#status_code-2
	type = object({
	host = optional(string)
	path = optional(string)
	port = optional(string)
	protocol = optional(string)
	query = optional(string)
	status_code = string
	})
	default = {
	host = null
	path = null
	port = null
	protocol = null
	query = null
	status_code = "HTTP_301"
	}
	}

Plan:

      ~ default_action {
          ~ type             = "forward" -> "redirect"
            # (2 unchanged attributes hidden)

          + redirect {
              + host        = "#{host}"
              + path        = "/#{path}"
              + port        = "#{port}"
              + protocol    = "#{protocol}"
              + query       = "#{query}"
              + status_code = "HTTP_301"
            }
        }

Explicitly passing listener_https_redirect = null to the module does prevent this change, please update the default or adjust the for_each.

@mschfh thanks for bringing this up, this would be quite an issue since it'll affect the default. Made a PR for this: #190

Really appreciate you putting this here!!