Skip to content

Commit

Permalink
Add account and region to (I)Resource.
Browse files Browse the repository at this point in the history
  • Loading branch information
skinny85 committed Jul 1, 2020
1 parent dfac0df commit a57f7b2
Show file tree
Hide file tree
Showing 31 changed files with 485 additions and 119 deletions.
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-certificatemanager/test/test.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export = {
domainName: 'www.example.com',
});

test.equals(getCertificateRegion(certificate), '${Token[AWS::Region.4]}');
test.equals(getCertificateRegion(certificate), '${Token[AWS.Region.4]}');
test.done();
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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 @@ -152,6 +152,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
12 changes: 6 additions & 6 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,11 +401,12 @@ 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;
if (this.region !== actionResource.region) {
actionRegion = actionResource.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) {
if (pipelineStack.account === actionResourceStack.account &&
actionResource.region === actionResourceStack.region) {
otherStack = actionResourceStack;
}
}
Expand Down Expand Up @@ -850,11 +851,10 @@ export class Pipeline extends PipelineBase {
}

private requireRegion(): string {
const region = Stack.of(this).region;
if (Token.isUnresolved(region)) {
if (Token.isUnresolved(this.region)) {
throw new Error('Pipeline stack which uses cross-environment actions must have an explicitly set region');
}
return region;
return this.region;
}

private requireApp(): App {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-dynamodb/test/dynamodb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2103,7 +2103,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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,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
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-ec2/test/userdata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ nodeunitShim({

test.equals(rendered, '<powershell>trap {\n' +
'$success=($PSItem.Exception.Message -eq "Success")\n' +
'cfn-signal --stack Stack --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} --success ($success.ToString().ToLower())\n' +
'cfn-signal --stack Stack --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 Stack --resource RESOURCE1989552F --region ${Token[AWS::Region.4]} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n' +
'/opt/aws/bin/cfn-signal --stack Stack --resource RESOURCE1989552F --region ${Token[AWS.Region.4]} -e $exitCode || echo \'Failed to send Cloudformation Signal\'\n' +
'}\n' +
'trap exitTrap EXIT\n' +
'command1');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ new ApplicationLoadBalancedFargateService(stack, 'myService', {
hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId',
stack,
node: stack.node,
account: stack.account,
region: stack.region,
},
});

app.synth();
app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,8 @@ export = {
hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId',
stack,
node: stack.node,
account: stack.account,
region: stack.region,
},
taskImageOptions: {
containerPort: 2015,
Expand Down Expand Up @@ -414,6 +416,8 @@ export = {
hostedZoneArn: 'arn:aws:route53:::hostedzone/fakeId',
stack,
node: stack.node,
account: stack.account,
region: stack.region,
},
taskImageOptions: {
containerPort: 2015,
Expand Down Expand Up @@ -641,4 +645,4 @@ export = {
test.done();
},

};
};
8 changes: 2 additions & 6 deletions packages/@aws-cdk/aws-eks-legacy/lib/kubectl-layer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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';
import * as crypto from 'crypto';

const KUBECTL_APP_ARN = 'arn:aws:serverlessrepo:us-east-1:903779448426:applications/lambda-layer-kubectl';
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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';
import * as crypto from 'crypto';

const KUBECTL_APP_ARN = 'arn:aws:serverlessrepo:us-east-1:903779448426: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
23 changes: 23 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/account-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as cdk from '@aws-cdk/core';

export enum AccountCompare {
SAME,
DIFFERENT,
UNKNOWN,
}

export function accountsAreSameOrUnresolved(account1: string | undefined, account2: string | undefined): AccountCompare {
const firstIsUnresolved = cdk.Token.isUnresolved(account1);
const secondIsUnresolved = cdk.Token.isUnresolved(account2);

if (firstIsUnresolved && secondIsUnresolved) {
return AccountCompare.SAME;
}
if (firstIsUnresolved || secondIsUnresolved) {
return AccountCompare.UNKNOWN;
}

return account1 === account2
? AccountCompare.SAME
: AccountCompare.DIFFERENT;
}
12 changes: 9 additions & 3 deletions packages/@aws-cdk/aws-iam/lib/grant.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as cdk from '@aws-cdk/core';
import { AccountCompare, accountsAreSameOrUnresolved } from './account-utils';
import { PolicyStatement } from './policy-statement';
import { IGrantable, IPrincipal } from './principals';

Expand Down Expand Up @@ -121,7 +122,12 @@ export class Grant implements cdk.IDependable {
scope: options.resource,
});

if (result.success) { return result; }
const definitelySameAccount = accountsAreSameOrUnresolved(
options.grantee.grantPrincipal.principalAccount,
options.resource.account) === AccountCompare.SAME;
if (result.success && definitelySameAccount) {
return result;
}

const statement = new PolicyStatement({
actions: options.actions,
Expand Down Expand Up @@ -292,7 +298,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
*/
Expand Down Expand Up @@ -332,4 +338,4 @@ export class CompositeDependable implements cdk.IDependable {
},
});
}
}
}
5 changes: 4 additions & 1 deletion packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.account;
public readonly assumeRoleAction: string = 'sts:AssumeRole';

private readonly attachedPolicies = new AttachedPolicies();
Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-iam/lib/lazy-role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,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.account;
public readonly assumeRoleAction: string = 'sts:AssumeRole';

private role?: Role;
Expand Down
7 changes: 3 additions & 4 deletions packages/@aws-cdk/aws-iam/lib/oidc-provider.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
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';
import * as path from 'path';

const RESOURCE_TYPE = 'Custom::AWSCDKOpenIdConnectProvider';
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'),
Expand Down
10 changes: 10 additions & 0 deletions packages/@aws-cdk/aws-iam/lib/principals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit a57f7b2

Please sign in to comment.