New Resource: aws_vpc_peering_connection_options #3909
Conversation
|
Acceptance and regression tests:
|
| @@ -24,6 +23,12 @@ func resourceAwsVpcPeeringConnection() *schema.Resource { | |||
| State: schema.ImportStatePassthrough, | |||
| }, | |||
|
|
|||
| Timeouts: &schema.ResourceTimeout{ | |||
There was a problem hiding this comment.
Added timeout configuration options and set defaults to the values in the existing code - 1 minute in all cases.
|
|
||
| pcRaw, status, err := resourceAwsVPCPeeringConnectionStateRefreshFunc(conn, d.Id())() |
There was a problem hiding this comment.
Simplified this code and removed cases that couldn't be reached.
radeksimko
added
new-resource
There was a problem hiding this comment.
Hey @ewbankkit! Thanks for your contribution here. I think architecturally this is going to be the best approach to fix the ordering issues. Can you see my below comments and let me know if you have any questions? It's looking pretty good so far.
| if err != nil { | ||
| return err | ||
| return errwrap.Wrapf("Error reading VPC Peering Connection: {{err}}", err) |
There was a problem hiding this comment.
Nitpick: We do not need errwrap for returning simple messages -- fmt.Errorf() is just fine 👍
There was a problem hiding this comment.
Will change this and other calls to errwrap.Wrapf() to use fmt.Errorf().
| // Sometimes AWS just has consistency issues and doesn't see | ||
| // our instance yet. Return an empty state. | ||
| return nil, "", nil | ||
| return nil, "", err | ||
| } | ||
|
|
||
| pc := resp.VpcPeeringConnections[0] |
There was a problem hiding this comment.
We should have nil/len() checks for resp, resp.VpcPeeringConnections, and pc.Status to prevent panics.
| @@ -432,3 +381,7 @@ func checkVpcPeeringConnectionAvailable(conn *ec2.EC2, id string) error { | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| func isNoSuchVpcPeeringConnectionErr(err error) bool { | |||
There was a problem hiding this comment.
Nitpick: We prefer to not introduce indirection in such simple cases.
|
|
||
| d.Set("vpc_peering_connection_id", pc.VpcPeeringConnectionId) | ||
|
|
||
| if pc.AccepterVpcInfo.PeeringOptions != nil { |
There was a problem hiding this comment.
We should have nil checks for pc.AccepterVpcInfo and pc.RequesterVpcInfo to prevent crashes.
| Provides a resource to manage a VPC peering connection. | ||
|
|
||
| ~> **NOTE on VPC Peering Connections and VPC Peering Connection Options:** Terraform provides | ||
| both a standalone [VPC Peering Connection Options](vpc_peering_options) and a VPC Peering Connection |
| # create an explicit dependency on the accepter. | ||
|
|
||
| resource "aws_vpc_peering_connection_options" "requester" { | ||
| vpc_peering_connection_id = "${aws_vpc_peering_connection_accepter.peer.id}" |
There was a problem hiding this comment.
I believe this should be "${aws_vpc_peering_connection.peer.id}"
There was a problem hiding this comment.
Actually it should be aws_vpc_peering_connection_accepter.peer.id.
The comment above
As options can't be set until the connection has been accepted
create an explicit dependency on the accepter.
is meant to explain why - We don't want to create the requester's options until the accepter has accepted.
I'll move the comment onto the line above the code.
|
|
||
| ```hcl | ||
| provider "aws" { | ||
| # Requester's credentials. |
There was a problem hiding this comment.
It might be helpful to very explicitly alias these as alias = "requester" and alias = "accepter" so the resources are very explicit as well.
There was a problem hiding this comment.
Verified that the new text works as expected:
$ terraform apply
data.aws_caller_identity.peer: Refreshing state...
aws_vpc.peer: Creating...
assign_generated_ipv6_cidr_block: "" => "false"
cidr_block: "" => "10.1.0.0/16"
default_network_acl_id: "" => "<computed>"
default_route_table_id: "" => "<computed>"
default_security_group_id: "" => "<computed>"
dhcp_options_id: "" => "<computed>"
enable_classiclink: "" => "<computed>"
enable_classiclink_dns_support: "" => "<computed>"
enable_dns_hostnames: "" => "true"
enable_dns_support: "" => "true"
instance_tenancy: "" => "<computed>"
ipv6_association_id: "" => "<computed>"
ipv6_cidr_block: "" => "<computed>"
main_route_table_id: "" => "<computed>"
aws_vpc.main: Creating...
assign_generated_ipv6_cidr_block: "" => "false"
cidr_block: "" => "10.0.0.0/16"
default_network_acl_id: "" => "<computed>"
default_route_table_id: "" => "<computed>"
default_security_group_id: "" => "<computed>"
dhcp_options_id: "" => "<computed>"
enable_classiclink: "" => "<computed>"
enable_classiclink_dns_support: "" => "<computed>"
enable_dns_hostnames: "" => "true"
enable_dns_support: "" => "true"
instance_tenancy: "" => "<computed>"
ipv6_association_id: "" => "<computed>"
ipv6_cidr_block: "" => "<computed>"
main_route_table_id: "" => "<computed>"
aws_vpc.main: Creation complete after 5s (ID: vpc-48684831)
aws_vpc.peer: Creation complete after 6s (ID: vpc-876343fe)
aws_vpc_peering_connection.peer: Creating...
accept_status: "" => "<computed>"
accepter.#: "" => "<computed>"
auto_accept: "" => "false"
peer_owner_id: "" => "000000000000"
peer_region: "" => "<computed>"
peer_vpc_id: "" => "vpc-876343fe"
requester.#: "" => "<computed>"
tags.%: "" => "1"
tags.Side: "" => "Requester"
vpc_id: "" => "vpc-48684831"
aws_vpc_peering_connection.peer: Creation complete after 2s (ID: pcx-47b7a92e)
aws_vpc_peering_connection_accepter.peer: Creating...
accept_status: "" => "<computed>"
accepter.#: "" => "<computed>"
auto_accept: "" => "true"
peer_owner_id: "" => "<computed>"
peer_region: "" => "<computed>"
peer_vpc_id: "" => "<computed>"
requester.#: "" => "<computed>"
tags.%: "" => "1"
tags.Side: "" => "Accepter"
vpc_id: "" => "<computed>"
vpc_peering_connection_id: "" => "pcx-47b7a92e"
aws_vpc_peering_connection_accepter.peer: Creation complete after 3s (ID: pcx-47b7a92e)
aws_vpc_peering_connection_options.accepter: Creating...
accepter.#: "" => "1"
accepter.1102046665.allow_classic_link_to_remote_vpc: "" => "false"
accepter.1102046665.allow_remote_vpc_dns_resolution: "" => "true"
accepter.1102046665.allow_vpc_to_remote_classic_link: "" => "false"
requester.#: "" => "<computed>"
vpc_peering_connection_id: "" => "pcx-47b7a92e"
aws_vpc_peering_connection_options.requester: Creating...
accepter.#: "" => "<computed>"
requester.#: "" => "1"
requester.1102046665.allow_classic_link_to_remote_vpc: "" => "false"
requester.1102046665.allow_remote_vpc_dns_resolution: "" => "true"
requester.1102046665.allow_vpc_to_remote_classic_link: "" => "false"
vpc_peering_connection_id: "" => "pcx-47b7a92e"
aws_vpc_peering_connection_options.requester: Creation complete after 1s (ID: pcx-47b7a92e)
aws_vpc_peering_connection_options.accepter: Creation complete after 1s (ID: pcx-47b7a92e)
Apply complete! Resources: 6 added, 0 changed, 0 destroyed.
|
|
||
| # aws_vpc_peering_connection_options | ||
|
|
||
| Provides a resource to manage VPC peering connection options. |
There was a problem hiding this comment.
It might be helpful to mention the ordering problem here to explain why you'd want to use this resource over the arguments in the original resources. Same vice-versa.
There was a problem hiding this comment.
Agreed.
I'll use text from this PR's description:
Using a VPC Peering Connection Options resource decouples management of the connection options from
management of the VPC Peering Connection and allows options to be set correctly in cross-account scenarios.
bflad
added
the
waiting-response
|
Sure, I should be able to get to these changes this week. |
ewbankkit
force-pushed
the
add-aws-vpc-peering-connection-options-resource
branch
from
5afea8d to
45c82fa
Compare
|
Made suggested changes, rebased to fix conflicts and re-ran tests: |
bflad
removed
the
waiting-response
|
This has been released in version 1.17.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
@ewbankkit i cannot enable remote vpc dns resolution,when create Inter-Region VPC Peering in the same account with Terraform v0.11.10 + provider.aws v1.41.0. resource "aws_vpc_peering_connection" "peer" {
vpc_id = "${module.vpc.vpc_id}"
peer_region = "ap-northeast-1"
peer_vpc_id = "vpc-1c77767e"
auto_accept = false
tags {
Name = "demo-pcx"
Side = "Requester"
}
}
resource "aws_vpc_peering_connection_accepter" "peer" {
provider = "aws.peer"
vpc_peering_connection_id = "${aws_vpc_peering_connection.peer.id}"
auto_accept = true
tags {
Side = "Accepter"
}
}
resource "aws_vpc_peering_connection_options" "requester" {
vpc_peering_connection_id = "${aws_vpc_peering_connection_accepter.peer.id}"
requester {
allow_remote_vpc_dns_resolution = true
}
}
resource "aws_vpc_peering_connection_options" "accepter" {
provider = "aws.peer"
vpc_peering_connection_id = "${aws_vpc_peering_connection_accepter.peer.id}"
accepter {
allow_remote_vpc_dns_resolution = true
}
}run apply get errors: Plan: 2 to add, 0 to change, 0 to destroy.
Do you want to perform these actions?
Terraform will perform the actions described above.
Only 'yes' will be accepted to approve.
Enter a value: yes
aws_vpc_peering_connection_options.requester: Creating...
accepter.#: "" => "<computed>"
requester.#: "" => "1"
requester.1102046665.allow_classic_link_to_remote_vpc: "" => "false"
requester.1102046665.allow_remote_vpc_dns_resolution: "" => "true"
requester.1102046665.allow_vpc_to_remote_classic_link: "" => "false"
vpc_peering_connection_id: "" => "pcx-0699c559e000e56e8"
aws_vpc_peering_connection_options.accepter: Creating...
accepter.#: "" => "1"
accepter.1102046665.allow_classic_link_to_remote_vpc: "" => "false"
accepter.1102046665.allow_remote_vpc_dns_resolution: "" => "true"
accepter.1102046665.allow_vpc_to_remote_classic_link: "" => "false"
requester.#: "" => "<computed>"
vpc_peering_connection_id: "" => "pcx-0699c559e000e56e8"
Releasing state lock. This may take a few moments...
Error: Error applying plan:
2 error(s) occurred:
* aws_vpc_peering_connection_options.requester: 1 error(s) occurred:
* aws_vpc_peering_connection_options.requester: Error modifying VPC Peering Connection Options: OperationNotPermitted: Modifying VPC peering connection options AllowEgressFromLocalClassicLinkToRemoteVpc, AllowEgressFromLocalVpcToRemoteClassicLink is not supported for cross-region VPC peering connections
status code: 400, request id: 6dab72db-4ee2-40d9-a301-1959980996a7
* aws_vpc_peering_connection_options.accepter: 1 error(s) occurred:
* aws_vpc_peering_connection_options.accepter: Error modifying VPC Peering Connection Options: OperationNotPermitted: Modifying VPC peering connection options AllowEgressFromLocalClassicLinkToRemoteVpc, AllowEgressFromLocalVpcToRemoteClassicLink is not supported for cross-region VPC peering connections
status code: 400, request id: b8341931-ce7a-43cc-9972-1fe19ac139ba
Terraform does not automatically rollback in the face of errors.
Instead, your Terraform state file has been partially updated with
any resources that successfully completed. Please address the error
above and apply again to incrementally change your infrastructure. |
|
@7rack The [AWS documentation] states that ClassicLink options can't be set cross-region but DNS options can so it looks like we need to be smarter here; I'll take a look. Thanks. |
|
@ewbankkit Any idea when this will be fixed? I just ran into this issue on cross-region peering. Terraform v0.11.10 ... Error applying plan: 2 error(s) occurred:
|
|
The above is happening, but I realized I didn't need the options section in my TF file and that fixed my issue for now. |
|
Hi there, I have the same issue with Terraform v0.11.11 |
|
How does this help in the situation where you don't have control in the accepters account? My use case is that I want to enable allow_remote_vpc_dns_resolution, which cannot be done until the peering connection has been accepted. My current workaround is to have a condition on the resource and run the apply twice, the second apply is run with the flag set to true after the accepter has accepted: Ideally, I would like to be able to run this without having to change any parameters. |
|
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
locked and limited conversation to collaborators
Fixes #3069, #3676 and #3850.
Add a new resource
aws_vpc_peering_connection_optionsthat manages VPC peering connection options.This resource decouples management of the connection options from management of the connection and allows options to be set correctly in cross-account scenarios: