Skip to content
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

[api-gateway] Access log must include at least AccessLogFormat.contextRequestId()when tokenized format is passed #9687

Closed
vad3x opened this issue Aug 13, 2020 · 6 comments · Fixed by #9769
Assignees
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@vad3x
Copy link

vad3x commented Aug 13, 2020

I'm trying to use Stage construct with format by SSM key (so my format is similar to '${Token[TOKEN.65]}').

    this.stage = new apigateway.Stage(this, 'Stage', {
      ...
      accessLogFormat: apigateway.AccessLogFormat.custom(props.apiGatewayAcessLogFormatSsmKey),
      accessLogDestination: new apigateway.LogGroupLogDestination(this.apiGatewayLogGroup),
      ...
    });

Reproduction Steps

What did you expect to happen?

No error.

What actually happened?

Error: "Access log must include at least AccessLogFormat.contextRequestId()"

https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-apigateway/lib/stage.ts#L213

Environment

  • CLI Version : 1.57
  • Framework Version:1.57
  • Node.js Version: v12.14.0
  • OS : macOS 10.15.4
  • Language (Version): TypeScript (3.9.7)

This is 🐛 Bug Report

@vad3x vad3x added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 13, 2020
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Aug 13, 2020
@nija-at
Copy link
Contributor

nija-at commented Aug 17, 2020

@vad3x -

As called out in the error message, the string value passed to AccessLogFormat.custom() must contain the context request id.

Update your string to include this.

accessLogFormat: apigateway.AccessLogFormat.custom(`${AccessLogFormat.contextRequestId()} ${props.apiGatewayAcessLogFormatSsmKey}`),

@nija-at nija-at added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Aug 17, 2020
@vad3x
Copy link
Author

vad3x commented Aug 17, 2020

@nija-at Thank you for the workaround.

I guess your proposal will omit the error, but this will change the expected log format provided by the external output (SSM in my case).

@nija-at
Copy link
Contributor

nija-at commented Aug 17, 2020

@vad3x
Copy link
Author

vad3x commented Aug 17, 2020

Correct, but I'm not sure why you do the check on CDK sync level, I assume CF will fail the resource creating in case when $context.requestId is not on the format.
But my format is not a string, it's a reference:

Expected CDK output:

...
          "Format": {
            "Fn::ImportValue": "stack-name:ExportsOutputRefSsmParameter......"
          }
...

This is working CF output because apiGatewayAcessLogFormatSsmKey output value already contains $context.requestId.

The check which you have just make the scenario not possible to implement.

@nija-at
Copy link
Contributor

nija-at commented Aug 17, 2020

Ah ok, I see what the problem is. We need to exclude the validation if the string is a token.

@vad3x
Copy link
Author

vad3x commented Aug 17, 2020

Looks like that 👍

@nija-at nija-at removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Aug 17, 2020
@nija-at nija-at added effort/small Small work item – less than a day of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Aug 17, 2020
@mergify mergify bot closed this as completed in #9769 Aug 17, 2020
mergify bot pushed a commit that referenced this issue Aug 17, 2020
fixes #9687


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants