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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ALB Advanced Routing rules #8268

merged 39 commits into from Dec 6, 2019


Copy link

@dpiddockcmp dpiddockcmp commented Apr 10, 2019

Community Note

  • Please vote on this pull request by adding a 馃憤 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #8126

Changes proposed in this pull request:

Output from acceptance testing:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSLBListenerRule'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 4 -run=TestAccAWSLBListenerRule -timeout 120m
=== RUN   TestAccAWSLBListenerRule_basic
=== PAUSE TestAccAWSLBListenerRule_basic
=== RUN   TestAccAWSLBListenerRuleBackwardsCompatibility
=== PAUSE TestAccAWSLBListenerRuleBackwardsCompatibility
=== RUN   TestAccAWSLBListenerRule_redirect
=== PAUSE TestAccAWSLBListenerRule_redirect
=== RUN   TestAccAWSLBListenerRule_fixedResponse
=== PAUSE TestAccAWSLBListenerRule_fixedResponse
=== RUN   TestAccAWSLBListenerRule_updateRulePriority
=== PAUSE TestAccAWSLBListenerRule_updateRulePriority
=== RUN   TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew
=== PAUSE TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew
=== RUN   TestAccAWSLBListenerRule_multipleConditionThrowsError
=== PAUSE TestAccAWSLBListenerRule_multipleConditionThrowsError
=== RUN   TestAccAWSLBListenerRule_priority
=== PAUSE TestAccAWSLBListenerRule_priority
=== RUN   TestAccAWSLBListenerRule_cognito
=== PAUSE TestAccAWSLBListenerRule_cognito
=== RUN   TestAccAWSLBListenerRule_oidc
=== PAUSE TestAccAWSLBListenerRule_oidc
=== RUN   TestAccAWSLBListenerRule_Action_Order
=== PAUSE TestAccAWSLBListenerRule_Action_Order
=== RUN   TestAccAWSLBListenerRule_Action_Order_Recreates
=== PAUSE TestAccAWSLBListenerRule_Action_Order_Recreates
=== RUN   TestAccAWSLBListenerRule_conditionAttributesCount
=== PAUSE TestAccAWSLBListenerRule_conditionAttributesCount
=== RUN   TestAccAWSLBListenerRule_conditionHostHeader
=== PAUSE TestAccAWSLBListenerRule_conditionHostHeader
=== RUN   TestAccAWSLBListenerRule_conditionHostHeader_deprecated
=== PAUSE TestAccAWSLBListenerRule_conditionHostHeader_deprecated
=== RUN   TestAccAWSLBListenerRule_conditionHttpHeader
=== PAUSE TestAccAWSLBListenerRule_conditionHttpHeader
=== RUN   TestAccAWSLBListenerRule_conditionHttpHeader_invalid
=== PAUSE TestAccAWSLBListenerRule_conditionHttpHeader_invalid
=== RUN   TestAccAWSLBListenerRule_conditionHttpRequestMethod
=== PAUSE TestAccAWSLBListenerRule_conditionHttpRequestMethod
=== RUN   TestAccAWSLBListenerRule_conditionPathPattern
=== PAUSE TestAccAWSLBListenerRule_conditionPathPattern
=== RUN   TestAccAWSLBListenerRule_conditionPathPattern_deprecated
=== PAUSE TestAccAWSLBListenerRule_conditionPathPattern_deprecated
=== RUN   TestAccAWSLBListenerRule_conditionQueryString
=== PAUSE TestAccAWSLBListenerRule_conditionQueryString
=== RUN   TestAccAWSLBListenerRule_conditionSourceIp
=== PAUSE TestAccAWSLBListenerRule_conditionSourceIp
=== RUN   TestAccAWSLBListenerRule_conditionMultiple
=== PAUSE TestAccAWSLBListenerRule_conditionMultiple
=== CONT  TestAccAWSLBListenerRule_basic
=== CONT  TestAccAWSLBListenerRule_conditionAttributesCount
=== CONT  TestAccAWSLBListenerRule_Action_Order_Recreates
=== CONT  TestAccAWSLBListenerRule_Action_Order
--- PASS: TestAccAWSLBListenerRule_basic (274.83s)
=== CONT  TestAccAWSLBListenerRule_oidc
--- PASS: TestAccAWSLBListenerRule_Action_Order_Recreates (294.26s)
=== CONT  TestAccAWSLBListenerRule_cognito
--- PASS: TestAccAWSLBListenerRule_Action_Order (296.43s)
=== CONT  TestAccAWSLBListenerRule_priority
--- PASS: TestAccAWSLBListenerRule_oidc (272.91s)
=== CONT  TestAccAWSLBListenerRule_multipleConditionThrowsError
--- PASS: TestAccAWSLBListenerRule_multipleConditionThrowsError (2.29s)
=== CONT  TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew
--- PASS: TestAccAWSLBListenerRule_cognito (302.83s)
=== CONT  TestAccAWSLBListenerRule_updateRulePriority
--- PASS: TestAccAWSLBListenerRule_conditionAttributesCount (759.26s)
=== CONT  TestAccAWSLBListenerRule_fixedResponse
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (315.04s)
=== CONT  TestAccAWSLBListenerRule_redirect
--- PASS: TestAccAWSLBListenerRule_priority (640.36s)
=== CONT  TestAccAWSLBListenerRuleBackwardsCompatibility
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (460.25s)
=== CONT  TestAccAWSLBListenerRule_conditionPathPattern
--- PASS: TestAccAWSLBListenerRule_fixedResponse (271.90s)
=== CONT  TestAccAWSLBListenerRule_conditionMultiple
--- PASS: TestAccAWSLBListenerRule_redirect (249.99s)
=== CONT  TestAccAWSLBListenerRule_conditionSourceIp
--- PASS: TestAccAWSLBListenerRuleBackwardsCompatibility (267.98s)
=== CONT  TestAccAWSLBListenerRule_conditionQueryString
--- PASS: TestAccAWSLBListenerRule_conditionPathPattern (270.64s)
=== CONT  TestAccAWSLBListenerRule_conditionPathPattern_deprecated
--- PASS: TestAccAWSLBListenerRule_conditionMultiple (264.14s)
=== CONT  TestAccAWSLBListenerRule_conditionHttpHeader
--- PASS: TestAccAWSLBListenerRule_conditionSourceIp (284.98s)
=== CONT  TestAccAWSLBListenerRule_conditionHttpRequestMethod
--- PASS: TestAccAWSLBListenerRule_conditionQueryString (250.74s)
=== CONT  TestAccAWSLBListenerRule_conditionHttpHeader_invalid
--- PASS: TestAccAWSLBListenerRule_conditionHttpHeader_invalid (2.32s)
=== CONT  TestAccAWSLBListenerRule_conditionHostHeader_deprecated
--- PASS: TestAccAWSLBListenerRule_conditionHttpHeader (251.61s)
=== CONT  TestAccAWSLBListenerRule_conditionHostHeader
--- PASS: TestAccAWSLBListenerRule_conditionPathPattern_deprecated (283.15s)
--- PASS: TestAccAWSLBListenerRule_conditionHostHeader_deprecated (279.90s)
--- PASS: TestAccAWSLBListenerRule_conditionHttpRequestMethod (328.54s)
--- PASS: TestAccAWSLBListenerRule_conditionHostHeader (293.96s)
ok	1840.930s


conditions custom hasher:

  • This is required due to the backwards compatible way that AWS implemented the advanced routing. condition.values and the applicable config block are set when condition.field is host-header or path-pattern.
  • Using a custom hasher means that we can migrate from condition.field and condition.values to config block. Users do not see a Diff on every plan.
  • Can probably be removed on next major version, if desired, after removing condition.field and condition.values attributes.

@ghost ghost added size/XL Managed by automation to categorize the size of a PR. service/elbv2 Issues and PRs that pertain to the elbv2 service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. and removed size/XL Managed by automation to categorize the size of a PR. labels Apr 10, 2019
@dpiddockcmp dpiddockcmp changed the title [WIP] Support ALB Advanced Routing rules Support ALB Advanced Routing rules Apr 12, 2019
Copy link

Hey guys, is there anything I can do to help out here? Is there a target date for release for this?

Copy link

@facundovictor facundovictor left a comment

Choose a reason for hiding this comment

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

The format is acceptable and it would help on future changes if more conditions are added, or more details are required to be included in the specific condition type blocks 馃憤

@bflad do we have an estimate on when this may be reviewed/included for a release?

Copy link

@allanhahn: Any idea when it will be released?

Copy link

@allanhahn any updates on when this feature is going to be release?

Copy link

Any updates on when this feature will be released? This would be really helpful.

CC: @allanhahn

Copy link

Great work - our organization would definitely use this. Can we please get it merged in?

Copy link

mvn-bachhuynh-dn commented May 13, 2019

Really want multi values in a condition:

    field  = "path-pattern"
    values = ["/404", "/assets/*"]

Please add

Copy link

is this going to be merged soon?

Copy link

cemo commented May 21, 2019

@bflad is this somehow missed? This is really crucial for us.

Copy link

Merge it please.

Copy link

The only "workaround" right now is creating null_resource with local-exec provider that calls aws-cli for us. That is horrendous.

Please, merge!

Copy link

pasali commented May 27, 2019

@bflad is there any estimate on when will this PR merged ? We are really waiting for this feature.

Copy link

dud225 commented May 28, 2019

Please @bflad @ewbankkit @nywilken could you take some time to review this PR and comment it if something needs to be changed ?

Copy link

t0klian commented Jun 5, 2019

