Add protocol_version to lb target groups #16132

fivehole68 · 2020-11-10T21:21:26Z

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" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #15929

Release note for CHANGELOG:

Add protocol version support to aws_lb_target_group

Output from acceptance testing:

make testacc TESTARGS='-run=TestAccAWSLBTargetGroup' ACCTEST_PARALLELISM='2'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSLBTargetGroup -timeout 120m
=== RUN   TestAccAWSLBTargetGroupAttachment_basic
=== PAUSE TestAccAWSLBTargetGroupAttachment_basic
=== RUN   TestAccAWSLBTargetGroupAttachment_disappears
=== PAUSE TestAccAWSLBTargetGroupAttachment_disappears
=== RUN   TestAccAWSLBTargetGroupAttachment_BackwardsCompatibility
=== PAUSE TestAccAWSLBTargetGroupAttachment_BackwardsCompatibility
=== RUN   TestAccAWSLBTargetGroupAttachment_Port
=== PAUSE TestAccAWSLBTargetGroupAttachment_Port
=== RUN   TestAccAWSLBTargetGroupAttachment_ipAddress
=== PAUSE TestAccAWSLBTargetGroupAttachment_ipAddress
=== RUN   TestAccAWSLBTargetGroupAttachment_lambda
=== PAUSE TestAccAWSLBTargetGroupAttachment_lambda
=== RUN   TestAccAWSLBTargetGroup_basic
=== PAUSE TestAccAWSLBTargetGroup_basic
=== RUN   TestAccAWSLBTargetGroup_basicUdp
=== PAUSE TestAccAWSLBTargetGroup_basicUdp
=== RUN   TestAccAWSLBTargetGroup_withoutHealthcheck
=== PAUSE TestAccAWSLBTargetGroup_withoutHealthcheck
=== RUN   TestAccAWSLBTargetGroup_networkLB_TargetGroup
=== PAUSE TestAccAWSLBTargetGroup_networkLB_TargetGroup
=== RUN   TestAccAWSLBTargetGroup_Protocol_Tcp_HealthCheck_Protocol
=== PAUSE TestAccAWSLBTargetGroup_Protocol_Tcp_HealthCheck_Protocol
=== RUN   TestAccAWSLBTargetGroup_Protocol_Tls
=== PAUSE TestAccAWSLBTargetGroup_Protocol_Tls
=== RUN   TestAccAWSLBTargetGroup_ProtocolVersion_GRPC_HealthCheck
=== PAUSE TestAccAWSLBTargetGroup_ProtocolVersion_GRPC_HealthCheck
=== RUN   TestAccAWSLBTargetGroup_ProtocolVersion_HTTP_GRPC_Update
=== PAUSE TestAccAWSLBTargetGroup_ProtocolVersion_HTTP_GRPC_Update
=== RUN   TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy
=== PAUSE TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy
=== RUN   TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck
=== PAUSE TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck
=== RUN   TestAccAWSLBTargetGroup_BackwardsCompatibility
=== PAUSE TestAccAWSLBTargetGroup_BackwardsCompatibility
=== RUN   TestAccAWSLBTargetGroup_namePrefix
=== PAUSE TestAccAWSLBTargetGroup_namePrefix
=== RUN   TestAccAWSLBTargetGroup_generatedName
=== PAUSE TestAccAWSLBTargetGroup_generatedName
=== RUN   TestAccAWSLBTargetGroup_changeNameForceNew
=== PAUSE TestAccAWSLBTargetGroup_changeNameForceNew
=== RUN   TestAccAWSLBTargetGroup_changeProtocolForceNew
=== PAUSE TestAccAWSLBTargetGroup_changeProtocolForceNew
=== RUN   TestAccAWSLBTargetGroup_changePortForceNew
=== PAUSE TestAccAWSLBTargetGroup_changePortForceNew
=== RUN   TestAccAWSLBTargetGroup_changeVpcForceNew
=== PAUSE TestAccAWSLBTargetGroup_changeVpcForceNew
=== RUN   TestAccAWSLBTargetGroup_tags
=== PAUSE TestAccAWSLBTargetGroup_tags
=== RUN   TestAccAWSLBTargetGroup_enableHealthCheck
=== PAUSE TestAccAWSLBTargetGroup_enableHealthCheck
=== RUN   TestAccAWSLBTargetGroup_updateHealthCheck
=== PAUSE TestAccAWSLBTargetGroup_updateHealthCheck
=== RUN   TestAccAWSLBTargetGroup_updateSticknessEnabled
=== PAUSE TestAccAWSLBTargetGroup_updateSticknessEnabled
=== RUN   TestAccAWSLBTargetGroup_defaults_application
=== PAUSE TestAccAWSLBTargetGroup_defaults_application
=== RUN   TestAccAWSLBTargetGroup_defaults_network
=== PAUSE TestAccAWSLBTargetGroup_defaults_network
=== RUN   TestAccAWSLBTargetGroup_stickinessDefaultNLB
=== PAUSE TestAccAWSLBTargetGroup_stickinessDefaultNLB
=== RUN   TestAccAWSLBTargetGroup_stickinessDefaultALB
=== PAUSE TestAccAWSLBTargetGroup_stickinessDefaultALB
=== RUN   TestAccAWSLBTargetGroup_stickinessValidNLB
=== PAUSE TestAccAWSLBTargetGroup_stickinessValidNLB
=== RUN   TestAccAWSLBTargetGroup_stickinessValidALB
=== PAUSE TestAccAWSLBTargetGroup_stickinessValidALB
=== RUN   TestAccAWSLBTargetGroup_stickinessInvalidNLB
=== PAUSE TestAccAWSLBTargetGroup_stickinessInvalidNLB
=== RUN   TestAccAWSLBTargetGroup_stickinessInvalidALB
=== PAUSE TestAccAWSLBTargetGroup_stickinessInvalidALB
=== CONT  TestAccAWSLBTargetGroupAttachment_basic
=== CONT  TestAccAWSLBTargetGroup_stickinessValidALB
--- PASS: TestAccAWSLBTargetGroup_stickinessValidALB (45.40s)
=== CONT  TestAccAWSLBTargetGroup_stickinessInvalidALB
--- PASS: TestAccAWSLBTargetGroup_stickinessInvalidALB (41.07s)
=== CONT  TestAccAWSLBTargetGroup_stickinessInvalidNLB
--- PASS: TestAccAWSLBTargetGroupAttachment_basic (89.69s)
=== CONT  TestAccAWSLBTargetGroup_BackwardsCompatibility
--- PASS: TestAccAWSLBTargetGroup_BackwardsCompatibility (24.48s)
=== CONT  TestAccAWSLBTargetGroup_stickinessValidNLB
--- PASS: TestAccAWSLBTargetGroup_stickinessInvalidNLB (41.66s)
=== CONT  TestAccAWSLBTargetGroup_stickinessDefaultALB
--- PASS: TestAccAWSLBTargetGroup_stickinessDefaultALB (24.15s)
=== CONT  TestAccAWSLBTargetGroup_stickinessDefaultNLB
--- PASS: TestAccAWSLBTargetGroup_stickinessValidNLB (89.70s)
=== CONT  TestAccAWSLBTargetGroup_defaults_network
--- PASS: TestAccAWSLBTargetGroup_stickinessDefaultNLB (68.40s)
=== CONT  TestAccAWSLBTargetGroup_defaults_application
--- PASS: TestAccAWSLBTargetGroup_defaults_network (31.83s)
=== CONT  TestAccAWSLBTargetGroup_updateSticknessEnabled
--- PASS: TestAccAWSLBTargetGroup_defaults_application (24.23s)
=== CONT  TestAccAWSLBTargetGroup_updateHealthCheck
--- PASS: TestAccAWSLBTargetGroup_updateHealthCheck (47.65s)
=== CONT  TestAccAWSLBTargetGroup_enableHealthCheck
--- PASS: TestAccAWSLBTargetGroup_updateSticknessEnabled (65.86s)
=== CONT  TestAccAWSLBTargetGroup_tags
--- PASS: TestAccAWSLBTargetGroup_enableHealthCheck (31.86s)
=== CONT  TestAccAWSLBTargetGroup_changeNameForceNew
--- PASS: TestAccAWSLBTargetGroup_tags (67.55s)
=== CONT  TestAccAWSLBTargetGroup_generatedName
--- PASS: TestAccAWSLBTargetGroup_changeNameForceNew (47.15s)
=== CONT  TestAccAWSLBTargetGroup_namePrefix
--- PASS: TestAccAWSLBTargetGroup_generatedName (23.89s)
=== CONT  TestAccAWSLBTargetGroup_withoutHealthcheck
--- PASS: TestAccAWSLBTargetGroup_namePrefix (23.71s)
=== CONT  TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck
--- PASS: TestAccAWSLBTargetGroup_withoutHealthcheck (16.96s)
=== CONT  TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy
--- PASS: TestAccAWSLBTargetGroup_TCP_HTTPHealthCheck (45.09s)
=== CONT  TestAccAWSLBTargetGroup_ProtocolVersion_HTTP_GRPC_Update
--- PASS: TestAccAWSLBTargetGroup_networkLB_TargetGroupWithProxy (44.74s)
=== CONT  TestAccAWSLBTargetGroup_ProtocolVersion_GRPC_HealthCheck
--- PASS: TestAccAWSLBTargetGroup_ProtocolVersion_GRPC_HealthCheck (24.55s)
=== CONT  TestAccAWSLBTargetGroup_Protocol_Tls
--- PASS: TestAccAWSLBTargetGroup_ProtocolVersion_HTTP_GRPC_Update (40.97s)
=== CONT  TestAccAWSLBTargetGroup_Protocol_Tcp_HealthCheck_Protocol
--- PASS: TestAccAWSLBTargetGroup_Protocol_Tls (20.37s)
=== CONT  TestAccAWSLBTargetGroup_changeProtocolForceNew
--- PASS: TestAccAWSLBTargetGroup_Protocol_Tcp_HealthCheck_Protocol (48.13s)
=== CONT  TestAccAWSLBTargetGroup_changeVpcForceNew
--- PASS: TestAccAWSLBTargetGroup_changeProtocolForceNew (48.01s)
=== CONT  TestAccAWSLBTargetGroup_changePortForceNew
--- PASS: TestAccAWSLBTargetGroup_changeVpcForceNew (47.32s)
=== CONT  TestAccAWSLBTargetGroup_networkLB_TargetGroup
--- PASS: TestAccAWSLBTargetGroup_changePortForceNew (47.30s)
=== CONT  TestAccAWSLBTargetGroupAttachment_ipAddress
--- PASS: TestAccAWSLBTargetGroup_networkLB_TargetGroup (78.03s)
=== CONT  TestAccAWSLBTargetGroup_basic
--- PASS: TestAccAWSLBTargetGroup_basic (24.75s)
=== CONT  TestAccAWSLBTargetGroup_basicUdp
--- PASS: TestAccAWSLBTargetGroup_basicUdp (24.96s)
=== CONT  TestAccAWSLBTargetGroupAttachment_BackwardsCompatibility
--- PASS: TestAccAWSLBTargetGroupAttachment_ipAddress (187.03s)
=== CONT  TestAccAWSLBTargetGroupAttachment_lambda
--- PASS: TestAccAWSLBTargetGroupAttachment_BackwardsCompatibility (80.56s)
=== CONT  TestAccAWSLBTargetGroupAttachment_Port
--- PASS: TestAccAWSLBTargetGroupAttachment_lambda (36.34s)
=== CONT  TestAccAWSLBTargetGroupAttachment_disappears
--- PASS: TestAccAWSLBTargetGroupAttachment_disappears (90.09s)
--- PASS: TestAccAWSLBTargetGroupAttachment_Port (203.65s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	990.329s
...

github-actions · 2020-11-10T21:21:59Z

Thank you for your contribution! 🚀

Please note that typically Go dependency changes are handled in this repository by dependabot or the maintainers. This is to prevent pull request merge conflicts and further delay reviews of contributions. Remove any changes to the go.mod or go.sum files and commit them into this pull request.

Additional details:

Check open pull requests with the dependencies label to view other dependency updates.
If this pull request includes an update the AWS Go SDK (or any other dependency) version, only updates submitted via dependabot will be merged. This pull request will need to remove these changes and will need to be rebased after the existing dependency update via dependabot has been merged for this pull request to be reviewed.
If this pull request is for supporting a new AWS service:
- Ensure the new AWS service changes are following the Contributing Guide section on new services, in particular that the dependency addition and initial provider support are in a separate pull request from other changes (e.g. new resources). Contributions not following this item will not be reviewed until the changes are split.
- If this pull request is already a separate pull request from the above item, you can ignore this message.

kevincantu · 2020-11-25T07:59:18Z

aws/resource_aws_lb_target_group_test.go

+  name     = "%s"
+  port     = 80
+  protocol = "HTTP"
+  protocol_version = "GRPC"
+  vpc_id   = aws_vpc.test2.id


I bet the terrafmt error is due to the spacing here (the = signs aren't all in line).

Looks like that is correct I fixed it.

kevincantu · 2020-11-25T08:00:32Z

tools/go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/client9/misspell v0.3.4
 	github.com/golangci/golangci-lint v1.32.2
 	github.com/katbyte/terrafmt v0.2.1-0.20200913185704-5ff4421407b4
+	github.com/pavius/impi v0.0.3 // indirect


Whether this came in by accident or from the merge, it seems worth rebasing and taking this out.

I can help you with how to rebase if you want a second pair of eyes!

I think all of go.mod files were mistakes. At the time it was required because I needed the latest aws-go lib. I attempted to revert it but I don't know if I did it correctly. If it isn't I would welcome some help.

Hmm, you've built a little merge-history puzzle!

Using an alias I have defined:

% git la -8 * 6bd6a2037 <Mike> (HEAD, fivehole/f-lb-target-group-protocol-version) Merge branch 'f-lb-target-group-protocol-version' of https://github.com/fivehole68/terraform-provider-aws into f-lb-target-group-protocol-version [ae448c373 b5065c049] |\ | * b5065c049 <Mike> fix terrafmt errors [e93c7ae6b] | * e93c7ae6b <fivehole68> Merge branch 'master' into f-lb-target-group-protocol-version [c76539c8c f66074baa] | |\ | * | c76539c8c <Mike> Remove comment [484151258] | * | 484151258 <Mike> Add support for protocol version to target groups [b1d2e1dbc] * | | ae448c373 <Mike> Revert go files [74d6b7c9d] * | | 74d6b7c9d <Mike> Add support for protocol version to target groups [f66074baa] | |/ |/| * | f66074baa <Dirk Avery> Update CHANGELOG.md [2713c9186]

I know I mentioned rebasing, but probably the easier next step is to just merge the latest master into your branch, and then overwrite a couple lingering go.mod and go.sum files.

Something like this might do the trick:

# look at your branch git checkout f-lb-target-group-protocol-version git log --decorate --graph --oneline -10 git diff --stat hashicorp/master f-lb-target-group-protocol-version # fetch and merge in the latest upstream git remote -v # below I assume Hashicorp's remote is named hashicorp git fetch hashicorp git merge hashicorp/master # look at your branch again git log --decorate --graph --oneline -10 git diff --stat hashicorp/master f-lb-target-group-protocol-version # use checkout to overwrite a couple unintentional differences git checkout hashicorp/master tools/go.mod git checkout hashicorp/master tools/go.sum git commit -am "Fix dep file merge issues" ...

Looks like that worked. Thanks

kevincantu · 2020-11-25T08:03:52Z

An idle thought: can you rename this pull request? I assume the "f" stands for your name?

I am not expert in the code being modified, but this handles a lot more of the protocol / protocol_version mismatch issues that #16004 doesn't handle yet, and updates the docs, too!

I like this pull request a lot! 😺

Remove comment fix terrafmt errors

…om/fivehole68/terraform-provider-aws into f-lb-target-group-protocol-version

…p-protocol-version

oliveirafilipe · 2020-12-03T14:34:09Z

If you allow me, one advice that I could give to you is use git rebase -i to format your commits in a more consistent way before merging this PR.

Hope this could be merge soon 🚀

kevincantu · 2020-12-03T22:24:06Z

The maintainers could also just do a squish merge to clean it all up in a click: what do they prefer?

fivehole68 · 2020-12-15T18:20:15Z

Is there anything else that is waiting on me to get this merged?

kevincantu · 2020-12-24T06:16:03Z

I've posted a comment here about this too hoping to get someone's attention:
https://discuss.hashicorp.com/t/can-a-maintainer-take-a-look-at-this-pull-req-to-add-support-for-aws-alb-protocol-versions/19161

rpsadarangani · 2021-01-05T07:46:00Z

Hi, is there any update on this PR when this is going to get merge.

yliu-d · 2021-01-06T22:19:54Z

@breathingdust ☝️ Can we have someone from Hashicorp to review this PR? Thanks!

breathingdust · 2021-01-13T23:26:07Z

Hi all 👋, just a quick update that this is in the queue for review and inclusion. Shouldn't be too much longer, we appreciate your patience!

bill-rich · 2021-02-09T18:24:35Z

Hi @fivehole68! Thank you for getting this enhancement together. I did some work on rebasing and making a few changes. Since I wasn't able to push to your branch, I opened a new PR (#17534). It still includes your commits, but will make it easier get it merged quickly. I'm going to close this PR so we can track things in the new one.

ghost · 2021-03-12T17:11:10Z

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!

mike-hightail added 2 commits November 10, 2020 11:57

Add support for protocol version to target groups

4841512

Remove comment

c76539c

fivehole68 requested a review from a team as a code owner November 10, 2020 21:21

ghost added size/L Managed by automation to categorize the size of a PR. service/elbv2 Issues and PRs that pertain to the elbv2 service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Nov 10, 2020

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Nov 10, 2020

Merge branch 'master' into f-lb-target-group-protocol-version

e93c7ae

fivehole68 mentioned this pull request Nov 10, 2020

Add support for protocol version to aws_lb_target_group #16004

Closed

oliveirafilipe approved these changes Nov 20, 2020

View reviewed changes

kevincantu reviewed Nov 25, 2020

View reviewed changes

kevincantu approved these changes Nov 25, 2020

View reviewed changes

fivehole68 changed the title ~~F lb target group protocol version~~ Add protocol_version to lb target groups Nov 25, 2020

mike-hightail added 6 commits November 25, 2020 12:56

fix terrafmt errors

b5065c0

Add support for protocol version to target groups

74d6b7c

Remove comment fix terrafmt errors

Revert go files

ae448c3

Merge branch 'f-lb-target-group-protocol-version' of https://github.c…

6bd6a20

…om/fivehole68/terraform-provider-aws into f-lb-target-group-protocol-version

Merge remote-tracking branch 'hashicorp/master' into f-lb-target-grou…

4ea0c87

…p-protocol-version

Fix dep file merge issues

6859e7d

kevincantu approved these changes Dec 1, 2020

View reviewed changes

Base automatically changed from master to main January 23, 2021 00:59

ericamador mentioned this pull request Jan 28, 2021

Add support for target group "protocol_version" terraform-aws-modules/terraform-aws-alb#179

Closed

kevincantu mentioned this pull request Feb 4, 2021

Add protocol_version to lb_target_group #17260

Merged

Merge branch 'main' into f-lb-target-group-protocol-version

7af097c

ghost added the documentation Introduces or discusses updates to documentation. label Feb 8, 2021

bill-rich closed this Feb 9, 2021

ewbankkit mentioned this pull request Feb 9, 2021

Please support gRPC status code [0-99] in the matcher of healthcheck of lb_target_group #17479

Closed

fivehole68 mentioned this pull request Feb 17, 2021

Use gRPC matcher with gRPC protocol #17534

Merged

ghost locked as resolved and limited conversation to collaborators Mar 12, 2021

breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add protocol_version to lb target groups #16132

Add protocol_version to lb target groups #16132

fivehole68 commented Nov 10, 2020

github-actions bot commented Nov 10, 2020

kevincantu Nov 25, 2020

fivehole68 Nov 25, 2020

kevincantu Nov 25, 2020

fivehole68 Nov 25, 2020

kevincantu Nov 26, 2020 •

edited

Loading

fivehole68 Nov 28, 2020

kevincantu commented Nov 25, 2020

oliveirafilipe commented Dec 3, 2020

kevincantu commented Dec 3, 2020

fivehole68 commented Dec 15, 2020

kevincantu commented Dec 24, 2020 •

edited

Loading

rpsadarangani commented Jan 5, 2021

yliu-d commented Jan 6, 2021

breathingdust commented Jan 13, 2021

bill-rich commented Feb 9, 2021

ghost commented Mar 12, 2021

Add protocol_version to lb target groups #16132

Add protocol_version to lb target groups #16132

Conversation

fivehole68 commented Nov 10, 2020

Community Note

github-actions bot commented Nov 10, 2020

kevincantu Nov 25, 2020

Choose a reason for hiding this comment

fivehole68 Nov 25, 2020

Choose a reason for hiding this comment

kevincantu Nov 25, 2020

Choose a reason for hiding this comment

fivehole68 Nov 25, 2020

Choose a reason for hiding this comment

kevincantu Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

fivehole68 Nov 28, 2020

Choose a reason for hiding this comment

kevincantu commented Nov 25, 2020

oliveirafilipe commented Dec 3, 2020

kevincantu commented Dec 3, 2020

fivehole68 commented Dec 15, 2020

kevincantu commented Dec 24, 2020 • edited Loading

rpsadarangani commented Jan 5, 2021

yliu-d commented Jan 6, 2021

breathingdust commented Jan 13, 2021

bill-rich commented Feb 9, 2021

ghost commented Mar 12, 2021

kevincantu Nov 26, 2020 •

edited

Loading

kevincantu commented Dec 24, 2020 •

edited

Loading