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

aws-cdk: Tag does not produce tags on the target resource #27355

Open
graemevwilson opened this issue Sep 29, 2023 · 5 comments
Open

aws-cdk: Tag does not produce tags on the target resource #27355

graemevwilson opened this issue Sep 29, 2023 · 5 comments
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2

Comments

@graemevwilson
Copy link

Describe the bug

According to https://docs.aws.amazon.com/cdk/v2/guide/tagging.html there are 2 ways of adding tags, either to all resources in a scope or to a specific resource:

Tags.of(scope).add('key', 'value')

or

Tag('key', 'value').visit(scope)

In CDK 2.99 the latter version, using the Tag class, does not create a tag in the CloudFormation template.

Expected Behavior

Tag('key', 'value').visit(scope)

Should add a tag of key: value to the specified scope or construct.

Current Behavior

Tag('key', 'value').visit(scope)

Does not add a tag to the specified scope or construct

Reproduction Steps

Create a CDK application and set app.py as follows and run cdk synth:

#!/usr/bin/env python3
import aws_cdk as cdk
from aws_cdk import (
    Duration,
    Stack,
    Tag,
    Tags,
    aws_sqs as sqs,
)
from constructs import Construct


class CdkTagTestStack(Stack):

    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        queue = sqs.Queue(
            self, "CdkTagTestQueue",
            visibility_timeout=Duration.seconds(300),
        )

        Tag('TestKey', 'TestValue').visit(queue)
        # Tags.of(queue).add('SomeKey', 'SomeValue')


app = cdk.App()

CdkTagTestStack(app, "CdkTagTestStack")

app.synth()

Inspect the .template file in cdk.out folder and note that the queue construct has no tag.

As a further test, comment line 23 and uncomment line 24. Run cdk synth again and on inspection of the .template file a tag will have been added to the queue resource.

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.99

Framework Version

No response

Node.js Version

18.17.1

OS

Windows and Linux

Language

Python

Language Version

3.10.2

Other information

No response

@graemevwilson graemevwilson added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 29, 2023
@github-actions github-actions bot added the @aws-cdk/aws-sqs Related to Amazon Simple Queue Service label Sep 29, 2023
@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 29, 2023
@khushail
Copy link
Contributor

khushail commented Sep 29, 2023

Hi @graemevwilson , the Tag is deprecated as mentioned here. Use of Tags is recommended way to add the tags. That would also mean the documentation need to be updated. So I am adding documentation label to this issue.

@khushail khushail added p2 effort/small Small work item – less than a day of effort documentation This is a problem with documentation. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Sep 29, 2023
@graemevwilson
Copy link
Author

Thanks @khushail but the deprecated methods that you're highlighting are Tag.add(s, k, v) and Tag.remove(s, k, v). I'm not using those methods, I'm using the constructor of Tag and Tag.visit(s). These are inherited from TagBase and are not marked as deprecated. I think that more than the documentation needs updating if Tag itself is deprecated.

@peterwoodworth
Copy link
Contributor

Hey @graemevwilson @khushail, there are currently no issues with the visit() method, it does seem there is some confusion here though.

On the topic of deprecation, please note that the Tag class itself is not deprecated, nor is the visit() method. We still fully support this class and method. Only specific methods on this class have been deprecated, which is not out of the norm.

On the topic of the .visit() method, it does work as described and expected. Keep in mind that in your example, you are wanting to tag specifically the Queue construct and none of its children. This will result in no tags in your template, because Queue doesn't directly map to the CloudFormation resource. Its default child does as the default child is a CfnQueue. So, as a general rule of thumb, you could do this:

    const vpc = new ec2.Vpc(this, 'Vpc')
    new cdk.Tag("Key", "Value").visit(vpc.node.defaultChild as cdk.CfnResource)

I think this would be good to clarify in our devguide, as I can see how this is unclear at first

@peterwoodworth peterwoodworth added p2 aws-cdk-lib Related to the aws-cdk-lib package and removed p1 @aws-cdk/aws-sqs Related to Amazon Simple Queue Service labels Oct 4, 2023
@graemevwilson
Copy link
Author

Thanks for the clarification @peterwoodworth - I have it working now!

@enpatrik
Copy link

Thanks @peterwoodworth ! 👍

I did the same mistake when trying to tag a specific resource (SQS Queue) within a custom aspect, just like the "PathTagger"-example described here: https://docs.aws.amazon.com/cdk/v2/guide/tagging.html#tagging_single

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-cdk-lib Related to the aws-cdk-lib package bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

No branches or pull requests

5 participants