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

No AWS Tagging Support on Stacks themselves #932

Closed
lestephane opened this issue Oct 15, 2018 · 11 comments · Fixed by #2185 or MechanicalRock/tech-radar#14 · May be fixed by MechanicalRock/cdk-constructs#5, MechanicalRock/cdk-constructs#6 or MechanicalRock/cdk-constructs#7
Labels
feature-request A feature should be added or improved.

Comments

@lestephane
Copy link

When I saw in another issue that tag propagation was added, my first instinct
was to try to set tags on the software.amazon.awscdk.App (not possible,
does not implement ITaggable, at least in java), then i tried to set tags on the
software.amazon.awscdk.Stack (same problem).

My use case is to (1) add tags on the App level with propagation, that (2) will be
set in all the AWS::CloudFormation::Stack resources created by cdk deploy, and
in turn (3) will be set on all the resources created within said stacks.

I tried implementing ITaggable on my App subclass and my Stack subclass.
The tags get passed and set all the way to the resources all right, so that's (1) and (3) covered.
But what about (2)? If I understand the design correctly, you're completely reliant
on changesets, and the tags for the Stacks cannot be passed in the stack template.
They would have to be passed in the api call to createChangeSet:

deploy-stack.ts

  const changeSet = await cfn.createChangeSet({
    StackName: deployName,
    ChangeSetName: changeSetName,
    ChangeSetType: update ? 'UPDATE' : 'CREATE',
    Description: `CDK Changeset for execution ${executionId}`,
    TemplateBody: bodyParameter.TemplateBody,
    TemplateURL: bodyParameter.TemplateURL,
    Parameters: params,
    Capabilities: [ 'CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM' ]
    # ADD TAGS SOMEWHERE HERE?
  }).promise();

Makes sense?

@rafcio19
Copy link

rafcio19 commented Apr 3, 2019

Tagging stacks would be very useful, it's supported by virtually all other CF deployment tools :)

@eladb
Copy link
Contributor

eladb commented Apr 3, 2019

Thanks to the awesome work contributed by @moofish32, this is naturally supported using aspects.

Here is a python version (my new favorite way to use the CDK):

from aws_cdk import cdk
from aws_cdk import aws_sqs
from aws_cdk import aws_dynamodb


class MyStack(cdk.Stack):
    def __init__(self, scope: cdk.Construct, cid: str, **kwargs) -> None:
        super().__init__(scope, cid, **kwargs)

        aws_sqs.Queue(self, 'MyQueue')
        aws_dynamodb.Table(self, 'MyTable',
                           partition_key=aws_dynamodb.Attribute(
                               name='id',
                               type=aws_dynamodb.AttributeType.String))


app = cdk.App()

stack = MyStack(app, 'stack1', stack_name='first-python-app')
stack.node.apply(cdk.Tag('Foo', 'Bar'))

app.run()

In TypeScript:

stack.node.apply(new cdk.Tag('Foo', 'Bar'));

The result:

Resources:
  MyQueueE6CA6235:
    Type: AWS::SQS::Queue
    Properties:
      Tags:
        - Key: Foo
          Value: Bar
  MyTable794EDED1:
    Type: AWS::DynamoDB::Table
    Properties:
      KeySchema:
        - AttributeName: id
          KeyType: HASH
      AttributeDefinitions:
        - AttributeName: id
          AttributeType: S
      ProvisionedThroughput:
        ReadCapacityUnits: 5
        WriteCapacityUnits: 5
      Tags:
        - Key: Foo
          Value: Bar

@eladb eladb closed this as completed Apr 3, 2019
@IsmaelMartinez
Copy link
Contributor

Hi @eladb , I have tried this but the stack itself is not been tagged. I think this issue should still be open.

@eladb
Copy link
Contributor

eladb commented Apr 3, 2019

oh... okay got it. I don't know if that's possible to do through the template, but it's a cool feature to support through the CLI.

@eladb
Copy link
Contributor

eladb commented Apr 3, 2019

Reopening

@eladb eladb reopened this Apr 3, 2019
@rafcio19
Copy link

rafcio19 commented Apr 4, 2019

@eladb thanks, this would need to be implemented alongside CreateStack and UpdateStack APIs, both support Tags request param:

https://docs.aws.amazon.com/AWSCloudFormation/latest/APIReference/API_CreateStack.html

The advantage of tagging this way is all the resources in the stack(with very few exceptions) will get tagged with these stack tags so individual resources don't have to. One API call vs number_of_resources * API calls

@IsmaelMartinez
Copy link
Contributor

I have done a local PoC with hard-coded values and seems to work fine. I will start the process to create add this as soon as I got time (probably next week from Wednesday). It might take a bit of time but I do the heads-up.

@moofish32
Copy link
Contributor

moofish32 commented Apr 5, 2019

@rafcio19 I agree on the reopen as we did not implement the CLI aspect of this which tags the actual stack. I do want to ask about this comment:

The advantage of tagging this way is all the resources in the stack(with very few exceptions) will get tagged with these stack tags so individual resources don't have to. One API call vs number_of_resources * API calls

Tags will be applied to all stack resources that support tagging with the example referenced @eladb. You can see more details here. I think we are accomplishing all this work for the user or did you mean something different?

@IsmaelMartinez
Copy link
Contributor

IsmaelMartinez commented Apr 5, 2019

Hi @moofish32

I got a hack poc of this and I can confirm tags applied via the CLI aspect, applied to the stack itself and to the resources the stack has (in my case, only a dynamodb)

We are using the feature that you implemented, and can't thank you enough for it, and I can see those tags only applied to the dynamodb but not to the stack itself.

Happy to share the hack code in here but is basically following @lestephane suggested change.

By the way, the suggested change did work for create and update.

I didn't test if by only changing the tags it does update the tags in the stack and resources (if you know what I mean)

Happy to help on this or start implementing it, but the later might take a few days due time constrains.

Hope helps

@moofish32
Copy link
Contributor

@IsmaelMartinez

I totally understand the gap you are closing. Going all the way back to the top, this issue is now focused only on item (2) from @lestephane original bug.

My only point was to @rafcio19 in attempt to clarify for other users. If you only need to tag resources, that problem is solved with the current features and you have more control if needed. If you actually need the describe-stacks API to return tags on the stack, then after your PR is merged this issue will be complete. Thanks for contributing!

@IsmaelMartinez
Copy link
Contributor

Ok, I got a PR almost ready. I just need to go via the Contribute process and hope that I haven't miss anything. This should get referenced fairly soon.

eladb pushed a commit that referenced this issue Jun 3, 2019
Adding tags parameter option to cdk deploy command to allow tagging full stacks and their associated resources. 

Now it will be possible to:

```
const app = new App();

const stack1 = new Stack(app, 'stack1', { tags: { foo: 'bar' } });
const stack2 = new Stacl(app, 'stack2');

stack1.node.apply(new Tag('fii', 'bug'));
stack2.node.apply(new Tag('boo', 'bug'));
```
That will produce 
* stack1 with tags `foo bar` and `fii bug`
* stack2 with tags `boo bug`

It is possible also to override constructor tags with the stack.node.apply. 

So doing:
```
stack1.node.apply(new Tag('foo', 'newBar');
```
stack1 will have tags `foo newBar` and `fii bug`

Last, but not least, it is also possible to pass it via arguments (using yargs) as in the following example:

```
cdk deploy --tags foo=bar --tags myTag=myValue
```
That will produce a stack with tags `foo bar`and `myTag myValue`

**Important**
That will ignore tags provided by the constructor and/or aspects. 

Fixes #932
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment