From dfe37acbc0c20dc9fbde5cd83fb98e6b472470a5 Mon Sep 17 00:00:00 2001 From: Andrew Starr-Bochicchio Date: Thu, 28 Mar 2019 14:13:22 -0400 Subject: [PATCH] Certificates: Move attribute validation from CustomizeDiff to Create (Fixes: #163). When CustomizeDiff is run for a new resource, it does not have access to the current state. In the digitalocean_certificate resource, we are using a CustomizeDiff function to validate that certain attributes are used together depending on the certificate type. When the value for one of those attributes is interpolated, as in the case of generating a cert using the acme provider, the validation fails since it depends on reading the value from state. As state is not present, the value is interpreted as empty. From the CustomizeDiff doc string: > // The phases Terraform runs this in, and the state available via functions > // like Get and GetChange, are as follows: > // > // * New resource: One run with no state https://godoc.org/github.com/hashicorp/terraform/helper/schema#Resource This PR moves the validation from the CustomizeDiff function into the Create function as well as adding acceptance tests: ``` $ make testacc TESTARGS='-run=TestAccDigitalOceanCertificate_ExpectedErrors' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccDigitalOceanCertificate_ExpectedE$ ? github.com/terraform-providers/terraform-provider-digitalocean [no test files] === RUN TestAccDigitalOceanCertificate_ExpectedErrors --- PASS: TestAccDigitalOceanCertificate_ExpectedErrors (0.10s) PASS ok github.com/terraform-providers/terraform-provider-digitalocean/digitalocean (cached) ``` --- .../resource_digitalocean_certificate.go | 37 +++++------- .../resource_digitalocean_certificate_test.go | 60 +++++++++++++++++++ 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/digitalocean/resource_digitalocean_certificate.go b/digitalocean/resource_digitalocean_certificate.go index 47f7e3d6..b2750ad3 100644 --- a/digitalocean/resource_digitalocean_certificate.go +++ b/digitalocean/resource_digitalocean_certificate.go @@ -92,27 +92,6 @@ func resourceDigitalOceanCertificate() *schema.Resource { Computed: true, }, }, - - CustomizeDiff: func(diff *schema.ResourceDiff, v interface{}) error { - - certificateType := diff.Get("type").(string) - if certificateType == "custom" { - if _, ok := diff.GetOk("private_key"); !ok { - return fmt.Errorf("`private_key` is required for when type is `custom` or empty") - } - - if _, ok := diff.GetOk("leaf_certificate"); !ok { - return fmt.Errorf("`leaf_certificate` is required for when type is `custom` or empty") - } - } else if certificateType == "lets_encrypt" { - - if _, ok := diff.GetOk("domains"); !ok { - return fmt.Errorf("`domains` is required for when type is `lets_encrypt`") - } - } - - return nil - }, } } @@ -142,6 +121,22 @@ func buildCertificateRequest(d *schema.ResourceData) (*godo.CertificateRequest, func resourceDigitalOceanCertificateCreate(d *schema.ResourceData, meta interface{}) error { client := meta.(*CombinedConfig).godoClient() + certificateType := d.Get("type").(string) + if certificateType == "custom" { + if _, ok := d.GetOk("private_key"); !ok { + return fmt.Errorf("`private_key` is required for when type is `custom` or empty") + } + + if _, ok := d.GetOk("leaf_certificate"); !ok { + return fmt.Errorf("`leaf_certificate` is required for when type is `custom` or empty") + } + } else if certificateType == "lets_encrypt" { + + if _, ok := d.GetOk("domains"); !ok { + return fmt.Errorf("`domains` is required for when type is `lets_encrypt`") + } + } + log.Printf("[INFO] Create a Certificate Request") certReq, err := buildCertificateRequest(d) diff --git a/digitalocean/resource_digitalocean_certificate_test.go b/digitalocean/resource_digitalocean_certificate_test.go index 3885bbb6..9a9e4589 100644 --- a/digitalocean/resource_digitalocean_certificate_test.go +++ b/digitalocean/resource_digitalocean_certificate_test.go @@ -11,6 +11,7 @@ import ( "fmt" "log" "math/big" + "regexp" "strings" "testing" "time" @@ -84,6 +85,31 @@ func TestAccDigitalOceanCertificate_Basic(t *testing.T) { }) } +func TestAccDigitalOceanCertificate_ExpectedErrors(t *testing.T) { + rInt := acctest.RandInt() + privateKeyMaterial, leafCertMaterial, certChainMaterial := generateTestCertMaterial(t) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckDigitalOceanCertificateDestroy, + Steps: []resource.TestStep{ + { + Config: testAccCheckDigitalOceanCertificateConfig_customNoLeaf(rInt, privateKeyMaterial, certChainMaterial), + ExpectError: regexp.MustCompile("`leaf_certificate` is required for when type is `custom` or empty"), + }, + { + Config: testAccCheckDigitalOceanCertificateConfig_customNoKey(rInt, leafCertMaterial, certChainMaterial), + ExpectError: regexp.MustCompile("`private_key` is required for when type is `custom` or empty"), + }, + { + Config: testAccCheckDigitalOceanCertificateConfig_noDomains(rInt), + ExpectError: regexp.MustCompile("`domains` is required for when type is `lets_encrypt`"), + }, + }, + }) +} + func testAccCheckDigitalOceanCertificateDestroy(s *terraform.State) error { client := testAccProvider.Meta().(*CombinedConfig).godoClient() @@ -219,3 +245,37 @@ EOF EOF }`, rInt, privateKeyMaterial, leafCert, certChain) } + +func testAccCheckDigitalOceanCertificateConfig_customNoLeaf(rInt int, privateKeyMaterial, certChain string) string { + return fmt.Sprintf(` +resource "digitalocean_certificate" "foobar" { + name = "certificate-%d" + private_key = <