From 8e6df0b580801a1754ab40c690f98e31f772618d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Sun, 8 Oct 2017 20:13:24 +0200 Subject: [PATCH 1/9] Update documentation with missing implemented field --- website/docs/d/acm_certificate.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/d/acm_certificate.html.markdown b/website/docs/d/acm_certificate.html.markdown index 992067e3062..7e4a607e309 100644 --- a/website/docs/d/acm_certificate.html.markdown +++ b/website/docs/d/acm_certificate.html.markdown @@ -29,7 +29,7 @@ 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` ## Attributes Reference * `arn` - Set to the ARN of the found certificate, suitable for referencing in other resources that support ACM certificates. From 18f17cc3eedda2515ec5b4ab28fba235e36ba45b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Sun, 8 Oct 2017 20:14:43 +0200 Subject: [PATCH 2/9] Documentation for 'most_recent' flag --- website/docs/d/acm_certificate.html.markdown | 2 ++ 1 file changed, 2 insertions(+) diff --git a/website/docs/d/acm_certificate.html.markdown b/website/docs/d/acm_certificate.html.markdown index 7e4a607e309..1e7c3664b12 100644 --- a/website/docs/d/acm_certificate.html.markdown +++ b/website/docs/d/acm_certificate.html.markdown @@ -20,6 +20,7 @@ them by domain without having to hard code the ARNs as input. data "aws_acm_certificate" "example" { domain = "tf.example.com" statuses = ["ISSUED"] + most_recent = true } ``` @@ -30,6 +31,7 @@ data "aws_acm_certificate" "example" { `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 certificates matched by previous criterias by the NotBefore field, returning only the most recent one. If set to false, it returns an error if more than one certificates are found. Defaults to false. ## Attributes Reference * `arn` - Set to the ARN of the found certificate, suitable for referencing in other resources that support ACM certificates. From 69645a60bc78ec1de25f3af334b56582935017ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Sun, 8 Oct 2017 20:17:04 +0200 Subject: [PATCH 3/9] Test for coverage based on guidelines --- aws/data_source_aws_acm_certificate_test.go | 41 +++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/aws/data_source_aws_acm_certificate_test.go b/aws/data_source_aws_acm_certificate_test.go index a862b12e742..1c9b0c89d00 100644 --- a/aws/data_source_aws_acm_certificate_test.go +++ b/aws/data_source_aws_acm_certificate_test.go @@ -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`), + }, }, }) } @@ -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) +} From 607f62000c9483d61bda28636e24316d2553a110 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Sun, 8 Oct 2017 20:17:53 +0200 Subject: [PATCH 4/9] Implement most_recent flag to filter matching certificates by NotBefore field --- aws/data_source_aws_acm_certificate.go | 69 ++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/aws/data_source_aws_acm_certificate.go b/aws/data_source_aws_acm_certificate.go index 12ce7157634..abf87350715 100644 --- a/aws/data_source_aws_acm_certificate.go +++ b/aws/data_source_aws_acm_certificate.go @@ -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}, + }, }, } } +type arnData struct { + arn string + notBefore *time.Time +} + +func describeCertificate(arn *arnData, conn *acm.ACM) (*acm.DescribeCertificateOutput, error) { + 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 @@ -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 } } @@ -93,12 +116,38 @@ 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) } + + // Get most recent sorting by notBefore date. Notice that createdAt field is only valid + // for ACM issued certificated but not for imported ones so in a mixed scenario only + // fields extracted from the certificate are valid. I cannot find a scenario where the + // most recent certificate is not the one with a most recent `NotBefore` field. + _, ok = d.GetOk("most_recent") + if ok { + mr := arns[0] + 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} + } + if len(arns) > 1 { 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 } From cbb12a8461ed665b9486ea07fc78128d4ce72179 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Sun, 8 Oct 2017 21:27:05 +0200 Subject: [PATCH 5/9] Fill base case fields when nil. --- aws/data_source_aws_acm_certificate.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/aws/data_source_aws_acm_certificate.go b/aws/data_source_aws_acm_certificate.go index abf87350715..66709ddc0e6 100644 --- a/aws/data_source_aws_acm_certificate.go +++ b/aws/data_source_aws_acm_certificate.go @@ -124,6 +124,14 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e _, 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) + } + + mr.notBefore = description.Certificate.NotBefore + } for _, arn := range arns[1:] { if arn.notBefore == nil { description, err := describeCertificate(arn, conn) From ab4396c5874db72d0a63aedf524dfe0bf9ce8e3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Tue, 10 Oct 2017 13:17:12 +0200 Subject: [PATCH 6/9] Fix documentation and include more examples --- website/docs/d/acm_certificate.html.markdown | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/website/docs/d/acm_certificate.html.markdown b/website/docs/d/acm_certificate.html.markdown index 1e7c3664b12..0d169a1bfda 100644 --- a/website/docs/d/acm_certificate.html.markdown +++ b/website/docs/d/acm_certificate.html.markdown @@ -20,6 +20,11 @@ them by domain without having to hard code the ARNs as input. 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 } ``` @@ -30,8 +35,9 @@ 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 certificates matched by previous criterias by the NotBefore field, returning only the most recent one. If set to false, it returns an error if more than one certificates are found. Defaults to false. + * `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 * `arn` - Set to the ARN of the found certificate, suitable for referencing in other resources that support ACM certificates. From ed070f29a13646784e1d76fad66f4c5ff679c2aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Tue, 10 Oct 2017 13:18:32 +0200 Subject: [PATCH 7/9] Improve efficiency by only using most_recent when there are several certs --- aws/data_source_aws_acm_certificate.go | 53 +++++++++++++------------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/aws/data_source_aws_acm_certificate.go b/aws/data_source_aws_acm_certificate.go index 66709ddc0e6..0b2b367e692 100644 --- a/aws/data_source_aws_acm_certificate.go +++ b/aws/data_source_aws_acm_certificate.go @@ -117,41 +117,40 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e return fmt.Errorf("No certificate 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 certificated but not for imported ones so in a mixed scenario only - // fields extracted from the certificate are valid. I cannot find a scenario where the - // most recent certificate is not the one with a most recent `NotBefore` field. - _, 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) - } - - mr.notBefore = description.Certificate.NotBefore - } - for _, arn := range arns[1:] { - if arn.notBefore == nil { - description, err := describeCertificate(arn, conn) + if len(arns) > 1 { + // 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) } - arn.notBefore = description.Certificate.NotBefore + 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 + if arn.notBefore.After(*mr.notBefore) { + mr = arn + } } - } - arns = []*arnData{mr} - } - - if len(arns) > 1 { - return fmt.Errorf("Multiple certificates for domain %q found in this region.", target) + arns = []*arnData{mr} + } else { + return fmt.Errorf("Multiple certificates for domain %q found in this region.", target) + } } d.SetId(time.Now().UTC().String()) From a75e3b4ad79251627ebb55cb1fb2b79ed2a45f26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Gonz=C3=A1lez?= Date: Sun, 28 Jan 2018 21:29:31 +0100 Subject: [PATCH 8/9] Apply feedback --- aws/data_source_aws_acm_certificate.go | 140 ++++++++++++------------- 1 file changed, 67 insertions(+), 73 deletions(-) diff --git a/aws/data_source_aws_acm_certificate.go b/aws/data_source_aws_acm_certificate.go index 0b2b367e692..14cfbfb0825 100644 --- a/aws/data_source_aws_acm_certificate.go +++ b/aws/data_source_aws_acm_certificate.go @@ -7,7 +7,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/acm" - "github.com/hashicorp/errwrap" "github.com/hashicorp/terraform/helper/schema" ) @@ -37,27 +36,18 @@ func dataSourceAwsAcmCertificate() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: false, - Elem: &schema.Schema{Type: schema.TypeString}, }, }, } } -type arnData struct { - arn string - notBefore *time.Time -} - -func describeCertificate(arn *arnData, conn *acm.ACM) (*acm.DescribeCertificateOutput, error) { - params := &acm.DescribeCertificateInput{} - params.CertificateArn = &arn.arn - - description, err := conn.DescribeCertificate(params) - if err != nil { - return nil, err +func describeCertificateByArn(arn *string, conn *acm.ACM) (*acm.CertificateDetail, error) { + input := &acm.DescribeCertificateInput{ + CertificateArn: aws.String(*arn), } - - return description, nil + log.Printf("[DEBUG] Describing ACM Certificate: %s", input) + output, err := conn.DescribeCertificate(input) + return output.Certificate, err } func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) error { @@ -73,88 +63,92 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e params.CertificateStatuses = []*string{aws.String("ISSUED")} } - var arns []*arnData + var arns []*string 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, &arnData{*cert.CertificateArn, nil}) + arns = append(arns, cert.CertificateArn) } } return true }) if err != nil { - 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 []*arnData - for _, arn := range arns { - 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, - &arnData{arn.arn, description.Certificate.NotBefore}, - ) - break - } - } - } - - arns = matchedArns + return fmt.Errorf("Error listing certificates: %q", err) } if len(arns) == 0 { - return fmt.Errorf("No certificate for domain %q found in this region.", target) + return fmt.Errorf("No certificate for domain %q found in this region", target) } - if len(arns) > 1 { - // 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) - } + filterMostRecent := d.Get("most_recent").(bool) + filterTypes, filterTypesOk := d.GetOk("types") + + var matchedCertificate *acm.CertificateDetail + + if !filterMostRecent && !filterTypesOk { + if len(arns) > 1 { + // Multiple certificates have been found and no flags set. Error + return fmt.Errorf("Multiple certificates for domain %q found in this region", target) + } + // Only 1 certificate has been found and no flags set. Get details and return it. + matchedCertificate, err = describeCertificateByArn(arns[0], conn) + if err != nil { + return fmt.Errorf("Error describing ACM certificate: %q", err) + } + } else { + typesStrings := expandStringList(filterTypes.([]interface{})) - mr.notBefore = description.Certificate.NotBefore + for _, arn := range arns { + certificate, err := describeCertificateByArn(arn, conn) + if err != nil { + return fmt.Errorf("Error describing ACM certificate: %q", err) } - 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) + if filterTypesOk { + for _, certType := range typesStrings { + if *certificate.Type == *certType { + // We do not have a candidate certificate + if matchedCertificate == nil { + matchedCertificate = certificate + continue + } + // At this point, we already have a candidate certificate + // Check if we are filtering by most recent and update if necessary + if filterMostRecent && (*certificate.NotBefore).After(*matchedCertificate.NotBefore) { + matchedCertificate = certificate + break + } + // Now we have multiple candidate certificates and we only allow one certificate + return fmt.Errorf("Multiple certificates for domain %q found in this region", target) } - - arn.notBefore = description.Certificate.NotBefore } - - if arn.notBefore.After(*mr.notBefore) { - mr = arn + continue + } + // At this point, we already have a candidate certificate + // Check if we are filtering by most recent and update if necessary + if filterMostRecent { + // We do not have a candidate certificate + if matchedCertificate == nil { + matchedCertificate = certificate + continue } + if (*certificate.NotBefore).After(*matchedCertificate.NotBefore) { + matchedCertificate = certificate + } + continue } - - arns = []*arnData{mr} - } else { - return fmt.Errorf("Multiple certificates for domain %q found in this region.", target) + // Now we have multiple candidate certificates and we only allow one certificate + return fmt.Errorf("Multiple certificates for domain %q found in this region", target) } } + if matchedCertificate == nil { + return fmt.Errorf("No certificate for domain %q found in this region", target) + } + d.SetId(time.Now().UTC().String()) - d.Set("arn", arns[0].arn) + d.Set("arn", matchedCertificate.CertificateArn) return nil } From 961c68dd497c07aa44006092bdea541a524eba32 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 1 Feb 2018 15:27:52 -0500 Subject: [PATCH 9/9] data-source/aws_acm_certificate: Prevent crash on most_recent filtering non-ISSUED certificates and add initial acceptance testing --- aws/data_source_aws_acm_certificate.go | 120 +++++++------ aws/data_source_aws_acm_certificate_test.go | 185 +++++++++++++++++--- 2 files changed, 226 insertions(+), 79 deletions(-) diff --git a/aws/data_source_aws_acm_certificate.go b/aws/data_source_aws_acm_certificate.go index 14cfbfb0825..a49ca9d88fc 100644 --- a/aws/data_source_aws_acm_certificate.go +++ b/aws/data_source_aws_acm_certificate.go @@ -41,15 +41,6 @@ func dataSourceAwsAcmCertificate() *schema.Resource { } } -func describeCertificateByArn(arn *string, conn *acm.ACM) (*acm.CertificateDetail, error) { - input := &acm.DescribeCertificateInput{ - CertificateArn: aws.String(*arn), - } - log.Printf("[DEBUG] Describing ACM Certificate: %s", input) - output, err := conn.DescribeCertificate(input) - return output.Certificate, err -} - func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*AWSClient).acmconn @@ -87,60 +78,65 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e var matchedCertificate *acm.CertificateDetail - if !filterMostRecent && !filterTypesOk { - if len(arns) > 1 { - // Multiple certificates have been found and no flags set. Error - return fmt.Errorf("Multiple certificates for domain %q found in this region", target) + if !filterMostRecent && !filterTypesOk && len(arns) > 1 { + // Multiple certificates have been found and no additional filtering set + return fmt.Errorf("Multiple certificates for domain %q found in this region", target) + } + + typesStrings := expandStringList(filterTypes.([]interface{})) + + for _, arn := range arns { + var err error + + input := &acm.DescribeCertificateInput{ + CertificateArn: aws.String(*arn), } - // Only 1 certificate has been found and no flags set. Get details and return it. - matchedCertificate, err = describeCertificateByArn(arns[0], conn) + log.Printf("[DEBUG] Describing ACM Certificate: %s", input) + output, err := conn.DescribeCertificate(input) if err != nil { return fmt.Errorf("Error describing ACM certificate: %q", err) } - } else { - typesStrings := expandStringList(filterTypes.([]interface{})) - - for _, arn := range arns { - certificate, err := describeCertificateByArn(arn, conn) - if err != nil { - return fmt.Errorf("Error describing ACM certificate: %q", err) - } - if filterTypesOk { - for _, certType := range typesStrings { - if *certificate.Type == *certType { - // We do not have a candidate certificate - if matchedCertificate == nil { - matchedCertificate = certificate - continue - } - // At this point, we already have a candidate certificate - // Check if we are filtering by most recent and update if necessary - if filterMostRecent && (*certificate.NotBefore).After(*matchedCertificate.NotBefore) { - matchedCertificate = certificate - break + certificate := output.Certificate + + if filterTypesOk { + for _, certType := range typesStrings { + if *certificate.Type == *certType { + // We do not have a candidate certificate + if matchedCertificate == nil { + matchedCertificate = certificate + break + } + // At this point, we already have a candidate certificate + // Check if we are filtering by most recent and update if necessary + if filterMostRecent { + matchedCertificate, err = mostRecentAcmCertificate(certificate, matchedCertificate) + if err != nil { + return err } - // Now we have multiple candidate certificates and we only allow one certificate - return fmt.Errorf("Multiple certificates for domain %q found in this region", target) + break } + // Now we have multiple candidate certificates and we only allow one certificate + return fmt.Errorf("Multiple certificates for domain %q found in this region", target) } - continue } - // At this point, we already have a candidate certificate - // Check if we are filtering by most recent and update if necessary - if filterMostRecent { - // We do not have a candidate certificate - if matchedCertificate == nil { - matchedCertificate = certificate - continue - } - if (*certificate.NotBefore).After(*matchedCertificate.NotBefore) { - matchedCertificate = certificate - } - continue + continue + } + // We do not have a candidate certificate + if matchedCertificate == nil { + matchedCertificate = certificate + continue + } + // At this point, we already have a candidate certificate + // Check if we are filtering by most recent and update if necessary + if filterMostRecent { + matchedCertificate, err = mostRecentAcmCertificate(certificate, matchedCertificate) + if err != nil { + return err } - // Now we have multiple candidate certificates and we only allow one certificate - return fmt.Errorf("Multiple certificates for domain %q found in this region", target) + continue } + // Now we have multiple candidate certificates and we only allow one certificate + return fmt.Errorf("Multiple certificates for domain %q found in this region", target) } if matchedCertificate == nil { @@ -152,3 +148,21 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e return nil } + +func mostRecentAcmCertificate(i, j *acm.CertificateDetail) (*acm.CertificateDetail, error) { + if *i.Status != *j.Status { + return nil, fmt.Errorf("most_recent filtering on different ACM certificate statues is not supported") + } + // Cover IMPORTED and ISSUED AMAZON_ISSUED certificates + if *i.Status == acm.CertificateStatusIssued { + if (*i.NotBefore).After(*j.NotBefore) { + return i, nil + } + return j, nil + } + // Cover non-ISSUED AMAZON_ISSUED certificates + if (*i.CreatedAt).After(*j.CreatedAt) { + return i, nil + } + return j, nil +} diff --git a/aws/data_source_aws_acm_certificate_test.go b/aws/data_source_aws_acm_certificate_test.go index 1c9b0c89d00..2cdd1c8c7af 100644 --- a/aws/data_source_aws_acm_certificate_test.go +++ b/aws/data_source_aws_acm_certificate_test.go @@ -2,18 +2,151 @@ package aws import ( "fmt" + "os" "regexp" "testing" + "github.com/aws/aws-sdk-go/service/acm" "github.com/hashicorp/terraform/helper/resource" ) -func TestAccAwsAcmCertificateDataSource_noMatchReturnsError(t *testing.T) { - domain := "hashicorp.com" +const ACMCertificateRe = `^arn:[^:]+:acm:[^:]+:[^:]+:certificate/.+$` + +func TestAccAwsAcmCertificateDataSource_singleIssued(t *testing.T) { + if os.Getenv("ACM_CERTIFICATE_ROOT_DOMAIN") == "" { + t.Skip("Environment variable ACM_CERTIFICATE_ROOT_DOMAIN is not set") + } + + var arnRe *regexp.Regexp + var domain string + + if os.Getenv("ACM_CERTIFICATE_SINGLE_ISSUED_MOST_RECENT_ARN") != "" { + arnRe = regexp.MustCompile(fmt.Sprintf("^%s$", os.Getenv("ACM_CERTIFICATE_SINGLE_ISSUED_MOST_RECENT_ARN"))) + } else { + arnRe = regexp.MustCompile(ACMCertificateRe) + } + + if os.Getenv("ACM_CERTIFICATE_SINGLE_ISSUED_DOMAIN") != "" { + domain = os.Getenv("ACM_CERTIFICATE_SINGLE_ISSUED_DOMAIN") + } else { + domain = fmt.Sprintf("tf-acc-single-issued.%s", os.Getenv("ACM_CERTIFICATE_ROOT_DOMAIN")) + } + + resourceName := "data.aws_acm_certificate.test" + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckAwsAcmCertificateDataSourceConfig(domain), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain, acm.CertificateStatusIssued), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain, acm.CertificateTypeAmazonIssued), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain, true), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain, acm.CertificateStatusIssued, true), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain, acm.CertificateTypeAmazonIssued, true), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + }, + }) +} + +func TestAccAwsAcmCertificateDataSource_multipleIssued(t *testing.T) { + if os.Getenv("ACM_CERTIFICATE_ROOT_DOMAIN") == "" { + t.Skip("Environment variable ACM_CERTIFICATE_ROOT_DOMAIN is not set") + } + + var arnRe *regexp.Regexp + var domain string + + if os.Getenv("ACM_CERTIFICATE_MULTIPLE_ISSUED_MOST_RECENT_ARN") != "" { + arnRe = regexp.MustCompile(fmt.Sprintf("^%s$", os.Getenv("ACM_CERTIFICATE_MULTIPLE_ISSUED_MOST_RECENT_ARN"))) + } else { + arnRe = regexp.MustCompile(ACMCertificateRe) + } + + if os.Getenv("ACM_CERTIFICATE_MULTIPLE_ISSUED_DOMAIN") != "" { + domain = os.Getenv("ACM_CERTIFICATE_MULTIPLE_ISSUED_DOMAIN") + } else { + domain = fmt.Sprintf("tf-acc-multiple-issued.%s", os.Getenv("ACM_CERTIFICATE_ROOT_DOMAIN")) + } + + resourceName := "data.aws_acm_certificate.test" + resource.Test(t, resource.TestCase{ - PreCheck: func() { - testAccPreCheck(t) + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccCheckAwsAcmCertificateDataSourceConfig(domain), + ExpectError: regexp.MustCompile(`Multiple certificates for domain`), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain, acm.CertificateStatusIssued), + ExpectError: regexp.MustCompile(`Multiple certificates for domain`), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain, acm.CertificateTypeAmazonIssued), + ExpectError: regexp.MustCompile(`Multiple certificates for domain`), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain, true), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain, acm.CertificateStatusIssued, true), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain, acm.CertificateTypeAmazonIssued, true), + Check: resource.ComposeTestCheckFunc( + resource.TestMatchResourceAttr(resourceName, "arn", arnRe), + ), + }, }, + }) +} + +func TestAccAwsAcmCertificateDataSource_noMatchReturnsError(t *testing.T) { + if os.Getenv("ACM_CERTIFICATE_ROOT_DOMAIN") == "" { + t.Skip("Environment variable ACM_CERTIFICATE_ROOT_DOMAIN is not set") + } + + domain := fmt.Sprintf("tf-acc-nonexistent.%s", os.Getenv("ACM_CERTIFICATE_ROOT_DOMAIN")) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, Steps: []resource.TestStep{ { @@ -21,23 +154,23 @@ func TestAccAwsAcmCertificateDataSource_noMatchReturnsError(t *testing.T) { ExpectError: regexp.MustCompile(`No certificate for domain`), }, { - Config: testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain), + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain, acm.CertificateStatusIssued), ExpectError: regexp.MustCompile(`No certificate for domain`), }, { - Config: testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain), + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain, acm.CertificateTypeAmazonIssued), ExpectError: regexp.MustCompile(`No certificate for domain`), }, { - Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain), + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain, true), ExpectError: regexp.MustCompile(`No certificate for domain`), }, { - Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain), + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain, acm.CertificateStatusIssued, true), ExpectError: regexp.MustCompile(`No certificate for domain`), }, { - Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain), + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain, acm.CertificateTypeAmazonIssued, true), ExpectError: regexp.MustCompile(`No certificate for domain`), }, }, @@ -52,49 +185,49 @@ data "aws_acm_certificate" "test" { `, domain) } -func testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain string) string { +func testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain, status string) string { return fmt.Sprintf(` data "aws_acm_certificate" "test" { domain = "%s" - statuses = ["ISSUED"] + statuses = ["%s"] } -`, domain) +`, domain, status) } -func testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain string) string { +func testAccCheckAwsAcmCertificateDataSourceConfigWithTypes(domain, certType string) string { return fmt.Sprintf(` data "aws_acm_certificate" "test" { domain = "%s" - types = ["IMPORTED"] + types = ["%s"] } -`, domain) +`, domain, certType) } -func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain string) string { +func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecent(domain string, mostRecent bool) string { return fmt.Sprintf(` data "aws_acm_certificate" "test" { domain = "%s" - most_recent = true + most_recent = %v } -`, domain) +`, domain, mostRecent) } -func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain string) string { +func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndStatus(domain, status string, mostRecent bool) string { return fmt.Sprintf(` data "aws_acm_certificate" "test" { domain = "%s" - statuses = ["ISSUED"] - most_recent = true + statuses = ["%s"] + most_recent = %v } -`, domain) +`, domain, status, mostRecent) } -func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain string) string { +func testAccCheckAwsAcmCertificateDataSourceConfigWithMostRecentAndTypes(domain, certType string, mostRecent bool) string { return fmt.Sprintf(` data "aws_acm_certificate" "test" { domain = "%s" - types = ["IMPORTED"] - most_recent = true + types = ["%s"] + most_recent = %v } -`, domain) +`, domain, certType, mostRecent) }