provider/aws: Implementing vpc_peering_connection_accept

[BSick7]

Problem

Currently aws_vpc_peering_connection provides a way to auto_accept a peering connection. This works great until you have a peering connection between 2 VPCs that are in different aws accounts. If you specify auto_accept = true to automate the peering connection accept, aws will error during a terraform apply that you are not authorized to accept.

Solution

The solution is to utilize provider aliases to execute the accept under different aws account credentials. See example below.

resource "aws_vpc_peering_connection" "peer1" {
  // use peering connection like normal
  auto_accept = false
}

provider "aws" {
  alias = "other"
  // other account credentials
}

resource "aws_vpc_peering_connection_accept" "peer1" {
  peering_connection_id = "${aws_vpc_peering_connection.peer1.id}"

  provider = "aws.other"
}

[BSick7]

Status

• Implement
• Acceptance Tests
• Update docs

[catsby]

This works great until you have a peering connection between 2 VPCs that are in different aws accounts.

What happens then?

[steve-jansen]

@clint

Currently an operator has to manually accept the peering connection in the target account, usually in the AWS console.

This resource would enable us to fully automate cross-account VPC peering, using aliased AWS providers in TF.

[steve-jansen]

Err, @catsby... Sorry autocorrect on mobile

[catsby]

Ah, I see.. so we solve this with the current resource with auto_accept, but that requires you be the owner of both VPCs, which you're saying we can't really rely on that being the case, yeah?

[steve-jansen]

@catsby exactly, auto_accept currently supports same-account peering only.

[BSick7]

That's correct @catsby. I updated the original overview.

[BSick7]

@catsby ready for review

[BSick7]

@catsby any word on review?

[BSick7]

@catsby @phinze it's been awhile, can we get an update on review?

[heimweh]

Any update on this? would love to see something like this implemented. I know that CloudFormation doesn't even support this natively. I solved it in CloudFormation using a Custom resource and having two Lambda functions (one in each account req/accept), before that I used my own custom tooling to solve this. I'm currently in the process of migrating from CloudFormation => Terraform so having this automated would mean so much.

[jen20]

Hi @BSick7! Thanks for the pull request and the clarifications. Sorry this has taken us a while to get around to, but someone will give it some attention shortly and get it merged. We're in the process of making some changes to the test framework to allow for cross account testing which will make it possible to run acceptance tests on this during the nightly build. Probably @stack72 will pick this up and land it once those changes are in.

[pielu]

@BSick7, Could you please add tag support to the PR?

[pielu]

Yes, it didn't work for me until after I added:

stateConf := &resource.StateChangeConf{
        Pending: []string{"pending-acceptance"},
        Target:  []string{"active"},
        Refresh: resourceAwsVPCPeeringConnectionStateRefreshFunc(conn, d.Id()),
        Timeout: 1 * time.Minute,
    }

    if _, err := stateConf.WaitForState(); err != nil {
        return errwrap.Wrapf(fmt.Sprintf(
            "Error waiting for VPC Peering Connection Accept (%s) to become available: {{err}}",
            d.Id()), err)
    }

[pielu]

The d.Id() was not immediately available for me, I had to assign it explicitly:
d.SetId(d.Get("peering_connection_id").(string))

[AndHei]

I suggest to add language to the docs of both commands that points out that aws_vpc_peering_connection in multi account setups requires vpc_peering_connection_accept

[steve-jansen]

@jen20 @stack72 friendly bump on this PR :)

[catsby]

Hey there –

• I rebased this PR locally to bring it up-to-date; no conflicts 👍
• the test didn't pass:

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVPCPeeringConnectionAccept_basic -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnectionAccept_basic
--- FAIL: TestAccAWSVPCPeeringConnectionAccept_basic (16.13s)
        testing.go:265: Step 0 error: Error applying: 1 error(s) occurred:

                * aws_vpc_peering_connection_accept.foo: MissingParameter: The request must contain the parameter vpcPeeringConnectionId
                        status code: 400, request id: 9c289c53-3626-4c6f-9f69-bcc036c8b346
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    16.155s
make: *** [testacc] Error 1

Unfortunately, without access to 2 accounts in the test, I don't believe the test is really testing this new resource.

Alternatively, what do you think about adding a source_connection_id attribute to the aws_vpc_peering_connection resource and maybe changing that resource to provide both sides of the functionality, instead of an entirely new resource?

[ewbankkit]

+1 to adding source_connection_id to the aws_vpc_peering_connection resource on the accepter side.
Using the aws_vpc_peering_connection data source - #10913 - I think you could then do something like:

Account A (requester):

resource "aws_vpc_peering_connection" "pc" {
  vpc_id = "vpc-12345678"
  peer_vpc_id = "vpc-90123456"
  peer_owner_id = "<Account B>"
}

Account B (accepter):

data "aws_vpc_peering_connection" "a" {
  vpc_id = "vpc-12345678"
  peer_vpc_id = "vpc-90123456"
}

resource "aws_vpc_peering_connection" "pc" {
  # Not even sure you'd require vpc_id as that's implicit in the source PCX ID.
  vpc_id = "vpc-12345678"
  source_connection_id = "${data.aws_vpc_peering_connection.a.id}"

  auto_accept = true
}

[magnusthorne]

+1 This would be very useful.

[ewbankkit]

@BSick7 I took the liberty of taking your great work here and extending it in #11505, hope that's not a problem.

[catsby]

I merged #11505, which included this one. Thanks!

[ghost]

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.