Skip to content

Commit 2cc2449

Browse files
shikha372shikha372
authored
fix: add validation for ALB access log bucket when KMS key is provided (#29382)
### Issue # (if applicable) Closes #22031. ### Reason for this change Adds a validation with correct error indicating ALB Access log bucket does not support KMS encryption ### Description of changes Currently access logs bucket encryption with KMS is not supported in case of ALB but while deploying it throws an error indicating the failure with bucket permissions. This validation introduces an upfront check to throw an error if `bucket.encryptionKey `is defined. Documentation: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html ### Description of how you validated changes Added unit tests for validation. ### Checklist - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 9a34664 commit 2cc2449

File tree

4 files changed

+113
-12
lines changed

4 files changed

+113
-12
lines changed

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

+18
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,24 @@ const lb = new elbv2.ApplicationLoadBalancer(this, 'LB', {
234234

235235
For more information, see [Load balancer attributes](https://docs.aws.amazon.com/elasticloadbalancing/latest/application/application-load-balancers.html#load-balancer-attributes)
236236

237+
### Setting up Access Log Bucket on Application Load Balancer
238+
239+
The only server-side encryption option that's supported is Amazon S3-managed keys (SSE-S3). For more information
240+
Documentation: https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html
241+
242+
```ts
243+
244+
declare const vpc: ec2.Vpc;
245+
246+
const bucket = new s3.Bucket(this, 'ALBAccessLogsBucket',{
247+
encryption: s3.BucketEncryption.S3_MANAGED,
248+
});
249+
250+
const lb = new elbv2.ApplicationLoadBalancer(this, 'LB', { vpc });
251+
lb.logAccessLogs(bucket);
252+
253+
```
254+
237255
## Defining a Network Load Balancer
238256

239257
Network Load Balancers are defined in a similar way to Application Load

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

+64-1
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ import { ApplicationListener, BaseApplicationListenerProps } from './application
33
import { ListenerAction } from './application-listener-action';
44
import * as cloudwatch from '../../../aws-cloudwatch';
55
import * as ec2 from '../../../aws-ec2';
6+
import { PolicyStatement } from '../../../aws-iam/lib/policy-statement';
7+
import { ServicePrincipal } from '../../../aws-iam/lib/principals';
8+
import * as s3 from '../../../aws-s3';
69
import * as cxschema from '../../../cloud-assembly-schema';
7-
import { Duration, Lazy, Names, Resource } from '../../../core';
10+
import { CfnResource, Duration, Lazy, Names, Resource, Stack } from '../../../core';
811
import * as cxapi from '../../../cx-api';
912
import { ApplicationELBMetrics } from '../elasticloadbalancingv2-canned-metrics.generated';
1013
import { BaseLoadBalancer, BaseLoadBalancerLookupOptions, BaseLoadBalancerProps, ILoadBalancerV2 } from '../shared/base-load-balancer';
@@ -170,6 +173,66 @@ export class ApplicationLoadBalancer extends BaseLoadBalancer implements IApplic
170173
});
171174
}
172175

176+
/**
177+
* Enable access logging for this load balancer.
178+
*
179+
* A region must be specified on the stack containing the load balancer; you cannot enable logging on
180+
* environment-agnostic stacks. See https://docs.aws.amazon.com/cdk/latest/guide/environments.html
181+
*/
182+
public logAccessLogs(bucket: s3.IBucket, prefix?: string) {
183+
184+
/**
185+
* KMS key encryption is not supported on Access Log bucket for ALB, the bucket must use Amazon S3-managed keys (SSE-S3).
186+
* See https://docs.aws.amazon.com/elasticloadbalancing/latest/application/enable-access-logging.html#bucket-permissions-troubleshooting
187+
*/
188+
189+
if (bucket.encryptionKey) {
190+
throw new Error('Encryption key detected. Bucket encryption using KMS keys is unsupported');
191+
}
192+
193+
prefix = prefix || '';
194+
this.setAttribute('access_logs.s3.enabled', 'true');
195+
this.setAttribute('access_logs.s3.bucket', bucket.bucketName.toString());
196+
this.setAttribute('access_logs.s3.prefix', prefix);
197+
198+
const logsDeliveryServicePrincipal = new ServicePrincipal('delivery.logs.amazonaws.com');
199+
bucket.addToResourcePolicy(new PolicyStatement({
200+
actions: ['s3:PutObject'],
201+
principals: [this.resourcePolicyPrincipal()],
202+
resources: [
203+
bucket.arnForObjects(`${prefix ? prefix + '/' : ''}AWSLogs/${Stack.of(this).account}/*`),
204+
],
205+
}));
206+
bucket.addToResourcePolicy(
207+
new PolicyStatement({
208+
actions: ['s3:PutObject'],
209+
principals: [logsDeliveryServicePrincipal],
210+
resources: [
211+
bucket.arnForObjects(`${prefix ? prefix + '/' : ''}AWSLogs/${this.env.account}/*`),
212+
],
213+
conditions: {
214+
StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' },
215+
},
216+
}),
217+
);
218+
bucket.addToResourcePolicy(
219+
new PolicyStatement({
220+
actions: ['s3:GetBucketAcl'],
221+
principals: [logsDeliveryServicePrincipal],
222+
resources: [bucket.bucketArn],
223+
}),
224+
);
225+
226+
// make sure the bucket's policy is created before the ALB (see https://github.com/aws/aws-cdk/issues/1633)
227+
// at the L1 level to avoid creating a circular dependency (see https://github.com/aws/aws-cdk/issues/27528
228+
// and https://github.com/aws/aws-cdk/issues/27928)
229+
const lb = this.node.defaultChild;
230+
const bucketPolicy = bucket.policy?.node.defaultChild;
231+
if (lb && bucketPolicy && CfnResource.isCfnResource(lb) && CfnResource.isCfnResource(bucketPolicy)) {
232+
lb.addDependency(bucketPolicy);
233+
}
234+
}
235+
173236
/**
174237
* Add a security group to this load balancer
175238
*/

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

+30-11
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Construct } from 'constructs';
22
import { Match, Template } from '../../../assertions';
33
import { Metric } from '../../../aws-cloudwatch';
44
import * as ec2 from '../../../aws-ec2';
5+
import { Key } from '../../../aws-kms';
56
import * as s3 from '../../../aws-s3';
67
import * as cdk from '../../../core';
78
import * as elbv2 from '../../lib';
@@ -284,11 +285,16 @@ describe('tests', () => {
284285
}
285286
}
286287

