Skip to content

Commit

Permalink
provider/aws: Fix panic in aws_acm_certificate datasource (#10051)
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
stack72 committed Nov 11, 2016
1 parent c843259 commit 06f4383
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 45 deletions.
13 changes: 3 additions & 10 deletions builtin/providers/aws/data_source_aws_acm_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package aws

import (
"fmt"
"strings"
"time"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -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")}
}
Expand All @@ -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())
Expand Down
53 changes: 18 additions & 35 deletions builtin/providers/aws/data_source_aws_acm_certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 06f4383

Please sign in to comment.