Skip to content

Commit

Permalink
Introduce IEnvComponent, and use it in IAM.
Browse files Browse the repository at this point in the history
  • Loading branch information
skinny85 committed Jul 1, 2020
1 parent a57f7b2 commit 226111d
Show file tree
Hide file tree
Showing 16 changed files with 189 additions and 114 deletions.
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-certificatemanager/test/test.util.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { PublicHostedZone } from '@aws-cdk/aws-route53';
import { App, Stack } from '@aws-cdk/core';
import { App, Stack, Token } from '@aws-cdk/core';
import { Test } from 'nodeunit';
import { Certificate, DnsValidatedCertificate } from '../lib';
import { apexDomain, getCertificateRegion, isDnsValidatedCertificate } from '../lib/util';
Expand Down Expand Up @@ -101,7 +101,7 @@ export = {
domainName: 'www.example.com',
});

test.equals(getCertificateRegion(certificate), '${Token[AWS.Region.4]}');
test.ok(Token.isUnresolved(getCertificateRegion(certificate)));
test.done();
},
},
Expand Down
19 changes: 10 additions & 9 deletions packages/@aws-cdk/aws-codepipeline/lib/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import * as iam from '@aws-cdk/aws-iam';
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,
App, BootstraplessSynthesizer, Construct, DefaultStackSynthesizer, IStackSynthesizer,
Lazy, PhysicalName, RemovalPolicy, Resource, Stack, Token,
} from '@aws-cdk/core';
import { ActionCategory, IAction, IPipeline, IStage } from './action';
import { CfnPipeline } from './codepipeline.generated';
Expand Down Expand Up @@ -400,13 +400,13 @@ export class Pipeline extends PipelineBase {

const actionResource = action.actionProperties.resource;
if (actionResource) {
const actionResourceStack = Stack.of(actionResource);
if (this.region !== actionResource.region) {
actionRegion = actionResource.region;
if (this.region.compare(actionResource.region).unEqual()) {
actionRegion = actionResource.region.toString();
const actionResourceStack = Stack.of(actionResource);
// 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 &&
actionResource.region === actionResourceStack.region) {
actionResource.region.compareToString(actionResourceStack.region).equalOrBothUnresolved()) {
otherStack = actionResourceStack;
}
}
Expand Down Expand Up @@ -851,10 +851,11 @@ export class Pipeline extends PipelineBase {
}

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

private requireApp(): App {
Expand Down Expand Up @@ -932,4 +933,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}')`;
}
}
}
Original file line number Diff line number Diff line change
@@ -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,15 +21,10 @@ 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,
account: stack.account,
region: stack.region,
},
}),
});

app.synth();
Original file line number Diff line number Diff line change
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,15 +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,
account: stack.account,
region: stack.region,
},
}),
taskImageOptions: {
containerPort: 2015,
image: ecs.ContainerImage.fromRegistry('abiosoft/caddy'),
Expand Down Expand Up @@ -410,15 +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,
account: stack.account,
region: stack.region,
},
}),
taskImageOptions: {
containerPort: 2015,
image: ecs.ContainerImage.fromRegistry('abiosoft/caddy'),
Expand Down
23 changes: 0 additions & 23 deletions packages/@aws-cdk/aws-iam/lib/account-utils.ts

This file was deleted.

16 changes: 11 additions & 5 deletions packages/@aws-cdk/aws-iam/lib/grant.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
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 @@ -122,10 +121,17 @@ export class Grant implements cdk.IDependable {
scope: options.resource,
});

