Merge pull request #6592 from hashicorp/b-aws-r53-name-state

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

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