Skip to content

Commit

Permalink
fix(elasticloadbalancingv2): upgrade to v1.92.0 drops certificates on…
Browse files Browse the repository at this point in the history
… ALB if more than 2 certificates exist (#13490)

Support for multiple certificates attached to a single ALB listener was
originally implemented by putting all certificates in an array on a single
`ListenerCertificate` resource. The docs state that only one certificate may be
specified, although multiple certificates do appear to work initially.  Initial
resource creation of a `ListenerCertificate` with multiple certificates appears
to succeed, but subsequent updates to this resource (to either add or remove
certificates) yields undefined and undesireable behavior.

The fix in #13332 attempted to fix this by creating a new `ListenerCertificate`
per certificate, and -- at my direction -- maintained partial backwards
compatibility by keeping the original ID for the first `ListenerCertificate`
resource. However, this has the effect of triggering an update to the existing
resource, which does not appear to work correctly.

By forcing a logical ID change for all `ListenerCertificate` resources, we can
force all existing resources to be deleted, and new resources created. This
avoids doing any updates on any `ListenerCertificate` resources with an array
of certificates, which appears to side-step the undefined behavior.

fixes #13437


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
njlynch authored Mar 9, 2021
1 parent b1449a1 commit 01b94f8
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,7 @@ export class ApplicationListener extends BaseListener implements IApplicationLis
// Only one certificate can be specified per resource, even though
// `certificates` is of type Array
for (let i = 0; i < additionalCerts.length; i++) {
// ids should look like: `id`, `id2`, `id3` (for backwards-compatibility)
const certId = (i > 0) ? `${id}${i + 1}` : id;
new ApplicationListenerCertificate(this, certId, {
new ApplicationListenerCertificate(this, `${id}${i + 1}`, {
listener: this,
certificates: [additionalCerts[i]],
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ describe('tests', () => {
],
});

expect(listener.node.tryFindChild('DefaultCertificates')).toBeDefined();
expect(listener.node.tryFindChild('DefaultCertificates1')).toBeDefined();
expect(listener.node.tryFindChild('DefaultCertificates2')).toBeDefined();
expect(listener.node.tryFindChild('DefaultCertificates3')).not.toBeDefined();

Expand Down

0 comments on commit 01b94f8

Please sign in to comment.