f-aws_ec2_traffic_mirror_target #26864

meetreks · 2022-09-19T19:25:39Z

Relates to #26767

Output from acceptance testing:

$ make testacc PKG=ec2 TESTS=TestAccVPCTrafficMirrorTarget_gwlb
~/terraform-provider-aws (f-aws_ec2_traffic_mirror_target) $ make testacc PKG=ec2 TESTS=TestAccVPCTrafficMirrorTarget_gwlb
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 20 -run='TestAccVPCTrafficMirrorTarget_gwlb'  -timeout 180m
=== RUN   TestAccVPCTrafficMirrorTarget_gwlb
=== PAUSE TestAccVPCTrafficMirrorTarget_gwlb
=== CONT  TestAccVPCTrafficMirrorTarget_gwlb
--- PASS: TestAccVPCTrafficMirrorTarget_gwlb (258.16s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/ec2        258.244s
:~/terraform-provider-aws (f-aws_ec2_traffic_mirror_target) $ 
:~/terraform-provider-aws (f-aws_ec2_traffic_mirror_target) $ 
...

github-actions · 2022-09-19T19:25:57Z

Community Note

Voting for Prioritization

Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
Please see our prioritization guide for information on how we prioritize.
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

Please review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

GlennChia

PR looks good @meetreks! Small changes requested to improve the docs and test. Once the test cases are run, the PR description above should also be updated with the results. Thanks!

website/docs/r/ec2_traffic_mirror_target.html.markdown

internal/service/ec2/vpc_traffic_mirror_target_test.go

GlennChia

After discussion, gateway_load_balancer_endpoint_id does not use the Load Balancer's ID. Rather it is the VPCE ID. We can also call this out in the documentation

GlennChia · 2022-09-22T18:08:55Z

website/docs/r/ec2_traffic_mirror_target.html.markdown

+
+resource "aws_ec2_traffic_mirror_target" "gwlb" {
+  description                       = "GWLB target"
+  gateway_load_balancer_endpoint_id = aws_lb.gwlb.arn


On further inspection, this accepts a VPC Endpoint ID and not the Load Balancer ARN. Will this change be better

Suggested change

gateway_load_balancer_endpoint_id = aws_lb.gwlb.arn

gateway_load_balancer_endpoint_id = aws_vpc_endpoint.example.id

GlennChia · 2022-09-22T18:09:44Z

internal/service/ec2/vpc_traffic_mirror_target_test.go

@@ -332,6 +363,19 @@ resource "aws_ec2_traffic_mirror_target" "test" {
 `, rName, description, tagKey1, tagValue1, tagKey2, tagValue2))
 }

+func testAccVPCTrafficMirrorTargetConfig_gwlb(rName, description string) string {
+	return acctest.ConfigCompose(
+		//testAccTrafficMirrorTargetConfigBase(rName),


We can remove this unused comment as well

Suggested change

//testAccTrafficMirrorTargetConfigBase(rName),

GlennChia · 2022-09-22T18:14:44Z

internal/service/ec2/vpc_traffic_mirror_target_test.go

+				Config: testAccVPCTrafficMirrorTargetConfig_gwlb(rName, description),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "description", description),
+					resource.TestCheckResourceAttrPair(resourceName, "gateway_load_balancer_endpoint_id", "aws_lb.test", "id"),


Changing aws_lb to aws_vpc_endpoint

Suggested change

resource.TestCheckResourceAttrPair(resourceName, "gateway_load_balancer_endpoint_id", "aws_lb.test", "id"),

resource.TestCheckResourceAttrPair(resourceName, "gateway_load_balancer_endpoint_id", "aws_vpc_endpoint.test", "id"),

meetreks · 2022-09-22T18:29:18Z

@GlennChia Thanks for the review. The PR is now set. Here are the acceptance test results.

meetreks · 2022-09-22T18:29:37Z

Acceptance Results: ~/terraform-provider-aws (f-aws_ec2_traffic_mirror_target) $ make testacc PKG=ec2 TESTS=TestAccVPCTrafficMirrorTarget_gwlb ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 20 -run='TestAccVPCTrafficMirrorTarget_gwlb' -timeout 180m === RUN TestAccVPCTrafficMirrorTarget_gwlb === PAUSE TestAccVPCTrafficMirrorTarget_gwlb === CONT TestAccVPCTrafficMirrorTarget_gwlb --- PASS: TestAccVPCTrafficMirrorTarget_gwlb (258.16s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ec2 258.244s :~/terraform-provider-aws (f-aws_ec2_traffic_mirror_target) $ :~/terraform-provider-aws (f-aws_ec2_traffic_mirror_target) $

GlennChia

LGTM @meetreks, approved!

…in acceptance test configurations.

ewbankkit

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccVPCTrafficMirrorTarget_' PKG=ec2 ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/ec2/... -v -count 1 -parallel 2  -run=TestAccVPCTrafficMirrorTarget_ -timeout 180m
=== RUN   TestAccVPCTrafficMirrorTarget_nlb
=== PAUSE TestAccVPCTrafficMirrorTarget_nlb
=== RUN   TestAccVPCTrafficMirrorTarget_eni
=== PAUSE TestAccVPCTrafficMirrorTarget_eni
=== RUN   TestAccVPCTrafficMirrorTarget_tags
=== PAUSE TestAccVPCTrafficMirrorTarget_tags
=== RUN   TestAccVPCTrafficMirrorTarget_disappears
=== PAUSE TestAccVPCTrafficMirrorTarget_disappears
=== RUN   TestAccVPCTrafficMirrorTarget_gwlb
=== PAUSE TestAccVPCTrafficMirrorTarget_gwlb
=== CONT  TestAccVPCTrafficMirrorTarget_nlb
=== CONT  TestAccVPCTrafficMirrorTarget_disappears
--- PASS: TestAccVPCTrafficMirrorTarget_disappears (223.78s)
=== CONT  TestAccVPCTrafficMirrorTarget_tags
--- PASS: TestAccVPCTrafficMirrorTarget_nlb (236.39s)
=== CONT  TestAccVPCTrafficMirrorTarget_eni
--- PASS: TestAccVPCTrafficMirrorTarget_eni (115.80s)
=== CONT  TestAccVPCTrafficMirrorTarget_gwlb
--- PASS: TestAccVPCTrafficMirrorTarget_tags (245.64s)
--- PASS: TestAccVPCTrafficMirrorTarget_gwlb (373.33s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/ec2	729.696s

ewbankkit · 2022-09-29T15:56:28Z

@meetreks Thanks for the contribution 🎉 👏.

github-actions · 2022-09-29T22:36:25Z

This functionality has been released in v4.33.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. Thank you!

meetreks · 2022-09-30T18:50:33Z

Thanks @ewbankkit looked at your commit and it made sense to me with the changes.

github-actions · 2022-11-01T02:44:56Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

EC2 Default User added 2 commits September 19, 2022 18:58

WIP PR for Internal Review aws_ec2_traffic_mirror_target

e5b40aa

WIP PR for Internal Review aws_ec2_traffic_mirror_target

4d83917

Ran go formatter for linting issues

996c101

meetreks mentioned this pull request Sep 20, 2022

AWS VPC Traffic Mirroring - enable GWLB as a valid traffic mirror target #26767

Closed

GlennChia suggested changes Sep 22, 2022

View reviewed changes

website/docs/r/ec2_traffic_mirror_target.html.markdown Outdated Show resolved Hide resolved

internal/service/ec2/vpc_traffic_mirror_target_test.go Outdated Show resolved Hide resolved

GlennChia suggested changes Sep 22, 2022

View reviewed changes

GlennChia reviewed Sep 22, 2022

View reviewed changes

incorporated PR review comments

6a8cee0

meetreks force-pushed the f-aws_ec2_traffic_mirror_target branch from bfb85a0 to 6a8cee0 Compare September 22, 2022 18:33

GlennChia approved these changes Sep 22, 2022

View reviewed changes

ewbankkit added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Sep 28, 2022

ewbankkit added 2 commits September 29, 2022 10:59

r/aws_ec2_traffic_mirror_target: Use '(acctest.ConfigVPCWithSubnets' …

c7ea6ef

…in acceptance test configurations.

Add and use 'FindTrafficMirrorTarget' and friends.

a1f938e

github-actions bot added size/XL Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Sep 29, 2022

ewbankkit approved these changes Sep 29, 2022

View reviewed changes

ewbankkit merged commit 9c21d5d into hashicorp:main Sep 29, 2022

github-actions bot added this to the v4.33.0 milestone Sep 29, 2022

meetreks deleted the f-aws_ec2_traffic_mirror_target branch September 29, 2022 17:10

github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2022

justinretzolk added the partner Contribution from a partner. label May 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

f-aws_ec2_traffic_mirror_target #26864

f-aws_ec2_traffic_mirror_target #26864

meetreks commented Sep 19, 2022 •

edited

Loading

github-actions bot commented Sep 19, 2022

GlennChia left a comment •

edited

Loading

GlennChia left a comment •

edited

Loading

GlennChia Sep 22, 2022

GlennChia Sep 22, 2022

GlennChia Sep 22, 2022

meetreks commented Sep 22, 2022

meetreks commented Sep 22, 2022

GlennChia left a comment

ewbankkit left a comment

ewbankkit commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

meetreks commented Sep 30, 2022

github-actions bot commented Nov 1, 2022

	gateway_load_balancer_endpoint_id = aws_lb.gwlb.arn
	gateway_load_balancer_endpoint_id = aws_vpc_endpoint.example.id

	resource.TestCheckResourceAttrPair(resourceName, "gateway_load_balancer_endpoint_id", "aws_lb.test", "id"),
	resource.TestCheckResourceAttrPair(resourceName, "gateway_load_balancer_endpoint_id", "aws_vpc_endpoint.test", "id"),

f-aws_ec2_traffic_mirror_target #26864

f-aws_ec2_traffic_mirror_target #26864

Conversation

meetreks commented Sep 19, 2022 • edited Loading

github-actions bot commented Sep 19, 2022

Community Note

GlennChia left a comment • edited Loading

Choose a reason for hiding this comment

GlennChia left a comment • edited Loading

Choose a reason for hiding this comment

GlennChia Sep 22, 2022

Choose a reason for hiding this comment

GlennChia Sep 22, 2022

Choose a reason for hiding this comment

GlennChia Sep 22, 2022

Choose a reason for hiding this comment

meetreks commented Sep 22, 2022

meetreks commented Sep 22, 2022

GlennChia left a comment

Choose a reason for hiding this comment

ewbankkit left a comment

Choose a reason for hiding this comment

ewbankkit commented Sep 29, 2022

github-actions bot commented Sep 29, 2022

meetreks commented Sep 30, 2022

github-actions bot commented Nov 1, 2022

meetreks commented Sep 19, 2022 •

edited

Loading

GlennChia left a comment •

edited

Loading

GlennChia left a comment •

edited

Loading