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

Add support for 'redirect' and 'fixed-response' into lb_listener_rule action type #5344

Closed
julianxhokaxhiu opened this issue Jul 26, 2018 · 12 comments · Fixed by #5430
Closed
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/elbv2 Issues and PRs that pertain to the elbv2 service.
Milestone

Comments

@julianxhokaxhiu
Copy link

julianxhokaxhiu commented Jul 26, 2018

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

AWS added support for ELB Redirects, which is very useful to do an out-of-the-box experience for HTTP to HTTPS redirects without using an EC2 instance with an HTTP server listening in there.

It would be cool if you could add the support for this feature.

New or Affected Resource(s)

  • aws_lb_listener_rule

Potential Terraform Configuration

resource "aws_lb_listener_rule" "static" {
  listener_arn = "${aws_lb_listener.front_end.arn}"
  priority     = 100

  action {
    type = "redirect" # "fixed-response"
    # the rest has to be concepted
  }
}

References

Official announcement: https://aws.amazon.com/about-aws/whats-new/2018/07/elastic-load-balancing-announces-support-for-redirects-and-fixed-responses-for-application-load-balancer/

Documentation:

@julianxhokaxhiu julianxhokaxhiu changed the title Add support for 'redirects' into lb_listener_rule action type Add support for 'redirect' into lb_listener_rule action type Jul 26, 2018
@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/elbv2 Issues and PRs that pertain to the elbv2 service. labels Jul 26, 2018
@jianyuan
Copy link
Contributor

I'm interested to work on this feature.

The aws_lb_listener resource would be affected as well.

As for the potential Terraform configuration, I propose the following:

Redirect

resource "aws_lb_listener" "web_port80" {
  load_balancer_arn = "${aws_lb.web_alb.arn}"
  port              = 80
  protocol          = "HTTP"

  default_action {
    type = "redirect"
    host = "#{host}"
    path = "/#{path}"
    port = "#{port}"
    protocol = "#{protocol}"
    query = "#{query}"
    status_code = "301"
  }
}

Fixed response

resource "aws_lb_listener" "web_port80" {
  load_balancer_arn = "${aws_lb.web_alb.arn}"
  port              = 80
  protocol          = "HTTP"

  default_action {
    type = "fixed-response"
    content_type = "text/plain"
    message_body = "Hello"
    status_code = "200"
  }
}

Config for aws_lb_listener_rule would be similar.

@bflad
Copy link
Contributor

bflad commented Jul 27, 2018

Caution: setting these types of rules outside Terraform will cause the AWS provider to crash in all versions up through the latest (version 1.29.0 as of this writing): #5357

I will be submitting a pull request today to prevent this type of crash across all ALB/NLB resources and we should probably wait on those fixes before adding this feature.

@bflad
Copy link
Contributor

bflad commented Jul 27, 2018

Crash prevention pull request submitted: #5367

@bflad
Copy link
Contributor

bflad commented Jul 27, 2018

The crash fix has been merged into master so it should be okay to work with the code for this on top of those changes. 👍

@jianyuan
Copy link
Contributor

jianyuan commented Jul 30, 2018

We now have 3 different types of actions: "fixed-response", "forward" and "redirect", each with different sets of parameters.

Would it be better if the action config for selected action type is grouped in a block instead of flattening them at the top level?

Examples:

resource "aws_lb_listener_rule" "static" {
  action {
    type = "forward"
    forward {
      target_group_arn = "..."
    }
  }
}
resource "aws_lb_listener_rule" "static" {
  action {
    type = "fixed-response"
    fixed_response {
      content_type = "text/plain"
      message_body = "Hello"
      status_code = "200"
    }
  }
}
resource "aws_lb_listener_rule" "static" {
  action {
    type = "redirect"
    redirect {
	  host = "#{host}"
	  path = "/#{path}"
	  port = "#{port}"
	  protocol = "#{protocol}"
	  query = "#{query}"
	  status_code = "HTTP_301"
    }
  }
}

The Action datatype now groups action type-specific config in a nested object, so doing it this way would be (almost) a 1:1 mapping: https://docs.aws.amazon.com/elasticloadbalancing/latest/APIReference/API_Action.html

Also, I noticed that the status_code for redirect action is prefixed by HTTP_ whereas that for fixed-response action is not:

If this status_code config were to be shared, we must do some kind of validation depending on the action type, or do some kind of value translation in the Terraform provider.

@bflad
Copy link
Contributor

bflad commented Jul 30, 2018

We recommend following the SDK structure in most cases, so separate blocks in this case would be appropriate. This allows us to easily add the argument validation as well as keep the code simpler.

@bacoboy
Copy link

bacoboy commented Aug 1, 2018

Since the scope of this includes fixed-response can we update the title? I was interested in that specifically and took me a bit to find this issue and saw it was already being worked on. Thx.

Additionally, I didn't see anything about adding headers in the API links above so not clear how I'd set something like an HSTS header in a response -- because the ALB in forward mode leaves that to the backing app.

@sklemmer
Copy link

sklemmer commented Aug 9, 2018

Are there any plans to add authentication too?

@bflad
Copy link
Contributor

bflad commented Aug 13, 2018

@sklemmer feature requests for authentication can be found at #4707 and #4711 😄

I'm hoping to review the open pull request for fixed-response/redirect (#5430) sometime this week.

@bflad bflad added this to the v1.33.0 milestone Aug 20, 2018
@bflad
Copy link
Contributor

bflad commented Aug 20, 2018

Support for fixed-response and redirect actions has been merged into master via #5430 and will release with version 1.33.0 of the AWS provider, later this week. 🚀

@bflad
Copy link
Contributor

bflad commented Aug 22, 2018

This has been released in version 1.33.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link

ghost commented Apr 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/elbv2 Issues and PRs that pertain to the elbv2 service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants