Skip to content

Commit 5ba35e4

Browse files
authored
fix(route53): cross-account delegation broken in opt-in regions (#23082)
This is a revert of #22370. The fix proposed there solved one very specific (but rare) use case, in exchange for breaking common use cases. - *It fixed the case of*: AWS service authors using their own global service principal in the delegation role, with both source and target account opted into the region. - *It broke the case of*: all teams that didn't have both accounts opted into the region. The second case is much more common, so revert to the old behavior. Since the regional behavior might still be useful to *some* people somewhere, it has been relegated to a context key, `@aws-cdk/aws-route53:useRegionalStsEndpoint`, instead. It can be configured, but is not advertised as 99.9% of users will not need this behavior. Since both STS and Route53 are global and regular customers cannot usefully use Service Principals in this particular trust policy anyway, there is no impact to regular customers. Fixes #23081. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent aa9c707 commit 5ba35e4

File tree

4 files changed

+66
-4
lines changed

4 files changed

+66
-4
lines changed

Diff for: packages/@aws-cdk/aws-route53/lib/cross-account-zone-delegation-handler/index.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ interface ResourceProperties {
88
DelegatedZoneName: string,
99
DelegatedZoneNameServers: string[],
1010
TTL: number,
11+
UseRegionalStsEndpoint?: string,
1112
}
1213

1314
export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {
@@ -23,13 +24,13 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
2324
}
2425

2526
async function cfnEventHandler(props: ResourceProperties, isDeleteEvent: boolean) {
26-
const { AssumeRoleArn, ParentZoneId, ParentZoneName, DelegatedZoneName, DelegatedZoneNameServers, TTL } = props;
27+
const { AssumeRoleArn, ParentZoneId, ParentZoneName, DelegatedZoneName, DelegatedZoneNameServers, TTL, UseRegionalStsEndpoint } = props;
2728

2829
if (!ParentZoneId && !ParentZoneName) {
2930
throw Error('One of ParentZoneId or ParentZoneName must be specified');
3031
}
3132

32-
const credentials = await getCrossAccountCredentials(AssumeRoleArn);
33+
const credentials = await getCrossAccountCredentials(AssumeRoleArn, !!UseRegionalStsEndpoint);
3334
const route53 = new Route53({ credentials });
3435

3536
const parentZoneId = ParentZoneId ?? await getHostedZoneIdByName(ParentZoneName!, route53);
@@ -50,8 +51,8 @@ async function cfnEventHandler(props: ResourceProperties, isDeleteEvent: boolean
5051
}).promise();
5152
}
5253

