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

(codepipeline): Cannot deploy multiple copies of the same Pipeline with crossAccountKeys: true #18828

Closed
olivier-schmitt-sonarsource opened this issue Feb 4, 2022 · 23 comments
Assignees
Labels
@aws-cdk/pipelines CDK Pipelines library bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@olivier-schmitt-sonarsource
Copy link

olivier-schmitt-sonarsource commented Feb 4, 2022

What is the problem?

Hi,

In our context we need to have multiple pipelines per pull request, each pull request deploys it own set of stacks in the same region and account.

We use a namespace partition key we name 'env name': this partition key is then used as a suffix to all ressources we create. This allows to have multiple isolated environments and provide a lot of flexibility.

Reproduction Steps

A basic pipeline is created:

    def _create_domain_pipeline(self):
        domain_pipeline = pipelines.CodePipeline(
            self,
            'Pipeline',
            pipeline_name=f'Quickstart-Pipeline-{self._env_name}',
            self_mutation=True,
            synth=self._create_codestar_synth_step(),
            cross_account_keys=True,
            reuse_cross_region_support_stacks=False,
        )

If the cross_account_keys props is true then a KMS alias is created in the synthetised pipeline template:

  "PipelineArtifactsBucketEncryptionKeyAlias94A07392": {
      "Type": "AWS::KMS::Alias",
      "Properties": {
        "AliasName": "alias/codepipeline-quickstartpipelineb94f72c0",
        "TargetKeyId": {
          "Fn::GetAtt": [
            "PipelineArtifactsBucketEncryptionKeyF5BF0670",
            "Arn"
          ]
        }
      },

What did you expect to happen?

I would like to create muliple pipelines stacks with different names like when the option cross_account_keys is False.

Please note that this was possible with CDK 1.134.0 version.

What actually happened?

If try to create multiple envs with different env names such as PR-1 and PR-10 it fails when the option cross_account_keys is True because the same KMS alias is shared between all the environments.

When I synthetised multiple times the same CDK app I got one stack template per env name which is good:

assembly-Quickstart-Pipeline-HelloWorld-PR-3
assembly-Quickstart-Pipeline-HelloWorld-PR-45

But I have only one template Quickstart-Pipeline.template.json and this template contains the same AliasName for the KMS key so it fails because for instance I already deployed PR-3 pipeline and the KMS alias is already there when I try to deploy PR-45.

CDK CLI Version

2.10.0

Framework Version

2.10.0

Node.js Version

v16.13.0

OS

Mac Monterey

Language

Python

Language Version

3.8

Other information

No response

@olivier-schmitt-sonarsource olivier-schmitt-sonarsource added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 4, 2022
@github-actions github-actions bot added the @aws-cdk/pipelines CDK Pipelines library label Feb 4, 2022
@ryparker ryparker added the p2 label Feb 4, 2022
@mrpackethead
Copy link

you may be able to work around this, and rename the KMS alias's with escape hatches?

@olivier-schmitt-sonarsource
Copy link
Author

you may be able to work around this, and rename the KMS alias's with escape hatches?

This KMS alias is not under my control it seems, it is generated by the CDK construct when the cross_account_keys is set to True. I did not find a way from the construct itself to change this, but I might miss something.

Just got the info that this use case was supported with CDK 1.134.0.

@kornicameister
Copy link
Contributor

AFAIR you can iterate over stack nodes and locate KMS key in question. Once you do that you can make this name change. I think findAll is the one that you should look at. Not sure if you to use aspect here as well (maybe so)

@olivier-schmitt-sonarsource
Copy link
Author

AFAIR you can iterate over stack nodes and locate KMS key in question. Once you do that you can make this name change. I think findAll is the one that you should look at. Not sure if you to use aspect here as well (maybe so)

I would tend to avoid to patch CDK behaviour at runtime before I understand what is the intent of the CDK: is it expected to not support a basic use case to save KMS keys cost? Is it something else I don't understand?

Especially if the patch relies on CDK template generation implementation details on which there are no API contract.
I don't want to base our entire CDK CI/CD strategy on such workaround.

@gshpychka
Copy link
Contributor

@olivier-schmitt-sonarsource
Copy link
Author

After discussion with the AWS support, it's even worse it seems.

If you try to re use a similar pipeline structure in a completely different repository because this CDK pipeline app is good for many repositories, you might end up with the same conflict whatever the physical names of the ressources you are using in the other repository.

In fact the CDK synth might generate the same key alias name: it means you can't even share a pipeline blue print between repositories if you try to deploy in the same account and region (Dev account in Ireland let say).

But everything is working fine if cross_account_key is False.

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 8, 2022

The solution will be to incorporate the stack name into the hard-coded key alias.

I would tend to avoid to patch CDK behaviour at runtime before I understand what is the intent of the CDK: is it expected to not support a basic use case to save KMS keys cost? Is it something else I don't understand?

There is no intent: this is an oversight.

Using escape hatches is the recommended way to work around bugs until they are fixed.

@rix0rrr rix0rrr changed the title (pipelines): Not possible to have multiple dev pipelines (one per PR) in the same account and region (codepipeline): Cannot deploy multiple copies of the same Pipeline with crossAccountKeys: true Feb 8, 2022
@rix0rrr rix0rrr added effort/small Small work item – less than a day of effort p1 and removed p2 needs-triage This issue or PR still needs to be triaged. labels Feb 8, 2022
@rix0rrr rix0rrr removed their assignment Feb 8, 2022
@olivier-schmitt-sonarsource
Copy link
Author

Thanks @rix0rrr!

So is it officially a bug?

Does this construct should supports this use case?

It's important to us because we want to build our Ci/CD pipelines on top of this construct, so it's better to understand the supported use cases and what is the vision of the pipeline you have.

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 8, 2022

So is it officially a bug?

Yes, no reason this shouldn't have worked. The solution will be somewhat involved since it's going to require a new feature flag (so other people's key aliases aren't affected).

It's literally: we wanted to provide a semi-readable name for encryption keys, but accidentally pick one that would be the same across multiple copies of the same app, which breaks.

To work around this, you can either change the alias name, remove the alias (using escape hatches) or provide your own buckets with your own keys (or lack of keys).

@olivier-schmitt-sonarsource
Copy link
Author

Thanks! It's all super clear now.

@olivier-schmitt-sonarsource
Copy link
Author

@rix0rrr : I would like to elaborate about what I called "intent" and a "CI/CD strategy".

Back in the days, AWS posted a serie of articles to explain how to support typical use cases you need to face when you have to develop CI/CD pipeline.

So those articles were the following:

In every company there are multiple layers to this topic: project wise, domain wise, team wise, company wise, mutli- account, multi-region, ... pipelines can be shared via common constructs, specific to each team or domain, .... based on Git flow or other approaches.

This is basically the type of CI/CD problems we need to solve currently.

We are close to the second article from a use case perspective like a lot of other companies, it's very common.

Some AWS customers have already implemented something with the CDK:
https://pythonawesome.com/multi-branch-ci-cd-pipeline-using-cdk-pipelines/

Maybe I missed something in the CDK doc or elsewhere (I'm sorry if it's the case) but I would expect that there is a a support of those use cases at the CDK Pipelines level and a vision of which types of customers problems this construct is trying to solve.

Thanks for your time.

@olivier-schmitt-sonarsource
Copy link
Author

I tried the recommended patch approach to patch the key alias node but without success:

"PipelineArtifactsBucketEncryptionKeyAlias94A07392": {
      "Type": "AWS::KMS::Alias",
      "Properties": {
        "AliasName": "alias/codepipeline-quickstartpipelineb94f72c0",
        "TargetKeyId": {
          "Fn::GetAtt": [
            "PipelineArtifactsBucketEncryptionKeyF5BF0670",
            "Arn"
          ]
        }
      }

This node is not in the stack which hosts the pipeline it seems.
I'm able to find everything but this node.

class PipelineStack(Stack):
    def __init__(
            self,
            scope, id,
            env_name: str,
            env_type: str,
            branch_name: str,
            **kwargs
    ):
        super().__init__(scope, id, **kwargs)
        self._env_name, self._env_type = env_name, env_type
        self._branch_name = branch_name
        self._create_domain_pipeline()
        # Does not not found it!
        key_alias = self.node.try_find_child('PipelineArtifactsBucketEncryptionKeyAlias')
        # Found it
        pipeline = self.node.try_find_child('Pipeline')
        # Found it
        hello_world_stage = self.node.try_find_child(f'HelloWorld-{self._env_name}')

    def _create_domain_pipeline(self):
        domain_pipeline = pipelines.CodePipeline(
            self,
            'Pipeline',
            pipeline_name=f'Quickstart-Pipeline-{self._env_name}',
            self_mutation=True,
            synth=self._create_codestar_synth_step(),
            cross_account_keys=True, # Create somehow the PipelineArtifactsBucketEncryptionKeyAlias resource
            reuse_cross_region_support_stacks=False,
            code_build_defaults=self._create_codebuild_options(),
        )
        self._create_hello_world(domain_pipeline)

It seems I could access the synth template inside the Cloud assemble but it seems quite dirty to do that.

How would you patch this node?

Thanks.

@rix0rrr
Copy link
Contributor

rix0rrr commented Feb 11, 2022

@olivier-schmitt-sonarsource, you are right, the construct tree can be a bit hard to navigate without visualization. And something else that doesn't help you here: the Pipeline construct lazily generates its children as late as possible (so you can keep on calling add_stage() as often as you want), which makes the command you have to call to reify the construct tree a bit hidden.

I ended up using the AWS VSCode plugin, which has an explorer pane for the construct tree, to locate the key alias:

image

And then the following code to change the name:

pipeline.add_wave("wave").add_stage(stage)

# After the pipeline is finished
from constructs import Node

# This is important to create the resources eagerly (instead of lazily later on)
pipeline.build_pipeline()

codepipeline = Node.of(pipeline).find_child('Pipeline')
key = Node.of(codepipeline).find_child('ArtifactsBucketEncryptionKeyAlias')
cfn_key = Node.of(key).find_child('Resource')
cfn_key.alias_name = 'myUniqueAlias'

Leading to:

image

Sorry for the inconvenience

@olivier-schmitt-sonarsource
Copy link
Author

Thanks!

This is great, it's working!

I need to do more extensive tests of course but know I'm able to generate an alias I control: I took the pipeline name as a suffix which is a nice way to hint about the dependency/cardinality between this key and the pipeline.

So for the pipeline Quickstart-Pipeline-PR-6 the key alias is alias/quickstart-pipeline-pr-6.

This key alias name could be the default one and then the cardinality would work in any pipeline use case I guess.

Did you have the opportunity to reflect on the typical DevOps use cases I sent (Git flow, ...) and how the CDK Pipelines construct could integrate those into its vision/support?

Thanks again.

@TheRealAmazonKendra TheRealAmazonKendra self-assigned this Mar 7, 2022
TheRealAmazonKendra added a commit that referenced this issue Mar 16, 2022
…alias name (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This change fixes that using the stack name instead of the stack ID to create a unique ID for the alias name.

Closes issue #18828
TheRealAmazonKendra added a commit that referenced this issue Mar 22, 2022
…alias name (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This change fixes that using the stack name instead of the stack ID to create a unique ID for the alias name.

Closes issue #18828
TheRealAmazonKendra added a commit that referenced this issue Mar 22, 2022
… twice (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

@aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId

Fixes issue #18828.
TheRealAmazonKendra added a commit that referenced this issue Mar 22, 2022
… twice (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

@aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId

Fixes issue #18828.
@skinny85
Copy link
Contributor

skinny85 commented May 3, 2022

@olivier-schmitt-sonarsource can you help me understand your use case here? Are you deploying the exact same CodePipeline in multiple CDK apps, that are identical, except they differ in the Stack name?

The generated Alias should include the full path of that Key in your CDK path, including the ID (not name) of your Stack.

So, I'm curious what setup makes this behavior problematic.

@olivier-schmitt-sonarsource
Copy link
Author

@skinny85 : this use case is very common and supported by most CI solutions like Github Actions.

Many developers can work on the same code base that contains multiple services. Let's say a code repository contains the code base for a business domain, and this domain offers multiple service.

On our side we have one AWS account for Dev, one for Staging, one for Prod.

For some repo we can have 20 different PRs on the same code repo but the pipeline code itself is the same, we need to deploy 20 times the domain but with different resource names to avoid conflict between PRs.

To avoid conflict we use what call the env name partition key: for PR is the the id of the PR plus a prefix (PR-2, PR-3, ...).

This env name is used to compute all the resource names, this provides isolation between different PRs deployments.

It starts with all the stack names we deploy and then the env name is used inside each stack to specify resource names.

The problem is that the KMS key name is the same with the current CDK pipelines construct whatever the stack name is because the KMS key name is generated from the CDK tree which stays the same whatever the stack name. So only one pipeline based on the same source code can exist in one region whatever the parametrization used to try to deploy multiple pipeline instances. It can not work with the actual code.

@skinny85
Copy link
Contributor

skinny85 commented May 4, 2022

Thanks for the explanation.

To avoid conflict we use what call the env name partition key: for PR is the the id of the PR plus a prefix (PR-2, PR-3, ...).

This env name is used to compute all the resource names, this provides isolation between different PRs deployments.

It starts with all the stack names we deploy and then the env name is used inside each stack to specify resource names.

Can you use the env name in the logical ID of the resources as well, not only with the name? This would actually solve this problem for you. Just using it in the logical ID of the Stack will make this problem go away in your case.

@skinny85
Copy link
Contributor

skinny85 commented May 4, 2022

BTW, do you really need to deploy the entire Pipeline for each PR? Why not deploy only the application Stacks?

TheRealAmazonKendra added a commit that referenced this issue May 5, 2022
…alias name (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This change fixes that using the stack name instead of the stack ID to create a unique ID for the alias name.

Closes issue #18828
TheRealAmazonKendra added a commit that referenced this issue May 5, 2022
… twice (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

@aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId

Fixes issue #18828.
mergify bot pushed a commit that referenced this issue May 11, 2022
… twice (under feature flag) (#19418)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:
    
    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId
    
Fixes issue #18828.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
wphilipw pushed a commit to wphilipw/aws-cdk that referenced this issue May 23, 2022
… twice (under feature flag) (aws#19418)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:
    
    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId
    
Fixes issue aws#18828.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@badfun
Copy link
Contributor

badfun commented Jun 7, 2022

I have been bitten by the same bug and ended up here twice (not remembering the fix the first time). In my case, I am experiencing cross-stack issues, in a common account. In other words, I am using an account that has multiple pipelines, and are now being expanded to include deployments to other accounts. Once I add cross-account keys, I get a alias/codepipeline-pipelinestackpipelinee95eedaa already exists in stack error in CloudFormation. The logical id's are set to basic names like 'Pipeline' and 'PipelineStack', which I assume creates a conflict. I would have thought the hash at the end would be unique, but it is created in separate projects which have nothing to do with each other.
My only choice is to re-create the stacks with new logical ID's, or use the aforementioned patch, like so:

    /**
     * Workaround for crossAccountKeys bug
     * (https://github.com/aws/aws-cdk/issues/18828)
     */
    const codepipeline = pipeline.node.findChild('Pipeline')
    const key = codepipeline.node.findChild('ArtifactsBucketEncryptionKeyAlias')
    let cfnKey = key.node.findChild('Resource') as CfnAlias
    //override the property. Format must start with 'alias/'
    cfnKey.addPropertyOverride('AliasName', `alias/${project}`)

@skinny85
Copy link
Contributor

skinny85 commented Jun 8, 2022

You can also try using the fix merged in #19418 by enabling the @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId feature flag in your cdk.json, or by simply changing at least one logical ID in your Pipeline's construct tree path (that will make the aliases different too).

@badfun
Copy link
Contributor

badfun commented Jun 9, 2022

thanks for the tips @skinny85

@TheRealAmazonKendra
Copy link
Contributor

Post fix in #19418, these keys should now be 100% unique. You'll just need to enable the feature flag and/or upgrade to v2.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TheRealAmazonKendra added a commit that referenced this issue Jun 15, 2022
… twice (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId

Fixes issue #18828.
TheRealAmazonKendra added a commit that referenced this issue Jun 15, 2022
… twice (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId

Fixes issue #18828.
TheRealAmazonKendra added a commit that referenced this issue Jun 15, 2022
… twice (under feature flag)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This hcange fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeUniqueId

Fixes issue #18828.
mergify bot pushed a commit that referenced this issue Jun 15, 2022
… twice (under feature flag) (#20745)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This change fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeResourceName

Fixes issue #18828.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
daschaa pushed a commit to daschaa/aws-cdk that referenced this issue Jul 9, 2022
… twice (under feature flag) (aws#20745)

When multiple copies of the same pipeline are deployed in separate stacks, the alias name for the KMS key is the same, causing the deployment to fail. This change fixes that using the stack name instead of the stack ID to create a stack safe uniqueId for the alias name. This fix is behind the following feature flag:

    @aws-cdk/aws-codepipeline:crossAccountKeyAliasStackSafeResourceName

Fixes issue aws#18828.

*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/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

9 participants