Merge pull request #11164 from johanneswuerbach/aws-route53-changeable-record-type

provider/aws: Make the type of a route53_record changeable

builtin/providers/aws/resource_aws_route53_record.go

@@ -51,7 +51,6 @@ func resourceAwsRoute53Record() *schema.Resource {
 			"type": &schema.Schema{
 				Type:         schema.TypeString,
 				Required:     true,
-				ForceNew:     true,
 				ValidateFunc: validateRoute53RecordType,
 			},
 
@@ -83,7 +82,6 @@ func resourceAwsRoute53Record() *schema.Resource {
 			"set_identifier": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
-				ForceNew: true,
 			},
 
 			"alias": &schema.Schema{
@@ -225,13 +223,124 @@ func resourceAwsRoute53RecordUpdate(d *schema.ResourceData, meta interface{}) er
 	// Route 53 supports CREATE, DELETE, and UPSERT actions. We use UPSERT, and
 	// AWS dynamically determines if a record should be created or updated.
 	// Amazon Route 53 can update an existing resource record set only when all
-	// of the following values match: Name, Type
+	// of the following values match: Name, Type and SetIdentifier
-	// (and SetIdentifier, which we don't use yet).
+	// See http://docs.aws.amazon.com/Route53/latest/APIReference/API_ChangeResourceRecordSets.html
-	// See http://docs.aws.amazon.com/Route53/latest/APIReference/API_ChangeResourceRecordSets_Requests.html#change-rrsets-request-action
+
-	//
+	if !d.HasChange("type") && !d.HasChange("set_identifier") {
-	// Because we use UPSERT, for resouce update here we simply fall through to
+		// If neither type nor set_identifier changed we use UPSERT,
-	// our resource create function.
+		// for resouce update here we simply fall through to
-	return resourceAwsRoute53RecordCreate(d, meta)
+		// our resource create function.
+		return resourceAwsRoute53RecordCreate(d, meta)
+	}
+
+	// Otherwise we delete the existing record and create a new record within
+	// a transactional change
+	conn := meta.(*AWSClient).r53conn
+	zone := cleanZoneID(d.Get("zone_id").(string))
+
+	var err error
+	zoneRecord, err := conn.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(zone)})
+	if err != nil {
+		return err
+	}
+	if zoneRecord.HostedZone == nil {
+		return fmt.Errorf("[WARN] No Route53 Zone found for id (%s)", zone)
+	}
+
+	// Build the to be deleted record
+	en := expandRecordName(d.Get("name").(string), *zoneRecord.HostedZone.Name)
+	typeo, _ := d.GetChange("type")
+
+	oldRec := &route53.ResourceRecordSet{
+		Name: aws.String(en),
+		Type: aws.String(typeo.(string)),
+	}
+
+	if v, _ := d.GetChange("ttl"); v.(int) != 0 {
+		oldRec.TTL = aws.Int64(int64(v.(int)))
+	}
+
+	// Resource records
+	if v, _ := d.GetChange("records"); v != nil {
+		recs := v.(*schema.Set).List()
+		if len(recs) > 0 {
+			oldRec.ResourceRecords = expandResourceRecords(recs, typeo.(string))
+		}
+	}
+
+	// Alias record
+	if v, _ := d.GetChange("alias"); v != nil {
+		aliases := v.(*schema.Set).List()
+		if len(aliases) == 1 {
+			alias := aliases[0].(map[string]interface{})
+			oldRec.AliasTarget = &route53.AliasTarget{
+				DNSName:              aws.String(alias["name"].(string)),
+				EvaluateTargetHealth: aws.Bool(alias["evaluate_target_health"].(bool)),
+				HostedZoneId:         aws.String(alias["zone_id"].(string)),
+			}
+		}
+	}
+
+	if v, _ := d.GetChange("set_identifier"); v.(string) != "" {
+		oldRec.SetIdentifier = aws.String(v.(string))
+	}
+
+	// Build the to be created record
+	rec, err := resourceAwsRoute53RecordBuildSet(d, *zoneRecord.HostedZone.Name)
+	if err != nil {
+		return err
+	}
+
+	// Delete the old and create the new records in a single batch. We abuse
+	// StateChangeConf for this to retry for us since Route53 sometimes returns
+	// errors about another operation happening at the same time.
+	changeBatch := &route53.ChangeBatch{
+		Comment: aws.String("Managed by Terraform"),
+		Changes: []*route53.Change{
+			&route53.Change{
+				Action:            aws.String("DELETE"),
+				ResourceRecordSet: oldRec,
+			},
+			&route53.Change{
+				Action:            aws.String("CREATE"),
+				ResourceRecordSet: rec,
+			},
+		},
+	}
+
+	req := &route53.ChangeResourceRecordSetsInput{
+		HostedZoneId: aws.String(cleanZoneID(*zoneRecord.HostedZone.Id)),
+		ChangeBatch:  changeBatch,
+	}
+
+	log.Printf("[DEBUG] Updating resource records for zone: %s, name: %s\n\n%s",
+		zone, *rec.Name, req)
+
+	respRaw, err := changeRoute53RecordSet(conn, req)
+	if err != nil {
+		return errwrap.Wrapf("[ERR]: Error building changeset: {{err}}", err)
+	}
+
+	changeInfo := respRaw.(*route53.ChangeResourceRecordSetsOutput).ChangeInfo
+
+	// Generate an ID
+	vars := []string{
+		zone,
+		strings.ToLower(d.Get("name").(string)),
+		d.Get("type").(string),
+	}
+	if v, ok := d.GetOk("set_identifier"); ok {
+		vars = append(vars, v.(string))
+	}
+
+	d.SetId(strings.Join(vars, "_"))
+
+	err = waitForRoute53RecordSetToSync(conn, cleanChangeID(*changeInfo.Id))
+	if err != nil {
+		return err
+	}
+
+	return resourceAwsRoute53RecordRead(d, meta)
 }
 
 func resourceAwsRoute53RecordCreate(d *schema.ResourceData, meta interface{}) error {

builtin/providers/aws/resource_aws_route53_record_test.go

@@ -339,6 +339,56 @@ func TestAccAWSRoute53Record_TypeChange(t *testing.T) {
 	})
 }
 