const definitelySameAccount = accountsAreSameOrUnresolved(
options.grantee.grantPrincipal.principalAccount,
options.resource.account) === AccountCompare.SAME;
if (result.success && definitelySameAccount) {
const compareResult = options.grantee.grantPrincipal.principalAccount
? options.resource.account.compareToString(options.grantee.grantPrincipal.principalAccount)
: undefined;
// if both accounts are tokens, we assume here they are the same
const sameAccount: boolean = compareResult?.equalOrBothUnresolved() ??
// if the principal doesn't have an account (for example, a service principal),
// we assume the accounts are the same
true;
// 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;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +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 principalAccount: string | undefined = this.account.toString();
public readonly assumeRoleAction: string = 'sts:AssumeRole';

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

private role?: Role;
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-iam/lib/private/immutable-role.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export class ImmutableRole extends Resource implements IRole {

constructor(scope: Construct, id: string, private readonly role: IRole) {
super(scope, id, {
account: role.account,
region: role.region,
account: role.account.toString(),
region: role.region.toString(),
});

// implement IDependable privately
Expand Down
19 changes: 6 additions & 13 deletions packages/@aws-cdk/aws-iam/lib/role.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Construct, Duration, Lazy, Resource, Stack, Token } from '@aws-cdk/core';
import { Construct, Duration, Lazy, Resource, Stack } from '@aws-cdk/core';
import { Grant } from './grant';
import { CfnRole } from './iam.generated';
import { IIdentity } from './identity-base';
Expand Down Expand Up @@ -213,9 +213,7 @@ export class Role extends Resource implements IRole {
}

public attachInlinePolicy(policy: Policy): void {
const policyAccount = Stack.of(policy).account;

if (accountsAreEqualOrOneIsUnresolved(policyAccount, roleAccount)) {
if (this.account.compare(policy.account).equalOrAnyUnresolved()) {
this.attachedPolicies.attach(policy);
policy.attachToRole(this);
}
Expand Down Expand Up @@ -246,20 +244,15 @@ export class Role extends Resource implements IRole {
}

const importedRole = new Import(scope, id);
return options.mutable !== false && accountsAreEqualOrOneIsUnresolved(scopeStack.account, importedRole.account)
return options.mutable !== false &&
// we only return an immutable Role if both accounts were explicitly provided, and different
importedRole.account.compareToString(scopeStack.account).equalOrAnyUnresolved()
? importedRole
: new ImmutableRole(scope, `ImmutableRole${id}`, importedRole);

function accountsAreEqualOrOneIsUnresolved(
account1: string | undefined,
account2: string | undefined): boolean {
return Token.isUnresolved(account1) || Token.isUnresolved(account2) ||
account1 === account2;
}
}

public readonly grantPrincipal: IPrincipal = this;
public readonly principalAccount: string | undefined = this.account;
public readonly principalAccount: string | undefined = this.account.toString();

public readonly assumeRoleAction: string = 'sts:AssumeRole';

Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/lib/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ export class User extends Resource implements IIdentity, IUser {
}

public readonly grantPrincipal: IPrincipal = this;
public readonly principalAccount: string | undefined = this.account;
public readonly principalAccount: string | undefined = this.account.toString();
public readonly assumeRoleAction: string = 'sts:AssumeRole';

/**
Expand Down
37 changes: 9 additions & 28 deletions packages/@aws-cdk/aws-route53/test/test.util.ts
Original file line number Diff line number Diff line change
@@ -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 = {
Expand All @@ -20,15 +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,
account: stack.account,
region: stack.region,
});
}));

// THEN
test.equal(qualified, 'test.domain.com.');
Expand All @@ -41,15 +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,
account: stack.account,
region: stack.region,
});
}));

// THEN
test.equal(qualified, 'test.domain.com.');
Expand All @@ -62,15 +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,
account: stack.account,
region: stack.region,
});
}));

// THEN
test.equal(qualified, 'test.domain.com.');
Expand All @@ -83,15 +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,
account: stack.account,
region: stack.region,
});
}));

// THEN
test.equal(qualified, 'test.domain.com.');
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-s3/lib/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -995,8 +995,8 @@ export class Bucket extends BucketBase {
public readonly bucketDualStackDomainName = attrs.bucketDualStackDomainName || `${bucketName}.s3.dualstack.${region}.${urlSuffix}`;
public readonly bucketWebsiteNewUrlFormat = newUrlFormat;
public readonly encryptionKey = attrs.encryptionKey;
public readonly account = attrs.account ?? stack.account;
public readonly region = attrs.region ?? stack.region;
public readonly account = Resource.makeEnvComponent(attrs.account ?? stack.account);
public readonly region = Resource.makeEnvComponent(attrs.region ?? stack.region);
public policy?: BucketPolicy = undefined;
protected autoCreatePolicy = false;
protected disallowPublicAccess = false;
Expand Down
Loading

0 comments on commit 226111d

Please sign in to comment.