53-
async function getCrossAccountCredentials(roleArn: string): Promise<Credentials> {
54-
const sts = new STS({ stsRegionalEndpoints: 'regional' });
54+
async function getCrossAccountCredentials(roleArn: string, regionalEndpoint: boolean): Promise<Credentials> {
55+
const sts = new STS(regionalEndpoint ? { stsRegionalEndpoints: 'regional' } : {});
5556
const timestamp = (new Date()).getTime();
5657

5758
const { Credentials: assumedCredentials } = await sts

Diff for: packages/@aws-cdk/aws-route53/lib/hosted-zone.ts

+12
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,18 @@ export interface PublicHostedZoneProps extends CommonHostedZoneProps {
200200
/**
201201
* A principal which is trusted to assume a role for zone delegation
202202
*
203+
* If supplied, this will create a Role in the same account as the Hosted
204+
* Zone, which can be assumed by the `CrossAccountZoneDelegationRecord` to
205+
* create a delegation record to a zone in a different account.
206+
*
207+
* Be sure to indicate the account(s) that you trust to create delegation
208+
* records, using either `iam.AccountPrincipal` or `iam.OrganizationPrincipal`.
209+
*
210+
* If you are planning to use `iam.ServicePrincipal`s here, be sure to include
211+
* region-specific service principals for every opt-in region you are going to
212+
* be delegating to; or don't use this feature and create separate roles
213+
* with appropriate permissions for every opt-in region instead.
214+
*
203215
* @default - No delegation configuration
204216
*/
205217
readonly crossAccountZoneDelegationPrincipal?: iam.IPrincipal;

Diff for: packages/@aws-cdk/aws-route53/lib/record-set.ts

+23
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,26 @@ import { determineFullyQualifiedDomainName } from './util';
1010
const CROSS_ACCOUNT_ZONE_DELEGATION_RESOURCE_TYPE = 'Custom::CrossAccountZoneDelegation';
1111
const DELETE_EXISTING_RECORD_SET_RESOURCE_TYPE = 'Custom::DeleteExistingRecordSet';
1212

13+
/**
14+
* Context key to control whether to use the regional STS endpoint, instead of the global one
15+
*
16+
* There is only exactly one use case where you want to turn this on. If:
17+
*
18+
* - you are building an AWS service; AND
19+
* - would like to your own Global Service Principal in the trust policy of the delegation role; AND
20+
* - the target account is opted in in the same region as well
21+
*
22+
* Then you can turn this on. For all other use cases, the global endpoint is preferable:
23+
*
24+
* - if you are a regular customer, your trust policy would be in terms of account ids or
25+
* organization ids, or ARNs, not Service Principals, so you don't care about this behavior.
26+
* - if the target account is not opted in as well, the AssumeRole call would fail
27+
*
28+
* Because this configuration option is so rare, turn it into a context setting instead
29+
* of a publicly available prop.
30+
*/
31+
const USE_REGIONAL_STS_ENDPOINT_CONTEXT_KEY = '@aws-cdk/aws-route53:useRegionalStsEndpoint';
32+
1333
/**
1434
* A record set
1535
*/
@@ -749,6 +769,8 @@ export class CrossAccountZoneDelegationRecord extends Construct {
749769
resources: [props.delegationRole.roleArn],
750770
}));
751771

772+
const useRegionalStsEndpoint = this.node.tryGetContext(USE_REGIONAL_STS_ENDPOINT_CONTEXT_KEY);
773+
752774
const customResource = new CustomResource(this, 'CrossAccountZoneDelegationCustomResource', {
753775
resourceType: CROSS_ACCOUNT_ZONE_DELEGATION_RESOURCE_TYPE,
754776
serviceToken: provider.serviceToken,
@@ -760,6 +782,7 @@ export class CrossAccountZoneDelegationRecord extends Construct {
760782
DelegatedZoneName: props.delegatedZone.zoneName,
761783
DelegatedZoneNameServers: props.delegatedZone.hostedZoneNameServers!,
762784
TTL: (props.ttl || Duration.days(2)).toSeconds(),
785+
UseRegionalStsEndpoint: useRegionalStsEndpoint ? 'true' : undefined,
763786
},
764787
});
765788

Diff for: packages/@aws-cdk/aws-route53/test/record-set.test.ts

+26
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,32 @@ describe('record set', () => {
845845
}
846846
});
847847

848+
test('Cross account zone context flag', () => {
849+
// GIVEN
850+
const stack = new Stack();
851+
stack.node.setContext('@aws-cdk/aws-route53:useRegionalStsEndpoint', true);
852+
const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
853+
zoneName: 'myzone.com',
854+
crossAccountZoneDelegationPrincipal: new iam.AccountPrincipal('123456789012'),
855+
});
856+
857+
// WHEN
858+
const childZone = new route53.PublicHostedZone(stack, 'ChildHostedZone', {
859+
zoneName: 'sub.myzone.com',
860+
});
861+
new route53.CrossAccountZoneDelegationRecord(stack, 'Delegation', {
862+
delegatedZone: childZone,
863+
parentHostedZoneName: 'myzone.com',
864+
delegationRole: parentZone.crossAccountZoneDelegationRole!,
865+
ttl: Duration.seconds(60),
866+
});
867+
868+
// THEN
869+
Template.fromStack(stack).hasResourceProperties('Custom::CrossAccountZoneDelegation', {
870+
UseRegionalStsEndpoint: 'true',
871+
});
872+
});
873+
848874
test('Delete existing record', () => {
849875
// GIVEN
850876
const stack = new Stack();

0 commit comments

Comments
 (0)