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

[cdk-pipelines] stacks cannot be tagged #9260

Closed
roskelleycj opened this issue Jul 25, 2020 · 7 comments · Fixed by #10533
Closed

[cdk-pipelines] stacks cannot be tagged #9260

roskelleycj opened this issue Jul 25, 2020 · 7 comments · Fixed by #10533
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@roskelleycj
Copy link

roskelleycj commented Jul 25, 2020

It does not appear that pipeline applies aspects properly. Ideally, it should apply the aspects to itself and stages. stage appears to apply aspects that are added directly, but do not apply tagging aspects.

Reproduction Steps

See #9256 and #9259 for the initial steps. In that both of these cases showed higher level issues.

First I attempted to solve the PermissionsBoundary issue. I used the App.node.applyAspects(...) and received success for the initial pipeline stack. I.e., all roles created in the pipeline stack had the PermissionsBoundary from the aspect applied.

Problem solved, right? Well, then I continued with the tutorial and when I added a 'stage' the issue re-appeared. In that the stack associated with the Stage did not have the PermissionsBoundary aspect applied. So I did the next level pipeline.node.applyAspects(...) and still no result. So I did the next level stage.node.applyAspects(...) and successfully the PermissionsBoundary aspect was applied to all IAM Roles in the Stage/Stack.

At this point I noticed that the Tags had been lost. I traced that back to the first update after the create of the Pipeline STack. So I started the same adventure, only this time I figured that the PermissionsBoundary aspect was a clue and so I did not apply at each level, but started at the two points that I had found to work. I.e., Tags.add(stage, "key", "value") and sure enough it did not work. So I had to go down the next level to the 'stack' itself that the stage was composed of. And success as the tags were resources. However, the tags are missing on the stack itself.

Ideally, if tags are specified at the App they should be applied at each component. And because Tagging is an aspect this leads to the next issue of consistency.

Ideally, if an aspect is specified at the App it should be applied to all components, including the pipeline/stage/stacks/resources.

Error Log

Observed missing aspects.

Environment

  • CLI Version : 1.54.0 (build c01b9b9)
  • Framework Version:
  • Node.js Version: v12.18.3
  • OS : osx 10.15.6
  • Language (Version): all

Other

I have a private git repo with the tutorial on it and each commit I made to discover the issues described above. I can provide that if it would be of interest or help.


This is 🐛 Bug Report

@roskelleycj roskelleycj added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 25, 2020
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Jul 25, 2020
@ericzbeard ericzbeard removed the needs-triage This issue or PR still needs to be triaged. label Jul 27, 2020
@ericzbeard ericzbeard removed their assignment Jul 28, 2020
@njlynch njlynch removed their assignment Aug 3, 2020
@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 4, 2020

Unfortunately this works as intended. There are practical reasons why we can not propagate tags across Stage boundaries.

I will agree that this leads to unintuitive behavior. I will have a think on relaxing the restrictions somewhat or signaling the failure better. In the mean time, you need to apply the aspects to every stage individually.

Can you describe your use case a little? What are you trying to achieve?

@rix0rrr rix0rrr added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 and removed bug This issue is a bug. labels Aug 4, 2020
@roskelleycj
Copy link
Author

So I belong to an organization that requires a very specific tagging specification be applied to all AWS deployed resources. And AWS has, for several years, been making tagging a critical aspect of all resources. I.e., roadmap for AWS at large Obviously you are familiar with tags and their purpose. My organization is using them primarily as for cost tracking. The secondary purpose, someday?, will be security, as that appears to be another roadmap item.

So when you use CDK and you specify tags from the command-line one would expect that ALL stacks that are created would receive the tags that are provided. Looking at the architecture it appears that this is an aspect. And generally aspects are intended to be applied across a large number of constructs.

Ideally, the command-line should allow for 'include tags' and 'exclude tags', but alas it does not. Making it very hard to determine what the rule should be about tags that are not present in the request and the tags currently present on the resource. That is rather an unfortunate design flaw.

