From ab43e80f20309311e84ff38804211cde776fe8be Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Sun, 29 Sep 2019 11:13:02 -0400 Subject: [PATCH] resource/aws_lb_listener_certificate: Retry CertificateNotFound errors on creation, use internal implementation for test TLS key/certificate Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/10023 Fixes the following issue discovered during acceptance testing due to IAM eventual consistency: ``` --- FAIL: TestAccAwsLbListenerCertificate_multiple (326.35s) testing.go:569: Step 1 error: errors during apply: Error: Error creating LB Listener Certificate: CertificateNotFound: Certificate 'arn:aws:iam::187416307283:server-certificate/tf-acc-test-3065966466410433081-additional-3' not found ``` This also refactors the testing configurations so they can share a common base configuration. Output from acceptance testing: ``` --- PASS: TestAccAwsLbListenerCertificate_basic (186.16s) --- PASS: TestAccAwsLbListenerCertificate_multiple (271.64s) ``` --- aws/resource_aws_lb_listener_certificate.go | 26 ++- ...source_aws_lb_listener_certificate_test.go | 186 +++++++++--------- 2 files changed, 109 insertions(+), 103 deletions(-) diff --git a/aws/resource_aws_lb_listener_certificate.go b/aws/resource_aws_lb_listener_certificate.go index 5ab93c07f16..4c1e426dbc6 100644 --- a/aws/resource_aws_lb_listener_certificate.go +++ b/aws/resource_aws_lb_listener_certificate.go @@ -1,7 +1,6 @@ package aws import ( - "errors" "fmt" "log" "time" @@ -46,13 +45,28 @@ func resourceAwsLbListenerCertificateCreate(d *schema.ResourceData, meta interfa } log.Printf("[DEBUG] Adding certificate: %s of listener: %s", d.Get("certificate_arn").(string), d.Get("listener_arn").(string)) - resp, err := conn.AddListenerCertificates(params) - if err != nil { - return fmt.Errorf("Error creating LB Listener Certificate: %s", err) + + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + _, err := conn.AddListenerCertificates(params) + + // Retry for IAM Server Certificate eventual consistency + if isAWSErr(err, elbv2.ErrCodeCertificateNotFoundException, "") { + return resource.RetryableError(err) + } + + if err != nil { + return resource.NonRetryableError(err) + } + + return nil + }) + + if isResourceTimeoutError(err) { + _, err = conn.AddListenerCertificates(params) } - if len(resp.Certificates) == 0 { - return errors.New("Error creating LB Listener Certificate: no certificates returned in response") + if err != nil { + return fmt.Errorf("error adding LB Listener Certificate: %s", err) } d.SetId(d.Get("listener_arn").(string) + "_" + d.Get("certificate_arn").(string)) diff --git a/aws/resource_aws_lb_listener_certificate_test.go b/aws/resource_aws_lb_listener_certificate_test.go index 3ddf2151806..6403f7486b7 100644 --- a/aws/resource_aws_lb_listener_certificate_test.go +++ b/aws/resource_aws_lb_listener_certificate_test.go @@ -13,40 +13,47 @@ import ( ) func TestAccAwsLbListenerCertificate_basic(t *testing.T) { + key := tlsRsaPrivateKeyPem(2048) + certificate := tlsRsaX509SelfSignedCertificatePem(key, "example.com") + iamServerCertificateResourceName := "aws_iam_server_certificate.test" + lbListenerResourceName := "aws_lb_listener.test" + resourceName := "aws_lb_listener_certificate.test" + rName := acctest.RandomWithPrefix("tf-acc-test") + resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProvidersWithTLS, + Providers: testAccProviders, CheckDestroy: testAccCheckAwsLbListenerCertificateDestroy, Steps: []resource.TestStep{ { - Config: testAccLbListenerCertificateConfig(acctest.RandString(5), acctest.RandString(5)), + Config: testAccLbListenerCertificateConfig(rName, key, certificate), Check: resource.ComposeTestCheckFunc( - testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"), - testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"), - testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_2"), - resource.TestCheckResourceAttrSet("aws_lb_listener_certificate.default", "listener_arn"), - resource.TestCheckResourceAttrSet("aws_lb_listener_certificate.default", "certificate_arn"), - resource.TestCheckResourceAttrSet("aws_lb_listener_certificate.additional_1", "listener_arn"), - resource.TestCheckResourceAttrSet("aws_lb_listener_certificate.additional_1", "certificate_arn"), - resource.TestCheckResourceAttrSet("aws_lb_listener_certificate.additional_2", "listener_arn"), - resource.TestCheckResourceAttrSet("aws_lb_listener_certificate.additional_2", "certificate_arn"), + testAccCheckAwsLbListenerCertificateExists(resourceName), + resource.TestCheckResourceAttrPair(resourceName, "certificate_arn", iamServerCertificateResourceName, "arn"), + resource.TestCheckResourceAttrPair(resourceName, "listener_arn", lbListenerResourceName, "arn"), ), }, }, }) } -func TestAccAwsLbListenerCertificate_cycle(t *testing.T) { - rName := acctest.RandString(5) - suffix := acctest.RandString(5) +func TestAccAwsLbListenerCertificate_multiple(t *testing.T) { + keys := make([]string, 4) + certificates := make([]string, 4) + for i := 0; i < 4; i++ { + keys[i] = tlsRsaPrivateKeyPem(2048) + certificates[i] = tlsRsaX509SelfSignedCertificatePem(keys[i], "example.com") + } + + rName := acctest.RandomWithPrefix("tf-acc-test") resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, - Providers: testAccProvidersWithTLS, + Providers: testAccProviders, CheckDestroy: testAccCheckAwsLbListenerCertificateDestroy, Steps: []resource.TestStep{ { - Config: testAccLbListenerCertificateConfig(rName, suffix), + Config: testAccLbListenerCertificateConfigMultiple(rName, keys, certificates), Check: resource.ComposeTestCheckFunc( testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"), testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"), @@ -60,7 +67,7 @@ func TestAccAwsLbListenerCertificate_cycle(t *testing.T) { ), }, { - Config: testAccLbListenerCertificateAddNew(rName, suffix), + Config: testAccLbListenerCertificateConfigMultipleAddNew(rName, keys, certificates), Check: resource.ComposeTestCheckFunc( testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"), testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"), @@ -77,7 +84,7 @@ func TestAccAwsLbListenerCertificate_cycle(t *testing.T) { ), }, { - Config: testAccLbListenerCertificateConfig(rName, suffix), + Config: testAccLbListenerCertificateConfigMultiple(rName, keys, certificates), Check: resource.ComposeTestCheckFunc( testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"), testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"), @@ -153,58 +160,46 @@ func testAccCheckAwsLbListenerCertificateNotExists(name string) resource.TestChe } } -func testAccLbListenerCertificateConfig(rName, suffix string) string { +func testAccLbListenerCertificateConfigLbListenerBase(rName, key, certificate string) string { return fmt.Sprintf(` -resource "tls_private_key" "test" { - count = 4 - - algorithm = "RSA" -} - -resource "tls_self_signed_cert" "test" { - count = 4 - - key_algorithm = "RSA" - private_key_pem = "${tls_private_key.test.*.private_key_pem[count.index]}" - validity_period_hours = 12 +data "aws_availability_zones" "available" {} - allowed_uses = [ - "key_encipherment", - "digital_signature", - "server_auth", - ] +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" - subject { - common_name = "example.com" - organization = "ACME Examples, Inc" + tags = { + Name = "terraform-testacc-lb-listener-certificate" } } -resource "aws_lb_listener_certificate" "default" { - listener_arn = "${aws_lb_listener.test.arn}" - certificate_arn = "${aws_iam_server_certificate.default.arn}" -} +resource "aws_subnet" "test" { + count = 2 -resource "aws_lb_listener_certificate" "additional_1" { - listener_arn = "${aws_lb_listener.test.arn}" - certificate_arn = "${aws_iam_server_certificate.additional_1.arn}" + availability_zone = "${data.aws_availability_zones.available.names[count.index]}" + cidr_block = "10.0.${count.index}.0/24" + vpc_id = "${aws_vpc.test.id}" + + tags = { + Name = "tf-acc-lb-listener-certificate-${count.index}" + } } -resource "aws_lb_listener_certificate" "additional_2" { - listener_arn = "${aws_lb_listener.test.arn}" - certificate_arn = "${aws_iam_server_certificate.additional_2.arn}" +resource "aws_lb_target_group" "test" { + port = 80 + protocol = "HTTP" + vpc_id = "${aws_vpc.test.id}" } resource "aws_lb" "test" { - name_prefix = "%s" - subnets = ["${aws_subnet.test.*.id[0]}", "${aws_subnet.test.*.id[1]}"] - internal = true + internal = true + name = "%[1]s" + subnets = ["${aws_subnet.test.*.id[0]}", "${aws_subnet.test.*.id[1]}"] } -resource "aws_lb_target_group" "test" { - port = "443" - protocol = "HTTP" - vpc_id = "${aws_vpc.test.id}" +resource "aws_iam_server_certificate" "test" { + name = "%[1]s" + certificate_body = "%[2]s" + private_key = "%[3]s" } resource "aws_lb_listener" "test" { @@ -212,70 +207,67 @@ resource "aws_lb_listener" "test" { port = "443" protocol = "HTTPS" ssl_policy = "ELBSecurityPolicy-2016-08" - certificate_arn = "${aws_iam_server_certificate.default.arn}" + certificate_arn = "${aws_iam_server_certificate.test.arn}" default_action { target_group_arn = "${aws_lb_target_group.test.arn}" type = "forward" } } - -resource "aws_iam_server_certificate" "default" { - name = "terraform-default-cert-%s" - certificate_body = "${tls_self_signed_cert.test.*.cert_pem[0]}" - private_key = "${tls_private_key.test.*.private_key_pem[0]}" +`, rName, tlsPemEscapeNewlines(certificate), tlsPemEscapeNewlines(key)) } -resource "aws_iam_server_certificate" "additional_1" { - name = "terraform-additional-cert-1-%s" - certificate_body = "${tls_self_signed_cert.test.*.cert_pem[1]}" - private_key = "${tls_private_key.test.*.private_key_pem[1]}" +func testAccLbListenerCertificateConfig(rName, key, certificate string) string { + return testAccLbListenerCertificateConfigLbListenerBase(rName, key, certificate) + fmt.Sprintf(` +resource "aws_lb_listener_certificate" "test" { + certificate_arn = "${aws_iam_server_certificate.test.arn}" + listener_arn = "${aws_lb_listener.test.arn}" } - -resource "aws_iam_server_certificate" "additional_2" { - name = "terraform-additional-cert-2-%s" - certificate_body = "${tls_self_signed_cert.test.*.cert_pem[2]}" - private_key = "${tls_private_key.test.*.private_key_pem[2]}" +`) } -resource "aws_iam_server_certificate" "additional_3" { - name = "terraform-additional-cert-3-%s" - certificate_body = "${tls_self_signed_cert.test.*.cert_pem[3]}" - private_key = "${tls_private_key.test.*.private_key_pem[3]}" +func testAccLbListenerCertificateConfigMultiple(rName string, keys, certificates []string) string { + return testAccLbListenerCertificateConfigLbListenerBase(rName, keys[0], certificates[0]) + fmt.Sprintf(` +resource "aws_lb_listener_certificate" "default" { + listener_arn = "${aws_lb_listener.test.arn}" + certificate_arn = "${aws_iam_server_certificate.test.arn}" } -data "aws_availability_zones" "available" {} - -resource "aws_vpc" "test" { - cidr_block = "10.0.0.0/16" - - tags = { - Name = "terraform-testacc-lb-listener-certificate" - } +resource "aws_lb_listener_certificate" "additional_1" { + listener_arn = "${aws_lb_listener.test.arn}" + certificate_arn = "${aws_iam_server_certificate.additional_1.arn}" } -variable "subnets" { - default = ["10.0.1.0/24", "10.0.2.0/24"] +resource "aws_lb_listener_certificate" "additional_2" { + listener_arn = "${aws_lb_listener.test.arn}" + certificate_arn = "${aws_iam_server_certificate.additional_2.arn}" } -resource "aws_subnet" "test" { - count = "${length(var.subnets)}" - vpc_id = "${aws_vpc.test.id}" - cidr_block = "${element(var.subnets, count.index)}" - availability_zone = "${element(data.aws_availability_zones.available.names, count.index)}" +resource "aws_iam_server_certificate" "additional_1" { + name = "%[1]s-additional-1" + certificate_body = "%[2]s" + private_key = "%[3]s" +} - tags = { - Name = "tf-acc-lb-listener-certificate-${count.index}" - } +resource "aws_iam_server_certificate" "additional_2" { + name = "%[1]s-additional-2" + certificate_body = "%[4]s" + private_key = "%[5]s" } -`, rName, suffix, suffix, suffix, suffix) +`, rName, tlsPemEscapeNewlines(certificates[1]), tlsPemEscapeNewlines(keys[1]), tlsPemEscapeNewlines(certificates[2]), tlsPemEscapeNewlines(keys[2])) +} + +func testAccLbListenerCertificateConfigMultipleAddNew(rName string, keys, certificates []string) string { + return testAccLbListenerCertificateConfigMultiple(rName, keys, certificates) + fmt.Sprintf(` +resource "aws_iam_server_certificate" "additional_3" { + name = "%[1]s-additional-3" + certificate_body = "%[2]s" + private_key = "%[3]s" } -func testAccLbListenerCertificateAddNew(rName, prefix string) string { - return fmt.Sprintf(testAccLbListenerCertificateConfig(rName, prefix) + ` resource "aws_lb_listener_certificate" "additional_3" { listener_arn = "${aws_lb_listener.test.arn}" certificate_arn = "${aws_iam_server_certificate.additional_3.arn}" } -`) +`, rName, tlsPemEscapeNewlines(certificates[3]), tlsPemEscapeNewlines(keys[3])) }