-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[WIP] Add Resource aws_ec2_transit_gateway_peering_attachment_accepter #11185
[WIP] Add Resource aws_ec2_transit_gateway_peering_attachment_accepter #11185
Conversation
Update and Delete functions implemented to interact with the AWS GO SDK for transit gateway peering attachments acceptance waitFor functions added for Delete terraform operations. Describe function added for transit gateway peering attachments acceptance.
Create and read functions implemented to interact with the AWS GO SDK for transit gateway peering attachment acceptance. waitFor functions added for Create/Read terraform operations & acceptance. Add provider resource mapping to provider.go
Work for transit_gateway_peering_attachment_accepter has now been moved into its own pr hashicorp#11185.
Move `waitForEc2TransitGatewayPeeringAttachmentAcceptance` into the correct place.
Work for transit_gateway_peering_attachment_accepter has now been moved into its own pr hashicorp#11185.
Default route table propogation doesn't appear to be supported by transit gateway peering attachments, only an assocation to the default route table is supported - removed references to propogation. Co-authored-by: Omarimcblack 43746683+Omarimcblack@users.noreply.github.com
@j-nix @Omarimcblack Let me know if you need any help with testing this. |
Hey, guys :)! Wondering if there is any timeline of getting this PR (and #11162) in master, perhaps in the next release already 🙊 ? |
if serves as an encouragement, AWS now supports inter-region peering in ALL regions 🍾 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @j-nix 👋 Thank you for starting this. I have left some initial feedback on the provided code.
Please also note that:
- This code will require rebase after merge of Add Resource & Data Source aws_ec2_transit_gateway_peering_attachment #11162 for likely merge conflicts
- Acceptance testing will need to be added. Add Resource & Data Source aws_ec2_transit_gateway_peering_attachment #11162 has some great examples that can serve as the base for this resource's testing. Please see the New Resource section of the Contributing Guide for more information and especially the cross-account/cross-region testing setup
- Documentation will need to be added, including adding this resource to
website/aws.erb
and creating a new documentation page atwebsite/docs/r/ec2_transit_gateway_peering_attachment_accepter.html.markdown
Please reach out if you have any questions or do not have time to implement the items.
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
aws/resource_aws_ec2_transit_gateway_peering_attachment_accepter.go
Outdated
Show resolved
Hide resolved
Hi @bflad ! Thanks a lot for leaving the comments here, I'm going to take a look at them and finish this PR off shortly as there's still a bit of work to be done here 👍 |
Tagging now handled by shared `keyvaluetags` package Co-Authored-By: Brian Flad <bflad417@gmail.com>
Fixes peer transit gateway/local transit gateway information to be the correct object. Co-Authored-By: Brian Flad <bflad417@gmail.com>
…ry items Default route table assocation is not necessary, this was copied from the attachment resource accidentally Co-Authored-By: Brian Flad <bflad417@gmail.com>
41e56ac
to
9ad7bbf
Compare
… aws_ec2_transit_gateway_peering_attachment (hashicorp#11162) Output from acceptance testing: ``` --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachment_basic (279.30s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachment_differentAccount (274.76s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachment_disappears (298.80s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachment_Tags_sameAccount (341.73s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentDataSource_Filter_differentAccount (290.68s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentDataSource_Filter_sameAccount (281.60s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentDataSource_ID_differentAccount (291.70s) --- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentDataSource_ID_sameAccount (308.69s) ```
9ad7bbf
to
4cee185
Compare
Drive-by note: The resource implementation is looking really good! The CI failure is due to us adding and enforcing provider-wide ignore tags functionality as of a few days ago -- two line fix: // In the Read function, near the conn :=
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig
// In the d.Set("tags", ...) call
d.Set("tags", keyvaluetags.Ec2KeyValueTags(transitGatewayPeeringAttachment.Tags).IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()) Hope this helps. 😄 Please reach out when this is ready for review. |
c74c3bf
to
d54dc11
Compare
Hi @bflad thanks for the help with the CI failure that should be pushed up now - very helpful! The tests are almost finished - will reach out by the end of the weekendwhen completed 👍 |
Hi @j-nix 👋 Any chance you could push up the testing you have (even incomplete)? We'd love to release this today if we can! |
c312715
to
9d69509
Compare
Hi @bflad , I've just pushed the tests up however they are still incomplete and have a little bit of work to do - I appreciate you want to get this out asap to go with the peering attachment resource! Apoloigies for the wait on this - sadly we haven't been able to find the time to complete them yet! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, @j-nix! The testing was really close -- just a few minor tweaks and it was good to go. Thanks again for working through this tougher one! 🚀
Output from acceptance testing:
--- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentAccepter_basic_sameAccount (625.85s)
--- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentAccepter_basic_differentAccount (628.47s)
--- PASS: TestAccAWSEc2TransitGatewayPeeringAttachmentAccepter_Tags_sameAccount (630.60s)
the end-user community certainly appreciates all the work put in here. cheers! |
This has been released in version 2.61.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! |
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! |
Community Note
Relates to #11117
Release note for CHANGELOG:
Output from acceptance testing:
Tests still outstanding