I'm assuming that since cdk-pipelines is in preview that this kind of item is something that should be addressed. It seems reasonable to me to confer with others in the CDK space to determine if the Tagging aspect is intended to be applied to all children. If not, then the deficiency of the Aspects should be documented in such away as to clarify any future misunderstandings.

@wsoula
Copy link

wsoula commented Aug 5, 2020

I've run into the issue he mentions where the stack [in the stage] itself is not tagged. I tried adding the tags using applyAspect on the stage and the stack and the stack was still not tagged. I am able to get resources within the stack tagged but not the stack itself. Is there anything I am overlooking?

@rix0rrr rix0rrr changed the title [cdk-pipelines] do not apply aspects from the app or stack properly [cdk-pipelines] stacks cannot be tagged Aug 12, 2020
@rix0rrr rix0rrr added bug This issue is a bug. and removed feature-request A feature should be added or improved. labels Aug 12, 2020
@rix0rrr rix0rrr added this to the [CDK Pipelines] Soon milestone Aug 12, 2020
@eikeon
Copy link

eikeon commented Sep 4, 2020

Ditto re: the stack [in the stage] itself not getting tagged

@michaelfecher
Copy link

michaelfecher commented Sep 21, 2020

Had the same issue (in combination with CDK-pipelines + stages).
Additionally, verified it with @udondan that the tags argument on a pure stack (without cdk-pipelines involved) isn't putting tags on the CF resources of the stack itself. Using v1.63.0.

@nbaillie
Copy link
Contributor

nbaillie commented Sep 22, 2020

This is also causing issues for me, as in my case the pipeline cant create change sets that dont have tags for the stacks in the stage due to condition restrictions. when using pipeline.addApplicationStage(x) it fails "not authorized to perform: cloudformation:CreateChangeSet" when deploying the application stage in pipeline.

Are there any workarounds?

Will it be needed to expose the TemplateConfiguration of the changeset action as described here:
CloudFormation configuration properties and allow setting the tags with a Template configuration file (perhaps a asset?)
as described here: Template configuration file

@rix0rrr rix0rrr added p1 and removed p2 labels Sep 23, 2020
@nbaillie
Copy link
Contributor

nbaillie commented Sep 24, 2020

I have implemented a workaround for now.

Added the TemplateConfiguration Json file to the Synth Output like this :

const actionConfig = new AssetStaging(this, 'actionConfig', {
      sourcePath: 'lib/configuration.template.json'
    })

Create the Pipeline then type it to cfnPipeline:

const pipeline = new CdkPipeline(,,{});
const cfnPipeline = pipeline.codePipeline.node.defaultChild as CfnPipeline

Then addPropertyOverride based on the number of stages at the point where the application Stage is added.
Note - Create Change set will always be an Action with an Even Number when used with addApplicationStage and in this case.

// Add ApplicationStage stages
```
const someStage = new SomeStage(this, 'SomeStage', someStageProps);
pipeline.addApplicationStage(someStage);
const someStageNumber = pipeline.codePipeline.stageCount - 2

// Fix Tags by Override
    ```
cfnPipeline.addPropertyOverride(`Stages.${devStageNumber}.Actions.0.Configuration.TemplateConfiguration`,
      `Artifact_Build_Synth::${actionConfig.stagedPath}`
    ); #

rix0rrr added a commit that referenced this issue Sep 25, 2020
Apply stack tags to the stacks deployed using CDK Pipelines.

Fixes #9260.
@mergify mergify bot closed this as completed in #10533 Sep 28, 2020
mergify bot pushed a commit that referenced this issue Sep 28, 2020
Apply stack tags to the stacks deployed using CDK Pipelines.

Taking this opportunity to make tags easier to work with -- move them from metadata into cloud artifact properties.

Fixes #9260.


----

*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 join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants