Skip to content

Commit

Permalink
fix: route53 CrossAccountZoneDelegationRecord fails at deployment tim…
Browse files Browse the repository at this point in the history
…e with imported `delegatedZone` (#30440)

### Issue # (if applicable)

Closes #28581.

### Reason for this change

An imported `delegatedZone` will not have info about the Name Servers. When it is passed to `CrossAccountZoneDelegationRecord`, the handler will see `undefined` when trying to retrieve the Name Servers info on `delegatedZone`, then throw exception during deployment.

This change throws the exception at build time for a faster feedback loop.

### Description of changes

`CrossAccountZoneDelegationRecord` throws exception if `delegatedZone.hostedZoneNameServers` is undefined.

### Description of how you validated changes

Add unit test to cover the case of passing an imported HostedZone to `CrossAccountZoneDelegationRecord`

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
samson-keung authored Jun 4, 2024
1 parent ddbbd00 commit a3d9b10
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-route53/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ const delegationRole = iam.Role.fromRoleArn(this, 'DelegationRole', delegationRo

// create the record
new route53.CrossAccountZoneDelegationRecord(this, 'delegate', {
delegatedZone: subZone,
delegatedZone: subZone, // Note that an imported HostedZone is not supported as Name Servers info will not be available
parentHostedZoneName: 'someexample.com', // or you can use parentHostedZoneId
delegationRole,
});
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/aws-route53/lib/record-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,10 @@ export class CrossAccountZoneDelegationRecord extends Construct {
throw Error('Only one of parentHostedZoneName and parentHostedZoneId is supported');
}

if (!props.delegatedZone.hostedZoneNameServers) {
throw Error(`Not able to retrieve Name Servers for ${props.delegatedZone.zoneName} due to it being imported.`);
}

const provider = CrossAccountZoneDelegationProvider.getOrCreateProvider(this, CROSS_ACCOUNT_ZONE_DELEGATION_RESOURCE_TYPE);

const role = iam.Role.fromRoleArn(this, 'cross-account-zone-delegation-handler-role', provider.roleArn);
Expand Down
21 changes: 21 additions & 0 deletions packages/aws-cdk-lib/aws-route53/test/record-set.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,27 @@ describe('record set', () => {
});
});

test('CrossAccountZoneDelegationRecord should throw if delegatedZone is imported', () => {
// GIVEN
const stack = new Stack();
const parentZone = new route53.PublicHostedZone(stack, 'ParentHostedZone', {
zoneName: 'myzone.com',
});

// WHEN
const childZone = route53.PublicHostedZone.fromPublicHostedZoneAttributes(stack, 'ChildHostedZone', {
hostedZoneId: 'fake-id',
zoneName: 'fake-name',
});

//THEN
expect(() => new route53.CrossAccountZoneDelegationRecord(stack, 'Delegation', {
delegatedZone: childZone,
parentHostedZoneId: parentZone.hostedZoneId,
delegationRole: parentZone.crossAccountZoneDelegationRole!,
})).toThrow(/Not able to retrieve Name Servers for fake-name due to it being imported./);
});

testDeprecated('Cross account zone delegation record with parentHostedZoneName', () => {
// GIVEN
const stack = new Stack();
Expand Down

0 comments on commit a3d9b10

Please sign in to comment.