diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.cfn-template-from-repo.lit.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.cfn-template-from-repo.lit.expected.json index dcc7e89566439..ce1f5d554bd45 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.cfn-template-from-repo.lit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.cfn-template-from-repo.lit.expected.json @@ -131,6 +131,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelinecloudformationpipeline7dbde619", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -379,10 +393,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json index 52a0ab9813f1a..a9cd3292bc0df 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-deployed-through-codepipeline.lit.expected.json @@ -182,6 +182,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-pipelinestackpipeline9db740af", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -504,10 +518,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json index 9d4c58d45426c..d1e562e2204e0 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.lambda-pipeline.expected.json @@ -107,6 +107,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelinelambdapipeline87a4b3d3", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -282,10 +296,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-alexa-deploy.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-alexa-deploy.expected.json index 1471e9f0443f9..4f6d52c4c1829 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-alexa-deploy.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-alexa-deploy.expected.json @@ -117,6 +117,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelinealexadeploypipeline961107f5", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -281,10 +295,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn.expected.json index dd5b6c6f849ba..44990099303a5 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-cfn.expected.json @@ -140,6 +140,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelinecloudformationpipeline7dbde619", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -367,10 +381,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit-build.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit-build.expected.json index 3609e47f5bf10..c18c56d5f9e4f 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit-build.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit-build.expected.json @@ -355,6 +355,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelinecodecommitcodebuildpipeline9540e1f5", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "UpdateReplacePolicy": "Retain", + "DeletionPolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -579,10 +593,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json index 6dcd3a65d6c81..5c53fa46db4ef 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-code-commit.expected.json @@ -180,6 +180,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelinecodecommitpipelinef780ca18", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -353,10 +367,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json index eb4ea22cafcf1..8ab452d92da02 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-events.expected.json @@ -128,6 +128,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "MyPipelineArtifactsBucketEncryptionKeyAlias9D4F8C59": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkpipelineeventtargetmypipeline4ae5d407", + "TargetKeyId": { + "Fn::GetAtt": [ + "MyPipelineArtifactsBucketEncryptionKey8BF0A7F3", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "MyPipelineRoleC0D47CA4": { "Type": "AWS::IAM::Role", "Properties": { @@ -316,10 +330,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "MyPipelineArtifactsBucketEncryptionKey8BF0A7F3", - "Arn" - ] + "Ref": "MyPipelineArtifactsBucketEncryptionKey8BF0A7F3" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-s3-deploy.expected.json b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-s3-deploy.expected.json index ee6a7452567f6..59590ad989c44 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-s3-deploy.expected.json +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/integ.pipeline-s3-deploy.expected.json @@ -138,6 +138,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-awscdkcodepipelines3deploypipeline907bf1e7", + "TargetKeyId": { + "Fn::GetAtt": [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "PipelineRoleD68726F7": { "Type": "AWS::IAM::Role", "Properties": { @@ -318,10 +332,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn" - ] + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-codepipeline/README.md b/packages/@aws-cdk/aws-codepipeline/README.md index 40dab2d19072a..4e439c665aa90 100644 --- a/packages/@aws-cdk/aws-codepipeline/README.md +++ b/packages/@aws-cdk/aws-codepipeline/README.md @@ -88,13 +88,18 @@ into a different region than your Pipeline is in. It works like this: -```ts +```typescript const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', { // ... crossRegionReplicationBuckets: { // note that a physical name of the replication Bucket must be known at synthesis time - 'us-west-1': s3.Bucket.fromBucketName(this, 'UsWest1ReplicationBucket', - 'my-us-west-1-replication-bucket'), + 'us-west-1': s3.Bucket.fromBucketAttributes(this, 'UsWest1ReplicationBucket', { + bucketName: 'my-us-west-1-replication-bucket', + // optional KMS key + encryptionKey: kms.Key.fromKeyArn(this, 'UsWest1ReplicationKey', + 'arn:aws:kms:us-west-1:123456789012:key/1234-5678-9012' + ), + }), }, }); @@ -128,6 +133,53 @@ $ cdk deploy MyMainStack See [the AWS docs here](https://docs.aws.amazon.com/codepipeline/latest/userguide/actions-create-cross-region.html) for more information on cross-region CodePipelines. +#### Creating an encrypted replication bucket + +If you're passing a replication bucket created in a different stack, +like this: + +```typescript +const replicationStack = new Stack(app, 'ReplicationStack', { + env: { + region: 'us-west-1', + }, +}); +const key = new kms.Key(replicationStack, 'ReplicationKey'); +const replicationBucket = new s3.Bucket(replicationStack, 'ReplicationBucket', { + // like was said above - replication buckets need a set physical name + bucketName: PhysicalName.GENERATE_IF_NEEDED, + encryptionKey: key, // does not work! +}); + +// later... +new codepipeline.Pipeline(pipelineStack, 'Pipeline', { + crossRegionReplicationBuckets: { + 'us-west-1': replicationBucket, + }, +}); +``` + +When trying to encrypt it +(and note that if any of the cross-region actions happen to be cross-account as well, +the bucket *has to* be encrypted - otherwise the pipeline will fail at runtime), +you cannot use a key directly - KMS keys don't have physical names, +and so you can't reference them across environments. + +In this case, you need to use an alias in place of the key when creating the bucket: + +```typescript +const key = new kms.Key(replicationStack, 'ReplicationKey'); +const alias = new kms.Alias(replicationStack, 'ReplicationAlias', { + // aliasName is required + aliasName: PhysicalName.GENERATE_IF_NEEDED, + targetKey: key, +}); +const replicationBucket = new s3.Bucket(replicationStack, 'ReplicationBucket', { + bucketName: PhysicalName.GENERATE_IF_NEEDED, + encryptionKey: alias, +}); +``` + ### Events #### Using a pipeline as an event target diff --git a/packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts b/packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts index e916924d3ac24..f87d6655fa41f 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/cross-region-support-stack.ts @@ -1,3 +1,4 @@ +import kms = require('@aws-cdk/aws-kms'); import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/core'); @@ -44,8 +45,15 @@ export class CrossRegionSupportStack extends cdk.Stack { }, }); + const encryptionKey = new kms.Key(this, 'CrossRegionCodePipelineReplicationBucketEncryptionKey'); + const encryptionAlias = new kms.Alias(this, 'CrossRegionCodePipelineReplicationBucketEncryptionAlias', { + targetKey: encryptionKey, + aliasName: cdk.PhysicalName.GENERATE_IF_NEEDED, + removalPolicy: cdk.RemovalPolicy.RETAIN, + }); this.replicationBucket = new s3.Bucket(this, 'CrossRegionCodePipelineReplicationBucket', { bucketName: cdk.PhysicalName.GENERATE_IF_NEEDED, + encryptionKey: encryptionAlias, }); } } diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 3cdc89d84c075..c3f73a12dafab 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -231,6 +231,12 @@ export class Pipeline extends PipelineBase { encryption: s3.BucketEncryption.KMS, removalPolicy: RemovalPolicy.RETAIN }); + // add an alias to make finding the key in the console easier + new kms.Alias(this, 'ArtifactsBucketEncryptionKeyAlias', { + aliasName: this.generateNameForDefaultBucketKeyAlias(), + targetKey: encryptionKey, + removalPolicy: RemovalPolicy.RETAIN, // alias should be retained, like the key + }); } this.artifactBucket = propsBucket; @@ -240,8 +246,8 @@ export class Pipeline extends PipelineBase { }); const codePipeline = new CfnPipeline(this, 'Resource', { - artifactStore: Lazy.anyValue({ produce: () => this.renderArtifactStore() }), - artifactStores: Lazy.anyValue({ produce: () => this.renderArtifactStores() }), + artifactStore: Lazy.anyValue({ produce: () => this.renderArtifactStoreProperty() }), + artifactStores: Lazy.anyValue({ produce: () => this.renderArtifactStoresProperty() }), stages: Lazy.anyValue({ produce: () => this.renderStages() }), roleArn: this.role.roleArn, restartExecutionOnUpdate: props && props.restartExecutionOnUpdate, @@ -367,9 +373,10 @@ export class Pipeline extends PipelineBase { return this.artifactBucket; } + const pipelineStack = Stack.of(this); + let otherStack: Stack; let replicationBucket = this.crossRegionReplicationBuckets[region]; if (!replicationBucket) { - const pipelineStack = Stack.of(this); const pipelineAccount = pipelineStack.account; if (Token.isUnresolved(pipelineAccount)) { throw new Error("You need to specify an explicit account when using CodePipeline's cross-region support"); @@ -382,23 +389,34 @@ export class Pipeline extends PipelineBase { account: pipelineAccount, }); replicationBucket = crossRegionScaffoldStack.replicationBucket; - pipelineStack.addDependency(crossRegionScaffoldStack); this._crossRegionSupport[region] = { stack: crossRegionScaffoldStack, replicationBucket, }; this.crossRegionReplicationBuckets[region] = replicationBucket; + otherStack = crossRegionScaffoldStack; + } else { + otherStack = Stack.of(replicationBucket); } + + // the stack containing the replication bucket must be deployed before the pipeline + pipelineStack.addDependency(otherStack); replicationBucket.grantReadWrite(this.role); - this.artifactStores[region] = { - location: replicationBucket.bucketName, - type: 'S3', - }; + this.artifactStores[region] = this.renderArtifactStore(replicationBucket); return replicationBucket; } + private generateNameForDefaultBucketKeyAlias(): string { + const prefix = 'alias/codepipeline-'; + const maxAliasLength = 256; + const uniqueId = this.node.uniqueId; + // 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(); + } + /** * Gets the role used for this action, * including handling the case when the action is supposed to be cross-account. @@ -644,7 +662,7 @@ export class Pipeline extends PipelineBase { return ret; } - private renderArtifactStores(): CfnPipeline.ArtifactStoreMapProperty[] | undefined { + private renderArtifactStoresProperty(): CfnPipeline.ArtifactStoreMapProperty[] | undefined { if (!this.crossRegion) { return undefined; } // add the Pipeline's artifact store @@ -657,30 +675,29 @@ export class Pipeline extends PipelineBase { })); } - private renderArtifactStore(): CfnPipeline.ArtifactStoreProperty | undefined { + private renderArtifactStoreProperty(): CfnPipeline.ArtifactStoreProperty | undefined { if (this.crossRegion) { return undefined; } return this.renderPrimaryArtifactStore(); } private renderPrimaryArtifactStore(): CfnPipeline.ArtifactStoreProperty { + return this.renderArtifactStore(this.artifactBucket); + } + + private renderArtifactStore(bucket: s3.IBucket): CfnPipeline.ArtifactStoreProperty { let encryptionKey: CfnPipeline.EncryptionKeyProperty | undefined; - const bucketKey = this.artifactBucket.encryptionKey; + const bucketKey = bucket.encryptionKey; if (bucketKey) { encryptionKey = { type: 'KMS', - id: bucketKey.keyArn, + id: bucketKey.keyId, }; } - const bucketName = this.artifactBucket.bucketName; - if (!bucketName) { - throw new Error('Artifacts bucket must have a bucket name'); - } - return { type: 'S3', - location: bucketName, - encryptionKey + location: bucket.bucketName, + encryptionKey, }; } diff --git a/packages/@aws-cdk/aws-codepipeline/test/fake-build-action.ts b/packages/@aws-cdk/aws-codepipeline/test/fake-build-action.ts index 5e29ffc05ef0d..20045461296d3 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/fake-build-action.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/fake-build-action.ts @@ -15,6 +15,8 @@ export interface FakeBuildActionProps extends codepipeline.CommonActionProps { role?: iam.IRole; account?: string; + + region?: string; } export class FakeBuildAction implements codepipeline.IAction { diff --git a/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts b/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts index 275596de6c826..c918c9dfcd6b2 100644 --- a/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/test/test.pipeline.ts @@ -1,5 +1,7 @@ -import { expect, haveResourceLike } from '@aws-cdk/assert'; +import { expect, haveResourceLike, ResourcePart } from '@aws-cdk/assert'; import iam = require('@aws-cdk/aws-iam'); +import kms = require('@aws-cdk/aws-kms'); +import s3 = require('@aws-cdk/aws-s3'); import cdk = require('@aws-cdk/core'); import { Test } from 'nodeunit'; import codepipeline = require('../lib'); @@ -43,6 +45,218 @@ export = { test.done(); }, + 'that is cross-region': { + 'allows passing an Alias in place of the KMS Key in the replication Bucket'(test: Test) { + const app = new cdk.App(); + + const replicationRegion = 'us-west-1'; + const replicationStack = new cdk.Stack(app, 'ReplicationStack', { + env: { region: replicationRegion, account: '123456789012' }, + }); + const replicationKey = new kms.Key(replicationStack, 'ReplicationKey'); + const replicationAlias = replicationKey.addAlias('alias/my-replication-alias'); + const replicationBucket = new s3.Bucket(replicationStack, 'ReplicationBucket', { + encryptionKey: replicationAlias, + bucketName: cdk.PhysicalName.GENERATE_IF_NEEDED, + }); + + const pipelineRegion = 'us-west-2'; + const pipelineStack = new cdk.Stack(app, 'PipelineStack', { + env: { region: pipelineRegion, account: '123456789012' }, + }); + const sourceOutput = new codepipeline.Artifact(); + new codepipeline.Pipeline(pipelineStack, 'Pipeline', { + crossRegionReplicationBuckets: { + [replicationRegion]: replicationBucket, + }, + stages: [ + { + stageName: 'Source', + actions: [new FakeSourceAction({ + actionName: 'Source', + output: sourceOutput, + })], + }, + { + stageName: 'Build', + actions: [new FakeBuildAction({ + actionName: 'Build', + input: sourceOutput, + region: replicationRegion, + })], + }, + ], + }); + + expect(pipelineStack).to(haveResourceLike('AWS::CodePipeline::Pipeline', { + "ArtifactStores": [ + { + "Region": replicationRegion, + "ArtifactStore": { + "Type": "S3", + "EncryptionKey": { + "Type": "KMS", + "Id": "alias/my-replication-alias", + }, + }, + }, + { + "Region": pipelineRegion, + }, + ], + })); + + expect(replicationStack).to(haveResourceLike('AWS::KMS::Key', { + "KeyPolicy": { + "Statement": [ + { + // owning account management permissions - we don't care about them in this test + }, + { + // KMS verifies whether the principal given in its key policy exists when creating that key. + // Since the replication bucket must be deployed before the pipeline, + // we cannot put the pipeline role as the principal here - + // hence, we put the account itself + "Action": [ + "kms:Decrypt", + "kms:DescribeKey", + "kms:Encrypt", + "kms:ReEncrypt*", + "kms:GenerateDataKey*", + ], + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::Join": ["", [ + "arn:", + { "Ref": "AWS::Partition" }, + ":iam::123456789012:root", + ]], + }, + }, + "Resource": "*", + }, + ], + }, + })); + + test.done(); + }, + + "generates ArtifactStores with the alias' name as the KeyID"(test: Test) { + const app = new cdk.App(); + const replicationRegion = 'us-west-1'; + + const pipelineRegion = 'us-west-2'; + const pipelineStack = new cdk.Stack(app, 'MyStack', { + env: { region: pipelineRegion, account: '123456789012' }, + }); + const sourceOutput = new codepipeline.Artifact(); + const pipeline = new codepipeline.Pipeline(pipelineStack, 'Pipeline', { + stages: [ + { + stageName: 'Source', + actions: [new FakeSourceAction({ + actionName: 'Source', + output: sourceOutput, + })], + }, + { + stageName: 'Build', + actions: [new FakeBuildAction({ + actionName: 'Build', + input: sourceOutput, + region: replicationRegion, + })], + }, + ], + }); + + expect(pipelineStack).to(haveResourceLike('AWS::CodePipeline::Pipeline', { + "ArtifactStores": [ + { + "Region": replicationRegion, + "ArtifactStore": { + "Type": "S3", + "EncryptionKey": { + "Type": "KMS", + "Id": "alias/mystack-support-us-west-1tencryptionalias9b344b2b8e6825cb1f7d", + }, + }, + }, + { + "Region": pipelineRegion, + }, + ], + })); + + expect(pipeline.crossRegionSupport[replicationRegion].stack).to(haveResourceLike('AWS::KMS::Alias', { + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain", + }, ResourcePart.CompleteDefinition)); + + test.done(); + }, + + 'allows passing an imported Bucket and Key for the replication Bucket'(test: Test) { + const replicationRegion = 'us-west-1'; + + const pipelineRegion = 'us-west-2'; + const pipelineStack = new cdk.Stack(undefined, undefined, { + env: { region: pipelineRegion }, + }); + const sourceOutput = new codepipeline.Artifact(); + new codepipeline.Pipeline(pipelineStack, 'Pipeline', { + crossRegionReplicationBuckets: { + [replicationRegion]: s3.Bucket.fromBucketAttributes(pipelineStack, 'ReplicationBucket', { + bucketArn: 'arn:aws:s3:::my-us-west-1-replication-bucket', + encryptionKey: kms.Key.fromKeyArn(pipelineStack, 'ReplicationKey', + `arn:aws:kms:${replicationRegion}:123456789012:key/1234-5678-9012` + ), + }), + }, + stages: [ + { + stageName: 'Source', + actions: [new FakeSourceAction({ + actionName: 'Source', + output: sourceOutput, + })], + }, + { + stageName: 'Build', + actions: [new FakeBuildAction({ + actionName: 'Build', + input: sourceOutput, + region: replicationRegion, + })], + }, + ], + }); + + expect(pipelineStack).to(haveResourceLike('AWS::CodePipeline::Pipeline', { + "ArtifactStores": [ + { + "Region": replicationRegion, + "ArtifactStore": { + "Type": "S3", + "Location": "my-us-west-1-replication-bucket", + "EncryptionKey": { + "Type": "KMS", + "Id": "1234-5678-9012", + }, + }, + }, + { + "Region": pipelineRegion, + }, + ], + })); + + test.done(); + }, + }, + 'that is cross-account': { 'does not allow passing a dynamic value in the Action account property'(test: Test) { const app = new cdk.App(); diff --git a/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json b/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json index 9912dc13b4b66..fe5590840de9e 100644 --- a/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json +++ b/packages/@aws-cdk/aws-events-targets/test/codepipeline/integ.pipeline-event-target.expected.json @@ -96,6 +96,20 @@ "DeletionPolicy": "Retain", "UpdateReplacePolicy": "Retain" }, + "pipelinePipeline22F2A91DArtifactsBucketEncryptionKeyAlias9530209A": { + "Type": "AWS::KMS::Alias", + "Properties": { + "AliasName": "alias/codepipeline-pipelineeventspipelinepipeline22f2a91dfbb66895", + "TargetKeyId": { + "Fn::GetAtt": [ + "pipelinePipeline22F2A91DArtifactsBucketEncryptionKey87C796D2", + "Arn" + ] + } + }, + "DeletionPolicy": "Retain", + "UpdateReplacePolicy": "Retain" + }, "pipelinePipeline22F2A91DRole58B7B05E": { "Type": "AWS::IAM::Role", "Properties": { @@ -268,10 +282,7 @@ "ArtifactStore": { "EncryptionKey": { "Id": { - "Fn::GetAtt": [ - "pipelinePipeline22F2A91DArtifactsBucketEncryptionKey87C796D2", - "Arn" - ] + "Ref": "pipelinePipeline22F2A91DArtifactsBucketEncryptionKey87C796D2" }, "Type": "KMS" }, diff --git a/packages/@aws-cdk/aws-iam/lib/grant.ts b/packages/@aws-cdk/aws-iam/lib/grant.ts index 04c060dfc0420..ccda8a5eff3e5 100644 --- a/packages/@aws-cdk/aws-iam/lib/grant.ts +++ b/packages/@aws-cdk/aws-iam/lib/grant.ts @@ -1,6 +1,6 @@ import cdk = require('@aws-cdk/core'); import { PolicyStatement } from "./policy-statement"; -import { IGrantable } from "./principals"; +import { IGrantable, IPrincipal } from "./principals"; /** * Basic options for a grant operation @@ -83,6 +83,13 @@ export interface GrantOnPrincipalAndResourceOptions extends CommonGrantOptions { * @default Same as regular resource ARNs */ readonly resourceSelfArns?: string[]; + + /** + * The principal to use in the statement for the resource policy. + * + * @default - the principal of the grantee will be used + */ + readonly resourcePolicyPrincipal?: IPrincipal; } /** @@ -160,7 +167,7 @@ export class Grant { const statement = new PolicyStatement({ actions: options.actions, resources: (options.resourceSelfArns || options.resourceArns), - principals: [options.grantee!.grantPrincipal] + principals: [options.resourcePolicyPrincipal || options.grantee!.grantPrincipal] }); options.resource.addToResourcePolicy(statement); diff --git a/packages/@aws-cdk/aws-kms/lib/alias.ts b/packages/@aws-cdk/aws-kms/lib/alias.ts index bf1dd240ecea9..41c10132750e2 100644 --- a/packages/@aws-cdk/aws-kms/lib/alias.ts +++ b/packages/@aws-cdk/aws-kms/lib/alias.ts @@ -1,4 +1,5 @@ -import { Construct, IResource, Resource, Token } from '@aws-cdk/core'; +import iam = require('@aws-cdk/aws-iam'); +import { Construct, RemovalPolicy, Resource, Stack, Token } from '@aws-cdk/core'; import { IKey } from './key'; import { CfnAlias } from './kms.generated'; @@ -7,8 +8,9 @@ const DISALLOWED_PREFIX = REQUIRED_ALIAS_PREFIX + 'aws/'; /** * A KMS Key alias. + * An alias can be used in all places that expect a key. */ -export interface IAlias extends IResource { +export interface IAlias extends IKey { /** * The name of the alias. * @@ -41,12 +43,55 @@ export interface AliasProps { * specify another alias. */ readonly targetKey: IKey; + + /** + * Policy to apply when the alias is removed from this stack. + * + * @default - The alias will be deleted + */ + readonly removalPolicy?: RemovalPolicy; } abstract class AliasBase extends Resource implements IAlias { public abstract readonly aliasName: string; public abstract readonly aliasTargetKey: IKey; + + public get keyArn(): string { + return Stack.of(this).formatArn({ + service: 'kms', + // aliasName already contains the '/' + resource: this.aliasName, + }); + } + + public get keyId(): string { + return this.aliasName; + } + + public addAlias(alias: string): Alias { + return this.aliasTargetKey.addAlias(alias); + } + + public addToResourcePolicy(statement: iam.PolicyStatement, allowNoOp?: boolean): void { + this.aliasTargetKey.addToResourcePolicy(statement, allowNoOp); + } + + public grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant { + return this.aliasTargetKey.grant(grantee, ...actions); + } + + public grantDecrypt(grantee: iam.IGrantable): iam.Grant { + return this.aliasTargetKey.grantDecrypt(grantee); + } + + public grantEncrypt(grantee: iam.IGrantable): iam.Grant { + return this.aliasTargetKey.grantEncrypt(grantee); + } + + public grantEncryptDecrypt(grantee: iam.IGrantable): iam.Grant { + return this.aliasTargetKey.grantEncryptDecrypt(grantee); + } } export interface AliasAttributes { @@ -79,8 +124,6 @@ export class Alias extends AliasBase { public readonly aliasTargetKey: IKey; constructor(scope: Construct, id: string, props: AliasProps) { - super(scope, id); - let aliasName = props.aliasName; if (!Token.isUnresolved(aliasName)) { @@ -92,7 +135,7 @@ export class Alias extends AliasBase { throw new Error(`Alias must include a value after "${REQUIRED_ALIAS_PREFIX}": ${aliasName}`); } - if (aliasName.startsWith(DISALLOWED_PREFIX)) { + if (aliasName.toLocaleLowerCase().startsWith(DISALLOWED_PREFIX)) { throw new Error(`Alias cannot start with ${DISALLOWED_PREFIX}: ${aliasName}`); } @@ -101,11 +144,25 @@ export class Alias extends AliasBase { } } + super(scope, id, { + physicalName: aliasName, + }); + + this.aliasTargetKey = props.targetKey; + const resource = new CfnAlias(this, 'Resource', { - aliasName, - targetKeyId: props.targetKey.keyArn + aliasName: this.physicalName, + targetKeyId: this.aliasTargetKey.keyArn }); - this.aliasName = resource.aliasName; + this.aliasName = this.getResourceNameAttribute(resource.aliasName); + + if (props.removalPolicy) { + resource.applyRemovalPolicy(props.removalPolicy); + } + } + + protected generatePhysicalName(): string { + return REQUIRED_ALIAS_PREFIX + super.generatePhysicalName(); } } diff --git a/packages/@aws-cdk/aws-kms/lib/key.ts b/packages/@aws-cdk/aws-kms/lib/key.ts index 2d6cb9cc27e51..c8406e1429c04 100644 --- a/packages/@aws-cdk/aws-kms/lib/key.ts +++ b/packages/@aws-cdk/aws-kms/lib/key.ts @@ -15,6 +15,14 @@ export interface IKey extends IResource { */ readonly keyArn: string; + /** + * The ID of the key + * (the part that looks something like: 1234abcd-12ab-34cd-56ef-1234567890ab). + * + * @attribute + */ + readonly keyId: string; + /** * Defines a new alias for the key. */ @@ -56,6 +64,8 @@ abstract class KeyBase extends Resource implements IKey { */ public abstract readonly keyArn: string; + public abstract readonly keyId: string; + /** * Optional policy document that represents the resource policy of this key. * @@ -109,12 +119,30 @@ abstract class KeyBase extends Resource implements IKey { * must not be empty and so default grants won't work. */ public grant(grantee: iam.IGrantable, ...actions: string[]): iam.Grant { + // KMS verifies whether the principals included in its key policy actually exist. + // This is a problem if the stack the grantee is part of depends on the key stack + // (as it won't exist before the key policy is attempted to be created). + // In that case, make the account the resource policy principal + const granteeStackDependsOnKeyStack = this.granteeStackDependsOnKeyStack(grantee); + const principal = granteeStackDependsOnKeyStack + ? new iam.AccountPrincipal(granteeStackDependsOnKeyStack) + : grantee.grantPrincipal; + + const crossAccountAccess = this.isGranteeFromAnotherAccount(grantee); + const crossRegionAccess = this.isGranteeFromAnotherRegion(grantee); + const crossEnvironment = crossAccountAccess || crossRegionAccess; return iam.Grant.addToPrincipalAndResource({ grantee, actions, - resourceArns: [this.keyArn], resource: this, - resourceSelfArns: ['*'] + resourcePolicyPrincipal: principal, + + // if the key is used in a cross-environment matter, + // we can't access the Key ARN (they don't have physical names), + // so fall back to using '*'. ToDo we need to make this better... somehow + resourceArns: crossEnvironment ? ['*'] : [this.keyArn], + + resourceSelfArns: crossEnvironment ? undefined : ['*'], }); } @@ -149,6 +177,46 @@ abstract class KeyBase extends Resource implements IKey { 'kms:GenerateDataKey*' ); } + + /** + * Checks whether the grantee belongs to a stack that will be deployed + * after the stack containing this key. + * + * @param grantee the grantee to give permissions to + * @returns the account ID of the grantee stack if its stack does depend on this stack, + * undefined otherwise + */ + private granteeStackDependsOnKeyStack(grantee: iam.IGrantable): string | undefined { + if (!(Construct.isConstruct(grantee))) { + return undefined; + } + const keyStack = Stack.of(this); + const granteeStack = Stack.of(grantee); + if (keyStack === granteeStack) { + return undefined; + } + return granteeStack.dependencies.includes(keyStack) + ? granteeStack.account + : undefined; + } + + private isGranteeFromAnotherRegion(grantee: iam.IGrantable): boolean { + if (!(Construct.isConstruct(grantee))) { + return false; + } + const bucketStack = Stack.of(this); + const identityStack = Stack.of(grantee); + return bucketStack.region !== identityStack.region; + } + + private isGranteeFromAnotherAccount(grantee: iam.IGrantable): boolean { + if (!(Construct.isConstruct(grantee))) { + return false; + } + const bucketStack = Stack.of(this); + const identityStack = Stack.of(grantee); + return bucketStack.account !== identityStack.account; + } } /** @@ -218,14 +286,27 @@ export class Key extends KeyBase { */ public static fromKeyArn(scope: Construct, id: string, keyArn: string): IKey { class Import extends KeyBase { - public keyArn = keyArn; - protected policy?: iam.PolicyDocument | undefined = undefined; + public readonly keyArn = keyArn; + public readonly keyId: string; + protected readonly policy?: iam.PolicyDocument | undefined = undefined; + + constructor(keyId: string) { + super(scope, id); + + this.keyId = keyId; + } + } + + const keyResourceName = Stack.of(scope).parseArn(keyArn).resourceName; + if (!keyResourceName) { + throw new Error(`KMS key ARN must be in the format 'arn:aws:kms:::key/', got: '${keyArn}'`); } - return new Import(scope, id); + return new Import(keyResourceName); } public readonly keyArn: string; + public readonly keyId: string; protected readonly policy?: PolicyDocument; constructor(scope: Construct, id: string, props: KeyProps = {}) { @@ -246,6 +327,7 @@ export class Key extends KeyBase { }); this.keyArn = resource.attrArn; + this.keyId = resource.ref; resource.applyRemovalPolicy(props.removalPolicy); if (props.alias !== undefined) { diff --git a/packages/@aws-cdk/aws-kms/test/test.alias.ts b/packages/@aws-cdk/aws-kms/test/test.alias.ts index dc2011a561dd9..5a71481745658 100644 --- a/packages/@aws-cdk/aws-kms/test/test.alias.ts +++ b/packages/@aws-cdk/aws-kms/test/test.alias.ts @@ -1,8 +1,10 @@ -import { expect, haveResource } from '@aws-cdk/assert'; -import { App, Stack } from '@aws-cdk/core'; +import { expect, haveResource, SynthUtils } from '@aws-cdk/assert'; +import { App, CfnOutput, Construct, Stack } from '@aws-cdk/core'; import { Test } from 'nodeunit'; -import { Key } from '../lib'; import { Alias } from '../lib/alias'; +import { IKey, Key } from '../lib/key'; + +// tslint:disable:object-literal-key-quotes export = { 'default alias'(test: Test) { @@ -103,21 +105,72 @@ export = { enabled: false }); - test.throws(() => new Alias(stack, 'Alias', { + test.throws(() => new Alias(stack, 'Alias1', { aliasName: 'alias/aws/', targetKey: key - })); + }), /Alias cannot start with alias\/aws\/: alias\/aws\//); - test.throws(() => new Alias(stack, 'Alias', { + test.throws(() => new Alias(stack, 'Alias2', { aliasName: 'alias/aws/Awesome', targetKey: key - })); + }), /Alias cannot start with alias\/aws\/: alias\/aws\/Awesome/); - test.throws(() => new Alias(stack, 'Alias', { + test.throws(() => new Alias(stack, 'Alias3', { aliasName: 'alias/AWS/awesome', targetKey: key - })); + }), /Alias cannot start with alias\/aws\/: alias\/AWS\/awesome/); test.done(); - } + }, + + 'can be used wherever a key is expected'(test: Test) { + const stack = new Stack(); + + const myKey = new Key(stack, 'MyKey', { + enableKeyRotation: true, + enabled: false + }); + const myAlias = new Alias(stack, 'MyAlias', { + targetKey: myKey, + aliasName: 'alias/myAlias', + }); + + class MyConstruct extends Construct { + constructor(scope: Construct, id: string, key: IKey) { + super(scope, id); + + new CfnOutput(stack, 'OutId', { + value: key.keyId, + }); + new CfnOutput(stack, 'OutArn', { + value: key.keyArn, + }); + } + } + + new MyConstruct(stack, 'MyConstruct', myAlias); + + const template = SynthUtils.synthesize(stack).template.Outputs; + + test.deepEqual(template, { + "OutId": { + "Value": "alias/myAlias", + }, + "OutArn": { + "Value": { + "Fn::Join": ["", [ + "arn:", + { Ref: "AWS::Partition" }, + ":kms:", + { Ref: "AWS::Region" }, + ":", + { Ref: "AWS::AccountId" }, + ":alias/myAlias", + ]], + }, + }, + }); + + test.done(); + }, }; diff --git a/packages/@aws-cdk/aws-kms/test/test.key.ts b/packages/@aws-cdk/aws-kms/test/test.key.ts index a8e6dcd4341fb..ed10ac6a7b010 100644 --- a/packages/@aws-cdk/aws-kms/test/test.key.ts +++ b/packages/@aws-cdk/aws-kms/test/test.key.ts @@ -1,9 +1,19 @@ -import { exactlyMatchTemplate, expect, haveResource, ResourcePart } from '@aws-cdk/assert'; +import { + exactlyMatchTemplate, + expect, + haveResource, + haveResourceLike, + ResourcePart, + SynthUtils +} from '@aws-cdk/assert'; import { PolicyStatement, User } from '@aws-cdk/aws-iam'; -import { App, RemovalPolicy, Stack, Tag } from '@aws-cdk/core'; +import iam = require('@aws-cdk/aws-iam'); +import { App, CfnOutput, RemovalPolicy, Stack, Tag } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import { Key } from '../lib'; +// tslint:disable:object-literal-key-quotes + export = { 'default key'(test: Test) { const stack = new Stack(); @@ -475,51 +485,135 @@ export = { test.done(); }, - 'import/export can be used to bring in an existing key'(test: Test) { - const stack2 = new Stack(); - const myKeyImported = Key.fromKeyArn(stack2, 'MyKeyImported', 'arn:of:key'); + 'grant for a principal in a dependent stack works correctly'(test: Test) { + const app = new App(); - // addAlias can be called on imported keys. - myKeyImported.addAlias('alias/hello'); + const principalStack = new Stack(app, 'PrincipalStack'); + const principal = new iam.Role(principalStack, 'Role', { + assumedBy: new iam.AnyPrincipal(), + }); - expect(stack2).toMatch({ - Resources: { - MyKeyImportedAliasB1C5269F: { - Type: "AWS::KMS::Alias", - Properties: { - AliasName: "alias/hello", - TargetKeyId: 'arn:of:key' - } - } - } + const keyStack = new Stack(app, 'KeyStack'); + const key = new Key(keyStack, 'Key'); + + principalStack.addDependency(keyStack); + + key.grantEncrypt(principal); + + expect(keyStack).to(haveResourceLike('AWS::KMS::Key', { + "KeyPolicy": { + "Statement": [ + { + // owning account management permissions - we don't care about them in this test + }, + { + "Action": [ + "kms:Encrypt", + "kms:ReEncrypt*", + "kms:GenerateDataKey*", + ], + "Effect": "Allow", + "Principal": { + "AWS": { + "Fn::Join": ["", [ + "arn:", + { "Ref": "AWS::Partition" }, + ":iam::", + { "Ref": "AWS::AccountId" }, + ":root", + ]], + }, + }, + "Resource": "*", + }, + ], + }, + })); + + test.done(); + }, + + 'keyId resolves to a Ref'(test: Test) { + const stack = new Stack(); + const key = new Key(stack, 'MyKey'); + + new CfnOutput(stack, 'Out', { + value: key.keyId, + }); + + const template = SynthUtils.synthesize(stack).template.Outputs; + + test.deepEqual(template, { + "Out": { + "Value": { + "Ref": "MyKey6AB29FA6", + }, + }, }); test.done(); }, - 'addToResourcePolicy allowNoOp and there is no policy': { - 'succeed if set to true (default)'(test: Test) { + 'imported keys': { + 'throw an error when providing something that is not a valid key ARN'(test: Test) { const stack = new Stack(); - const key = Key.fromKeyArn(stack, 'Imported', 'foo/bar'); - - key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] })); + test.throws(() => { + Key.fromKeyArn(stack, 'Imported', 'arn:aws:kms:us-east-1:123456789012:key'); + }, /KMS key ARN must be in the format 'arn:aws:kms:::key\/', got: 'arn:aws:kms:us-east-1:123456789012:key'/); test.done(); }, - 'fails if set to false'(test: Test) { + 'can have aliases added to them'(test: Test) { + const stack2 = new Stack(); + const myKeyImported = Key.fromKeyArn(stack2, 'MyKeyImported', + 'arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012'); - const stack = new Stack(); + // addAlias can be called on imported keys. + myKeyImported.addAlias('alias/hello'); - const key = Key.fromKeyArn(stack, 'Imported', 'foo/bar'); + test.equal(myKeyImported.keyId, '12345678-1234-1234-1234-123456789012'); - test.throws(() => - key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] }), /* allowNoOp */ false), - 'Unable to add statement to IAM resource policy for KMS key: "foo/bar"'); + expect(stack2).toMatch({ + Resources: { + MyKeyImportedAliasB1C5269F: { + Type: "AWS::KMS::Alias", + Properties: { + AliasName: "alias/hello", + TargetKeyId: "arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012" + } + } + } + }); test.done(); + }, + + 'addToResourcePolicy allowNoOp and there is no policy': { + 'succeed if set to true (default)'(test: Test) { + const stack = new Stack(); + + const key = Key.fromKeyArn(stack, 'Imported', + 'arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012'); + + key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] })); + + test.done(); + }, - } - } + 'fails if set to false'(test: Test) { + const stack = new Stack(); + + const key = Key.fromKeyArn(stack, 'Imported', + 'arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012'); + + test.throws(() => { + key.addToResourcePolicy(new PolicyStatement({ resources: ['*'], actions: ['*'] }), /* allowNoOp */ false); + }, 'Unable to add statement to IAM resource policy for KMS key: "foo/bar"'); + + test.done(); + }, + }, + }, }; diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index 98d14fe55b46a..120dfe3263697 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -240,6 +240,8 @@ export interface BucketAttributes { * @default false */ readonly bucketWebsiteNewUrlFormat?: boolean; + + readonly encryptionKey?: kms.IKey; } /** @@ -529,18 +531,7 @@ abstract class BucketBase extends Resource implements IBucket { } if (this.encryptionKey) { - if (crossAccountAccess) { - // we can't access the Key ARN (they don't have physical names), - // so fall back on using '*'. ToDo we need to make this better... somehow - iam.Grant.addToPrincipalAndResource({ - actions: keyActions, - grantee, - resourceArns: ['*'], - resource: this.encryptionKey, - }); - } else { - this.encryptionKey.grant(grantee, ...keyActions); - } + this.encryptionKey.grant(grantee, ...keyActions); } return ret; @@ -897,7 +888,7 @@ export class Bucket extends BucketBase { public readonly bucketRegionalDomainName = attrs.bucketRegionalDomainName || `${bucketName}.s3.${region}.${urlSuffix}`; public readonly bucketDualStackDomainName = attrs.bucketDualStackDomainName || `${bucketName}.s3.dualstack.${region}.${urlSuffix}`; public readonly bucketWebsiteNewUrlFormat = newUrlFormat; - public readonly encryptionKey?: kms.IKey; + public readonly encryptionKey = attrs.encryptionKey; public policy?: BucketPolicy = undefined; protected autoCreatePolicy = false; protected disallowPublicAccess = false; diff --git a/packages/@aws-cdk/core/lib/resource.ts b/packages/@aws-cdk/core/lib/resource.ts index e3c618046dc3f..99885d5b04a62 100644 --- a/packages/@aws-cdk/core/lib/resource.ts +++ b/packages/@aws-cdk/core/lib/resource.ts @@ -101,10 +101,14 @@ export abstract class Resource extends Construct implements IResource { } if (!this._physicalName) { - this._physicalName = generatePhysicalName(this); + this._physicalName = this.generatePhysicalName(); } } + protected generatePhysicalName(): string { + return generatePhysicalName(this); + } + /** * Returns an environment-sensitive token that should be used for the * resource's "name" attribute (e.g. `bucket.bucketName`). diff --git a/packages/decdk/test/__snapshots__/synth.test.js.snap b/packages/decdk/test/__snapshots__/synth.test.js.snap index b725565815377..3f3370f18a219 100644 --- a/packages/decdk/test/__snapshots__/synth.test.js.snap +++ b/packages/decdk/test/__snapshots__/synth.test.js.snap @@ -2012,6 +2012,20 @@ Object { "Type": "AWS::KMS::Key", "UpdateReplacePolicy": "Retain", }, + "PipelineArtifactsBucketEncryptionKeyAlias5C510EEE": Object { + "DeletionPolicy": "Retain", + "Properties": Object { + "AliasName": "alias/codepipeline-pipelinepipeline22f2a91d", + "TargetKeyId": Object { + "Fn::GetAtt": Array [ + "PipelineArtifactsBucketEncryptionKey01D58D69", + "Arn", + ], + }, + }, + "Type": "AWS::KMS::Alias", + "UpdateReplacePolicy": "Retain", + }, "PipelineBuildCodePipelineActionRoleD77A08E6": Object { "Properties": Object { "AssumeRolePolicyDocument": Object { @@ -2083,10 +2097,7 @@ Object { "ArtifactStore": Object { "EncryptionKey": Object { "Id": Object { - "Fn::GetAtt": Array [ - "PipelineArtifactsBucketEncryptionKey01D58D69", - "Arn", - ], + "Ref": "PipelineArtifactsBucketEncryptionKey01D58D69", }, "Type": "KMS", },