From db1d5a0a0231ed515dd4130a0ca342de7f34a1e6 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 16 Aug 2023 17:22:20 +0200 Subject: [PATCH 1/2] fix(lambda): use of `currentVersion` fails on upgrade Between version `2.87.0` and version `2.88.0`, the hash calculation used to make sure that `fn.currentVersion` is automatically updated when a new version of the Lambda Function is deployed changed. This causes a creation of a new Version upon upgrading CDK, but that new Version creation will fail because the underlying Function hasn't changed. The change was due to property ordering used in calculating a unique hash for the Function configuration. This change restores the property ordering to the pre-2.88.0 behavior. Fixes #26739. --- .../aws-lambda/lib/function-hash.ts | 141 ++++++++++++------ .../aws-lambda/test/function.test.ts | 62 ++++++++ packages/aws-cdk-lib/aws-lambda/test/x.zip | 1 + 3 files changed, 160 insertions(+), 44 deletions(-) create mode 100644 packages/aws-cdk-lib/aws-lambda/test/x.zip diff --git a/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts b/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts index f853db60b1d8f..f28ba4b59c272 100644 --- a/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts +++ b/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts @@ -8,26 +8,16 @@ export function calculateFunctionHash(fn: LambdaFunction, additional: string = ' const stack = Stack.of(fn); const functionResource = fn.node.defaultChild as CfnResource; - - // render the cloudformation resource from this function - const config = stack.resolve((functionResource as any)._toCloudFormation()); - // config is of the shape: { Resources: { LogicalId: { Type: 'Function', Properties: { ... } }}} - const resources = config.Resources; - const resourceKeys = Object.keys(resources); - if (resourceKeys.length !== 1) { - throw new Error(`Expected one rendered CloudFormation resource but found ${resourceKeys.length}`); - } - const logicalId = resourceKeys[0]; - const properties = resources[logicalId].Properties; + const { properties, template, logicalId } = resolveSingleResourceProperties(stack, functionResource); let stringifiedConfig; if (FeatureFlags.of(fn).isEnabled(LAMBDA_RECOGNIZE_VERSION_PROPS)) { - const updatedProps = sortProperties(filterUsefulKeys(properties)); + const updatedProps = sortFunctionProperties(filterUsefulKeys(properties)); stringifiedConfig = JSON.stringify(updatedProps); } else { - const sorted = sortProperties(properties); - config.Resources[logicalId].Properties = sorted; - stringifiedConfig = JSON.stringify(config); + const sorted = sortFunctionProperties(properties); + template.Resources[logicalId].Properties = sorted; + stringifiedConfig = JSON.stringify(template); } if (FeatureFlags.of(fn).isEnabled(LAMBDA_RECOGNIZE_LAYER_VERSION)) { @@ -103,26 +93,6 @@ function filterUsefulKeys(properties: any) { return ret; } -function sortProperties(properties: any) { - const ret: any = {}; - // We take all required properties in the order that they were historically, - // to make sure the hash we calculate is stable. - // There cannot be more required properties added in the future, - // as that would be a backwards-incompatible change. - const requiredProperties = ['Code', 'Handler', 'Role', 'Runtime']; - for (const requiredProperty of requiredProperties) { - ret[requiredProperty] = properties[requiredProperty]; - } - // then, add all of the non-required properties, - // in the original order - for (const property of Object.keys(properties)) { - if (requiredProperties.indexOf(property) === -1) { - ret[property] = properties[property]; - } - } - return ret; -} - function calculateLayersHash(layers: ILayerVersion[]): string { const layerConfig: {[key: string]: any } = {}; for (const layer of layers) { @@ -143,17 +113,100 @@ function calculateLayersHash(layers: ILayerVersion[]): string { } continue; } - const config = stack.resolve((layerResource as any)._toCloudFormation()); - const resources = config.Resources; - const resourceKeys = Object.keys(resources); - if (resourceKeys.length !== 1) { - throw new Error(`Expected one rendered CloudFormation resource but found ${resourceKeys.length}`); - } - const logicalId = resourceKeys[0]; - const properties = resources[logicalId].Properties; + + const { properties } = resolveSingleResourceProperties(stack, layerResource); + // all properties require replacement, so they are all version locked. - layerConfig[layer.node.id] = properties; + layerConfig[layer.node.id] = sortLayerVersionProperties(properties); } return md5hash(JSON.stringify(layerConfig)); } + +/** + * Sort properties in an object according to a sort order of known keys + * + * Any additional keys are added at the end, but also sorted. + */ +class PropertySort { + constructor(private readonly knownKeysOrder: string[]) { + } + + public sortAny(x: any): any { + if (Array.isArray(x)) { + return x.map((e) => this.sortAny(e)); + } + if (x && typeof x === 'object') { + return this.sortObject(x); + } + return x; + } + + public sortObject(properties: any): any { + const ret: any = {}; + + // Scratch-off set for keys we don't know about yet + const unusedKeys = new Set(Object.keys(properties)); + for (const prop of this.knownKeysOrder) { + ret[prop] = recurse(properties[prop]); + unusedKeys.delete(prop); + } + + for (const prop of Array.from(unusedKeys).sort()) { + ret[prop] = recurse(properties[prop]); + } + + return ret; + + function recurse(x: any): any { + return new PropertySort([]).sortAny(x); + } + } +} + +/** + * Sort properties in a stable order, even as we switch to new codegen + * + * <=2.87.0, we used to generate properties in the order that they occurred in + * the CloudFormation spec. >= 2.88.0, we switched to a new spec source, which + * sorts the properties lexicographically. The order change changed the hash, + * even though the properties themselves have not changed. + * + * We now have a set of properties with the sort order <=2.87.0, and add any + * additional properties later on, but also sort them. + * + * We should be making sure that the orderings for all subobjects + * between 2.87.0 and 2.88.0 are the same, but fortunately all the subobjects + * were already in lexicographic order in <=2.87.0 so we only need to sort some + * top-level properties on the resource. + */ +function sortFunctionProperties(properties: any) { + return new PropertySort([ + // <= 2.87 explicitly fixed order + 'Code', 'Handler', 'Role', 'Runtime', + // <= 2.87 implicitly fixed order + 'Architectures', 'CodeSigningConfigArn', 'DeadLetterConfig', 'Description', 'Environment', + 'EphemeralStorage', 'FileSystemConfigs', 'FunctionName', 'ImageConfig', 'KmsKeyArn', 'Layers', + 'MemorySize', 'PackageType', 'ReservedConcurrentExecutions', 'RuntimeManagementConfig', 'SnapStart', + 'Tags', 'Timeout', 'TracingConfig', 'VpcConfig', + ]).sortObject(properties); +} + +function sortLayerVersionProperties(properties: any) { + return new PropertySort([ + // <=2.87.0 implicit sort order + 'Content', 'CompatibleArchitectures', 'CompatibleRuntimes', 'Description', + 'LayerName', 'LicenseInfo', + ]).sortObject(properties); +} + +function resolveSingleResourceProperties(stack: Stack, res: CfnResource): any { + const template = stack.resolve(res._toCloudFormation()); + const resources = template.Resources; + const resourceKeys = Object.keys(resources); + if (resourceKeys.length !== 1) { + throw new Error(`Expected one rendered CloudFormation resource but found ${resourceKeys.length}`); + } + const logicalId = resourceKeys[0]; + return { properties: resources[logicalId].Properties, template, logicalId }; +} diff --git a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts b/packages/aws-cdk-lib/aws-lambda/test/function.test.ts index 83d25a6c13155..f4364373cdce7 100644 --- a/packages/aws-cdk-lib/aws-lambda/test/function.test.ts +++ b/packages/aws-cdk-lib/aws-lambda/test/function.test.ts @@ -3246,6 +3246,68 @@ test('set SnapStart to desired value', () => { }); }); +test('test 2.87.0 version hash stability', () => { + // GIVEN + const app = new cdk.App({ + context: { + '@aws-cdk/aws-lambda:recognizeLayerVersion': true, + }, + }); + const stack = new cdk.Stack(app, 'Stack'); + + // WHEN + const layer = new lambda.LayerVersion(stack, 'MyLayer', { + code: lambda.Code.fromAsset(path.join(__dirname, 'x.zip')), + compatibleRuntimes: [ + lambda.Runtime.NODEJS_18_X, + ], + }); + + const role = new iam.Role(stack, 'MyRole', { + assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'), + managedPolicies: [ + iam.ManagedPolicy.fromAwsManagedPolicyName('service-role/AWSLambdaBasicExecutionRole'), + iam.ManagedPolicy.fromAwsManagedPolicyName('AWSXRayDaemonWriteAccess'), + ], + }); + + const lambdaFn = new lambda.Function(stack, 'MyLambda', { + runtime: lambda.Runtime.NODEJS_18_X, + memorySize: 128, + handler: 'index.handler', + timeout: cdk.Duration.seconds(30), + environment: { + VARIABLE_1: 'ONE', + }, + code: lambda.Code.fromAsset(path.join(__dirname, 'x.zip')), + role, + currentVersionOptions: { + removalPolicy: cdk.RemovalPolicy.RETAIN, + }, + layers: [ + layer, + ], + }); + + new lambda.Alias(stack, 'MyAlias', { + aliasName: 'current', + version: lambdaFn.currentVersion, + }); + + // THEN + // Precalculated version hash using 2.87.0 version + Template.fromStack(stack).hasResource('AWS::Lambda::Alias', { + Properties: { + FunctionVersion: { + 'Fn::GetAtt': [ + 'MyLambdaCurrentVersionE7A382CCd55a48b26bd9a860d8842137f2243c37', + 'Version', + ], + }, + }, + }); +}); + function newTestLambda(scope: constructs.Construct) { return new lambda.Function(scope, 'MyLambda', { code: new lambda.InlineCode('foo'), diff --git a/packages/aws-cdk-lib/aws-lambda/test/x.zip b/packages/aws-cdk-lib/aws-lambda/test/x.zip new file mode 100644 index 0000000000000..c1b0730e01334 --- /dev/null +++ b/packages/aws-cdk-lib/aws-lambda/test/x.zip @@ -0,0 +1 @@ +x \ No newline at end of file From 994b0916bd3f5957a79abff7b5f0713a0d0e86a1 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 17 Aug 2023 10:20:25 +0200 Subject: [PATCH 2/2] Pull back a little on future stability to fix b/w compat now --- .../aws-lambda/lib/function-hash.ts | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts b/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts index f28ba4b59c272..87a224016b052 100644 --- a/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts +++ b/packages/aws-cdk-lib/aws-lambda/lib/function-hash.ts @@ -127,40 +127,30 @@ function calculateLayersHash(layers: ILayerVersion[]): string { * Sort properties in an object according to a sort order of known keys * * Any additional keys are added at the end, but also sorted. + * + * We only sort one level deep, because we rely on the fact that everything + * that needs to be sorted happens to be sorted by the codegen already, and + * we explicitly rely on some objects NOT being sorted. */ class PropertySort { constructor(private readonly knownKeysOrder: string[]) { } - public sortAny(x: any): any { - if (Array.isArray(x)) { - return x.map((e) => this.sortAny(e)); - } - if (x && typeof x === 'object') { - return this.sortObject(x); - } - return x; - } - public sortObject(properties: any): any { const ret: any = {}; // Scratch-off set for keys we don't know about yet const unusedKeys = new Set(Object.keys(properties)); for (const prop of this.knownKeysOrder) { - ret[prop] = recurse(properties[prop]); + ret[prop] = properties[prop]; unusedKeys.delete(prop); } for (const prop of Array.from(unusedKeys).sort()) { - ret[prop] = recurse(properties[prop]); + ret[prop] = properties[prop]; } return ret; - - function recurse(x: any): any { - return new PropertySort([]).sortAny(x); - } } } @@ -179,6 +169,11 @@ class PropertySort { * between 2.87.0 and 2.88.0 are the same, but fortunately all the subobjects * were already in lexicographic order in <=2.87.0 so we only need to sort some * top-level properties on the resource. + * + * We also can't deep-sort everything, because for backwards compatibility + * reasons we have a test that ensures that environment variables are not + * lexicographically sorted, but emitted in the order they are added in source + * code, so for now we rely on the codegen being lexicographically sorted. */ function sortFunctionProperties(properties: any) { return new PropertySort([