Skip to content

Commit

Permalink
chore(codepipeline): revert "cannot deploy pipeline stack with crossA…
Browse files Browse the repository at this point in the history
…ccountKeys twice" (#20427)

This PR fails in CDK v2 because the added unit tests that use `testFutureBehavior` fail on the CDK v2 branch. I believe they're failing because the `testFutureBehavior` utility function was written before CDK v2 was released, and so it automatically discards all feature flags - which should not be happening for new feature flags. I'm not sure what the best fix for this is so I'm just reverting it for the time being to unblock the release.

Test logs:
<details>

```
FAIL test/pipeline.test.js (12.04 s)
  ●  › cross account key alias name tests › cross account key alias is named with stack name instead of ID when feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-pipelinestackpipeline9db740af",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "PipelineArtifactsBucketEncryptionKey01D58D69",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-actual-stack-name-pipeline-0a412eb5 but received alias/codepipeline-pipelinestackpipeline9db740af at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:500:33)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias is named with generated stack name when stack name is undefined and feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-pipelinestackpipeline9db740af",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "PipelineArtifactsBucketEncryptionKey01D58D69",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-pipelinestack-pipeline-9db740af but received alias/codepipeline-pipelinestackpipeline9db740af at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:525:33)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias is named with stack name and nested stack ID when feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-toplevelstacknestedpipelinestackactualpipeline3161a537",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "ActualPipelineArtifactsBucketEncryptionKeyDF448A3D",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-actual-stack-name-nestedpipelinestack-actualpipeline-23a98110 but received alias/codepipeline-toplevelstacknestedpipelinestackactualpipeline3161a537 at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:552:46)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias is named with generated stack name and nested stack ID when stack name is undefined and feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-toplevelstacknestedpipelinestackactualpipeline3161a537",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "ActualPipelineArtifactsBucketEncryptionKeyDF448A3D",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-toplevelstack-nestedpipelinestack-actualpipeline-3161a537 but received alias/codepipeline-toplevelstacknestedpipelinestackactualpipeline3161a537 at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:581:46)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias is properly shortened to 256 characters when stack name is too long and feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-toolongactualpipelinewithextrasuperlongnamethatwillneedtobeshortenedduetothealsoverysuperextralongnameofthestackalsowithsomedifferentcharactersaddedtotheendc9bb503e",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "ActualPipelineWithExtraSuperLongNameThatWillNeedToBeShortenedDueToTheAlsoVerySuperExtraLongNameOfTheStackAlsoWithSomeDifferentCharactersAddedToTheEndArtifactsBucketEncryptionKeyABD1BD7F",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-actual-stack-needstobeshortenedduetothelengthofthisabsurdnamethatnooneshouldusebutitstillmighthappensowemusttestfohatwillneedtobeshortenedduetothealsoverysuperextralongnameofthestack-alsowithsomedifferentcharactersaddedtotheend-384b9343 but received alias/codepipeline-toolongactualpipelinewithextrasuperlongnamethatwillneedtobeshortenedduetothealsoverysuperextralongnameofthestackalsowithsomedifferentcharactersaddedtotheendc9bb503e at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:609:33)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias names do not conflict when the stack ID is the same and pipeline ID is the same and feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-stackidpipeline32fb88b3",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "PipelineArtifactsBucketEncryptionKey01D58D69",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-actual-stack-1-pipeline-b09fefee but received alias/codepipeline-stackidpipeline32fb88b3 at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:643:34)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias names do not conflict for nested stacks when pipeline ID is the same and nested stacks have the same ID when feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-stackidnestedpipelineid3e91360a",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "PIPELINEIDArtifactsBucketEncryptionKeyE292C50C",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-actual-stack-name-1-nested-pipeline-id-c8c9f252 but received alias/codepipeline-stackidnestedpipelineid3e91360a at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:697:47)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)

  ●  › cross account key alias name tests › cross account key alias names do not conflict for nested stacks when in the same stack but nested stacks have different IDs when feature flag is enabled

    Template has 1 resources with type AWS::KMS::Alias, but none match as expected.
    The closest result is:
      {
        "Type": "AWS::KMS::Alias",
        "Properties": {
          "AliasName": "alias/codepipeline-stackidfirstpipelineid5abca693",
          "TargetKeyId": {
            "Fn::GetAtt": [
              "PIPELINEIDArtifactsBucketEncryptionKeyE292C50C",
              "Arn"
            ]
          }
        },
        "UpdateReplacePolicy": "Delete",
        "DeletionPolicy": "Delete"
      }
    with the following mismatches:
    	Expected alias/codepipeline-actual-stack-name-1-first-pipeline-id-3c59cb88 but received alias/codepipeline-stackidfirstpipelineid5abca693 at /Properties/AliasName (using objectLike matcher)

      83 |     const matchError = hasResourceProperties(this.template, type, props);
      84 |     if (matchError) {
    > 85 |       throw new Error(matchError);
         |             ^
      86 |     }
      87 |   }
      88 |

      at Template.hasResourceProperties (../assertions/lib/template.ts:85:13)
      at fn (test/pipeline.test.ts:749:46)
      at Object.<anonymous> (../../../tools/@aws-cdk/cdk-build-tools/lib/feature-flag.ts:34:35)
```

</details>

----

### All Submissions:

* [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Chriscbr authored May 19, 2022
1 parent ae64183 commit f9552c0
Show file tree
Hide file tree
Showing 21 changed files with 20 additions and 672 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-cloudformation-pipeline-7dbde619",
"AliasName": "alias/codepipeline-awscdkcodepipelinecloudformationpipeline7dbde619",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-pipelinestack-pipeline-9db740af",
"AliasName": "alias/codepipeline-pipelinestackpipeline9db740af",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-lambda-pipeline-87a4b3d3",
"AliasName": "alias/codepipeline-awscdkcodepipelinelambdapipeline87a4b3d3",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-alexa-deploy-pipeline-961107f5",
"AliasName": "alias/codepipeline-awscdkcodepipelinealexadeploypipeline961107f5",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-cloudformation-pipeline-7dbde619",
"AliasName": "alias/codepipeline-awscdkcodepipelinecloudformationpipeline7dbde619",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-codecommit-codebuild-pipeline-9540e1f5",
"AliasName": "alias/codepipeline-awscdkcodepipelinecodecommitcodebuildpipeline9540e1f5",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@
"PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-codecommit-pipeline-f780ca18",
"AliasName": "alias/codepipeline-awscdkcodepipelinecodecommitpipelinef780ca18",
"TargetKeyId": {
"Fn::GetAtt": [
"PipelineArtifactsBucketEncryptionKey01D58D69",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"MyPipelineArtifactsBucketEncryptionKeyAlias9D4F8C59": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-pipeline-event-target-mypipeline-4ae5d407",
"AliasName": "alias/codepipeline-awscdkpipelineeventtargetmypipeline4ae5d407",
"TargetKeyId": {
"Fn::GetAtt": [
"MyPipelineArtifactsBucketEncryptionKey8BF0A7F3",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
"MyPipelineArtifactsBucketEncryptionKeyAlias9D4F8C59": {
"Type": "AWS::KMS::Alias",
"Properties": {
"AliasName": "alias/codepipeline-aws-cdk-codepipeline-stepfunctions-mypipeline-ce88aa28",
"AliasName": "alias/codepipeline-awscdkcodepipelinestepfunctionsmypipelinece88aa28",
"TargetKeyId": {
"Fn::GetAtt": [
"MyPipelineArtifactsBucketEncryptionKey8BF0A7F3",
Expand Down
19 changes: 4 additions & 15 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
ArnFormat,
BootstraplessSynthesizer,
DefaultStackSynthesizer,
FeatureFlags,
IStackSynthesizer,
Lazy,
Names,
Expand All @@ -18,7 +17,6 @@ import {
Stage as CdkStage,
Token,
} from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Construct } from 'constructs';
import { ActionCategory, IAction, IPipeline, IStage, PipelineNotificationEvents, PipelineNotifyOnOptions } from './action';
import { CfnPipeline } from './codepipeline.generated';
Expand Down Expand Up @@ -699,19 +697,10 @@ export class Pipeline extends PipelineBase {
private generateNameForDefaultBucketKeyAlias(): string {
const prefix = 'alias/codepipeline-';
const maxAliasLength = 256;
const maxResourceNameLength = maxAliasLength - prefix.length;
// Names.uniqueId() may have naming collisions when the IDs of resources are similar
// and/or when they are too long and sliced. We do not want to update this and
// automatically change the name of every KMS key already generated so we are putting
// this under a feature flag.
const uniqueId = FeatureFlags.of(this).isEnabled(cxapi.CODEPIPELINE_CROSS_ACCOUNT_KEY_ALIAS_STACK_SAFE_UNIQUE_ID) ?
Names.uniqueResourceName(this, {
separator: '-',
maxLength: maxResourceNameLength,
allowedSpecialCharacters: '/_-',
}) :
Names.uniqueId(this).slice(-maxResourceNameLength);
return prefix + uniqueId.toLowerCase();
const uniqueId = Names.uniqueId(this);
// take the last 256 - (prefix length) characters of uniqueId
const startIndex = Math.max(0, uniqueId.length - (maxAliasLength - prefix.length));
return prefix + uniqueId.substring(startIndex).toLowerCase();
}

/**
Expand Down
3 changes: 1 addition & 2 deletions packages/@aws-cdk/aws-codepipeline/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.0",
"jest": "^27.5.1"
Expand All @@ -99,7 +100,6 @@
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"constructs": "^3.3.69"
},
"homepage": "https://github.com/aws/aws-cdk",
Expand All @@ -110,7 +110,6 @@
"@aws-cdk/aws-kms": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/core": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"constructs": "^3.3.69"
},
"engines": {
Expand Down
Loading

0 comments on commit f9552c0

Please sign in to comment.