From bee58cd598f24c498eeffab456a9a911d5f8dbca Mon Sep 17 00:00:00 2001 From: Austen Collins Date: Wed, 9 Dec 2015 15:36:22 -0800 Subject: [PATCH] EndpointBuildApiGateway: Fix bug that prevents deploying endpoints with duplicate resource names (users/users), remove delay to speed up API Gateway depoyment, clean code --- lib/actions/EndpointBuildApiGateway.js | 246 +++++++++++++------------ 1 file changed, 133 insertions(+), 113 deletions(-) diff --git a/lib/actions/EndpointBuildApiGateway.js b/lib/actions/EndpointBuildApiGateway.js index 62a5958cc..acc3115c7 100644 --- a/lib/actions/EndpointBuildApiGateway.js +++ b/lib/actions/EndpointBuildApiGateway.js @@ -11,6 +11,7 @@ const SPlugin = require('../ServerlessPlugin'), SUtils = require('../utils/index'), BbPromise = require('bluebird'), path = require('path'), + async = require('async'), fs = require('fs'), os = require('os'); @@ -90,13 +91,12 @@ class Builder { + evt.region.region + '.amazonaws.com/' + evt.stage - + '/' + evt.endpoint.path; SUtils.sDebug( '"' + evt.stage - + '" successfully deployed endpoint to API Gateway in the region "' + + '" successfully built endpoint on API Gateway in the region "' + evt.region.region + '". Access it via ' + evt.endpoint.method @@ -152,8 +152,9 @@ class Builder { throw new SError('Endpoint does not have a "responses" property'); } - // Sanitize path - if (evt.endpoint.path.charAt(0) === '/') evt.endpoint.path = evt.endpoint.path.substring(1); + // Sanitize path - Remove excessive forward slashes + if (evt.endpoint.path.charAt(0) !== '/') evt.endpoint.path = '/' + evt.endpoint.path; + if (evt.endpoint.path.charAt(evt.endpoint.path.length) === '/') evt.endpoint.path = evt.endpoint.path.slice(0, -1); // Sanitize method evt.endpoint.method = evt.endpoint.method.toUpperCase(); @@ -206,11 +207,11 @@ class Builder { evt.apiResources = response.items; // Get Parent Resource ID - for (let i = 0; i < evt.apiResources.length; i++) { - if (evt.apiResources[i].path === '/') { - evt.apiParentResourceId = evt.apiResources[i].id; - } - } + //for (let i = 0; i < evt.apiResources.length; i++) { + // if (evt.apiResources[i].path === '/') { + // evt.resourceParent = evt.apiResources[i]; + // } + //} SUtils.sDebug( '"' @@ -226,125 +227,148 @@ class Builder { /** * Create Endpoint Resources - * @returns {Promise} - * @private */ _createEndpointResources(evt) { - let _this = this, - endpoint = evt.endpoint, - eResources = endpoint.path.split('/'); + let _this = this; /** - * Private Function to find resource - * @param resource - * @param parent - * @returns {*} + * Find Parent + * - We always want to provide the parent resource on the EVENT object. + * - Here is a private, reusable function to find and add it */ - let findEndpointResource = function(resource, parent) { + let findParent = function(resource) { - // Replace slashes in resource - resource = resource.replace(/\//g, ''); - let index = eResources.indexOf(resource), - resourcePath, - resourceIndex; - - if (parent) { - index = index - 1; - resource = eResources[index]; - } - - if (index < 0) { - resourcePath = '/'; + let parentPath = resource.split('/'); + if (parentPath.length > 1) { + parentPath.pop(); + parentPath = '/' + parentPath.join('/'); } else { - resourceIndex = endpoint.path.indexOf(resource); - resourcePath = '/' + endpoint.path.substring(0, resourceIndex + resource.length); + parentPath = '/'; } - // If resource has already been created, skip it for (let i = 0; i < evt.apiResources.length; i++) { - - // Check if path matches, in case there are duplicate resources (users/list, org/list) - if (evt.apiResources[i].path === resourcePath) { - return evt.apiResources[i]; + if (evt.apiResources[i].path === parentPath) { + evt.resourceParent = evt.apiResources[i]; + continue; } } + + return evt.resourceParent; }; - return BbPromise.try(function() { - return eResources; + // Check paths to see if resources need building + for (let i = 0; i < evt.apiResources.length; i++) { + if (evt.apiResources[i].path === evt.endpoint.path) { + evt.resource = evt.apiResources[i]; + break; + } + } - }).each(function(eResource) { + // If all Endpoint resources exist already, load parent resource, skip the rest of this function + if (evt.resource) { + findParent(evt.resource); + return BbPromise.resolve(evt); + } - // Remove slashes in resource - eResource = eResource.replace(/\//g, ''); + let eResources = evt.endpoint.path.split('/'); + eResources[0] = '/'; // Our split removes the initial '/' and leaves an empty string, replace it - // If resource exists, skip it - let resource = findEndpointResource(eResource); - if (resource) return resource; + return new BbPromise(function(resolve, reject) { - // Get Parent Resource - evt.apiParentResourceId = findEndpointResource(eResource, true).id; + // Loop through each resource in this Endpoint and create it if it is missing. + let incrementedPath = ''; + async.eachSeries(eResources, function(eResource, cb) { - let params = { - parentId: evt.apiParentResourceId, /* required */ - pathPart: eResource, /* required */ - restApiId: evt.region.restApiId /* required */ - }; + // Build the path w/ new resource on each iteration + if (incrementedPath === '') { + incrementedPath = eResource; + } else if (incrementedPath === '/') { + incrementedPath = incrementedPath + eResource; + } else { + incrementedPath = incrementedPath + '/' + eResource; + } - // Create Resource - return _this.ApiGateway.createResourcePromised(params) - .then(function(response) { + // If exists in APIG resources, skip this + let parentPath = ''; + let resourceExists = false; - // Add resource to _this.resources and callback - evt.apiResources.push(response); + for (let i = 0; i < evt.apiResources.length; i++) { + // If incrementedPath exists in the API Resource path, store the longest string, this is the parent. + if (incrementedPath.indexOf(evt.apiResources[i].path) !== -1) { + if (evt.apiResources[i].path.length > parentPath.length) { + evt.resourceParent = evt.apiResources[i]; + } + } - SUtils.sDebug( - '"' - + evt.stage + ' - ' - + evt.region.region - + ' - ' + evt.endpoint.path + '": ' - + 'created resource: ' - + response.pathPart); - }); + // Resource exists, save it to Event object, break loop + if (evt.apiResources[i].path === incrementedPath) { + resourceExists = true; + continue; + } + } - }).then(function() { + // Resource exists, skip this iteration + if (resourceExists) return cb(); - // Attach the last resource to endpoint for later use - let endpointResource = endpoint.path.split('/').pop().replace(/\//g, ''); - evt.resource = findEndpointResource(endpointResource); + // Resource doesn't exist, so make it + let params = { + parentId: evt.resourceParent.id, /* required */ + pathPart: eResource, /* required */ + restApiId: evt.region.restApiId /* required */ + }; - return evt; + // Create Resource + return _this.ApiGateway.createResourcePromised(params) + .then(function(response) { + + // Save resource + evt.resource = response; + + // Add resource to _this.resources and callback + evt.apiResources.push(response); + + SUtils.sDebug( + '"' + + evt.stage + ' - ' + + evt.region.region + + ' - ' + evt.endpoint.path + '": ' + + 'created resource: ' + + response.pathPart); + + // Return callback to iterate loop + return cb(); + }); + }, function() { + return resolve(evt); + }); // async.eachSeries }); } /** * Create Endpoint Method - * @returns {Promise} - * @private */ _createEndpointMethod(evt) { - let _this = this, - endpoint = evt.endpoint, + let _this = this, requestParameters = {}; // If Request Params, add them - if (endpoint.RequestParameters) { + if (evt.endpoint.RequestParameters) { // Format them per APIG API's Expectations - for (let prop in endpoint.requestParameters) { - let requestParam = endpoint.requestParameters[prop]; + for (let prop in evt.endpoint.requestParameters) { + let requestParam = evt.endpoint.requestParameters[prop]; requestParameters[requestParam] = true; } } let params = { - httpMethod: endpoint.method, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId /* required */ }; @@ -360,7 +384,7 @@ class Builder { } let params = { - httpMethod: endpoint.method, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId /* required */ }; @@ -369,12 +393,12 @@ class Builder { .then(function() { let params = { - authorizationType: endpoint.authorizationType, /* required */ - httpMethod: endpoint.method, /* required */ + authorizationType: evt.endpoint.authorizationType, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId, /* required */ - apiKeyRequired: endpoint.apiKeyRequired, - requestModels: endpoint.requestModels, + apiKeyRequired: evt.endpoint.apiKeyRequired, + requestModels: evt.endpoint.requestModels, requestParameters: requestParameters }; @@ -385,18 +409,17 @@ class Builder { // Method does not exist. Create it. let params = { - authorizationType: endpoint.authorizationType, /* required */ - httpMethod: endpoint.method, /* required */ + authorizationType: evt.endpoint.authorizationType, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId, /* required */ - apiKeyRequired: endpoint.apiKeyRequired, - requestModels: endpoint.requestModels, + apiKeyRequired: evt.endpoint.apiKeyRequired, + requestModels: evt.endpoint.requestModels, requestParameters: requestParameters }; return _this.ApiGateway.putMethodPromised(params); }) - .delay(250) // API Gateway takes time to delete Methods. Might have to increase this. .then(function(response) { SUtils.sDebug( @@ -405,7 +428,7 @@ class Builder { + evt.region.region + ' - ' + evt.endpoint.path + '": ' + 'created method: ' - + endpoint.method); + + evt.endpoint.method); return evt; }); @@ -417,8 +440,7 @@ class Builder { _createEndpointIntegration(evt) { - let _this = this, - endpoint = evt.endpoint; + let _this = this; // Alias Lambda, default ot $LATEST let alias; @@ -426,12 +448,12 @@ class Builder { else alias = '${stageVariables.functionAlias}'; let params = { - httpMethod: endpoint.method, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId, /* required */ type: 'AWS', /* required */ - cacheKeyParameters: endpoint.cacheKeyParameters || [], - cacheNamespace: endpoint.cacheNamespace || null, + cacheKeyParameters: evt.endpoint.cacheKeyParameters || [], + cacheNamespace: evt.endpoint.cacheNamespace || null, // Due to a bug in API Gateway reported here: https://github.com/awslabs/aws-apigateway-swagger-importer/issues/41 // Specifying credentials within API Gateway causes extra latency (~500ms) // Until API Gateway is fixed, we need to make a separate call to Lambda to add credentials to API Gateway @@ -439,8 +461,8 @@ class Builder { // _this._regionJson.iamRoleArnApiGateway credentials: null, integrationHttpMethod: 'POST', - requestParameters: endpoint.requestParameters || {}, - requestTemplates: endpoint.requestTemplates || {}, + requestParameters: evt.endpoint.requestParameters || {}, + requestTemplates: evt.endpoint.requestTemplates || {}, uri: 'arn:aws:apigateway:' // Make ARN for apigateway - lambda + evt.region.region + ':lambda:path/2015-03-31/functions/arn:aws:lambda:' @@ -483,13 +505,12 @@ class Builder { _createEndpointMethodResponses(evt) { - let _this = this, - endpoint = evt.endpoint; + let _this = this; return BbPromise.try(function() { // Collect Response Keys - if (endpoint.responses) return Object.keys(endpoint.responses); + if (evt.endpoint.responses) return Object.keys(evt.endpoint.responses); else return []; }) @@ -497,7 +518,7 @@ class Builder { // Iterate through each response to be created - let thisResponse = endpoint.responses[responseKey]; + let thisResponse = evt.endpoint.responses[responseKey]; let responseParameters = {}; let responseModels = {}; @@ -519,7 +540,7 @@ class Builder { } let params = { - httpMethod: endpoint.method, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId, /* required */ statusCode: thisResponse.statusCode, /* required */ @@ -554,18 +575,17 @@ class Builder { _createEndpointMethodIntegResponses(evt) { - let _this = this, - endpoint = evt.endpoint; + let _this = this; return BbPromise.try(function() { // Collect Response Keys - if (endpoint.responses) return Object.keys(endpoint.responses); + if (evt.endpoint.responses) return Object.keys(evt.endpoint.responses); else return []; }) .each(function(responseKey) { - let thisResponse = endpoint.responses[responseKey]; + let thisResponse = evt.endpoint.responses[responseKey]; // Add Response Parameters let responseParameters = thisResponse.responseParameters || {}; @@ -577,7 +597,7 @@ class Builder { let selectionPattern = thisResponse.selectionPattern || (responseKey === 'default' ? null : responseKey); let params = { - httpMethod: endpoint.method, /* required */ + httpMethod: evt.endpoint.method, /* required */ resourceId: evt.resource.id, /* required */ restApiId: evt.region.restApiId, /* required */ statusCode: thisResponse.statusCode, /* required */ @@ -703,8 +723,8 @@ class Builder { endpoint = evt.endpoint; // Sanitize Path - Remove first and last slashes, if any - endpoint.path = endpoint.path.split('/'); - endpoint.path = endpoint.path.join('/'); + evt.endpoint.path = evt.endpoint.path.split('/'); + evt.endpoint.path = evt.endpoint.path.join('/'); // Create new access policy statement let params = {}; @@ -719,9 +739,9 @@ class Builder { + ':' + evt.region.restApiId + '/*/' - + endpoint.method + + evt.endpoint.method + '/' - + endpoint.path; + + evt.endpoint.path; return _this.Lambda.addPermissionPromised(params) .then(function(data) { @@ -737,7 +757,7 @@ class Builder { }) .catch(function(error) { - return endpoint; + throw new SError(error.message); }); }