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.

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

Fixes #134 - Does not create http_forward when aws_lb_target_group.default is disabled #149

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

spazm
Copy link

@spazm spazm commented Oct 25, 2023

What

Fixes #134 - Does not create http_forward when aws_lb_target_group.default is disabled

  • disables aws_lb_listener.http_forward when default_target_group is not enabled
  • target_group is required when type is 'redirect'

Why

See #134. Fixes this Validation error when default_target_group_enabled == 0 :

module.alb.aws_lb_listener.http_forward[0]: Creating...
╷
│ Error: creating ELBv2 Listener
(arn:aws:elasticloadbalancing:...:...:loadbalancer/...):
ValidationError: A target group ARN must be specified
│       status code: 400, request id:
7cf9d727-fc77-4d32-a160-deadbeefcafe
│
│   with module.alb.aws_lb_listener.http_forward[0],
│   on .terraform/modules/alb/main.tf line 150, in resource
"aws_lb_listener" "http_forward":
│  150: resource "aws_lb_listener" "http_forward" {

references

closes #134

@spazm spazm requested review from a team as code owners October 25, 2023 14:13
@spazm spazm requested review from srhopkins and woz5999 and removed request for a team October 25, 2023 14:13
@spazm spazm force-pushed the 134_disable_arn_when_default_target_group_disabled branch 2 times, most recently from 8a9eb28 to 8cb422d Compare October 25, 2023 14:27
…fault is disabled

* disables aws_lb_listener.http_forward when default_target_group is not enabled
* target_group is required when type is 'redirect'

Fixes this Validation error when default_target_group_enabled == 0 :

```
module.alb.aws_lb_listener.http_forward[0]: Creating...
╷
│ Error: creating ELBv2 Listener
(arn:aws:elasticloadbalancing:...:...:loadbalancer/...):
ValidationError: A target group ARN must be specified
│       status code: 400, request id:
7cf9d727-fc77-4d32-a160-cbd175e16e20
│
│   with module.alb.aws_lb_listener.http_forward[0],
│   on .terraform/modules/alb/main.tf line 150, in resource
"aws_lb_listener" "http_forward":
│  150: resource "aws_lb_listener" "http_forward" {

```
@spazm spazm force-pushed the 134_disable_arn_when_default_target_group_disabled branch from 8cb422d to d1a69cb Compare November 2, 2023 06:03
@spazm spazm changed the title Fixes #134 - Provides guard against referencing disabled default aws_lb_target_group Fixes #134 - Does not create http_forward when aws_lb_target_group.default is disabled Nov 2, 2023
@spazm
Copy link
Author

spazm commented Nov 2, 2023

Edited to remove unnecessary guard rails for the other aws_lb_listener blocks; they work fine when the default target group is disabled.

This leaves a small, clear change. Please reply with any questions and feel welcome to reformat or cherry-pick as necessary.

@Gowiem
Copy link
Member

Gowiem commented Nov 15, 2023

/terratest

@Gowiem Gowiem self-requested a review November 15, 2023 02:11
@hans-d
Copy link

hans-d commented Mar 2, 2024

/terratest

@hans-d
Copy link

hans-d commented Mar 7, 2024

@spazm Thanks. For the failing test, can you do

README.md is outdated. Please run the following commands locally and push the file:
  make init
  make readme

@hans-d hans-d added the stale This PR has gone stale label Mar 8, 2024
Copy link

mergify bot commented Mar 9, 2024

Thanks @spazm for creating this pull request!

A maintainer will review your changes shortly. Please don't be discouraged if it takes a while.

While you wait, make sure to review our contributor guidelines.

Tip

Need help or want to ask for a PR review to be expedited?

Join us on Slack in the #pr-reviews channel.

@mergify mergify bot added triage Needs triage and removed stale This PR has gone stale labels Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A target group ARN must be specified when disabling the default target group
4 participants