provider/heroku: Fix heroku_cert update of ssl cert (hashicorp#14240)

* Attempt to write a new test for cert update

Trying to surface this bug with a test:
hashicorp#5930

* Fix the error

* Fix the test for the update operation

* Break apart tests for EU vs US to cleanse test run

* Refactor Update to more closely match create, increase debug logging

* Reflect differences of EU and US regions via separate tests

* Add comment re: why of test breakout

* Removed the “SetId” as it was unnecessary

* Ensure the SSL Addon has been provisioned

resource_heroku_addon.go

@@ -6,8 +6,10 @@ import (
 	"log"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/cyberdelia/heroku-go/v3"
+	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 )
 
@@ -89,6 +91,20 @@ func resourceHerokuAddonCreate(d *schema.ResourceData, meta interface{}) error {
 	d.SetId(a.ID)
 	log.Printf("[INFO] Addon ID: %s", d.Id())
 
+	// Wait for the Addon to be provisioned
+	log.Printf("[DEBUG] Waiting for Addon (%s) to be provisioned", d.Id())
+	stateConf := &resource.StateChangeConf{
+		Pending: []string{"provisioning"},
+		Target:  []string{"provisioned"},
+		Refresh: AddOnStateRefreshFunc(client, app, d.Id()),
+		Timeout: 20 * time.Minute,
+	}
+
+	if _, err := stateConf.WaitForState(); err != nil {
+		return fmt.Errorf("Error waiting for Addon (%s) to be provisioned: %s", d.Id(), err)
+	}
+	log.Printf("[INFO] Addon provisioned: %s", d.Id())
+
 	return resourceHerokuAddonRead(d, meta)
 }
 
@@ -167,3 +183,18 @@ func resourceHerokuAddonRetrieve(app string, id string, client *heroku.Service)
 
 	return addon, nil
 }
+
+// AddOnStateRefreshFunc returns a resource.StateRefreshFunc that is used to
+// watch an AddOn.
+func AddOnStateRefreshFunc(client *heroku.Service, appID, addOnID string) resource.StateRefreshFunc {
+	return func() (interface{}, string, error) {
+		addon, err := client.AddOnInfo(context.TODO(), appID, addOnID)
+		if err != nil {
+			return nil, "", err
+		}
+
+		// The type conversion here can be dropped when the vendored version of
+		// heroku-go is updated.
+		return (*heroku.AddOn)(addon), addon.State, nil
+	}
+}

resource_heroku_cert.go

@@ -88,22 +88,20 @@ func resourceHerokuCertUpdate(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*heroku.Service)
 
 	app := d.Get("app").(string)
+	preprocess := true
+	rollback := false
+	opts := heroku.SSLEndpointUpdateOpts{
+		CertificateChain: heroku.String(d.Get("certificate_chain").(string)),
+		Preprocess:       &preprocess,
+		PrivateKey:       heroku.String(d.Get("private_key").(string)),
+		Rollback:         &rollback}
 
