From d6278b37ce787c587e1079a3f2844566291d7c27 Mon Sep 17 00:00:00 2001 From: Adam Ruka Date: Wed, 19 Aug 2020 14:45:49 -0700 Subject: [PATCH] feat: make imported resources account/region-aware (#8280) Add the `account` and `region` properties to the `IResource` interface and `Resource` class. By default, these are equal to the account and region of the Stack the resource belongs to; however, they can be set to different values in resources that are imported. Use those new properties in two places: * In CodePipeline, to determine whether a given action is cross-account (with support for specifying the account and region in S3's `BucketAttributes`, as a first use case). * IAM's `addToPrincipalOrResource()`, to correctly know when to modify the receiver's resource policy. This is aided by adding an optional `principalAccount` property to `IPrincipal`, as a way to compare to the account present in the passed `IResource` instance. Fixes #2807 Fixes #5740 Fixes #7012 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../aws-certificatemanager/test/util.test.ts | 2 +- .../aws-codepipeline-actions/README.md | 15 +- .../test/s3/test.s3-deploy-action.ts | 34 ++- .../@aws-cdk/aws-codepipeline/lib/pipeline.ts | 24 +- .../aws-dynamodb/test/dynamodb.test.ts | 2 +- packages/@aws-cdk/aws-ec2/lib/bastion-host.ts | 7 +- packages/@aws-cdk/aws-ec2/package.json | 1 + .../test/integ.instance-init.expected.json | 8 +- .../@aws-cdk/aws-ec2/test/userdata.test.ts | 4 +- .../integ.load-balanced-fargate-service.ts | 10 +- .../test.load-balanced-fargate-service.ts | 17 +- .../aws-eks-legacy/lib/kubectl-layer.ts | 8 +- .../@aws-cdk/aws-eks/lib/kubectl-layer.ts | 8 +- packages/@aws-cdk/aws-iam/lib/grant.ts | 21 +- packages/@aws-cdk/aws-iam/lib/group.ts | 5 +- packages/@aws-cdk/aws-iam/lib/lazy-role.ts | 1 + .../@aws-cdk/aws-iam/lib/oidc-provider.ts | 7 +- packages/@aws-cdk/aws-iam/lib/principals.ts | 10 + .../aws-iam/lib/private/immutable-role.ts | 12 +- packages/@aws-cdk/aws-iam/lib/role.ts | 42 ++-- packages/@aws-cdk/aws-iam/lib/user.ts | 4 +- packages/@aws-cdk/aws-iam/package.json | 1 + .../aws-iam/test/cross-account.test.ts | 235 ++++++++++++++++++ packages/@aws-cdk/aws-iam/test/grant.test.ts | 13 +- .../@aws-cdk/aws-lambda/lib/function-base.ts | 2 + .../@aws-cdk/aws-route53/test/test.util.ts | 29 +-- packages/@aws-cdk/aws-s3/lib/bucket.ts | 53 ++-- packages/@aws-cdk/aws-s3/test/test.util.ts | 35 +-- packages/@aws-cdk/core/lib/arn.ts | 10 +- packages/@aws-cdk/core/lib/cfn-pseudo.ts | 7 +- packages/@aws-cdk/core/lib/resource.ts | 55 ++++ packages/@aws-cdk/core/lib/token.ts | 42 ++++ packages/@aws-cdk/core/test/test.arn.ts | 21 +- .../lib/actions/deploy-cdk-stack-action.ts | 64 ++--- .../test/cross-environment-infra.test.ts | 14 +- .../integ.pipeline-with-assets.expected.json | 59 ++++- .../test/integ.pipeline-with-assets.ts | 6 +- .../test/integ.pipeline.expected.json | 55 +++- .../@aws-cdk/pipelines/test/integ.pipeline.ts | 6 +- .../@aws-cdk/pipelines/test/pipeline.test.ts | 126 +++++++++- 40 files changed, 826 insertions(+), 249 deletions(-) create mode 100644 packages/@aws-cdk/aws-iam/test/cross-account.test.ts diff --git a/packages/@aws-cdk/aws-certificatemanager/test/util.test.ts b/packages/@aws-cdk/aws-certificatemanager/test/util.test.ts index 5f0c372414a9e..3fc833eb1d79e 100644 --- a/packages/@aws-cdk/aws-certificatemanager/test/util.test.ts +++ b/packages/@aws-cdk/aws-certificatemanager/test/util.test.ts @@ -99,7 +99,7 @@ describe('getCertificateRegion', () => { domainName: 'www.example.com', }); - expect(getCertificateRegion(certificate)).toEqual('${Token[AWS::Region.4]}'); + expect(getCertificateRegion(certificate)).toEqual('${Token[AWS.Region.4]}'); }); }); diff --git a/packages/@aws-cdk/aws-codepipeline-actions/README.md b/packages/@aws-cdk/aws-codepipeline-actions/README.md index 3a4497529b38d..27860da603548 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/README.md +++ b/packages/@aws-cdk/aws-codepipeline-actions/README.md @@ -167,6 +167,19 @@ pipeline.addStage({ }); ``` +The region of the action will be determined by the region the bucket itself is in. +When using a newly created bucket, +that region will be taken from the stack the bucket belongs to; +for an imported bucket, +you can specify the region explicitly: + +```ts +const sourceBucket = s3.Bucket.fromBucketAttributes(this, 'SourceBucket', { + bucketName: 'my-bucket', + region: 'ap-southeast-1', +}); +``` + By default, the Pipeline will poll the Bucket to detect changes. You can change that behavior to use CloudWatch Events by setting the `trigger` property to `S3Trigger.EVENTS` (it's `S3Trigger.POLL` by default). @@ -872,4 +885,4 @@ pipeline.addStage({ ``` See [the AWS documentation](https://docs.aws.amazon.com/codepipeline/latest/userguide/action-reference-StepFunctions.html) -for information on Action structure reference. \ No newline at end of file +for information on Action structure reference. diff --git a/packages/@aws-cdk/aws-codepipeline-actions/test/s3/test.s3-deploy-action.ts b/packages/@aws-cdk/aws-codepipeline-actions/test/s3/test.s3-deploy-action.ts index 1d95c8764ed66..8184c61bc3fce 100644 --- a/packages/@aws-cdk/aws-codepipeline-actions/test/s3/test.s3-deploy-action.ts +++ b/packages/@aws-cdk/aws-codepipeline-actions/test/s3/test.s3-deploy-action.ts @@ -1,7 +1,7 @@ import { expect, haveResourceLike } from '@aws-cdk/assert'; import * as codepipeline from '@aws-cdk/aws-codepipeline'; import * as s3 from '@aws-cdk/aws-s3'; -import { Duration, SecretValue, Stack } from '@aws-cdk/core'; +import { App, Duration, SecretValue, Stack } from '@aws-cdk/core'; import { Test } from 'nodeunit'; import * as cpactions from '../../lib'; @@ -153,6 +153,38 @@ export = { test.done(); }, + + 'correctly makes the action cross-region for a Bucket imported with a different region'(test: Test) { + const app = new App(); + const stack = new Stack(app, 'PipelineStack', { + env: { account: '123456789012', region: 'us-west-2' }, + }); + const deployBucket = s3.Bucket.fromBucketAttributes(stack, 'DeployBucket', { + bucketName: 'my-deploy-bucket', + region: 'ap-southeast-1', + }); + + minimalPipeline(stack, { + bucket: deployBucket, + }); + + expect(stack).to(haveResourceLike('AWS::CodePipeline::Pipeline', { + Stages: [ + {}, + { + Name: 'Deploy', + Actions: [ + { + Name: 'CopyFiles', + Region: 'ap-southeast-1', + }, + ], + }, + ], + })); + + test.done(); + }, }, }; diff --git a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts index 2a77ecdc532b5..ebd84c87f1366 100644 --- a/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts +++ b/packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts @@ -4,7 +4,7 @@ import * as kms from '@aws-cdk/aws-kms'; import * as s3 from '@aws-cdk/aws-s3'; import { App, BootstraplessSynthesizer, Construct, DefaultStackSynthesizer, - IStackSynthesizer, Lazy, PhysicalName, RemovalPolicy, Resource, Stack, Token, + IStackSynthesizer, Lazy, PhysicalName, RemovalPolicy, Resource, Stack, Token, TokenComparison, } from '@aws-cdk/core'; import { ActionCategory, IAction, IPipeline, IStage } from './action'; import { CfnPipeline } from './codepipeline.generated'; @@ -400,12 +400,22 @@ export class Pipeline extends PipelineBase { const actionResource = action.actionProperties.resource; if (actionResource) { - const actionResourceStack = Stack.of(actionResource); - if (pipelineStack.region !== actionResourceStack.region) { - actionRegion = actionResourceStack.region; + const pipelineAndActionRegionComparison = Token.compareStrings(this.env.region, actionResource.env.region); + const pipelineAndActionInDifferentRegions = pipelineAndActionRegionComparison === TokenComparison.ONE_UNRESOLVED || + pipelineAndActionRegionComparison === TokenComparison.DIFFERENT; + if (pipelineAndActionInDifferentRegions) { + actionRegion = actionResource.env.region; + // if the resource is from a different stack in another region but the same account, // use that stack as home for the cross-region support resources - if (pipelineStack.account === actionResourceStack.account) { + const actionResourceStack = Stack.of(actionResource); + const actionResourceAndItsStackRegionComparison = Token.compareStrings(actionResource.env.region, actionResourceStack.region); + const actionResourceInSameRegionAsItsStack = actionResourceAndItsStackRegionComparison === TokenComparison.SAME || + actionResourceAndItsStackRegionComparison === TokenComparison.BOTH_UNRESOLVED; + const pipelineAndActionResourceStackAccountComparison = Token.compareStrings(this.env.account, actionResourceStack.account); + const pipelineAndActionResourceStackInSameAccount = pipelineAndActionResourceStackAccountComparison === TokenComparison.SAME || + pipelineAndActionResourceStackAccountComparison === TokenComparison.BOTH_UNRESOLVED; + if (pipelineAndActionResourceStackInSameAccount && actionResourceInSameRegionAsItsStack) { otherStack = actionResourceStack; } } @@ -850,7 +860,7 @@ export class Pipeline extends PipelineBase { } private requireRegion(): string { - const region = Stack.of(this).region; + const region = this.env.region; if (Token.isUnresolved(region)) { throw new Error('Pipeline stack which uses cross-environment actions must have an explicitly set region'); } @@ -932,4 +942,4 @@ class PipelineLocation { // runOrders are 1-based, so make the stageIndex also 1-based otherwise it's going to be confusing. return `Stage ${this.stageIndex + 1} Action ${this.action.runOrder} ('${this.stageName}'/'${this.actionName}')`; } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts b/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts index b50a78c97fac0..5c6efcf4385f0 100644 --- a/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts +++ b/packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts @@ -2107,7 +2107,7 @@ describe('import', () => { 'Roles': [{ 'Ref': 'NewRole99763075' }], }); - expect(table.tableArn).toBe('arn:${Token[AWS::Partition.3]}:dynamodb:${Token[AWS::Region.4]}:${Token[AWS::AccountId.0]}:table/MyTable'); + expect(table.tableArn).toBe('arn:${Token[AWS.Partition.3]}:dynamodb:${Token[AWS.Region.4]}:${Token[AWS.AccountId.0]}:table/MyTable'); expect(stack.resolve(table.tableName)).toBe(tableName); }); diff --git a/packages/@aws-cdk/aws-ec2/lib/bastion-host.ts b/packages/@aws-cdk/aws-ec2/lib/bastion-host.ts index 00b6cd396ddf9..9795551abf5a4 100644 --- a/packages/@aws-cdk/aws-ec2/lib/bastion-host.ts +++ b/packages/@aws-cdk/aws-ec2/lib/bastion-host.ts @@ -1,5 +1,5 @@ import { IPrincipal, IRole, PolicyStatement } from '@aws-cdk/aws-iam'; -import { CfnOutput, Construct, Stack } from '@aws-cdk/core'; +import { CfnOutput, Construct, Resource, Stack } from '@aws-cdk/core'; import { AmazonLinuxGeneration, InstanceClass, InstanceSize, InstanceType } from '.'; import { Connections } from './connections'; import { IInstance, Instance } from './instance'; @@ -90,8 +90,9 @@ export interface BastionHostLinuxProps { * You can also configure this bastion host to allow connections via SSH * * @experimental + * @resource AWS::EC2::Instance */ -export class BastionHostLinux extends Construct implements IInstance { +export class BastionHostLinux extends Resource implements IInstance { public readonly stack: Stack; /** @@ -192,4 +193,4 @@ export class BastionHostLinux extends Construct implements IInstance { this.connections.allowFrom(p, Port.tcp(22), 'SSH access'); }); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-ec2/package.json b/packages/@aws-cdk/aws-ec2/package.json index cae87529d643b..209b34723806e 100644 --- a/packages/@aws-cdk/aws-ec2/package.json +++ b/packages/@aws-cdk/aws-ec2/package.json @@ -120,6 +120,7 @@ "props-physical-name:@aws-cdk/aws-ec2.InterfaceVpcEndpointProps", "from-method:@aws-cdk/aws-ec2.Instance", "from-method:@aws-cdk/aws-ec2.VpcEndpointService", + "attribute-tag:@aws-cdk/aws-ec2.BastionHostLinux.instance", "attribute-tag:@aws-cdk/aws-ec2.Instance.instance", "from-signature:@aws-cdk/aws-ec2.SecurityGroup.fromSecurityGroupId", "docs-public-apis:@aws-cdk/aws-ec2.WindowsVersion.WINDOWS_SERVER_2016_ENGLISH_DEEP_LEARNING", diff --git a/packages/@aws-cdk/aws-ec2/test/integ.instance-init.expected.json b/packages/@aws-cdk/aws-ec2/test/integ.instance-init.expected.json index f75c3e665eb13..0127cd6df4208 100644 --- a/packages/@aws-cdk/aws-ec2/test/integ.instance-init.expected.json +++ b/packages/@aws-cdk/aws-ec2/test/integ.instance-init.expected.json @@ -130,7 +130,7 @@ ] } }, - "Instance255F3526524bcb6d7e0439c4c": { + "Instance255F3526589c13387332ee3de": { "Type": "AWS::EC2::Instance", "Properties": { "AvailabilityZone": "us-east-1a", @@ -161,7 +161,7 @@ "Fn::Join": [ "", [ - "#!/bin/bash\n# fingerprint: ab7f06cf7eda4e4a\n(\n set +e\n /opt/aws/bin/cfn-init -v --region ", + "#!/bin/bash\n# fingerprint: 061ec8b06d437783\n(\n set +e\n /opt/aws/bin/cfn-init -v --region ", { "Ref": "AWS::Region" }, @@ -169,7 +169,7 @@ { "Ref": "AWS::StackName" }, - " --resource Instance255F3526524bcb6d7e0439c4c -c default\n /opt/aws/bin/cfn-signal -e $? --region ", + " --resource Instance255F3526589c13387332ee3de -c default\n /opt/aws/bin/cfn-signal -e $? --region ", { "Ref": "AWS::Region" }, @@ -177,7 +177,7 @@ { "Ref": "AWS::StackName" }, - " --resource Instance255F3526524bcb6d7e0439c4c\n cat /var/log/cfn-init.log >&2\n)" + " --resource Instance255F3526589c13387332ee3de\n cat /var/log/cfn-init.log >&2\n)" ] ] } diff --git a/packages/@aws-cdk/aws-ec2/test/userdata.test.ts b/packages/@aws-cdk/aws-ec2/test/userdata.test.ts index 5be98eb41668c..bda3c6cc71ddc 100644 --- a/packages/@aws-cdk/aws-ec2/test/userdata.test.ts +++ b/packages/@aws-cdk/aws-ec2/test/userdata.test.ts @@ -52,7 +52,7 @@ nodeunitShim({ test.equals(rendered, 'trap {\n' + '$success=($PSItem.Exception.Message -eq "Success")\n' + - 'cfn-signal --stack Default --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} --success ($success.ToString().ToLower())\n' + + 'cfn-signal --stack Default --resource RESOURCE1989552F --region ${Token[AWS.Region.4]} --success ($success.ToString().ToLower())\n' + 'break\n' + '}\n' + 'command1\n' + @@ -157,7 +157,7 @@ nodeunitShim({ test.equals(rendered, '#!/bin/bash\n' + 'function exitTrap(){\n' + 'exitCode=$?\n' + - '/opt/aws/bin/cfn-signal --stack Default --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n' + + '/opt/aws/bin/cfn-signal --stack Default --resource RESOURCE1989552F --region ${Token[AWS.Region.4]} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n' + '}\n' + 'trap exitTrap EXIT\n' + 'command1'); diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.load-balanced-fargate-service.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.load-balanced-fargate-service.ts index aac90206ac978..b78ff8da2304f 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.load-balanced-fargate-service.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.load-balanced-fargate-service.ts @@ -1,6 +1,7 @@ import { Vpc } from '@aws-cdk/aws-ec2'; import { Cluster, ContainerImage } from '@aws-cdk/aws-ecs'; import { ApplicationProtocol } from '@aws-cdk/aws-elasticloadbalancingv2'; +import * as route53 from '@aws-cdk/aws-route53'; import { App, Stack } from '@aws-cdk/core'; import { ApplicationLoadBalancedFargateService } from '../../lib'; @@ -20,14 +21,11 @@ new ApplicationLoadBalancedFargateService(stack, 'myService', { protocol: ApplicationProtocol.HTTPS, enableECSManagedTags: true, domainName: 'test.example.com', - domainZone: { + domainZone: route53.HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'example.com.', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }, + }), redirectHTTP: true, }); -app.synth(); \ No newline at end of file +app.synth(); diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts index 65b711e5a36ab..60559f4be1120 100644 --- a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts +++ b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.load-balanced-fargate-service.ts @@ -3,6 +3,7 @@ import * as ec2 from '@aws-cdk/aws-ec2'; import * as ecs from '@aws-cdk/aws-ecs'; import { ApplicationLoadBalancer, ApplicationProtocol, NetworkLoadBalancer } from '@aws-cdk/aws-elasticloadbalancingv2'; import * as iam from '@aws-cdk/aws-iam'; +import * as route53 from '@aws-cdk/aws-route53'; import * as cdk from '@aws-cdk/core'; import { Test } from 'nodeunit'; import * as ecsPatterns from '../../lib'; @@ -370,13 +371,10 @@ export = { cluster, protocol: ApplicationProtocol.HTTPS, domainName: 'domain.com', - domainZone: { + domainZone: route53.HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'domain.com', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }, + }), taskImageOptions: { containerPort: 2015, image: ecs.ContainerImage.fromRegistry('abiosoft/caddy'), @@ -408,13 +406,10 @@ export = { cluster, protocol: ApplicationProtocol.HTTPS, domainName: 'test.domain.com', - domainZone: { + domainZone: route53.HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'domain.com.', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }, + }), taskImageOptions: { containerPort: 2015, image: ecs.ContainerImage.fromRegistry('abiosoft/caddy'), @@ -642,4 +637,4 @@ export = { test.done(); }, -}; \ No newline at end of file +}; diff --git a/packages/@aws-cdk/aws-eks-legacy/lib/kubectl-layer.ts b/packages/@aws-cdk/aws-eks-legacy/lib/kubectl-layer.ts index 90e7869a184cf..33aebc224d6ce 100644 --- a/packages/@aws-cdk/aws-eks-legacy/lib/kubectl-layer.ts +++ b/packages/@aws-cdk/aws-eks-legacy/lib/kubectl-layer.ts @@ -1,6 +1,6 @@ import * as crypto from 'crypto'; import * as lambda from '@aws-cdk/aws-lambda'; -import { CfnResource, Construct, Stack, Token } from '@aws-cdk/core'; +import { CfnResource, Construct, Resource, Stack, Token } from '@aws-cdk/core'; const KUBECTL_APP_ARN = 'arn:aws:serverlessrepo:us-east-1:903779448426:applications/lambda-layer-kubectl'; const KUBECTL_APP_VERSION = '1.13.7'; @@ -19,7 +19,7 @@ export interface KubectlLayerProps { * * @see https://github.com/aws-samples/aws-lambda-layer-kubectl */ -export class KubectlLayer extends Construct implements lambda.ILayerVersion { +export class KubectlLayer extends Resource implements lambda.ILayerVersion { /** * Gets or create a singleton instance of this construct. @@ -68,10 +68,6 @@ export class KubectlLayer extends Construct implements lambda.ILayerVersion { this.layerVersionArn = Token.asString(resource.getAtt('Outputs.LayerVersionArn')); } - public get stack() { - return Stack.of(this); - } - public addPermission(_id: string, _permission: lambda.LayerVersionPermission): void { return; } diff --git a/packages/@aws-cdk/aws-eks/lib/kubectl-layer.ts b/packages/@aws-cdk/aws-eks/lib/kubectl-layer.ts index c6dc90fc8ac96..6112d0b81765b 100644 --- a/packages/@aws-cdk/aws-eks/lib/kubectl-layer.ts +++ b/packages/@aws-cdk/aws-eks/lib/kubectl-layer.ts @@ -1,6 +1,6 @@ import * as crypto from 'crypto'; import * as lambda from '@aws-cdk/aws-lambda'; -import { CfnResource, Construct, Stack, Token } from '@aws-cdk/core'; +import { CfnResource, Construct, Resource, Stack, Token } from '@aws-cdk/core'; const KUBECTL_APP_ARN = 'arn:aws:serverlessrepo:us-east-1:903779448426:applications/lambda-layer-kubectl'; const KUBECTL_APP_CN_ARN = 'arn:aws-cn:serverlessrepo:cn-north-1:487369736442:applications/lambda-layer-kubectl'; @@ -21,7 +21,7 @@ export interface KubectlLayerProps { * * @see https://github.com/aws-samples/aws-lambda-layer-kubectl */ -export class KubectlLayer extends Construct implements lambda.ILayerVersion { +export class KubectlLayer extends Resource implements lambda.ILayerVersion { /** * Gets or create a singleton instance of this construct. @@ -70,10 +70,6 @@ export class KubectlLayer extends Construct implements lambda.ILayerVersion { this.layerVersionArn = Token.asString(resource.getAtt('Outputs.LayerVersionArn')); } - public get stack() { - return Stack.of(this); - } - public addPermission(_id: string, _permission: lambda.LayerVersionPermission): void { return; } diff --git a/packages/@aws-cdk/aws-iam/lib/grant.ts b/packages/@aws-cdk/aws-iam/lib/grant.ts index c0cb065f7ee2c..6529db181c712 100644 --- a/packages/@aws-cdk/aws-iam/lib/grant.ts +++ b/packages/@aws-cdk/aws-iam/lib/grant.ts @@ -121,7 +121,22 @@ export class Grant implements cdk.IDependable { scope: options.resource, }); - if (result.success) { return result; } + const resourceAndPrincipalAccountComparison = options.grantee.grantPrincipal.principalAccount + ? cdk.Token.compareStrings(options.resource.env.account, options.grantee.grantPrincipal.principalAccount) + : undefined; + // if both accounts are tokens, we assume here they are the same + const equalOrBothUnresolved = resourceAndPrincipalAccountComparison === cdk.TokenComparison.SAME + || resourceAndPrincipalAccountComparison == cdk.TokenComparison.BOTH_UNRESOLVED; + const sameAccount: boolean = resourceAndPrincipalAccountComparison + ? equalOrBothUnresolved + // if the principal doesn't have an account (for example, a service principal), + // we should modify the resource's trust policy + : false; + // If we added to the principal AND we're in the same account, then we're done. + // If not, it's a different account and we must also add a trust policy on the resource. + if (result.success && sameAccount) { + return result; + } const statement = new PolicyStatement({ actions: options.actions, @@ -292,7 +307,7 @@ interface GrantProps { /** * A resource with a resource policy that can be added to */ -export interface IResourceWithPolicy extends cdk.IConstruct { +export interface IResourceWithPolicy extends cdk.IResource { /** * Add a statement to the resource's resource policy */ @@ -332,4 +347,4 @@ export class CompositeDependable implements cdk.IDependable { }, }); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-iam/lib/group.ts b/packages/@aws-cdk/aws-iam/lib/group.ts index 08ef30cba9203..d04df1948662c 100644 --- a/packages/@aws-cdk/aws-iam/lib/group.ts +++ b/packages/@aws-cdk/aws-iam/lib/group.ts @@ -72,6 +72,7 @@ abstract class GroupBase extends Resource implements IGroup { public abstract readonly groupArn: string; public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount: string | undefined = this.env.account; public readonly assumeRoleAction: string = 'sts:AssumeRole'; private readonly attachedPolicies = new AttachedPolicies(); @@ -143,10 +144,12 @@ export class Group extends GroupBase { * @param groupArn the ARN of the group to import (e.g. `arn:aws:iam::account-id:group/group-name`) */ public static fromGroupArn(scope: Construct, id: string, groupArn: string): IGroup { - const groupName = Stack.of(scope).parseArn(groupArn).resourceName!; + const arnComponents = Stack.of(scope).parseArn(groupArn); + const groupName = arnComponents.resourceName!; class Import extends GroupBase { public groupName = groupName; public groupArn = groupArn; + public principalAccount = arnComponents.account; } return new Import(scope, id); diff --git a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts index febb6372d25e6..8d299c5d14bb6 100644 --- a/packages/@aws-cdk/aws-iam/lib/lazy-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/lazy-role.ts @@ -26,6 +26,7 @@ export interface LazyRoleProps extends RoleProps { */ export class LazyRole extends cdk.Resource implements IRole { public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount: string | undefined = this.env.account; public readonly assumeRoleAction: string = 'sts:AssumeRole'; private role?: Role; diff --git a/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts b/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts index b8345827699d1..352c6a7bf70b0 100644 --- a/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts +++ b/packages/@aws-cdk/aws-iam/lib/oidc-provider.ts @@ -1,5 +1,5 @@ import * as path from 'path'; -import { Construct, CustomResource, CustomResourceProvider, CustomResourceProviderRuntime, IResource, Resource, Stack, Token } from '@aws-cdk/core'; +import { Construct, CustomResource, CustomResourceProvider, CustomResourceProviderRuntime, IResource, Resource, Token } from '@aws-cdk/core'; const RESOURCE_TYPE = 'Custom::AWSCDKOpenIdConnectProvider'; @@ -88,8 +88,9 @@ export interface OpenIdConnectProviderProps { * @see https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_providers_oidc.html * * @experimental + * @resource AWS::CloudFormation::CustomResource */ -export class OpenIdConnectProvider extends Construct implements IOpenIdConnectProvider { +export class OpenIdConnectProvider extends Resource implements IOpenIdConnectProvider { /** * Imports an Open ID connect provider from an ARN. * @param scope The definition scope @@ -130,8 +131,6 @@ export class OpenIdConnectProvider extends Construct implements IOpenIdConnectPr this.openIdConnectProviderArn = Token.asString(resource.ref); } - public get stack() { return Stack.of(this); } - private getOrCreateProvider() { return CustomResourceProvider.getOrCreate(this, RESOURCE_TYPE, { codeDirectory: path.join(__dirname, 'oidc-provider'), diff --git a/packages/@aws-cdk/aws-iam/lib/principals.ts b/packages/@aws-cdk/aws-iam/lib/principals.ts index 53e7aca1968d7..ee4cdf0c43e27 100644 --- a/packages/@aws-cdk/aws-iam/lib/principals.ts +++ b/packages/@aws-cdk/aws-iam/lib/principals.ts @@ -42,6 +42,15 @@ export interface IPrincipal extends IGrantable { */ readonly policyFragment: PrincipalPolicyFragment; + /** + * The AWS account ID of this principal. + * Can be undefined when the account is not known + * (for example, for service principals). + * Can be a Token - in that case, + * it's assumed to be AWS::AccountId. + */ + readonly principalAccount?: string; + /** * Add to the policy of this principal. * @@ -83,6 +92,7 @@ export interface AddToPrincipalPolicyResult { */ export abstract class PrincipalBase implements IPrincipal { public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount: string | undefined = undefined; /** * Return the policy fragment that identifies this principal in a Policy. diff --git a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts index 6a39cfcc8ebd7..f3d03b29ec69b 100644 --- a/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts +++ b/packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts @@ -1,4 +1,4 @@ -import { ConcreteDependable, Construct, DependableTrait } from '@aws-cdk/core'; +import { ConcreteDependable, Construct, DependableTrait, Resource } from '@aws-cdk/core'; import { Grant } from '../grant'; import { IManagedPolicy } from '../managed-policy'; import { Policy } from '../policy'; @@ -19,16 +19,20 @@ import { IRole } from '../role'; * which was imported into the CDK with {@link Role.fromRoleArn}, you don't have to use this class - * simply pass the property mutable = false when calling {@link Role.fromRoleArn}. */ -export class ImmutableRole extends Construct implements IRole { +export class ImmutableRole extends Resource implements IRole { public readonly assumeRoleAction = this.role.assumeRoleAction; public readonly policyFragment = this.role.policyFragment; public readonly grantPrincipal = this; + public readonly principalAccount = this.role.principalAccount; public readonly roleArn = this.role.roleArn; public readonly roleName = this.role.roleName; public readonly stack = this.role.stack; constructor(scope: Construct, id: string, private readonly role: IRole) { - super(scope, id); + super(scope, id, { + account: role.env.account, + region: role.env.region, + }); // implement IDependable privately DependableTrait.implement(this, { @@ -60,4 +64,4 @@ export class ImmutableRole extends Construct implements IRole { public grantPassRole(grantee: IPrincipal): Grant { return this.role.grantPassRole(grantee); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-iam/lib/role.ts b/packages/@aws-cdk/aws-iam/lib/role.ts index 23cde93dc101c..4e4174de420cd 100644 --- a/packages/@aws-cdk/aws-iam/lib/role.ts +++ b/packages/@aws-cdk/aws-iam/lib/role.ts @@ -1,4 +1,4 @@ -import { Construct, Duration, Lazy, Resource, Stack, Token } from '@aws-cdk/core'; +import { Construct, Duration, Lazy, Resource, Stack, Token, TokenComparison } from '@aws-cdk/core'; import { Grant } from './grant'; import { CfnRole } from './iam.generated'; import { IIdentity } from './identity-base'; @@ -176,6 +176,7 @@ export class Role extends Resource implements IRole { const scopeStack = Stack.of(scope); const parsedArn = scopeStack.parseArn(roleArn); const resourceName = parsedArn.resourceName!; + const roleAccount = parsedArn.account; // service roles have an ARN like 'arn:aws:iam:::role/service-role/' // or 'arn:aws:iam:::role/service-role/servicename.amazonaws.com/service-role/' // we want to support these as well, so we just use the element after the last slash as role name @@ -183,6 +184,7 @@ export class Role extends Resource implements IRole { class Import extends Resource implements IRole { public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount = roleAccount; public readonly assumeRoleAction: string = 'sts:AssumeRole'; public readonly policyFragment = new ArnPrincipal(roleArn).policyFragment; public readonly roleArn = roleArn; @@ -190,6 +192,12 @@ export class Role extends Resource implements IRole { private readonly attachedPolicies = new AttachedPolicies(); private defaultPolicy?: Policy; + constructor(_scope: Construct, _id: string) { + super(_scope, _id, { + account: roleAccount, + }); + } + public addToPolicy(statement: PolicyStatement): boolean { return this.addToPrincipalPolicy(statement).statementAdded; } @@ -204,9 +212,11 @@ export class Role extends Resource implements IRole { } public attachInlinePolicy(policy: Policy): void { - const policyAccount = Stack.of(policy).account; - - if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) { + const thisAndPolicyAccountComparison = Token.compareStrings(this.env.account, policy.env.account); + const equalOrAnyUnresolved = thisAndPolicyAccountComparison === TokenComparison.SAME || + thisAndPolicyAccountComparison === TokenComparison.BOTH_UNRESOLVED || + thisAndPolicyAccountComparison === TokenComparison.ONE_UNRESOLVED; + if (equalOrAnyUnresolved) { this.attachedPolicies.attach(policy); policy.attachToRole(this); } @@ -236,23 +246,19 @@ export class Role extends Resource implements IRole { } } - const roleAccount = parsedArn.account; - - const scopeAccount = scopeStack.account; - - return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeAccount, roleAccount) - ? new Import(scope, id) - : new ImmutableRole(scope, `ImmutableRole${id}`, new Import(scope, id)); - - function accountsAreEqualOrOneIsUnresolved( - account1: string | undefined, - account2: string | undefined): boolean { - return Token.isUnresolved(account1) || Token.isUnresolved(account2) || - account1 === account2; - } + const importedRole = new Import(scope, id); + const roleArnAndScopeStackAccountComparison = Token.compareStrings(importedRole.env.account, scopeStack.account); + const equalOrAnyUnresolved = roleArnAndScopeStackAccountComparison === TokenComparison.SAME || + roleArnAndScopeStackAccountComparison === TokenComparison.BOTH_UNRESOLVED || + roleArnAndScopeStackAccountComparison === TokenComparison.ONE_UNRESOLVED; + // we only return an immutable Role if both accounts were explicitly provided, and different + return options.mutable !== false && equalOrAnyUnresolved + ? importedRole + : new ImmutableRole(scope, `ImmutableRole${id}`, importedRole); } public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount: string | undefined = this.env.account; public readonly assumeRoleAction: string = 'sts:AssumeRole'; diff --git a/packages/@aws-cdk/aws-iam/lib/user.ts b/packages/@aws-cdk/aws-iam/lib/user.ts index 886937fcf3cb8..1491d2c454e70 100644 --- a/packages/@aws-cdk/aws-iam/lib/user.ts +++ b/packages/@aws-cdk/aws-iam/lib/user.ts @@ -1,4 +1,4 @@ -import { Construct, Lazy, Resource, SecretValue, Stack } from '@aws-cdk/core'; +import { Aws, Construct, Lazy, Resource, SecretValue, Stack } from '@aws-cdk/core'; import { IGroup } from './group'; import { CfnUser } from './iam.generated'; import { IIdentity } from './identity-base'; @@ -139,6 +139,7 @@ export class User extends Resource implements IIdentity, IUser { class Import extends Resource implements IUser { public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount = Aws.ACCOUNT_ID; public readonly userName: string = userName; public readonly userArn: string = arn; public readonly assumeRoleAction: string = 'sts:AssumeRole'; @@ -175,6 +176,7 @@ export class User extends Resource implements IIdentity, IUser { } public readonly grantPrincipal: IPrincipal = this; + public readonly principalAccount: string | undefined = this.env.account; public readonly assumeRoleAction: string = 'sts:AssumeRole'; /** diff --git a/packages/@aws-cdk/aws-iam/package.json b/packages/@aws-cdk/aws-iam/package.json index a0dcc893ca6da..0f37067cedab6 100644 --- a/packages/@aws-cdk/aws-iam/package.json +++ b/packages/@aws-cdk/aws-iam/package.json @@ -89,6 +89,7 @@ "exclude": [ "from-signature:@aws-cdk/aws-iam.Role.fromRoleArn", "construct-interface-extends-iconstruct:@aws-cdk/aws-iam.IManagedPolicy", + "props-physical-name:@aws-cdk/aws-iam.OpenIdConnectProviderProps", "resource-interface-extends-resource:@aws-cdk/aws-iam.IManagedPolicy", "docs-public-apis:@aws-cdk/aws-iam.IUser" ] diff --git a/packages/@aws-cdk/aws-iam/test/cross-account.test.ts b/packages/@aws-cdk/aws-iam/test/cross-account.test.ts new file mode 100644 index 0000000000000..619a3bf5b5b34 --- /dev/null +++ b/packages/@aws-cdk/aws-iam/test/cross-account.test.ts @@ -0,0 +1,235 @@ +import '@aws-cdk/assert/jest'; +import * as cdk from '@aws-cdk/core'; +import * as iam from '../lib'; + +// Test cross-account grant scenario's for principals +// +// When doing a grant on a resource with a resource policy: +// +// - Permissions are added to the identity if possible. +// - Trust is added to the resource if necessary (identity is in +// a different account than the resource). + +let app: cdk.App; +const stack1Account = '1234'; +let stack1: cdk.Stack; +const stack2Account = '5678'; +let stack2: cdk.Stack; +const thirdAccount = '123456789012'; + +beforeEach(() => { + app = new cdk.App(); + stack1 = new cdk.Stack(app, 'Stack1', { env: { account: stack1Account, region: 'us-bla-5' } }); + stack2 = new cdk.Stack(app, 'Stack2', { env: { account: stack2Account, region: 'us-bla-5' } }); +}); + +test('cross-account Role grant creates permissions AND trust', () => { + // GIVEN + const role = new iam.Role(stack1, 'Role', { + roleName: cdk.PhysicalName.GENERATE_IF_NEEDED, + assumedBy: new iam.ServicePrincipal('some.service'), + }); + const resource = new FakeResource(stack2, 'Resource'); + + // WHEN + doGrant(resource, role); + + // THEN + assertPolicyCreated(stack1); + assertTrustCreated(stack2, { + AWS: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + `:iam::${stack1Account}:role/stack1stack1rolef3c14260253562f428b7`, + ]], + }, + }); +}); + +test('Service Principal grant creates trust', () => { + const resource = new FakeResource(stack2, 'Resource'); + + // WHEN + doGrant(resource, new iam.ServicePrincipal('service.amazonaws.com')); + + // THEN + assertTrustCreated(stack2, { Service: 'service.amazonaws.com' }); +}); + +test('Imported Role with definitely different account grant creates trust', () => { + const resource = new FakeResource(stack2, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', `arn:aws:iam::${thirdAccount}:role/S3Access`, { mutable: true }); + + // WHEN + doGrant(resource, role); + + // THEN + noPolicyCreated(stack2); + assertTrustCreated(stack2, { + AWS: `arn:aws:iam::${thirdAccount}:role/S3Access`, + }); +}); + +test('Imported Role with partition token in ARN (definitely different account) grant creates trust', () => { + const resource = new FakeResource(stack2, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', `arn:${stack2.partition}:iam::${thirdAccount}:role/S3Access`, { mutable: true }); + + // WHEN + doGrant(resource, role); + + // THEN + noPolicyCreated(stack2); + assertTrustCreated(stack2, { + AWS: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + `:iam::${thirdAccount}:role/S3Access`, + ]], + }, + }); +}); + +test('Imported Role with definitely same account grant does not create trust', () => { + const resource = new FakeResource(stack2, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', `arn:aws:iam::${stack2Account}:role/S3Access`, { mutable: true }); + + // WHEN + doGrant(resource, role); + + // THEN + assertPolicyCreated(stack2); + noTrustCreated(stack2); +}); + +test('Imported Role with partition token and definitely same account grant does not create trust', () => { + const resource = new FakeResource(stack2, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', `arn:${stack2.partition}:iam::5678:role/S3Access`, { mutable: true }); + + // WHEN + doGrant(resource, role); + + // THEN + assertPolicyCreated(stack2); + noTrustCreated(stack2); +}); + +test('Agnostic stack with concrete imported role adds trust', () => { + // GIVEN + const stack = new cdk.Stack(app, 'AgStack'); + const resource = new FakeResource(stack, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', 'arn:aws:iam::5678:role/S3Access', { mutable: true }); + + // WHEN + doGrant(resource, role); + + // THEN + assertTrustCreated(stack, { AWS: 'arn:aws:iam::5678:role/S3Access' }); + noPolicyCreated(stack); +}); + +test('Agnostic stack with agnostic imported role does not add trust', () => { + // GIVEN + const stack = new cdk.Stack(app, 'AgStack'); + const resource = new FakeResource(stack, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', `arn:aws:iam::${cdk.Aws.ACCOUNT_ID}:role/S3Access`, { mutable: true }); + + // WHEN + doGrant(resource, role); + + // THEN + assertPolicyCreated(stack2); + noTrustCreated(stack); +}); + +test('Immutable role in same account adds no policy and no trust', () => { + // GIVEN + const resource = new FakeResource(stack2, 'Resource'); + const role = iam.Role.fromRoleArn(stack2, 'Role', `arn:aws:iam::${stack2Account}:role/S3Access`, { mutable: false }); + + // require('util').inspect.defaultOptions.customInspect = false; // ? + + // WHEN + doGrant(resource, role); + + // THEN + noTrustCreated(stack2); + noPolicyCreated(stack2); + +}); + +class FakeResource extends cdk.Resource implements iam.IResourceWithPolicy { + public readonly arn = 'arn:aws:resource'; + private readonly policy = new iam.PolicyDocument(); + + constructor(scope: cdk.Construct, id: string) { + super(scope, id); + + new cdk.CfnResource(this, 'Default', { + type: 'Test::Fake::Resource', + properties: { + ResourcePolicy: cdk.Lazy.anyValue({ produce: () => this.policy }), + }, + }); + } + + public addToResourcePolicy(statement: iam.PolicyStatement): iam.AddToResourcePolicyResult { + this.policy.addStatements(statement); + return { statementAdded: true, policyDependable: this.policy }; + } +} + +function doGrant(resource: FakeResource, principal: iam.IPrincipal) { + iam.Grant.addToPrincipalOrResource({ + actions: ['some:action'], + grantee: principal, + resourceArns: [resource.arn], + resource, + }); +} + +function assertTrustCreated(stack: cdk.Stack, principal: any) { + expect(stack).toHaveResource('Test::Fake::Resource', { + ResourcePolicy: { + Statement: [ + { + Action: 'some:action', + Effect: 'Allow', + Resource: 'arn:aws:resource', + Principal: principal, + }, + ], + Version: '2012-10-17', + }, + }); +} + +function noTrustCreated(stack: cdk.Stack) { + expect(stack).not.toHaveResourceLike('Test::Fake::Resource', { + ResourcePolicy: { + Statement: [ + {}, + ], + }, + }); +} + +function assertPolicyCreated(stack: cdk.Stack) { + expect(stack).toHaveResource('AWS::IAM::Policy', { + PolicyDocument: { + Statement: [ + { + Action: 'some:action', + Effect: 'Allow', + Resource: 'arn:aws:resource', + }, + ], + Version: '2012-10-17', + }, + }); +} + +function noPolicyCreated(stack: cdk.Stack) { + expect(stack).not.toHaveResource('AWS::IAM::Policy'); +} diff --git a/packages/@aws-cdk/aws-iam/test/grant.test.ts b/packages/@aws-cdk/aws-iam/test/grant.test.ts index 93143f07c8125..3a1d7d580fcb0 100644 --- a/packages/@aws-cdk/aws-iam/test/grant.test.ts +++ b/packages/@aws-cdk/aws-iam/test/grant.test.ts @@ -1,6 +1,6 @@ import { ResourcePart } from '@aws-cdk/assert'; import '@aws-cdk/assert/jest'; -import { CfnResource, Construct, Stack } from '@aws-cdk/core'; +import { CfnResource, Construct, Resource, Stack } from '@aws-cdk/core'; import * as iam from '../lib'; let stack: Stack; @@ -113,13 +113,12 @@ function expectDependencyOn(id: string) { }, ResourcePart.CompleteDefinition); } -class FakeResourceWithPolicy extends CfnResource implements iam.IResourceWithPolicy { - private policy: CfnResource; +class FakeResourceWithPolicy extends Resource implements iam.IResourceWithPolicy { + private readonly policy: CfnResource; constructor(scope: Construct, id: string) { - super(scope, id, { - type: 'CDK::Test::Buckaroo', - }); + super(scope, id); + this.policy = new CfnResource(this, 'Policy', { type: 'CDK::Test::BuckarooPolicy', }); @@ -128,4 +127,4 @@ class FakeResourceWithPolicy extends CfnResource implements iam.IResourceWithPol public addToResourcePolicy(_statement: iam.PolicyStatement): iam.AddToResourcePolicyResult { return { statementAdded: true, policyDependable: this.policy }; } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/aws-lambda/lib/function-base.ts b/packages/@aws-cdk/aws-lambda/lib/function-base.ts index 41a7a3307cf52..02b0e5d3285b1 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function-base.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function-base.ts @@ -287,6 +287,8 @@ export abstract class FunctionBase extends Resource implements IFunction { return { statementAdded: true, policyDependable: this._functionNode().findChild(identifier) } as iam.AddToResourcePolicyResult; }, node: this.node, + stack: this.stack, + env: this.env, }, }); } diff --git a/packages/@aws-cdk/aws-route53/test/test.util.ts b/packages/@aws-cdk/aws-route53/test/test.util.ts index 006527d3ad869..d589b058e40cc 100644 --- a/packages/@aws-cdk/aws-route53/test/test.util.ts +++ b/packages/@aws-cdk/aws-route53/test/test.util.ts @@ -1,5 +1,6 @@ import * as cdk from '@aws-cdk/core'; import { Test } from 'nodeunit'; +import { HostedZone } from '../lib'; import * as util from '../lib/util'; export = { @@ -20,13 +21,10 @@ export = { // WHEN const providedName = 'test.domain.com.'; - const qualified = util.determineFullyQualifiedDomainName(providedName, { + const qualified = util.determineFullyQualifiedDomainName(providedName, HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'ignored', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }); + })); // THEN test.equal(qualified, 'test.domain.com.'); @@ -39,13 +37,10 @@ export = { // WHEN const providedName = 'test.domain.com'; - const qualified = util.determineFullyQualifiedDomainName(providedName, { + const qualified = util.determineFullyQualifiedDomainName(providedName, HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'test.domain.com.', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }); + })); // THEN test.equal(qualified, 'test.domain.com.'); @@ -58,13 +53,10 @@ export = { // WHEN const providedName = 'test.domain.com'; - const qualified = util.determineFullyQualifiedDomainName(providedName, { + const qualified = util.determineFullyQualifiedDomainName(providedName, HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'domain.com.', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }); + })); // THEN test.equal(qualified, 'test.domain.com.'); @@ -77,13 +69,10 @@ export = { // WHEN const providedName = 'test'; - const qualified = util.determineFullyQualifiedDomainName(providedName, { + const qualified = util.determineFullyQualifiedDomainName(providedName, HostedZone.fromHostedZoneAttributes(stack, 'HostedZone', { hostedZoneId: 'fakeId', zoneName: 'domain.com.', - hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId', - stack, - node: stack.node, - }); + })); // THEN test.equal(qualified, 'test.domain.com.'); diff --git a/packages/@aws-cdk/aws-s3/lib/bucket.ts b/packages/@aws-cdk/aws-s3/lib/bucket.ts index d251815c97a94..d767011ecb5d3 100644 --- a/packages/@aws-cdk/aws-s3/lib/bucket.ts +++ b/packages/@aws-cdk/aws-s3/lib/bucket.ts @@ -291,6 +291,20 @@ export interface BucketAttributes { * @default false */ readonly isWebsite?: boolean; + + /** + * The account this existing bucket belongs to. + * + * @default - it's assumed the bucket belongs to the same account as the scope it's being imported into + */ + readonly account?: string; + + /** + * The region this existing bucket is in. + * + * @default - it's assumed the bucket is in the same region as the scope it's being imported into + */ + readonly region?: string; } /** @@ -633,25 +647,12 @@ abstract class BucketBase extends Resource implements IBucket { resourceArn: string, ...otherResourceArns: string[]) { const resources = [resourceArn, ...otherResourceArns]; - const crossAccountAccess = this.isGranteeFromAnotherAccount(grantee); - let ret: iam.Grant; - if (crossAccountAccess) { - // if the access is cross-account, we need to trust the accessing principal in the bucket's policy - ret = iam.Grant.addToPrincipalAndResource({ - grantee, - actions: bucketActions, - resourceArns: resources, - resource: this, - }); - } else { - // if not, we don't need to modify the resource policy if the grantee is an identity principal - ret = iam.Grant.addToPrincipalOrResource({ - grantee, - actions: bucketActions, - resourceArns: resources, - resource: this, - }); - } + const ret = iam.Grant.addToPrincipalOrResource({ + grantee, + actions: bucketActions, + resourceArns: resources, + resource: this, + }); if (this.encryptionKey && keyActions && keyActions.length !== 0) { this.encryptionKey.grant(grantee, ...keyActions); @@ -659,15 +660,6 @@ abstract class BucketBase extends Resource implements IBucket { return ret; } - - 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; - } } export interface BlockPublicAccessOptions { @@ -1172,7 +1164,10 @@ export class Bucket extends BucketBase { } } - return new Import(scope, id); + return new Import(scope, id, { + account: attrs.account, + region: attrs.region, + }); } public readonly bucketArn: string; diff --git a/packages/@aws-cdk/aws-s3/test/test.util.ts b/packages/@aws-cdk/aws-s3/test/test.util.ts index dfa612da7d335..96276f681d499 100644 --- a/packages/@aws-cdk/aws-s3/test/test.util.ts +++ b/packages/@aws-cdk/aws-s3/test/test.util.ts @@ -51,37 +51,10 @@ export = { const stack = new cdk.Stack(); const bucketArn = `arn:aws:s3:::${cdk.Token.asString({ Ref: 'my-bucket' })}`; - test.deepEqual(stack.resolve(parseBucketName(stack, { bucketArn })), { - 'Fn::Select': [ - 0, - { - 'Fn::Split': [ - '/', - { - 'Fn::Select': [ - 5, - { - 'Fn::Split': [ - ':', - { - 'Fn::Join': [ - '', - [ - 'arn:aws:s3:::', - { - Ref: 'my-bucket', - }, - ], - ], - }, - ], - }, - ], - }, - ], - }, - ], - }); + test.deepEqual( + stack.resolve(parseBucketName(stack, { bucketArn })), + { Ref: 'my-bucket' }, + ); test.done(); }, diff --git a/packages/@aws-cdk/core/lib/arn.ts b/packages/@aws-cdk/core/lib/arn.ts index 16b487280b432..60dc79ab79d35 100644 --- a/packages/@aws-cdk/core/lib/arn.ts +++ b/packages/@aws-cdk/core/lib/arn.ts @@ -135,11 +135,15 @@ export class Arn { * components of the ARN. */ public static parse(arn: string, sepIfToken: string = '/', hasName: boolean = true): ArnComponents { - if (Token.isUnresolved(arn)) { + const components = arn.split(':') as Array; + const looksLikeArn = arn.startsWith('arn:') && components.length >= 6 && components.length <= 7; + if (Token.isUnresolved(arn) && !looksLikeArn) { return parseToken(arn, sepIfToken, hasName); } - - const components = arn.split(':') as Array; + // If the ARN merely contains Tokens, but otherwise *looks* mostly like an ARN, + // it's a string of the form 'arn:${partition}:service:${region}:${account}:abc/xyz'. + // Parse fields out to the best of our ability. + // Tokens won't contain ":", so this won't break them. if (components.length < 6) { throw new Error('ARNs must have at least 6 components: ' + arn); diff --git a/packages/@aws-cdk/core/lib/cfn-pseudo.ts b/packages/@aws-cdk/core/lib/cfn-pseudo.ts index dc1848aa49ca4..7144b7edc2af1 100644 --- a/packages/@aws-cdk/core/lib/cfn-pseudo.ts +++ b/packages/@aws-cdk/core/lib/cfn-pseudo.ts @@ -77,5 +77,10 @@ export class ScopedAws { } function pseudoString(name: string): string { - return Token.asString({ Ref: name }, { displayHint: name }); + // we don't want any ':' in the serialized form, + // as ':' is the ARN separator, + // and so we don't want ARN components + // (which these CFN references like AWS::Partition certainly can be) + // to contain ':'s themselves + return Token.asString({ Ref: name }, { displayHint: name.replace('::', '.') }); } diff --git a/packages/@aws-cdk/core/lib/resource.ts b/packages/@aws-cdk/core/lib/resource.ts index 6574dbe331da0..1eb1da50f7335 100644 --- a/packages/@aws-cdk/core/lib/resource.ts +++ b/packages/@aws-cdk/core/lib/resource.ts @@ -6,6 +6,30 @@ import { IResolveContext } from './resolvable'; import { Stack } from './stack'; import { Token } from './token'; +/** + * Represents the environment a given resource lives in. + * Used as the return value for the {@link IResource.env} property. + */ +export interface ResourceEnvironment { + /** + * The AWS account ID that this resource belongs to. + * Since this can be a Token + * (for example, when the account is CloudFormation's AWS::AccountId intrinsic), + * make sure to use Token.compareStrings() + * instead of just comparing the values for equality. + */ + readonly account: string; + + /** + * The AWS region that this resource belongs to. + * Since this can be a Token + * (for example, when the region is CloudFormation's AWS::Region intrinsic), + * make sure to use Token.compareStrings() + * instead of just comparing the values for equality. + */ + readonly region: string; +} + /** * Interface for the Resource construct. */ @@ -14,6 +38,17 @@ export interface IResource extends IConstruct { * The stack in which this resource is defined. */ readonly stack: Stack; + + /** + * The environment this resource belongs to. + * For resources that are created and managed by the CDK + * (generally, those created by creating new class instances like Role, Bucket, etc.), + * this is always the same as the environment of the stack they belong to; + * however, for imported resources + * (those obtained from static methods like fromRoleArn, fromBucketName, etc.), + * that might be different than the stack they were imported into. + */ + readonly env: ResourceEnvironment; } /** @@ -32,6 +67,20 @@ export interface ResourceProps { * @default - The physical name will be allocated by CloudFormation at deployment time */ readonly physicalName?: string; + + /** + * The AWS account ID this resource belongs to. + * + * @default - the resource is in the same account as the stack it belongs to + */ + readonly account?: string; + + /** + * The AWS region this resource belongs to. + * + * @default - the resource is in the same region as the stack it belongs to + */ + readonly region?: string; } /** @@ -39,6 +88,7 @@ export interface ResourceProps { */ export abstract class Resource extends Construct implements IResource { public readonly stack: Stack; + public readonly env: ResourceEnvironment; /** * Returns a string-encoded token that resolves to the physical name that @@ -59,7 +109,12 @@ export abstract class Resource extends Construct implements IResource { constructor(scope: Construct, id: string, props: ResourceProps = {}) { super(scope, id); + this.stack = Stack.of(this); + this.env = { + account: props.account ?? this.stack.account, + region: props.region ?? this.stack.region, + }; let physicalName = props.physicalName; diff --git a/packages/@aws-cdk/core/lib/token.ts b/packages/@aws-cdk/core/lib/token.ts index 1816eb32da3dd..73a43c74f6e7f 100644 --- a/packages/@aws-cdk/core/lib/token.ts +++ b/packages/@aws-cdk/core/lib/token.ts @@ -7,6 +7,33 @@ import { TokenMap } from './private/token-map'; import { IResolvable, ITokenResolver } from './resolvable'; import { TokenizedStringFragments } from './string-fragments'; +/** + * An enum-like class that represents the result of comparing two Tokens. + * The return type of {@link Token.compareStrings}. + */ +export class TokenComparison { + /** + * This means we're certain the two components are NOT + * Tokens, and identical. + */ + public static readonly SAME = new TokenComparison(); + + /** + * This means we're certain the two components are NOT + * Tokens, and different. + */ + public static readonly DIFFERENT = new TokenComparison(); + + /** This means exactly one of the components is a Token. */ + public static readonly ONE_UNRESOLVED = new TokenComparison(); + + /** This means both components are Tokens. */ + public static readonly BOTH_UNRESOLVED = new TokenComparison(); + + private constructor() { + } +} + /** * Represents a special or lazily-evaluated value. * @@ -75,6 +102,21 @@ export class Token { return isResolvableObject(value) ? value : new Intrinsic(value); } + /** Compare two strings that might contain Tokens with each other. */ + public static compareStrings(possibleToken1: string, possibleToken2: string): TokenComparison { + const firstIsUnresolved = Token.isUnresolved(possibleToken1); + const secondIsUnresolved = Token.isUnresolved(possibleToken2); + + if (firstIsUnresolved && secondIsUnresolved) { + return TokenComparison.BOTH_UNRESOLVED; + } + if (firstIsUnresolved || secondIsUnresolved) { + return TokenComparison.ONE_UNRESOLVED; + } + + return possibleToken1 === possibleToken2 ? TokenComparison.SAME : TokenComparison.DIFFERENT; + } + private constructor() { } } diff --git a/packages/@aws-cdk/core/test/test.arn.ts b/packages/@aws-cdk/core/test/test.arn.ts index 6467ca78df8df..820ceeae21f40 100644 --- a/packages/@aws-cdk/core/test/test.arn.ts +++ b/packages/@aws-cdk/core/test/test.arn.ts @@ -1,5 +1,5 @@ import { Test } from 'nodeunit'; -import { ArnComponents, CfnOutput, ScopedAws, Stack } from '../lib'; +import { ArnComponents, Aws, CfnOutput, ScopedAws, Stack } from '../lib'; import { Intrinsic } from '../lib/private/intrinsic'; import { toCloudFormation } from './util'; @@ -258,4 +258,23 @@ export = { test.done(); }, + + 'parse other fields if only some are tokens'(test: Test) { + // GIVEN + const stack = new Stack(); + + // WHEN + const parsed = stack.parseArn(`arn:${Aws.PARTITION}:iam::123456789012:role/S3Access`); + + // THEN + test.deepEqual(stack.resolve(parsed.partition), { Ref: 'AWS::Partition' }); + test.deepEqual(stack.resolve(parsed.service), 'iam'); + test.equal(stack.resolve(parsed.region), ''); + test.deepEqual(stack.resolve(parsed.account), '123456789012'); + test.deepEqual(stack.resolve(parsed.resource), 'role'); + test.deepEqual(stack.resolve(parsed.resourceName), 'S3Access'); + test.equal(parsed.sep, '/'); + + test.done(); + }, }; diff --git a/packages/@aws-cdk/pipelines/lib/actions/deploy-cdk-stack-action.ts b/packages/@aws-cdk/pipelines/lib/actions/deploy-cdk-stack-action.ts index c13dad054c790..902409c43a331 100644 --- a/packages/@aws-cdk/pipelines/lib/actions/deploy-cdk-stack-action.ts +++ b/packages/@aws-cdk/pipelines/lib/actions/deploy-cdk-stack-action.ts @@ -4,7 +4,7 @@ import * as codepipeline from '@aws-cdk/aws-codepipeline'; import * as cpactions from '@aws-cdk/aws-codepipeline-actions'; import * as events from '@aws-cdk/aws-events'; import * as iam from '@aws-cdk/aws-iam'; -import { Arn, Construct, Fn, Stack } from '@aws-cdk/core'; +import { Aws, Construct, Stack } from '@aws-cdk/core'; import * as cxapi from '@aws-cdk/cx-api'; import { appOf, assemblyBuilderOf } from '../private/construct-internals'; @@ -144,11 +144,13 @@ export class DeployCdkStackAction implements codepipeline.IAction { throw new Error(`Stack '${artifact.stackName}' does not have deployment role information; use the 'DefaultStackSynthesizer' synthesizer, or set the '@aws-cdk/core:newStyleStackSynthesis' context key.`); } - const actionRole = roleFromPlaceholderArn(scope, artifact.assumeRoleArn); - const cloudFormationExecutionRole = roleFromPlaceholderArn(scope, artifact.cloudFormationExecutionRoleArn); - const artRegion = artifact.environment.region; const region = artRegion === Stack.of(scope).region || artRegion === cxapi.UNKNOWN_REGION ? undefined : artRegion; + const artAccount = artifact.environment.account; + const account = artAccount === Stack.of(scope).account || artAccount === cxapi.UNKNOWN_ACCOUNT ? undefined : artAccount; + + const actionRole = roleFromPlaceholderArn(scope, region, account, artifact.assumeRoleArn); + const cloudFormationExecutionRole = roleFromPlaceholderArn(scope, region, account, artifact.cloudFormationExecutionRoleArn); // We need the path of the template relative to the root Cloud Assembly // It should be easier to get this, but for now it is what it is. @@ -259,16 +261,18 @@ export class DeployCdkStackAction implements codepipeline.IAction { } } -function roleFromPlaceholderArn(scope: Construct, arn: string): iam.IRole; -function roleFromPlaceholderArn(scope: Construct, arn: string | undefined): iam.IRole | undefined; -function roleFromPlaceholderArn(scope: Construct, arn: string | undefined): iam.IRole | undefined { +function roleFromPlaceholderArn(scope: Construct, region: string | undefined, + account: string | undefined, arn: string): iam.IRole; +function roleFromPlaceholderArn(scope: Construct, region: string | undefined, + account: string | undefined, arn: string | undefined): iam.IRole | undefined; +function roleFromPlaceholderArn(scope: Construct, region: string | undefined, + account: string | undefined, arn: string | undefined): iam.IRole | undefined { + if (!arn) { return undefined; } // Use placeholdered arn as construct ID. const id = arn; - scope = hackyRoleScope(scope, arn); - // https://github.com/aws/aws-cdk/issues/7255 let existingRole = scope.node.tryFindChild(`ImmutableRole${id}`) as iam.IRole; if (existingRole) { return existingRole; } @@ -276,44 +280,12 @@ function roleFromPlaceholderArn(scope: Construct, arn: string | undefined): iam. existingRole = scope.node.tryFindChild(id) as iam.IRole; if (existingRole) { return existingRole; } - return iam.Role.fromRoleArn(scope, id, cfnExpressionFromManifestString(arn), { mutable: false }); -} - -/** - * MASSIVE HACK - * - * We have a bug in the CDK where it's only going to consider Roles that are physically in a - * different Stack object from the Pipeline "cross-account", and will add the appropriate - * Bucket/Key policies. - * https://github.com/aws/aws-cdk/pull/8280 will resolve this, but for now we fake it by hacking - * up a Stack object to root the role in! - * - * Fortunatey, we can just 'new up' an unrooted Stack (unit tests do this all the time) and toss it - * away. It will never be synthesized, but all the logic happens to work out! - */ -function hackyRoleScope(scope: Construct, arn: string): Construct { - const parts = Arn.parse(cxapi.EnvironmentPlaceholders.replace(arn, { - accountId: '', // Empty string on purpose, see below - partition: '', - region: '', - })); - return new Stack(undefined, undefined, { - env: { - // Empty string means ARN had a placeholder which means same account as pipeline stack - account: parts.account || Stack.of(scope).account, - // 'region' from an IAM ARN is always an empty string, so no point. - }, + const arnToImport = cxapi.EnvironmentPlaceholders.replace(arn, { + region: region ?? Aws.REGION, + accountId: account ?? Aws.ACCOUNT_ID, + partition: Aws.PARTITION, }); -} - -/** - * Return a CloudFormation expression from a manifest string with placeholders - */ -function cfnExpressionFromManifestString(s: string) { - // This implementation relies on the fact that the manifest placeholders are - // '${AWS::Partition}' etc., and so are the same values as those that are - // trivially substituable using a `Fn.sub`. - return Fn.sub(s); + return iam.Role.fromRoleArn(scope, id, arnToImport, { mutable: false }); } /** diff --git a/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts b/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts index c1370c4378b16..15ee878022574 100644 --- a/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts +++ b/packages/@aws-cdk/pipelines/test/cross-environment-infra.test.ts @@ -35,7 +35,11 @@ test('in a cross-account/cross-region setup, artifact bucket can be read by depl Action: arrayWith('s3:GetObject*', 's3:GetBucket*', 's3:List*'), Principal: { AWS: { - 'Fn::Sub': stringLike('*-deploy-role-*'), + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + stringLike('*-deploy-role-*'), + ]], }, }, })), @@ -56,7 +60,11 @@ test('in a cross-account/same-region setup, artifact bucket can be read by deplo Action: ['s3:GetObject*', 's3:GetBucket*', 's3:List*'], Principal: { AWS: { - 'Fn::Sub': stringLike('*-deploy-role-*'), + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + stringLike('*-deploy-role-*'), + ]], }, }, })), @@ -72,4 +80,4 @@ class TestApplication extends Stage { super(scope, id, props); new BucketStack(this, 'Stack'); } -} \ No newline at end of file +} diff --git a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json index 777cffa9703f6..67229ae806bb0 100644 --- a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json +++ b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.expected.json @@ -411,7 +411,16 @@ "Effect": "Allow", "Principal": { "AWS": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] } }, "Resource": "*" @@ -580,7 +589,16 @@ "Action": "sts:AssumeRole", "Effect": "Allow", "Resource": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] } } ], @@ -797,7 +815,16 @@ "StackName": "PreProd-Stack", "Capabilities": "CAPABILITY_NAMED_IAM,CAPABILITY_AUTO_EXPAND", "RoleArn": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-cfn-exec-role-12345678-test-region" + ] + ] }, "ActionMode": "CHANGE_SET_REPLACE", "ChangeSetName": "PipelineChange", @@ -810,7 +837,16 @@ ], "Name": "Stack.Prepare", "RoleArn": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] }, "RunOrder": 1 }, @@ -828,7 +864,16 @@ }, "Name": "Stack.Deploy", "RoleArn": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] }, "RunOrder": 2 } @@ -1575,7 +1620,7 @@ ] }, "Source": { - "BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"8289faf53c7da377bb2b90615999171adef5e1d8f6b88810e5fef75e6ca09ba5:current_account-current_region\\\"\"\n ]\n }\n }\n}", + "BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"8289faf53c7da377bb2b90615999171adef5e1d8f6b88810e5fef75e6ca09ba5:12345678-test-region\\\"\"\n ]\n }\n }\n}", "Type": "CODEPIPELINE" }, "EncryptionKey": { @@ -1605,7 +1650,7 @@ ] }, "Source": { - "BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"ac76997971c3f6ddf37120660003f1ced72b4fc58c498dfd99c78fa77e721e0e:current_account-current_region\\\"\"\n ]\n }\n }\n}", + "BuildSpec": "{\n \"version\": \"0.2\",\n \"phases\": {\n \"install\": {\n \"commands\": \"npm install -g cdk-assets\"\n },\n \"build\": {\n \"commands\": [\n \"cdk-assets --path \\\"assembly-PipelineStack-PreProd/PipelineStackPreProdStack65A0AD1F.assets.json\\\" --verbose publish \\\"ac76997971c3f6ddf37120660003f1ced72b4fc58c498dfd99c78fa77e721e0e:12345678-test-region\\\"\"\n ]\n }\n }\n}", "Type": "CODEPIPELINE" }, "EncryptionKey": { diff --git a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.ts b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.ts index ce3698f5aba24..07a59ce4370eb 100644 --- a/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.ts +++ b/packages/@aws-cdk/pipelines/test/integ.pipeline-with-assets.ts @@ -10,7 +10,7 @@ class MyStage extends Stage { constructor(scope: Construct, id: string, props?: StageProps) { super(scope, id, props); - const stack = new Stack(this, 'Stack'); + const stack = new Stack(this, 'Stack', props); new s3_assets.Asset(stack, 'Asset', { path: path.join(__dirname, 'test-file-asset.txt'), @@ -65,7 +65,9 @@ class CdkpipelinesDemoPipelineStack extends Stack { // This is where we add the application stages // ... - const stage = pipeline.addApplicationStage(new MyStage(this, 'PreProd')); + const stage = pipeline.addApplicationStage(new MyStage(this, 'PreProd', { + env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, + })); stage.addActions( new cdkp.ShellScriptAction({ actionName: 'UseSource', diff --git a/packages/@aws-cdk/pipelines/test/integ.pipeline.expected.json b/packages/@aws-cdk/pipelines/test/integ.pipeline.expected.json index 7063a7cf29a0d..78e618e62ae12 100644 --- a/packages/@aws-cdk/pipelines/test/integ.pipeline.expected.json +++ b/packages/@aws-cdk/pipelines/test/integ.pipeline.expected.json @@ -377,7 +377,16 @@ "Effect": "Allow", "Principal": { "AWS": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] } }, "Resource": "*" @@ -536,7 +545,16 @@ "Action": "sts:AssumeRole", "Effect": "Allow", "Resource": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] } } ], @@ -696,7 +714,16 @@ "StackName": "PreProd-Stack", "Capabilities": "CAPABILITY_NAMED_IAM,CAPABILITY_AUTO_EXPAND", "RoleArn": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-cfn-exec-role-12345678-test-region" + ] + ] }, "ActionMode": "CHANGE_SET_REPLACE", "ChangeSetName": "PipelineChange", @@ -709,7 +736,16 @@ ], "Name": "Stack.Prepare", "RoleArn": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] }, "RunOrder": 1 }, @@ -727,7 +763,16 @@ }, "Name": "Stack.Deploy", "RoleArn": { - "Fn::Sub": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}" + "Fn::Join": [ + "", + [ + "arn:", + { + "Ref": "AWS::Partition" + }, + ":iam::12345678:role/cdk-hnb659fds-deploy-role-12345678-test-region" + ] + ] }, "RunOrder": 2 } diff --git a/packages/@aws-cdk/pipelines/test/integ.pipeline.ts b/packages/@aws-cdk/pipelines/test/integ.pipeline.ts index f0a4da9dde073..e901758cde4e9 100644 --- a/packages/@aws-cdk/pipelines/test/integ.pipeline.ts +++ b/packages/@aws-cdk/pipelines/test/integ.pipeline.ts @@ -8,7 +8,7 @@ class MyStage extends Stage { constructor(scope: Construct, id: string, props?: StageProps) { super(scope, id, props); - const stack = new Stack(this, 'Stack'); + const stack = new Stack(this, 'Stack', props); new CfnResource(stack, 'Resource', { type: 'AWS::Test::SomeResource', }); @@ -55,7 +55,9 @@ class CdkpipelinesDemoPipelineStack extends Stack { // This is where we add the application stages // ... - const stage = pipeline.addApplicationStage(new MyStage(this, 'PreProd')); + const stage = pipeline.addApplicationStage(new MyStage(this, 'PreProd', { + env: { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }, + })); stage.addActions( new cdkp.ShellScriptAction({ actionName: 'UseSource', diff --git a/packages/@aws-cdk/pipelines/test/pipeline.test.ts b/packages/@aws-cdk/pipelines/test/pipeline.test.ts index 93137064a84cc..3f216464c55ed 100644 --- a/packages/@aws-cdk/pipelines/test/pipeline.test.ts +++ b/packages/@aws-cdk/pipelines/test/pipeline.test.ts @@ -51,15 +51,48 @@ test('action has right settings for same-env deployment', () => { Actions: [ objectLike({ Name: 'Stack.Prepare', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':role/cdk-hnb659fds-deploy-role-', + { Ref: 'AWS::AccountId' }, + '-', + { Ref: 'AWS::Region' }, + ]], + }, Configuration: objectLike({ StackName: 'Same-Stack', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':role/cdk-hnb659fds-cfn-exec-role-', + { Ref: 'AWS::AccountId' }, + '-', + { Ref: 'AWS::Region' }, + ]], + }, }), }), objectLike({ Name: 'Stack.Deploy', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':role/cdk-hnb659fds-deploy-role-', + { Ref: 'AWS::AccountId' }, + '-', + { Ref: 'AWS::Region' }, + ]], + }, Configuration: objectLike({ StackName: 'Same-Stack', }), @@ -80,15 +113,36 @@ test('action has right settings for cross-account deployment', () => { Actions: [ objectLike({ Name: 'Stack.Prepare', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::you:role/cdk-hnb659fds-deploy-role-you-${AWS::Region}' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::you:role/cdk-hnb659fds-deploy-role-you-', + { Ref: 'AWS::Region' }, + ]], + }, Configuration: objectLike({ StackName: 'CrossAccount-Stack', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::you:role/cdk-hnb659fds-cfn-exec-role-you-${AWS::Region}' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::you:role/cdk-hnb659fds-cfn-exec-role-you-', + { Ref: 'AWS::Region' }, + ]], + }, }), }), objectLike({ Name: 'Stack.Deploy', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::you:role/cdk-hnb659fds-deploy-role-you-${AWS::Region}' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::you:role/cdk-hnb659fds-deploy-role-you-', + { Ref: 'AWS::Region' }, + ]], + }, Configuration: objectLike({ StackName: 'CrossAccount-Stack', }), @@ -109,16 +163,46 @@ test('action has right settings for cross-region deployment', () => { Actions: [ objectLike({ Name: 'Stack.Prepare', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-elsewhere' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':role/cdk-hnb659fds-deploy-role-', + { Ref: 'AWS::AccountId' }, + '-elsewhere', + ]], + }, Region: 'elsewhere', Configuration: objectLike({ StackName: 'CrossRegion-Stack', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-elsewhere' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':role/cdk-hnb659fds-cfn-exec-role-', + { Ref: 'AWS::AccountId' }, + '-elsewhere', + ]], + }, }), }), objectLike({ Name: 'Stack.Deploy', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-elsewhere' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::', + { Ref: 'AWS::AccountId' }, + ':role/cdk-hnb659fds-deploy-role-', + { Ref: 'AWS::AccountId' }, + '-elsewhere', + ]], + }, Region: 'elsewhere', Configuration: objectLike({ StackName: 'CrossRegion-Stack', @@ -140,16 +224,34 @@ test('action has right settings for cross-account/cross-region deployment', () = Actions: [ objectLike({ Name: 'Stack.Prepare', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::you:role/cdk-hnb659fds-deploy-role-you-elsewhere' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::you:role/cdk-hnb659fds-deploy-role-you-elsewhere', + ]], + }, Region: 'elsewhere', Configuration: objectLike({ StackName: 'CrossBoth-Stack', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::you:role/cdk-hnb659fds-cfn-exec-role-you-elsewhere' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::you:role/cdk-hnb659fds-cfn-exec-role-you-elsewhere', + ]], + }, }), }), objectLike({ Name: 'Stack.Deploy', - RoleArn: { 'Fn::Sub': 'arn:${AWS::Partition}:iam::you:role/cdk-hnb659fds-deploy-role-you-elsewhere' }, + RoleArn: { + 'Fn::Join': ['', [ + 'arn:', + { Ref: 'AWS::Partition' }, + ':iam::you:role/cdk-hnb659fds-deploy-role-you-elsewhere', + ]], + }, Region: 'elsewhere', Configuration: objectLike({ StackName: 'CrossBoth-Stack',