fix(core): toJsonString() cannot handle list intrinsics (aws#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 aws#13465.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

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');
+  }
 }

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',
 }

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),
+    },
+  };
+}

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);
-}
+}
+
+let stringifyCounter = 1;

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'] },
+        '}',
+      ]],
     });
   });