Skip to content

Commit 2eef81d

Browse files
fix(core): Peering connection role construct issue (#743)
* Fixing peerign connection role construct issue - Creating only one role per account in default region with all targets in assume policy - To make it work for existing installs didn't change name formation, it will still have source and target in role name but role belongs to a source with all targets defined in configuration. * fixing staticRoutesOnly for vpnconnection * Peering connection routes * Fixing tgw and natgw routes for multiple routes defined in config
1 parent 4b4bd99 commit 2eef81d

File tree

5 files changed

+70
-26
lines changed

5 files changed

+70
-26
lines changed

src/deployments/cdk/src/apps/phase-1.ts

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,15 @@ import * as ssm from '../deployments/ssm/session-manager';
2727
import * as macie from '../deployments/macie';
2828
import * as guardDutyDeployment from '../deployments/guardduty';
2929
import { PhaseInput } from './shared';
30-
import { getIamUserPasswordSecretValue } from '../deployments/iam';
30+
import { createIamRoleOutput, getIamUserPasswordSecretValue } from '../deployments/iam';
3131
import * as cwlCentralLoggingToS3 from '../deployments/central-services/central-logging-s3';
3232
import * as vpcDeployment from '../deployments/vpc';
3333
import * as transitGateway from '../deployments/transit-gateway';
3434
import * as centralEndpoints from '../deployments/central-endpoints';
3535
import { CfnResourceStackCleanupOutput } from '../deployments/cleanup/outputs';
3636
import { VpcOutputFinder, VpcSubnetOutput } from '@aws-accelerator/common-outputs/src/vpc';
3737
import { TransitGatewayAttachmentOutputFinder } from '@aws-accelerator/common-outputs/src/transit-gateway';
38+
import { IamRoleOutputFinder } from '@aws-accelerator/common-outputs/src/iam-role';
3839

3940
export interface IamPolicyArtifactsOutput {
4041
bucketArn: string;
@@ -82,7 +83,7 @@ export async function deploy({ acceleratorConfig, accountStacks, accounts, conte
8283
throw new Error(`Cannot find mandatory primary account ${masterAccountKey}`);
8384
}
8485

85-
const { acceleratorName, installerVersion } = context;
86+
const { acceleratorName, installerVersion, defaultRegion, acceleratorExecutionRoleName } = context;
8687
// Find the central bucket in the outputs
8788
const centralBucket = CentralBucketOutput.getBucket({
8889
accountStacks,
@@ -110,25 +111,29 @@ export async function deploy({ acceleratorConfig, accountStacks, accounts, conte
110111
* @param sourceAccount : Source Account Key, Role will be created in this
111112
* @param accountKey : Target Account Key, Access will be provided to this account
112113
*/
113-
const createIamRoleForPCXAcceptence = (
114-
roleName: string,
115-
sourceAccount: string,
116-
sourceVpcConfig: VpcConfig,
117-
targetAccount: string,
118-
) => {
119-
const accountStack = accountStacks.tryGetOrCreateAccountStack(sourceAccount, sourceVpcConfig.region);
114+
const createIamRoleForPCXAcceptence = (roleName: string, sourceAccount: string) => {
115+
const accountStack = accountStacks.tryGetOrCreateAccountStack(sourceAccount, defaultRegion);
120116
if (!accountStack) {
121117
console.warn(`Cannot find account stack ${sourceAccount}`);
122118
return;
123119
}
124-
const existing = accountStack.node.tryFindChild(roleName);
120+
const existing = accountStack.node.tryFindChild('PeeringRole');
125121
if (existing) {
126122
return;
127123
}
124+
const targetAccounts = acceleratorConfig
125+
.getVpcConfigs()
126+
.filter(rsv => PeeringConnectionConfig.is(rsv.vpcConfig.pcx) && rsv.vpcConfig.pcx.source === sourceAccount);
127+
const targetAccountKeys = Array.from(new Set(targetAccounts.map(rsv => rsv.accountKey)));
128128
const peeringRole = new iam.Role(accountStack, 'PeeringRole', {
129129
roleName,
130-
assumedBy: new iam.ArnPrincipal(
131-
`arn:aws:iam::${getAccountId(accounts, targetAccount)}:role/${context.acceleratorExecutionRoleName}`,
130+
assumedBy: new iam.CompositePrincipal(
131+
...targetAccountKeys.map(
132+
targetAccountKey =>
133+
new iam.ArnPrincipal(
134+
`arn:aws:iam::${getAccountId(accounts, targetAccountKey)}:role/${acceleratorExecutionRoleName}`,
135+
),
136+
),
132137
),
133138
});
134139

@@ -138,6 +143,8 @@ export async function deploy({ acceleratorConfig, accountStacks, accounts, conte
138143
actions: ['ec2:AcceptVpcPeeringConnection'],
139144
}),
140145
);
146+
147+
createIamRoleOutput(accountStack, peeringRole, 'PeeringConnectionAcceptRole');
141148
};
142149

143150
// Auxiliary method to create a VPC in the account with given account key
@@ -246,9 +253,16 @@ export async function deploy({ acceleratorConfig, accountStacks, accounts, conte
246253
console.warn(`Cannot find PCX source VPC ${pcxConfig['source-vpc']} in account ${pcxConfig.source}`);
247254
} else {
248255
// Create Accepter Role for Peering Connection **WITHOUT** random suffix
249-
// TODO Region support
250-
const roleName = createRoleName(`VPC-PCX-${pascalCase(accountKey)}To${pascalCase(pcxConfig.source)}`, 0);
251-
createIamRoleForPCXAcceptence(roleName, pcxConfig.source, sourceVpcConfig.vpcConfig, accountKey);
256+
const pcxAcceptRole = IamRoleOutputFinder.tryFindOneByName({
257+
outputs,
258+
accountKey: pcxConfig.source,
259+
roleKey: 'PeeringConnectionAcceptRole',
260+
});
261+
let roleName = createRoleName(`VPC-PCX-${pascalCase(accountKey)}To${pascalCase(pcxConfig.source)}`, 0);
262+
if (pcxAcceptRole) {
263+
roleName = pcxAcceptRole.roleName;
264+
}
265+
createIamRoleForPCXAcceptence(roleName, pcxConfig.source);
252266
}
253267
}
254268

src/deployments/cdk/src/apps/phase-2.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import * as snsDeployment from '../deployments/sns';
2828
import * as ssmDeployment from '../deployments/ssm';
2929
import { getStackJsonOutput } from '@aws-accelerator/common-outputs/src/stack-output';
3030
import { logArchiveReadOnlyAccess } from '../deployments/s3/log-archive-read-access';
31+
import { IamRoleOutputFinder } from '@aws-accelerator/common-outputs/src/iam-role';
3132

3233
/**
3334
* This is the main entry point to deploy phase 2
@@ -86,10 +87,18 @@ export async function deploy({ acceleratorConfig, accountStacks, accounts, conte
8687
continue;
8788
}
8889

89-
// TODO store role name in outputs
90-
// Get the exact same role name as in phase 1
91-
const roleName = createRoleName(`VPC-PCX-${pascalCase(accountKey)}To${pascalCase(pcxConfig.source)}`, 0);
92-
const peerRoleArn = `arn:aws:iam::${getAccountId(accounts, pcxConfig.source)}:role/${roleName}`;
90+
const pcxAcceptRole = IamRoleOutputFinder.tryFindOneByName({
91+
outputs,
92+
accountKey: pcxConfig.source,
93+
roleKey: 'PeeringConnectionAcceptRole',
94+
});
95+
let peerRoleArn = '';
96+
if (pcxAcceptRole) {
97+
peerRoleArn = pcxAcceptRole.roleArn;
98+
} else {
99+
const roleName = createRoleName(`VPC-PCX-${pascalCase(accountKey)}To${pascalCase(pcxConfig.source)}`, 0);
100+
peerRoleArn = `arn:aws:iam::${getAccountId(accounts, pcxConfig.source)}:role/${roleName}`;
101+
}
93102

94103
// Get Peer VPC Configuration
95104
const pcxSourceVpc = pcxConfig['source-vpc'];

src/deployments/cdk/src/common/peering-connection.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,13 @@ export namespace PeeringConnection {
7373
});
7474
// Find the PCX output that contains the PCX route's VPC
7575
const peerVpcOutput = peerVpcOutputs.find(output => {
76-
const pcxVpc = output.vpcs.find(vpc => vpc.accountKey === pcxRoute.account && vpc.vpcName === pcxRoute.vpc);
77-
return !!pcxVpc;
76+
const sourcePcxVpc = output.vpcs.find(
77+
vpc => vpc.accountKey === pcxRoute.account && vpc.vpcName === pcxRoute.vpc,
78+
);
79+
const targetPcxVpc = output.vpcs.find(
80+
vpc => vpc.accountKey === accountKey && vpc.vpcName === vpcConfig?.name!,
81+
);
82+
return !!sourcePcxVpc && !!targetPcxVpc;
7883
});
7984
const pcxId = peerVpcOutput?.pcxId;
8085
if (!pcxId) {

src/deployments/cdk/src/common/vpc.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -526,20 +526,36 @@ export class Vpc extends cdk.Construct implements constructs.Vpc {
526526
dynamoRoutes.push(routeTableObj);
527527
continue;
528528
} else if (route.target === 'TGW' && tgw && tgwAttachment) {
529-
const tgwRoute = new ec2.CfnRoute(this, `${routeTableName}_${route.target}`, {
529+
let constructName = `${routeTableName}_${route.target}`;
530+
if (typeof route.destination !== 'string') {
531+
console.warn(`Route for TGW only supports cidr as destination`);
532+
continue;
533+
}
534+
if (route.destination !== '0.0.0.0/0') {
535+
constructName = `${routeTableName}_${route.target}_${route.destination}`;
536+
}
537+
const tgwRoute = new ec2.CfnRoute(this, constructName, {
530538
routeTableId: routeTableObj,
531-
destinationCidrBlock: route.destination as string,
539+
destinationCidrBlock: route.destination,
532540
transitGatewayId: tgw.tgwId,
533541
});
534542
tgwRoute.addDependsOn(tgwAttachment.resource);
535543
continue;
536544
} else if (route.target.startsWith('NATGW_')) {
545+
if (typeof route.destination !== 'string') {
546+
console.warn(`Route for NATGW only supports cidr as destination`);
547+
continue;
548+
}
549+
let constructName = `${routeTableName}_natgw_route`;
550+
if (route.destination !== '0.0.0.0/0') {
551+
constructName = `${routeTableName}_natgw_${route.destination}_route`;
552+
}
537553
const routeParams: ec2.CfnRouteProps = {
538554
routeTableId: routeTableObj,
539-
destinationCidrBlock: typeof route.destination === 'string' ? route.destination : '0.0.0.0/0',
555+
destinationCidrBlock: route.destination,
540556
natGatewayId: this.natgwNameToIdMap[route.target.toLowerCase()],
541557
};
542-
new ec2.CfnRoute(this, `${routeTableName}_natgw_route`, routeParams);
558+
new ec2.CfnRoute(this, constructName, routeParams);
543559
continue;
544560
} else {
545561
// Need to add for different Routes

src/deployments/cdk/src/deployments/firewall/cluster/step-2.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ async function createCustomerGateways(props: {
135135
type: 'ipsec.1',
136136
transitGatewayId: transitGateway.tgwId,
137137
customerGatewayId: customerGateway.ref,
138-
staticRoutesOnly: firewallCgwRouting === 'static' ? true : false,
138+
staticRoutesOnly: firewallCgwRouting === 'static' ? true : undefined,
139139
});
140140

141141
const options = new VpnTunnelOptions(scope, `VpnTunnelOptions${index}`, {

0 commit comments

Comments
 (0)