-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Add Lambda proxy functionality for API Gateway #2185
Changes from all commits
3b1eec6
090aa52
0ea8ab7
2ebfd90
2c9c6d2
9653775
0e9ad41
8ba191a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ module.exports = { | |
let method; | ||
let path; | ||
let requestPassThroughBehavior = 'NEVER'; | ||
let integrationType = 'AWS_PROXY'; | ||
|
||
if (typeof event.http === 'object') { | ||
method = event.http.method; | ||
|
@@ -342,6 +343,49 @@ module.exports = { | |
const normalizedFunctionName = functionName[0].toUpperCase() | ||
+ functionName.substr(1); | ||
|
||
// check if LAMBDA or LAMBDA-PROXY was used for the integration type | ||
if (typeof event.http === 'object') { | ||
if (Boolean(event.http.integration) === true) { | ||
// normalize the integration for further processing | ||
const normalizedIntegration = event.http.integration.toUpperCase(); | ||
// check if the user has entered a non-valid integration | ||
const allowedIntegrations = [ | ||
'LAMBDA', 'LAMBDA-PROXY', | ||
]; | ||
if (allowedIntegrations.indexOf(normalizedIntegration) === -1) { | ||
const errorMessage = [ | ||
`Invalid APIG integration "${event.http.integration}"`, | ||
` in function "${functionName}".`, | ||
' Supported integrations are: lambda, lambda-proxy.', | ||
].join(''); | ||
throw new this.serverless.classes.Error(errorMessage); | ||
} | ||
// map the Serverless integration to the corresponding CloudFormation types | ||
// LAMBDA --> AWS | ||
// LAMBDA-PROXY --> AWS_PROXY | ||
if (normalizedIntegration === 'LAMBDA') { | ||
integrationType = 'AWS'; | ||
} else if (normalizedIntegration === 'LAMBDA-PROXY') { | ||
integrationType = 'AWS_PROXY'; | ||
} else { | ||
// default to AWS_PROXY (just in case...) | ||
integrationType = 'AWS_PROXY'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it seems that the official name is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mapping is used so that users are not confused. In the console AWS calls it /cc @flomotlik came up with this mapping which might be confusing when you come from CF but makes sense if you played around with it on the console / lambda in general. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They call it Lambda and Lambda_proxy in the UI https://www.dropbox.com/s/oi9gzlbtptwjkan/Screenshot%202016-09-22%2010.27.41.png?dl=0 https://www.dropbox.com/s/k884s3y31ph54s8/Screenshot%202016-09-22%2010.27.29.png?dl=0 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but the user never sees There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that's true, then I don't see why we call it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. aaah wait nvm 😄 ... I just looked at it again and noticed the normalize logic. Now it make sense. So the user can do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. It basically uppercases everything before performing the comparison. You can do integration: LAMBDA
integration: lambda
integration: LAMBDA-PROXY
integration: lambda-proxy or a mixture. I'm more in favor of uppercasing since AWS does the same but don't want to force the user to do this as we don't do this with the Methods either. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
this is great. can't wait for this! |
||
} | ||
} | ||
} | ||
|
||
// show a warning when request / response config is used with AWS_PROXY (LAMBDA-PROXY) | ||
if (integrationType === 'AWS_PROXY' && ( | ||
(!!event.http.request) || (!!event.http.response) | ||
)) { | ||
const warningMessage = [ | ||
'Warning! You\'re using the LAMBDA-PROXY in combination with request / response', | ||
` configuration in your function "${functionName}".`, | ||
' This configuration will be ignored during deployment.', | ||
].join(''); | ||
this.serverless.cli.log(warningMessage); | ||
} | ||
|
||
const methodTemplate = ` | ||
{ | ||
"Type" : "AWS::ApiGateway::Method", | ||
|
@@ -352,7 +396,7 @@ module.exports = { | |
"RequestParameters" : {}, | ||
"Integration" : { | ||
"IntegrationHttpMethod" : "POST", | ||
"Type" : "AWS", | ||
"Type" : "${integrationType}", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't Template Literals need to be wrapped in back-ticks? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The entire template is wrapped in back-ticks as if it's a complete JSON file, so this would work 😊 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good one 👍 (acutally I was thinking the same when implementing this). Turns out that the whole JSON String is wrapped in back-ticks (if you look a few lines above). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To tell you the truth I'm not a big fan of this approach though when working with objects. It's hard to debug if you're missing a comma or something (because it's treated as JSON), plus we're gonna parse anyway to merge it with the entire CF template. I hope we can switch all our templates to just plain JS objects unless there's a pressing reason not to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eahefnawy I've converted to "Objects" here (vs. Strings): #2262 agreed it's much better. |
||
"Uri" : { | ||
"Fn::Join": [ "", | ||
[ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!! 👍