Hi Everyone! Is there any ETA on review and merge of this PR?

Copy link

Could you please merge this PR? This is an important future for us.

Copy link

@bflad would it be feasible to merge this PR anytime soon? Echoing the sentiment of many commentators in this thread, this addition would help me fully terraform my ALB requirements.

Thank you for your time and effort. Cheers!

@bflad bflad added this to the v2.18.0 milestone Jun 18, 2019
@aeschright aeschright added this to the v2.42.0 milestone Dec 6, 2019
Copy link

@aeschright aeschright left a comment

Choose a reason for hiding this comment

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

Thank you so much! 馃殌 馃帀

--- PASS: TestAccAWSLBListenerRule_conditionQueryString (190.50s)
--- PASS: TestAccAWSLBListenerRule_conditionMultiple (201.69s)
--- PASS: TestAccAWSLBListenerRule_conditionHostHeader_deprecated (202.03s)
--- PASS: TestAccAWSLBListenerRule_changeListenerRuleArnForcesNew (203.73s)
--- PASS: TestAccAWSLBListenerRule_Action_Order_Recreates (204.14s)
--- PASS: TestAccAWSLBListenerRule_fixedResponse (206.32s)
--- PASS: TestAccAWSLBListenerRule_conditionHostHeader (208.64s)
--- PASS: TestAccAWSLBListenerRule_conditionPathPattern (210.67s)
--- PASS: TestAccAWSLBListenerRule_oidc (220.87s)
--- PASS: TestAccAWSLBListenerRule_conditionHttpHeader (226.07s)
--- PASS: TestAccAWSLBListenerRule_conditionPathPattern_deprecated (227.85s)
--- PASS: TestAccAWSLBListenerRule_conditionHttpRequestMethod (238.79s)
--- PASS: TestAccAWSLBListenerRuleBackwardsCompatibility (237.24s)
--- PASS: TestAccAWSLBListenerRule_Action_Order (251.73s)
--- PASS: TestAccAWSLBListenerRule_basic (268.04s)
--- PASS: TestAccAWSLBListenerRule_cognito (299.88s)
--- PASS: TestAccAWSLBListenerRule_redirect (299.05s)
--- PASS: TestAccAWSLBListenerRule_conditionSourceIp (309.71s)
--- PASS: TestAccAWSLBListenerRule_updateRulePriority (273.67s)
--- PASS: TestAccAWSLBListenerRule_priority (389.79s)

@aeschright aeschright merged commit 923cc6e into hashicorp:master Dec 6, 2019
Copy link

This will be included in next week's release, v2.42.0.

aeschright added a commit that referenced this pull request Dec 6, 2019
Copy link
Contributor Author

Wonderful! Thank you @aeschright .

@dpiddockcmp dpiddockcmp deleted the 8126-alb-adv branch Dec 7, 2019
Copy link

@dpiddockcmp @aeschright Thank You for all your work on this! 鉂わ笍

Copy link

dfens1 commented Dec 7, 2019

Awesome! Thank you all!!

Copy link

Wow. Don't believe in it. Finally!

Copy link

Thanks, finally its merged. Was waiting for this from long time.

Copy link

will this have the new weighted target group feature?

Copy link
Contributor Author

Hi @sandangel,

This PR has been merged and no further changes will be made to it.

Target group weighting is a feature of the forward Action. This PR was only adding the advanced routing rules which are part of the Conditions. Weighting would never have been added to this PR even if the feature had been released when this PR was being written.

Issue #10942 has been raised for weighted target groups but no PR so far.

Copy link

ghost commented Dec 13, 2019

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

gil-fugue pushed a commit to LuminalHQ/terraform-provider-aws that referenced this pull request Dec 16, 2019
jaspervdj-luminal pushed a commit to LuminalHQ/terraform-provider-aws that referenced this pull request Dec 31, 2019
mwarkentin added a commit to mwarkentin/terraform-aws-atlantis that referenced this pull request Jan 7, 2020
Upstream change: hashicorp/terraform-provider-aws#8268

Should fix this warning:

Warning: "condition.0.values": [DEPRECATED] use 'host_header' or 'path_pattern' attribute instead

  on .terraform/modules/atlantis/terraform-aws-modules-terraform-aws-atlantis-e1242e3/ line 213, in resource "aws_lb_listener_rule" "redirect_http_to_https":
 213: resource "aws_lb_listener_rule" "redirect_http_to_https" {
curtis-fugue added a commit to LuminalHQ/terraform-provider-aws that referenced this pull request Jan 13, 2020
Copy link

ghost commented Mar 28, 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!

@hashicorp hashicorp locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. provider Pertains to the provider itself, rather than any interaction with AWS. service/elbv2 Issues and PRs that pertain to the elbv2 service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
None yet

Successfully merging this pull request may close these issues.

Support ALB advanced request routing