From 341c7bf76625c8609ca1b6bb20a75c54a38c2334 Mon Sep 17 00:00:00 2001 From: Clint Date: Wed, 24 Aug 2016 13:24:42 -0500 Subject: [PATCH] provider/aws: Update VPC Peering connect accept/request attributes (supersedes #8338) (#8432) * Fix crash when reading VPC Peering Connection options. This resolves the issue introduced in #8310. Signed-off-by: Krzysztof Wilczynski * Do not de-reference values when using Set(). Signed-off-by: Krzysztof Wilczynski * 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 --- .../resource_aws_vpc_peering_connection.go | 118 +++++++++++------- ...esource_aws_vpc_peering_connection_test.go | 29 +++-- .../providers/aws/r/vpc_peering.html.markdown | 5 + 3 files changed, 99 insertions(+), 53 deletions(-) diff --git a/builtin/providers/aws/resource_aws_vpc_peering_connection.go b/builtin/providers/aws/resource_aws_vpc_peering_connection.go index 94d3c7f6c..5da41ccff 100644 --- a/builtin/providers/aws/resource_aws_vpc_peering_connection.go +++ b/builtin/providers/aws/resource_aws_vpc_peering_connection.go @@ -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" ) @@ -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 @@ -124,20 +125,28 @@ func resourceAwsVPCPeeringRead(d *schema.ResourceData, meta interface{}) error { return nil } } + 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) + 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 + // 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)) @@ -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) } } @@ -202,18 +211,17 @@ func resourceAwsVPCPeeringUpdate(d *schema.ResourceData, meta interface{}) error d.SetPartial("tags") } + 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 { - pcRaw, _, err := resourceAwsVPCPeeringConnectionStateRefreshFunc(conn, d.Id())() - if err != nil { - return err - } - - if pcRaw == nil { - d.SetId("") - return nil - } - pc := pcRaw.(*ec2.VpcPeeringConnection) - if pc.Status != nil && *pc.Status.Code == "pending-acceptance" { status, err := resourceVPCPeeringConnectionAccept(conn, d.Id()) if err != nil { @@ -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) } } @@ -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 } } @@ -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, @@ -283,17 +298,17 @@ 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, }, }, }, @@ -301,19 +316,38 @@ func vpcPeeringConnectionOptionsSchema() *schema.Schema { } 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 } diff --git a/builtin/providers/aws/resource_aws_vpc_peering_connection_test.go b/builtin/providers/aws/resource_aws_vpc_peering_connection_test.go index 67d6a60ad..3ceb9ee2b 100644 --- a/builtin/providers/aws/resource_aws_vpc_peering_connection_test.go +++ b/builtin/providers/aws/resource_aws_vpc_peering_connection_test.go @@ -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{ @@ -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"}, @@ -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", @@ -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{ @@ -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{ @@ -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{ @@ -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" } @@ -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" { diff --git a/website/source/docs/providers/aws/r/vpc_peering.html.markdown b/website/source/docs/providers/aws/r/vpc_peering.html.markdown index 82b94cdd7..a78abef02 100644 --- a/website/source/docs/providers/aws/r/vpc_peering.html.markdown +++ b/website/source/docs/providers/aws/r/vpc_peering.html.markdown @@ -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.