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

(annotations): annotations that include tokens are output as [object Object] #33707

Closed
1 task
swachter opened this issue Feb 20, 2025 · 6 comments · Fixed by #33706
Closed
1 task

(annotations): annotations that include tokens are output as [object Object] #33707

swachter opened this issue Feb 20, 2025 · 6 comments · Fixed by #33706
Labels

Comments

@swachter
Copy link
Contributor

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]

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

The string representation of the token should be output.

Current Behavior

The message is output as "[object Object]"

Reproduction Steps

class MyStack extends Stack {
  constructor(scope: Construct, id: string) {
    super(scope, id)
    Annotations.of(this).addInfo(`stackId: ${this.stackId}`)
  }
}

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.1000.2 (build bc82193)

Framework Version

No response

Node.js Version

v22.12.0

OS

Ubuntu

Language

TypeScript

Language Version

No response

Other information

No response

@pahud pahud self-assigned this Feb 20, 2025
@pahud
Copy link
Contributor

pahud commented Feb 20, 2025

let normalizedMessage = typeof message === 'string' ? message : JSON.stringify(message);

I think the fix would be to use Token.isUnresolved() to detect if the string contains any Tokens, and if it does, use Token.asString() to ensure proper resolution at synthesis time.

e.g.

private addMessage(level: string, message: string | IResolvable) {
  const isNew = !this.scope.node.metadata.find((x) => x.data === message);
  if (isNew) {
    // If message already implements IResolvable (like a Token), or is a string containing tokens,
    // we want to preserve its resolvable nature rather than prematurely converting to string
    const normalizedMessage = Token.isUnresolved(message) ? Token.asString(message) : message;
    this.scope.node.addMetadata(level, normalizedMessage, { stackTrace: this.stackTraces });
  }
}

Making it a p1 and requesting inputs from the team.

@go-to-k
Copy link
Contributor

go-to-k commented Feb 22, 2025

@pahud

I think the fix would be to use Token.isUnresolved() to detect if the string contains any Tokens, and if it does, use Token.asString() to ensure proper resolution at synthesis time.

Even with Token.asString(), all tokens are resolved before the timing of message output by annotation, so I think it is necessary to avoid resolving only annotation tokens in the first place.

However, in that case, the tokens are also output to manifest.json of the cloud assembly. The tokens are for CDK apps, but the cloud assembly is for CFn or CDK CLI. Therefore, the cloud assembly should be output with the token resolved. (It already contains strings in other CFn formats such as ${AWS::AccountId}.)

So I submitted a PR to the CDK CLI repository without modifying the aws-cdk-lib. Because I thought that was the best way to fix just the message output part without changing the manifest format.

Please see the details: aws/aws-cdk-cli#101

@go-to-k
Copy link
Contributor

go-to-k commented Mar 7, 2025

@godwingrs22 @mrgrain @pahud

Could you please put this issue back into aws-cdk(lib)?

After talking to Rico, it was agreed that this should be done on the library side. (aws/aws-cdk-cli#101 (comment))

I submitted the PR but it is now P2; moving the issue would change it to P1.

@mrgrain mrgrain transferred this issue from aws/aws-cdk-cli Mar 7, 2025
@mrgrain
Copy link
Contributor

mrgrain commented Mar 7, 2025

Could you please put this issue back into aws-cdk(lib)?

🫡

@mergify mergify bot closed this as completed in #33706 Mar 12, 2025
@mergify mergify bot closed this as completed in 55a3c4c Mar 12, 2025
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

1 similar comment
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 12, 2025
hwum pushed a commit to hwum/aws-cdk that referenced this issue Mar 12, 2025
…rrectly (aws#33706)

### Issue # (if applicable)

Closes aws#33707

### Reason for this change



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

```ts
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]}` -> ...)

```json
{
  // ...
  "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](aws/aws-cdk-cli#101) 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.

aws@63fd78b

```ts
    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: 

aws#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
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants