From 2e7ec82153724b2c2dc6b10ff47943341dd0e8f5 Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Wed, 13 Oct 2021 14:35:26 -0300 Subject: [PATCH 1/3] fix: use provider.request instead of custom ec2 instance --- index.js | 54 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/index.js b/index.js index fab1bb2..afd8408 100644 --- a/index.js +++ b/index.js @@ -1,35 +1,46 @@ "use strict"; -const d = require("d") - , lazy = require("d/lazy") - , BbPromise = require("bluebird") - , Ec2 = require("aws-sdk/clients/ec2") - , { inspect } = require("util"); +const d = require("d") + , lazy = require("d/lazy") + , BbPromise = require("bluebird") + , { inspect } = require("util"); const noopPromise = Promise.resolve(); class ServerlessPluginVpcEniCleanup { constructor(serverless) { this.serverless = serverless; + this.provider = this.serverless.getProvider(this.serverless.service.provider.name); this.hooks = { "before:remove:remove": this.cleanup.bind(this), "after:remove:remove": () => this.isDisabled = true }; } + + describeNetworkInterfaces(args) { + return this.provider.request("EC2", "describeNetworkInterfaces", args); + } + + detachNetworkInterface(args) { + return this.provider.request("EC2", "detachNetworkInterface", args); + } + + deleteNetworkInterface(args) { + return this.provider.request("EC2", "deleteNetworkInterface", args); + } + cleanup() { if (this.isDisabled) return; // Intentionally we do not return promise, // as we want to have polling done in parallel in the background BbPromise.all( this.functionNames.map(functionName => - this.ec2 - .describeNetworkInterfaces({ - Filters: [ - { Name: "requester-id", Values: [`*:${ functionName }`] }, - { Name: "description", Values: ["AWS Lambda VPC ENI*"] } - ] - }) - .promise() + this.describeNetworkInterfaces({ + Filters: [ + { Name: "requester-id", Values: [`*:${ functionName }`] }, + { Name: "description", Values: ["AWS Lambda VPC ENI*"] } + ] + }) .then( result => Promise.all( @@ -43,23 +54,22 @@ class ServerlessPluginVpcEniCleanup { setTimeout(this.cleanup.bind(this), this.cleanupInterval); }); } + _deleteNetworkInterface(networkInterface, functionName) { const interfaceId = networkInterface.NetworkInterfaceId; let detachmentPromise = noopPromise; if (networkInterface.Attachment) { - detachmentPromise = this.ec2 - .detachNetworkInterface({ AttachmentId: networkInterface.Attachment.AttachmentId }) - .promise(); + detachmentPromise = this.detachNetworkInterface({ AttachmentId: networkInterface.Attachment.AttachmentId }); } return detachmentPromise.then( () => - this.ec2.deleteNetworkInterface({ NetworkInterfaceId: interfaceId }).promise().then( + this.deleteNetworkInterface({ NetworkInterfaceId: interfaceId }).then( () => this.serverless.cli.log( "VPC ENI Cleanup: " + - `Deleted ${ interfaceId } ` + - `ENI of ${ functionName } ` + - "VPC function" + `Deleted ${ interfaceId } ` + + `ENI of ${ functionName } ` + + "VPC function" ), error => { if (error.code === "InvalidParameterValue") { @@ -78,6 +88,7 @@ class ServerlessPluginVpcEniCleanup { this.handleError.bind(this) ); } + handleError(error) { this.isDisabled = true; this.serverless.cli.log(`VPC ENI Cleanup: Error: ${ error.message }\n${ inspect(error) }`); @@ -92,9 +103,6 @@ Object.defineProperties( cleanupInterval: d(5000) // Attempt ENI cleanup every 5 seconds }, lazy({ - ec2: d(function () { - return new Ec2({ region: this.serverless.service.provider.region }); - }), functionNames: d(function () { return Object.keys(this.serverless.service.functions).map( functionName => this.serverless.service.functions[functionName].name From 6552d5b470f43fcd6d9e4ca6fbb67a8df6bffb67 Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Thu, 14 Oct 2021 11:45:20 -0300 Subject: [PATCH 2/3] formatting suggestions --- index.js | 85 ++++++++++++++++++++++++++------------------------------ 1 file changed, 39 insertions(+), 46 deletions(-) diff --git a/index.js b/index.js index afd8408..7f25527 100644 --- a/index.js +++ b/index.js @@ -1,8 +1,8 @@ "use strict"; -const d = require("d") - , lazy = require("d/lazy") - , BbPromise = require("bluebird") +const d = require("d") + , lazy = require("d/lazy") + , BbPromise = require("bluebird") , { inspect } = require("util"); const noopPromise = Promise.resolve(); @@ -16,31 +16,19 @@ class ServerlessPluginVpcEniCleanup { "after:remove:remove": () => this.isDisabled = true }; } - - describeNetworkInterfaces(args) { - return this.provider.request("EC2", "describeNetworkInterfaces", args); - } - - detachNetworkInterface(args) { - return this.provider.request("EC2", "detachNetworkInterface", args); - } - - deleteNetworkInterface(args) { - return this.provider.request("EC2", "deleteNetworkInterface", args); - } - cleanup() { if (this.isDisabled) return; // Intentionally we do not return promise, // as we want to have polling done in parallel in the background BbPromise.all( this.functionNames.map(functionName => - this.describeNetworkInterfaces({ - Filters: [ - { Name: "requester-id", Values: [`*:${ functionName }`] }, - { Name: "description", Values: ["AWS Lambda VPC ENI*"] } - ] - }) + this.provider + .request("EC2", "describeNetworkInterfaces", { + Filters: [ + { Name: "requester-id", Values: [`*:${ functionName }`] }, + { Name: "description", Values: ["AWS Lambda VPC ENI*"] } + ] + }) .then( result => Promise.all( @@ -54,41 +42,46 @@ class ServerlessPluginVpcEniCleanup { setTimeout(this.cleanup.bind(this), this.cleanupInterval); }); } - _deleteNetworkInterface(networkInterface, functionName) { const interfaceId = networkInterface.NetworkInterfaceId; let detachmentPromise = noopPromise; if (networkInterface.Attachment) { - detachmentPromise = this.detachNetworkInterface({ AttachmentId: networkInterface.Attachment.AttachmentId }); + detachmentPromise = this.provider + .request( + "EC2", + "detachNetworkInterface", + { AttachmentId: networkInterface.Attachment.AttachmentId } + ); } return detachmentPromise.then( () => - this.deleteNetworkInterface({ NetworkInterfaceId: interfaceId }).then( - () => - this.serverless.cli.log( - "VPC ENI Cleanup: " + - `Deleted ${ interfaceId } ` + - `ENI of ${ functionName } ` + - "VPC function" - ), - error => { - if (error.code === "InvalidParameterValue") { - // Network interface is currently in use - // Skip on this error, as it may happen - // few times for given interface - return; - } - if (error.code === "InvalidNetworkInterfaceID.NotFound") { - // Interface was already deleted - return; + this.provider + .request("EC2", "deleteNetworkInterface", { NetworkInterfaceId: interfaceId }) + .then( + () => + this.serverless.cli.log( + "VPC ENI Cleanup: " + + `Deleted ${ interfaceId } ` + + `ENI of ${ functionName } ` + + "VPC function" + ), + error => { + if (error.code === "InvalidParameterValue") { + // Network interface is currently in use + // Skip on this error, as it may happen + // few times for given interface + return; + } + if (error.code === "InvalidNetworkInterfaceID.NotFound") { + // Interface was already deleted + return; + } + this.handleError(error); } - this.handleError(error); - } - ), + ), this.handleError.bind(this) ); } - handleError(error) { this.isDisabled = true; this.serverless.cli.log(`VPC ENI Cleanup: Error: ${ error.message }\n${ inspect(error) }`); From e8b2b97f159ece8f1a8f9e653dfb460105818b01 Mon Sep 17 00:00:00 2001 From: Marcelo Luiz Onhate Date: Thu, 14 Oct 2021 13:15:52 -0300 Subject: [PATCH 3/3] fix tests --- test/index.js | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/test/index.js b/test/index.js index d21c252..2a1ac85 100644 --- a/test/index.js +++ b/test/index.js @@ -1,22 +1,29 @@ "use strict"; const noop = require("es5-ext/function/noop") - , proxyquire = require("proxyquire") , sinon = require("sinon") , test = require("tape") - , Plugin = proxyquire("../", { "aws-sdk/clients/ec2": require("./__ec2-mock") }); + , Ec2 = require("./__ec2-mock") + , Plugin = require("../"); test("Serverless Plugin VPC ENI Cleanup", t => { - const serverlessMock = { + const serverlessMock = ec2 => ({ cli: { log: noop }, - service: { functions: { foo: { name: "foo" } }, provider: { region: "eu-west-1" } } - }; + service: { functions: { foo: { name: "foo" } }, provider: { region: "eu-west-1" } }, + getProvider() { + return { + request(service, method, params) { + return ec2[method](params).promise(); + } + }; + } + }); t.test("Success run", t => { - const plugin = new Plugin(serverlessMock); + const ec2 = new Ec2(); + const plugin = new Plugin(serverlessMock(ec2)); plugin.cleanupInterval = 0; plugin.handleError = error => { throw error; }; - const { ec2 } = plugin; sinon.spy(ec2, "describeNetworkInterfaces"); sinon.spy(ec2, "detachNetworkInterface"); @@ -40,9 +47,10 @@ test("Serverless Plugin VPC ENI Cleanup", t => { }); t.test("Error run", t => { - const plugin = new Plugin(serverlessMock); + const ec2 = new Ec2(); + const plugin = new Plugin(serverlessMock(ec2)); plugin.cleanupInterval = 0; - plugin.ec2.errors = [new Error("Unknown error")]; + ec2.errors = [new Error("Unknown error")]; sinon.spy(plugin, "handleError");