From 584579e80bdc0cb4d5288b19920feffd490afbbb Mon Sep 17 00:00:00 2001 From: Simon-Pierre Gingras Date: Mon, 20 May 2019 06:13:42 -0700 Subject: [PATCH] fix(lambda): compare Runtimes by value instead of identity (#2543) Different Runtime objects that represent the same runtime now compare as equal. This is important when using Layers, which declare compatible runtimes. --- packages/@aws-cdk/aws-lambda/lib/function.ts | 2 +- packages/@aws-cdk/aws-lambda/lib/runtime.ts | 6 ++ .../@aws-cdk/aws-lambda/package-lock.json | 6 ++ packages/@aws-cdk/aws-lambda/package.json | 4 +- .../@aws-cdk/aws-lambda/test/test.function.ts | 76 +++++++++++++++++++ .../@aws-cdk/aws-lambda/test/test.runtime.ts | 70 +++++++++++++++++ 6 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 packages/@aws-cdk/aws-lambda/test/test.function.ts create mode 100644 packages/@aws-cdk/aws-lambda/test/test.runtime.ts diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index 16b0d468e3d06..73857c38d984d 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -479,7 +479,7 @@ export class Function extends FunctionBase { if (this.layers.length === 5) { throw new Error('Unable to add layer: this lambda function already uses 5 layers.'); } - if (layer.compatibleRuntimes && layer.compatibleRuntimes.indexOf(this.runtime) === -1) { + if (layer.compatibleRuntimes && !layer.compatibleRuntimes.find(runtime => runtime.equals(this.runtime))) { const runtimes = layer.compatibleRuntimes.map(runtime => runtime.name).join(', '); throw new Error(`This lambda function uses a runtime that is incompatible with this layer (${this.runtime.name} is not in [${runtimes}])`); } diff --git a/packages/@aws-cdk/aws-lambda/lib/runtime.ts b/packages/@aws-cdk/aws-lambda/lib/runtime.ts index 809938cf4b9a6..98aa0ccb4a747 100644 --- a/packages/@aws-cdk/aws-lambda/lib/runtime.ts +++ b/packages/@aws-cdk/aws-lambda/lib/runtime.ts @@ -69,4 +69,10 @@ export class Runtime { public toString(): string { return this.name; } + + public equals(other: Runtime): boolean { + return other.name === this.name && + other.family === this.family && + other.supportsInlineCode === this.supportsInlineCode; + } } diff --git a/packages/@aws-cdk/aws-lambda/package-lock.json b/packages/@aws-cdk/aws-lambda/package-lock.json index 899890f49ed6b..85064aeab0cd2 100644 --- a/packages/@aws-cdk/aws-lambda/package-lock.json +++ b/packages/@aws-cdk/aws-lambda/package-lock.json @@ -46,6 +46,12 @@ "integrity": "sha512-9F70bcb2oBGZUZCyisE1Ap3vwgt04uiSmr4s9mhQf89vBrttgraBs2Rc8l9YrKiemCndCH7wxnYNWk5qEuv6rA==", "dev": true }, + "@types/lodash": { + "version": "4.14.128", + "resolved": "https://registry.npmjs.org/@types/lodash/-/lodash-4.14.128.tgz", + "integrity": "sha512-09tfcHEZ+UEMouF6wRE/h+rqKiujbEUk8ftZsA+eKeIISt8uotgoLEZpfb+f8Qxli572Q0CBIFMVHm4KQr15Hg==", + "dev": true + }, "@types/nock": { "version": "9.3.1", "resolved": "https://registry.npmjs.org/@types/nock/-/nock-9.3.1.tgz", diff --git a/packages/@aws-cdk/aws-lambda/package.json b/packages/@aws-cdk/aws-lambda/package.json index 9ab3e71f08f47..5dd30f55f1956 100644 --- a/packages/@aws-cdk/aws-lambda/package.json +++ b/packages/@aws-cdk/aws-lambda/package.json @@ -66,6 +66,7 @@ "@types/aws-lambda": "^8.10.24", "@types/nock": "^9.3.1", "@types/sinon": "^7.0.11", + "@types/lodash": "^4.14.128", "aws-sdk": "^2.438.0", "aws-sdk-mock": "^4.4.0", "cdk-build-tools": "^0.31.0", @@ -73,7 +74,8 @@ "cfn2ts": "^0.31.0", "nock": "^10.0.6", "pkglint": "^0.31.0", - "sinon": "^7.3.1" + "sinon": "^7.3.1", + "lodash": "^4.17.11" }, "dependencies": { "@aws-cdk/assets": "^0.31.0", diff --git a/packages/@aws-cdk/aws-lambda/test/test.function.ts b/packages/@aws-cdk/aws-lambda/test/test.function.ts new file mode 100644 index 0000000000000..7ede8b0fcf085 --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/test.function.ts @@ -0,0 +1,76 @@ +import s3 = require('@aws-cdk/aws-s3'); +import cdk = require('@aws-cdk/cdk'); +import _ = require('lodash'); +import {Test, testCase} from 'nodeunit'; +import lambda = require('../lib'); + +export = testCase({ + 'add incompatible layer'(test: Test) { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack'); + const bucket = new s3.Bucket(stack, 'Bucket'); + const code = new lambda.S3Code(bucket, 'ObjectKey'); + + const func = new lambda.Function(stack, 'myFunc', { + runtime: lambda.Runtime.Python37, + handler: 'index.handler', + code, + }); + const layer = new lambda.LayerVersion(stack, 'myLayer', { + code, + compatibleRuntimes: [lambda.Runtime.NodeJS] + }); + + // THEN + test.throws(() => func.addLayer(layer), + /This lambda function uses a runtime that is incompatible with this layer/); + + test.done(); + }, + 'add compatible layer'(test: Test) { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack'); + const bucket = new s3.Bucket(stack, 'Bucket'); + const code = new lambda.S3Code(bucket, 'ObjectKey'); + + const func = new lambda.Function(stack, 'myFunc', { + runtime: lambda.Runtime.Python37, + handler: 'index.handler', + code, + }); + const layer = new lambda.LayerVersion(stack, 'myLayer', { + code, + compatibleRuntimes: [lambda.Runtime.Python37] + }); + + // THEN + // should not throw + func.addLayer(layer); + + test.done(); + }, + 'add compatible layer for deep clone'(test: Test) { + // GIVEN + const stack = new cdk.Stack(undefined, 'TestStack'); + const bucket = new s3.Bucket(stack, 'Bucket'); + const code = new lambda.S3Code(bucket, 'ObjectKey'); + + const runtime = lambda.Runtime.Python37; + const func = new lambda.Function(stack, 'myFunc', { + runtime, + handler: 'index.handler', + code, + }); + const clone = _.cloneDeep(runtime); + const layer = new lambda.LayerVersion(stack, 'myLayer', { + code, + compatibleRuntimes: [clone] + }); + + // THEN + // should not throw + func.addLayer(layer); + + test.done(); + }, +}); diff --git a/packages/@aws-cdk/aws-lambda/test/test.runtime.ts b/packages/@aws-cdk/aws-lambda/test/test.runtime.ts new file mode 100644 index 0000000000000..d1cc57fc7174a --- /dev/null +++ b/packages/@aws-cdk/aws-lambda/test/test.runtime.ts @@ -0,0 +1,70 @@ +import {Test, testCase} from 'nodeunit'; +import {RuntimeFamily} from "../lib"; +import lambda = require('../lib'); + +export = testCase({ + 'runtimes are equal for different instances'(test: Test) { + // GIVEN + const runtime1 = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: true}); + const runtime2 = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: true}); + + // WHEN + const result = runtime1.equals(runtime2); + + // THEN + test.strictEqual(result, true, 'Runtimes should be equal'); + + test.done(); + }, + 'runtimes are equal for same instance'(test: Test) { + // GIVEN + const runtime = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: true}); + + // WHEN + const result = runtime.equals(runtime); + + // THEN + test.strictEqual(result, true, 'Runtimes should be equal'); + + test.done(); + }, + 'unequal when name changes'(test: Test) { + // GIVEN + const runtime1 = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: true}); + const runtime2 = new lambda.Runtime('python3.6', RuntimeFamily.Python, {supportsInlineCode: true}); + + // WHEN + const result = runtime1.equals(runtime2); + + // THEN + test.strictEqual(result, false, 'Runtimes should be unequal when name changes'); + + test.done(); + }, + 'unequal when family changes'(test: Test) { + // GIVEN + const runtime1 = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: true}); + const runtime2 = new lambda.Runtime('python3.7', RuntimeFamily.Java, {supportsInlineCode: true}); + + // WHEN + const result = runtime1.equals(runtime2); + + // THEN + test.strictEqual(result, false, 'Runtimes should be unequal when family changes'); + + test.done(); + }, + 'unequal when supportsInlineCode changes'(test: Test) { + // GIVEN + const runtime1 = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: true}); + const runtime2 = new lambda.Runtime('python3.7', RuntimeFamily.Python, {supportsInlineCode: false}); + + // WHEN + const result = runtime1.equals(runtime2); + + // THEN + test.strictEqual(result, false, 'Runtimes should be unequal when supportsInlineCode changes'); + + test.done(); + }, +});