+func TestAccAWSRoute53Record_SetIdentiferChange(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:      func() { testAccPreCheck(t) },
+		IDRefreshName: "aws_route53_record.basic_to_weighted",
+		Providers:     testAccProviders,
+		CheckDestroy:  testAccCheckRoute53RecordDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccRoute53RecordSetIdentifierChangePre,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53RecordExists("aws_route53_record.basic_to_weighted"),
+				),
+			},
+
+			// Cause a change, which will trigger a refresh
+			resource.TestStep{
+				Config: testAccRoute53RecordSetIdentifierChangePost,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53RecordExists("aws_route53_record.basic_to_weighted"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccAWSRoute53Record_AliasChange(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:      func() { testAccPreCheck(t) },
+		IDRefreshName: "aws_route53_record.elb_alias_change",
+		Providers:     testAccProviders,
+		CheckDestroy:  testAccCheckRoute53RecordDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccRoute53RecordAliasChangePre,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53RecordExists("aws_route53_record.elb_alias_change"),
+				),
+			},
+
+			// Cause a change, which will trigger a refresh
+			resource.TestStep{
+				Config: testAccRoute53RecordAliasChangePost,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckRoute53RecordExists("aws_route53_record.elb_alias_change"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAWSRoute53Record_empty(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:      func() { testAccPreCheck(t) },
@@ -1022,6 +1072,82 @@ resource "aws_route53_record" "sample" {
 }
 `
 
+const testAccRoute53RecordSetIdentifierChangePre = `
+resource "aws_route53_zone" "main" {
+	name = "notexample.com"
+}
+
+resource "aws_route53_record" "basic_to_weighted" {
+  zone_id = "${aws_route53_zone.main.zone_id}"
+  name = "sample"
+  type = "A"
+  ttl = "30"
+  records = ["127.0.0.1", "8.8.8.8"]
+}
+`
+
+const testAccRoute53RecordSetIdentifierChangePost = `
+resource "aws_route53_zone" "main" {
+	name = "notexample.com"
+}
+
+resource "aws_route53_record" "basic_to_weighted" {
+  zone_id = "${aws_route53_zone.main.zone_id}"
+  name = "sample"
+  type = "A"
+  ttl = "30"
+  records = ["127.0.0.1", "8.8.8.8"]
+  set_identifier = "cluster-a"
+  weighted_routing_policy {
+    weight = 100
+  }
+}
+`
+
+const testAccRoute53RecordAliasChangePre = `
+resource "aws_route53_zone" "main" {
+	name = "notexample.com"
+}
+
+resource "aws_elb" "alias_change" {
+  name = "foobar-tf-elb-alias-change"
+  availability_zones = ["us-west-2a"]
+
+  listener {
+    instance_port = 80
+    instance_protocol = "http"
+    lb_port = 80
+    lb_protocol = "http"
+  }
+}
+
+resource "aws_route53_record" "elb_alias_change" {
+  zone_id = "${aws_route53_zone.main.zone_id}"
+  name = "alias-change"
+  type = "A"
+
+  alias {
+    zone_id = "${aws_elb.alias_change.zone_id}"
+    name = "${aws_elb.alias_change.dns_name}"
+    evaluate_target_health = true
+  }
+}
+`
+
+const testAccRoute53RecordAliasChangePost = `
+resource "aws_route53_zone" "main" {
+	name = "notexample.com"
+}
+
+resource "aws_route53_record" "elb_alias_change" {
+  zone_id = "${aws_route53_zone.main.zone_id}"
+  name = "alias-change"
+  type = "CNAME"
+  ttl = "30"
+  records = ["www.terraform.io"]
+}
+`
+
 const testAccRoute53RecordConfigEmptyName = `
 resource "aws_route53_zone" "main" {
 	name = "notexample.com"