Skip to content

Commit

Permalink
provider/aws: Update VPC Peering connect accept/request attributes (s…
Browse files Browse the repository at this point in the history
…upersedes #8338) (#8432)

* Fix crash when reading VPC Peering Connection options.

This resolves the issue introduced in #8310.

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* Do not de-reference values when using Set().

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* provider/aws: Update VPC Peering connect accept/request attributes

* change from type list to type set

* provider/aws: Update VPC Peering accept/requst options, tests

* errwrap some things
  • Loading branch information
catsby authored Aug 24, 2016
1 parent 48685fe commit 341c7bf
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 55 deletions.
122 changes: 78 additions & 44 deletions builtin/providers/aws/resource_aws_vpc_peering_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/errwrap"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
)
Expand Down Expand Up @@ -67,7 +68,7 @@ func resourceAwsVPCPeeringCreate(d *schema.ResourceData, meta interface{}) error

resp, err := conn.CreateVpcPeeringConnection(createOpts)
if err != nil {
return fmt.Errorf("Error creating vpc peering connection: %s", err)
return errwrap.Wrapf("Error creating VPC Peering Connection: {{err}}", err)
}

// Get the ID and store it
Expand Down Expand Up @@ -124,20 +125,28 @@ func resourceAwsVPCPeeringRead(d *schema.ResourceData, meta interface{}) error {
return nil
}
}

d.Set("accept_status", *pc.Status.Code)
d.Set("peer_owner_id", *pc.AccepterVpcInfo.OwnerId)
d.Set("peer_vpc_id", *pc.AccepterVpcInfo.VpcId)
d.Set("vpc_id", *pc.RequesterVpcInfo.VpcId)

