From 9c0888ca8854220304a4a2a11fce9781bfbce0c9 Mon Sep 17 00:00:00 2001
From: mjsteger <mjsteger1@gmail.com>
Date: Tue, 16 May 2017 02:10:34 -0700
Subject: [PATCH] Fix parsing of digitalocean dns records (#14215)

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 40ff3730a..794787eae 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 af4066ca4..bb62db240 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"