From a5be04270c2a372132964ab13d080a16f1a6f00c Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Tue, 16 Mar 2021 17:13:55 +0100 Subject: [PATCH] fix(core): `toJsonString()` cannot handle list intrinsics (#13544) The previous attempt at a fix missed one important case: the types of the values involved in the `{ Fn::Join }` expression didn't actually match up. They all needed to be strings, but the previous implentation just dropped list-typed values in there. Unfortunately, there is no way to do it correctly with just string manipulation in CloudFormation (`{ Fn::Join }` etc), so we'll have to resort to using a Custom Resource if we encounter list values. Actually fixes #13465. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../core/lib/private/cfn-utils-provider.ts | 33 ++++++++++++++++ .../lib/private/cfn-utils-provider/consts.ts | 7 +++- .../lib/private/cfn-utils-provider/index.ts | 11 ++++++ .../core/lib/private/cloudformation-lang.ts | 39 ++++++++++++++++--- .../core/test/cloudformation-json.test.ts | 6 ++- 5 files changed, 88 insertions(+), 8 deletions(-) diff --git a/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts b/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts index 8200165fbfe34..04a68394fe2f3 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts @@ -1,5 +1,7 @@ import { Construct } from '../construct-compat'; +import { CustomResource } from '../custom-resource'; import { CustomResourceProvider, CustomResourceProviderRuntime } from '../custom-resource-provider'; +import { CfnUtilsResourceType } from './cfn-utils-provider/consts'; /** * A custom resource provider for CFN utilities such as `CfnJson`. @@ -11,4 +13,35 @@ export class CfnUtilsProvider extends Construct { codeDirectory: `${__dirname}/cfn-utils-provider`, }); } +} + +/** + * Utility functions provided by the CfnUtilsProvider + */ +export abstract class CfnUtils { + /** + * Encode a structure to JSON at CloudFormation deployment time + * + * This would have been suitable for the JSON-encoding of abitrary structures, however: + * + * - It uses a custom resource to do the encoding, and we'd rather not use a custom + * resource if we can avoid it. + * - It cannot be used to encode objects where the keys of the objects can contain + * tokens--because those cannot be represented in the JSON encoding that CloudFormation + * templates use. + * + * This helper is used by `CloudFormationLang.toJSON()` if and only if it encounters + * objects that cannot be stringified any other way. + */ + public static stringify(scope: Construct, id: string, value: any): string { + const resource = new CustomResource(scope, id, { + serviceToken: CfnUtilsProvider.getOrCreate(scope), + resourceType: CfnUtilsResourceType.CFN_JSON_STRINGIFY, + properties: { + Value: value, + }, + }); + + return resource.getAttString('Value'); + } } \ No newline at end of file diff --git a/packages/@aws-cdk/core/lib/private/cfn-utils-provider/consts.ts b/packages/@aws-cdk/core/lib/private/cfn-utils-provider/consts.ts index b1571cabd5b42..9718dcef40645 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-utils-provider/consts.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-utils-provider/consts.ts @@ -5,5 +5,10 @@ export const enum CfnUtilsResourceType { /** * CfnJson */ - CFN_JSON = 'Custom::AWSCDKCfnJson' + CFN_JSON = 'Custom::AWSCDKCfnJson', + + /** + * CfnJsonStringify + */ + CFN_JSON_STRINGIFY = 'Custom::AWSCDKCfnJsonStringify', } diff --git a/packages/@aws-cdk/core/lib/private/cfn-utils-provider/index.ts b/packages/@aws-cdk/core/lib/private/cfn-utils-provider/index.ts index 87bd6bb070e16..f082001f80159 100644 --- a/packages/@aws-cdk/core/lib/private/cfn-utils-provider/index.ts +++ b/packages/@aws-cdk/core/lib/private/cfn-utils-provider/index.ts @@ -9,6 +9,9 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent if (event.ResourceType === CfnUtilsResourceType.CFN_JSON) { return cfnJsonHandler(event); } + if (event.ResourceType === CfnUtilsResourceType.CFN_JSON_STRINGIFY) { + return cfnJsonStringifyHandler(event); + } throw new Error(`unexpected resource type "${event.ResourceType}`); } @@ -20,3 +23,11 @@ function cfnJsonHandler(event: AWSLambda.CloudFormationCustomResourceEvent) { }, }; } + +function cfnJsonStringifyHandler(event: AWSLambda.CloudFormationCustomResourceEvent) { + return { + Data: { + Value: JSON.stringify(event.ResourceProperties.Value), + }, + }; +} diff --git a/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts b/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts index 310a4632f4e8f..c2be580474426 100644 --- a/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts +++ b/packages/@aws-cdk/core/lib/private/cloudformation-lang.ts @@ -1,6 +1,8 @@ import { Lazy } from '../lazy'; import { DefaultTokenResolver, IFragmentConcatenator, IResolveContext } from '../resolvable'; +import { Stack } from '../stack'; import { Token } from '../token'; +import { CfnUtils } from './cfn-utils-provider'; import { INTRINSIC_KEY_PREFIX, ResolutionTypeHint, resolvedTypeHint } from './resolve'; /** @@ -170,7 +172,8 @@ function tokenAwareStringify(root: any, space: number, ctx: IResolveContext) { // AND it's the result of a token resolution. Otherwise, we just treat this // value as a regular old JSON object (that happens to look a lot like an intrinsic). if (isIntrinsic(obj) && resolvedTypeHint(obj)) { - return renderIntrinsic(obj); + renderIntrinsic(obj); + return; } return renderCollection('{', '}', definedEntries(obj), ([key, value]) => { @@ -211,12 +214,34 @@ function tokenAwareStringify(root: any, space: number, ctx: IResolveContext) { pushLiteral('"'); pushIntrinsic(deepQuoteStringLiterals(intrinsic)); pushLiteral('"'); - break; - - default: + return; + + case ResolutionTypeHint.LIST: + // We need this to look like: + // + // '{"listValue":' ++ STRINGIFY(CFN_EVAL({ Ref: MyList })) ++ '}' + // + // However, STRINGIFY would need to execute at CloudFormation deployment time, and that doesn't exist. + // + // We could *ALMOST* use: + // + // '{"listValue":["' ++ JOIN('","', { Ref: MyList }) ++ '"]}' + // + // But that has the unfortunate side effect that if `CFN_EVAL({ Ref: MyList }) == []`, then it would + // evaluate to `[""]`, which is a different value. Since CloudFormation does not have arbitrary + // conditionals there's no way to deal with this case properly. + // + // Therefore, if we encounter lists we need to defer to a custom resource to handle + // them properly at deploy time. + pushIntrinsic(CfnUtils.stringify(Stack.of(ctx.scope), `CdkJsonStringify${stringifyCounter++}`, intrinsic)); + return; + + case ResolutionTypeHint.NUMBER: pushIntrinsic(intrinsic); - break; + return; } + + throw new Error(`Unexpected type hint: ${resolvedTypeHint(intrinsic)}`); } /** @@ -391,4 +416,6 @@ function deepQuoteStringLiterals(x: any): any { function quoteString(s: string) { s = JSON.stringify(s); return s.substring(1, s.length - 1); -} \ No newline at end of file +} + +let stringifyCounter = 1; \ No newline at end of file diff --git a/packages/@aws-cdk/core/test/cloudformation-json.test.ts b/packages/@aws-cdk/core/test/cloudformation-json.test.ts index cb96020e04904..e9a0957a86269 100644 --- a/packages/@aws-cdk/core/test/cloudformation-json.test.ts +++ b/packages/@aws-cdk/core/test/cloudformation-json.test.ts @@ -103,7 +103,11 @@ describe('tokens that return literals', () => { // WHEN expect(stack.resolve(stack.toJsonString({ someList }))).toEqual({ - 'Fn::Join': ['', ['{"someList":', { Ref: 'Thing' }, '}']], + 'Fn::Join': ['', [ + '{"someList":', + { 'Fn::GetAtt': [expect.stringContaining('CdkJsonStringify'), 'Value'] }, + '}', + ]], }); });