New Resource: aws_lb_listener_certificate #2686

Puneeth-n · 2017-12-16T13:55:29Z

Fixes the acceptance tests in #2649 and closes #2649 and #1853. as requested in this comment here

Removed the TLS resource in tests because sometimes the tests were failing because of "malformed certificate".
Fixed the segmentation fault
Add debug statements
Fixed the Destroy function to handle LB deletions in acceptance tests
Added a full cycle test

For some reason, I couldn't create a PR on the original branch hence creating the PR against master.

make testacc TESTARGS='-run=TestAccAwsLbListenerCertificate_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAwsLbListenerCertificate_ -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAwsLbListenerCertificate_basic
--- PASS: TestAccAwsLbListenerCertificate_basic (227.65s)
=== RUN   TestAccAwsLbListenerCertificate_cycle
--- PASS: TestAccAwsLbListenerCertificate_cycle (288.34s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	516.021s

Puneeth-n · 2017-12-22T13:09:30Z

@Ninir @radeksimko Can you please review it so that I and @oarmstrong see it to completion :)

oarmstrong · 2017-12-22T15:00:26Z

Should my PR #2649 be closed in favour of this one now then?

Puneeth-n · 2017-12-22T19:18:50Z

@oarmstrong feel free to to either close and work on this one or merge this to your branch and I close this one. Basically, I just fixed the acceptance tests in your branch.

ArmandSag · 2018-01-03T15:40:24Z

Hello guys,

Great feature here ! Do you have any idea when this will be released ?

Puneeth-n · 2018-01-08T10:08:55Z

@radeksimko @Ninir Can you please review it?

ArmandSag · 2018-01-15T12:41:41Z

Does anyone plan to review this ? This is so important to have only one application load balancer with all the certificates. Tx !!

egorchabala · 2018-01-15T21:09:06Z

Pls, review, since this functionality can be very useful for many cases

naveenacharya · 2018-01-28T16:11:37Z

Is there anyway to get this feature up asap? We have been waiting for this since a long time.

derBroBro · 2018-02-13T14:10:08Z

We have also a issue which would be solved by this. Maybe someone else like @bflad can support?

paddycarver

Hi @Puneeth-n,

Thanks for the PR! In running the tests, both fail for me:

 make testacc TEST=./aws TESTARGS='-run=TestAccAwsLbListenerCertificate_*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAwsLbListenerCertificate_* -timeout 120m
=== RUN   TestAccAwsLbListenerCertificate_basic
--- FAIL: TestAccAwsLbListenerCertificate_basic (192.75s)
	testing.go:503: Step 0 error: Error applying: 1 error(s) occurred:

		* aws_iam_server_certificate.default: 1 error(s) occurred:

		* aws_iam_server_certificate.default: [WARN] Error uploading server certificate, error: EntityAlreadyExists: The Server Certificate with name terraform-default-cert already exists.
=== RUN   TestAccAwsLbListenerCertificate_cycle
--- FAIL: TestAccAwsLbListenerCertificate_cycle (191.70s)
	testing.go:503: Step 0 error: Error applying: 1 error(s) occurred:

		* aws_iam_server_certificate.default: 1 error(s) occurred:

		* aws_iam_server_certificate.default: [WARN] Error uploading server certificate, error: EntityAlreadyExists: The Server Certificate with name terraform-default-cert already exists.
FAIL
exit status 1
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	384.503s

It seems the tests can't be run in parallel, because the certificate names chosen are hardcoded, but must be unique.

The following patch resolved the issue for me:

