Skip to content

Commit

Permalink
fix(apigateway): relax access log format check to allow either reques…
Browse files Browse the repository at this point in the history 
…tId or extendedRequestId (aws#22591)

APIGW's recommendation is either:

- Use `$context.extendedRequestId` only
- Use `$context.requestId` AND `$context.extendedRequestId`

In fact, API Gateway ControlPlane requires either `$context.requestId` and `$context.extendedRequestId`.

CDK is following suit with requiring either one, not just `requestId`.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@kaizencc @mrgrain
kaizencc authored and mrgrain committed Oct 24, 2022
1 parent 7cbd1bf commit 9c4a3c8
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 53 deletions.
14 changes: 7 additions & 7 deletions packages/@aws-cdk/aws-apigateway/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -1159,8 +1159,8 @@ new apigateway.Stage(this, 'dev', {
resourcePath: true,
responseLength: true,
status: true,
user: true
})
user: true,
}),
});
```

Expand Down
8 changes: 5 additions & 3 deletions packages/@aws-cdk/aws-apigateway/lib/stage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -387,9 +389,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');
Expand Down
173 changes: 130 additions & 43 deletions packages/@aws-cdk/aws-apigateway/test/stage.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 `contextRequestIdXxx`', () => {
// 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({
requestIdXxx: '$context.requestIdXxx',
}));

// 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', () => {
Expand Down

0 comments on commit 9c4a3c8

Please sign in to comment.