Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(custom-resources): unable to use a tokens in dictionary keys in AwsCustomResource #13063

Closed
eladb opened this issue Feb 15, 2021 · 1 comment · Fixed by #13074
Closed

(custom-resources): unable to use a tokens in dictionary keys in AwsCustomResource #13063

eladb opened this issue Feb 15, 2021 · 1 comment · Fixed by #13074
Assignees
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources effort/small Small work item – less than a day of effort p1

Comments

@eladb
Copy link
Contributor

eladb commented Feb 15, 2021

Reproduction Steps

In the following example, we are using CfnJson to render the request for a dynamodb:BatchWriteItem request. The reason CfnJson is needed is because this request has the table name as a dictionary key. Since table name can only be resolved during deployment, we need to pass it through CfnJson so it is serialized appropriately.

const table = new Table(this, 'MyDynamoTable', { partitionKey: { name: 'ID', type: AttributeType.STRING } });
const cfnJson = new CfnJson(this, 'Request', {
  value: {
    RequestItems: {
      [table.tableName]: [
        { PutRequest: { Item: { ID: { S: 'Item1' }, Value: { S: 'World' } } } },
        { PutRequest: { Item: { ID: { S: 'Item2' }, Value: { S: 'World' } } } }
      ]
    }          
  }
});

new AwsCustomResource(this, 'BatchWrite', {
  onCreate: {
    action: 'batchWriteItem',
    service: 'DynamoDB',
    physicalResourceId: PhysicalResourceId.of('PHYSICAL_RESOURCE_ID'),
    parameters: cfnJson,
  },
  policy: AwsCustomResourcePolicy.fromSdkCalls({ resources: [ table.tableArn ] })
});

What did you expect to happen?

We expected "parameters" to resolve from the CfnJson custom resource's "Value" attribute:

"parameters": { "Fn::GetAtt": [ "Request4BE5F30C", "Value" ] }

What actually happened?

The output CloudFormation template renders like this:

"parameters": {
  "Fn::Join": [
    "",
    [
      "{\"RequestItems\":{\"",
      {
        "Ref": "MyDynamoTableACB5AA21"
      },
      "\":[{\"PutRequest\":{\"Item\":{\"ID\":{\"S\":\"Item1\"},\"Value\":{\"S\":\"World\"}}}},{\"PutRequest\":{\"Item\":{\"ID\":{\"S\":\"Item2\"},\"Value\":{\"S\":\"World\"}}}}]}}"
    ]
  ]
}

Environment

  • CDK CLI Version : 1.89.0
  • Framework Version: 1.89.0
  • Node.js Version: v14.15.4
  • OS : Mac
  • Language (Version): TypeScript

Other

The root cause seems to be the fact that encodeBoolean calls JSON.stringify() on the parameters object here, combined with the fact that CfnJson implements toJSON() here, which returns the encoded JSON string of the /input/ instead of a token to the Value attribute.

I am not 100% sure why toJSON() of CfnJson does not return a token that resolves to the attribute, but I am also wondering if perhaps AwsCustomResource might be able to utilize a similar approach as CfnJson so to allow arbitrary JSON blobs in parameters without restrictions (basically the parameters field will need to be encoded as a string when passed to the AwsCustomResource handler).

Workaround

As a workaround, one can dynamically replace the toJSON() method on the CfnJson object to:

cfnJson.toJSON = () => Stack.of(this).resolve(cfnJson);

This is 🐛 Bug Report

@eladb eladb added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 15, 2021
@github-actions github-actions bot added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Feb 15, 2021
@eladb eladb added effort/small Small work item – less than a day of effort p1 and removed @aws-cdk/custom-resources Related to AWS CDK Custom Resources bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 15, 2021
@github-actions github-actions bot added the @aws-cdk/custom-resources Related to AWS CDK Custom Resources label Feb 15, 2021
eladb pushed a commit that referenced this issue Feb 16, 2021
…ary keys in AwsCustomResource

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
eladb pushed a commit that referenced this issue Feb 19, 2021
…ary keys in AwsCustomResource

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
@mergify mergify bot closed this as completed in #13074 Feb 24, 2021
mergify bot pushed a commit that referenced this issue Feb 24, 2021
…ary keys in AwsCustomResource (#13074)

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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/custom-resources Related to AWS CDK Custom Resources effort/small Small work item – less than a day of effort p1
Projects
None yet
2 participants