Skip to content

Commit

Permalink
resource/aws_acm_certificate: Correctly handle SAN entries that match…
Browse files Browse the repository at this point in the history 
… domain_name

AWS automatically adds a SAN entry that matches the `domain_name` value. This entry
was being deleted in order to avoid spurious diffs where user's config didn't include
the `domain_name` value in `subject_alternative_names`.

This change reverses the logic so as to better reflect the actual state of a certificate
in ACM; all SANs are now tracked in Terraform state, including the default SAN that AWS
adds. If a user hasn't specified the `domain_name` entry in `subject_alternative_names`
we add it for them to avoid spurious diffs.
  • Loading branch information
@mattburgess
mattburgess committed Oct 26, 2021
1 parent fd3cb39 commit 17f0695
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .changelog/19790.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_acm_certificate: Correctly handle SAN entries that match domain_name
```
21 changes: 16 additions & 5 deletions internal/service/acm/certificate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,19 @@ func ResourceCertificate() *schema.Resource {
}
}

// ACM automatically adds the domain_name value to the list of SANs. Mimic ACM's behavior
// so that the user doesn't need to explicitly set it themselves.
if diff.HasChange("domain_name") || diff.HasChange("subject_alternative_names") {
domain_name := diff.Get("domain_name").(string)

if sanSet, ok := diff.Get("subject_alternative_names").(*schema.Set); ok {
sanSet.Add(domain_name)
if err := diff.SetNew("subject_alternative_names", sanSet); err != nil {
return fmt.Errorf("error setting new subject_alternative_names diff: %w", err)
}
}
}

return nil
},
verify.SetTagsDiff,
Expand Down Expand Up @@ -338,7 +351,7 @@ func resourceCertificateRead(d *schema.ResourceData, meta interface{}) error {
d.Set("arn", resp.Certificate.CertificateArn)
d.Set("certificate_authority_arn", resp.Certificate.CertificateAuthorityArn)

if err := d.Set("subject_alternative_names", cleanUpSubjectAlternativeNames(resp.Certificate)); err != nil {
if err := d.Set("subject_alternative_names", flattenSubjectAlternativeNames(resp.Certificate)); err != nil {
return resource.NonRetryableError(err)
}

Expand Down Expand Up @@ -434,13 +447,11 @@ func resourceCertificateUpdate(d *schema.ResourceData, meta interface{}) error {
return resourceCertificateRead(d, meta)
}

func cleanUpSubjectAlternativeNames(cert *acm.CertificateDetail) []string {
func flattenSubjectAlternativeNames(cert *acm.CertificateDetail) []string {
sans := cert.SubjectAlternativeNames
vs := make([]string, 0)
for _, v := range sans {
if aws.StringValue(v) != aws.StringValue(cert.DomainName) {
vs = append(vs, aws.StringValue(v))
}
vs = append(vs, aws.StringValue(v))
}
return vs
}
Expand Down
62 changes: 54 additions & 8 deletions internal/service/acm/certificate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ func TestAccACMCertificate_privateCert(t *testing.T) {
resource.TestCheckResourceAttr(resourceName, "domain_name", certificateDomainName),
resource.TestCheckResourceAttr(resourceName, "domain_validation_options.#", "0"),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusFailed), // FailureReason: PCA_INVALID_STATE (PCA State: PENDING_CERTIFICATE)
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", certificateDomainName),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", "NONE"),
resource.TestCheckResourceAttrPair(resourceName, "certificate_authority_arn", certificateAuthorityResourceName, "arn"),
Expand Down Expand Up @@ -200,7 +201,8 @@ func TestAccACMCertificate_rootAndWildcardSan(t *testing.T) {
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", rootDomain),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", wildcardDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down Expand Up @@ -260,7 +262,8 @@ func TestAccACMCertificate_San_single(t *testing.T) {
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", domain),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", sanDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down Expand Up @@ -307,7 +310,8 @@ func TestAccACMCertificate_San_multiple(t *testing.T) {
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "2"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "3"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", domain),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", sanDomain1),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", sanDomain2),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
Expand Down Expand Up @@ -350,7 +354,8 @@ func TestAccACMCertificate_San_trailingPeriod(t *testing.T) {
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", domain),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", strings.TrimSuffix(sanDomain, ".")),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand All @@ -365,6 +370,44 @@ func TestAccACMCertificate_San_trailingPeriod(t *testing.T) {
})
}

func TestAccACMCertificate_San_matches_domain(t *testing.T) {
resourceName := "aws_acm_certificate.cert"
rootDomain := acctest.ACMCertificateDomainFromEnv(t)
domain := acctest.ACMCertificateRandomSubDomain(rootDomain)
sanDomain := rootDomain

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, acm.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckAcmCertificateDestroy,
Steps: []resource.TestStep{
{
Config: testAccAcmCertificateConfig_subjectAlternativeNames(domain, strconv.Quote(sanDomain), acm.ValidationMethodDns),
Check: resource.ComposeTestCheckFunc(
acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "acm", regexp.MustCompile(`certificate/.+`)),
resource.TestCheckResourceAttr(resourceName, "domain_name", domain),
resource.TestCheckResourceAttr(resourceName, "domain_validation_options.#", "1"),
resource.TestCheckTypeSetElemNestedAttrs(resourceName, "domain_validation_options.*", map[string]string{
"domain_name": domain,
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", domain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccACMCertificate_wildcard(t *testing.T) {
resourceName := "aws_acm_certificate.cert"
rootDomain := acctest.ACMCertificateDomainFromEnv(t)
Expand All @@ -387,7 +430,8 @@ func TestAccACMCertificate_wildcard(t *testing.T) {
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", wildcardDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
),
Expand Down Expand Up @@ -427,8 +471,9 @@ func TestAccACMCertificate_wildcardAndRootSan(t *testing.T) {
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "2"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", rootDomain),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", wildcardDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
),
Expand Down Expand Up @@ -462,7 +507,8 @@ func TestAccACMCertificate_disableCTLogging(t *testing.T) {
"domain_name": rootDomain,
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckTypeSetElemAttr(resourceName, "subject_alternative_names.*", rootDomain),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down

0 comments on commit 17f0695

Please sign in to comment.