fix(custom-resources): unable to use a resource attributes as dictionary keys in AwsCustomResource

[eladb]

AWS calls sometimes require to specify resource names or ARNs in dictionary keys (e.g. dynamodb:BatchWriteItem). Since resource attributes resolve only during deployment, we
expect to be able to pass them in dictionary keys. This does not work out of the box because JSON only allows strings in dictionary keys and attributes in CFN are modeled as JSON objects ({ "Fn::GetAtt": ["X", "Y" ] }, etc).

To address this, we simply encode Create, Update and Delete properties passed to the custom resource as token-aware JSON strings using stack.toJsonString(), and decode them in the custom resource.

Since the entire object is encoded, the special-casing for boolean encoding is no longer needed.

Additionally, we added support for toJSON() in the token resolution logic (stack.resolve()). If resolve encounters an object that has a toJSON() method, it will call it and continue resolution with the output value. This is needed here before AwsCustomResource uses this standard JS behavior to encode physical name references in requests.

We also made CfnJson.value public to allow directly accessing the referenced value in case it is needed (this could have been used as an alternative to the built-in support for token-aware stringification if we didn’t support it in AwsCustomResource) - see referenced issue.

Resolves #13063

---

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

[gitpod-io]

[eladb]

@jogold can you please take a look?

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
• Commit ID: 3ac2523
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[hughevans]

I guess this is what broke my use of getResponseField() on AwsCustomResource in v1.92.0.

So now I need to somehow use CfnJson to retrieve those values? It's not immediately clear to me how to do that.

Broken custom resource

1:17:15 pm | UPDATE_FAILED        | AWS::IAM::Policy                    | …/WildcardCer...Role/DefaultPolicy
CustomResource attribute error: Vendor response doesn't contain HostedZone.Id key in object arn:aws:cloudformation:…

// Allows CDK to use a reusable delegation set. Sourced from:
// https://pablissimo.com/1100/creating-a-route-53-public-hosted-zone-with-a-reusable-delegation-set-id-in-cdk

import {
  AwsCustomResource,
  AwsCustomResourcePolicy,
} from "@aws-cdk/custom-resources";
import { Construct, Fn, Names } from "@aws-cdk/core";
import {
  IPublicHostedZone,
  PublicHostedZone,
  PublicHostedZoneProps,
} from "@aws-cdk/aws-route53";
import { PhysicalResourceId } from "@aws-cdk/custom-resources";

export interface PublicHostedZoneWithReusableDelegationSetProps
  extends PublicHostedZoneProps {
  delegationSetId: string;
}

export class PublicHostedZoneWithReusableDelegationSet extends Construct {
  private publicHostedZone: AwsCustomResource;
  private hostedZoneName: string;

  constructor(
    scope: Construct,
    id: string,
    props: PublicHostedZoneWithReusableDelegationSetProps
  ) {
    super(scope, id);

    this.hostedZoneName = props.zoneName;

    const normaliseId = (id: string) => id.split("/").slice(-1)[0];
    const normalisedDelegationSetId = normaliseId(props.delegationSetId);

    this.publicHostedZone = new AwsCustomResource(
      this,
      "CreatePublicHostedZone",
      {
        onCreate: {
          service: "Route53",
          action: "createHostedZone",
          parameters: {
            CallerReference: Names.uniqueId(this),
            Name: this.hostedZoneName,
            DelegationSetId: normalisedDelegationSetId,
            HostedZoneConfig: {
              Comment: props.comment,
              PrivateZone: false,
            },
          },
          physicalResourceId: PhysicalResourceId.fromResponse("HostedZone.Id"),
        },
        policy: AwsCustomResourcePolicy.fromSdkCalls({
          resources: AwsCustomResourcePolicy.ANY_RESOURCE,
        }),
      }
    );

    new AwsCustomResource(this, "DeletePublicHostedZone", {
      onDelete: {
        service: "Route53",
        action: "deleteHostedZone",
        parameters: {
          Id: this.publicHostedZone.getResponseField("HostedZone.Id"),
        },
      },
      policy: AwsCustomResourcePolicy.fromSdkCalls({
        resources: AwsCustomResourcePolicy.ANY_RESOURCE,
      }),
    });
  }

  asPublicHostedZone(): IPublicHostedZone {
    return PublicHostedZone.fromHostedZoneAttributes(
      this,
      "CreatedPublicHostedZone",
      {
        hostedZoneId: Fn.select(
          2,
          Fn.split("/", this.publicHostedZone.getResponseField("HostedZone.Id"))
        ),
        zoneName: this.hostedZoneName,
      }
    );
  }
}

Update Please ignore, this issue happens on v1.91.0 whenever I change any of the attributes on my custom resources so it's likely something else.

[charles-salmon]

The changes that were made here are actually breaking, from the perspective of @aws-cdk/assert. The release notes don't currently make this clear (although I acknowledge that this is not a breaking change to any particular feature).

Specifically, this makes usage of the hasResourceLike function fairly awkward, if testing against specific properties under the Create, Update or Delete properties, given that these have now been encoded. Further, I believe this now enforces that tests must specify all properties in the same order as those that are output.

I see that a number of tests have been refactored as part of this PR to include the entire encoded contents of the Create, Update or Delete properties. I believe this potentially adds some fragility to the tests.

@rix0rrr Can you potentially comment as to whether or not implementing IResolvable, as opposed to encoding these properties, would alleviate these pains?

[seeebiii]

Specifically, this makes usage of the hasResourceLike function fairly awkward, if testing against specific properties under the Create, Update or Delete properties, given that these have now been encoded. Further, I believe this now enforces that tests must specify all properties in the same order as those that are output.

In response to @charles-salmon , I had to go through this problem with tests defined in my constructs, e.g. @seeebiii/ses-verify-identities. Here's how I solved it in case anyone else faces this "breaking change":

Simple Checks

If your custom resource parameters are rather simple and don't reference other resources, you can use a combination of encodedJson() and objectLike() (from @aws-cdk/assert) to verify the properties:

// testing a custom resource using AwsCustomResource that is executing SES:deleteIdentity

expectCDK(stack).to(
    haveResourceLike('Custom::AWS', {
      Create: encodedJson(objectLike({
        service: 'SES',
        action: 'deleteIdentity',
        parameters: {
          Identity: emailAddress,
        },
      })),
    }),
  );

Advanced Checks

In cases where your parameters reference other resources, CDK adds a Fn::Join to the Create/Update/Delete parameter and builds the JSON string by breaking it up into separate parts, so they can reference other resources. Here testing is a bit harder and I solved it using deepObjectLike() and stringLike() and anything() (you can get all from @aws-cdk/assert):

// testing a custom resource using AwsCustomResource that is executing SES:setIdentityNotificationTopic

const domain = ...

expectCDK(stack).to(haveResourceLike('Custom::AWS', {
      Create: deepObjectLike({
        'Fn::Join': [
          '', [
            stringLike(`{\"service\":\"SES\",\"action\":\"setIdentityNotificationTopic\",\"parameters\":{\"Identity\":\"${domain}\",\"NotificationType\":\"Bounce\"*`),
            anything(),
            anything(),
          ],
        ],
      }),
    }));

This isn't pretty but it's enough in my case. You can use stringLike() by having a string with a glob pattern, e.g. stringLike(foo*). I'm using anything() for the rest of the JSON string because I don't care about the remaining contents in this case.

I hope this helps anyone in case you run into the same problem when upgrading your CDK construct to 1.92.0.

[0xdevalias]

This PR looks as though it's likely the source of the following issue(s):

• (core): deletion of custom resources is broken since CDK 1.92 #13486
• (custom-resources): Document possibility of deletion failure when migrating from AwsCustomResource created prior to 1.92.0 #25668

Doing a little digging, I suspect this may have been introduced with this change introduced in 1.92.0 (2021-03-06):

• (custom-resources): unable to use a tokens in dictionary keys in AwsCustomResource #13063
  • fix(custom-resources): unable to use a resource attributes as dictionary keys in AwsCustomResource #13074

    • To address this, we simply encode Create, Update and Delete properties passed to the custom resource as token-aware JSON strings using stack.toJsonString(), and decode them in the custom resource.

Originally posted by @0xdevalias in #25668 (comment)