Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(elasticloadbalancingv2): adding a new certificate to a multi-certificate listener will lose old extra ceritifactes #13150

Closed
akefirad opened this issue Feb 19, 2021 · 3 comments · Fixed by #13332
Assignees
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1

Comments

@akefirad
Copy link

akefirad commented Feb 19, 2021

If you try to add a new certificate to a listener that has already at least one extra certificate (i.e. one in addition to the main certificate), the old extract certificate will be lost by the end of update operation.

Reproduction Steps

(In my case, it happened like this:)

  1. Create an HTTPS listener with two certificates; foo.com (main one) and bar.com (additional one) stack and deploy it.
  2. Change the code and add a new certificate; baz.com to the list and deploy it.

What did you expect to happen?

The listener should keep one main certificate and two extra certificates.

What actually happened?

The old extra certificate (bar.com) is gone.

Environment

  • CLI Version : 1.89.0
  • Framework Version: 1.89.0 (build df7253c)
  • Node.js Version: v15.5.1
  • OS : Mac OS X 11.2
  • Language (Version): typescript ~3.8.2

Other

Please note that, according to my findings from AWS support, this seems to be a bug/feature of CloudFormation. Interestingly, even though the Certificates field is of type Array, the documentation says: You can specify one certificate per resource.
If my understanding is correct, CDK should not put all additional certificates into one AWS::ElasticLoadBalancingV2::ListenerCertificate resource. It should be one AWS::ElasticLoadBalancingV2::ListenerCertificate resource per each additional certificate.


This is 🐛 Bug Report

@akefirad akefirad added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 19, 2021
@github-actions github-actions bot added the @aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 label Feb 19, 2021
@akefirad akefirad changed the title (elasticloadbalancingv2): adding a new certificcate to a multi-certificate listener will lose old extra ceritifactes (elasticloadbalancingv2): adding a new certificate to a multi-certificate listener will lose old extra ceritifactes Feb 19, 2021
@njlynch njlynch added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 1, 2021
@njlynch
Copy link
Contributor

njlynch commented Mar 1, 2021

Thanks for the bug report -- and for including the response from support!

I suspect the answer is to alter the below code block:

if (additionalCerts.length > 0) {
new ApplicationListenerCertificate(this, id, {
listener: this,
certificates: additionalCerts,
});
}

Rather than creating one ApplicationListenerCertificate for the remainder certificates, it appears one ApplicationListenerCertificate needs to be created per certificate.

This should be a relatively straightforward change; PRs welcome if anyone has the time to go after it!

@andreialecu
Copy link
Contributor

andreialecu commented Mar 1, 2021

@njlynch: I just made an attempt at this in #13332

@mergify mergify bot closed this as completed in #13332 Mar 2, 2021
mergify bot pushed a commit that referenced this issue Mar 2, 2021
…13332)

Fixes #13150 

> Interestingly, even though the Certificates field is of type Array, the documentation says: You can specify one certificate per resource.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Mar 2, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

cornerwings pushed a commit to cornerwings/aws-cdk that referenced this issue Mar 8, 2021
…ws#13332)

Fixes aws#13150 

> Interestingly, even though the Certificates field is of type Array, the documentation says: You can specify one certificate per resource.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-elasticloadbalancingv2 Related to Amazon Elastic Load Balancing V2 bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p1
Projects
None yet
3 participants