fix(toolkit-lib): message including tokens from annotations cannot output correctly

[go-to-k]

Fixes aws/aws-cdk#33707

Describe the bug

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.

Approach

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.

https://github.com/aws/aws-cdk-cli/blob/main/packages/%40aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts#L764

Actually, the type for data is any in aws-cdk-lib (with constructs library).

https://github.com/aws/constructs/blob/10.x/src/metadata.ts#L13

Alternative Approach

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

The following changes to the code would make this possible.

https://github.com/aws/aws-cdk/blob/v2.178.0/packages/aws-cdk-lib/core/lib/stack-synthesizers/_shared.ts#L103

    if (node.node.metadata.length > 0) {
      // Make the path absolute
      output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => {
        // If Annotations include a token, the message is resolved and output as `[object Object]` after synth
        // because the message will be object type using 'Fn::Join'.
        // It would be easier for users to understand if the message from Annotations were output in token form,
        // rather than in `[object Object]` or in object type using 'Fn::Join'.
        // Therefore, we don't resolve the message if it's from Annotations.
        // see: https://github.com/aws/aws-cdk/issues/33707
        if ([
          cxschema.ArtifactMetadataEntryType.ERROR,
          cxschema.ArtifactMetadataEntryType.WARN,
          cxschema.ArtifactMetadataEntryType.INFO,
        ].includes(md.type as cxschema.ArtifactMetadataEntryType)) {
          return md;
        }

        const resolved = stack.resolve(md);
        return resolved as cxschema.MetadataEntry;
      });
    }

But that would also output a token format in manifest.json for cloud assembly.

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

However Cloud assembly is for CFn or CDK CLI, and the tokens are for CDK Apps and should be resolved in the synth phase. Therefore, manifest.json should output the resolved value.

{
  // ...
  "CdkSampleStack": {
    // ...
      "metadata": {
        "/CdkSampleStack": [
          {
            "type": "aws:cdk:info",
            "data": {
              "Fn::Join": [
                "",
                [
                  "stackId: ",
                  {
                    "Ref": "AWS::StackId"
                  }
                ]
              ]
            },

In fact, the information in CFn format using ${AWS::AccountId} etc. is already in manifest.json as shown below.

 "lookupRole": {
          "arn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-lookup-role-${AWS::AccountId}-${AWS::Region}",
          "requiresBootstrapStackVersion": 8,
          "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version"
        }

So I decided the first approach in this PR.

Additional Information

When I reverted the code back to the original process and ran the unit test I created this time, the output was [object Object] as expected.

"message": "[warn at test-stack] [object Object]",

  ● metadata message formatting › converts object data to string for log message types

    expect(jest.fn()).toHaveBeenCalledWith(...expected)

    Expected: ObjectContaining {"data": {"entry": {"data": {"Fn::Join": ["", "test"], "Ref": "someRef"}, "type": "aws:cdk:warning"}, "id": "test-stack", "level": "warning"}, "level": "warn", "message": StringContaining "{\"Fn::Join\":[\"\",\"test\"],\"Ref\":\"someRef\"}"}
    Received
           1: {"action": "synth", "code": "CDK_ASSEMBLY_W9999", "data": {"entry": {"data": {"Fn::Join": ["", "test"], "Ref": "someRef"}, "type": "aws:cdk:warning"}, "id": "test-stack", "level": "warning"}, "level": "warn", "message": "[warn at test-stack] [object Object]", "time": 2025-02-22T18:00:01.083Z}

When I ran the test with the code reflected, it was in the correct format, so this change is considered appropriate.

CLI Integ Tests

Added a new cli integ test.

see: aws/aws-cdk-cli-testing#43

---

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

[rix0rrr]

I don't think this is a particularly nice rendering.

How do these { Fn::Join } objects get into the annotations in the first place? They only have meaning inside a CFN template, so if they are produced outside the context of a CFN template that is wrong.

Are they resolve()d by any chance? Because they probably shouldn't be.

[go-to-k]

@rix0rrr

How do these { Fn::Join } objects get into the annotations in the first place?

The issue mentioned about a situation where the user calls Annotations like the following:

(this is Stack class and ${this.stackId} is a token. So stackId: ${this.stackId} will be a string with { Fn::Join } after resolve.)

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

Are they resolve()d by any chance?

It seems that the Stack Synthesizer resolves the metadatas including them added by Annotations.

We can avoid resolving tokens added by Annotations by changing this code as follows.

    if (node.node.metadata.length > 0) {
      // Make the path absolute
      output[Node.PATH_SEP + node.node.path] = node.node.metadata.map(md => {
        if ([
          cxschema.ArtifactMetadataEntryType.ERROR,
          cxschema.ArtifactMetadataEntryType.WARN,
          cxschema.ArtifactMetadataEntryType.INFO,
        ].includes(md.type as cxschema.ArtifactMetadataEntryType)) {
          return md;
        }

        const resolved = stack.resolve(md);
        return resolved as cxschema.MetadataEntry;
      });

However, in that case, the metadatas would be written as the token format ("stackId: ${Token[AWS::StackId.1119]}") in manifest.json. I feel uncomfortable with this because I think the tokens are for CDK Apps and should be resolved in the synth phase, so I didn't decide it for now. (see the "Alternative Approach" in the description of this PR). But if this approach is fine, then I can adopt the approach.

Or what do you think of a way to make it an error if the user tries to put a token in Annotations?
(But in that case, the CDK application that the user including the issue author is currently running may not work, so a feature flag is needed...?)

[rix0rrr]

Or what do you think of a way to make it an error if the user tries to put a token in Annotations?

Yes this would have been ideal, but we can't do that anymore for the reason you already mentioned. People are already putting tokens into annotations (as useless as it is), otherwise you wouldn't have had to file this PR.

But yes, the change should be made inside the construct library. The data payload of an annotation should be a string, whatever the string is. So the library should make sure it's a string, and we don't have to deal with the situation where it's a non-string in the CLI.

I feel comfortable leaving the token unrendered for now; if people come to complain that they were parsing this data structure we will come up with a correct behavior then.

[go-to-k]

I feel comfortable leaving the token unrendered for now; if people come to complain that they were parsing this data structure we will come up with a correct behavior then.

Yes, got it. I will submit the another PR to not resolve the tokens by annotations in aws-cdk-lib.

Thanks.