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 <krzysztof.wilczynski@linux.com>

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

Signed-off-by: Krzysztof Wilczynski <krzysztof.wilczynski@linux.com>

* 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

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("accept_status", pc.Status.Code)
-	d.Set("peer_owner_id", *pc.AccepterVpcInfo.OwnerId)
+	d.Set("peer_owner_id", pc.AccepterVpcInfo.OwnerId)
-	d.Set("peer_vpc_id", *pc.AccepterVpcInfo.VpcId)
+	d.Set("peer_vpc_id", pc.AccepterVpcInfo.VpcId)
-	d.Set("vpc_id", *pc.RequesterVpcInfo.VpcId)
+	d.Set("vpc_id", pc.RequesterVpcInfo.VpcId)
 
-	err = d.Set("accepter", flattenPeeringOptions(pc.AccepterVpcInfo.PeeringOptions))
+	// When the VPC Peering Connection is pending acceptance,
-	if err != nil {
+	// the details about accepter and/or requester peering
-		return err
+	// 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 pc.RequesterVpcInfo != nil && pc.RequesterVpcInfo.PeeringOptions != nil {
-	if err != nil {
+		err := d.Set("requester", flattenPeeringOptions(pc.RequesterVpcInfo.PeeringOptions))
-		return err
+		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 {
+		if s := v.(*schema.Set); len(s.List()) > 0 {
-			modifyOpts.AccepterPeeringConnectionOptions = expandPeeringOptions(
+			co := s.List()[0].(map[string]interface{})
-				s[0].(map[string]interface{}))
+			modifyOpts.AccepterPeeringConnectionOptions = expandPeeringOptions(co)
 		}
 	}
 
 	if v, ok := d.GetOk("requester"); ok {
-		if s := v.([]interface{}); len(s) > 0 {
+		if s := v.(*schema.Set); len(s.List()) > 0 {
-			modifyOpts.RequesterPeeringConnectionOptions = expandPeeringOptions(
+			co := s.List()[0].(map[string]interface{})
-				s[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{}{
+	m := make(map[string]interface{})
-		"allow_remote_vpc_dns_resolution":  *options.AllowDnsResolutionFromRemoteVpc,
+
-		"allow_classic_link_to_remote_vpc": *options.AllowEgressFromLocalClassicLinkToRemoteVpc,
+	if options.AllowDnsResolutionFromRemoteVpc != nil {
-		"allow_vpc_to_remote_classic_link": *options.AllowEgressFromLocalVpcToRemoteClassicLink,
+		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{
+	r := &ec2.PeeringConnectionOptionsRequest{}
-		AllowDnsResolutionFromRemoteVpc:            aws.Bool(m["allow_remote_vpc_dns_resolution"].(bool)),
+
-		AllowEgressFromLocalClassicLinkToRemoteVpc: aws.Bool(m["allow_classic_link_to_remote_vpc"].(bool)),
+	if v, ok := m["allow_remote_vpc_dns_resolution"]; ok {
-		AllowEgressFromLocalVpcToRemoteClassicLink: aws.Bool(m["allow_vpc_to_remote_classic_link"].(bool)),
+		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
 }

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" {

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.