DNS v2: Fix recordset diffs (terraform-provider-openstack#636)

This commit fixes an issue where the records attribute in the
`openstack_dns_recordset_v2` resource would sometimes be returned in a
different order than what the user specified.

The hardest part about implementing this fix is managing the internal
feature of removing brackets from IPv6 addresses.

In order to successfully continue removing the brackets as well as
retain backwards compatibility, the following changes were made:

1. The `records` field is no longer being set in the Read function. This
is because the `records` field is not computed, so it does not need to
be set during Read.

2. An import function has been created so `records` is set when a user
imports a recordset.

3. `DiffSuppressFunc` has been replaced with a `StateFunc`. Because
`records` is no longer being set in Read, a diff will never occur, but
the modified IPv6 records still need to have records stripped.

4. `records` is being re-set in state during the Create function with
the IPv6 brackets stripped out.

openstack/dns_recordset_v2.go

@@ -7,7 +7,6 @@ import (
 	"strings"
 
 	"github.com/hashicorp/terraform/helper/resource"
-	"github.com/hashicorp/terraform/helper/schema"
 
 	"github.com/gophercloud/gophercloud"
 	"github.com/gophercloud/gophercloud/openstack/dns/v2/recordsets"
@@ -79,17 +78,14 @@ func expandDNSRecordSetV2Records(v []interface{}) []string {
 	return records
 }
 
-// dnsRecordSetV2SuppressRecordDiffs will suppress diffs when the format
-// of a record is different yet still a valid DNS record. For example, if a
-// user specifies an IPv6 address using [bracket] notation, but the record is
-// returned without brackets, it is still a valid record.
-func dnsRecordSetV2SuppressRecordDiffs(k, old, new string, d *schema.ResourceData) bool {
-	re := regexp.MustCompile("[][]")
-	new = re.ReplaceAllString(new, "")
+// dnsRecordSetV2RecordsStateFunc will strip brackets from IPv6 addresses.
+func dnsRecordSetV2RecordsStateFunc(v interface{}) string {
+	if addr, ok := v.(string); ok {
+		re := regexp.MustCompile("[][]")
+		addr = re.ReplaceAllString(addr, "")
 
-	if old == new {
-		return true
+		return addr
 	}
 
-	return false
+	return ""
 }

openstack/dns_recordset_v2_test.go

@@ -17,10 +17,12 @@ func TestDNSRecordSetV2ParseID(t *testing.T) {
 	assert.Equal(t, expectedRecordSetID, actualRecordSetID)
 }
 
-func TestExpandDNSRecordSetV2Records(t *testing.T) {
+func TestDNSRecordSetV2RecordsStateFunc(t *testing.T) {
 	data := []interface{}{"foo", "[bar]", "baz"}
 	expected := []string{"foo", "bar", "baz"}
 
-	actual := expandDNSRecordSetV2Records(data)
-	assert.Equal(t, expected, actual)
+	for i, record := range data {
+		actual := dnsRecordSetV2RecordsStateFunc(record)
+		assert.Equal(t, expected[i], actual)
+	}
 }

openstack/resource_openstack_dns_recordset_v2.go

@@ -18,7 +18,7 @@ func resourceDNSRecordSetV2() *schema.Resource {
 		Update: resourceDNSRecordSetV2Update,
 		Delete: resourceDNSRecordSetV2Delete,
 		Importer: &schema.ResourceImporter{
-			State: schema.ImportStatePassthrough,
+			State: resourceDNSRecordSetV2Import,
 		},
 
 		Timeouts: &schema.ResourceTimeout{
@@ -54,11 +54,13 @@ func resourceDNSRecordSetV2() *schema.Resource {
 			},
 
 			"records": {
-				Type:             schema.TypeList,
-				Optional:         true,
-				ForceNew:         false,
-				Elem:             &schema.Schema{Type: schema.TypeString},
-				DiffSuppressFunc: dnsRecordSetV2SuppressRecordDiffs,
+				Type:     schema.TypeList,
+				Optional: true,
+				ForceNew: false,
+				Elem: &schema.Schema{
+					Type:      schema.TypeString,
+					StateFunc: dnsRecordSetV2RecordsStateFunc,
+				},
 			},
 
 			"ttl": {
@@ -126,6 +128,12 @@ func resourceDNSRecordSetV2Create(d *schema.ResourceData, meta interface{}) erro
 	id := fmt.Sprintf("%s/%s", zoneID, n.ID)
 	d.SetId(id)
 
+	// This is a workaround to store the modified IP addresses in the state
+	// because we don't want to make records computed or change it to TypeSet
+	// in order to retain backwards compatibility.
+	// Because of the StateFunc, this will not cause issues.
+	d.Set("records", records)
+
 	log.Printf("[DEBUG] Created openstack_dns_recordset_v2 %s: %#v", n.ID, n)
 	return resourceDNSRecordSetV2Read(d, meta)
 }
@@ -154,7 +162,6 @@ func resourceDNSRecordSetV2Read(d *schema.ResourceData, meta interface{}) error
 	d.Set("description", n.Description)
 	d.Set("ttl", n.TTL)
 	d.Set("type", n.Type)
-	d.Set("records", n.Records)
 	d.Set("zone_id", zoneID)
 	d.Set("region", GetRegion(d, config))
 
@@ -241,3 +248,24 @@ func resourceDNSRecordSetV2Delete(d *schema.ResourceData, meta interface{}) erro
 
 	return nil
 }
+
+func resourceDNSRecordSetV2Import(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
+	config := meta.(*Config)
+	dnsClient, err := config.dnsV2Client(GetRegion(d, config))
+	if err != nil {
+		return nil, fmt.Errorf("Error creating OpenStack DNS client: %s", err)
+	}
+
+	zoneID, recordsetID, err := dnsRecordSetV2ParseID(d.Id())
+	if err != nil {
+		return nil, err
+	}
+
+	n, err := recordsets.Get(dnsClient, zoneID, recordsetID).Extract()
+	if err != nil {
+		return nil, fmt.Errorf("Error retrieving openstack_dns_recordset_v2 %s: %s", d.Id(), err)
+	}
+
+	d.Set("records", n.Records)
+	return []*schema.ResourceData{d}, nil
+}

website/docs/r/dns_recordset_v2.html.markdown

@@ -54,7 +54,9 @@ The following arguments are supported:
 
 * `description` - (Optional) A description of the  record set.
 
-* `records` - (Optional) An array of DNS records.
+* `records` - (Optional) An array of DNS records. _Note:_ if an IPv6 address
+  contains brackets (`[ ]`), the brackets will be stripped and the modified
+  address will be recorded in the state.
 
 * `value_specs` - (Optional) Map of additional options. Changing this creates a
   new record set.