287-
function loggingSetup(): { stack: cdk.Stack; bucket: s3.Bucket; lb: elbv2.ApplicationLoadBalancer } {
288+
function loggingSetup(withEncryption: boolean = false ): { stack: cdk.Stack; bucket: s3.Bucket; lb: elbv2.ApplicationLoadBalancer } {
288289
const app = new cdk.App();
289290
const stack = new cdk.Stack(app, undefined, { env: { region: 'us-east-1' } });
290291
const vpc = new ec2.Vpc(stack, 'Stack');
291-
const bucket = new s3.Bucket(stack, 'AccessLoggingBucket');
292+
let bucketProps = {};
293+
if (withEncryption) {
294+
const kmsKey = new Key(stack, 'TestKMSKey');
295+
bucketProps = { ...bucketProps, encryption: s3.BucketEncryption.KMS, encyptionKey: kmsKey };
296+
}
297+
const bucket = new s3.Bucket(stack, 'AccessLogBucket', { ...bucketProps });
292298
const lb = new elbv2.ApplicationLoadBalancer(stack, 'LB', { vpc });
293299
return { stack, bucket, lb };
294300
}
@@ -309,7 +315,7 @@ describe('tests', () => {
309315
},
310316
{
311317
Key: 'access_logs.s3.bucket',
312-
Value: { Ref: 'AccessLoggingBucketA6D88F29' },
318+
Value: { Ref: 'AccessLogBucketDA470295' },
313319
},
314320
{
315321
Key: 'access_logs.s3.prefix',
@@ -329,7 +335,7 @@ describe('tests', () => {
329335
// THEN
330336
// verify the ALB depends on the bucket policy
331337
Template.fromStack(stack).hasResource('AWS::ElasticLoadBalancingV2::LoadBalancer', {
332-
DependsOn: ['AccessLoggingBucketPolicy700D7CC6'],
338+
DependsOn: ['AccessLogBucketPolicyF52D2D01'],
333339
});
334340
});
335341

@@ -351,7 +357,7 @@ describe('tests', () => {
351357
Effect: 'Allow',
352358
Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::127311923021:root']] } },
353359
Resource: {
354-
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/AWSLogs/',
360+
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/AWSLogs/',
355361
{ Ref: 'AWS::AccountId' }, '/*']],
356362
},
357363
},
@@ -360,7 +366,7 @@ describe('tests', () => {
360366
Effect: 'Allow',
361367
Principal: { Service: 'delivery.logs.amazonaws.com' },
362368
Resource: {
363-
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/AWSLogs/',
369+
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/AWSLogs/',
364370
{ Ref: 'AWS::AccountId' }, '/*']],
365371
},
366372
Condition: { StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' } },
@@ -370,7 +376,7 @@ describe('tests', () => {
370376
Effect: 'Allow',
371377
Principal: { Service: 'delivery.logs.amazonaws.com' },
372378
Resource: {
373-
'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'],
379+
'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'],
374380
},
375381
},
376382
],
@@ -395,7 +401,7 @@ describe('tests', () => {
395401
},
396402
{
397403
Key: 'access_logs.s3.bucket',
398-
Value: { Ref: 'AccessLoggingBucketA6D88F29' },
404+
Value: { Ref: 'AccessLogBucketDA470295' },
399405
},
400406
{
401407
Key: 'access_logs.s3.prefix',
@@ -414,7 +420,7 @@ describe('tests', () => {
414420
Effect: 'Allow',
415421
Principal: { AWS: { 'Fn::Join': ['', ['arn:', { Ref: 'AWS::Partition' }, ':iam::127311923021:root']] } },
416422
Resource: {
417-
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
423+
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
418424
{ Ref: 'AWS::AccountId' }, '/*']],
419425
},
420426
},
@@ -423,7 +429,7 @@ describe('tests', () => {
423429
Effect: 'Allow',
424430
Principal: { Service: 'delivery.logs.amazonaws.com' },
425431
Resource: {
426-
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
432+
'Fn::Join': ['', [{ 'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'] }, '/prefix-of-access-logs/AWSLogs/',
427433
{ Ref: 'AWS::AccountId' }, '/*']],
428434
},
429435
Condition: { StringEquals: { 's3:x-amz-acl': 'bucket-owner-full-control' } },
@@ -433,14 +439,27 @@ describe('tests', () => {
433439
Effect: 'Allow',
434440
Principal: { Service: 'delivery.logs.amazonaws.com' },
435441
Resource: {
436-
'Fn::GetAtt': ['AccessLoggingBucketA6D88F29', 'Arn'],
442+
'Fn::GetAtt': ['AccessLogBucketDA470295', 'Arn'],
437443
},
438444
},
439445
],
440446
},
441447
});
442448
});
443449

