Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
33 changes: 33 additions & 0 deletions packages/@aws-cdk/core/lib/private/cfn-utils-provider.ts
Original file line number Diff line number Diff line change
@@ -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`.
Expand All @@ -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');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,10 @@ export const enum CfnUtilsResourceType {
/**
* CfnJson
*/
CFN_JSON = 'Custom::AWSCDKCfnJson'
CFN_JSON = 'Custom::AWSCDKCfnJson',

/**
* CfnJsonStringify
*/
CFN_JSON_STRINGIFY = 'Custom::AWSCDKCfnJsonStringify',
}
11 changes: 11 additions & 0 deletions packages/@aws-cdk/core/lib/private/cfn-utils-provider/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand All @@ -20,3 +23,11 @@ function cfnJsonHandler(event: AWSLambda.CloudFormationCustomResourceEvent) {
},
};
}

function cfnJsonStringifyHandler(event: AWSLambda.CloudFormationCustomResourceEvent) {
return {
Data: {
Value: JSON.stringify(event.ResourceProperties.Value),
},
};
}
39 changes: 33 additions & 6 deletions packages/@aws-cdk/core/lib/private/cloudformation-lang.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -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]) => {
Expand Down Expand Up @@ -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.
Comment on lines +230 to +232
Copy link
Contributor

@jogold jogold Mar 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rix0rrr I think I found a way with a Fn::Join on a Fn::Split acting like a replace:

cdk.Fn.join('', cdk.Fn.split('""', myString)) // acts like a replace

For #13465 and the issue with the array of ENIs, it looks like that

[cdk.Token.asList(cdk.Fn.join('', cdk.Fn.split('""', cdk.Stack.of(stack).toJsonString(cdk.Fn.join('","', endpoint.vpcEndpointNetworkInterfaceIds)))))]

Gives the following CF

{
  "Fn::Join": [
    "",
    [
      "{\"action\":\"describeNetworkInterfaces\",\"service\":\"EC2\",\"parameters\":{\"NetworkInterfaceIds\":[",
      {
        "Fn::Join": [
          "",
          {
            "Fn::Split": [
              "\"\"",
              {
                "Fn::Join": [
                  "",
                  [
                    "\"",
                    {
                      "Fn::Join": [
                        "\\\",\\\"",
                        {
                          "Fn::GetAtt": [
                            "OtherEndpoint88C37F72",
                            "NetworkInterfaceIds"
                          ]
                        }
                      ]
                    },
                    "\""
                  ]
                ]
              }
            ]
          }
        ]
      },
      "]},\"physicalResourceId\":{\"id\":\"physical-id\"},\"outputPath\":\"NetworkInterfaces.0.PrivateIpAddress\"}"
    ]
  ]
}

Successfully tested this with a GatewayVpcEndpoint which has no ENIs and hence returns an empty list for the vpcEndpointNetworkInterfaceIds attribute.

Even if it's really really ugly, it's better than a custom resource, right? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EHRMAGERD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are like a wizard.

A dirty wizard...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it though.

Copy link
Contributor Author

@rix0rrr rix0rrr Mar 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait no. This solution will break when presented with [""].

That value would successfully make it through the transformation initially and then be substituted away to [].

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bu then maybe the CR is the right solution

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also thinking about it... afraid nothing's to be done, as:

JOIN(<whatever>, []) == JOIN(<whatever>, [""]) == ""

Once we've JOINed, we've lost the ability to make the distinction between [] and [""]... right?

(I'm starting to doubt myself now)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there's a way to do it. You loose information after the first join:

[].join('","') === [''].join('","')

and the only "first operation" you can do on the array with CF is a Fn::Join.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK our comments crossed each other... I don't think it's possible in pure CF. This is sad 😢 .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IF ONLY we had a functional { Fn::If }...

//
// 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)}`);
}

/**
Expand Down Expand Up @@ -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;
6 changes: 5 additions & 1 deletion packages/@aws-cdk/core/test/cloudformation-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'] },
'}',
]],
});
});

Expand Down