Skip to content

Commit

Permalink
Fix SNS for case when function names where function name A startwith …
Browse files Browse the repository at this point in the history
…function name B

- test stack updated with case that covers the issue
  • Loading branch information
Sergii Kovalev authored and Aleksander Dikanski committed May 10, 2019
1 parent 81f79f4 commit 6ac8882
Show file tree
Hide file tree
Showing 7 changed files with 265 additions and 22 deletions.
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
/node_modules
/coverage
/.nyc_output

# Jetbrains IDEs
.idea/
Expand Down
10 changes: 5 additions & 5 deletions lib/stackops/apiGateway.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
this.options.verbose && this._serverless.cli.log('Configuring stage');
const stageResource = internal.createStageResource.call(this, `${stackName}-ApiGatewayRestApi`, deploymentName);
aliasResources.push({ ApiGatewayStage: stageResource });

const baseMapping = _.assign({}, _.pickBy(stageStack.Resources, ['Type', 'AWS::ApiGateway::BasePathMapping']));
if (!_.isEmpty(baseMapping)) {
const baseMappingName = _.keys(baseMapping)[0];
Expand All @@ -198,7 +198,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
aliasResources.push(baseMapping);
delete stageStack.Resources[baseMappingName];
}
}
}

// Fetch lambda permissions, methods and resources. These have to be updated later to allow the aliased functions.
const apiLambdaPermissions =
Expand Down Expand Up @@ -280,7 +280,7 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
const functionName = _.replace(name, /LambdaPermissionApiGateway$/, '');
const versionName = _.find(_.keys(versions), version => _.startsWith(version, `${functionName}LambdaVersion`));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, `${functionName}Alias`));
const isExternalRef = isExternalRefAuthorizerPredicate(permission.Properties.FunctionName);
const isExternalRef = isExternalRefAuthorizerPredicate(permission.Properties.FunctionName);

// Adjust references and alias permissions
if (!isExternalRef) {
Expand All @@ -305,9 +305,9 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
]
};
}

// Add dependency on function version
if (!isExternalRef) {
if (!isExternalRef) {
permission.DependsOn = [ versionName, aliasName ];
} else {
permission.DependsOn = _.compact([ versionName, aliasName ]);
Expand Down
12 changes: 6 additions & 6 deletions lib/stackops/snsEvents.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
_.forOwn(lambdaSubscriptions, subscription => {
const functionNameRef = utils.findAllReferences(_.get(subscription, 'Endpoint'));
const functionName = _.replace(_.get(functionNameRef, '[0].ref', ''), /LambdaFunction$/, '');
const versionName = _.find(_.keys(versions), version => _.startsWith(version, functionName));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, functionName));
const versionName = _.find(_.keys(versions), version => _.startsWith(version, `${functionName}LambdaVersion`));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, `${functionName}Alias`));

subscription.Endpoint = { Ref: aliasName };

Expand All @@ -54,8 +54,8 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac

const functionNameRef = utils.findAllReferences(_.get(subscription.Properties, 'Endpoint'));
const functionName = _.replace(_.get(functionNameRef, '[0].ref', ''), /LambdaFunction$/, '');
const versionName = _.find(_.keys(versions), version => _.startsWith(version, functionName));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, functionName));
const versionName = _.find(_.keys(versions), version => _.startsWith(version, `${functionName}LambdaVersion`));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, `${functionName}Alias`));