diff --git a/aws/resource_aws_lb_listener_certificate_test.go b/aws/resource_aws_lb_listener_certificate_test.go
index ee564cd1..46f15268 100644
--- a/aws/resource_aws_lb_listener_certificate_test.go
+++ b/aws/resource_aws_lb_listener_certificate_test.go
@@ -20,7 +20,7 @@ func TestAccAwsLbListenerCertificate_basic(t *testing.T) {
 		CheckDestroy: testAccCheckAwsLbListenerCertificateDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccLbListenerCertificateConfig(acctest.RandString(5)),
+				Config: testAccLbListenerCertificateConfig(acctest.RandString(5), acctest.RandString(5)),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"),
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"),
@@ -39,6 +39,7 @@ func TestAccAwsLbListenerCertificate_basic(t *testing.T) {

 func TestAccAwsLbListenerCertificate_cycle(t *testing.T) {
 	rName := acctest.RandString(5)
+	suffix := acctest.RandString(5)

 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
@@ -46,7 +47,7 @@ func TestAccAwsLbListenerCertificate_cycle(t *testing.T) {
 		CheckDestroy: testAccCheckAwsLbListenerCertificateDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccLbListenerCertificateConfig(rName),
+				Config: testAccLbListenerCertificateConfig(rName, suffix),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"),
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"),
@@ -60,7 +61,7 @@ func TestAccAwsLbListenerCertificate_cycle(t *testing.T) {
 				),
 			},
 			{
-				Config: testAccLbListenerCertificateAddNew(rName),
+				Config: testAccLbListenerCertificateAddNew(rName, suffix),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"),
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"),
@@ -77,7 +78,7 @@ func TestAccAwsLbListenerCertificate_cycle(t *testing.T) {
 				),
 			},
 			{
-				Config: testAccLbListenerCertificateConfig(rName),
+				Config: testAccLbListenerCertificateConfig(rName, suffix),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.default"),
 					testAccCheckAwsLbListenerCertificateExists("aws_lb_listener_certificate.additional_1"),
@@ -153,7 +154,7 @@ func testAccCheckAwsLbListenerCertificateNotExists(name string) resource.TestChe
 	}
 }

-func testAccLbListenerCertificateConfig(rName string) string {
+func testAccLbListenerCertificateConfig(rName, suffix string) string {
 	return fmt.Sprintf(`
 resource "aws_lb_listener_certificate" "default" {
   listener_arn    = "${aws_lb_listener.test.arn}"
@@ -196,7 +197,7 @@ resource "aws_lb_listener" "test" {
 }

 resource "aws_iam_server_certificate" "default" {
-  name             = "terraform-default-cert"
+  name             = "terraform-default-cert-%s"
   certificate_body = <<EOF
 -----BEGIN CERTIFICATE-----
 MIICpDCCAYwCCQC8EdACDsZ33jANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAls
@@ -252,7 +253,7 @@ EOF
 }

 resource "aws_iam_server_certificate" "additional_1" {
-  name             = "terraform-additional-cert-1"
+  name             = "terraform-additional-cert-1-%s"
   certificate_body = <<EOF
 -----BEGIN CERTIFICATE-----
 MIICpDCCAYwCCQD3BjmOb0++dDANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAls
@@ -308,7 +309,7 @@ EOF
 }

 resource "aws_iam_server_certificate" "additional_2" {
-  name             = "terraform-additional-cert-2"
+  name             = "terraform-additional-cert-2-%s"
   certificate_body = <<EOF
 -----BEGIN CERTIFICATE-----
 MIICpDCCAYwCCQDZ2oRa1sGckDANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAls
@@ -364,7 +365,7 @@ EOF
 }

 resource "aws_iam_server_certificate" "additional_3" {
-  name             = "terraform-additional-cert-3"
+  name             = "terraform-additional-cert-3-%s"
   certificate_body = <<EOF
 -----BEGIN CERTIFICATE-----
 MIICpDCCAYwCCQC5bnxXukDHoTANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDDAls
@@ -434,11 +435,11 @@ resource "aws_subnet" "test" {
   vpc_id            = "${aws_vpc.test.id}"
   cidr_block        = "${element(var.subnets, count.index)}"
   availability_zone = "${element(data.aws_availability_zones.available.names, count.index)}"
-}`, rName)
+}`, rName, suffix, suffix, suffix, suffix)
 }

-func testAccLbListenerCertificateAddNew(rName string) string {
-	return fmt.Sprintf(testAccLbListenerCertificateConfig(rName) + `
+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}"

Feel free to use that, or come up with your own solution.

Puneeth-n · 2018-02-20T12:43:38Z

@paddycarver Sure. Will give it a spin and get back to you.

…vider-aws into chore/alb-sni-fix

…ovider-aws into chore/alb-sni-fix

Puneeth-n · 2018-02-20T15:01:41Z

@paddycarver done!

TF_ACC=1 go test ./... -v -run=TestAccAwsLbListenerCertificate_ -timeout 120m -parallel=5
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
=== RUN   TestAccAwsLbListenerCertificate_basic
--- PASS: TestAccAwsLbListenerCertificate_basic (205.78s)
=== RUN   TestAccAwsLbListenerCertificate_cycle
--- PASS: TestAccAwsLbListenerCertificate_cycle (280.92s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	486.728s

paddycarver · 2018-02-20T16:14:17Z

Tests pass for me! Thanks!

Puneeth-n · 2018-02-20T16:46:33Z

@oarmstrong @paddycarver Thanks!

oarmstrong · 2018-02-20T16:59:46Z

Thanks for all your help @Puneeth-n on my original PR and for all the maintainers involved in getting this merged! Great work everyone!

cemo · 2018-02-20T19:44:15Z

thanks guys, huge milestone.

@paddycarver You may want also change milestone of this issue since it is targeted to v1.11.0. #1853 also requires a change.

bflad · 2018-02-27T12:41:52Z

This has been released in version 1.10.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

ghost · 2020-04-07T17:55:09Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

oarmstrong and others added 5 commits December 15, 2017 20:13

Create r/aws_lb_listener_certificate

11803d1

Register r/aws_lb_listener_certificate

99197c6

Document r/aws_lb_listener_certificate

08f4f1d

Add acctests for r/aws_lb_listner_certificate

036ae40

fix seg fault

cc65bb6

Puneeth-n force-pushed the chore/alb-sni-fix branch from ee52629 to 0f526d1 Compare December 16, 2017 14:04

Puneeth-n mentioned this pull request Dec 16, 2017

ALB SNI Support #1853

Closed

fix acceptance tests

be7076a

Puneeth-n force-pushed the chore/alb-sni-fix branch from 0f526d1 to be7076a Compare December 16, 2017 14:17

jen20 added the new-resource Introduces a new resource. label Dec 17, 2017

ringods mentioned this pull request Dec 18, 2017

Modules for ALB, NLB & corresponding listeners skyscrapers/terraform-loadbalancers#13

Merged

oarmstrong mentioned this pull request Jan 8, 2018

[WIP] New Resource: LB Listener Additional Certificates #2649

Closed

radeksimko added service/elb Issues and PRs that pertain to the elb service. service/elbv2 Issues and PRs that pertain to the elbv2 service. and removed service/elb Issues and PRs that pertain to the elb service. labels Jan 16, 2018

radeksimko changed the title ~~New Resource: LB Listener Additional Certificates~~ New Resource: aws_lb_listener_certificate Jan 16, 2018

radeksimko added this to the v1.11.0 milestone Jan 16, 2018

paddycarver self-requested a review January 29, 2018 15:13

paddycarver self-assigned this Jan 29, 2018

paddycarver suggested changes Feb 20, 2018

View reviewed changes

Puneeth-n added 5 commits February 20, 2018 13:46

Merge branch 'master' of github.com:terraform-providers/terraform-pro…

221362c

…vider-aws into chore/alb-sni-fix

make acceptance tests run in paralel

393cc6c

Merge branch 'chore/alb-sni-fix' of github.com:Puneeth-n/terraform-pr…

68a4328

…ovider-aws into chore/alb-sni-fix

Merge branch 'chore/alb-sni-fix' of github.com:Puneeth-n/terraform-pr…

2df0c2c

…ovider-aws into chore/alb-sni-fix

Merge branch 'chore/alb-sni-fix' of github.com:Puneeth-n/terraform-pr…

b67e363

…ovider-aws into chore/alb-sni-fix

ghost added the size/XL Managed by automation to categorize the size of a PR. label Feb 20, 2018

paddycarver approved these changes Feb 20, 2018

View reviewed changes

paddycarver merged commit 5a8f237 into hashicorp:master Feb 20, 2018

Puneeth-n deleted the chore/alb-sni-fix branch February 20, 2018 16:46

paddycarver modified the milestones: v1.11.0, v1.10.0 Feb 22, 2018

ghost locked and limited conversation to collaborators Apr 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: aws_lb_listener_certificate #2686

New Resource: aws_lb_listener_certificate #2686

Puneeth-n commented Dec 16, 2017 •

edited

Loading

Puneeth-n commented Dec 22, 2017

oarmstrong commented Dec 22, 2017

Puneeth-n commented Dec 22, 2017 •

edited

Loading

ArmandSag commented Jan 3, 2018

Puneeth-n commented Jan 8, 2018

ArmandSag commented Jan 15, 2018

egorchabala commented Jan 15, 2018

naveenacharya commented Jan 28, 2018

derBroBro commented Feb 13, 2018

paddycarver left a comment

Puneeth-n commented Feb 20, 2018

Puneeth-n commented Feb 20, 2018

paddycarver commented Feb 20, 2018

Puneeth-n commented Feb 20, 2018

oarmstrong commented Feb 20, 2018

cemo commented Feb 20, 2018

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

New Resource: aws_lb_listener_certificate #2686

New Resource: aws_lb_listener_certificate #2686

Conversation

Puneeth-n commented Dec 16, 2017 • edited Loading

Puneeth-n commented Dec 22, 2017

oarmstrong commented Dec 22, 2017

Puneeth-n commented Dec 22, 2017 • edited Loading

ArmandSag commented Jan 3, 2018

Puneeth-n commented Jan 8, 2018

ArmandSag commented Jan 15, 2018

egorchabala commented Jan 15, 2018

naveenacharya commented Jan 28, 2018

derBroBro commented Feb 13, 2018

paddycarver left a comment

Choose a reason for hiding this comment

Puneeth-n commented Feb 20, 2018

Puneeth-n commented Feb 20, 2018

paddycarver commented Feb 20, 2018

Puneeth-n commented Feb 20, 2018

oarmstrong commented Feb 20, 2018

cemo commented Feb 20, 2018

bflad commented Feb 27, 2018

ghost commented Apr 7, 2020

Puneeth-n commented Dec 16, 2017 •

edited

Loading

Puneeth-n commented Dec 22, 2017 •

edited

Loading