From 865efbcea3283779f436fca898eae91e0179a265 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Thu, 20 Oct 2022 17:10:37 -0400 Subject: [PATCH 1/4] fix(apigateway): relax access log format synth-time check to allow either requestId or extendedRequestId --- packages/@aws-cdk/aws-apigateway/lib/stage.ts | 4 +- .../aws-apigateway/test/stage.test.ts | 173 +++++++++++++----- 2 files changed, 132 insertions(+), 45 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/stage.ts b/packages/@aws-cdk/aws-apigateway/lib/stage.ts index 67d834b41f698..9df374bf20084 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/stage.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/stage.ts @@ -387,9 +387,9 @@ export class Stage extends StageBase { } else { if (accessLogFormat !== undefined && !Token.isUnresolved(accessLogFormat.toString()) && - !/.*\$context.requestId.*/.test(accessLogFormat.toString())) { + !/.*\$context.(requestId|extendedRequestId)\b.*/.test(accessLogFormat.toString())) { - throw new Error('Access log must include at least `AccessLogFormat.contextRequestId()`'); + throw new Error('Access log must include either `AccessLogFormat.contextRequestId()` or `AccessLogFormat.contextExtendedRequestId()`'); } if (accessLogFormat !== undefined && accessLogDestination === undefined) { throw new Error('Access log format is specified without a destination'); diff --git a/packages/@aws-cdk/aws-apigateway/test/stage.test.ts b/packages/@aws-cdk/aws-apigateway/test/stage.test.ts index 7bea84c67e9b7..c5c6ea53ceebf 100644 --- a/packages/@aws-cdk/aws-apigateway/test/stage.test.ts +++ b/packages/@aws-cdk/aws-apigateway/test/stage.test.ts @@ -343,59 +343,146 @@ describe('stage', () => { }); }); - test('fails when access log format does not contain `AccessLogFormat.contextRequestId()`', () => { - // GIVEN - const stack = new cdk.Stack(); - const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); - const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); - api.root.addMethod('GET'); + describe('access log check', () => { + test('fails when access log format does not contain `contextRequestId()` or `contextExtendedRequestId()', () => { + // GIVEN + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); - // WHEN - const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); - const testFormat = apigateway.AccessLogFormat.custom(''); + // WHEN + const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); + const testFormat = apigateway.AccessLogFormat.custom(''); - // THEN - expect(() => new apigateway.Stage(stack, 'my-stage', { - deployment, - accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), - accessLogFormat: testFormat, - })).toThrow(/Access log must include at least `AccessLogFormat.contextRequestId\(\)`/); - }); + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), + accessLogFormat: testFormat, + })).toThrow('Access log must include either `AccessLogFormat.contextRequestId()` or `AccessLogFormat.contextExtendedRequestId()`'); + }); + + test('succeeds when access log format contains `contextRequestId()`', () => { + // GIVEN + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); + + // WHEN + const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); + const testFormat = apigateway.AccessLogFormat.custom(JSON.stringify({ + requestId: apigateway.AccessLogField.contextRequestId(), + })); + + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), + accessLogFormat: testFormat, + })).not.toThrow(); + }); + + test('succeeds when access log format contains `contextExtendedRequestId()`', () => { + // GIVEN + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); + + // WHEN + const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); + const testFormat = apigateway.AccessLogFormat.custom(JSON.stringify({ + extendedRequestId: apigateway.AccessLogField.contextExtendedRequestId(), + })); + + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), + accessLogFormat: testFormat, + })).not.toThrow(); + }); + + test('succeeds when access log format contains both `contextRequestId()` and `contextExtendedRequestId`', () => { + // GIVEN + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); + + // WHEN + const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); + const testFormat = apigateway.AccessLogFormat.custom(JSON.stringify({ + requestId: apigateway.AccessLogField.contextRequestId(), + extendedRequestId: apigateway.AccessLogField.contextExtendedRequestId(), + })); + + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), + accessLogFormat: testFormat, + })).not.toThrow(); + }); + + test('fails when access log format contains `contextRequestIdWillBeByPassed()`', () => { + // GIVEN + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); + + // WHEN + const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); + const testFormat = apigateway.AccessLogFormat.custom(JSON.stringify({ + requestIdBypassed: '$context.requestIdBypassed', + })); + + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), + accessLogFormat: testFormat, + })).toThrow('Access log must include either `AccessLogFormat.contextRequestId()` or `AccessLogFormat.contextExtendedRequestId()`'); + }); - test('does not fail when access log format is a token', () => { + test('does not fail when access log format is a token', () => { // GIVEN - const stack = new cdk.Stack(); - const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); - const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); - api.root.addMethod('GET'); + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); - // WHEN - const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); - const testFormat = apigateway.AccessLogFormat.custom(cdk.Lazy.string({ produce: () => 'test' })); + // WHEN + const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); + const testFormat = apigateway.AccessLogFormat.custom(cdk.Lazy.string({ produce: () => 'test' })); - // THEN - expect(() => new apigateway.Stage(stack, 'my-stage', { - deployment, - accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), - accessLogFormat: testFormat, - })).not.toThrow(); - }); + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogDestination: new apigateway.LogGroupLogDestination(testLogGroup), + accessLogFormat: testFormat, + })).not.toThrow(); + }); - test('fails when access log destination is empty', () => { + test('fails when access log destination is empty', () => { // GIVEN - const stack = new cdk.Stack(); - const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); - const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); - api.root.addMethod('GET'); + const stack = new cdk.Stack(); + const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); + const deployment = new apigateway.Deployment(stack, 'my-deployment', { api }); + api.root.addMethod('GET'); - // WHEN - const testFormat = apigateway.AccessLogFormat.jsonWithStandardFields(); + // WHEN + const testFormat = apigateway.AccessLogFormat.jsonWithStandardFields(); - // THEN - expect(() => new apigateway.Stage(stack, 'my-stage', { - deployment, - accessLogFormat: testFormat, - })).toThrow(/Access log format is specified without a destination/); + // THEN + expect(() => new apigateway.Stage(stack, 'my-stage', { + deployment, + accessLogFormat: testFormat, + })).toThrow(/Access log format is specified without a destination/); + }); }); test('default throttling settings', () => { From 85f71e9d58c3f33a77072a0bd9d0ee5867de6c78 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Thu, 20 Oct 2022 17:14:22 -0400 Subject: [PATCH 2/4] minor edits --- packages/@aws-cdk/aws-apigateway/test/stage.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/test/stage.test.ts b/packages/@aws-cdk/aws-apigateway/test/stage.test.ts index c5c6ea53ceebf..41422403c165e 100644 --- a/packages/@aws-cdk/aws-apigateway/test/stage.test.ts +++ b/packages/@aws-cdk/aws-apigateway/test/stage.test.ts @@ -427,7 +427,7 @@ describe('stage', () => { })).not.toThrow(); }); - test('fails when access log format contains `contextRequestIdWillBeByPassed()`', () => { + test('fails when access log format contains `contextRequestIdXxx`', () => { // GIVEN const stack = new cdk.Stack(); const api = new apigateway.RestApi(stack, 'test-api', { cloudWatchRole: false, deploy: false }); @@ -437,7 +437,7 @@ describe('stage', () => { // WHEN const testLogGroup = new logs.LogGroup(stack, 'LogGroup'); const testFormat = apigateway.AccessLogFormat.custom(JSON.stringify({ - requestIdBypassed: '$context.requestIdBypassed', + requestIdXxx: '$context.requestIdXxx', })); // THEN From ae7c359bde588c0a64ba145c37ae94e06f255ebb Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Fri, 21 Oct 2022 09:10:18 -0400 Subject: [PATCH 3/4] Update stage.ts --- packages/@aws-cdk/aws-apigateway/lib/stage.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/stage.ts b/packages/@aws-cdk/aws-apigateway/lib/stage.ts index 9df374bf20084..df28bdc711256 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/stage.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/stage.ts @@ -48,7 +48,9 @@ export interface StageOptions extends MethodDeploymentOptions { /** * A single line format of access logs of data, as specified by selected $content variables. - * The format must include at least `AccessLogFormat.contextRequestId()`. + * The format must include either `AccessLogFormat.contextRequestId()` + * or `AccessLogFormat.contextExtendedRequestId()`. + * * @see https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-mapping-template-reference.html#context-variable-reference * * @default - Common Log Format From 5b1ab26b71f7e3b55b359e0add1a0271d8efd9b0 Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Fri, 21 Oct 2022 09:12:59 -0400 Subject: [PATCH 4/4] update readme --- packages/@aws-cdk/aws-apigateway/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/README.md b/packages/@aws-cdk/aws-apigateway/README.md index 5a596f5a6da11..f485d8a48eab3 100644 --- a/packages/@aws-cdk/aws-apigateway/README.md +++ b/packages/@aws-cdk/aws-apigateway/README.md @@ -1128,8 +1128,8 @@ Access logging creates logs every time an API method is accessed. Access logs ca who has accessed the API, how the caller accessed the API and what responses were generated. Access logs are configured on a Stage of the RestApi. Access logs can be expressed in a format of your choosing, and can contain any access details, with a -minimum that it must include the 'requestId'. The list of variables that can be expressed in the access -log can be found +minimum that it must include either 'requestId' or 'extendedRequestId'. The list of variables that +can be expressed in the access log can be found [here](https://docs.aws.amazon.com/apigateway/latest/developerguide/api-gateway-mapping-template-reference.html#context-variable-reference). Read more at [Setting Up CloudWatch API Logging in API Gateway](https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-logging.html) @@ -1140,9 +1140,9 @@ const prdLogGroup = new logs.LogGroup(this, "PrdLogs"); const api = new apigateway.RestApi(this, 'books', { deployOptions: { accessLogDestination: new apigateway.LogGroupLogDestination(prdLogGroup), - accessLogFormat: apigateway.AccessLogFormat.jsonWithStandardFields() - } -}) + accessLogFormat: apigateway.AccessLogFormat.jsonWithStandardFields(), + }, +}); const deployment = new apigateway.Deployment(this, 'Deployment', {api}); // development stage @@ -1159,8 +1159,8 @@ new apigateway.Stage(this, 'dev', { resourcePath: true, responseLength: true, status: true, - user: true - }) + user: true, + }), }); ```