From 125987994c7fe275191edca68caf52ea3519a05f Mon Sep 17 00:00:00 2001 From: clint shryock Date: Thu, 28 Jan 2016 16:44:24 -0600 Subject: [PATCH 1/3] provider/aws: Refactor Route53 record to fix regression in deleting refactored to add a `findRecord` method to find the matching record set, and use that for the `DELETE` method call. --- .../aws/resource_aws_route53_record.go | 90 ++++++++----------- 1 file changed, 39 insertions(+), 51 deletions(-) diff --git a/builtin/providers/aws/resource_aws_route53_record.go b/builtin/providers/aws/resource_aws_route53_record.go index 84a1e3ac4..e4506fa6b 100644 --- a/builtin/providers/aws/resource_aws_route53_record.go +++ b/builtin/providers/aws/resource_aws_route53_record.go @@ -151,7 +151,7 @@ func resourceAwsRoute53RecordCreate(d *schema.ResourceData, meta interface{}) er return fmt.Errorf("[WARN] No Route53 Zone found for id (%s)", zone) } - // Get the record + // Build the record rec, err := resourceAwsRoute53RecordBuildSet(d, *zoneRecord.HostedZone.Name) if err != nil { return err @@ -242,19 +242,44 @@ func resourceAwsRoute53RecordCreate(d *schema.ResourceData, meta interface{}) er } func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) error { - conn := meta.(*AWSClient).r53conn + record, err := findRecord(d, meta) + if err != nil { + log.Printf("[DEBUG] No matching record found for: %s, removing from state file", d.Id()) + d.SetId("") + return nil + } + err = d.Set("records", flattenResourceRecords(record.ResourceRecords)) + if err != nil { + return fmt.Errorf("[DEBUG] Error setting records for: %s, error: %#v", d.Id(), err) + } + + d.Set("ttl", record.TTL) + // Only set the weight if it's non-nil, otherwise we end up with a 0 weight + // which has actual contextual meaning with Route 53 records + // See http://docs.aws.amazon.com/fr_fr/Route53/latest/APIReference/API_ChangeResourceRecordSets_Examples.html + if record.Weight != nil { + d.Set("weight", record.Weight) + } + d.Set("set_identifier", record.SetIdentifier) + d.Set("failover", record.Failover) + d.Set("health_check_id", record.HealthCheckId) + + return nil +} + +func findRecord(d *schema.ResourceData, meta interface{}) (*route53.ResourceRecordSet, error) { + conn := meta.(*AWSClient).r53conn + // Scan for a zone := cleanZoneID(d.Get("zone_id").(string)) // get expanded name zoneRecord, err := conn.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(zone)}) if err != nil { if r53err, ok := err.(awserr.Error); ok && r53err.Code() == "NoSuchHostedZone" { - log.Printf("[DEBUG] No matching Route 53 Record found for: %s, removing from state file", d.Id()) - d.SetId("") - return nil + return nil, fmt.Errorf("no such hosted zone") } - return err + return nil, err } en := expandRecordName(d.Get("name").(string), *zoneRecord.HostedZone.Name) log.Printf("[DEBUG] Expanded record name: %s", en) @@ -270,11 +295,9 @@ func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) erro zone, lopts) resp, err := conn.ListResourceRecordSets(lopts) if err != nil { - return err + return nil, err } - // Scan for a matching record - found := false for _, record := range resp.ResourceRecordSets { name := cleanRecordName(*record.Name) if FQDN(strings.ToLower(name)) != FQDN(strings.ToLower(*lopts.StartRecordName)) { @@ -287,55 +310,18 @@ func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) erro if record.SetIdentifier != nil && *record.SetIdentifier != d.Get("set_identifier") { continue } - - found = true - - err := d.Set("records", flattenResourceRecords(record.ResourceRecords)) - if err != nil { - return fmt.Errorf("[DEBUG] Error setting records for: %s, error: %#v", en, err) - } - - d.Set("ttl", record.TTL) - // Only set the weight if it's non-nil, otherwise we end up with a 0 weight - // which has actual contextual meaning with Route 53 records - // See http://docs.aws.amazon.com/fr_fr/Route53/latest/APIReference/API_ChangeResourceRecordSets_Examples.html - if record.Weight != nil { - d.Set("weight", record.Weight) - } - d.Set("set_identifier", record.SetIdentifier) - d.Set("failover", record.Failover) - d.Set("health_check_id", record.HealthCheckId) - - break + return record, nil } - - if !found { - log.Printf("[DEBUG] No matching record found for: %s, removing from state file", en) - d.SetId("") - } - - return nil + return nil, fmt.Errorf("nothing found") } func resourceAwsRoute53RecordDelete(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).r53conn - - zone := cleanZoneID(d.Get("zone_id").(string)) - log.Printf("[DEBUG] Deleting resource records for zone: %s, name: %s", - zone, d.Get("name").(string)) - var err error - zoneRecord, err := conn.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(zone)}) - if err != nil { - if r53err, ok := err.(awserr.Error); ok && r53err.Code() == "NoSuchHostedZone" { - log.Printf("[DEBUG] No matching Route 53 Record found for: %s, removing from state file", d.Id()) - d.SetId("") - return nil - } - return err - } // Get the records - rec, err := resourceAwsRoute53RecordBuildSet(d, *zoneRecord.HostedZone.Name) + rec, err := findRecord(d, meta) if err != nil { + log.Printf("[DEBUG] No matching record found for: %s, removing from state file", d.Id()) + d.SetId("") return err } @@ -350,6 +336,8 @@ func resourceAwsRoute53RecordDelete(d *schema.ResourceData, meta interface{}) er }, } + zone := cleanZoneID(d.Get("zone_id").(string)) + req := &route53.ChangeResourceRecordSetsInput{ HostedZoneId: aws.String(cleanZoneID(zone)), ChangeBatch: changeBatch, From 3bbb21d1151317dda42375d0f7ae3ee2a7968bd9 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Fri, 29 Jan 2016 11:56:19 -0600 Subject: [PATCH 2/3] refactor error handling in findRecord --- .../aws/resource_aws_route53_record.go | 35 ++++++++++++++++--- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/builtin/providers/aws/resource_aws_route53_record.go b/builtin/providers/aws/resource_aws_route53_record.go index e4506fa6b..5d9b8e150 100644 --- a/builtin/providers/aws/resource_aws_route53_record.go +++ b/builtin/providers/aws/resource_aws_route53_record.go @@ -2,6 +2,7 @@ package aws import ( "bytes" + "errors" "fmt" "log" "strings" @@ -16,6 +17,9 @@ import ( "github.com/aws/aws-sdk-go/service/route53" ) +var r53NoRecordsFound = errors.New("No matching Hosted Zone found") +var r53NoHostedZoneFound = errors.New("No matching records found") + func resourceAwsRoute53Record() *schema.Resource { return &schema.Resource{ Create: resourceAwsRoute53RecordCreate, @@ -244,9 +248,14 @@ func resourceAwsRoute53RecordCreate(d *schema.ResourceData, meta interface{}) er func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) error { record, err := findRecord(d, meta) if err != nil { - log.Printf("[DEBUG] No matching record found for: %s, removing from state file", d.Id()) - d.SetId("") - return nil + switch err { + case r53NoHostedZoneFound, r53NoRecordsFound: + log.Printf("[DEBUG] %s for: %s, removing from state file", err, d.Id()) + d.SetId("") + return nil + default: + return err + } } err = d.Set("records", flattenResourceRecords(record.ResourceRecords)) @@ -268,6 +277,21 @@ func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) erro return nil } +// findRecord takes a ResourceData struct for aws_resource_route53_record. It +// uses the referenced zone_id to query Route53 and find information on it's +// records. +// +// If records are found, it returns the matching +// route53.ResourceRecordSet and nil for the error. +// +// If no hosted zone is found, it returns a nil recordset and r53NoHostedZoneFound +// error. +// +// If no matching recordset is found, it returns nil and a r53NoRecordsFound +// error +// +// If there are other errors, it returns nil a nil recordset and passes on the +// error. func findRecord(d *schema.ResourceData, meta interface{}) (*route53.ResourceRecordSet, error) { conn := meta.(*AWSClient).r53conn // Scan for a @@ -277,7 +301,7 @@ func findRecord(d *schema.ResourceData, meta interface{}) (*route53.ResourceReco zoneRecord, err := conn.GetHostedZone(&route53.GetHostedZoneInput{Id: aws.String(zone)}) if err != nil { if r53err, ok := err.(awserr.Error); ok && r53err.Code() == "NoSuchHostedZone" { - return nil, fmt.Errorf("no such hosted zone") + return nil, r53NoHostedZoneFound } return nil, err } @@ -310,9 +334,10 @@ func findRecord(d *schema.ResourceData, meta interface{}) (*route53.ResourceReco if record.SetIdentifier != nil && *record.SetIdentifier != d.Get("set_identifier") { continue } + // The only safe return where a record is found return record, nil } - return nil, fmt.Errorf("nothing found") + return nil, r53NoRecordsFound } func resourceAwsRoute53RecordDelete(d *schema.ResourceData, meta interface{}) error { From 39f5a7e751308885afc24dec5b817d56e9598980 Mon Sep 17 00:00:00 2001 From: clint shryock Date: Fri, 29 Jan 2016 12:38:22 -0600 Subject: [PATCH 3/3] use the same error checking in DELETE --- builtin/providers/aws/resource_aws_route53_record.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/builtin/providers/aws/resource_aws_route53_record.go b/builtin/providers/aws/resource_aws_route53_record.go index 5d9b8e150..2678385c1 100644 --- a/builtin/providers/aws/resource_aws_route53_record.go +++ b/builtin/providers/aws/resource_aws_route53_record.go @@ -345,9 +345,14 @@ func resourceAwsRoute53RecordDelete(d *schema.ResourceData, meta interface{}) er // Get the records rec, err := findRecord(d, meta) if err != nil { - log.Printf("[DEBUG] No matching record found for: %s, removing from state file", d.Id()) - d.SetId("") - return err + switch err { + case r53NoHostedZoneFound, r53NoRecordsFound: + log.Printf("[DEBUG] %s for: %s, removing from state file", err, d.Id()) + d.SetId("") + return nil + default: + return err + } } // Create the new records