Skip to content

Commit

Permalink
fix(lambda): currentVersion fails when architecture specified (#16849)
Browse files Browse the repository at this point in the history
The cause for this is that the new 'Architecture' property added to the
CloudFormation specification is not classified as version locked or not.

Added a test to ensure that any missing properties are caught in the
future.

Further, deprecated the `architectures` property and replaced with a
singular `architecture` prop. Lambda Functions only support one
architecture.

Additionally, removed the CFN spec patch, now that the CloudFormation
resource specification includes the Architecture fields.

fixes #16814


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Niranjan Jayakar authored Oct 7, 2021
1 parent 6194ad4 commit 8a0d369
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 48 deletions.
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 > 1) {
throw new Error('Only one architecture must be specified.');
}
const architecture = props.architecture ?? (props.architectures && props.architectures[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.

0 comments on commit 8a0d369

Please sign in to comment.