-	if d.HasChange("certificate_chain") {
-		preprocess := true
-		rollback := false
-		ad, err := client.SSLEndpointUpdate(
-			context.TODO(), app, d.Id(), heroku.SSLEndpointUpdateOpts{
-				CertificateChain: d.Get("certificate_chain").(*string),
-				Preprocess:       &preprocess,
-				PrivateKey:       d.Get("private_key").(*string),
-				Rollback:         &rollback})
+	if d.HasChange("certificate_chain") || d.HasChange("private_key") {
+		log.Printf("[DEBUG] SSL Certificate update configuration: %#v, %#v", app, opts)
+		_, err := client.SSLEndpointUpdate(context.TODO(), app, d.Id(), opts)
 		if err != nil {
-			return err
+			return fmt.Errorf("Error updating SSL endpoint: %s", err)
 		}
-
-		// Store the new ID
-		d.SetId(ad.ID)
 	}
 
 	return resourceHerokuCertRead(d, meta)

resource_heroku_cert_test.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
+	"regexp"
+	"strings"
 	"testing"
 
 	"github.com/cyberdelia/heroku-go/v3"
@@ -13,39 +15,32 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestAccHerokuCert_Basic(t *testing.T) {
+// We break apart testing for EU and US because at present, Heroku deals with
+// each a bit differently and the setup/teardown of separate tests seems to
+// help them to perform more consistently.
+// https://devcenter.heroku.com/articles/ssl-endpoint#add-certificate-and-intermediaries
+func TestAccHerokuCert_EU(t *testing.T) {
 	var endpoint heroku.SSLEndpointInfoResult
+	appName := fmt.Sprintf("tftest-%s", acctest.RandString(10))
+
 	wd, _ := os.Getwd()
-	certificateChainFile := wd + "/test-fixtures/terraform.cert"
-	certificateChainBytes, _ := ioutil.ReadFile(certificateChainFile)
+	certFile := wd + "/test-fixtures/terraform.cert"
+	certFile2 := wd + "/test-fixtures/terraform2.cert"
+	keyFile := wd + "/test-fixtures/terraform.key"
+	keyFile2 := wd + "/test-fixtures/terraform2.key"
+
+	certificateChainBytes, _ := ioutil.ReadFile(certFile)
 	certificateChain := string(certificateChainBytes)
-	appName := fmt.Sprintf("tftest-%s", acctest.RandString(10))
-	testAccCheckHerokuCertConfig_basic := `
-    resource "heroku_app" "foobar" {
-        name = "` + appName + `"
-        region = "eu"
-    }
-
-    resource "heroku_addon" "ssl" {
-        app = "${heroku_app.foobar.name}"
-        plan = "ssl:endpoint"
-    }
-
-    resource "heroku_cert" "ssl_certificate" {
-        app = "${heroku_app.foobar.name}"
-        depends_on = ["heroku_addon.ssl"]
-        certificate_chain="${file("` + certificateChainFile + `")}"
-        private_key="${file("` + wd + `/test-fixtures/terraform.key")}"
-    }
-    `
+	certificateChain2Bytes, _ := ioutil.ReadFile(certFile2)
+	certificateChain2 := string(certificateChain2Bytes)
 
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckHerokuCertDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccCheckHerokuCertConfig_basic,
+				Config: testAccCheckHerokuCertEUConfig(appName, certFile, keyFile),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckHerokuCertExists("heroku_cert.ssl_certificate", &endpoint),
 					testAccCheckHerokuCertificateChain(&endpoint, certificateChain),
@@ -54,10 +49,104 @@ func TestAccHerokuCert_Basic(t *testing.T) {
 						"cname", fmt.Sprintf("%s.herokuapp.com", appName)),
 				),
 			},
+			{
+				Config: testAccCheckHerokuCertEUConfig(appName, certFile2, keyFile2),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckHerokuCertExists("heroku_cert.ssl_certificate", &endpoint),
+					testAccCheckHerokuCertificateChain(&endpoint, certificateChain2),
+					resource.TestCheckResourceAttr(
+						"heroku_cert.ssl_certificate",
+						"cname", fmt.Sprintf("%s.herokuapp.com", appName)),
+				),
+			},
 		},
 	})
 }
 