450+
test('bucket with KMS throws validation error', () => {
451+
//GIVEN
452+
const { stack, bucket, lb } = loggingSetup(true);
453+
454+
// WHEN
455+
const logAccessLogFunctionTest = () => lb.logAccessLogs(bucket);
456+
457+
// THEN
458+
// verify failure in case the access log bucket is encrypted with KMS
459+
expect(logAccessLogFunctionTest).toThrow('Encryption key detected. Bucket encryption using KMS keys is unsupported');
460+
461+
});
462+
444463
test('access logging on imported bucket', () => {
445464
// GIVEN
446465
const { stack, lb } = loggingSetup();

‎packages/aws-cdk-lib/rosetta/aws_elasticloadbalancingv2/default.ts-fixture

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import * as elbv2 from 'aws-cdk-lib/aws-elasticloadbalancingv2';
55
import * as ec2 from 'aws-cdk-lib/aws-ec2';
66
import * as autoscaling from 'aws-cdk-lib/aws-autoscaling';
77
import * as cloudwatch from 'aws-cdk-lib/aws-cloudwatch';
8+
import * as s3 from 'aws-cdk-lib/aws-s3';
89

910
class Fixture extends Stack {
1011
constructor(scope: Construct, id: string) {

0 commit comments

Comments
 (0)