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 and lb_listener_rule #5430

Merged
merged 13 commits into from
Aug 20, 2018
Merged

Add support for 'redirect' and 'fixed-response' into lb_listener and lb_listener_rule #5430

merged 13 commits into from
Aug 20, 2018

Conversation

jianyuan
Copy link
Contributor

@jianyuan jianyuan commented Aug 2, 2018

Fixes #5344.

Changes proposed in this pull request:

  • Add support for redirect and fixed-response into lb_listener.
  • Add support for redirect and fixed-response into lb_listener_rule.
  • Updated docs.

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSLBListener\(Rule\)?'

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSLBListener\(Rule\)? -timeout 120m
=== RUN   TestAccAWSLBListenerRule_basic
--- PASS: TestAccAWSLBListenerRule_basic (223.11s)
=== RUN   TestAccAWSLBListenerRuleBackwardsCompatibility
--- PASS: TestAccAWSLBListenerRuleBackwardsCompatibility (250.42s)
=== RUN   TestAccAWSLBListenerRule_redirect
--- PASS: TestAccAWSLBListenerRule_redirect (234.50s)
=== RUN   TestAccAWSLBListenerRule_fixedResponse
--- PASS: TestAccAWSLBListenerRule_fixedResponse (221.45s)
=== RUN   TestAccAWSLBListenerRule_updateRulePriority
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (338.63s)
=== RUN   TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (265.98s)
=== RUN   TestAccAWSLBListenerRule_multipleConditionThrowsError
--- PASS: TestAccAWSLBListenerRule_multipleConditionThrowsError (1.53s)
=== RUN   TestAccAWSLBListenerRule_priority
--- PASS: TestAccAWSLBListenerRule_priority (429.05s)
=== RUN   TestAccAWSLBListener_basic
--- PASS: TestAccAWSLBListener_basic (233.51s)
=== RUN   TestAccAWSLBListenerBackwardsCompatibility
--- PASS: TestAccAWSLBListenerBackwardsCompatibility (230.56s)
=== RUN   TestAccAWSLBListener_https
--- PASS: TestAccAWSLBListener_https (243.43s)
=== RUN   TestAccAWSLBListener_redirect
--- PASS: TestAccAWSLBListener_redirect (232.45s)
=== RUN   TestAccAWSLBListener_fixedResponse
--- PASS: TestAccAWSLBListener_fixedResponse (236.17s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	3140.836s

@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Aug 2, 2018
@jianyuan jianyuan changed the title Aws lb listener rule redirect fixed response Add support for 'redirect' and 'fixed-response' into lb_listener and lb_listener_rule Aug 2, 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 Aug 3, 2018
@melo
Copy link

melo commented Aug 6, 2018

Any idea when this PR will have a milestone associated?

Thank you

@bflad
Copy link
Contributor

bflad commented Aug 6, 2018

@melo it will generally receive a milestone right before merge unless its an important bug/crash fix

@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Aug 6, 2018
@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Aug 7, 2018
@ghost ghost mentioned this pull request Aug 8, 2018
Copy link

@danielpereira-hotmart danielpereira-hotmart left a comment

Choose a reason for hiding this comment

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

ok!

@bacoboy
Copy link

bacoboy commented Aug 13, 2018

Unfortunately AWS doesn't support custom headers in the fixed-response (for like an HSTS header), but AWS support said they'd take it under advisement. Just an FYI for the group...

Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Nice work, @jianyuan! LGTM 🚀

(Test failure unrelated)

Tests failed: 1 (1 new), passed: 12
--- PASS: TestAccAWSLBListenerRule_multipleConditionThrowsError (1.50s)
--- PASS: TestAccAWSLBListenerRule_basic (174.19s)
--- PASS: TestAccAWSLBListener_basic (175.31s)
--- PASS: TestAccAWSLBListenerRuleBackwardsCompatibility (190.57s)
--- PASS: TestAccAWSLBListenerRule_fixedResponse (193.58s)
--- PASS: TestAccAWSLBListener_redirect (193.93s)
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (198.41s)
--- PASS: TestAccAWSLBListener_https (199.13s)
--- PASS: TestAccAWSLBListenerRule_redirect (200.23s)
--- PASS: TestAccAWSLBListener_fixedResponse (200.61s)
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (203.08s)
--- PASS: TestAccAWSLBListenerBackwardsCompatibility (217.40s)
--- FAIL: TestAccAWSLBListenerRule_priority (238.18s)

@bflad bflad added this to the v1.33.0 milestone Aug 20, 2018
@bflad bflad merged commit 822b107 into hashicorp:master Aug 20, 2018
bflad added a commit that referenced this pull request Aug 20, 2018
@tdmalone
Copy link
Contributor

Thanks so much @jianyuan!

@jianyuan jianyuan deleted the aws-lb-listener-rule-redirect-fixed-response branch August 21, 2018 11:11
@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.

@stszap
Copy link
Contributor

stszap commented Aug 23, 2018

Thanks to all who contributed to add this feature, it is awesome! I can specify path = "#{path}" and terrafrom tries to create a listener but gets an error from aws:

* aws_lb_listener.external_http: Error creating LB Listener: InvalidLoadBalancerAction: The Path parameter must be a valid path, should start with a '/', and may contain up to one of each of these placeholders: '#{path}', '#{host}', '#{port}'.
	status code: 400, request id: febe9779-a6a8-11e8-9d4a-818ad1227d6

It works withpath = "/#{path}"but maybe there should be an additional check for leading "/" just to fail faster?

@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. size/XXL Managed by automation to categorize the size of a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for 'redirect' and 'fixed-response' into lb_listener_rule action type
7 participants