From a025816d555d620a68c71251c91fbeab1cf18fb0 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Sat, 3 Oct 2020 00:59:05 +0530 Subject: [PATCH 01/14] added awsRequest for aws-sdk call backoff and retry in customResources --- .../aws/customResources/resources/utils.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index e855cd5b685..d63e9a3d568 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -90,6 +90,32 @@ function wait(ms) { return new Promise(resolve => setTimeout(resolve, ms)); } +const MAX_AWS_REQUEST_TRY = (() => { + const DEFAULT_MAX_AWS_REQUEST_TRY = 4 + const userValue = Number(process.env.MAX_AWS_REQUEST_TRY); + return userValue >= 0 ? userValue : DEFAULT_MAX_AWS_REQUEST_TRY; +})(); + +function awsRequest(serviceInstance, method, ...args) { + const callAws = (requestTry) => { + return serviceInstance[method](...args) + .promise() + .then( + result => result, + error => { + if (requestTry < MAX_AWS_REQUEST_TRY && error && + (error.statusCode === 429 || + (error.retryable && error.statusCode !== 403 && error.code !== 'CredentialsError') + )) { + return wait( 4000 + Math.random() * 3000).then(() => callAws(++requestTry)); + } + throw error; + } + ) + } + return callAws(1); +} + module.exports = { logger, response, @@ -97,4 +123,5 @@ module.exports = { getLambdaArn, handlerWrapper, wait, + awsRequest, }; From 49b9f15a4e76abcbe30243c18b11f29bcb9fcc33 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Sat, 3 Oct 2020 01:14:52 +0530 Subject: [PATCH 02/14] update for prettier --- .../aws/customResources/resources/utils.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index d63e9a3d568..35e85db9ad3 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -91,28 +91,30 @@ function wait(ms) { } const MAX_AWS_REQUEST_TRY = (() => { - const DEFAULT_MAX_AWS_REQUEST_TRY = 4 + const DEFAULT_MAX_AWS_REQUEST_TRY = 4; const userValue = Number(process.env.MAX_AWS_REQUEST_TRY); return userValue >= 0 ? userValue : DEFAULT_MAX_AWS_REQUEST_TRY; })(); function awsRequest(serviceInstance, method, ...args) { - const callAws = (requestTry) => { + const callAws = requestTry => { return serviceInstance[method](...args) .promise() .then( result => result, error => { - if (requestTry < MAX_AWS_REQUEST_TRY && error && + if ( + requestTry < MAX_AWS_REQUEST_TRY && + error && (error.statusCode === 429 || - (error.retryable && error.statusCode !== 403 && error.code !== 'CredentialsError') - )) { - return wait( 4000 + Math.random() * 3000).then(() => callAws(++requestTry)); + (error.retryable && error.statusCode !== 403 && error.code !== 'CredentialsError')) + ) { + return wait(4000 + Math.random() * 3000).then(() => callAws(++requestTry)); } throw error; } - ) - } + ); + }; return callAws(1); } From cbafc9c6fe7e0446d70c2e794f145d5815b79bdc Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Sat, 3 Oct 2020 11:27:44 +0530 Subject: [PATCH 03/14] using existing env variable SLS_AWS_REQUEST_MAX_RETRIES --- lib/plugins/aws/customResources/resources/utils.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index 35e85db9ad3..dc2e15edc95 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -92,7 +92,7 @@ function wait(ms) { const MAX_AWS_REQUEST_TRY = (() => { const DEFAULT_MAX_AWS_REQUEST_TRY = 4; - const userValue = Number(process.env.MAX_AWS_REQUEST_TRY); + const userValue = Number(process.env.SLS_AWS_REQUEST_MAX_RETRIES); return userValue >= 0 ? userValue : DEFAULT_MAX_AWS_REQUEST_TRY; })(); From 8f0b61d528aaa34e4a49f7c7b12151e098b479b7 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 20:38:34 +0530 Subject: [PATCH 04/14] added functionality to resolve aws service instance in awsRequest --- .../aws/customResources/resources/utils.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index dc2e15edc95..364d8c9d859 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -1,5 +1,7 @@ 'use strict'; +const AWS = require('aws-sdk'); +const _ = require('lodash'); const https = require('https'); const url = require('url'); @@ -96,7 +98,20 @@ const MAX_AWS_REQUEST_TRY = (() => { return userValue >= 0 ? userValue : DEFAULT_MAX_AWS_REQUEST_TRY; })(); -function awsRequest(serviceInstance, method, ...args) { +const getServiceInstance = _.memoize( + nameInput => { + const params = Object.assign({ region: 'us-east-1' }, nameInput.params); + const name = typeof nameInput === 'string' ? nameInput : nameInput.name; + const Service = _.get(AWS, name); + return new Service(params); + }, + nameInput => { + return typeof nameInput === 'string' ? nameInput : JSON.stringify(nameInput); + } +); + +function awsRequest(service, method, ...args) { + const serviceInstance = getServiceInstance(service); const callAws = requestTry => { return serviceInstance[method](...args) .promise() From 2c43a32a16633ae544218a258d5c8570380209f5 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 20:42:29 +0530 Subject: [PATCH 05/14] integrated awsRequest with s3 custom resource --- .../resources/s3/lib/bucket.js | 24 ++++++++++++------- .../resources/s3/lib/permissions.js | 8 +++---- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/s3/lib/bucket.js b/lib/plugins/aws/customResources/resources/s3/lib/bucket.js index b8a36a6b38b..5fd48c8918c 100644 --- a/lib/plugins/aws/customResources/resources/s3/lib/bucket.js +++ b/lib/plugins/aws/customResources/resources/s3/lib/bucket.js @@ -1,7 +1,7 @@ 'use strict'; const crypto = require('crypto'); -const AWS = require('aws-sdk'); +const { awsRequest } = require('../../utils'); function generateId(functionName, bucketConfig) { const md5 = crypto @@ -34,20 +34,21 @@ function createFilter(config) { function getConfiguration(config) { const { bucketName, region } = config; - const s3 = new AWS.S3({ region }); const Bucket = bucketName; const payload = { Bucket, }; - return s3 - .getBucketNotificationConfiguration(payload) + return awsRequest( + { name: 'S3', params: { region } }, + 'getBucketNotificationConfiguration', + payload + ) .promise() .then(data => data); } function updateConfiguration(config) { const { lambdaArn, functionName, bucketName, bucketConfigs, region } = config; - const s3 = new AWS.S3({ region }); const Bucket = bucketName; return getConfiguration(config).then(NotificationConfiguration => { @@ -74,13 +75,16 @@ function updateConfiguration(config) { Bucket, NotificationConfiguration, }; - return s3.putBucketNotificationConfiguration(payload).promise(); + return awsRequest( + { name: 'S3', params: { region } }, + 'putBucketNotificationConfiguration', + payload + ).promise(); }); } function removeConfiguration(config) { const { functionName, bucketName, region } = config; - const s3 = new AWS.S3({ region }); const Bucket = bucketName; return getConfiguration(config).then(NotificationConfiguration => { @@ -93,7 +97,11 @@ function removeConfiguration(config) { Bucket, NotificationConfiguration, }; - return s3.putBucketNotificationConfiguration(payload).promise(); + return awsRequest( + { name: 'S3', params: { region } }, + 'putBucketNotificationConfiguration', + payload + ).promise(); }); } diff --git a/lib/plugins/aws/customResources/resources/s3/lib/permissions.js b/lib/plugins/aws/customResources/resources/s3/lib/permissions.js index e3b6b778c32..08128b0b15c 100644 --- a/lib/plugins/aws/customResources/resources/s3/lib/permissions.js +++ b/lib/plugins/aws/customResources/resources/s3/lib/permissions.js @@ -1,6 +1,6 @@ 'use strict'; -const AWS = require('aws-sdk'); +const { awsRequest } = require('../../utils'); function getStatementId(functionName, bucketName) { const normalizedBucketName = bucketName.replace(/[.:*]/g, ''); @@ -13,7 +13,6 @@ function getStatementId(functionName, bucketName) { function addPermission(config) { const { functionName, bucketName, partition, region, accountId } = config; - const lambda = new AWS.Lambda({ region }); const payload = { Action: 'lambda:InvokeFunction', FunctionName: functionName, @@ -22,17 +21,16 @@ function addPermission(config) { SourceArn: `arn:${partition}:s3:::${bucketName}`, SourceAccount: accountId, }; - return lambda.addPermission(payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload).promise(); } function removePermission(config) { const { functionName, bucketName, region } = config; - const lambda = new AWS.Lambda({ region }); const payload = { FunctionName: functionName, StatementId: getStatementId(functionName, bucketName), }; - return lambda.removePermission(payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload).promise(); } module.exports = { From 53d52aa98afa80934bbc59f389e1b7ab70f55da3 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 20:53:37 +0530 Subject: [PATCH 06/14] integrated awsRequest with Event Bridge custom resource --- .../resources/eventBridge/lib/eventBridge.js | 74 +++++++------------ .../resources/eventBridge/lib/permissions.js | 12 ++- 2 files changed, 33 insertions(+), 53 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js b/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js index b813afce0f7..ef0a6154854 100644 --- a/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js +++ b/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js @@ -1,6 +1,6 @@ 'use strict'; -const EventBridge = require('aws-sdk/clients/eventbridge'); +const { awsRequest } = require('../../utils'); const { getEventBusName, getEventBusTargetId } = require('./utils'); function createEventBus(config) { @@ -10,12 +10,9 @@ function createEventBus(config) { if (eventBus.startsWith('arn')) { return Promise.resolve(); } - const eventBridge = new EventBridge({ region }); - return eventBridge - .createEventBus({ - Name: eventBus, - }) - .promise(); + return awsRequest({ name: 'EventBridge', params: { region } }, 'createEventBus', { + Name: eventBus, + }).promise(); } return Promise.resolve(); } @@ -28,50 +25,40 @@ function deleteEventBus(config) { return Promise.resolve(); } - const eventBridge = new EventBridge({ region }); - return eventBridge - .deleteEventBus({ - Name: eventBus, - }) - .promise(); + return awsRequest({ name: 'EventBridge', params: { region } }, 'deleteEventBus', { + Name: eventBus, + }).promise(); } return Promise.resolve(); } function updateRuleConfiguration(config) { const { ruleName, eventBus, pattern, schedule, region } = config; - const eventBridge = new EventBridge({ region }); const EventBusName = getEventBusName(eventBus); - return eventBridge - .putRule({ - Name: ruleName, - EventBusName, - EventPattern: JSON.stringify(pattern), - ScheduleExpression: schedule, - State: 'ENABLED', - }) - .promise(); + return awsRequest({ name: 'EventBridge', params: { region } }, 'putRule', { + Name: ruleName, + EventBusName, + EventPattern: JSON.stringify(pattern), + ScheduleExpression: schedule, + State: 'ENABLED', + }).promise(); } function removeRuleConfiguration(config) { const { ruleName, eventBus, region } = config; - const eventBridge = new EventBridge({ region }); const EventBusName = getEventBusName(eventBus); - return eventBridge - .deleteRule({ - Name: ruleName, - EventBusName, - }) - .promise(); + return awsRequest({ name: 'EventBridge', params: { region } }, 'deleteRule', { + Name: ruleName, + EventBusName, + }).promise(); } function updateTargetConfiguration(config) { const { lambdaArn, ruleName, eventBus, input, inputPath, inputTransformer, region } = config; - const eventBridge = new EventBridge({ region }); const EventBusName = getEventBusName(eventBus); @@ -89,29 +76,24 @@ function updateTargetConfiguration(config) { } return removeTargetConfiguration(config).then(() => - eventBridge - .putTargets({ - Rule: ruleName, - EventBusName, - Targets: [target], - }) - .promise() + awsRequest({ name: 'EventBridge', params: { region } }, 'putTargets', { + Rule: ruleName, + EventBusName, + Targets: [target], + }).promise() ); } function removeTargetConfiguration(config) { const { ruleName, eventBus, region } = config; - const eventBridge = new EventBridge({ region }); const EventBusName = getEventBusName(eventBus); - return eventBridge - .removeTargets({ - Ids: [getEventBusTargetId(ruleName)], - Rule: ruleName, - EventBusName, - }) - .promise(); + return awsRequest({ name: 'EventBridge', params: { region } }, 'removeTargets', { + Ids: [getEventBusTargetId(ruleName)], + Rule: ruleName, + EventBusName, + }).promise(); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js b/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js index be4094d3328..293e08dffeb 100644 --- a/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js +++ b/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js @@ -1,6 +1,6 @@ 'use strict'; -const AWS = require('aws-sdk'); +const { awsRequest } = require('../../utils'); const { getEventBusName } = require('./utils'); function getStatementId(functionName, ruleName) { @@ -14,30 +14,28 @@ function getStatementId(functionName, ruleName) { function addPermission(config) { const { functionName, partition, region, accountId, eventBus, ruleName } = config; - const lambda = new AWS.Lambda({ region }); let SourceArn = `arn:${partition}:events:${region}:${accountId}:rule/${ruleName}`; if (eventBus) { const eventBusName = getEventBusName(eventBus); SourceArn = `arn:${partition}:events:${region}:${accountId}:rule/${eventBusName}/${ruleName}`; } - const params = { + const payload = { Action: 'lambda:InvokeFunction', FunctionName: functionName, Principal: 'events.amazonaws.com', StatementId: getStatementId(functionName, ruleName), SourceArn, }; - return lambda.addPermission(params).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload).promise(); } function removePermission(config) { const { functionName, region, ruleName } = config; - const lambda = new AWS.Lambda({ region }); - const params = { + const payload = { FunctionName: functionName, StatementId: getStatementId(functionName, ruleName), }; - return lambda.removePermission(params).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload).promise(); } module.exports = { From d2afc2cbabd1fb0998130c0aa436e4483b363129 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 21:02:53 +0530 Subject: [PATCH 07/14] integrated awsRequest with CognitoUserPool custom resource --- .../cognitoUserPool/lib/permissions.js | 12 +++--- .../resources/cognitoUserPool/lib/userPool.js | 37 +++++++++++-------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js index d8823334321..2a45d23a53f 100644 --- a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js +++ b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js @@ -1,6 +1,6 @@ 'use strict'; -const Lambda = require('aws-sdk/clients/lambda'); +const { awsRequest } = require('../../utils'); function getStatementId(functionName, userPoolName) { const normalizedUserPoolName = userPoolName.toLowerCase().replace(/[.:*\s]/g, ''); @@ -13,25 +13,23 @@ function getStatementId(functionName, userPoolName) { function addPermission(config) { const { functionName, userPoolName, partition, region, accountId, userPoolId } = config; - const lambda = new Lambda({ region }); - const params = { + const payload = { Action: 'lambda:InvokeFunction', FunctionName: functionName, Principal: 'cognito-idp.amazonaws.com', StatementId: getStatementId(functionName, userPoolName), SourceArn: `arn:${partition}:cognito-idp:${region}:${accountId}:userpool/${userPoolId}`, }; - return lambda.addPermission(params).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload).promise(); } function removePermission(config) { const { functionName, userPoolName, region } = config; - const lambda = new Lambda({ region }); - const params = { + const payload = { FunctionName: functionName, StatementId: getStatementId(functionName, userPoolName), }; - return lambda.removePermission(params).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload).promise(); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js index 5186da07cf6..00aaaa549d5 100644 --- a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js +++ b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js @@ -1,6 +1,6 @@ 'use strict'; -const CognitoIdentityServiceProvider = require('aws-sdk/clients/cognitoidentityserviceprovider'); +const { awsRequest } = require('../../utils'); function getUpdateConfigFromCurrentSetup(currentSetup) { const updatedConfig = Object.assign({}, currentSetup); @@ -28,16 +28,18 @@ function getUpdateConfigFromCurrentSetup(currentSetup) { function findUserPoolByName(config) { const { userPoolName, region } = config; - const cognito = new CognitoIdentityServiceProvider({ region }); - const params = { + const payload = { MaxResults: 60, }; function recursiveFind(nextToken) { - if (nextToken) params.NextToken = nextToken; - return cognito - .listUserPools(params) + if (nextToken) payload.NextToken = nextToken; + return awsRequest( + { name: 'CognitoIdentityServiceProvider', params: { region } }, + 'listUserPools', + payload + ) .promise() .then(result => { const matches = result.UserPools.filter(pool => pool.Name === userPoolName); @@ -54,20 +56,16 @@ function findUserPoolByName(config) { function getConfiguration(config) { const { region } = config; - const cognito = new CognitoIdentityServiceProvider({ region }); return findUserPoolByName(config).then(userPool => - cognito - .describeUserPool({ - UserPoolId: userPool.Id, - }) - .promise() + awsRequest({ name: 'CognitoIdentityServiceProvider', params: { region } }, 'describeUserPool', { + UserPoolId: userPool.Id, + }).promise() ); } function updateConfiguration(config) { const { lambdaArn, userPoolConfigs, region } = config; - const cognito = new CognitoIdentityServiceProvider({ region }); return getConfiguration(config).then(res => { const UserPoolId = res.UserPool.Id; @@ -88,13 +86,16 @@ function updateConfiguration(config) { LambdaConfig, }); - return cognito.updateUserPool(updatedConfig).promise(); + return awsRequest( + { name: 'CognitoIdentityServiceProvider', params: { region } }, + 'updateUserPool', + updatedConfig + ).promise(); }); } function removeConfiguration(config) { const { lambdaArn, region } = config; - const cognito = new CognitoIdentityServiceProvider({ region }); return getConfiguration(config).then(res => { const UserPoolId = res.UserPool.Id; @@ -111,7 +112,11 @@ function removeConfiguration(config) { LambdaConfig, }); - return cognito.updateUserPool(updatedConfig).promise(); + return awsRequest( + { name: 'CognitoIdentityServiceProvider', params: { region } }, + 'updateUserPool', + updatedConfig + ).promise(); }); } From 8a4523b240fc7ec771083e9d6fa69affd512a6e1 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 21:12:54 +0530 Subject: [PATCH 08/14] integrated awsRequest with APIGatewayCloudWatchRole custom resource --- .../apiGatewayCloudWatchRole/handler.js | 77 +++++++++---------- 1 file changed, 35 insertions(+), 42 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js index 08722332625..58de08447da 100644 --- a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js +++ b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js @@ -1,7 +1,6 @@ 'use strict'; -const ApiGateway = require('aws-sdk/clients/apigateway'); -const Iam = require('aws-sdk/clients/iam'); +const { awsRequest } = require('../utils'); const { getEnvironment, handlerWrapper, wait } = require('../utils'); function handler(event, context) { @@ -19,8 +18,9 @@ async function create(event, context) { const { RoleArn } = event.ResourceProperties; const { Partition: partition, AccountId: accountId, Region: region } = getEnvironment(context); - const apiGateway = new ApiGateway({ region }); - const assignedRoleArn = (await apiGateway.getAccount().promise()).cloudwatchRoleArn; + const assignedRoleArn = ( + await awsRequest({ name: 'ApiGateway', params: { region } }, 'getAccount').promise() + ).cloudwatchRoleArn; let roleArn = `arn:${partition}:iam::${accountId}:role/serverlessApiGatewayCloudWatchRole`; if (RoleArn) { @@ -32,33 +32,30 @@ async function create(event, context) { const roleName = roleArn.slice(roleArn.lastIndexOf('/') + 1); - const iam = new Iam(); - const attachedPolicies = await (async () => { try { - return (await iam.listAttachedRolePolicies({ RoleName: roleName }).promise()) - .AttachedPolicies; + return ( + await awsRequest('IAM', 'listAttachedRolePolicies', { RoleName: roleName }).promise() + ).AttachedPolicies; } catch (error) { if (error.code === 'NoSuchEntity') { // Role doesn't exist yet, create; - await iam - .createRole({ - AssumeRolePolicyDocument: JSON.stringify({ - Version: '2012-10-17', - Statement: [ - { - Effect: 'Allow', - Principal: { - Service: ['apigateway.amazonaws.com'], - }, - Action: ['sts:AssumeRole'], + await awsRequest('IAM', 'createRole', { + AssumeRolePolicyDocument: JSON.stringify({ + Version: '2012-10-17', + Statement: [ + { + Effect: 'Allow', + Principal: { + Service: ['apigateway.amazonaws.com'], }, - ], - }), - Path: '/', - RoleName: roleName, - }) - .promise(); + Action: ['sts:AssumeRole'], + }, + ], + }), + Path: '/', + RoleName: roleName, + }).promise(); return []; } throw error; @@ -68,12 +65,10 @@ async function create(event, context) { if ( !attachedPolicies.some(policy => policy.PolicyArn === apiGatewayPushToCloudWatchLogsPolicyArn) ) { - await iam - .attachRolePolicy({ - PolicyArn: apiGatewayPushToCloudWatchLogsPolicyArn, - RoleName: roleName, - }) - .promise(); + await awsRequest('IAM', 'attachRolePolicy', { + PolicyArn: apiGatewayPushToCloudWatchLogsPolicyArn, + RoleName: roleName, + }).promise(); } } @@ -82,17 +77,15 @@ async function create(event, context) { const updateAccount = async (counter = 1) => { try { - await apiGateway - .updateAccount({ - patchOperations: [ - { - op: 'replace', - path: '/cloudwatchRoleArn', - value: roleArn, - }, - ], - }) - .promise(); + await awsRequest({ name: 'ApiGateway', params: { region } }, 'updateAccount', { + patchOperations: [ + { + op: 'replace', + path: '/cloudwatchRoleArn', + value: roleArn, + }, + ], + }).promise(); } catch (error) { if (counter < 10) { // Observed fails with errors marked as non-retryable. Still they're outcome of From ff2a285f70e3973c217b91940aa85abb4cb185e0 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 21:18:10 +0530 Subject: [PATCH 09/14] fix case of APIGateway for awsRequest in APIGatewayCloudWatchRole custom resource --- .../resources/apiGatewayCloudWatchRole/handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js index 58de08447da..e744f0c46d3 100644 --- a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js +++ b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js @@ -19,7 +19,7 @@ async function create(event, context) { const { Partition: partition, AccountId: accountId, Region: region } = getEnvironment(context); const assignedRoleArn = ( - await awsRequest({ name: 'ApiGateway', params: { region } }, 'getAccount').promise() + await awsRequest({ name: 'APIGateway', params: { region } }, 'getAccount').promise() ).cloudwatchRoleArn; let roleArn = `arn:${partition}:iam::${accountId}:role/serverlessApiGatewayCloudWatchRole`; From 3d1a69794819dc9833ac3b8ffe8222f8bbf0d104 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Mon, 5 Oct 2020 21:28:05 +0530 Subject: [PATCH 10/14] fix case of APIGateway for awsRequest in APIGatewayCloudWatchRole custom resource --- .../resources/apiGatewayCloudWatchRole/handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js index e744f0c46d3..65e42b53dca 100644 --- a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js +++ b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js @@ -77,7 +77,7 @@ async function create(event, context) { const updateAccount = async (counter = 1) => { try { - await awsRequest({ name: 'ApiGateway', params: { region } }, 'updateAccount', { + await awsRequest({ name: 'APIGateway', params: { region } }, 'updateAccount', { patchOperations: [ { op: 'replace', From ae40f38f9fc7aaeff6457bd9d7effaaf7998c6fb Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Tue, 6 Oct 2020 19:41:36 +0530 Subject: [PATCH 11/14] removed lodash dependency --- .../aws/customResources/resources/utils.js | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index 364d8c9d859..809c51eb25c 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -1,7 +1,6 @@ 'use strict'; const AWS = require('aws-sdk'); -const _ = require('lodash'); const https = require('https'); const url = require('url'); @@ -98,17 +97,17 @@ const MAX_AWS_REQUEST_TRY = (() => { return userValue >= 0 ? userValue : DEFAULT_MAX_AWS_REQUEST_TRY; })(); -const getServiceInstance = _.memoize( - nameInput => { - const params = Object.assign({ region: 'us-east-1' }, nameInput.params); - const name = typeof nameInput === 'string' ? nameInput : nameInput.name; - const Service = _.get(AWS, name); - return new Service(params); - }, - nameInput => { - return typeof nameInput === 'string' ? nameInput : JSON.stringify(nameInput); +const getServiceInstance = nameInput => { + let name; + let params = {}; + if (typeof nameInput === 'string') { + name = nameInput; + } else { + name = nameInput.name; + params = nameInput.params; } -); + return new AWS[name](params); +}; function awsRequest(service, method, ...args) { const serviceInstance = getServiceInstance(service); From 587553926f64e9f56fc492d5e4e64e1dfc6b944b Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Tue, 6 Oct 2020 21:31:13 +0530 Subject: [PATCH 12/14] changed awsRequest to async and removed unnecessary promise - changed awsRequest to async - removed unnecessary promise in awsRequest --- .../apiGatewayCloudWatchRole/handler.js | 43 +++++++------------ .../cognitoUserPool/lib/permissions.js | 4 +- .../resources/cognitoUserPool/lib/userPool.js | 24 +++++------ .../resources/eventBridge/lib/eventBridge.js | 12 +++--- .../resources/eventBridge/lib/permissions.js | 4 +- .../resources/s3/lib/bucket.js | 8 ++-- .../resources/s3/lib/permissions.js | 4 +- .../aws/customResources/resources/utils.js | 2 +- 8 files changed, 43 insertions(+), 58 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js index 65e42b53dca..7d80df8672c 100644 --- a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js +++ b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js @@ -1,7 +1,7 @@ 'use strict'; const { awsRequest } = require('../utils'); -const { getEnvironment, handlerWrapper, wait } = require('../utils'); +const { getEnvironment, handlerWrapper } = require('../utils'); function handler(event, context) { if (event.RequestType === 'Create') { @@ -19,7 +19,7 @@ async function create(event, context) { const { Partition: partition, AccountId: accountId, Region: region } = getEnvironment(context); const assignedRoleArn = ( - await awsRequest({ name: 'APIGateway', params: { region } }, 'getAccount').promise() + await awsRequest({ name: 'APIGateway', params: { region } }, 'getAccount') ).cloudwatchRoleArn; let roleArn = `arn:${partition}:iam::${accountId}:role/serverlessApiGatewayCloudWatchRole`; @@ -34,9 +34,8 @@ async function create(event, context) { const attachedPolicies = await (async () => { try { - return ( - await awsRequest('IAM', 'listAttachedRolePolicies', { RoleName: roleName }).promise() - ).AttachedPolicies; + return (await awsRequest('IAM', 'listAttachedRolePolicies', { RoleName: roleName })) + .AttachedPolicies; } catch (error) { if (error.code === 'NoSuchEntity') { // Role doesn't exist yet, create; @@ -55,7 +54,7 @@ async function create(event, context) { }), Path: '/', RoleName: roleName, - }).promise(); + }); return []; } throw error; @@ -68,33 +67,23 @@ async function create(event, context) { await awsRequest('IAM', 'attachRolePolicy', { PolicyArn: apiGatewayPushToCloudWatchLogsPolicyArn, RoleName: roleName, - }).promise(); + }); } } // there's nothing to do if the role is the same if (roleArn === assignedRoleArn) return null; - const updateAccount = async (counter = 1) => { - try { - await awsRequest({ name: 'APIGateway', params: { region } }, 'updateAccount', { - patchOperations: [ - { - op: 'replace', - path: '/cloudwatchRoleArn', - value: roleArn, - }, - ], - }).promise(); - } catch (error) { - if (counter < 10) { - // Observed fails with errors marked as non-retryable. Still they're outcome of - // temporary state where just created AWS role is not being ready for use (yet) - await wait(10000); - return updateAccount(++counter); - } - throw error; - } + const updateAccount = async () => { + await awsRequest({ name: 'APIGateway', params: { region } }, 'updateAccount', { + patchOperations: [ + { + op: 'replace', + path: '/cloudwatchRoleArn', + value: roleArn, + }, + ], + }); return null; }; diff --git a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js index 2a45d23a53f..b971f9d56e9 100644 --- a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js +++ b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js @@ -20,7 +20,7 @@ function addPermission(config) { StatementId: getStatementId(functionName, userPoolName), SourceArn: `arn:${partition}:cognito-idp:${region}:${accountId}:userpool/${userPoolId}`, }; - return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload); } function removePermission(config) { @@ -29,7 +29,7 @@ function removePermission(config) { FunctionName: functionName, StatementId: getStatementId(functionName, userPoolName), }; - return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js index 00aaaa549d5..c53a0caee9d 100644 --- a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js +++ b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/userPool.js @@ -39,16 +39,14 @@ function findUserPoolByName(config) { { name: 'CognitoIdentityServiceProvider', params: { region } }, 'listUserPools', payload - ) - .promise() - .then(result => { - const matches = result.UserPools.filter(pool => pool.Name === userPoolName); - if (matches.length) { - return matches.shift(); - } - if (result.NextToken) return recursiveFind(result.NextToken); - return null; - }); + ).then(result => { + const matches = result.UserPools.filter(pool => pool.Name === userPoolName); + if (matches.length) { + return matches.shift(); + } + if (result.NextToken) return recursiveFind(result.NextToken); + return null; + }); } return recursiveFind(); @@ -60,7 +58,7 @@ function getConfiguration(config) { return findUserPoolByName(config).then(userPool => awsRequest({ name: 'CognitoIdentityServiceProvider', params: { region } }, 'describeUserPool', { UserPoolId: userPool.Id, - }).promise() + }) ); } @@ -90,7 +88,7 @@ function updateConfiguration(config) { { name: 'CognitoIdentityServiceProvider', params: { region } }, 'updateUserPool', updatedConfig - ).promise(); + ); }); } @@ -116,7 +114,7 @@ function removeConfiguration(config) { { name: 'CognitoIdentityServiceProvider', params: { region } }, 'updateUserPool', updatedConfig - ).promise(); + ); }); } diff --git a/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js b/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js index ef0a6154854..ca02332a5e7 100644 --- a/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js +++ b/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js @@ -12,7 +12,7 @@ function createEventBus(config) { } return awsRequest({ name: 'EventBridge', params: { region } }, 'createEventBus', { Name: eventBus, - }).promise(); + }); } return Promise.resolve(); } @@ -27,7 +27,7 @@ function deleteEventBus(config) { return awsRequest({ name: 'EventBridge', params: { region } }, 'deleteEventBus', { Name: eventBus, - }).promise(); + }); } return Promise.resolve(); } @@ -43,7 +43,7 @@ function updateRuleConfiguration(config) { EventPattern: JSON.stringify(pattern), ScheduleExpression: schedule, State: 'ENABLED', - }).promise(); + }); } function removeRuleConfiguration(config) { @@ -54,7 +54,7 @@ function removeRuleConfiguration(config) { return awsRequest({ name: 'EventBridge', params: { region } }, 'deleteRule', { Name: ruleName, EventBusName, - }).promise(); + }); } function updateTargetConfiguration(config) { @@ -80,7 +80,7 @@ function updateTargetConfiguration(config) { Rule: ruleName, EventBusName, Targets: [target], - }).promise() + }) ); } @@ -93,7 +93,7 @@ function removeTargetConfiguration(config) { Ids: [getEventBusTargetId(ruleName)], Rule: ruleName, EventBusName, - }).promise(); + }); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js b/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js index 293e08dffeb..535dc5fdca5 100644 --- a/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js +++ b/lib/plugins/aws/customResources/resources/eventBridge/lib/permissions.js @@ -26,7 +26,7 @@ function addPermission(config) { StatementId: getStatementId(functionName, ruleName), SourceArn, }; - return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload); } function removePermission(config) { @@ -35,7 +35,7 @@ function removePermission(config) { FunctionName: functionName, StatementId: getStatementId(functionName, ruleName), }; - return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/s3/lib/bucket.js b/lib/plugins/aws/customResources/resources/s3/lib/bucket.js index 5fd48c8918c..51985e47a1b 100644 --- a/lib/plugins/aws/customResources/resources/s3/lib/bucket.js +++ b/lib/plugins/aws/customResources/resources/s3/lib/bucket.js @@ -42,9 +42,7 @@ function getConfiguration(config) { { name: 'S3', params: { region } }, 'getBucketNotificationConfiguration', payload - ) - .promise() - .then(data => data); + ).then(data => data); } function updateConfiguration(config) { @@ -79,7 +77,7 @@ function updateConfiguration(config) { { name: 'S3', params: { region } }, 'putBucketNotificationConfiguration', payload - ).promise(); + ); }); } @@ -101,7 +99,7 @@ function removeConfiguration(config) { { name: 'S3', params: { region } }, 'putBucketNotificationConfiguration', payload - ).promise(); + ); }); } diff --git a/lib/plugins/aws/customResources/resources/s3/lib/permissions.js b/lib/plugins/aws/customResources/resources/s3/lib/permissions.js index 08128b0b15c..614494e448b 100644 --- a/lib/plugins/aws/customResources/resources/s3/lib/permissions.js +++ b/lib/plugins/aws/customResources/resources/s3/lib/permissions.js @@ -21,7 +21,7 @@ function addPermission(config) { SourceArn: `arn:${partition}:s3:::${bucketName}`, SourceAccount: accountId, }; - return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'addPermission', payload); } function removePermission(config) { @@ -30,7 +30,7 @@ function removePermission(config) { FunctionName: functionName, StatementId: getStatementId(functionName, bucketName), }; - return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload).promise(); + return awsRequest({ name: 'Lambda', params: { region } }, 'removePermission', payload); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index 809c51eb25c..a92faacfae4 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -109,7 +109,7 @@ const getServiceInstance = nameInput => { return new AWS[name](params); }; -function awsRequest(service, method, ...args) { +async function awsRequest(service, method, ...args) { const serviceInstance = getServiceInstance(service); const callAws = requestTry => { return serviceInstance[method](...args) From 803324df4ad36826a8ec1dc014a908e0528d69dc Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Wed, 7 Oct 2020 13:35:12 +0530 Subject: [PATCH 13/14] reverted changes of try/catch for APIGateway update account --- .../apiGatewayCloudWatchRole/handler.js | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js index 7d80df8672c..eb33c4318a4 100644 --- a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js +++ b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js @@ -1,6 +1,6 @@ 'use strict'; -const { awsRequest } = require('../utils'); +const { awsRequest, wait } = require('../utils'); const { getEnvironment, handlerWrapper } = require('../utils'); function handler(event, context) { @@ -74,16 +74,26 @@ async function create(event, context) { // there's nothing to do if the role is the same if (roleArn === assignedRoleArn) return null; - const updateAccount = async () => { - await awsRequest({ name: 'APIGateway', params: { region } }, 'updateAccount', { - patchOperations: [ - { - op: 'replace', - path: '/cloudwatchRoleArn', - value: roleArn, - }, - ], - }); + const updateAccount = async (counter = 1) => { + try { + await awsRequest({ name: 'APIGateway', params: { region } }, 'updateAccount', { + patchOperations: [ + { + op: 'replace', + path: '/cloudwatchRoleArn', + value: roleArn, + }, + ], + }); + } catch (error) { + if (counter < 10) { + // Observed fails with errors marked as non-retryable. Still they're outcome of + // temporary state where just created AWS role is not being ready for use (yet) + await wait(10000); + return updateAccount(++counter); + } + throw error; + } return null; }; From ea0863f53c5cfb28f492ac3887199137f81984a0 Mon Sep 17 00:00:00 2001 From: pratik-vii Date: Wed, 7 Oct 2020 15:15:36 +0530 Subject: [PATCH 14/14] added async/await for awsRequest - changed backoff time to (2000 + Math.random() * 1000) --- .../aws/customResources/resources/utils.js | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index a92faacfae4..d1c046bc5c1 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -92,7 +92,7 @@ function wait(ms) { } const MAX_AWS_REQUEST_TRY = (() => { - const DEFAULT_MAX_AWS_REQUEST_TRY = 4; + const DEFAULT_MAX_AWS_REQUEST_TRY = 8; const userValue = Number(process.env.SLS_AWS_REQUEST_MAX_RETRIES); return userValue >= 0 ? userValue : DEFAULT_MAX_AWS_REQUEST_TRY; })(); @@ -111,23 +111,20 @@ const getServiceInstance = nameInput => { async function awsRequest(service, method, ...args) { const serviceInstance = getServiceInstance(service); - const callAws = requestTry => { - return serviceInstance[method](...args) - .promise() - .then( - result => result, - error => { - if ( - requestTry < MAX_AWS_REQUEST_TRY && - error && - (error.statusCode === 429 || - (error.retryable && error.statusCode !== 403 && error.code !== 'CredentialsError')) - ) { - return wait(4000 + Math.random() * 3000).then(() => callAws(++requestTry)); - } - throw error; - } - ); + const callAws = async requestTry => { + try { + return await serviceInstance[method](...args).promise(); + } catch (error) { + if ( + requestTry < MAX_AWS_REQUEST_TRY && + error && + (error.statusCode === 429 || error.retryable) + ) { + await wait(2000 + Math.random() * 1000); + return callAws(++requestTry); + } + throw error; + } }; return callAws(1); }