feat(elasticloadbalancingv2): connection logs for ALB #30599

sakurai-ryo · 2024-06-20T15:24:50Z

Reason for this change

ALB can output connection 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/load-balancer-connection-logs.html

Description of changes

The implementation is almost the same as for access logs.
However, since connection logs are not supported by NLB, but only by ALB, the logConnectionLogs method is added to the ApplicationLoadBalancer instead of the BaseLoadBalancer.

The needed BucketPolicy is described in the documentation only as follows, but to support buckets that still use the ACL, it is necessary to set the same policy that is currently set in the access logs.

{
  "Version": "2012-10-17",
  "Statement": [
    {
      "Effect": "Allow",
      "Principal": {
        "AWS": "arn:aws:iam::elb-account-id:root"
      },
      "Action": "s3:PutObject",
       "Resource": "arn:aws:s3:::bucket-name/prefix/AWSLogs/aws-account-id/*"
    }
  ]
}

https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-connection-logging.html#attach-bucket-policy-connection

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

sakurai-ryo · 2024-06-20T15:30:43Z

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

+    if (bucket.encryptionKey) {
+      throw new Error('Encryption key detected. Bucket encryption using KMS keys is unsupported');
+    }
+    super.logAccessLogs(bucket, prefix);


This PR adds a method with the same content as BaseLoadBalancer.logAccessLogs to ApplicationLoadBalancer to perform validation for ALB only.
However, modifying both methods would be necessary when changing the access log settings, so I changed it to call the base class implementation except for the validation.
#29382

Isn't this change fundamentally not related to this PR for the connection logs? If so, it would be better to create a separated PR.

If not, please let me know.

go-to-k

Thanks for this PR. I just made a minor comment.

go-to-k · 2024-07-03T13:02:36Z

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

+    if (bucket.encryptionKey) {
+      throw new Error('Encryption key detected. Bucket encryption using KMS keys is unsupported');
+    }
+    super.logAccessLogs(bucket, prefix);


Isn't this change fundamentally not related to this PR for the connection logs? If so, it would be better to create a separated PR.

If not, please let me know.

go-to-k · 2024-07-03T13:42:40Z

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