+func TestAccHerokuCert_US(t *testing.T) {
+	var endpoint heroku.SSLEndpointInfoResult
+	appName := fmt.Sprintf("tftest-%s", acctest.RandString(10))
+
+	wd, _ := os.Getwd()
+	certFile := wd + "/test-fixtures/terraform.cert"
+	certFile2 := wd + "/test-fixtures/terraform2.cert"
+	keyFile := wd + "/test-fixtures/terraform.key"
+	keyFile2 := wd + "/test-fixtures/terraform2.key"
+
+	certificateChainBytes, _ := ioutil.ReadFile(certFile)
+	certificateChain := string(certificateChainBytes)
+	certificateChain2Bytes, _ := ioutil.ReadFile(certFile2)
+	certificateChain2 := string(certificateChain2Bytes)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckHerokuCertDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCheckHerokuCertUSConfig(appName, certFile2, keyFile2),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckHerokuCertExists("heroku_cert.ssl_certificate", &endpoint),
+					testAccCheckHerokuCertificateChain(&endpoint, certificateChain2),
+					resource.TestMatchResourceAttr(
+						"heroku_cert.ssl_certificate",
+						"cname", regexp.MustCompile(`herokussl`)),
+				),
+			},
+			{
+				Config: testAccCheckHerokuCertUSConfig(appName, certFile, keyFile),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckHerokuCertExists("heroku_cert.ssl_certificate", &endpoint),
+					testAccCheckHerokuCertificateChain(&endpoint, certificateChain),
+					resource.TestMatchResourceAttr(
+						"heroku_cert.ssl_certificate",
+						"cname", regexp.MustCompile(`herokussl`)),
+				),
+			},
+		},
+	})
+}
+
+func testAccCheckHerokuCertEUConfig(appName, certFile, keyFile string) string {
+	return strings.TrimSpace(fmt.Sprintf(`
+resource "heroku_app" "foobar" {
+  name = "%s"
+  region = "eu"
+}
+
+resource "heroku_addon" "ssl" {
+  app = "${heroku_app.foobar.name}"
+  plan = "ssl:endpoint"
+}
+
+resource "heroku_cert" "ssl_certificate" {
+  app = "${heroku_app.foobar.name}"
+  depends_on = ["heroku_addon.ssl"]
+  certificate_chain="${file("%s")}"
+  private_key="${file("%s")}"
+}`, appName, certFile, keyFile))
+}
+
+func testAccCheckHerokuCertUSConfig(appName, certFile, keyFile string) string {
+	return strings.TrimSpace(fmt.Sprintf(`
+resource "heroku_app" "foobar" {
+  name = "%s"
+  region = "us"
+}
+
+resource "heroku_addon" "ssl" {
+  app = "${heroku_app.foobar.name}"
+  plan = "ssl:endpoint"
+}
+
+resource "heroku_cert" "ssl_certificate" {
+  app = "${heroku_app.foobar.name}"
+  depends_on = ["heroku_addon.ssl"]
+  certificate_chain="${file("%s")}"
+  private_key="${file("%s")}"
+}`, appName, certFile, keyFile))
+}
+
 func testAccCheckHerokuCertDestroy(s *terraform.State) error {
 	client := testAccProvider.Meta().(*heroku.Service)
 

test-fixtures/terraform2.cert

@@ -0,0 +1,29 @@
+-----BEGIN CERTIFICATE-----
+MIIE5jCCAs6gAwIBAgIBATANBgkqhkiG9w0BAQsFADATMREwDwYDVQQDEwhDZXJ0
+QXV0aDAeFw0xNzA1MDUwMDI4NDhaFw0yNzA1MDUwMDI4NTVaMBMxETAPBgNVBAMT
+CENlcnRBdXRoMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIICCgKCAgEA3f07ApBL
+Infz43y96n6iWtQgk4nVEMN2Qc5a4bGm7Ha/gnNGYnv87CGpd6VNBrd07zXlOGXo
+5DCly3AxjmFaHw9wzKadXmU2P91tQ4S253tFN8lirMPe+2hS5RnAPw9NsvYsU0Iy
+wp/+1a2OpDGtUv00fyU9tY8M1wQo/33xgTyc+8DdEDQ08/PjCKbA5vvpfra5fE+S
+g0Gcw8j6Y/iXcnj6LrTvY7I9G3doyAUezGCpAqPErVCDj5nokdJ17zs0kWbr/e4z
+o7yKvQOw7majlaX/R6v8tuYvEZ8fokXR7ecGeco86d7sH+OHafOkYQgf+forGMh6
+PSJ5z3WWnLKPvzGgvqoDemGfIyfeRHGTI+e8+4DrcvJGlLS0vBFCYxqXhlvmi6xf
+u9G0/zKV/VbIeJS1hTus6sYIIRMzABNwYIGGIz4eBjnO1jakowsdxfgsBvpMwVB+
+yZ+yxv/L9DCVK/VW3VfxQoQ+IDRTokfU/6yKWeonYVtTH57upJ7tRKRy2pmFCJEo
+c4T7IKg5ENfGhSkKHuN8CLoWdRREh2KTTijET0qUHCWPbOl9MgW6Y4Jf2qYkggGk
+CTRDk7iGuLKT6tMqVrqcmA/AhBpgZKg5N+rIbwTfIHzGNql93bc1FX0KVh7LlV64
+9ZRR4mksrk26pDnLqAfiT15aRsIrYZYdbh0CAwEAAaNFMEMwDgYDVR0PAQH/BAQD
+AgEGMBIGA1UdEwEB/wQIMAYBAf8CAQAwHQYDVR0OBBYEFDq5vNYjPHDUyIgkTKCX
+9N27PvuiMA0GCSqGSIb3DQEBCwUAA4ICAQCW4RfKiHsFLOwDTTVkQzb0G2A9Ajeg
+66lWSuNidntbr+w5595G0ae3qCumaVQSNWKtMAkdahzI+0J00YGWpnqS2rJIIgD+
+8I67EpQ6wi60EUMAECNl7vJvLH/IXt6IVY1wr3ci3G56aKVZIsnaoFi2vxnwHNz3
+uOkNcj0OO4Nosw8aThzsitiddu4BVk+8AFq0cylroQzWjoCrkKOyWMyKTcK6qnS7
+ayyCK02AQPhXFYxet7NVY5hgldto2BAsijBX1Xl6XR5QGSrIQw/2nkAywVSnAGip
+Ofk/njzZH9FhUnEY1C5vB2SU310LIOq1evvF8nd7csnI7wdjfHQ3m70PO6p9DGK/
+W0tET8NtuW2JV38KSNMrYxF2Hs7Il92x8JVQu9LtjiKTcJLdnnAQl0OvHUAgAmU9
+BRHOsfWD4Cxiwes0OZHuOpoghh7HV1A+JS5e9qNCCEzarQe4H2Zv8JkeBFbIaEH/
+bcT3a2Rtt6cvDw9mSXSw7p85/810n8af4T1D3aeLFdoVrpeTZVUdyFygcuWo1U4D
+JxaRAyJHvi4IJmwpjehn7DoFasNefPBVVFi28QCJXFHpe0DNm8MvI5fGhyZ2uW8t
++6PDgumsZLQT7jJNl9ubYV3U0Nsymvvwqx4LY2nql0agEzJ0F9ekoA50csoxe2ir
+/DOgG1nKIc186A==
+-----END CERTIFICATE-----

test-fixtures/terraform2.key

@@ -0,0 +1,51 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIJKQIBAAKCAgEA3f07ApBLInfz43y96n6iWtQgk4nVEMN2Qc5a4bGm7Ha/gnNG
+Ynv87CGpd6VNBrd07zXlOGXo5DCly3AxjmFaHw9wzKadXmU2P91tQ4S253tFN8li
+rMPe+2hS5RnAPw9NsvYsU0Iywp/+1a2OpDGtUv00fyU9tY8M1wQo/33xgTyc+8Dd
+EDQ08/PjCKbA5vvpfra5fE+Sg0Gcw8j6Y/iXcnj6LrTvY7I9G3doyAUezGCpAqPE
+rVCDj5nokdJ17zs0kWbr/e4zo7yKvQOw7majlaX/R6v8tuYvEZ8fokXR7ecGeco8
+6d7sH+OHafOkYQgf+forGMh6PSJ5z3WWnLKPvzGgvqoDemGfIyfeRHGTI+e8+4Dr
+cvJGlLS0vBFCYxqXhlvmi6xfu9G0/zKV/VbIeJS1hTus6sYIIRMzABNwYIGGIz4e
+BjnO1jakowsdxfgsBvpMwVB+yZ+yxv/L9DCVK/VW3VfxQoQ+IDRTokfU/6yKWeon
+YVtTH57upJ7tRKRy2pmFCJEoc4T7IKg5ENfGhSkKHuN8CLoWdRREh2KTTijET0qU
+HCWPbOl9MgW6Y4Jf2qYkggGkCTRDk7iGuLKT6tMqVrqcmA/AhBpgZKg5N+rIbwTf
+IHzGNql93bc1FX0KVh7LlV649ZRR4mksrk26pDnLqAfiT15aRsIrYZYdbh0CAwEA
+AQKCAgAKdpIed9CiykablViaQefDIjZ63cdGKABd760G8Emu4ZX7PxW1NKTiOF/1
+fLwZsfH4CHFKbDtC7iwSX7JmRJ5r0l19t+i490pMTlKFGS9Jz9yeWYamIAFVlkA5
+/jG6hy0hX0sNjZQ46jOnvKt5f8HspHSh/Y5gDWMMi2ynRjdo4QOBNkD1L5DDYt5z
+nPCAsqT5zQEHI/UC7MfHzqRGrAPvaFZadzrFVzRcJA+zRdKCzZeJwVBW3vGkhhuZ
+K/NVGFRM+i3rZRvX/t4HNLJVOk9BkXZr2WZq9ISJbxednW7cqMP8X5TpbRFyG1ZZ
+nxtDW4+uR6VaYLCqSwK0zZUQw7XUtbSSlxPG90dZyNkR5n3grK0wORro8zV/8BMg
+qON/jxUJFLOxCBDT5jThjFm6mk9dkUF/uzubm7k5LdRP4v1J8LojEF1BrP5g8JEN
+cOBwC2z10uwcSSdVPF7FMJ35EctIzFw+eA2qeYqCywTSd7J1AQ5vNcjc+2wiY2Ff
+4qIDAvgaXV+g7JsmqaEFWTs4o5PwKZXp4qEU4hhZ1636vi9aGJPYcsZqWb5os/+j
+OQFWGrk16gDyzC3J3Qh8cnVHQko7yrWbhMOXSAN5W/1npovhVeYOVUtCLRfog66b
+ZrCmRkoFdRaQeRD3iYHu5dLye8g4k1PWB/+Uq17GjAWp12wenQKCAQEA9usfQVgt
+4C2tRwsHUXaRihiY+MM7YlXDwh4qzDXQk8cfdGabR6MIvGPcpsDpxDidPHUuU0qy
+NZC4y4rrZnFJaQ3YaD6lDlAuICyHqrQcYBYB7wB9gZy/Un34P8WnE93kTfDZRhd0
+7SF94lBW+cb2BjzcyUsYLf/0UVEt8kTWxP1T39fHyQEGxwJcbPvjRLZoYWkTnPZb
+cx/hBk8gclVPUV0w9idNuWz4ittkEF3oEEtu+06pjP8DZ+lJ/Hn6hwJjN66oCXKH
+9TLnvygJZjub8BKXRpbRJ989dQvGl7w+rlT/gyJvewjtn/GU6OEvFyaHh2Hgqy6y
+ROM9nQgGIwtqWwKCAQEA5idim5tM6lYbR5oTeczqVTNE69gnzmQVMFML7Eb2Qnne
+BFvpHuKouJksY514rSrXusirYRqhX5WU8exkn/h3T9LkRyLA3pUfBHxqcejMfuRG
+MYIy9nIgiV9xEc73hZsi4xCWEgFfmS9WCB7Z46Zi3PayZg4Hrq7C69ez1drt6njQ
+R09qCLUJD+DRTgEax08eZNYLeqcy20ofCGu2UZyyr3ZIS7wGLxNqzic26E24r2GG
+K2KqiFH6isUS4EnthwV29EqvPcsq57A4s9Uva8xkORUhJCtY/7M1klUo/89HxVGp
+OZ4+sxYqZCKcQJO8+i/4RPwF3QYZ/uh/gcZM2z9C5wKCAQAyjsQQkiiajV+8ezKd
+aIS2XQD9dqQzJ1J07c5fj+lMSOpU4CmNSoGgaWYlsrxq1BjF50x7+4Bv3VkpPCGl
+ES8x1oboGWOcgahgKB4DQuvIdNkigdww7NJz5p0tGaBzPezgVJ94bZcgcsoey8pz
+TFzVvCKNCNZDnPP+rnuU7ql3HlPNMpaSvqYPm5knK5BGYn8O6v/8FKl28iEWNJ91
+KaibBVTgIf4VKI3fiLp9a2z34SoxRNMMrq6Y2Tiv/J3ihQehwB5iCNRzzV+MUXtT
+NoNgbb4R0xGyc1BXJfkc2ouPEJJc3HEtJQ/avxF5eZo1yErZ2p2xD1erKUhVXe47
+wLufAoIBAQC853DBJXvBD0G+yFDZ9P4VRkp4hWdOuMjHbDJqEWiI8Xvv+fxilElF
+krtjW9mz0GlW7uPzhKcVTDH/SzbgMlDDnOYvGPBTAPR/exrnOdu2/ug6NJJdwxi/
+iC3HHyf8anP9CR0T1DrCAZ9MdP4EIwocMQQGTdeyYdCtQNNjYRlMDTNuhFkUonq4
+pJ9GthNjqaXZv/GWD2vnn3PPNpFjdQkYiS4Xs1EkDHzqjjc7/qbqlFJKg+ZSk27f
+vZebrjIeU7bqFe61+m7R0csIl58fjJhqXdRg2o9m+JGs9Ob86AYRh9As8Zym4zeS
+DvJO8rP2aa8N+Alb+2kU14HoY3mrrsXbAoIBAQDK0jCxdc73h4u2B2zlX4eyHAy7
+oPpwhIjuuMVXbsR5MqlIOpD7QjqujnMTN0MSslV1GzwhfQO7cN6ijQQiWWiHzCKd
+O6NqetPQnn19ddqFLWcrl/WzZdVTDeXyhAaFffy+x8dhPVUdPs/ZDXXAF43LFwly
+2kSTWnfwZ5Yvi3K2SB/dO48I77qEUF370/wstdHviSttbI5HhtiRljCU6mwpms34
+4KdDCCxPleZ7Dl2m8v+FkdWkZomLi9wo/XzBo/z5RcI5gjt83OJ6pLBTcLDo7WOc
+g2XM7rqQoQr8bilH+eMAZtEm/axwZHIcTTqsyt3Mp09KL65MB+581V5TnGSu
+-----END RSA PRIVATE KEY-----