service/route53: Finalize cross-account VPC association handling

Reference: #10333 Reference: #13527 Reference: #13826 Reference: #14215 Changes: * Add disappears testing to aws_route53_vpc_association_authorization. * Ensure aws_route53_vpc_association_authorization example shows implicit ordering for downstream aws_route53_zone_association usage * Fix aws_route53_zone_association VPC Region handling by including it in the ID, falling back to the attribute state, or falling back to the provider region. Document additional import handling. * Add aws_route53_zone_association error handling to remove resource from state. * Standardize aws_route53_zone_association disappears testing. Output from acceptance testing: ``` --- PASS: TestAccAWSRoute53VpcAssociationAuthorization_disappears (110.26s) --- PASS: TestAccAWSRoute53VpcAssociationAuthorization_basic (113.19s) --- PASS: TestAccAWSRoute53ZoneAssociation_CrossAccount (211.48s) --- PASS: TestAccAWSRoute53ZoneAssociation_CrossRegion (275.34s) --- PASS: TestAccAWSRoute53ZoneAssociation_disappears_Zone (280.22s) --- PASS: TestAccAWSRoute53ZoneAssociation_disappears (281.72s) --- PASS: TestAccAWSRoute53ZoneAssociation_disappears_VPC (281.85s) --- PASS: TestAccAWSRoute53ZoneAssociation_basic (283.83s) ```
hashicorp · Aug 6, 2020 · fb151fe · fb151fe
1 parent 22aee4f
commit fb151fe
Show file tree

Hide file tree

Showing 5 changed files with 215 additions and 109 deletions.
diff --git a/aws/resource_aws_route53_vpc_association_authorization_test.go b/aws/resource_aws_route53_vpc_association_authorization_test.go
@@ -40,6 +40,30 @@ func TestAccAWSRoute53VpcAssociationAuthorization_basic(t *testing.T) {
 	})
 }
 
+func TestAccAWSRoute53VpcAssociationAuthorization_disappears(t *testing.T) {
+	var providers []*schema.Provider
+	resourceName := "aws_route53_vpc_association_authorization.test"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck: func() {
+			testAccPreCheck(t)
+			testAccAlternateAccountPreCheck(t)
+		},
+		ProviderFactories: testAccProviderFactories(&providers),
+		CheckDestroy:      testAccCheckRoute53VPCAssociationAuthorizationDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccRoute53VPCAssociationAuthorizationConfig(),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53VPCAssociationAuthorizationExists(resourceName),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsRoute53VPCAssociationAuthorization(), resourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
 func testAccCheckRoute53VPCAssociationAuthorizationDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).r53conn
 
@@ -48,7 +72,7 @@ func testAccCheckRoute53VPCAssociationAuthorizationDestroy(s *terraform.State) e
 			continue
 		}
 
-		zone_id, vpc_id, err := resourceAwsRoute53ZoneAssociationParseId(rs.Primary.ID)
+		zone_id, vpc_id, err := resourceAwsRoute53VPCAssociationAuthorizationParseId(rs.Primary.ID)
 		if err != nil {
 			return err
 		}
@@ -85,7 +109,7 @@ func testAccCheckRoute53VPCAssociationAuthorizationExists(n string) resource.Tes
 			return fmt.Errorf("No VPC association authorization ID is set")
 		}
 
-		zone_id, vpc_id, err := resourceAwsRoute53ZoneAssociationParseId(rs.Primary.ID)
+		zone_id, vpc_id, err := resourceAwsRoute53VPCAssociationAuthorizationParseId(rs.Primary.ID)
 		if err != nil {
 			return err
 		}

