Skip to content

Commit

Permalink
refactor: Abstract resolution of deployment role (#8751)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dmitry Shirokov committed Jan 14, 2021
1 parent 33cffc3 commit 4afdb83
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 110 deletions.
8 changes: 4 additions & 4 deletions lib/plugins/aws/customResources/index.js
View file
Expand Up @@ -79,9 +79,9 @@ async function addCustomResourceToService(awsProvider, resourceName, iamRoleStat
const s3FileName = zipFileBasename;
const S3Key = `${s3Folder}/${s3FileName}`;

const cfnRoleArn = serverless.service.provider.cfnRole;
const customDeploymentRole = awsProvider.getCustomDeploymentRole();

if (!cfnRoleArn) {
if (!customDeploymentRole) {
let customResourceRole = Resources[customResourcesRoleLogicalId];
if (!customResourceRole) {
customResourceRole = {
Expand Down Expand Up @@ -174,8 +174,8 @@ async function addCustomResourceToService(awsProvider, resourceName, iamRoleStat
};
Resources[customResourceFunctionLogicalId] = customResourceFunction;

if (cfnRoleArn) {
customResourceFunction.Properties.Role = cfnRoleArn;
if (customDeploymentRole) {
customResourceFunction.Properties.Role = customDeploymentRole;
} else {
customResourceFunction.Properties.Role = {
'Fn::GetAtt': [customResourcesRoleLogicalId, 'Arn'],
Expand Down
5 changes: 3 additions & 2 deletions lib/plugins/aws/deploy/lib/createStack.js
View file
Expand Up @@ -41,8 +41,9 @@ module.exports = {
params.Capabilities.push('CAPABILITY_AUTO_EXPAND');
}

if (this.serverless.service.provider.cfnRole) {
params.RoleARN = this.serverless.service.provider.cfnRole;
const customDeploymentRole = this.provider.getCustomDeploymentRole();
if (customDeploymentRole) {
params.RoleARN = customDeploymentRole;
}

if (this.serverless.service.provider.notificationArns) {
Expand Down
10 changes: 6 additions & 4 deletions lib/plugins/aws/lib/updateStack.js
View file
Expand Up @@ -47,8 +47,9 @@ module.exports = {
params.Capabilities.push('CAPABILITY_AUTO_EXPAND');
}

if (this.serverless.service.provider.cfnRole) {
params.RoleARN = this.serverless.service.provider.cfnRole;
const customDeploymentRole = this.provider.getCustomDeploymentRole();
if (customDeploymentRole) {
params.RoleARN = customDeploymentRole;
}

if (this.serverless.service.provider.notificationArns) {
Expand Down Expand Up @@ -103,8 +104,9 @@ module.exports = {
params.Capabilities.push('CAPABILITY_AUTO_EXPAND');
}

if (this.serverless.service.provider.cfnRole) {
params.RoleARN = this.serverless.service.provider.cfnRole;
const customDeploymentRole = this.provider.getCustomDeploymentRole();
if (customDeploymentRole) {
params.RoleARN = customDeploymentRole;
}

if (this.serverless.service.provider.notificationArns) {
Expand Down
6 changes: 5 additions & 1 deletion lib/plugins/aws/provider.js
View file
Expand Up @@ -61,7 +61,7 @@ const impl = {
/**
* Determine whether the given credentials are valid. It turned out that detecting invalid
* credentials was more difficult than detecting the positive cases we know about. Hooray for
* whak-a-mole!
* whack-a-mole!
* @param credentials The credentials to test for validity
* @return {boolean} Whether the given credentials were valid
*/
Expand Down Expand Up @@ -1510,6 +1510,10 @@ class AwsProvider {
return `${provider.deploymentPrefix}`;
}

getCustomDeploymentRole() {
return this.serverless.service.provider.cfnRole;
}

resolveFunctionArn(functionAddress) {
if (isLambdaArn(functionAddress)) return functionAddress;
const functionData = this.serverless.service.getFunction(functionAddress);
Expand Down
5 changes: 3 additions & 2 deletions lib/plugins/aws/remove/lib/stack.js
View file
Expand Up @@ -10,8 +10,9 @@ module.exports = {
StackName: stackName,
};

if (this.serverless.service.provider.cfnRole) {
params.RoleARN = this.serverless.service.provider.cfnRole;
const customDeploymentRole = this.provider.getCustomDeploymentRole();
if (customDeploymentRole) {
params.RoleARN = customDeploymentRole;
}

const cfData = {
Expand Down
127 changes: 30 additions & 97 deletions test/unit/lib/plugins/aws/customResources/index.test.js
View file
Expand Up @@ -11,6 +11,7 @@ const { createTmpDir } = require('../../../../../utils/fs');
const {
addCustomResourceToService,
} = require('../../../../../../lib/plugins/aws/customResources/index.js');
const runServerless = require('../../../../../utils/run-serverless');

const expect = chai.expect;
chai.use(require('sinon-chai'));
Expand Down Expand Up @@ -203,107 +204,39 @@ describe('#addCustomResourceToService()', () => {
]);
}));

it('Should not setup new IAM role, when cfnRole is provided', () => {
const cfnRoleArn = (serverless.service.provider.cfnRole =
'arn:aws:iam::999999999999:role/some-role');
return BbPromise.all([
// add the custom S3 resource
addCustomResourceToService(provider, 's3', [
...iamRoleStatements,
{
Effect: 'Allow',
Resource: 'arn:aws:s3:::some-bucket',
Action: ['s3:PutBucketNotification', 's3:GetBucketNotification'],
},
]),
// add the custom Cognito User Pool resource
addCustomResourceToService(provider, 'cognitoUserPool', [
...iamRoleStatements,
{
Effect: 'Allow',
Resource: '*',
Action: [
'cognito-idp:ListUserPools',
'cognito-idp:DescribeUserPool',
'cognito-idp:UpdateUserPool',
],
},
]),
// add the custom Event Bridge resource
addCustomResourceToService(provider, 'eventBridge', [
...iamRoleStatements,
{
Effect: 'Allow',
Resource: 'arn:aws:events:*:*:rule/some-rule',
Action: [
'events:PutRule',
'events:RemoveTargets',
'events:PutTargets',
'events:DeleteRule',
],
},
{
Action: ['events:CreateEventBus', 'events:DeleteEventBus'],
Effect: 'Allow',
Resource: 'arn:aws:events:*:*:event-bus/some-event-bus',
},
]),
]).then(() => {
const { Resources } = serverless.service.provider.compiledCloudFormationTemplate;
// S3 Lambda Function
expect(Resources.CustomDashresourceDashexistingDashs3LambdaFunction).to.deep.equal({
Type: 'AWS::Lambda::Function',
Properties: {
Code: {
S3Bucket: { Ref: 'ServerlessDeploymentBucket' },
S3Key: 'artifact-dir-name/custom-resources.zip',
},
FunctionName: `${serviceName}-dev-custom-resource-existing-s3`,
Handler: 's3/handler.handler',
MemorySize: 1024,
Role: cfnRoleArn,
Runtime: 'nodejs12.x',
Timeout: 180,
},
DependsOn: [],
});
// Cognito User Pool Lambda Function
expect(Resources.CustomDashresourceDashexistingDashcupLambdaFunction).to.deep.equal({
Type: 'AWS::Lambda::Function',
Properties: {
Code: {
S3Bucket: { Ref: 'ServerlessDeploymentBucket' },
S3Key: 'artifact-dir-name/custom-resources.zip',
},
FunctionName: `${serviceName}-dev-custom-resource-existing-cup`,
Handler: 'cognitoUserPool/handler.handler',
MemorySize: 1024,
Role: cfnRoleArn,
Runtime: 'nodejs12.x',
Timeout: 180,
it('should use custom role when cfnRole is provided', async () => {
const role = 'arn:aws:iam::999999999999:role/my-role';
const { cfTemplate } = await runServerless({
fixture: 'function',
cliArgs: ['package'],
configExt: {
provider: {
cfnRole: role,
},
DependsOn: [],
});
// Event Bridge Lambda Function
expect(Resources.CustomDashresourceDasheventDashbridgeLambdaFunction).to.deep.equal({
Type: 'AWS::Lambda::Function',
Properties: {
Code: {
S3Bucket: { Ref: 'ServerlessDeploymentBucket' },
S3Key: 'artifact-dir-name/custom-resources.zip',
functions: {
foo: {
handler: 'foo.bar',
events: [
{ s3: { bucket: 'my-bucket', existing: true } },
{ cognitoUserPool: { pool: 'my-user-pool', trigger: 'PreSignUp', existing: true } },
{
eventBridge: {
eventBus: 'arn:aws:events:us-east-1:12345:event-bus/default',
schedule: 'rate(10 minutes)',
},
},
],
},
FunctionName: `${serviceName}-dev-custom-resource-event-bridge`,
Handler: 'eventBridge/handler.handler',
MemorySize: 1024,
Role: cfnRoleArn,
Runtime: 'nodejs12.x',
Timeout: 180,
},
DependsOn: [],
});
// Iam Role
expect(Resources.IamRoleCustomResourcesLambdaExecution).to.be.undefined;
},
});

const { Resources } = cfTemplate;
expect([
Resources.CustomDashresourceDashexistingDashs3LambdaFunction.Properties.Role, // S3
Resources.CustomDashresourceDashexistingDashcupLambdaFunction.Properties.Role, // Cognito User Pool
Resources.CustomDashresourceDasheventDashbridgeLambdaFunction.Properties.Role, // Event Bridge
]).to.eql([role, role, role]);
});

it('should setup CloudWatch Logs when logs.frameworkLambda is true', () => {
Expand Down

0 comments on commit 4afdb83

Please sign in to comment.