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.

Sign up for GitHub

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

Jump to bottom

Add support for referenced custom authorizer lambdas #102

Merged
merged 6 commits into from
Mar 10, 2018
Merged

Add support for referenced custom authorizer lambdas #102

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f502a95
Add support for referenced custom authorizer lambdas
Feb 13, 2018
6c66070
Resolve linting issues
Feb 13, 2018
931e541
Fix intendation
Feb 13, 2018
696c24a
Seperate code path for referenced authorizers permissions
Feb 15, 2018
1764e1d
Add test for referenced authorizers
Feb 15, 2018
f27ceaf
Fix linting errors
Feb 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions lib/stackops/apiGateway.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
const authorizerType = _.get(authorizer, 'Properties.Type');
if (authorizerType === 'TOKEN' || authorizerType === 'REQUEST') {
const uriParts = authorizer.Properties.AuthorizerUri['Fn::Join'][1];
const funcIndex = _.findIndex(uriParts, part => _.has(part, 'Fn::GetAtt'));
const funcIndex = _.findIndex(uriParts, part =>
_.has(part, 'Fn::GetAtt') || _.startsWith(part, 'arn:aws:lambda'));

// Use the SERVERLESS_ALIAS stage variable to determine the called function alias
uriParts.splice(funcIndex + 1, 0, ':${stageVariables.SERVERLESS_ALIAS}');
Expand Down Expand Up @@ -267,7 +268,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, functionName));

// Adjust references and alias permissions
permission.Properties.FunctionName = { Ref: aliasName };
permission.Properties.FunctionName = aliasName ? { Ref: aliasName } : permission.Properties.FunctionName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this change is not an appropriate fix. It will affect all permission resources and unconditionally use any existing function name in case something is wrong with the alias.

Using the original function name must only be done if we are really sure that the target is an external function, and not a function deployed by the current service. If that happens for a local function this leads to untraceable errors and lets the permissions be attached to $LATEST instead of the alias silently.

We should first check if the permission targets a service external function and handle that in a separate code branch. This allows for handling configuration errors differently for internal functions and external ones instead of mixing them and swallowing errors.

The _.compact() change below is also affected. It must only be in place for external functions, not internal ones. If permissions that reference internal functions are not dependent on an alias, there is a problem with the service.

if (permission.Properties.SourceArn) {
// Authorizers do not set the SourceArn property
permission.Properties.SourceArn = {
Expand All @@ -287,7 +288,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
}

// Add dependency on function version
permission.DependsOn = [ versionName, aliasName ];
permission.DependsOn = _.compact([ versionName, aliasName ]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above.


delete stageStack.Resources[name];
});
Expand Down