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

(Tags): Tagging within custom construct does not apply tag to the AWS resource #19390

Closed
abramcav opened this issue Mar 14, 2022 · 8 comments
Closed
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p1 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@abramcav
Copy link

abramcav commented Mar 14, 2022

What is the problem?

When inside a custom construct I call Tags.of(asg).add("Name", "my-asg-name");, the resource (asg in this case) does not get the tag set in AWS during a deploy. The resources are created properly, and inherited tags are set, but not tags added using the Tags.of(...).add(...) method inside the construct.

Reproduction Steps

create custom construct:

import { Construct } from "constructs";
import { Cluster } from "aws-cdk-lib/aws-ecs";
import { Tags } from "aws-cdk-lib";
import { Vpc } from "aws-cdk-lib/aws-ec2";

export class MyEcsCluster extends Construct {
    constructor(scope: Construct, id: string, vpc: Vpc) {
        super(scope, id);
        const cluster = new Cluster(this, "my-ecs-cluster", { vpc: vpc });
        Tags.of(cluster).add("Name", "my-ecs-cluster");
    }
}

Create a stack that instantiates the construct:

import { Stack, StackProps } from "aws-cdk-lib";
import { Construct } from "constructs";
import { Vpc } from "aws-cdk-lib/aws-ec2";
import { MyEcsCluster } from "my-constructs";
export class TestStack extends Stack { constructor(scope: Construct, id: string, props: StackProps) {
    super(scope, id, props);
    // Import existing VPC
    const vpc: Vpc = Vpc.fromLookup(this, "primary", {
      vpcName: "primary",
    }) as Vpc;

    new MyEcsCluster(this, "test-custom-construct", vpc);

  }
}

What did you expect to happen?

When you run cdk deploy on this stack it should create an ECS cluster with a tag key of "Name" and value of "my-ecs-cluster"

What actually happened?

It created the ECS cluster, but did not add the tag

CDK CLI Version

2.16.0 (build 4c77925)

Framework Version

aws-cdk-lib: 2.16.0, constructs: 10.0.87

Node.js Version

v14.18.0

OS

macOS Catalina v10.15.7

Language

Typescript

Language Version

TypeScript 4.6.2

Other information

Current workaround is to use escape hatch to directly add tags within the construct:

const cfnCluster = cluster.node.defaultChild as CfnCluster;
 cfnCluster.addPropertyOverride('Tags', [{
     Key: 'Name',
     Value: 'my-ecs-cluster',
 },]);
@abramcav abramcav added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 14, 2022
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label Mar 14, 2022
@ryparker ryparker added the p1 label Mar 14, 2022
@corymhall
Copy link
Contributor

@abramcav using the example that you provided I can't reproduce the issue. I've tried with a couple different resource types and the tags are added fine. Is it specific resource types that you are having issues with that are not referenced here?

@corymhall corymhall added needs-reproduction This issue needs reproduction. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Mar 15, 2022
@abramcav
Copy link
Author

The constructs in question create ECS cluster, services, tasks, autoscaling group, target groups, IAM roles all of which fail to add tags (for me).

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 16, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 16, 2022

Can you post code that reproduces the problem?

@abramcav
Copy link
Author

abramcav commented Mar 17, 2022

The code I posted above is what reproduces the problem for me with the exclusion of the app entry point:

#!/usr/bin/env node
import "source-map-support/register";
import * as cdk from "aws-cdk-lib";
import { MyEcsCluster } from "./stacks/my-ecs-cluster";

const app = new cdk.App();
const _env = {
    region: 'my-region',
    account: 'my-account',
};

new MyEcsClustserStack(app, 'my-ecs-cluster-stack', {
    stackName: 'my-ecs-cluster-stack',
    env: _env,
});

app.synth();

The constructs live in a different app, and are installed via npm as a file: dependency from package.json:

....
"dependencies": {
    "aws-cdk-lib": "2.16.0",
    "cdk-dia": "^0.6.1",
    "cdk-ec2-key-pair": "^3.2.0",
    "cdk-iam-floyd": "^0.300.0",
    "constructs": "10.0.87",
    "source-map-support": "^0.5.21",
    "my-constructs":"file:../../shared/custom"
  }

Not sure if that matters, but trying to help repro my environment as much as possible. As a side note, I originally developed these constructs as user defined functions that I imported into the stack; this is when I realized the tagging issue and refactored the functions into custom constructs thinking it had to do with scope.

@corymhall
Copy link
Contributor

This looks to be related to #18778 & #18921. There is a known issue when working with aspects and symlinks, but I'm not sure why this would be the case with the Tags aspect

@sdomme
Copy link

sdomme commented Mar 18, 2022

Stumbled upon this issue during investigating an issue I have with subnets tags you need to use the albController subnet auto discovery. We have a custom construct wrapped around the aws-cdk-lib/aws-eks/cluster construct.
The method we relying on is


If I locally develop the library and have to npm link it to my stack repo, the tagging usually happened within the lib doesn't work at all. Not even if I use the Tags.of function.
To test I build the npm package and used it in a normal way in my stack. cdk diff showed me all the missing tags.
Working with:

  • Node 16
  • CDK 2.17

rix0rrr added a commit that referenced this issue Mar 21, 2022
When construct libraries are purposely symlinked (as opposed of
collectively `npm install`ed), depending on how this is done they may
end up with multiple copies of `aws-cdk-lib`. If that happens, Aspects
from a different `aws-cdk-lib` copy than the one providing `App` will
not be applied.

The reason is the use of `Symbol('...')` instead of `Symbol.for('...')`
to keep the list of aspects on the construct object.

- The first version creates a unique symbol per library, while adding
  a naming hint.
- The second version deduplicates: all symbols with the same naming
  hint will receive the same symbol.

The second version is necessary to make sure that different copies
of the `aws-cdk-lib` library store their aspects under the same key.

Fixes #18921, #18778, #19390.
@corymhall corymhall removed needs-triage This issue or PR still needs to be triaged. needs-reproduction This issue needs reproduction. labels Mar 21, 2022
mergify bot pushed a commit that referenced this issue Mar 22, 2022
When construct libraries are purposely symlinked (as opposed of
collectively `npm install`ed), depending on how this is done they may
end up with multiple copies of `aws-cdk-lib`. If that happens, Aspects
from a different `aws-cdk-lib` copy than the one providing `App` will
not be applied.

The reason is the use of `Symbol('...')` instead of `Symbol.for('...')`
to keep the list of aspects on the construct object.

- The first version creates a unique symbol per library, while adding
  a naming hint.
- The second version deduplicates: all symbols with the same naming
  hint will receive the same symbol.

The second version is necessary to make sure that different copies
of the `aws-cdk-lib` library store their aspects under the same key.

Fixes #18921, #18778, #19390, #18914
@corymhall
Copy link
Contributor

The fix for this should have been released in v1.150.0 & v2.18.0. Let us know if you are still
seeing it.

@corymhall corymhall added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label May 24, 2022
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. p1 response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

5 participants