fix(route53): delete old NS records on cross-account delegation renam…

…ing (aws#21249)
garciparedes · Apr 25, 2023 · 03abc80 · 03abc80
1 parent 8b248d9
commit 03abc80
Show file tree

Hide file tree

Showing 2 changed files with 112 additions and 10 deletions.
diff --git a/packages/aws-cdk-lib/aws-route53/lib/cross-account-zone-delegation-handler/index.ts b/packages/aws-cdk-lib/aws-route53/lib/cross-account-zone-delegation-handler/index.ts
@@ -1,6 +1,11 @@
 // eslint-disable-next-line import/no-extraneous-dependencies
 import { Credentials, Route53, STS } from 'aws-sdk';
 
+export type CrossAccountZoneDelegationEvent = AWSLambda.CloudFormationCustomResourceEvent & {
+  ResourceProperties: ResourceProperties;
+  OldResourceProperties?: ResourceProperties;
+}
+
 interface ResourceProperties {
   AssumeRoleArn: string,
   ParentZoneName?: string,
@@ -11,18 +16,25 @@ interface ResourceProperties {
   UseRegionalStsEndpoint?: string,
 }
 
-export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent) {
-  const resourceProps = event.ResourceProperties as unknown as ResourceProperties;
-
+export async function handler(event: CrossAccountZoneDelegationEvent) {
+  const resourceProps = event.ResourceProperties;
   switch (event.RequestType) {
     case 'Create':
-    case 'Update':
       return cfnEventHandler(resourceProps, false);
+    case 'Update':
+      return cfnUpdateEventHandler(resourceProps, event.OldResourceProperties);
     case 'Delete':
       return cfnEventHandler(resourceProps, true);
   }
 }
 
+async function cfnUpdateEventHandler(props: ResourceProperties, oldProps: ResourceProperties | undefined) {
+  if (oldProps && props.DelegatedZoneName !== oldProps.DelegatedZoneName) {
+    await cfnEventHandler(oldProps, true);
+  }
+  await cfnEventHandler(props, false);
+}
+
 async function cfnEventHandler(props: ResourceProperties, isDeleteEvent: boolean) {
   const { AssumeRoleArn, ParentZoneId, ParentZoneName, DelegatedZoneName, DelegatedZoneNameServers, TTL, UseRegionalStsEndpoint } = props;
 

diff --git a/packages/aws-cdk-lib/aws-route53/test/cross-account-zone-delegation-handler/index.test.ts b/packages/aws-cdk-lib/aws-route53/test/cross-account-zone-delegation-handler/index.test.ts
@@ -1,4 +1,4 @@
-import { handler } from '../../lib/cross-account-zone-delegation-handler';
+import { handler, CrossAccountZoneDelegationEvent } from '../../lib/cross-account-zone-delegation-handler';
 
 const mockStsClient = {
   assumeRole: jest.fn().mockReturnThis(),
@@ -83,6 +83,96 @@ test('calls create resource record set with Upsert for Create event', async () =
   });
 });
 
+test('calls update resource record set with Upsert for Update event', async () => {
+  // GIVEN
+  mockStsClient.promise.mockResolvedValueOnce({ Credentials: { AccessKeyId: 'K', SecretAccessKey: 'S', SessionToken: 'T' } });
+  mockRoute53Client.promise.mockResolvedValueOnce({});
+
+  // WHEN
+  const event= getCfnEvent({
+    RequestType: 'Update',
+    OldResourceProperties: {
+      ServiceToken: 'Foo',
+      AssumeRoleArn: 'roleArn',
+      ParentZoneId: '1',
+      DelegatedZoneName: 'recordName',
+      DelegatedZoneNameServers: ['three', 'four'],
+      TTL: 172800,
+    },
+  });
+  await invokeHandler(event);
+
+  // THEN
+  expect(mockRoute53Client.changeResourceRecordSets).toHaveBeenCalledTimes(1);
+  expect(mockRoute53Client.changeResourceRecordSets).toHaveBeenCalledWith({
+    HostedZoneId: '1',
+    ChangeBatch: {
+      Changes: [{
+        Action: 'UPSERT',
+        ResourceRecordSet: {
+          Name: 'recordName',
+          Type: 'NS',
+          TTL: 172800,
+          ResourceRecords: [{ Value: 'one' }, { Value: 'two' }],
+        },
+      }],
+    },
+  });
+});
+
+
+test('calls update resource record set with Upsert and Delete for Update event', async () => {
+  // GIVEN
+  mockStsClient.promise.mockResolvedValue({ Credentials: { AccessKeyId: 'K', SecretAccessKey: 'S', SessionToken: 'T' } });
+  mockRoute53Client.promise.mockResolvedValue({});
+
+  // WHEN
+  const event= getCfnEvent({
+    RequestType: 'Update',
+    OldResourceProperties: {
+      ServiceToken: 'Foo',
+      AssumeRoleArn: 'roleArn',
+      ParentZoneId: '1',
+      DelegatedZoneName: 'anotherRecordName',
+      DelegatedZoneNameServers: ['three', 'four'],
+      TTL: 172800,
+    },
+  });
+  await invokeHandler(event);
+
+  // THEN
+  expect(mockStsClient.assumeRole).toHaveBeenCalledTimes(2);
+  expect(mockRoute53Client.changeResourceRecordSets).toHaveBeenCalledTimes(2);
+  expect(mockRoute53Client.changeResourceRecordSets).toHaveBeenCalledWith({
+    HostedZoneId: '1',
+    ChangeBatch: {
+      Changes: [{
+        Action: 'UPSERT',
+        ResourceRecordSet: {
+          Name: 'recordName',
+          Type: 'NS',
+          TTL: 172800,
+          ResourceRecords: [{ Value: 'one' }, { Value: 'two' }],
+        },
+      }],
+    },
+  });
+  expect(mockRoute53Client.changeResourceRecordSets).toHaveBeenCalledWith({
+    HostedZoneId: '1',
+    ChangeBatch: {
+      Changes: [{
+        Action: 'DELETE',
+        ResourceRecordSet: {
+          Name: 'anotherRecordName',
+          Type: 'NS',
+          TTL: 172800,
+          ResourceRecords: [{ Value: 'three' }, { Value: 'four' }],
+        },
+      }],
+    },
+  });
+});
+
 test('calls create resource record set with DELETE for Delete event', async () => {
   // GIVEN
   mockStsClient.promise.mockResolvedValueOnce({ Credentials: { AccessKeyId: 'K', SecretAccessKey: 'S', SessionToken: 'T' } });
@@ -147,7 +237,7 @@ test('calls listHostedZonesByName to get zoneId if ParentZoneId is not provided'
   });
 });
 