err = d.Set("accepter", flattenPeeringOptions(pc.AccepterVpcInfo.PeeringOptions))
if err != nil {
return err
log.Printf("[DEBUG] VPC Peering Connection response: %#v", pc)

d.Set("accept_status", pc.Status.Code)
d.Set("peer_owner_id", pc.AccepterVpcInfo.OwnerId)
d.Set("peer_vpc_id", pc.AccepterVpcInfo.VpcId)
d.Set("vpc_id", pc.RequesterVpcInfo.VpcId)

// When the VPC Peering Connection is pending acceptance,
// the details about accepter and/or requester peering
// options would not be included in the response.
if pc.AccepterVpcInfo != nil && pc.AccepterVpcInfo.PeeringOptions != nil {
err := d.Set("accepter", flattenPeeringOptions(pc.AccepterVpcInfo.PeeringOptions))
if err != nil {
log.Printf("[ERR] Error setting VPC Peering connection accepter information: %s", err)
}
}

err = d.Set("requester", flattenPeeringOptions(pc.RequesterVpcInfo.PeeringOptions))
if err != nil {
return err
if pc.RequesterVpcInfo != nil && pc.RequesterVpcInfo.PeeringOptions != nil {
err := d.Set("requester", flattenPeeringOptions(pc.RequesterVpcInfo.PeeringOptions))
if err != nil {
log.Printf("[ERR] Error setting VPC Peering connection requester information: %s", err)
}
}

err = d.Set("tags", tagsToMap(pc.Tags))
Expand Down Expand Up @@ -172,16 +181,16 @@ func resourceVPCPeeringConnectionOptionsModify(d *schema.ResourceData, meta inte
}

if v, ok := d.GetOk("accepter"); ok {
if s := v.([]interface{}); len(s) > 0 {
modifyOpts.AccepterPeeringConnectionOptions = expandPeeringOptions(
s[0].(map[string]interface{}))
if s := v.(*schema.Set); len(s.List()) > 0 {
co := s.List()[0].(map[string]interface{})
modifyOpts.AccepterPeeringConnectionOptions = expandPeeringOptions(co)
}
}

if v, ok := d.GetOk("requester"); ok {
if s := v.([]interface{}); len(s) > 0 {
modifyOpts.RequesterPeeringConnectionOptions = expandPeeringOptions(
s[0].(map[string]interface{}))
if s := v.(*schema.Set); len(s.List()) > 0 {
co := s.List()[0].(map[string]interface{})
modifyOpts.RequesterPeeringConnectionOptions = expandPeeringOptions(co)
}
}

Expand All @@ -202,18 +211,17 @@ func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error
d.SetPartial("tags")
}

if _, ok := d.GetOk("auto_accept"); ok {
pcRaw, _, err := resourceAwsVPCPeeringConnectionStateRefreshFunc(conn, d.Id())()
if err != nil {
return err
}

if pcRaw == nil {
d.SetId("")
return nil
}
pc := pcRaw.(*ec2.VpcPeeringConnection)
pcRaw, _, err := resourceAwsVPCPeeringConnectionStateRefreshFunc(conn, d.Id())()
if err != nil {
return err
}
if pcRaw == nil {
d.SetId("")
return nil
}
pc := pcRaw.(*ec2.VpcPeeringConnection)

if _, ok := d.GetOk("auto_accept"); ok {
if pc.Status != nil && *pc.Status.Code == "pending-acceptance" {
status, err := resourceVPCPeeringConnectionAccept(conn, d.Id())
if err != nil {
Expand All @@ -224,8 +232,15 @@ func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error
}

if d.HasChange("accepter") || d.HasChange("requester") {
_, ok := d.GetOk("auto_accept")
if !ok && pc.Status != nil && *pc.Status.Code != "active" {
return fmt.Errorf("Unable to modify peering options. The VPC Peering Connection "+
"%q is not active. Please set `auto_accept` attribute to `true`, "+
"or activate VPC Peering Connection manually.", d.Id())
}

if err := resourceVPCPeeringConnectionOptionsModify(d, meta); err != nil {
return err
return errwrap.Wrapf("Error modifying VPC Peering Connection options: {{err}}", err)
}
}

Expand Down Expand Up @@ -255,7 +270,7 @@ func resourceAwsVPCPeeringConnectionStateRefreshFunc(conn *ec2.EC2, id string) r
if ec2err, ok := err.(awserr.Error); ok && ec2err.Code() == "InvalidVpcPeeringConnectionID.NotFound" {
resp = nil
} else {
log.Printf("Error on VPCPeeringConnectionStateRefresh: %s", err)
log.Printf("Error reading VPC Peering Connection details: %s", err)
return nil, "", err
}
}
Expand All @@ -274,7 +289,7 @@ func resourceAwsVPCPeeringConnectionStateRefreshFunc(conn *ec2.EC2, id string) r

func vpcPeeringConnectionOptionsSchema() *schema.Schema {
return &schema.Schema{
Type: schema.TypeList,
Type: schema.TypeSet,
Optional: true,
Computed: true,
MaxItems: 1,
Expand All @@ -283,37 +298,56 @@ func vpcPeeringConnectionOptionsSchema() *schema.Schema {
"allow_remote_vpc_dns_resolution": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Computed: true,
Default: false,
},
"allow_classic_link_to_remote_vpc": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Computed: true,
Default: false,
},
"allow_vpc_to_remote_classic_link": &schema.Schema{
Type: schema.TypeBool,
Optional: true,
Computed: true,
Default: false,
},
},
},
}
}

func flattenPeeringOptions(options *ec2.VpcPeeringConnectionOptionsDescription) (results []map[string]interface{}) {
m := map[string]interface{}{
"allow_remote_vpc_dns_resolution": *options.AllowDnsResolutionFromRemoteVpc,
"allow_classic_link_to_remote_vpc": *options.AllowEgressFromLocalClassicLinkToRemoteVpc,
"allow_vpc_to_remote_classic_link": *options.AllowEgressFromLocalVpcToRemoteClassicLink,
m := make(map[string]interface{})

if options.AllowDnsResolutionFromRemoteVpc != nil {
m["allow_remote_vpc_dns_resolution"] = *options.AllowDnsResolutionFromRemoteVpc
}

if options.AllowEgressFromLocalClassicLinkToRemoteVpc != nil {
m["allow_classic_link_to_remote_vpc"] = *options.AllowEgressFromLocalClassicLinkToRemoteVpc
}

if options.AllowEgressFromLocalVpcToRemoteClassicLink != nil {
m["allow_vpc_to_remote_classic_link"] = *options.AllowEgressFromLocalVpcToRemoteClassicLink
}

results = append(results, m)
return
}

func expandPeeringOptions(m map[string]interface{}) *ec2.PeeringConnectionOptionsRequest {
return &ec2.PeeringConnectionOptionsRequest{
AllowDnsResolutionFromRemoteVpc: aws.Bool(m["allow_remote_vpc_dns_resolution"].(bool)),
AllowEgressFromLocalClassicLinkToRemoteVpc: aws.Bool(m["allow_classic_link_to_remote_vpc"].(bool)),
AllowEgressFromLocalVpcToRemoteClassicLink: aws.Bool(m["allow_vpc_to_remote_classic_link"].(bool)),
r := &ec2.PeeringConnectionOptionsRequest{}

if v, ok := m["allow_remote_vpc_dns_resolution"]; ok {
r.AllowDnsResolutionFromRemoteVpc = aws.Bool(v.(bool))
}

if v, ok := m["allow_classic_link_to_remote_vpc"]; ok {
r.AllowEgressFromLocalClassicLinkToRemoteVpc = aws.Bool(v.(bool))
}

if v, ok := m["allow_vpc_to_remote_classic_link"]; ok {
r.AllowEgressFromLocalVpcToRemoteClassicLink = aws.Bool(v.(bool))
}

return r
}
29 changes: 18 additions & 11 deletions builtin/providers/aws/resource_aws_vpc_peering_connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ func TestAccAWSVPCPeeringConnection_plan(t *testing.T) {
t.Fatal("AWS_ACCOUNT_ID must be set.")
}
},

IDRefreshIgnore: []string{"auto_accept"},

Providers: testAccProviders,
CheckDestroy: testAccCheckAWSVpcPeeringConnectionDestroy,
Steps: []resource.TestStep{
Expand All @@ -85,13 +88,14 @@ func TestAccAWSVPCPeeringConnection_plan(t *testing.T) {

func TestAccAWSVPCPeeringConnection_tags(t *testing.T) {
var connection ec2.VpcPeeringConnection
peerId := os.Getenv("TF_PEER_ID")
if peerId == "" {
t.Skip("Error: TestAccAWSVPCPeeringConnection_tags requires a peer ID to be set.")
}

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
PreCheck: func() {
testAccPreCheck(t)
if os.Getenv("AWS_ACCOUNT_ID") == "" {
t.Fatal("AWS_ACCOUNT_ID must be set.")
}
},

IDRefreshName: "aws_vpc_peering_connection.foo",
IDRefreshIgnore: []string{"auto_accept"},
Expand All @@ -100,7 +104,7 @@ func TestAccAWSVPCPeeringConnection_tags(t *testing.T) {
CheckDestroy: testAccCheckVpcDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: fmt.Sprintf(testAccVpcPeeringConfigTags, peerId),
Config: testAccVpcPeeringConfigTags,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSVpcPeeringConnectionExists(
"aws_vpc_peering_connection.foo",
Expand Down Expand Up @@ -157,7 +161,7 @@ func TestAccAWSVPCPeeringConnection_options(t *testing.T) {
"accepter.#", "1"),
resource.TestCheckResourceAttr(
"aws_vpc_peering_connection.foo",
"accepter.0.allow_remote_vpc_dns_resolution", "true"),
"accepter.1102046665.allow_remote_vpc_dns_resolution", "true"),
testAccCheckAWSVpcPeeringConnectionOptions(
"aws_vpc_peering_connection.foo", "accepter",
&ec2.VpcPeeringConnectionOptionsDescription{
Expand All @@ -170,10 +174,10 @@ func TestAccAWSVPCPeeringConnection_options(t *testing.T) {
"requester.#", "1"),
resource.TestCheckResourceAttr(
"aws_vpc_peering_connection.foo",
"requester.0.allow_classic_link_to_remote_vpc", "true"),
"requester.41753983.allow_classic_link_to_remote_vpc", "true"),
resource.TestCheckResourceAttr(
"aws_vpc_peering_connection.foo",
"requester.0.allow_vpc_to_remote_classic_link", "true"),
"requester.41753983.allow_vpc_to_remote_classic_link", "true"),
testAccCheckAWSVpcPeeringConnectionOptions(
"aws_vpc_peering_connection.foo", "requester",
&ec2.VpcPeeringConnectionOptionsDescription{
Expand All @@ -197,7 +201,7 @@ func TestAccAWSVPCPeeringConnection_options(t *testing.T) {
"accepter.#", "1"),
resource.TestCheckResourceAttr(
"aws_vpc_peering_connection.foo",
"accepter.0.allow_remote_vpc_dns_resolution", "true"),
"accepter.1102046665.allow_remote_vpc_dns_resolution", "true"),
testAccCheckAWSVpcPeeringConnectionOptions(
"aws_vpc_peering_connection.foo", "accepter",
&ec2.VpcPeeringConnectionOptionsDescription{
Expand Down Expand Up @@ -355,7 +359,7 @@ resource "aws_vpc" "bar" {
resource "aws_vpc_peering_connection" "foo" {
vpc_id = "${aws_vpc.foo.id}"
peer_vpc_id = "${aws_vpc.bar.id}"
peer_owner_id = "%s"
auto_accept = true
tags {
foo = "bar"
}
Expand All @@ -365,6 +369,9 @@ resource "aws_vpc_peering_connection" "foo" {
const testAccVpcPeeringConfigOptions = `
resource "aws_vpc" "foo" {
cidr_block = "10.0.0.0/16"
tags {
Name = "TestAccAWSVPCPeeringConnection_options"
}
}
resource "aws_vpc" "bar" {
Expand Down
5 changes: 5 additions & 0 deletions website/source/docs/providers/aws/r/vpc_peering.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ resource "aws_vpc" "bar" {

## Argument Reference

-> **Note:** Modifying the VPC Peering Connection options requires peering to be active. An automatic activation
can be done using the [`auto_accept`](vpc_peering.html#auto_accept) attribute. Alternatively, the VPC Peering
Connection has to be made active manually using other means. See [notes](vpc_peering.html#notes) below for
more information.

The following arguments are supported:

* `peer_owner_id` - (Required) The AWS account ID of the owner of the peer VPC.
Expand Down

0 comments on commit 341c7bf

Please sign in to comment.