feat(elasticloadbalancingv2): health check logs for ALB #36227

ren-yamanashi · 2025-11-27T14:31:34Z

This PR is similar to #30599

Reason for this change

ALB can output health check logs as well as access logs to the S3 bucket, but this is not yet supported by L2 Construct.
https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-health-check-logging.html

Description of changes

Since health check logs are not supported by NLB, but only by ALB, the logHealthCheckLogs method is added to the ApplicationLoadBalancer instead of the BaseLoadBalancer.

The needed BucketPolicy is described in the documentation only as follows.

{
  "Version":"2012-10-17",		 	 	 
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "Service": "logdelivery.elasticloadbalancing.amazonaws.com"
      },
      "Action": "s3:PutObject",
      "Resource": "arn:aws:s3:::amzn-s3-demo-bucket/prefix/AWSLogs/123456789012/*"
    }
  ]
}

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

aws-cdk-automation

(This review is outdated)

ren-yamanashi · 2025-11-28T14:36:13Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

+    bucket.addToResourcePolicy(
+      new PolicyStatement({
+        actions: ['s3:PutObject'],
+        principals: [this.resourcePolicyPrincipal()],


This code is followed logConnectionLogs method.

see: https://github.com/aws/aws-cdk/blob/v2.230.0/packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts#L368

The key point when use resourcePolicyPrincipal method, this public method (logHealthCheckLogs) needs specified region on the stack.

But, based on the documents I think we can choice don't use resourcePolicyPrincipal method like here code.
(see: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-health-check-logging.html)

principals: [new iam.ServicePrincipal('logdelivery.elasticloadbalancing.amazonaws.com')],

And when we choice it, not needs specified region on the stack.

Q. Which I choice? Use resourcePolicyPrincipal method? or not use?

Current situation is not same when implementation logConnectionLogs method, so I want advice for you.

ren-yamanashi · 2025-11-28T16:16:24Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

+   * @see https://docs.aws.amazon.com/cdk/latest/guide/environments.html
+   */
+  @MethodMetadata()
+  public logHealthCheckLogs(bucket: s3.IBucket, prefix?: string) {


Based on documents (https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-health-check-logging.html) intuitively, you would think two validations are necessary.

region for specified bucket is same alb region

must use SSE-S3

However, I not implemented the validation logic "the region of the specified bucket is the same as the region of the ALB" for the following reasons.

Basically, S3 Buckets and ALB created within the same Stack are in the same region.

The case where they are not in the same region is when an imported S3 Bucket is specified. However, since the ARN for S3 does not include the region, we determined that it would be difficult to verify that they "exist in the same region."

Does IBucket class have a region information from IBucket.env.region? If it is true, I think we can add a validation for the same region restriction.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

…feat/alb-healthcheck-log

ren-yamanashi · 2025-11-29T04:35:35Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

+   * @see https://docs.aws.amazon.com/cdk/latest/guide/environments.html
+   */
+  @MethodMetadata()
+  public logHealthCheckLogs(bucket: s3.IBucket, prefix?: string) {


Similar methods to this one (e.g, logConnectionLogs) have the following code implemented, but this method doesn't.

// We still need this policy for the bucket using the ACL bucket.addToResourcePolicy( new PolicyStatement({ actions: ['s3:PutObject'], principals: [logsDeliveryServicePrincipal], resources: [ bucket.arnForObjects(`${prefix ? prefix + '/' : ''}AWSLogs/${Stack.of(this).account}/*`), ], conditions: { StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' }, }, }), ); bucket.addToResourcePolicy( new PolicyStatement({ actions: ['s3:GetBucketAcl'], principals: [logsDeliveryServicePrincipal], resources: [bucket.bucketArn], }), );

Because, if ACL are enabled on the S3 Bucket, health check logs can be logged even without that code.

I think this has been verified in packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.log.enable-acl.ts integ test, but please comment if verification method is incorrect.

I think this has been verified in packages/@aws-cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.log.enable-acl.ts integ test

If insufficient permissions, I think integ test will failed.

(In fact, I tried deleting the following permissions and running integ tests, deployment failed)

- bucket.addToResourcePolicy( - new PolicyStatement({ - actions: ['s3:PutObject'], - principals: [this.resourcePolicyPrincipal()], - resources: [ - bucket.arnForObjects( - `${prefix ? prefix + '/' : ''}AWSLogs/${Stack.of(this).account}/*`, - ), - ], - }), - );

Because the official docs does not mention about bucket policy relates to ACL, I think you don't need to add these policies.

badmintoncryer

Thank you for your contribution! I've added some comments.

badmintoncryer · 2025-12-02T03:45:03Z

...cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.log.enable-acl.ts

+
+const app = new cdk.App();
+const stack = new cdk.Stack(app, 'aws-cdk-elbv2-integ-enable-acl', { env: { region: 'us-west-2' } });
+stack.node.setContext(EC2_RESTRICT_DEFAULT_SECURITY_GROUP, false);


Is this feature flag is essential?

badmintoncryer · 2025-12-02T03:45:03Z

...cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.log.enable-acl.ts

+
+const app = new cdk.App();
+const stack = new cdk.Stack(app, 'aws-cdk-elbv2-integ-enable-acl', { env: { region: 'us-west-2' } });
+stack.node.setContext(EC2_RESTRICT_DEFAULT_SECURITY_GROUP, false);


Is this feature flag essential?

badmintoncryer · 2025-12-02T03:46:03Z

...cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.log.enable-acl.ts

+
+new IntegTest(app, 'cdk-integ-alb-log-enable-acl', {
+  testCases: [stack],
+  diffAssets: true,


Is this option needed?

badmintoncryer · 2025-12-02T03:48:01Z

...cdk-testing/framework-integ/test/aws-elasticloadbalancingv2/test/integ.alb.log.enable-acl.ts

+vpc.publicSubnets.forEach((subnet) => {
+  group1.node.addDependency(subnet);
+});


I'm curious that why is this dependency needed?

badmintoncryer · 2025-12-02T03:56:35Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

+   * @see https://docs.aws.amazon.com/cdk/latest/guide/environments.html
+   */
+  @MethodMetadata()
+  public logHealthCheckLogs(bucket: s3.IBucket, prefix?: string) {


Does IBucket class have a region information from IBucket.env.region? If it is true, I think we can add a validation for the same region restriction.

badmintoncryer · 2025-12-02T04:02:30Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

+     * See https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-health-check-logging.html
+     */
+    if (bucket.encryptionKey) {
+      throw new ValidationError('Encryption key detected. Bucket encryption using KMS keys is unsupported', this);


I think it might be better to add a description about when the bucket encryption using KMS keys is unsupported.

badmintoncryer · 2025-12-02T04:09:03Z

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts

+   * @see https://docs.aws.amazon.com/cdk/latest/guide/environments.html
+   */
+  @MethodMetadata()
+  public logHealthCheckLogs(bucket: s3.IBucket, prefix?: string) {


Because the official docs does not mention about bucket policy relates to ACL, I think you don't need to add these policies.

feat(elasticloadbalancingv2): healthCheck logs for ALB

b2ff5fb

aws-cdk-automation requested a review from a team November 27, 2025 14:31

github-actions bot added p2 valued-contributor [Pilot] contributed between 6-12 PRs to the CDK labels Nov 27, 2025

ren-yamanashi changed the title ~~feat(elasticloadbalancingv2): healthCheck logs for ALB~~ feat(elasticloadbalancingv2): health check logs for ALB Nov 27, 2025

aws-cdk-automation previously requested changes Nov 27, 2025

View reviewed changes

pahud mentioned this pull request Nov 27, 2025

Failed CodeBuild CI - 24 PR(s) need attention pahud/aws-cdk#17

Open

ren_yamanashi added 3 commits November 28, 2025 22:47

test: add unit test

07c7645

test: add comment

412c6e7

test: update

03271c1

ren-yamanashi commented Nov 28, 2025

View reviewed changes

chore: update jsdoc

f0bccf7

ren-yamanashi commented Nov 28, 2025

View reviewed changes

ren_yamanashi and others added 4 commits November 29, 2025 01:21

docs: update README.md

8d35eaf

chore: rm changes

747a861

chore: rm changes

c778084

test: add integ test

756938f

ren-yamanashi added 2 commits November 29, 2025 11:50

test: add integ test

f95848b

Merge branch 'main' of https://github.com/ren-yamanashi/aws-cdk into …

6498fa7

…feat/alb-healthcheck-log

ren-yamanashi commented Nov 29, 2025

View reviewed changes

test: add integ test

ed1f307

ren-yamanashi marked this pull request as ready for review November 29, 2025 05:15

test: update integ test assets

645eafc

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

github-actions bot mentioned this pull request Dec 1, 2025

Monthly PR metrics report #36251

Open

badmintoncryer suggested changes Dec 2, 2025

View reviewed changes

aws-cdk-automation removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Dec 2, 2025

feat(elasticloadbalancingv2): health check logs for ALB #36227

Are you sure you want to change the base?

feat(elasticloadbalancingv2): health check logs for ALB #36227

Conversation

ren-yamanashi commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reason for this change

Description of changes

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ren-yamanashi Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ren-yamanashi Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ren-yamanashi Nov 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

badmintoncryer left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ren-yamanashi commented Nov 27, 2025 •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading

ren-yamanashi Nov 28, 2025 •

edited

Loading

ren-yamanashi Nov 28, 2025 •

edited

Loading

ren-yamanashi Nov 29, 2025 •

edited

Loading