From 7aabffafd71f5b7d8d43140137b69436ced01985 Mon Sep 17 00:00:00 2001 From: stack72 Date: Fri, 11 Nov 2016 15:12:21 +0200 Subject: [PATCH] provider/aws: Fix panic in aws_acm_certificate datasource Fixes #10042 Fixes #9989 Another panic was found with this resource. IT essentially was causing a panic when no certificates were found. This was due to the casting of status to []string There are times when there are no statuses passed in. Made the error message a lot more generic now rather than having something like this ``` No certificate with statuses [] for domain mytestdomain.com found in this region. ``` This now becomes: ``` No certificate for domain mytestdomain.com found in this region. ``` Also, added a test to show that the panic is gone ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAwsAcmCertificateDataSource_' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2016/11/11 15:11:33 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAwsAcmCertificateDataSource_ -timeout 120m === RUN TestAccAwsAcmCertificateDataSource_noMatchReturnsError --- PASS: TestAccAwsAcmCertificateDataSource_noMatchReturnsError (6.07s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws6.094s ``` --- .../aws/data_source_aws_acm_certificate.go | 13 ++--- .../data_source_aws_acm_certificate_test.go | 53 +++++++------------ 2 files changed, 21 insertions(+), 45 deletions(-) diff --git a/builtin/providers/aws/data_source_aws_acm_certificate.go b/builtin/providers/aws/data_source_aws_acm_certificate.go index 0258e1721905..68b22184a2fd 100644 --- a/builtin/providers/aws/data_source_aws_acm_certificate.go +++ b/builtin/providers/aws/data_source_aws_acm_certificate.go @@ -2,7 +2,6 @@ package aws import ( "fmt" - "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -41,11 +40,7 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e statuses, ok := d.GetOk("statuses") if ok { statusStrings := statuses.([]interface{}) - statusList := make([]*string, len(statusStrings)) - for i, status := range statusStrings { - statusList[i] = aws.String(strings.ToUpper(status.(string))) - } - params.CertificateStatuses = statusList + params.CertificateStatuses = expandStringList(statusStrings) } else { params.CertificateStatuses = []*string{aws.String("ISSUED")} } @@ -65,12 +60,10 @@ func dataSourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) e } if len(arns) == 0 { - return fmt.Errorf("No certificate with statuses [%s] for domain %q found in this region.", - strings.Join(statuses.([]string), ", "), target) + return fmt.Errorf("No certificate for domain %q found in this region.", target) } if len(arns) > 1 { - return fmt.Errorf("Multiple certificates with statuses [%s] for domain %s found in this region.", - strings.Join(statuses.([]string), ","), target) + return fmt.Errorf("Multiple certificates for domain %q found in this region.", target) } d.SetId(time.Now().UTC().String()) diff --git a/builtin/providers/aws/data_source_aws_acm_certificate_test.go b/builtin/providers/aws/data_source_aws_acm_certificate_test.go index 382911eede01..a5b7140bf7de 100644 --- a/builtin/providers/aws/data_source_aws_acm_certificate_test.go +++ b/builtin/providers/aws/data_source_aws_acm_certificate_test.go @@ -2,62 +2,45 @@ package aws import ( "fmt" - "os" + "regexp" "testing" "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/terraform" ) -func TestAccAwsAcmCertificateDataSource_basic(t *testing.T) { - region := os.Getenv("AWS_ACM_TEST_REGION") - domain := os.Getenv("AWS_ACM_TEST_DOMAIN") - certArn := os.Getenv("AWS_ACM_TEST_CERT_ARN") +func TestAccAwsAcmCertificateDataSource_noMatchReturnsError(t *testing.T) { + domain := "hashicorp.com" resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) - if region == "" { - t.Skip("AWS_ACM_TEST_REGION must be set to a region an ACM certificate pre-created for this test.") - } - if domain == "" { - t.Skip("AWS_ACM_TEST_DOMAIN must be set to a domain with an ACM certificate pre-created for this test.") - } - if certArn == "" { - t.Skip("AWS_ACM_TEST_CERT_ARN must be set to the ARN of an ACM cert pre-created for this test.") - } }, Providers: testAccProviders, Steps: []resource.TestStep{ { - Config: testAccCheckAwsAcmCertificateDataSourceConfig(region, domain), - Check: testAccCheckAcmArnMatches("data.aws_acm_certificate.test", certArn), + Config: testAccCheckAwsAcmCertificateDataSourceConfig(domain), + ExpectError: regexp.MustCompile(`No certificate for domain`), + }, + { + Config: testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain), + ExpectError: regexp.MustCompile(`No certificate for domain`), }, }, }) } -func testAccCheckAcmArnMatches(name, expectArn string) resource.TestCheckFunc { - return func(s *terraform.State) error { - ms := s.RootModule() - rs, ok := ms.Resources[name] - if !ok { - return fmt.Errorf("Not found: %s", name) - } - gotArn := rs.Primary.Attributes["arn"] - if gotArn != expectArn { - return fmt.Errorf("Expected cert to have arn: %s, got: %s", expectArn, gotArn) - } - return nil - } +func testAccCheckAwsAcmCertificateDataSourceConfig(domain string) string { + return fmt.Sprintf(` +data "aws_acm_certificate" "test" { + domain = "%s" +} +`, domain) } -func testAccCheckAwsAcmCertificateDataSourceConfig(region, domain string) string { +func testAccCheckAwsAcmCertificateDataSourceConfigWithStatus(domain string) string { return fmt.Sprintf(` -provider "aws" { - region = "%s" -} data "aws_acm_certificate" "test" { domain = "%s" + statuses = ["ISSUED"] } - `, region, domain) +`, domain) }