-test('throws if more than one HostedZones are returnd for the provided ParentHostedZone', async () => {
+test('throws if more than one HostedZones are returned for the provided ParentHostedZone', async () => {
   // GIVEN
   const parentZoneName = 'some.zone';
   const parentZoneId = 'zone-id';
@@ -173,9 +263,9 @@ test('throws if more than one HostedZones are returnd for the provided ParentHos
 });
 
 function getCfnEvent(
-  event?: Partial<AWSLambda.CloudFormationCustomResourceEvent>,
+  event?: Partial<CrossAccountZoneDelegationEvent>,
   resourceProps?: any,
-): Partial<AWSLambda.CloudFormationCustomResourceEvent> {
+): Partial<CrossAccountZoneDelegationEvent> {
   return {
     RequestType: 'Create',
     ResourceProperties: {
@@ -193,6 +283,6 @@ function getCfnEvent(
 
 // helper function to get around TypeScript expecting a complete event object,
 // even though our tests only need some of the fields
-async function invokeHandler(event: Partial<AWSLambda.CloudFormationCustomResourceEvent>) {
-  return handler(event as AWSLambda.CloudFormationCustomResourceEvent);
+async function invokeHandler(event: Partial<CrossAccountZoneDelegationEvent>) {
+  return handler(event as CrossAccountZoneDelegationEvent);
 }