Add RequestParameters Support

[beck3905]

Issue #, if available:
#931

Description of changes:
Added support for RequestParameters in the Api event source properties

Description of how you validated changes:
unit tests
end to end tests
ran transform locally and ran created a stack from the transformed template

Checklist:

• Write/update tests
• make pr passes
• Update documentation

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[beck3905]

I'm not really sure what needs to be done to update documentation. I couldn't see in the repo how the documentation gets generated in order to update it.

[beck3905]

@praneetap @jlhood @keetonian It appears that the latest version of cfn-lint==0.22.0 is breaking 4 tests in my branch. These tests are also broken in the develop branch. I'm not sure if I should pin requirements/dev.txt to cfn-lint==0.21.6, which does pass, and I'm not sure how best to fix the failures as they are related to AWS::Cognito::UserPools present in tests templates that have nothing to do with the feature I added. Any suggestions?

[beck3905]

The build is failing due to this issue with cfn-lint v0.22.0 which is also impacting the develop branch: #1004. I've noticed several commits in the develop branch that are also failing tests due to this issue.

[beck3905]

@keetonian @praneetap @jlhood Looks like the build issue has been fixed. Any chance of getting the last review?

[beck3905]

@brettstack I believe I've addressed your comments. Would you please have a look?

Too many updates since the review, will re-review again.

[jlhood]

Apologies for the delayed response on this. I'm really excited about this feature! Thanks so much! I have some minor comments and one question around query vs querystring. I see in the conversation you said you've deployed these templates to CloudFormation and they worked as expected so just want to confirm you deployed a template with this specific case (query string param).

[beck3905]

@jlhood @praneetap I've made updates based on more review comments. Please have look.

[jlhood]

Caught a couple minor cleanups, but I'll go ahead and take care of them before merging. This is a super solid PR. Really appreciate all your great work on this!

[beck3905]

@keetonian @praneetap @jlhood Thank you for approving my PR and merging. I noticed that it is staged for release in v1.15.0. We are currently on v1.13.2, which suggests that it will be a while until this is released since we still have to go through v1.14.x. Is there any way to get this released faster? I know this is considered a new feature, however in my case the lack of the ability to cache parameters is more like a bug. That is why I submitted this PR.

For example, I have a function with the following path: /my-api/thing/{thing_id}. SAM automatically will add a parameter to the API swagger definition for thing_id as a path parameter, however it does not add it as a cache key parameter. This means that I get the following unexpected behavior from my API that has caching enabled on the stage:

GET request to /my-api/thing/thing1 returns thing1
GET request to /my-api/thing/thing2 returns thing1 when I expect thing2. thing1 is returned because it is in the cache and the thing_id path parameter is not part of the cache key.

This seems like basic functionality that is required if caching is enabled, otherwise the API will return incorrect resources from the cache. I had a similar example with the Authorization header where regardless of the token in the Authorization header if a resource was in the cache, a user could access it.

Thanks again and please advise.

[jlhood]

@beck3905 This change missed the cutoff for the v1.14.0 release so it got tagged for v1.15.0. Unfortunately, we are not able to share specific dates for releases, but v1.15.0 will be the next release we cut after v1.14.0, which is currently in progress. Patch releases like v1.13.1 and v1.13.2 were due to bugs found during the release where we needed to cherry-pick a specific bugfix related to the v1.13.0 release.

We're working on making our release process more transparent to external customers, but we're not quite where we want to be yet. For now, you can see when we cut release branches, which can give you some indication of when we begin working on a specific release.

[azarboon]

Adding RequestParameters to AWS::Serverless::Function, doesn't enforce it. Even though the query string is defined as required, but it's not enforced.

            RequestParameters: 
              - method.request.querystring.myString: 
                  Required: true

And in API GW console, under URL Query String Parameters, it shows this warning:

You have marked some query string parameter as required but thee request validator assigned to this method is not configured to validate parameters. To ensure that incoming HTTP requests include the required query string parameters, select an appropriate request validator.

This makes us to need to updated RequestParameters in template (in order to have updated Swagger specs) and to update Lambda business logic (to perform validation). So it's extra work and can lead to inconsistency.