-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(elasticloadbalancingv2): health check logs for ALB #36227
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
b2ff5fb
07c7645
412c6e7
03271c1
f0bccf7
8d35eaf
747a861
c778084
756938f
f95848b
6498fa7
ed1f307
645eafc
9055a51
db0fccd
961b907
974e516
364abd3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -401,6 +401,56 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Enable health check logging for this load balancer. | ||
| */ | ||
| @MethodMetadata() | ||
| public logHealthCheckLogs(bucket: s3.IBucket, prefix?: string) { | ||
ren-yamanashi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** | ||
| * The bucket must be located in the same Region as the load balancer | ||
| */ | ||
| if (Stack.of(this).region !== Stack.of(bucket).region) { | ||
| throw new ValidationError('Health Check Log bucket must be in the same region as the Application Load Balancer', this); | ||
| } | ||
|
|
||
| /** | ||
| * KMS key encryption is not supported on HealthCheck Log bucket for ALB, the bucket must use Amazon S3-managed keys (SSE-S3). | ||
| * 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); | ||
ren-yamanashi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| prefix = prefix || ''; | ||
| this.setAttribute('health_check_logs.s3.enabled', 'true'); | ||
| this.setAttribute( | ||
| 'health_check_logs.s3.bucket', | ||
| bucket.bucketName.toString(), | ||
| ); | ||
| this.setAttribute('health_check_logs.s3.prefix', prefix); | ||
|
|
||
| bucket.addToResourcePolicy( | ||
| new PolicyStatement({ | ||
| actions: ['s3:PutObject'], | ||
| principals: [this.resourcePolicyPrincipal()], | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code is followed The key point when use But, based on the documents I think we can choice don't use 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 Current situation is not same when implementation logConnectionLogs method, so I want advice for you.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What could you think about this?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally, I’d prefer to implement it using the ServicePrincipal method, but I think it’s better to align with the existing implementation… For now, let’s keep the current implementation as is, and could you discuss this with the maintainers again?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay. I follow you. |
||
| resources: [ | ||
| bucket.arnForObjects( | ||
| `${prefix ? prefix + '/' : ''}AWSLogs/${Stack.of(this).account}/*`, | ||
| ), | ||
| ], | ||
| }), | ||
| ); | ||
|
|
||
| // make sure the bucket's policy is created before the ALB (see https://github.com/aws/aws-cdk/issues/1633) | ||
| // at the L1 level to avoid creating a circular dependency (see https://github.com/aws/aws-cdk/issues/27528 | ||
| // and https://github.com/aws/aws-cdk/issues/27928) | ||
| const lb = this.node.defaultChild; | ||
| const bucketPolicy = bucket.policy?.node.defaultChild; | ||
| if (lb && bucketPolicy && CfnResource.isCfnResource(lb) && CfnResource.isCfnResource(bucketPolicy)) { | ||
| lb.addDependency(bucketPolicy); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Add a security group to this load balancer | ||
| */ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.