provider/aws: Update Route53 Record to schema v1, normalizing name

The `name` attribute will always be normalized to a FQDN, with a trailing "dot"
at the end when returned from the API.

We store the name as it's provided in the configuration, so "www" stays as "www"
and "www.terraformtesting.io." stays as "www.terraformtesting.io.".

The problem here is that if we use a full name as above, and the configuraiton
does *not* include the trailing dot, the API will return a version that does,
and we'll have a conflict.

This is particularly bad when we have a lifecycle block with
`create_before_destroy`; the record will get an update posted (which ends up
being a no-op on AWS's side), but then we'll delete the same record immediately
after, resulting in no record at all.

This PR addresses that by trimming the trailing dot from the `name` when saving
to state. We migrate existing state to match, to avoid false-positive diffs.

builtin/providers/aws/resource_aws_route53_record.go

@@ -27,13 +27,16 @@ func resourceAwsRoute53Record() *schema.Resource {
 		Update: resourceAwsRoute53RecordUpdate,
 		Delete: resourceAwsRoute53RecordDelete,
 
+		SchemaVersion: 1,
+		MigrateState:  resourceAwsRoute53RecordMigrateState,
+
 		Schema: map[string]*schema.Schema{
 			"name": &schema.Schema{
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,
 				StateFunc: func(v interface{}) string {
-					value := v.(string)
+					value := strings.TrimSuffix(v.(string), ".")
 					return strings.ToLower(value)
 				},
 			},
@@ -330,6 +333,7 @@ func findRecord(d *schema.ResourceData, meta interface{}) (*route53.ResourceReco
 		}
 		return nil, err
 	}
+
 	en := expandRecordName(d.Get("name").(string), *zoneRecord.HostedZone.Name)
 	log.Printf("[DEBUG] Expanded record name: %s", en)
 	d.Set("fqdn", en)

builtin/providers/aws/resource_aws_route53_record_migrate.go

@@ -0,0 +1,33 @@
+package aws
+
+import (
+	"fmt"
+	"log"
+	"strings"
+
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func resourceAwsRoute53RecordMigrateState(
+	v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) {
+	switch v {
+	case 0:
+		log.Println("[INFO] Found AWS Route53 Record State v0; migrating to v1")
+		return migrateRoute53RecordStateV0toV1(is)
+	default:
+		return is, fmt.Errorf("Unexpected schema version: %d", v)
+	}
+}
+
+func migrateRoute53RecordStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) {
+	if is.Empty() {
+		log.Println("[DEBUG] Empty InstanceState; nothing to migrate.")
+		return is, nil
+	}
+
+	log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes)
+	newName := strings.TrimSuffix(is.Attributes["name"], ".")
+	is.Attributes["name"] = newName
+	log.Printf("[DEBUG] Attributes after migration: %#v, new name: %s", is.Attributes, newName)
+	return is, nil
+}

builtin/providers/aws/resource_aws_route53_record_migrate_test.go

@@ -0,0 +1,59 @@
+package aws
+
+import (
+	"testing"
+
+	"github.com/hashicorp/terraform/terraform"
+)
+
+func TestAWSRoute53RecordMigrateState(t *testing.T) {
+	cases := map[string]struct {
+		StateVersion int
+		ID           string
+		Attributes   map[string]string
+		Expected     string
+		Meta         interface{}
+	}{
+		"v0_0": {
+			StateVersion: 0,
+			ID:           "some_id",
+			Attributes: map[string]string{
+				"name": "www",
+			},
+			Expected: "www",
+		},
+		"v0_1": {
+			StateVersion: 0,
+			ID:           "some_id",
+			Attributes: map[string]string{
+				"name": "www.notdomain.com.",
+			},
+			Expected: "www.notdomain.com",
+		},
+		"v0_2": {
+			StateVersion: 0,
+			ID:           "some_id",
+			Attributes: map[string]string{
+				"name": "www.notdomain.com",
+			},
+			Expected: "www.notdomain.com",
+		},
+	}
+
+	for tn, tc := range cases {
+		is := &terraform.InstanceState{
+			ID:         tc.ID,
+			Attributes: tc.Attributes,
+		}
+		is, err := resourceAwsRoute53RecordMigrateState(
+			tc.StateVersion, is, tc.Meta)
+
+		if err != nil {
+			t.Fatalf("bad: %s, err: %#v", tn, err)
+		}
+
+		if is.Attributes["name"] != tc.Expected {
+			t.Fatalf("bad Route 53 Migrate: %s\n\n expected: %s", is.Attributes["name"], tc.Expected)
+		}
+	}
+}

builtin/providers/aws/resource_aws_route53_record_test.go

@@ -68,6 +68,36 @@ func TestAccAWSRoute53Record_basic(t *testing.T) {
 	})
 }
 
+func TestAccAWSRoute53Record_basic_fqdn(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:      func() { testAccPreCheck(t) },
+		IDRefreshName: "aws_route53_record.default",
+		Providers:     testAccProviders,
+		CheckDestroy:  testAccCheckRoute53RecordDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccRoute53RecordConfig_fqdn,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53RecordExists("aws_route53_record.default"),
+				),
+			},
+
+			// Ensure that changing the name to include a trailing "dot" results in
+			// nothing happening, because the name is stripped of trailing dots on
+			// save. Otherwise, an update would occur and due to the
+			// create_before_destroy, the record would actually be destroyed, and a
+			// non-empty plan would appear, and the record will fail to exist in
+			// testAccCheckRoute53RecordExists
+			resource.TestStep{
+				Config: testAccRoute53RecordConfig_fqdn_no_op,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53RecordExists("aws_route53_record.default"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAWSRoute53Record_txtSupport(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:        func() { testAccPreCheck(t) },
@@ -416,6 +446,41 @@ resource "aws_route53_record" "default" {
 	records = ["127.0.0.1", "127.0.0.27"]
 }
 `
+const testAccRoute53RecordConfig_fqdn = `
+resource "aws_route53_zone" "main" {
+  name = "notexample.com"
+}
+
+resource "aws_route53_record" "default" {
+  zone_id = "${aws_route53_zone.main.zone_id}"
+  name    = "www.NOTexamplE.com"
+  type    = "A"
+  ttl     = "30"
+  records = ["127.0.0.1", "127.0.0.27"]
+
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+`
+
+const testAccRoute53RecordConfig_fqdn_no_op = `
+resource "aws_route53_zone" "main" {
+  name = "notexample.com"
+}
+
+resource "aws_route53_record" "default" {
+  zone_id = "${aws_route53_zone.main.zone_id}"
+  name    = "www.NOTexamplE.com."
+  type    = "A"
+  ttl     = "30"
+  records = ["127.0.0.1", "127.0.0.27"]
+
+  lifecycle {
+    create_before_destroy = true
+  }
+}
+`
 
 const testAccRoute53RecordNoConfig = `
 resource "aws_route53_zone" "main" {