Skip to content

Commit

Permalink
feat: make imported resources account/region-aware (#8280)
Browse files Browse the repository at this point in the history
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*
  • Loading branch information
skinny85 committed Aug 19, 2020
1 parent e4720bf commit d6278b3
Show file tree
Hide file tree
Showing 40 changed files with 826 additions and 249 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-certificatemanager/test/util.test.ts
Expand Up @@ -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]}');
});

});
15 changes: 14 additions & 1 deletion packages/@aws-cdk/aws-codepipeline-actions/README.md
Expand Up @@ -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).
Expand Down Expand Up @@ -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.
for information on Action structure reference.
@@ -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';

Expand Down Expand Up @@ -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();
},
},
};

Expand Down
24 changes: 17 additions & 7 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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');
}
Expand Down Expand Up @@ -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}')`;
}
}
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts
Expand Up @@ -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);
});

Expand Down
7 changes: 4 additions & 3 deletions 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';
Expand Down Expand Up @@ -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;

/**
Expand Down Expand Up @@ -192,4 +193,4 @@ export class BastionHostLinux extends Construct implements IInstance {
this.connections.allowFrom(p, Port.tcp(22), 'SSH access');
});
}
}
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-ec2/package.json
Expand Up @@ -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",
Expand Down
Expand Up @@ -130,7 +130,7 @@
]
}
},
"Instance255F3526524bcb6d7e0439c4c": {
"Instance255F3526589c13387332ee3de": {
"Type": "AWS::EC2::Instance",
"Properties": {
"AvailabilityZone": "us-east-1a",
Expand Down Expand Up @@ -161,23 +161,23 @@
"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"
},
" --stack ",
{
"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"
},
" --stack ",
{
"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)"
]
]
}
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ec2/test/userdata.test.ts
Expand Up @@ -52,7 +52,7 @@ nodeunitShim({

test.equals(rendered, '<powershell>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' +
Expand Down Expand Up @@ -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');
Expand Down
@@ -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';
Expand All @@ -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();
app.synth();
Expand Up @@ -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';
Expand Down Expand Up @@ -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'),
Expand Down Expand Up @@ -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'),
Expand Down Expand Up @@ -642,4 +637,4 @@ export = {
test.done();
},

};
};
8 changes: 2 additions & 6 deletions 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';
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down
8 changes: 2 additions & 6 deletions 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';
Expand All @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit d6278b3

Please sign in to comment.