aws_route53_record: Only unquote the record types that we quote. (hashicorp#11257)

Also clean up the code a little.

Author: cakoose

builtin/providers/aws/resource_aws_route53_record.go

@@ -484,7 +484,7 @@ func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) erro
 		}
 	}
 
-	err = d.Set("records", flattenResourceRecords(record.ResourceRecords))
+	err = d.Set("records", flattenResourceRecords(record.ResourceRecords, *record.Type))
 	if err != nil {
 		return fmt.Errorf("[DEBUG] Error setting records for: %s, error: %#v", d.Id(), err)
 	}

builtin/providers/aws/structure.go

@@ -814,11 +814,14 @@ func flattenStepAdjustments(adjustments []*autoscaling.StepAdjustment) []map[str
 	return result
 }
 
-func flattenResourceRecords(recs []*route53.ResourceRecord) []string {
+func flattenResourceRecords(recs []*route53.ResourceRecord, typeStr string) []string {
 	strs := make([]string, 0, len(recs))
 	for _, r := range recs {
 		if r.Value != nil {
-			s := strings.Replace(*r.Value, "\"", "", 2)
+			s := *r.Value
+			if typeStr == "TXT" || typeStr == "SPF" {
+				s = strings.Replace(s, "\"", "", 2)
+			}
 			strs = append(strs, s)
 		}
 	}
@@ -829,13 +832,11 @@ func expandResourceRecords(recs []interface{}, typeStr string) []*route53.Resour
 	records := make([]*route53.ResourceRecord, 0, len(recs))
 	for _, r := range recs {
 		s := r.(string)
-		switch typeStr {
-		case "TXT", "SPF":
-			str := fmt.Sprintf("\"%s\"", s)
-			records = append(records, &route53.ResourceRecord{Value: aws.String(str)})
-		default:
-			records = append(records, &route53.ResourceRecord{Value: aws.String(s)})
+		if typeStr == "TXT" || typeStr == "SPF" {
+			// `flattenResourceRecords` removes quotes.  Add them back.
+			s = fmt.Sprintf("\"%s\"", s)
 		}
+		records = append(records, &route53.ResourceRecord{Value: aws.String(s)})
 	}
 	return records
 }

builtin/providers/aws/structure_test.go

@@ -819,23 +819,56 @@ func TestFlattenStepAdjustments(t *testing.T) {
 }
 
 func TestFlattenResourceRecords(t *testing.T) {
-	expanded := []*route53.ResourceRecord{
-		&route53.ResourceRecord{
-			Value: aws.String("127.0.0.1"),
-		},
-		&route53.ResourceRecord{
-			Value: aws.String("127.0.0.3"),
-		},
+	original := []string{
+		`127.0.0.1`,
+		`"abc def"`,
 	}
 
-	result := flattenResourceRecords(expanded)
+	dequoted := []string{
+		`127.0.0.1`,
+		`abc def`,
+	}
+
+	var wrapped []*route53.ResourceRecord = nil
+	for _, original := range original {
+		wrapped = append(wrapped, &route53.ResourceRecord{Value: aws.String(original)})
+	}
+
+	sub := func(recordType string, expected []string) {
+		t.Run(recordType, func(t *testing.T) {
+			checkFlattenResourceRecords(t, recordType, wrapped, expected)
+		})
+	}
+
+	// These record types should be dequoted.
+	sub("TXT", dequoted)
+	sub("SPF", dequoted)
+
+	// These record types should not be touched.
+	sub("CNAME", original)
+	sub("MX", original)
+}
+
+func checkFlattenResourceRecords(
+	t *testing.T,
+	recordType string,
+	expanded []*route53.ResourceRecord,
+	expected []string) {
+
+	result := flattenResourceRecords(expanded, recordType)
 
 	if result == nil {
 		t.Fatal("expected result to have value, but got nil")
 	}
 
-	if len(result) != 2 {
-		t.Fatal("expected result to have value, but got nil")
+	if len(result) != len(expected) {
+		t.Fatalf("expected %v, got %v", expected, result)
+	}
+
+	for i, e := range expected {
+		if result[i] != e {
+			t.Fatalf("expected %v, got %v", expected, result)
+		}
 	}
 }