From a3ebc01f2bcd6484cfd790bd576bc12962f1b2ff Mon Sep 17 00:00:00 2001 From: Pratik Prajapati Date: Wed, 7 Oct 2020 16:52:16 +0530 Subject: [PATCH] feat(AWS Deploy): Retry retryable SDK errors in custom resources (#8338) --- .../apiGatewayCloudWatchRole/handler.js | 76 +++++++++---------- .../cognitoUserPool/lib/permissions.js | 12 ++- .../resources/cognitoUserPool/lib/userPool.js | 53 +++++++------ .../resources/eventBridge/lib/eventBridge.js | 74 +++++++----------- .../resources/eventBridge/lib/permissions.js | 12 ++- .../resources/s3/lib/bucket.js | 26 ++++--- .../resources/s3/lib/permissions.js | 8 +- .../aws/customResources/resources/utils.js | 40 ++++++++++ 8 files changed, 159 insertions(+), 142 deletions(-) diff --git a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js index 08722332625..eb33c4318a4 100644 --- a/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js +++ b/lib/plugins/aws/customResources/resources/apiGatewayCloudWatchRole/handler.js @@ -1,8 +1,7 @@ 'use strict'; -const ApiGateway = require('aws-sdk/clients/apigateway'); -const Iam = require('aws-sdk/clients/iam'); -const { getEnvironment, handlerWrapper, wait } = require('../utils'); +const { awsRequest, wait } = require('../utils'); +const { getEnvironment, handlerWrapper } = require('../utils'); function handler(event, context) { if (event.RequestType === 'Create') { @@ -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') + ).cloudwatchRoleArn; let roleArn = `arn:${partition}:iam::${accountId}:role/serverlessApiGatewayCloudWatchRole`; if (RoleArn) { @@ -32,33 +32,29 @@ 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()) + return (await awsRequest('IAM', 'listAttachedRolePolicies', { RoleName: roleName })) .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, + }); return []; } throw error; @@ -68,12 +64,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, + }); } } @@ -82,17 +76,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, + }, + ], + }); } catch (error) { if (counter < 10) { // Observed fails with errors marked as non-retryable. Still they're outcome of diff --git a/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js b/lib/plugins/aws/customResources/resources/cognitoUserPool/lib/permissions.js index d8823334321..b971f9d56e9 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); } 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); } 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..c53a0caee9d 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,25 +28,25 @@ 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) - .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; - }); + if (nextToken) payload.NextToken = nextToken; + return awsRequest( + { name: 'CognitoIdentityServiceProvider', params: { region } }, + 'listUserPools', + payload + ).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(); @@ -54,20 +54,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, + }) ); } 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 +84,16 @@ function updateConfiguration(config) { LambdaConfig, }); - return cognito.updateUserPool(updatedConfig).promise(); + return awsRequest( + { name: 'CognitoIdentityServiceProvider', params: { region } }, + 'updateUserPool', + updatedConfig + ); }); } function removeConfiguration(config) { const { lambdaArn, region } = config; - const cognito = new CognitoIdentityServiceProvider({ region }); return getConfiguration(config).then(res => { const UserPoolId = res.UserPool.Id; @@ -111,7 +110,11 @@ function removeConfiguration(config) { LambdaConfig, }); - return cognito.updateUserPool(updatedConfig).promise(); + return awsRequest( + { name: 'CognitoIdentityServiceProvider', params: { region } }, + 'updateUserPool', + updatedConfig + ); }); } diff --git a/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js b/lib/plugins/aws/customResources/resources/eventBridge/lib/eventBridge.js index b813afce0f7..ca02332a5e7 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, + }); } 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, + }); } 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', + }); } 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, + }); } 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], + }) ); } 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, + }); } 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..535dc5fdca5 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); } 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); } 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 b8a36a6b38b..51985e47a1b 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,19 @@ 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) - .promise() - .then(data => data); + return awsRequest( + { name: 'S3', params: { region } }, + 'getBucketNotificationConfiguration', + payload + ).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 +73,16 @@ function updateConfiguration(config) { Bucket, NotificationConfiguration, }; - return s3.putBucketNotificationConfiguration(payload).promise(); + return awsRequest( + { name: 'S3', params: { region } }, + 'putBucketNotificationConfiguration', + payload + ); }); } function removeConfiguration(config) { const { functionName, bucketName, region } = config; - const s3 = new AWS.S3({ region }); const Bucket = bucketName; return getConfiguration(config).then(NotificationConfiguration => { @@ -93,7 +95,11 @@ function removeConfiguration(config) { Bucket, NotificationConfiguration, }; - return s3.putBucketNotificationConfiguration(payload).promise(); + return awsRequest( + { name: 'S3', params: { region } }, + 'putBucketNotificationConfiguration', + payload + ); }); } diff --git a/lib/plugins/aws/customResources/resources/s3/lib/permissions.js b/lib/plugins/aws/customResources/resources/s3/lib/permissions.js index e3b6b778c32..614494e448b 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); } 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); } module.exports = { diff --git a/lib/plugins/aws/customResources/resources/utils.js b/lib/plugins/aws/customResources/resources/utils.js index e855cd5b685..d1c046bc5c1 100644 --- a/lib/plugins/aws/customResources/resources/utils.js +++ b/lib/plugins/aws/customResources/resources/utils.js @@ -1,5 +1,6 @@ 'use strict'; +const AWS = require('aws-sdk'); const https = require('https'); const url = require('url'); @@ -90,6 +91,44 @@ function wait(ms) { return new Promise(resolve => setTimeout(resolve, ms)); } +const MAX_AWS_REQUEST_TRY = (() => { + 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; +})(); + +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); +}; + +async function awsRequest(service, method, ...args) { + const serviceInstance = getServiceInstance(service); + 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); +} + module.exports = { logger, response, @@ -97,4 +136,5 @@ module.exports = { getLambdaArn, handlerWrapper, wait, + awsRequest, };