feat(elasticloadbalancingv2): support Mutual Authentication with TLS for Application Load Balancer #30784

mazyu36 · 2024-07-08T09:33:00Z

Issue # (if applicable)

Closes #28206.

Reason for this change

To support mTLS for ALB

Description of changes

Add TrustStore and TrustStoreRevocation class
Add MutualAuthentication property for ApplicationListener

Description of how you validated changes

add unit tests and integ tests

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

mazyu36 · 2024-07-08T09:45:20Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

+
+  constructor(scope: Construct, id: string, props: TrustStoreProps) {
+    super(scope, id, {
+      physicalName: props.trustStoreName ?? Lazy.string({


In CFN document, name property is optional.

However, deploying without setting it resulted in an error, so I made it generate a name instead.

mazyu36 · 2024-07-08T09:46:50Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

+  /**
+   * The bucket that the trust store is hosted in
+   */
+  readonly bucket: IBucket;


In CFn document, this property is optional.
However, deploying without setting it resulted in an error, so I made it required.

Also, the documentation states the following:

You must specify CaCertificatesBundleS3Bucket and CaCertificatesBundleS3Key.

mazyu36 · 2024-07-08T09:47:02Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

+  /**
+   * The key in S3 to look at for the trust store
+   */
+  readonly key: string;


In CFn document, this property is optional.
However, deploying without setting it resulted in an error, so I made it required.

Also, the documentation states the following:

You must specify CaCertificatesBundleS3Bucket and CaCertificatesBundleS3Key.

mazyu36 · 2024-07-08T09:48:32Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store-revocation.ts

+  /**
+   * The Amazon S3 bucket for the revocation file
+   */
+  readonly bucket: IBucket;


In CFn document, this property is optional.
However, deploying without setting it resulted in an error, so I made it required.

Also, the documentation states the following:

You must specify S3Bucket and S3Key.

mazyu36 · 2024-07-08T09:48:38Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store-revocation.ts

+  /**
+   * The Amazon S3 path for the revocation file
+   */
+  readonly key: string;


In CFn document, this property is optional.
However, deploying without setting it resulted in an error, so I made it required.

Also, the documentation states the following:

You must specify S3Bucket and S3Key.

mazyu36 · 2024-07-08T09:50:45Z

packages/aws-cdk-lib/awslint.json

@@ -115,6 +115,7 @@
    "props-physical-name:aws-cdk-lib.aws_elasticloadbalancing.LoadBalancerProps",
    "props-physical-name:aws-cdk-lib.aws_elasticloadbalancingv2.ApplicationListenerProps",
    "props-physical-name:aws-cdk-lib.aws_elasticloadbalancingv2.NetworkListenerProps",
+    "props-physical-name:aws-cdk-lib.aws_elasticloadbalancingv2.TrustStoreRevocationProps",


AWS::ElasticLoadBalancingV2::TrustStoreRevocation doesn't have name property, so I've added.

mazyu36 · 2024-07-08T15:47:02Z

packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/mtls/crl.pem

@@ -0,0 +1 @@
+dummy


I thought it wasn't good to store the pem file, but I decided to place a dummy file because the build fails if the file doesn't exist. When actually running the integration test, you need to create the pem file using a command.
If there's a better way, please let me know.

mazyu36 · 2024-07-08T15:47:28Z

...s/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/mtls/rootCA_cert.pem

@@ -0,0 +1 @@
+dummy


I thought it wasn't good to store the pem file, but I decided to place a dummy file because the build fails if the file doesn't exist. When actually running the integration test, you need to create the pem file using a command.
If there's a better way, please let me know.

lpizzinidev

Nice 👍 Left comments for minor adjustments

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

lpizzinidev · 2024-07-30T13:18:52Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

+      }),
+    });
+
+    const resource = new CfnTrustStore(this, 'Resource', {


Can you please add validation for props.trustStoreName?

Thanks. I've added.

lpizzinidev · 2024-07-30T13:24:16Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

+/**
+ * The client certificate handling method
+ */
+export enum Mode {


Suggested change

export enum Mode {

export enum MutualAuthenticationMode {

More meaningful?

Thanks. I've updated.

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md

Co-authored-by: Luca Pizzini <[email protected]>

…store.ts Co-authored-by: Luca Pizzini <[email protected]>

mazyu36 · 2024-08-02T02:44:02Z

@Leo10Gama
Thank you for the review. I've addressed all your comments.

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/listener.test.ts

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/listener.test.ts

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-store.ts

xazhao · 2024-08-02T20:44:25Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

+    if (props.mutualAuthentication) {
+      const currentMode = props.mutualAuthentication.mutualAuthenticationMode;
+
+      if (currentMode === MutualAuthenticationMode.VERIFY && !props.mutualAuthentication.trustStore) {


I think these two if blocks can be merged with something like this.

if props.mutualAuthentication.trustStore if currentMode !== MutualAuthenticationMode.VERIFY throw new Error ('trustStore can only be set when the mode is verify'

Thank you. I've extracted the validation into a separate function to make it cleaner.
What do you think of this approach?

https://github.com/aws/aws-cdk/pull/30784/files#diff-7bb91966b981c92a62f92075cd5bc4de58634ddcbe15dd492b0466f24b49383cR1041

Leo10Gama

Thanks for the quick response and changes! Just a few nits and typo fixes, but otherwise LGTM

…ner.test.ts Co-authored-by: Leonardo Gama <[email protected]>

Pull request has been modified.

Co-authored-by: Leonardo Gama <[email protected]>

…store.ts Co-authored-by: Leonardo Gama <[email protected]>

mazyu36 · 2024-08-03T09:17:02Z

@Leo10Gama
Thanks.
I’ve addressed all comments.

Leo10Gama

LGTM, thanks for your contribution!

mergify · 2024-08-06T17:11:35Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-08-06T17:43:10Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 366f8a8
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-08-06T17:43:32Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-08-06T17:43:48Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

aws-cdk-automation requested a review from a team July 8, 2024 09:33

github-actions bot added star-contributor [Pilot] contributed between 25-49 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Jul 8, 2024

mazyu36 commented Jul 8, 2024

View reviewed changes

mazyu36 force-pushed the alb-mtls-28206 branch 5 times, most recently from 6dde3d4 to 0bbe528 Compare July 8, 2024 12:58

feat: alb mtls

98bba74

mazyu36 force-pushed the alb-mtls-28206 branch from 0bbe528 to 98bba74 Compare July 8, 2024 13:31

mazyu36 marked this pull request as ready for review July 8, 2024 13:50

mazyu36 added 3 commits July 8, 2024 22:50

Merge branch 'main' into alb-mtls-28206

6b46d25

fix

71f2bbf

fix README.md

5da43b4

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jul 8, 2024

mazyu36 commented Jul 8, 2024

View reviewed changes

github-actions bot mentioned this pull request Jul 15, 2024

Weekly PR metrics report #30858

Closed

lpizzinidev suggested changes Jul 30, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jul 30, 2024

mazyu36 and others added 2 commits July 30, 2024 23:56

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/README.md

b0d5e4d

Co-authored-by: Luca Pizzini <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-…

d9c3852

…store.ts Co-authored-by: Luca Pizzini <[email protected]>

Leo10Gama reviewed Aug 2, 2024

View reviewed changes

xazhao reviewed Aug 2, 2024

View reviewed changes

Leo10Gama previously requested changes Aug 2, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 2, 2024

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/liste…

8fe139c

…ner.test.ts Co-authored-by: Leonardo Gama <[email protected]>

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 3, 2024

mazyu36 and others added 10 commits August 3, 2024 10:52

Update listener.test.ts

c395f73

Co-authored-by: Leonardo Gama <[email protected]>

Update listener.test.ts

ae81045

Co-authored-by: Leonardo Gama <[email protected]>

Update application-listener.ts

9453cfc

Co-authored-by: Leonardo Gama <[email protected]>

Update trust-store-revocation.ts

e9e91a3

Co-authored-by: Leonardo Gama <[email protected]>

Update trust-store.ts

8907791

Co-authored-by: Leonardo Gama <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-…

b4cbedf

…store.ts Co-authored-by: Leonardo Gama <[email protected]>

Update packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/trust-…

4e4eaea

…store.ts Co-authored-by: Leonardo Gama <[email protected]>

fix unit tests

4c3d37d

add validation

0b52315

Merge branch 'main' into alb-mtls-28206

a2e994c

mazyu36 requested a review from Leo10Gama August 3, 2024 09:17

Leo10Gama approved these changes Aug 6, 2024

View reviewed changes

Merge branch 'main' into alb-mtls-28206

366f8a8

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Aug 6, 2024

mergify bot merged commit 7eae4d1 into aws:main Aug 6, 2024
12 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Aug 6, 2024

mazyu36 deleted the alb-mtls-28206 branch August 6, 2024 22:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticloadbalancingv2): support Mutual Authentication with TLS for Application Load Balancer #30784

feat(elasticloadbalancingv2): support Mutual Authentication with TLS for Application Load Balancer #30784

mazyu36 commented Jul 8, 2024

mazyu36 Jul 8, 2024

mazyu36 Jul 8, 2024 •

edited

Loading

mazyu36 Jul 8, 2024 •

edited

Loading

mazyu36 Jul 8, 2024

mazyu36 Jul 8, 2024

mazyu36 Jul 8, 2024

mazyu36 Jul 8, 2024 •

edited

Loading

mazyu36 Jul 8, 2024 •

edited

Loading

lpizzinidev left a comment

lpizzinidev Jul 30, 2024

mazyu36 Jul 30, 2024

lpizzinidev Jul 30, 2024

mazyu36 Jul 30, 2024

mazyu36 commented Aug 2, 2024

xazhao Aug 2, 2024

mazyu36 Aug 3, 2024

Leo10Gama left a comment

mazyu36 commented Aug 3, 2024

Leo10Gama left a comment

mergify bot commented Aug 6, 2024

aws-cdk-automation commented Aug 6, 2024

mergify bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

feat(elasticloadbalancingv2): support Mutual Authentication with TLS for Application Load Balancer #30784

feat(elasticloadbalancingv2): support Mutual Authentication with TLS for Application Load Balancer #30784

Conversation

mazyu36 commented Jul 8, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Choose a reason for hiding this comment

mazyu36 Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

mazyu36 Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazyu36 Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

mazyu36 Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mazyu36 commented Aug 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Leo10Gama left a comment

Choose a reason for hiding this comment

mazyu36 commented Aug 3, 2024

Leo10Gama left a comment

Choose a reason for hiding this comment

mergify bot commented Aug 6, 2024

aws-cdk-automation commented Aug 6, 2024

AWS CodeBuild CI Report

mergify bot commented Aug 6, 2024

github-actions bot commented Aug 6, 2024

mazyu36 Jul 8, 2024 •

edited

Loading

mazyu36 Jul 8, 2024 •

edited

Loading

mazyu36 Jul 8, 2024 •

edited

Loading

mazyu36 Jul 8, 2024 •

edited

Loading