From 5f0ddc28709ee3f124684bf015607729f1ebb14f Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Thu, 8 Aug 2019 12:50:31 -0400 Subject: [PATCH 1/3] Fix update of imported ACM certificates --- aws/resource_aws_acm_certificate.go | 27 +++++++----- aws/resource_aws_acm_certificate_test.go | 54 +++++++++++++++++++++--- 2 files changed, 63 insertions(+), 18 deletions(-) diff --git a/aws/resource_aws_acm_certificate.go b/aws/resource_aws_acm_certificate.go index 53da3da8343..f2d3a4f4db7 100644 --- a/aws/resource_aws_acm_certificate.go +++ b/aws/resource_aws_acm_certificate.go @@ -25,21 +25,20 @@ func resourceAwsAcmCertificate() *schema.Resource { }, Schema: map[string]*schema.Schema{ "certificate_body": { - Type: schema.TypeString, - Optional: true, - StateFunc: normalizeCert, + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: suppressACMCertificateDiff, }, - "certificate_chain": { - Type: schema.TypeString, - Optional: true, - StateFunc: normalizeCert, + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: suppressACMCertificateDiff, }, "private_key": { - Type: schema.TypeString, - Optional: true, - StateFunc: normalizeCert, - Sensitive: true, + Type: schema.TypeString, + Optional: true, + DiffSuppressFunc: suppressACMCertificateDiff, + Sensitive: true, }, "certificate_authority_arn": { Type: schema.TypeString, @@ -445,3 +444,9 @@ func flattenAcmCertificateOptions(co *acm.CertificateOptions) []interface{} { return []interface{}{m} } + +func suppressACMCertificateDiff(k, old, new string, d *schema.ResourceData) bool { + // old == normalizeCert(new) is there for legacy reasons. The certificates used to be stored as a hash in the state + // However that prevented updates + return normalizeCert(old) == normalizeCert(new) || old == normalizeCert(new) +} diff --git a/aws/resource_aws_acm_certificate_test.go b/aws/resource_aws_acm_certificate_test.go index 814100a5c4e..6642be41d39 100644 --- a/aws/resource_aws_acm_certificate_test.go +++ b/aws/resource_aws_acm_certificate_test.go @@ -539,6 +539,15 @@ func TestAccAWSAcmCertificate_imported_DomainName(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "domain_name", "example.com"), ), + ExpectNonEmptyPlan: true, // The certificate body is regenerated every time + }, + { + Config: testAccAcmCertificateConfig_selfSigned("example"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + resource.TestCheckResourceAttr(resourceName, "domain_name", "example.com"), + ), + ExpectNonEmptyPlan: true, // The certificate body is regenerated every time }, { Config: testAccAcmCertificateConfig_selfSigned("example2"), @@ -546,13 +555,14 @@ func TestAccAWSAcmCertificate_imported_DomainName(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "domain_name", "example2.com"), ), + ExpectNonEmptyPlan: true, // The certificate body is regenerated every time }, { ResourceName: resourceName, ImportState: true, ImportStateVerify: true, // These are not returned by the API - ImportStateVerifyIgnore: []string{"private_key", "certificate_body"}, + ImportStateVerifyIgnore: []string{"private_key", "certificate_body", "certificate_chain"}, }, }, }) @@ -661,17 +671,46 @@ resource "tls_private_key" "%[1]s" { algorithm = "RSA" } -resource "tls_self_signed_cert" "%[1]s" { +resource "tls_cert_request" "%[1]s" { key_algorithm = "RSA" - private_key_pem = "${tls_private_key.%[1]s.private_key_pem}" - + private_key_pem = "${tls_private_key.%[1]s.private_key_pem}" + subject { common_name = "%[1]s.com" organization = "ACME Examples, Inc" } +} + +resource "tls_self_signed_cert" "%[1]s" { + key_algorithm = "${tls_private_key.%[1]s.algorithm}" + private_key_pem = "${tls_private_key.%[1]s.private_key_pem}" + + validity_period_hours = 4 + early_renewal_hours = 2 + is_ca_certificate = true # Reasonable set of uses for a server SSL certificate. + + allowed_uses = [ + "key_encipherment", + "digital_signature", + "server_auth", + "cert_signing", + ] + + subject { + common_name = "test.com" + organization = "ACME Examples, Inc" + } +} - validity_period_hours = 12 +resource "tls_locally_signed_cert" "%[1]s" { + cert_request_pem = "${tls_cert_request.%[1]s.cert_request_pem}" + ca_key_algorithm = "RSA" + ca_private_key_pem = "${tls_private_key.%[1]s.private_key_pem}" + ca_cert_pem = "${tls_self_signed_cert.%[1]s.cert_pem}" + validity_period_hours = 3000 + early_renewal_hours = 3000 + allowed_uses = [ "key_encipherment", "digital_signature", @@ -680,8 +719,9 @@ resource "tls_self_signed_cert" "%[1]s" { } resource "aws_acm_certificate" "cert" { - private_key = "${tls_private_key.%[1]s.private_key_pem}" - certificate_body = "${tls_self_signed_cert.%[1]s.cert_pem}" + private_key = "${tls_private_key.%[1]s.private_key_pem}" + certificate_body = "${tls_locally_signed_cert.%[1]s.cert_pem}" + certificate_chain = "${tls_self_signed_cert.%[1]s.cert_pem}" } `, certName) } From 05aea59ae7db00fe7d6804feb4ebedafe5a595c9 Mon Sep 17 00:00:00 2001 From: Julien Duchesne Date: Tue, 22 Oct 2019 13:35:48 -0400 Subject: [PATCH 2/3] Fix ACM certificates update tests --- aws/resource_aws_acm_certificate_test.go | 40 +++++++++++++++++------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/aws/resource_aws_acm_certificate_test.go b/aws/resource_aws_acm_certificate_test.go index 99c9b9a72d0..c44986d51bf 100644 --- a/aws/resource_aws_acm_certificate_test.go +++ b/aws/resource_aws_acm_certificate_test.go @@ -528,34 +528,42 @@ func TestAccAWSAcmCertificate_tags(t *testing.T) { func TestAccAWSAcmCertificate_imported_DomainName(t *testing.T) { resourceName := "aws_acm_certificate.test" + commonName := "example.com" + caKey := tlsRsaPrivateKeyPem(2048) + caCertificate := tlsRsaX509SelfSignedCaCertificatePem(caKey) + key := tlsRsaPrivateKeyPem(2048) + certificate := tlsRsaX509LocallySignedCertificatePem(caKey, caCertificate, key, commonName) + + newCaKey := tlsRsaPrivateKeyPem(2048) + newCaCertificate := tlsRsaX509SelfSignedCaCertificatePem(newCaKey) + newCertificate := tlsRsaX509LocallySignedCertificatePem(newCaKey, newCaCertificate, key, commonName) + + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAcmCertificateDestroy, Steps: []resource.TestStep{ { - Config: testAccAcmCertificateConfigPrivateKey("example.com"), + Config: testAccAcmCertificateConfigPrivateKey(certificate, key, caCertificate), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - resource.TestCheckResourceAttr(resourceName, "domain_name", "example.com"), + resource.TestCheckResourceAttr(resourceName, "domain_name", commonName), ), - ExpectNonEmptyPlan: true, // The certificate body is regenerated every time }, { - Config: testAccAcmCertificateConfig_selfSigned("example"), + Config: testAccAcmCertificateConfigPrivateKey(newCertificate, key, newCaCertificate), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - resource.TestCheckResourceAttr(resourceName, "domain_name", "example.com"), + resource.TestCheckResourceAttr(resourceName, "domain_name", commonName), ), - ExpectNonEmptyPlan: true, // The certificate body is regenerated every time }, { - Config: testAccAcmCertificateConfig_selfSigned("example2"), + Config: testAccAcmCertificateConfigPrivateKeyWithoutChain("example2.com"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), - resource.TestCheckResourceAttr(resourceName, "domain_name", "example.org"), + resource.TestCheckResourceAttr(resourceName, "domain_name", "example2.com"), ), - ExpectNonEmptyPlan: true, // The certificate body is regenerated every time }, { ResourceName: resourceName, @@ -578,7 +586,7 @@ func TestAccAWSAcmCertificate_imported_IpAddress(t *testing.T) { CheckDestroy: testAccCheckAcmCertificateDestroy, Steps: []resource.TestStep{ { - Config: testAccAcmCertificateConfigPrivateKey("1.2.3.4"), + Config: testAccAcmCertificateConfigPrivateKeyWithoutChain("1.2.3.4"), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "domain_name", ""), resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"), @@ -665,7 +673,7 @@ resource "aws_acm_certificate" "cert" { `, domainName, validationMethod, tag1Key, tag1Value, tag2Key, tag2Value) } -func testAccAcmCertificateConfigPrivateKey(commonName string) string { +func testAccAcmCertificateConfigPrivateKeyWithoutChain(commonName string) string { key := tlsRsaPrivateKeyPem(2048) certificate := tlsRsaX509SelfSignedCertificatePem(key, commonName) @@ -677,6 +685,16 @@ resource "aws_acm_certificate" "test" { `, tlsPemEscapeNewlines(certificate), tlsPemEscapeNewlines(key)) } +func testAccAcmCertificateConfigPrivateKey(certificate, privateKey, chain string) string { + return fmt.Sprintf(` +resource "aws_acm_certificate" "test" { + certificate_body = "%[1]s" + private_key = "%[2]s" + certificate_chain = "%[3]s" +} +`, tlsPemEscapeNewlines(certificate), tlsPemEscapeNewlines(privateKey), tlsPemEscapeNewlines(chain)) +} + func testAccAcmCertificateConfig_disableCTLogging(domainName, validationMethod string) string { return fmt.Sprintf(` resource "aws_acm_certificate" "cert" { From bac34d5a76879d665f9a7c6e132127b1d734dc24 Mon Sep 17 00:00:00 2001 From: Jo Giroux Date: Tue, 22 Oct 2019 14:53:45 -0400 Subject: [PATCH 3/3] Apply go fmt Remove an extra empty line --- aws/resource_aws_acm_certificate_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/aws/resource_aws_acm_certificate_test.go b/aws/resource_aws_acm_certificate_test.go index c44986d51bf..1a1c9ca42af 100644 --- a/aws/resource_aws_acm_certificate_test.go +++ b/aws/resource_aws_acm_certificate_test.go @@ -538,7 +538,6 @@ func TestAccAWSAcmCertificate_imported_DomainName(t *testing.T) { newCaCertificate := tlsRsaX509SelfSignedCaCertificatePem(newCaKey) newCertificate := tlsRsaX509LocallySignedCertificatePem(newCaKey, newCaCertificate, key, commonName) - resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders,