fix: AWS API Gateway OpenAPI validators trigger AwsSolutions-APIG2 #1312
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dontirun
reviewed
May 23, 2023
| node, | ||
| node.getMetadata('aws:asset:path') | ||
| ); | ||
| console.log(assetOutdir, assetPath); |
There was a problem hiding this comment.
Need to remove the log statement
There was a problem hiding this comment.
Sorry, I must have forgotten to remove that. It should be fixed now.
dontirun
requested changes
May 23, 2023
Comment on lines
+42
to
+44
| const specFile = yamlparse( | ||
| readFileSync(assetOutdir + '/' + assetPath, 'utf-8') | ||
| ); |
There was a problem hiding this comment.
I think we should use the official OpenApi Library/Types for processing the spec
| @@ -27,6 +29,36 @@ export default Object.defineProperty( | |||
| } | |||
| } | |||
| } | |||
| if (node.bodyS3Location) { | |||
There was a problem hiding this comment.
This doesn't capture the case where a user uses the L1 CfnRestApi directly and uses the Body property
| const specFile = yamlparse( | ||
| readFileSync(assetOutdir + '/' + assetPath, 'utf-8') | ||
| ); | ||
| if ('x-amazon-apigateway-request-validators' in specFile) { |
There was a problem hiding this comment.
The docs mentions that this can be done on a per method basis as well instead of just globally.
Each method can also override the global config, that needs to be checked as well
|
@awsntheule Are you still working on this or can I close the PR? |
|
Closing due to inactivity |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1075
This fix has been implemented by checking if a
CfnRestApiresource has thebodyS3Locationproperty defined. The property identifies that an api is being created by importing an api from a YAML or JSON file (note that thebodyproperty that is used when creating an inline api definition or by adding methods later in code does not get defined when importing an api definition).If the
bodyS3Locationproperty is defined, the changes proposed here will construct the local path to the asset file that is created for the import so that it can verify the imported api definition uses request validation.An additional dependency was also added (the npm page can be found here). This is a YAML parser that is used to parse a provided api definition file and verify that it uses request validation. Typescript does not include a YAML parser, so this would need to be provided externally so that YAML files can be checked. It also will automatically parse JSON as well with no additional code required.
Adding this dependency could also be useful in the future for checking other services that may import files in a similar way.