hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.74k stars 9.1k forks source link

[Docs]: Clarification needed for aws_lb_listener_rule Documentation on 'Action Block' Argument #31712

Open biagolini opened 1 year ago

biagolini commented 1 year ago

Documentation Link

https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/lb_listener_rule#attributes-reference

Description

The current Terraform documentation for the resource aws_lb_listener_rule, specifically concerning the 'Action Block' argument, requires updates for clarification and accuracy. The definition of the 'forward' attribute as stated in the documentation is misleading and has resulted in confusion when implementing code.

Currently, the documentation states:

"forward - (Optional) Information for creating an action that distributes requests among one or more target groups. Specify only if type is forward. If you specify both forward block and target_group_arn attribute, you can specify only one target group using forward and it must be the same target group specified in target_group_arn."

However, based on practical implementation and testing, this does not seem to be entirely accurate.

The first two statements seem correct; it is the third statement that is misleading. The suggestion is that if both forward block and target_group_arn attribute are specified, only one target group can be specified using forward, and it must be the same target group specified in target_group_arn.

While attempting to follow this suggestion in the current documentation, an "Insufficient target_group blocks" error is generated, indicating that at least 2 "target_group" blocks are required when using a forward block.

The documentation should instead explain that when specifying a single target group, one should use target_group_arn within the action block directly, and when aiming to distribute between multiple target groups with weights, use forward block with multiple target_group blocks inside, specifying the ARN and weight for each target group.

Examples of the correct implementations are given in the initial issue description above. It is requested that the current documentation be revised for clarity and precision to avoid future confusion and errors.

# See correct mode to specify only one target group
resource "aws_lb_listener_rule" "forward_single" {
  listener_arn = aws_lb_listener.lb_listener_single.arn
  priority     = 100
  action {
    type             = "forward"
    target_group_arn = aws_lb_target_group.target_group_b.arn
  }
  condition {
    path_pattern {
      values = ["/b/*"]
    }
  }
}
# See correct mode to use weighted forward
resource "aws_lb_listener_rule" "forward_weight" {
  listener_arn = aws_lb_listener.lb_listener_single.arn
  priority     = 200
  action {
    type = "forward"

    forward {
      target_group {
        arn    = aws_lb_target_group.target_group_c.arn
        weight = 80
      }
      target_group {
        arn    = aws_lb_target_group.target_group_d.arn
        weight = 20
      }
    }
  }
  condition {
    path_pattern {
      values = ["/c/*", "/d/*"]
    }
  }
}
# See how documentation suggest to make this configuration.
resource "aws_lb_listener_rule" "forward_incorrect_documentation" {
  listener_arn = aws_lb_listener.lb_listener_single.arn 
  priority     = 300
  action {
    type = "forward"
    target_group_arn = aws_lb_target_group.target_group_e.arn
    forward {
      target_group {
        arn    = aws_lb_target_group.target_group_e.arn
        weight = 100
      }
    }
  }
  condition {
    path_pattern {
      values = ["/e/*"]
    }
  }
}
# # Error log generated with this config
# ╷
# │ Error: Insufficient target_group blocks
# │
# │   on aws_lb_listener.tf line 81, in resource "aws_lb_listener_rule" "forward_incorrect_documentation":
# │   81:     forward {
# │
# │ At least 2 "target_group" blocks are required.

References

No response

Would you like to implement a fix?

Yes

github-actions[bot] commented 1 year ago

Community Note

Voting for Prioritization

Volunteering to Work on This Issue

biagolini commented 1 year ago

New forward description suggestion: "forward - (Optional) This argument is used to distribute requests among target groups and is only specified if action type is 'forward'. For a single target group, specify the target_group_arn attribute directly in the action block, without using a forward block. For multiple target groups, use a forward block with multiple target_group blocks, each containing an ARN and weight. Do not specify target_group_arn in the action block in this case. Specifying the same target group in target_group_arn and forward block simultaneously is not recommended and will result in an error."

justinretzolk commented 1 year ago

Hey @biagolini 👋 Thank you for taking the time to raise this, for testing this out, and for including such a thorough description! I'd like to raise a pull request to update the description as you noted, but wanted to get a bit of clarification before I do. In following the linked issue and reading through it, one of the things that caught my eye was this comment indicating that this doesn't work when you need to pass stickiness as well. Is this something that you have insight into, before I dive down that rabbit hole?

craig-reeson commented 1 year ago

trying to add stickiness seems to break working code fwiw, I'm running TF 1.4.6 and AWS provider 5.4.0

jbcorreia commented 1 year ago

I think correct design would be to use either fixed forward or dynamic, and the dynamic should support only 1 too.

This is quite useful when you're trying to build modules that have different configurations and avoids creating extra resources and repeating code.

Hope this can be considered too.

BlinkyStitt commented 1 year ago

If the docs could also include how to add stickiness that would be great. Right now, terraform says it is going to apply the rules, but Amazon doesn't actually show them:

resource "aws_lb_listener" "haproxy-https-to-strip-staging" {
  load_balancer_arn = aws_lb.haproxy-staging.arn
  port              = 443
  protocol          = "HTTPS"
  certificate_arn = var.wildcard_ssl_certificate_arn

  default_action {
    type = "forward"

    forward {
      target_group {
        arn = aws_lb_target_group.haproxy-stripped-https-staging.arn
      }
      stickiness {
        enabled  = true
        duration = 600
      }
    }
  }
}
  # module.haproxy-asg-us-east-2.aws_lb_listener.haproxy-https-to-strip-staging will be updated in-place
  ~ resource "aws_lb_listener" "haproxy-https-to-strip-staging" {
        id                = "arn:aws:elasticloadbalancing:us-east-2:322555996595:listener/app/haproxy-staging-alb/2eac9de486863dcd/9a207311652d2b84"
        # (7 unchanged attributes hidden)

      ~ default_action {
          - target_group_arn = "arn:aws:elasticloadbalancing:us-east-2:322555996595:targetgroup/haproxy-stripped-https-staging/27c6949d5bcd4018" -> null
            # (2 unchanged attributes hidden)

          + forward {
              + stickiness {
                  + duration = 600
                  + enabled  = true
                }
              + target_group {
                  + arn    = "arn:aws:elasticloadbalancing:us-east-2:322555996595:targetgroup/haproxy-stripped-https-staging/27c6949d5bcd4018"
                  + weight = 1
                }
            }
        }
    }

Even if I change it in the console to match what terraform says it will do, it undoes the stickiness when it runs.