Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lambda): currentVersion fails when architecture specified #16849

Merged
merged 4 commits into from
Oct 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/@aws-cdk/aws-lambda/lib/function-hash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ export function trimFromStart(s: string, maxLength: number) {
* must not be generated.
*
* Adding a new property to this list - If the property is part of the UpdateFunctionConfiguration
* API, then it must be classified as true, otherwise false.
* See https://docs.aws.amazon.com/lambda/latest/dg/API_UpdateFunctionConfiguration.html
* API or UpdateFunctionCode API, then it must be classified as true, otherwise false.
* See https://docs.aws.amazon.com/lambda/latest/dg/API_UpdateFunctionConfiguration.html and
* https://docs.aws.amazon.com/lambda/latest/dg/API_UpdateFunctionConfiguration.html
*/
const VERSION_LOCKED: { [key: string]: boolean } = {
export const VERSION_LOCKED: { [key: string]: boolean } = {
// locked to the version
Architectures: true,
Code: true,
DeadLetterConfig: true,
Description: true,
Expand Down
19 changes: 17 additions & 2 deletions packages/@aws-cdk/aws-lambda/lib/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,10 +313,17 @@ export interface FunctionOptions extends EventInvokeConfigOptions {
readonly codeSigningConfig?: ICodeSigningConfig;

/**
* The system architectures compatible with this lambda function.
* DEPRECATED
* @default [Architecture.X86_64]
* @deprecated use architecture
*/
readonly architectures?: Architecture[];

/**
* The system architectures compatible with this lambda function.
* @default Architecture.X86_64
*/
readonly architecture?: Architecture;
}

export interface FunctionProps extends FunctionOptions {
Expand Down Expand Up @@ -655,6 +662,14 @@ export class Function extends FunctionBase {
}];
}

if (props.architecture && props.architectures !== undefined) {
throw new Error('Either architecture or architectures must be specified but not both.');
}
if (props.architectures && props.architectures.length > 0) {
throw new Error('Only one architecture must be specified.');
}
const architecture = props.architecture ?? props.architectures?.at(0);

const resource: CfnFunction = new CfnFunction(this, 'Resource', {
functionName: this.physicalName,
description: props.description,
Expand Down Expand Up @@ -687,7 +702,7 @@ export class Function extends FunctionBase {
kmsKeyArn: props.environmentEncryption?.keyArn,
fileSystemConfigs,
codeSigningConfigArn: props.codeSigningConfig?.codeSigningConfigArn,
architectures: props.architectures?.map(a => a.name),
architectures: architecture ? [architecture.name] : undefined,
});

resource.node.addDependency(this.role);
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-lambda/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/cdk-integ-tools": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/cfnspec": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/aws-lambda": "^8.10.83",
"@types/jest": "^26.0.24",
Expand Down
11 changes: 10 additions & 1 deletion packages/@aws-cdk/aws-lambda/test/function-hash.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import '@aws-cdk/assert-internal/jest';
import * as path from 'path';
import { resourceSpecification } from '@aws-cdk/cfnspec';
import { App, CfnOutput, CfnResource, Stack } from '@aws-cdk/core';
import { LAMBDA_RECOGNIZE_VERSION_PROPS } from '@aws-cdk/cx-api';
import * as lambda from '../lib';
import { calculateFunctionHash, trimFromStart } from '../lib/function-hash';
import { calculateFunctionHash, trimFromStart, VERSION_LOCKED } from '../lib/function-hash';

describe('function hash', () => {
describe('trimFromStart', () => {
Expand Down Expand Up @@ -274,5 +275,13 @@ describe('function hash', () => {
(fn1.node.defaultChild as CfnResource).addPropertyOverride('UnclassifiedProp', 'Value');
expect(calculateFunctionHash(fn1)).toEqual(original);
});

test('all CFN properties are classified', () => {
const spec = resourceSpecification('AWS::Lambda::Function');
expect(spec.Properties).toBeDefined();
const expected = Object.keys(spec.Properties!).sort();
const actual = Object.keys(VERSION_LOCKED).sort();
expect(actual).toEqual(expected);
});
});
});
40 changes: 39 additions & 1 deletion packages/@aws-cdk/aws-lambda/test/function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2174,7 +2174,7 @@ describe('function', () => {
})).toThrow(/Layers are not supported for container image functions/);
});

test('specified architecture is recognized', () => {
test('specified architectures is recognized', () => {
const stack = new cdk.Stack();
new lambda.Function(stack, 'MyFunction', {
code: lambda.Code.fromInline('foo'),
Expand All @@ -2189,6 +2189,44 @@ describe('function', () => {
});
});

test('specified architecture is recognized', () => {
const stack = new cdk.Stack();
new lambda.Function(stack, 'MyFunction', {
code: lambda.Code.fromInline('foo'),
runtime: lambda.Runtime.NODEJS_12_X,
handler: 'index.handler',

architecture: lambda.Architecture.ARM_64,
});

expect(stack).toHaveResource('AWS::Lambda::Function', {
Architectures: ['arm64'],
});
});

test('both architectures and architecture are not recognized', () => {
const stack = new cdk.Stack();
expect(() => new lambda.Function(stack, 'MyFunction', {
code: lambda.Code.fromInline('foo'),
runtime: lambda.Runtime.NODEJS_12_X,
handler: 'index.handler',

architecture: lambda.Architecture.ARM_64,
architectures: [lambda.Architecture.X86_64],
})).toThrow(/architecture or architectures must be specified/);
});

test('Only one architecture allowed', () => {
const stack = new cdk.Stack();
expect(() => new lambda.Function(stack, 'MyFunction', {
code: lambda.Code.fromInline('foo'),
runtime: lambda.Runtime.NODEJS_12_X,
handler: 'index.handler',

architectures: [lambda.Architecture.X86_64, lambda.Architecture.ARM_64],
})).toThrow(/one architecture must be specified/);
});

});

function newTestLambda(scope: constructs.Construct) {
Expand Down
41 changes: 0 additions & 41 deletions packages/@aws-cdk/cfnspec/spec-source/530_Lambda_ARM_patch.json

This file was deleted.