From 8807cdd8f46c55d3e1f8bdead45a08a7a9fe56d2 Mon Sep 17 00:00:00 2001
From: Michael Steger <mjsteger1@gmail.com>
Date: Wed, 3 May 2017 14:41:48 -0700
Subject: [PATCH] Fix parsing of digitalocean dns records

This changeset fixes how some digitalocean dns records were getting
parsed. In particular, it allows for understanding "@" as shorthand for
the domain itself, preventing terraform from suggesting changes that
wouldn't have any actual effect. This changeset also adds a trailing "."
to certain record types which are required to be submitted with a
trailing dot, but which digitalocean does not return with a trailing
dot, again preventing changes that wouldn't have an effect.

Tests have been added for the above, and with just adding the tests, the
current code is failing, as it is handling some records(e.g. MX)
incorrectly
---
 .../resource_digitalocean_record.go           | 11 +--
 .../resource_digitalocean_record_test.go      | 88 +++++++++++++++++++
 2 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/builtin/providers/digitalocean/resource_digitalocean_record.go b/builtin/providers/digitalocean/resource_digitalocean_record.go
index 40ff3730aa16..794787eae6ad 100644
--- a/builtin/providers/digitalocean/resource_digitalocean_record.go
+++ b/builtin/providers/digitalocean/resource_digitalocean_record.go
@@ -132,16 +132,11 @@ func resourceDigitalOceanRecordRead(d *schema.ResourceData, meta interface{}) er
 		return err
 	}
 
-	// Update response data for records with domain value
 	if t := rec.Type; t == "CNAME" || t == "MX" || t == "NS" || t == "SRV" {
-		// Append dot to response if resource value is absolute
-		if value := d.Get("value").(string); strings.HasSuffix(value, ".") {
-			rec.Data += "."
-			// If resource value ends with current domain, make response data absolute
-			if strings.HasSuffix(value, domain+".") {
-				rec.Data += domain + "."
-			}
+		if rec.Data == "@" {
+			rec.Data = domain
 		}
+		rec.Data += "."
 	}
 
 	d.Set("name", rec.Name)
diff --git a/builtin/providers/digitalocean/resource_digitalocean_record_test.go b/builtin/providers/digitalocean/resource_digitalocean_record_test.go
index af4066ca44d5..bb62db2408c1 100644
--- a/builtin/providers/digitalocean/resource_digitalocean_record_test.go
+++ b/builtin/providers/digitalocean/resource_digitalocean_record_test.go
@@ -164,6 +164,64 @@ func TestAccDigitalOceanRecord_ExternalHostnameValue(t *testing.T) {
 	})
 }
 
+func TestAccDigitalOceanRecord_MX(t *testing.T) {
+	var record godo.DomainRecord
+	domain := fmt.Sprintf("foobar-test-terraform-%s.com", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckDigitalOceanRecordDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(
+					testAccCheckDigitalOceanRecordConfig_mx, domain),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDigitalOceanRecordExists("digitalocean_record.foo_record", &record),
+					testAccCheckDigitalOceanRecordAttributesHostname("foobar."+domain, &record),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "name", "terraform"),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "domain", domain),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "value", "foobar."+domain+"."),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "type", "MX"),
+				),
+			},
+		},
+	})
+}
+
+func TestAccDigitalOceanRecord_MX_at(t *testing.T) {
+	var record godo.DomainRecord
+	domain := fmt.Sprintf("foobar-test-terraform-%s.com", acctest.RandString(10))
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckDigitalOceanRecordDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(
+					testAccCheckDigitalOceanRecordConfig_mx_at, domain),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDigitalOceanRecordExists("digitalocean_record.foo_record", &record),
+					testAccCheckDigitalOceanRecordAttributesHostname("@", &record),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "name", "terraform"),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "domain", domain),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "value", domain+"."),
+					resource.TestCheckResourceAttr(
+						"digitalocean_record.foo_record", "type", "MX"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckDigitalOceanRecordDestroy(s *terraform.State) error {
 	client := testAccProvider.Meta().(*godo.Client)
 
@@ -298,6 +356,36 @@ resource "digitalocean_record" "foobar" {
   type  = "CNAME"
 }`
 
+const testAccCheckDigitalOceanRecordConfig_mx_at = `
+resource "digitalocean_domain" "foobar" {
+  name       = "%s"
+  ip_address = "192.168.0.10"
+}
+
+resource "digitalocean_record" "foo_record" {
+  domain = "${digitalocean_domain.foobar.name}"
+
+  name  = "terraform"
+  value = "${digitalocean_domain.foobar.name}."
+  type  = "MX"
+  priority = "10"
+}`
+
+const testAccCheckDigitalOceanRecordConfig_mx = `
+resource "digitalocean_domain" "foobar" {
+  name       = "%s"
+  ip_address = "192.168.0.10"
+}
+
+resource "digitalocean_record" "foo_record" {
+  domain = "${digitalocean_domain.foobar.name}"
+
+  name  = "terraform"
+  value = "foobar.${digitalocean_domain.foobar.name}."
+  type  = "MX"
+  priority = "10"
+}`
+
 const testAccCheckDigitalOceanRecordConfig_external_cname = `
 resource "digitalocean_domain" "foobar" {
   name       = "%s"