Skip to content

Commit 22681b1

Browse files
authored
fix(route53): cannot delete existing alias record (#20858)
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`. See aws/aws-sdk-js#3411 Closes #20847 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 4d003a5 commit 22681b1

File tree

2 files changed

+65
-7
lines changed

2 files changed

+65
-7
lines changed

packages/@aws-cdk/aws-route53/lib/delete-existing-record-set-handler/index.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,13 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
3535
ChangeBatch: {
3636
Changes: [{
3737
Action: 'DELETE',
38-
ResourceRecordSet: {
38+
ResourceRecordSet: removeUndefinedAndEmpty({
3939
Name: existingRecord.Name,
4040
Type: existingRecord.Type,
41-
// changeResourceRecordSets does not correctly handle undefined values
42-
// https://github.com/aws/aws-sdk-js/issues/3506
43-
...existingRecord.TTL ? { TTL: existingRecord.TTL } : {},
44-
...existingRecord.AliasTarget ? { AliasTarget: existingRecord.AliasTarget } : {},
45-
...existingRecord.ResourceRecords ? { ResourceRecords: existingRecord.ResourceRecords } : {},
46-
},
41+
TTL: existingRecord.TTL,
42+
AliasTarget: existingRecord.AliasTarget,
43+
ResourceRecords: existingRecord.ResourceRecords,
44+
}),
4745
}],
4846
},
4947
}).promise();
@@ -54,3 +52,17 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
5452
PhysicalResourceId: `${existingRecord.Name}-${existingRecord.Type}`,
5553
};
5654
}
55+
56+
// https://github.com/aws/aws-sdk-js/issues/3411
57+
// https://github.com/aws/aws-sdk-js/issues/3506
58+
function removeUndefinedAndEmpty<T>(obj: T): T {
59+
const ret: { [key: string]: any } = {};
60+
61+
for (const [k, v] of Object.entries(obj)) {
62+
if (v && (!Array.isArray(v) || v.length !== 0)) {
63+
ret[k] = v;
64+
}
65+
}
66+
67+
return ret as T;
68+
}

packages/@aws-cdk/aws-route53/test/delete-existing-record-set-handler.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,49 @@ test('delete request', async () => {
133133

134134
expect(mockRoute53.changeResourceRecordSets).not.toHaveBeenCalled();
135135
});
136+
137+
test('with alias target', async () => {
138+
mockListResourceRecordSetsResponse.mockResolvedValueOnce({
139+
ResourceRecordSets: [
140+
{
141+
Name: 'dev.cdk.aws.',
142+
Type: 'A',
143+
TTL: undefined,
144+
ResourceRecords: [],
145+
AliasTarget: {
146+
HostedZoneId: 'hosted-zone-id',
147+
DNSName: 'dns-name',
148+
EvaluateTargetHealth: false,
149+
},
150+
},
151+
],
152+
});
153+
154+
mockChangeResourceRecordSetsResponse.mockResolvedValueOnce({
155+
ChangeInfo: {
156+
Id: 'change-id',
157+
},
158+
});
159+
160+
await handler(event);
161+
162+
expect(mockRoute53.changeResourceRecordSets).toHaveBeenCalledWith({
163+
HostedZoneId: 'hosted-zone-id',
164+
ChangeBatch: {
165+
Changes: [
166+
{
167+
Action: 'DELETE',
168+
ResourceRecordSet: {
169+
Name: 'dev.cdk.aws.',
170+
Type: 'A',
171+
AliasTarget: {
172+
HostedZoneId: 'hosted-zone-id',
173+
DNSName: 'dns-name',
174+
EvaluateTargetHealth: false,
175+
},
176+
},
177+
},
178+
],
179+
},
180+
});
181+
});

0 commit comments

Comments
 (0)