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

New resource: aws_ec2_transit_gateway_vpc_attachment_accepter #8679

Merged
merged 6 commits into from
May 27, 2019

Conversation

ewbankkit
Copy link
Contributor

@ewbankkit ewbankkit commented May 17, 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 #8125.

Release note for CHANGELOG:

FEATURES:

* New Resource: aws_ec2_transit_gateway_vpc_attachment_accepter (#8125)

Acceptance tests:

$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=estAccAWSEc2TransitGatewayVpcAttachmentAccepter_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=estAccAWSEc2TransitGatewayVpcAttachmentAccepter_ -timeout 120m
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic (400.18s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	400.206s

@ghost ghost added size/M Managed by automation to categorize the size of a PR. provider Pertains to the provider itself, rather than any interaction with AWS. service/ec2 Issues and PRs that pertain to the ec2 service. documentation Introduces or discusses updates to documentation. labels May 17, 2019
@ghost ghost added size/L Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed size/M Managed by automation to categorize the size of a PR. labels May 18, 2019
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels May 19, 2019
@ewbankkit ewbankkit changed the title [WIP] New resource: aws_ec2_transit_gateway_vpc_attachment_accepter New resource: aws_ec2_transit_gateway_vpc_attachment_accepter May 19, 2019
@ewbankkit
Copy link
Contributor Author

Removed WIP. Ready for review.

@ewbankkit
Copy link
Contributor Author

ewbankkit commented May 19, 2019

I didn't add transit_gateway_default_route_table_association or transit_gateway_default_route_table_propagation attributes to this resource because of the notes in the documentation:

This cannot be configured or perform drift detection with Resource Access Manager shared EC2 Transit Gateways.

but maybe this resource, since it runs in the context of the AWS account that owns the transit gateway, is exactly where these attributes should be specified in the cross-account case?

Back to WIP until this is investigated.

Verified that these attributes can be modified on the accepter's side via this resource, so I'll add an acceptance test around that.

@ewbankkit ewbankkit changed the title New resource: aws_ec2_transit_gateway_vpc_attachment_accepter [WIP] New resource: aws_ec2_transit_gateway_vpc_attachment_accepter May 20, 2019
@ewbankkit ewbankkit changed the title [WIP] New resource: aws_ec2_transit_gateway_vpc_attachment_accepter New resource: aws_ec2_transit_gateway_vpc_attachment_accepter May 22, 2019
@ewbankkit
Copy link
Contributor Author

Removed WIP.

Acceptance tests:

$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_'==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 20 -run=TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_ -timeout 120m
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic (366.19s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation (555.40s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	555.436s

@bflad bflad added the new-resource Introduces a new resource. label May 24, 2019
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.

Hey @ewbankkit 👋 Overall looking pretty good. The big item here being that resource's (even accepters) should implement the Delete function to ensure either side can remove attachments. Thanks!

}

func resourceAwsEc2TransitGatewayVpcAttachmentAccepterDelete(d *schema.ResourceData, meta interface{}) error {
log.Printf("[WARN] Will not delete EC2 Transit Gateway VPC Attachment. Terraform will remove this resource from the state file, however resources may remain.")
Copy link
Contributor

Choose a reason for hiding this comment

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

The accepting side should be able to call DeleteTransitGatewayVpcAttachment here to break the attachment. 👍

),
},
{
Config: testAccAWSEc2TransitGatewayVpcAttachmentAccepterConfig_updated(rName),
Copy link
Contributor

Choose a reason for hiding this comment

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

The updated configuration here seems to be only modifying tags. We prefer the _basic tests to only check the required configuration for a resource (in this case, no tags) and creating a separate Tags test to check tags on create and update. 😄

* `transit_gateway_default_route_table_propagation` - (Optional) Boolean whether the VPC Attachment should propagate routes with the EC2 Transit Gateway propagation default route table. Default value: `true`.
* `tags` - (Optional) Key-value tags for the EC2 Transit Gateway VPC Attachment.

### Removing `aws_ec2_transit_gateway_vpc_attachment_accepter` from your configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

When the Delete function is implemented, this section of the resource documentation should be removed. If this restriction is actually necessary, please instead move it to a note at the top of the page:

~> **NOTE:** Deleting this resource only removes it from state management, etc.

@ewbankkit
Copy link
Contributor Author

Review comments addressed.
Re-ran acceptance tests:

$ AWS_ALTERNATE_ACCESS_KEY_ID=AAAAAAAAAAAAAAAAAAAA AWS_ALTERNATE_SECRET_ACCESS_KEY=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -parallel 2 -run=TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_ -timeout 120m
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_Tags
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_Tags
=== RUN   TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation
=== PAUSE TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic (332.64s)
=== CONT  TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_Tags
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation (564.78s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_Tags (355.98s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	688.702s

@bflad bflad added this to the v2.13.0 milestone May 27, 2019
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.

Thanks so much for the update, @ewbankkit, looks great. 🚀

Output from acceptance testing:

--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_basic (352.97s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_Tags (354.94s)
--- PASS: TestAccAWSEc2TransitGatewayVpcAttachmentAccepter_TransitGatewayDefaultRouteTableAssociationAndPropagation (538.85s)

@bflad bflad merged commit 014bf2e into hashicorp:master May 27, 2019
bflad added a commit that referenced this pull request May 27, 2019
@ewbankkit ewbankkit deleted the issue-8125 branch May 28, 2019 11:34
@bflad
Copy link
Contributor

bflad commented May 31, 2019

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

@ghost
Copy link

ghost commented Mar 29, 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 Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/ec2 Issues and PRs that pertain to the ec2 service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement AcceptTransitGatewayVpcAttachment API call
2 participants