+  public logConnectionLogs(bucket: s3.IBucket, prefix?: string) {
+    /**
+    * KMS key encryption is not supported on Connection Log bucket for ALB, the bucket must use Amazon S3-managed keys (SSE-S3).
+    * See https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html#bucket-permissions-troubleshooting


The link for the troubleshooting section is for the access logs not the connection logs.

I think the correct link is here.

…b-connection-log

sakurai-ryo · 2024-07-08T14:49:18Z

@go-to-k
Thanks for the review!
I fixed the wrong link and deleted unrelated changes for the connection logs feat.

go-to-k · 2024-07-08T14:58:25Z

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

+   * Enable connection logging for this load balancer.
+   *
+   * A region must be specified on the stack containing the load balancer; you cannot enable logging on
+   * environment-agnostic stacks. See https://docs.aws.amazon.com/cdk/latest/guide/environments.html


How about using the @see label?

@go-to-k
Yes, add the @see label.

go-to-k

Thanks for the changes.

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

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

...nteg.alb.log.imported-bucket.js.snapshot/aws-cdk-alb-log-imported-bucket-integ.template.json

…b-connection-log

sakurai-ryo · 2024-07-31T13:46:47Z

@shikha372
Thanks for the review!
I've made the changes based on your feedback

shikha372 · 2024-08-09T18:51:58Z

Thank you for your contribution @sakurai-ryo !!

mergify · 2024-08-09T18:52:52Z

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-09T19:24:12Z

AWS CodeBuild CI Report

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

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

mergify · 2024-08-09T19:24:34Z

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-09T19:24:50Z

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.

ryo_sakurai and others added 2 commits June 20, 2024 20:22

feat: alb connection log

0802c5a

run integ tests

50caddb

github-actions bot added the star-contributor [Pilot] contributed between 25-49 PRs to the CDK label Jun 20, 2024

aws-cdk-automation requested a review from a team June 20, 2024 15:25

github-actions bot added the p2 label Jun 20, 2024

sakurai-ryo commented Jun 20, 2024

View reviewed changes

sakurai-ryo changed the title ~~feat(elasticloadbalancingv2): setting up connection logs for ALB~~ feat(elasticloadbalancingv2): connection logs for ALB Jun 20, 2024

comment

9a7d3fd

sakurai-ryo changed the title ~~feat(elasticloadbalancingv2): connection logs for ALB~~ feat(elasticloadbalancingv2): connection logging for ALB Jun 20, 2024

sakurai-ryo changed the title ~~feat(elasticloadbalancingv2): connection logging for ALB~~ feat(elasticloadbalancingv2): connection logs for ALB Jun 20, 2024

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

This was referenced Jun 24, 2024

Weekly PR metrics report #30637

Closed

Monthly PR metrics report #30715

Closed

go-to-k suggested changes Jul 3, 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 3, 2024

go-to-k reviewed Jul 3, 2024

View reviewed changes

sakurai-ryo added 3 commits July 8, 2024 21:52

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into al…

6ca5b0e

…b-connection-log

fix

93b9583

fix link

09fbf83

go-to-k reviewed Jul 8, 2024

View reviewed changes

fix: inline doc

6f66717

go-to-k approved these changes Jul 8, 2024

View reviewed changes

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

shikha372 self-assigned this Jul 12, 2024

shikha372 removed their assignment Jul 23, 2024

shikha372 reviewed Jul 23, 2024

View reviewed changes

packages/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-load-balancer.ts Outdated Show resolved Hide resolved

shikha372 reviewed Jul 23, 2024

View reviewed changes

packages/aws-cdk-lib/aws-elasticloadbalancingv2/test/alb/load-balancer.test.ts Show resolved Hide resolved

shikha372 reviewed Jul 23, 2024

View reviewed changes

...nteg.alb.log.imported-bucket.js.snapshot/aws-cdk-alb-log-imported-bucket-integ.template.json Show resolved Hide resolved

shikha372 self-assigned this Jul 23, 2024

sakurai-ryo added 2 commits July 30, 2024 21:04

Merge branch 'main' of https://github.com/sakurai-ryo/aws-cdk into al…

42f844e

…b-connection-log

fix: fix for the review

11a433a

shikha372 approved these changes Aug 9, 2024

View reviewed changes

Merge branch 'main' into alb-connection-log

800df62

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

mergify bot merged commit 7c4f423 into aws:main Aug 9, 2024
12 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(elasticloadbalancingv2): connection logs for ALB #30599

feat(elasticloadbalancingv2): connection logs for ALB #30599

sakurai-ryo commented Jun 20, 2024

sakurai-ryo Jun 20, 2024

go-to-k Jul 3, 2024 •

edited

Loading

go-to-k left a comment

go-to-k Jul 3, 2024 •

edited

Loading

go-to-k Jul 3, 2024 •

edited

Loading

sakurai-ryo commented Jul 8, 2024

go-to-k Jul 8, 2024

sakurai-ryo Jul 8, 2024

go-to-k left a comment

sakurai-ryo commented Jul 31, 2024

shikha372 commented Aug 9, 2024

mergify bot commented Aug 9, 2024

aws-cdk-automation commented Aug 9, 2024

mergify bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024

feat(elasticloadbalancingv2): connection logs for ALB #30599

feat(elasticloadbalancingv2): connection logs for ALB #30599

Conversation

sakurai-ryo commented Jun 20, 2024

Reason for this change

Description of changes

Description of how you validated changes

Checklist

sakurai-ryo Jun 20, 2024

Choose a reason for hiding this comment

go-to-k Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

go-to-k Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

go-to-k Jul 3, 2024 • edited Loading

Choose a reason for hiding this comment

sakurai-ryo commented Jul 8, 2024

go-to-k Jul 8, 2024

Choose a reason for hiding this comment

sakurai-ryo Jul 8, 2024

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

sakurai-ryo commented Jul 31, 2024

shikha372 commented Aug 9, 2024

mergify bot commented Aug 9, 2024

aws-cdk-automation commented Aug 9, 2024

AWS CodeBuild CI Report

mergify bot commented Aug 9, 2024

github-actions bot commented Aug 9, 2024

go-to-k Jul 3, 2024 •

edited

Loading

go-to-k Jul 3, 2024 •

edited

Loading

go-to-k Jul 3, 2024 •

edited

Loading