Adding ApiKeySourceType.

[lee-11]

#1274

This PR attempts to make the changes needed to support the feature of API Gateway that allows a Lambda Custom Authorizer to take on the responsibility of identifying the API Key for authorized requests (versus having the client specify the key via the x-api-key header). This PR adds the optional string property ApiKeySourceType to the AWS:Serverless::Api and adds validation that it holds one of the two acceptable values: "HEADER" or "AUTHORIZER".

To validate this work (I'm not a Python developer, so this is a bit of a learning experience for me) I ran the tests as described in the development guide (i.e. make pr). I didn't see any failed tests and the code coverage remained over 95% (this is a very small change, so the impact was minimal), so the "optional" nature of the new property seems correct (backward compatible).

I also added a new test for the AUTHORIZER use case. The test makes use of correct value since the default is already tested in others.

Finally, I added an integration test to verify it all works as intended.

I haven't implemented intrinsic functions since I don't believe they are useful for enumerated values like this. That said, I'm happy to add that in if it makes sense.

Checklist:

• Add/update tests using:
  • Correct values
  • Bad/wrong values (None, empty, wrong type, length, etc.)
  • [n/a] Intrinsic Functions
• make pr passes
• Update documentation
• Verify transformed template deploys and application functions as expected

I have not added examples for this property since it is a fairly complicated topic to explain (requires an Api, Authorizer, Plan and Key at minimum). Also, it looks like the examples have been moved to another repo.

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

[codecov-commenter]

Codecov Report

Merging #2298 (0b07ad8) into develop (e7a1496) will increase coverage by 0.71%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #2298      +/-   ##
===========================================
+ Coverage    93.58%   94.29%   +0.71%     
===========================================
  Files           90       97       +7     
  Lines         6124     7119     +995     
  Branches      1260     1467     +207     
===========================================
+ Hits          5731     6713     +982     
- Misses         183      196      +13     
  Partials       210      210              

Impacted Files	Coverage Δ
samtranslator/__init__.py	100.00% <100.00%> (ø)
samtranslator/feature_toggle/dialup.py	100.00% <100.00%> (ø)
samtranslator/feature_toggle/feature_toggle.py	100.00% <100.00%> (+12.16%)	⬆️
samtranslator/intrinsics/actions.py	98.78% <100.00%> (+0.06%)	⬆️
samtranslator/intrinsics/resource_refs.py	95.83% <100.00%> (-0.17%)	⬇️
samtranslator/region_configuration.py	77.77% <0.00%> (-22.23%)	⬇️
samtranslator/validator/validator.py	91.80% <0.00%> (-3.85%)	⬇️
samtranslator/model/exceptions.py	97.67% <0.00%> (-2.33%)	⬇️
samtranslator/open_api/open_api.py	89.70% <0.00%> (-2.27%)	⬇️
samtranslator/model/s3_utils/uri_parser.py	68.42% <0.00%> (-0.81%)	⬇️
... and 43 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66f6d3b...0b07ad8. Read the comment docs.

[hawflau]

Thanks for your contributions.

There are some failed tests. Also, you should add the expected output under tests/translator/output/aws-us-gov and tests/translator/output/aws-cn/.

[hawflau]

@lee-11 I didn't notice the PR was still in draft. I also saw the issue discussing about the behavior of ApiKeySourceType getting reset. I guess you are still working on it. Please let us know if you need any support!

[lee-11]

Please let us know if you need any support!

Oh, yes. So far so good, but I'm winging it a bit here and could use some guidance. I'm not sure how to test a transformed template can actually be deployed. Is there a convenient way to transform a known good SAM template I have on hand?

[lee-11]

Is there documentation that should be updated with this PR? I've poked around the repo and don't see anything. Sorry for asking; I'm totally unfamiliar with both Python and this repo.

[hawflau]

Oh, yes. So far so good, but I'm winging it a bit here and could use some guidance. I'm not sure how to test a transformed template can actually be deployed. Is there a convenient way to transform a known good SAM template I have on hand?

One way is, for example, in the test template you provided, you can replace the dummy s3 bucket/key with actual s3 bucket/key that exists. Then you can deploy it in CloudFormation (via AWS CLI or in AWS Console).
Alternatively you can add an integration test case under the integration/ directory. Our integration tests are end-to-end tests which are supposed to be deployable when they run.

Is there documentation that should be updated with this PR? I've poked around the repo and don't see anything. Sorry for asking; I'm totally unfamiliar with both Python and this repo.

Development Guide
Integration Test

[NickHeap2]

@lee-11 Did you actually get an example to deploy with this change? I did the same type of changes and whilst I could get sam validate to work it failed during sam deploy.

[lee-11]

@lee-11 Did you actually get an example to deploy with this change? I did the same type of changes and whilst I could get sam validate to work it failed during sam deploy.

I did, yes. However my deploy wasn't through the sam cli, but using the bin/sam-translate.py tool. My usual toolset is to use the cloudformation API directly to create change sets and deploy (which isn't really an option here).

Can you provide the details of how you used the SAM cli to test this?

[lee-11]

@hawflau if I may ask, what's the process from here for an external PR? Anything needed from me or that I should be expecting as next steps?

[hawflau]

lee-11 Thanks for updating the PR and adding the integration test case. I tried to run it but encountered some failed validations. Could you please check and update accordingly? Thanks

[sawilde]

@lee-11 thanks for this - I have crashed into this issue recently and looked into the myriad of workarounds that seem to involve a lot of make-work, the reason we use AWS::Serverless::APi (all I can say is CORS/Swagger).

@hawflau @lee-11 what do we need to do to progress this PR forward?

[hawflau]

@lee-11 There's still some issues that should be fixed. Would you be able to continue working on this?

@sawilde We will continue to work on this if @lee-11 isn't able to.

[sawilde]

awesome work folks - in what release will we see this fix emerge?

[jfuss]

@sawilde We don't track specific releases, since things outside our control can happen (security patches, non public features, etc). We tend to not combine PRs with some of those processes so we don't accidentally affect others timelines.

With that said, we are taking about cutting a release in the coming weeks to get Http IAM auth and some other bug fixes merged out. This PR (more specifically anything in develop not yet released) would be all released together but have not timeline to share at this point.

[sawilde]

Thanks for the update, I'll keep on eye out.

Is there a releases thread I could follow?

[bitconym]

@sawilde This is already released:

• Pproperty ApiKeySourceType not defined for resource of type AWS::Serverless::Api #1274
• https://github.com/aws/serverless-application-model/releases/tag/v1.46.0

[sawilde]

@imjustd thanks for the update