Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement aws_acm_certificate flag to query for most recent certificate #1837

78 changes: 67 additions & 11 deletions aws/data_source_aws_acm_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,33 @@ func dataSourceAwsAcmCertificate() *schema.Resource {
Optional: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"most_recent": {
Type: schema.TypeBool,
Optional: true,
Default: false,
Elem: &schema.Schema{Type: schema.TypeString},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extraneous Elem here

},
},
}
}

type arnData struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the SDK provided *acm.CertificateDetail instead of our own custom type? https://docs.aws.amazon.com/sdk-for-go/api/service/acm/#CertificateDetail

arn string
notBefore *time.Time
}

func describeCertificate(arn *arnData, conn *acm.ACM) (*acm.DescribeCertificateOutput, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update this function to have a signature and functionality like the following?
describeAcmCertificate(arn *string, conn *acm.ACM) (*acm.CertificateDetail, error)

Although if we squash the logic in the most_recent portion of the code to only ever have one place to call this function, its logic can be moved down into where the code is being used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to squash the logic in only one place but unfortunately there are two places when this code comes in handy. I've kept the function, with the signature you've proposed.

params := &acm.DescribeCertificateInput{}
params.CertificateArn = &arn.arn

description, err := conn.DescribeCertificate(params)
if err != nil {
return nil, err
}

return description, nil
}

func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).acmconn

Expand All @@ -50,38 +73,38 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e
params.CertificateStatuses = []*string{aws.String("ISSUED")}
}

var arns []string
var arns []*arnData
log.Printf("[DEBUG] Reading ACM Certificate: %s", params)
err := conn.ListCertificatesPages(params, func(page *acm.ListCertificatesOutput, lastPage bool) bool {
for _, cert := range page.CertificateSummaryList {
if *cert.DomainName == target {
arns = append(arns, *cert.CertificateArn)
arns = append(arns, &arnData{*cert.CertificateArn, nil})
}
}

return true
})
if err != nil {
return errwrap.Wrapf("Error describing certificates: {{err}}", err)
return errwrap.Wrapf("Error listing certificates: {{err}}", err)
}

// filter based on certificate type (imported or aws-issued)
types, ok := d.GetOk("types")
if ok {
typesStrings := expandStringList(types.([]interface{}))
var matchedArns []string
var matchedArns []*arnData
for _, arn := range arns {
params := &acm.DescribeCertificateInput{}
params.CertificateArn = &arn

description, err := conn.DescribeCertificate(params)
description, err := describeCertificate(arn, conn)
if err != nil {
return errwrap.Wrapf("Error describing certificates: {{err}}", err)
}

for _, certType := range typesStrings {
if *description.Certificate.Type == *certType {
matchedArns = append(matchedArns, arn)
matchedArns = append(
matchedArns,
&arnData{arn.arn, description.Certificate.NotBefore},
)
break
}
}
Expand All @@ -93,12 +116,45 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e
if len(arns) == 0 {
return fmt.Errorf("No certificate for domain %q found in this region.", target)
}

if len(arns) > 1 {
return fmt.Errorf("Multiple certificates for domain %q found in this region.", target)
// Get most recent sorting by notBefore date. Notice that createdAt field is only valid
// for ACM issued certificates but not for imported ones so in a mixed scenario only
// fields extracted from the certificate are valid.
_, ok = d.GetOk("most_recent")
if ok {
mr := arns[0]
if mr.notBefore == nil {
description, err := describeCertificate(mr, conn)
if err != nil {
return errwrap.Wrapf("Error describing certificates: {{err}}", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you switch these to fmt.Errorf() please? We do not need to use errwrap.Wrapf() for its extra context in these cases. (Sorry I know there's some other code here which is already using errwrap 🙁 .)

}

mr.notBefore = description.Certificate.NotBefore
}
for _, arn := range arns[1:] {
if arn.notBefore == nil {
description, err := describeCertificate(arn, conn)
if err != nil {
return errwrap.Wrapf("Error describing certificates: {{err}}", err)
}

arn.notBefore = description.Certificate.NotBefore
}

if arn.notBefore.After(*mr.notBefore) {
mr = arn
}
}

arns = []*arnData{mr}
} else {
return fmt.Errorf("Multiple certificates for domain %q found in this region.", target)
}
}

d.SetId(time.Now().UTC().String())
d.Set("arn", arns[0])
d.Set("arn", arns[0].arn)

return nil
}
41 changes: 41 additions & 0 deletions aws/data_source_aws_acm_certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ func TestAccAwsAcmCertificateDataSource_noMatchReturnsError(t *testing.T) {
Config: testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain),
ExpectError: regexp.MustCompile(`No certificate for domain`),
},
{
Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain),
ExpectError: regexp.MustCompile(`No certificate for domain`),
},
{
Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain),
ExpectError: regexp.MustCompile(`No certificate for domain`),
},
{
Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain),
ExpectError: regexp.MustCompile(`No certificate for domain`),
},
},
})
}
Expand Down Expand Up @@ -57,3 +69,32 @@ data "aws_acm_certificate" "test" {
}
`, domain)
}

func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain string) string {
return fmt.Sprintf(`
data "aws_acm_certificate" "test" {
domain = "%s"
most_recent = true
}
`, domain)
}

func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain string) string {
return fmt.Sprintf(`
data "aws_acm_certificate" "test" {
domain = "%s"
statuses = ["ISSUED"]
most_recent = true
}
`, domain)
}

func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain string) string {
return fmt.Sprintf(`
data "aws_acm_certificate" "test" {
domain = "%s"
types = ["IMPORTED"]
most_recent = true
}
`, domain)
}
8 changes: 8 additions & 0 deletions website/docs/d/acm_certificate.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ data "aws_acm_certificate" "example" {
domain = "tf.example.com"
statuses = ["ISSUED"]
}

data "aws_acm_certificate" "example" {
domain = "tf.example.com"
types = ["AMAZON_ISSUED"]
most_recent = true
}
```

## Argument Reference
Expand All @@ -29,6 +35,8 @@ data "aws_acm_certificate" "example" {
* `statuses` - (Optional) A list of statuses on which to filter the returned list. Valid values are `PENDING_VALIDATION`, `ISSUED`,
`INACTIVE`, `EXPIRED`, `VALIDATION_TIMED_OUT`, `REVOKED` and `FAILED`. If no value is specified, only certificates in the `ISSUED` state
are returned.
* `types` - (Optional) A list of types on which to filter the returned list. Valid values are `AMAZON_ISSUED` and `IMPORTED`.
* `most_recent` - (Optional) If set to true, it sorts the certificates matched by previous criteria by the NotBefore field, returning only the most recent one. If set to false, it returns an error if more than one certificate is found. Defaults to false.

## Attributes Reference

Expand Down