diff --git a/aws/resource_aws_route53_zone_association.go b/aws/resource_aws_route53_zone_association.go
@@ -51,42 +51,46 @@ func resourceAwsRoute53ZoneAssociation() *schema.Resource {
 }
 
 func resourceAwsRoute53ZoneAssociationCreate(d *schema.ResourceData, meta interface{}) error {
-	r53 := meta.(*AWSClient).r53conn
+	conn := meta.(*AWSClient).r53conn
+
+	vpcRegion := meta.(*AWSClient).region
+	vpcID := d.Get("vpc_id").(string)
+	zoneID := d.Get("zone_id").(string)
+
+	if v, ok := d.GetOk("vpc_region"); ok {
+		vpcRegion = v.(string)
+	}
 
-	req := &route53.AssociateVPCWithHostedZoneInput{
-		HostedZoneId: aws.String(d.Get("zone_id").(string)),
+	input := &route53.AssociateVPCWithHostedZoneInput{
+		HostedZoneId: aws.String(zoneID),
 		VPC: &route53.VPC{
-			VPCId:     aws.String(d.Get("vpc_id").(string)),
-			VPCRegion: aws.String(meta.(*AWSClient).region),
+			VPCId:     aws.String(vpcID),
+			VPCRegion: aws.String(vpcRegion),
 		},
 		Comment: aws.String("Managed by Terraform"),
 	}
-	if w := d.Get("vpc_region"); w != "" {
-		req.VPC.VPCRegion = aws.String(w.(string))
-	}
 
-	log.Printf("[DEBUG] Associating Route53 Private Zone %s with VPC %s with region %s", *req.HostedZoneId, *req.VPC.VPCId, *req.VPC.VPCRegion)
+	output, err := conn.AssociateVPCWithHostedZone(input)
 
-	resp, err := r53.AssociateVPCWithHostedZone(req)
 	if err != nil {
-		return err
+		return fmt.Errorf("error associating Route 53 Hosted Zone (%s) to EC2 VPC (%s): %w", zoneID, vpcID, err)
 	}
 
-	// Store association id
-	d.SetId(fmt.Sprintf("%s:%s", *req.HostedZoneId, *req.VPC.VPCId))
+	d.SetId(fmt.Sprintf("%s:%s:%s", zoneID, vpcID, vpcRegion))
 
-	// Wait until we are done initializing
-	wait := resource.StateChangeConf{
-		Delay:      30 * time.Second,
-		Pending:    []string{route53.ChangeStatusPending},
-		Target:     []string{route53.ChangeStatusInsync},
-		Timeout:    10 * time.Minute,
-		MinTimeout: 2 * time.Second,
-		Refresh:    resourceAwsRoute53ZoneAssociationRefreshFunc(r53, cleanChangeID(*resp.ChangeInfo.Id), d.Id()),
-	}
-	_, err = wait.WaitForState()
-	if err != nil {
-		return err
+	if output != nil && output.ChangeInfo != nil && output.ChangeInfo.Id != nil {
+		wait := resource.StateChangeConf{
+			Delay:      30 * time.Second,
+			Pending:    []string{route53.ChangeStatusPending},
+			Target:     []string{route53.ChangeStatusInsync},
+			Timeout:    10 * time.Minute,
+			MinTimeout: 2 * time.Second,
+			Refresh:    resourceAwsRoute53ZoneAssociationRefreshFunc(conn, cleanChangeID(aws.StringValue(output.ChangeInfo.Id)), d.Id()),
+		}
+
+		if _, err := wait.WaitForState(); err != nil {
+			return fmt.Errorf("error waiting for Route 53 Zone Association (%s) synchronization: %w", d.Id(), err)
+		}
 	}
 
 	return resourceAwsRoute53ZoneAssociationRead(d, meta)
@@ -95,20 +99,38 @@ func resourceAwsRoute53ZoneAssociationCreate(d *schema.ResourceData, meta interf
 func resourceAwsRoute53ZoneAssociationRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).r53conn
 
-	zoneID, vpcID, err := resourceAwsRoute53ZoneAssociationParseId(d.Id())
-	vpcRegion := meta.(*AWSClient).region
+	zoneID, vpcID, vpcRegion, err := resourceAwsRoute53ZoneAssociationParseId(d.Id())
 
 	if err != nil {
 		return err
 	}
 
+	// Continue supporting older resources without VPC Region in ID
+	if vpcRegion == "" {
+		vpcRegion = d.Get("vpc_region").(string)
+	}
+
+	if vpcRegion == "" {
+		vpcRegion = meta.(*AWSClient).region
+	}
+
 	hostedZoneSummary, err := route53GetZoneAssociation(conn, zoneID, vpcID, vpcRegion)
 
+	if isAWSErr(err, "AccessDenied", "is not owned by you") && !d.IsNewResource() {
+		log.Printf("[WARN] Route 53 Zone Association (%s) not found, removing from state", d.Id())
+		d.SetId("")
+		return nil
+	}
+
 	if err != nil {
-		return fmt.Errorf("error getting Route 53 Hosted Zone (%s): %s", zoneID, err)
+		return fmt.Errorf("error getting Route 53 Zone Association (%s): %w", d.Id(), err)
 	}
 
 	if hostedZoneSummary == nil {
+		if d.IsNewResource() {
+			return fmt.Errorf("error getting Route 53 Zone Association (%s): missing after creation", d.Id())
+		}
+
 		log.Printf("[WARN] Route 53 Hosted Zone (%s) Association (%s) not found, removing from state", zoneID, vpcID)
 		d.SetId("")
 		return nil
@@ -125,38 +147,51 @@ func resourceAwsRoute53ZoneAssociationRead(d *schema.ResourceData, meta interfac
 func resourceAwsRoute53ZoneAssociationDelete(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).r53conn
 
-	zoneID, vpcID, err := resourceAwsRoute53ZoneAssociationParseId(d.Id())
+	zoneID, vpcID, vpcRegion, err := resourceAwsRoute53ZoneAssociationParseId(d.Id())
 
 	if err != nil {
 		return err
 	}
 
-	log.Printf("[DEBUG] Disassociating Route 53 Hosted Zone (%s) Association: %s", zoneID, vpcID)
+	// Continue supporting older resources without VPC Region in ID
+	if vpcRegion == "" {
+		vpcRegion = d.Get("vpc_region").(string)
+	}
+
+	if vpcRegion == "" {
+		vpcRegion = meta.(*AWSClient).region
+	}
 
-	req := &route53.DisassociateVPCFromHostedZoneInput{
+	input := &route53.DisassociateVPCFromHostedZoneInput{
 		HostedZoneId: aws.String(zoneID),
 		VPC: &route53.VPC{
 			VPCId:     aws.String(vpcID),
-			VPCRegion: aws.String(d.Get("vpc_region").(string)),
+			VPCRegion: aws.String(vpcRegion),
 		},
 		Comment: aws.String("Managed by Terraform"),
 	}
 
-	_, err = conn.DisassociateVPCFromHostedZone(req)
+	_, err = conn.DisassociateVPCFromHostedZone(input)
 
 	if err != nil {
-		return fmt.Errorf("error disassociating Route 53 Hosted Zone (%s) Association (%s): %s", zoneID, vpcID, err)
+		return fmt.Errorf("error disassociating Route 53 Hosted Zone (%s) from EC2 VPC (%s): %w", zoneID, vpcID, err)
 	}
 
 	return nil
 }
 
-func resourceAwsRoute53ZoneAssociationParseId(id string) (string, string, error) {
+func resourceAwsRoute53ZoneAssociationParseId(id string) (string, string, string, error) {
 	parts := strings.Split(id, ":")
+
+	if len(parts) == 3 && parts[0] != "" && parts[1] != "" && parts[2] != "" {
+		return parts[0], parts[1], parts[2], nil
+	}
+
 	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
-		return "", "", fmt.Errorf("Unexpected format of ID (%q), expected ZONEID:VPCID", id)
+		return "", "", "", fmt.Errorf("Unexpected format of ID (%q), expected ZONEID:VPCID or ZONEID:VPCID:VPCREGION", id)
 	}
-	return parts[0], parts[1], nil
+
+	return parts[0], parts[1], "", nil
 }
 
 func resourceAwsRoute53ZoneAssociationRefreshFunc(conn *route53.Route53, changeId, id string) resource.StateRefreshFunc {