subscription.Properties.Endpoint = { Ref: aliasName };
subscription.DependsOn = [ versionName, aliasName ];
Expand All @@ -72,8 +72,8 @@ module.exports = function(currentTemplate, aliasStackTemplates, currentAliasStac
// Adjust permission to reference the function aliases
_.forOwn(snsLambdaPermissions, (permission, name) => {
const functionName = _.replace(name, /LambdaPermission.*$/, '');
const versionName = _.find(_.keys(versions), version => _.startsWith(version, functionName));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, functionName));
const versionName = _.find(_.keys(versions), version => _.startsWith(version, `${functionName}LambdaVersion`));
const aliasName = _.find(_.keys(aliases), alias => _.startsWith(alias, `${functionName}Alias`));

// Adjust references and alias permissions
permission.Properties.FunctionName = { Ref: aliasName };
Expand Down
3 changes: 2 additions & 1 deletion lib/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ class Utils {

/**
* Checks if a CF resource permission targets the given service as Principal.
* @param {string} service
* @param {Object} permission
* @param {string} service
*/
static hasPermissionPrincipal(permission, service) {
const principal = _.get(permission, 'Properties.Principal');
Expand Down
64 changes: 64 additions & 0 deletions test/data/alias-stack-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,25 @@
"RetentionInDays": 7
}
},
"Testfct1WithSuffixAlias": {
"Type": "AWS::Lambda::Alias",
"Properties": {
"Description": "Echo function echoes alias",
"FunctionName": {
"Fn::ImportValue": "sls-test-project-dev-Testfct1WithSuffix-LambdaFunctionArn"
},
"FunctionVersion": {
"Fn::GetAtt": [
"Testfct1WithSuffixLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU",
"Version"
]
},
"Name": "myAlias"
},
"DependsOn": [
"Testfct1WithSuffixLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU"
]
},
"Testfct1Alias": {
"Type": "AWS::Lambda::Alias",
"Properties": {
Expand Down Expand Up @@ -47,6 +66,17 @@
"WarmUpPluginLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU"
]
},
"Testfct1WithSuffixLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU": {
"Type": "AWS::Lambda::Version",
"DeletionPolicy": "Delete",
"Properties": {
"FunctionName": {
"Fn::ImportValue": "sls-test-project-dev-Testfct1WithSuffix-LambdaFunctionArn"
},
"CodeSha256": "Wh5jTkiTR67+V05RPWQIlzPI25WiPbdHDYNgbtAMneU=",
"Description": "Echo function echoes alias"
}
},
"Testfct1LambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU": {
"Type": "AWS::Lambda::Version",
"DeletionPolicy": "Delete",
Expand Down Expand Up @@ -97,6 +127,40 @@
"ApiGatewayDeployment1494367071211"
]
},
"Testfct1WithSuffixLambdaPermissionApiGateway": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"FunctionName": {
"Ref": "Testfct1WithSuffixAlias"
},
"Action": "lambda:InvokeFunction",
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Join": [
"",
[
"arn:aws:execute-api:",
{
"Ref": "AWS::Region"
},
":",
{
"Ref": "AWS::AccountId"
},
":",
{
"Fn::ImportValue": "sls-test-project-dev-ApiGatewayRestApi"
},
"/*/*"
]
]
}
},
"DependsOn": [
"Testfct1WithSuffixLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU",
"Testfct1WithSuffixAlias"
]
},
"Testfct1LambdaPermissionApiGateway": {
"Type": "AWS::Lambda::Permission",
"Properties": {
Expand Down
174 changes: 174 additions & 0 deletions test/data/sls-stack-1.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
"LogGroupName": "/aws/lambda/sls-test-project-dev-testfct1"
}
},
"Testfct1WithSuffixLogGroup": {
"Type": "AWS::Logs::LogGroup",
"Properties": {
"LogGroupName": "/aws/lambda/sls-test-project-dev-testfct1-with-suffix"
}
},
"WarmUpPluginLogGroup": {
"Type": "AWS::Logs::LogGroup",
"Properties": {
Expand Down Expand Up @@ -197,6 +203,70 @@
"Description": "Echo function echoes alias"
}
},
"Testfct1WithSuffixLambdaFunction": {
"Type": "AWS::Lambda::Function",
"Properties": {
"Code": {
"S3Bucket": {
"Ref": "ServerlessDeploymentBucket"
},
"S3Key": "serverless/sls-test-project/dev/1494367071172-2017-05-09T21:57:51.172Z/sls-test-project.zip"
},
"FunctionName": "sls-test-project-dev-testfct1-with-suffix",
"Handler": "handlers/testfct1-with-suffix/handler.handle",
"MemorySize": 512,
"Role": {
"Fn::GetAtt": [
"IamRoleLambdaExecution",
"Arn"
]
},
"Runtime": "nodejs4.3",
"Timeout": 15,
"Description": "Echo function echoes alias",
"Environment": {
"Variables": {
"SERVERLESS_PROJECT_NAME": "sls-test-project",
"SERVERLESS_PROJECT": "sls-test-project",
"SERVERLESS_STAGE": "dev",
"SERVERLESS_REGION": "us-east-1",
"TEST_TABLE_NAME": {
"Ref": "TestDynamoDbTable"
}
}
},
"VpcConfig": {
"SecurityGroupIds": [
{
"Fn::ImportValue": "stashimi-dev-PrivateSG"
}
],
"SubnetIds": [
{
"Fn::ImportValue": "stashimi-dev-PrivateSubnet1"
},
{
"Fn::ImportValue": "stashimi-dev-PrivateSubnet2"
}
]
}
},
"DependsOn": [
"Testfct1WithSuffixLogGroup",
"IamRoleLambdaExecution"
]
},
"Testfct1WithSuffixLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU": {
"Type": "AWS::Lambda::Version",
"DeletionPolicy": "Retain",
"Properties": {
"FunctionName": {
"Ref": "Testfct1WithSuffixLambdaFunction"
},
"CodeSha256": "Wh5jTkiTR67+V05RPWQIlzPI25WiPbdHDYNgbtAMneU=",
"Description": "Echo function echoes alias"
}
},
"WarmUpPluginLambdaFunction": {
"Type": "AWS::Lambda::Function",
"Properties": {
Expand Down Expand Up @@ -401,6 +471,104 @@
]
}
}
},
"ApiGatewayResourceFunc2": {
"Type": "AWS::ApiGateway::Resource",
"Properties": {
"ParentId": {
"Fn::GetAtt": [
"ApiGatewayRestApi",
"RootResourceId"
]
},
"PathPart": "func1-with-suffix",
"RestApiId": {
"Ref": "ApiGatewayRestApi"
}
}
},
"ApiGatewayMethodFunc2Get": {
"Type": "AWS::ApiGateway::Method",
"Properties": {
"HttpMethod": "GET",
"RequestParameters": {},
"ResourceId": {
"Ref": "ApiGatewayResourceFunc2"
},
"RestApiId": {
"Ref": "ApiGatewayRestApi"
},
"AuthorizationType": "NONE",
"Integration": {
"IntegrationHttpMethod": "POST",
"Type": "AWS_PROXY",
"Uri": {
"Fn::Join": [
"",
[
"arn:aws:apigateway:",
{
"Ref": "AWS::Region"
},
":lambda:path/2015-03-31/functions/",
{
"Fn::GetAtt": [
"Testfct1WithSuffixLambdaFunction",
"Arn"
]
},
"/invocations"
]
]
}
},
"MethodResponses": []
}
},
"ApiGatewayDeployment1494367071212": {
"Type": "AWS::ApiGateway::Deployment",
"Properties": {
"RestApiId": {
"Ref": "ApiGatewayRestApi"
},
"StageName": "dev"
},
"DependsOn": [
"ApiGatewayMethodFunc2Get"
]
},
"Testfct1WithSuffixLambdaPermissionApiGateway": {
"Type": "AWS::Lambda::Permission",
"Properties": {
"FunctionName": {
"Fn::GetAtt": [
"Testfct1WithSuffixLambdaFunction",
"Arn"
]
},
"Action": "lambda:InvokeFunction",
"Principal": "apigateway.amazonaws.com",
"SourceArn": {
"Fn::Join": [
"",
[
"arn:aws:execute-api:",
{
"Ref": "AWS::Region"
},
":",
{
"Ref": "AWS::AccountId"
},
":",
{
"Ref": "ApiGatewayRestApi"
},
"/*/*"
]
]
}
}
}
},
"Outputs": {
Expand All @@ -415,6 +583,12 @@
"Ref": "Testfct1LambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU"
}
},
"Testfct1WithSuffixLambdaFunctionQualifiedArn": {
"Description": "Current Lambda function version",
"Value": {
"Ref": "Testfct1WithSuffixLambdaVersionWh5jTkiTR67V05RPWQIlzPI25WiPbdHDYNgbtAMneU"
}
},
"WarmUpPluginLambdaFunctionQualifiedArn": {
"Description": "Current Lambda function version",
"Value": {
Expand Down
Loading

0 comments on commit 6ac8882

Please sign in to comment.