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_core: NestedStack tags cannot override parent stack tags in constructor #30996

Open
sumpfork opened this issue Aug 1, 2024 · 7 comments
Open
Labels
@aws-cdk/core Related to core CDK functionality documentation This is a problem with documentation. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2

Comments

@sumpfork
Copy link

sumpfork commented Aug 1, 2024

Describe the bug

In this python code snippet:

app = aws_cdk.App()

stack1 = aws_cdk.Stack(app, "stack1")
tags = aws_cdk.Tags.of(stack1)
tags.add("foo", "bar")

stack2 = aws_cdk.NestedStack(stack1, "stack2")
tags = aws_cdk.Tags.of(stack2)
tags.add("foo", "zap", priority=2000)

The nested stack has tag "foo": "bar".

Expected Behavior

I expected stack2 to have tag "foo":"zap" but it has tag "foo":"bar" inherited from the parent stack. Similarly, removing a tag that was added in the parent stack does not work, but adding a new ones does.

Current Behavior

The inherited parent stack's tags cannot be modified by the nested stack.

Reproduction Steps

import aws_cdk

app = aws_cdk.App()

stack1 = aws_cdk.Stack(app, "stack1")
tags = aws_cdk.Tags.of(stack1)
tags.add("foo", "bar")

stack2 = aws_cdk.NestedStack(stack1, "stack2")
tags = aws_cdk.Tags.of(stack2)
tags.add("foo", "zap", priority=2000)

app.synth()

Observe that stack2 in the resulting cloudformation template has tag "foo":"bar".

Possible Solution

If this is expected behaviour I feel it should at least be documented (unless I missed that documentation somewhere).

Additional Information/Context

No response

CDK CLI Version

2.150.0 (build 3f93027)

Framework Version

2.149.0

Node.js Version

v20.12.1

OS

Mac os X 14.5

Language

Python

Language Version

Python 3.10.11

Other information

No response

@sumpfork sumpfork added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 1, 2024
@github-actions github-actions bot added the @aws-cdk/core Related to core CDK functionality label Aug 1, 2024
@sumpfork
Copy link
Author

sumpfork commented Aug 1, 2024

I simplified the example (I was wrong before, this also doesn't work outside of constructors).

@khushail khushail added investigating This issue is being investigated and/or work is in progress to resolve the issue. p2 and removed needs-triage This issue or PR still needs to be triaged. labels Aug 1, 2024
@khushail khushail self-assigned this Aug 1, 2024
@sumpfork
Copy link
Author

sumpfork commented Aug 1, 2024

As another note, this does work if you access the L1 construct to add the second tag directly:

cfn_nested_stack = stack2.node.default_child
cfn_nested_stack.tags.set_tag(
    "foo", "baz", priority=300
)

@khushail
Copy link
Contributor

khushail commented Aug 2, 2024

Hi @sumpfork , thanks for reaching out.

This is my analysis -

The tags are implemented using Aspects and the way nested stack would work is , one has to mention the Parent stack in the scope value(thats how CDK knows its a nested stack) of the Nested stack construct. If you add tags to a scope that includes a NestedStack construct, the CDK propagates the tags down to the nested stack's child resources at synth-time.

* Manages AWS tags for all resources within a construct scope.

you can add as many as tags using

cdk.Tags.of(nestedStack).add('tag2', 'nestedStack03');

but I am skeptical about changing the tag obtained from Parent stack. I have not tried it though, will try it out.

@khushail
Copy link
Contributor

khushail commented Aug 2, 2024

I also see that existing documentation covers these -

  1. Tagging single construct
  2. Tags - Manages AWS tags for all resources within a construct scope.
  3. Tag - The Tag Aspect will handle adding a tag to this node and cascading tags to children.

Having clear documentation around tagging would definitely be very helpful.

@khushail khushail added effort/small Small work item – less than a day of effort documentation This is a problem with documentation. labels Aug 2, 2024
@pahud
Copy link
Contributor

pahud commented Aug 2, 2024

Tags.of(scope).add('foo', 'bar'); is essentially to set tags for "resources in that scope".

Consider this example:

export class DummyStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    // create a dummy sqs.Queue
    new sqs.Queue(this, 'Queue1');

    // set tags for everything within current stack, including Queue1
    Tags.of(this).add('foo', 'bar');

    const nested = new NestedStack(this, 'NestedStack');
    // place Queue2 in the nested stack
    new sqs.Queue(nested, 'Queue2');

    // set everything in the nested stack, including the Queue2
    Tags.of(nested).add('foo', 'zap', { priority: 1000 });

  }
} 

Now if you check the synthesized templates in cdk.out.

Queue1 is having foo:bar, which makes sense.

  "Type": "AWS::SQS::Queue",
   "Properties": {
    "Tags": [
     {
      "Key": "foo",
      "Value": "bar"
     }

And the template of nested stack would be in another template file under cdk.out

And if you check that

Queue2 is having foo:zap

  "Queue26CB7866F": {
   "Type": "AWS::SQS::Queue",
   "Properties": {
    "Tags": [
     {
      "Key": "foo",
      "Value": "zap"
     }
    ]
   },

@khushail
Copy link
Contributor

khushail commented Aug 2, 2024

@sumpfork , I tried to change the nested stack tags using the node.defaultchild().

const cfnstack = nestedStack.node.defaultChild as CfnStack;
cfnstack.tags.setTag('tag1', 'parentStackTagValueReplaced');

It does not work and the reason is Aspects. Since the tagging of nested stack is done during synth time, the Aspects take over and having the highest priority, they override the set value to previous value propagated by parent stack . So the tags from the Parent stack might not be changeable.

(credits: @pahud )

However I am marking this issue for core team's input here. A clear documentation would really help in this regard!

@khushail khushail removed the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Aug 2, 2024
@khushail khushail removed their assignment Aug 2, 2024
@comcalvi
Copy link
Contributor

We can't change Tags.of() to allow child constructs to override the parent's tags. We should clarify our docs around this.

@comcalvi comcalvi removed the bug This issue is a bug. label Aug 23, 2024
@moelasmar moelasmar added the feature-request A feature should be added or improved. label Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/core Related to core CDK functionality documentation This is a problem with documentation. effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

5 participants