fix(core): message including tokens from annotations cannot output correctly

[go-to-k]

Issue # (if applicable)

Closes #33707

Reason for this change

If a stack with name 'some-stack' includes an info annotation

Annotations.of(this).addInfo(`stackId: ${this.stackId}`);

then the following output results:

[Info at /some-stack] [object Object]

That's because data comes from Annotations and the data can be of object type containing 'Fn::Join' or 'Ref' when tokens are included in Annotations.

The issue mentioned a proposal to output the data in the form of tokens like [Info at /CdkSampleStack] ${Token[AWS::StackId.1116]}.

Description of changes

Approach 1 for now. (I am still wondering if approach 3 would be better...)

See below:

Approach 1

The PR makes messages with tokens by annotations unresolved.

NOTE

This change would also output a token format in manifest.json.

If users run integ tests with annotations including tokens, the manifest.json would change for every run. (like ${Token[AWS::StackId.1119]} -> ${Token[AWS::StackId.123]} -> ${Token[AWS::StackId.521]} -> ...)

{
  // ...
  "CdkSampleStack": {
    // ...
      "metadata": {
        "/CdkSampleStack": [
          {
            "type": "aws:cdk:info",
            "data": "stackId: ${Token[AWS::StackId.1119]}",

Approach 2

Change the type for the msg.entry.data (MetadataEntryData for MetadataEntry) to a string type with JSON.stringify if the type is an objective type in cdk-cli.

https://github.com/aws/aws-cdk-cli/blob/cdk%40v2.1003.0/packages/%40aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts#L771

Then I had submitted the PR in aws-cdk-cli.

But talked with Rico that the change should be made inside cdk-lib and leave the token unrendered.

aws/aws-cdk-cli#101 (comment)

Approach 3

Change the data type to a string type after resolve if the data is by annotations with tokens.

This approach doesn't make differences in manifest.json for every run and the original format (with 'Ref' or 'Fn::Join') is kept.

However, the issue for this PR and comments in the PR submitted (aws-cdk-cli) has proposed the approach with unresolved tokens, I decided the approach 1 for now.

63fd78b

    if (node.node.metadata.length > 0) {
      // Make the path absolute
      output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => {
        const resolved = stack.resolve(md) as cxschema.MetadataEntry;

        const isAnnotation = [
          cxschema.ArtifactMetadataEntryType.ERROR,
          cxschema.ArtifactMetadataEntryType.WARN,
          cxschema.ArtifactMetadataEntryType.INFO,
        ].includes(md.type as cxschema.ArtifactMetadataEntryType);

        // Transform the data to a string for the case where Annotations include a token.
        // Otherwise, the message is resolved and output as `[object Object]` after synth
        // because the message will be object type using 'Ref' or 'Fn::Join'.
        const mdWithStringData: cxschema.MetadataEntry = {
          ...resolved,
          data: (isAnnotation && typeof resolved.data === 'object') ? JSON.stringify(resolved.data) : resolved.data,
        };
        return mdWithStringData;
      });
    }

This approach outputs the message as the following style:

{"Fn::Join":["",["Cannot add a resource policy to your dead letter queue associated with rule ",{"Ref":"Rule4C995B7F"}," because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account 444455556666. [ack: @aws-cdk/aws-events-targets:manuallyAddDLQResourcePolicy]"]]}

Additional Information

see:

#33707 (comment)

aws/aws-cdk-cli#101 (comment)

Describe any new or updated permissions being added

Description of how you validated changes

Unit tests.

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[aws-cdk-automation]

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

✅ A exemption request has been requested. Please wait for a maintainer's review.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.25%. Comparing base (3ed7c4d) to head (31f41d7).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #33706      +/-   ##
==========================================
+ Coverage   82.24%   82.25%   +0.01%     
==========================================
  Files         119      119              
  Lines        6875     6879       +4     
  Branches     1161     1162       +1     
==========================================
+ Hits         5654     5658       +4     
  Misses       1118     1118              
  Partials      103      103              

Flag	Coverage Δ
suite.unit	82.25% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	∅ <ø> (∅)
packages/aws-cdk-lib/core	82.25% <100.00%> (+0.01%)	⬆️

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[go-to-k]

Exemption Request:

This PR could covered with unit tests only.

And I think the integration test is unnecessary, as it should originally not allow tokens to be included in annotations. (CDK CLI would expect the type of string.)

Also integ tests for annotations with tokens will make differences in manifest.json for every run.

aws/aws-cdk-cli#101 (comment)

[go-to-k]

Dear team,

I came up with the approach to change the data type to a string type after resolve if the data is by annotations with tokens. (see Approach 3 in the description in this PR.)

This approach doesn't make differences in manifest.json for every run and the original format (with 'Ref' or 'Fn::Join') is kept.

However, the issue for this PR and comments in the PR submitted (aws-cdk-cli) has proposed the approach with unresolved tokens, I decided the approach 1 for now.

Please let me know if the approach would be better.

63fd78b

    if (node.node.metadata.length > 0) {
      // Make the path absolute
      output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => {
        const resolved = stack.resolve(md) as cxschema.MetadataEntry;

        const isAnnotation = [
          cxschema.ArtifactMetadataEntryType.ERROR,
          cxschema.ArtifactMetadataEntryType.WARN,
          cxschema.ArtifactMetadataEntryType.INFO,
        ].includes(md.type as cxschema.ArtifactMetadataEntryType);

        // Transform the data to a string for the case where Annotations include a token.
        // Otherwise, the message is resolved and output as `[object Object]` after synth
        // because the message will be object type using 'Ref' or 'Fn::Join'.
        const mdWithStringData: cxschema.MetadataEntry = {
          ...resolved,
          data: (isAnnotation && typeof resolved.data === 'object') ? JSON.stringify(resolved.data) : resolved.data,
        };
        return mdWithStringData;
      });
    }

This approach outputs the message as the following style:

{"Fn::Join":["",["Cannot add a resource policy to your dead letter queue associated with rule ",{"Ref":"Rule4C995B7F"}," because the queue is in a different account. You must add the resource policy manually to the dead letter queue in account 444455556666. [ack: @aws-cdk/aws-events-targets:manuallyAddDLQResourcePolicy]"]]}

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 31f41d7
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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