From ef0196f75477896504e91426c3912feaeefa9eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johannes=20W=C3=BCrbach?= Date: Thu, 12 Jan 2017 00:08:20 +0100 Subject: [PATCH] provider/aws: Make the type of a route53_record changeable Utilize the ChangeResourceRecordSets to change the type of a record by deleting and recreating with a new type. As change batches are considered transactional changes, Amazon Route 53 either makes all or none of the changes in the batch request ensuring the update will never be partially applied. --- .../aws/resource_aws_route53_record.go | 127 ++++++++++++++++-- .../aws/resource_aws_route53_record_test.go | 126 +++++++++++++++++ 2 files changed, 244 insertions(+), 9 deletions(-) diff --git a/builtin/providers/aws/resource_aws_route53_record.go b/builtin/providers/aws/resource_aws_route53_record.go index d7d4cadca..e03d79179 100644 --- a/builtin/providers/aws/resource_aws_route53_record.go +++ b/builtin/providers/aws/resource_aws_route53_record.go @@ -49,7 +49,6 @@ func resourceAwsRoute53Record() *schema.Resource { "type": &schema.Schema{ Type: schema.TypeString, Required: true, - ForceNew: true, ValidateFunc: validateRoute53RecordType, }, @@ -81,7 +80,6 @@ func resourceAwsRoute53Record() *schema.Resource { "set_identifier": &schema.Schema{ Type: schema.TypeString, Optional: true, - ForceNew: true, }, "alias": &schema.Schema{ @@ -223,13 +221,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 - // (and SetIdentifier, which we don't use yet). - // See http://docs.aws.amazon.com/Route53/latest/APIReference/API_ChangeResourceRecordSets_Requests.html#change-rrsets-request-action - // - // Because we use UPSERT, for resouce update here we simply fall through to - // our resource create function. - return resourceAwsRoute53RecordCreate(d, meta) + // of the following values match: Name, Type and SetIdentifier + // See http://docs.aws.amazon.com/Route53/latest/APIReference/API_ChangeResourceRecordSets.html + + if !d.HasChange("type") && !d.HasChange("set_identifier") { + // If neither type nor set_identifier changed we use UPSERT, + // for resouce update here we simply fall through to + // 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 { diff --git a/builtin/providers/aws/resource_aws_route53_record_test.go b/builtin/providers/aws/resource_aws_route53_record_test.go index 667337abc..146767bf7 100644 --- a/builtin/providers/aws/resource_aws_route53_record_test.go +++ b/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"