From 5e612ce28fbbabe065fb3e0717ba12f5c8e680cc Mon Sep 17 00:00:00 2001 From: Jonathan Goldwasser Date: Fri, 24 Jun 2022 11:24:02 +0200 Subject: [PATCH] fix(route53): cannot delete existing alias record For an alias record, the `ListResourceRecordSets` call returns an empty array that the `ChangeResourceRecordSets` call doesn't accept. Remove undefined and empty arrays when calling `ChangeResourceRecordSets`. Closes #20847 --- .../index.ts | 26 ++++++++--- ...delete-existing-record-set-handler.test.ts | 46 +++++++++++++++++++ 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk/aws-route53/lib/delete-existing-record-set-handler/index.ts b/packages/@aws-cdk/aws-route53/lib/delete-existing-record-set-handler/index.ts index e04eb9e6a46b6..1abf667a5ff6e 100644 --- a/packages/@aws-cdk/aws-route53/lib/delete-existing-record-set-handler/index.ts +++ b/packages/@aws-cdk/aws-route53/lib/delete-existing-record-set-handler/index.ts @@ -35,15 +35,13 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent ChangeBatch: { Changes: [{ Action: 'DELETE', - ResourceRecordSet: { + ResourceRecordSet: removeUndefinedAndEmpty({ Name: existingRecord.Name, Type: existingRecord.Type, - // changeResourceRecordSets does not correctly handle undefined values - // https://github.com/aws/aws-sdk-js/issues/3506 - ...existingRecord.TTL ? { TTL: existingRecord.TTL } : {}, - ...existingRecord.AliasTarget ? { AliasTarget: existingRecord.AliasTarget } : {}, - ...existingRecord.ResourceRecords ? { ResourceRecords: existingRecord.ResourceRecords } : {}, - }, + TTL: existingRecord.TTL, + AliasTarget: existingRecord.AliasTarget, + ResourceRecords: existingRecord.ResourceRecords, + }), }], }, }).promise(); @@ -54,3 +52,17 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent PhysicalResourceId: `${existingRecord.Name}-${existingRecord.Type}`, }; } + +// https://github.com/aws/aws-sdk-js/issues/3411 +// https://github.com/aws/aws-sdk-js/issues/3506 +function removeUndefinedAndEmpty(obj: T): T { + const ret: { [key: string]: any } = {}; + + for (const [k, v] of Object.entries(obj)) { + if (v && (!Array.isArray(v) || v.length !== 0)) { + ret[k] = v; + } + } + + return ret as T; +} diff --git a/packages/@aws-cdk/aws-route53/test/delete-existing-record-set-handler.test.ts b/packages/@aws-cdk/aws-route53/test/delete-existing-record-set-handler.test.ts index 26e8519eb97c5..77d0de4b1b439 100644 --- a/packages/@aws-cdk/aws-route53/test/delete-existing-record-set-handler.test.ts +++ b/packages/@aws-cdk/aws-route53/test/delete-existing-record-set-handler.test.ts @@ -133,3 +133,49 @@ test('delete request', async () => { expect(mockRoute53.changeResourceRecordSets).not.toHaveBeenCalled(); }); + +test('with alias target', async () => { + mockListResourceRecordSetsResponse.mockResolvedValueOnce({ + ResourceRecordSets: [ + { + Name: 'dev.cdk.aws.', + Type: 'A', + TTL: undefined, + ResourceRecords: [], + AliasTarget: { + HostedZoneId: 'hosted-zone-id', + DNSName: 'dns-name', + EvaluateTargetHealth: false, + }, + }, + ], + }); + + mockChangeResourceRecordSetsResponse.mockResolvedValueOnce({ + ChangeInfo: { + Id: 'change-id', + }, + }); + + await handler(event); + + expect(mockRoute53.changeResourceRecordSets).toHaveBeenCalledWith({ + HostedZoneId: 'hosted-zone-id', + ChangeBatch: { + Changes: [ + { + Action: 'DELETE', + ResourceRecordSet: { + Name: 'dev.cdk.aws.', + Type: 'A', + AliasTarget: { + HostedZoneId: 'hosted-zone-id', + DNSName: 'dns-name', + EvaluateTargetHealth: false, + }, + }, + }, + ], + }, + }); +});