Route53: fix deleting records which are part of a multi value record set - #2177
Route53: fix deleting records which are part of a multi value record set#2177Sanjays2402 wants to merge 2 commits into
Conversation
delete_record built a DELETE changeset containing only the value of the record being deleted. Route53 requires a DELETE to list every value in the record set, so deleting any record which is part of a multi value set (for example one MX value) was rejected as an invalid change batch and surfaced as RecordDoesNotExistError. update_record already handles this via the _multi_value/_other_records metadata that _to_records attaches. delete_record now uses the same metadata and includes the values of the other records in the set. The priority prefix which _to_record strips out of MX/SRV values is restored so the emitted values match what Route53 stores. Adds a regression test asserting the full set of MX values is present in the emitted changeset.
There was a problem hiding this comment.
Pull request overview
This PR fixes Route53 delete_record for multi-value record sets by ensuring DELETE change batches include all values in the set (matching Route53’s API requirements), and adds a regression test plus a changelog entry.
Changes:
- Update Route53
delete_recordto use multi-value metadata (_multi_value/_other_records) and emit a DELETE changeset containing all values in the record set. - Restore MX priority prefixes (stripped during parsing) when emitting record values for deletion.
- Add a unit test asserting the DELETE changeset for a multi-value MX record contains every
<Value>.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| libcloud/dns/drivers/route53.py | Adds multi-value-aware delete logic and value reconstruction helper for Route53 changesets. |
| libcloud/test/dns/test_route53.py | Adds a regression test validating multi-value MX deletion sends all values in the change batch. |
| CHANGES.rst | Documents the Route53 multi-value delete fix in the DNS changelog section. |
Suppressed comments (1)
libcloud/dns/drivers/route53.py:303
_to_recordstrips SRV values intopriority/weight/port+data, but_to_record_valueonly restorespriority. For SRV record sets, Route53 expects the full value (priority weight port target) in a DELETE changeset, so multi-value SRV deletes will still be rejected.
if extra and "priority" in extra:
return "{} {}".format(extra["priority"], data)
return data
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if r.extra.get("_multi_value", False) and r.extra.get("_other_records", []): | ||
| self._delete_multi_value_record(record=r) | ||
| else: | ||
| batch = [("DELETE", r.name, r.type, r.data, r.extra)] | ||
| self._post_changeset(record.zone, batch) |
There was a problem hiding this comment.
Good catch - that was a real gap. A Record from create_record() / ex_create_multi_value_record() (or one built by hand) has no _multi_value / _other_records, so both delete_record() and update_record() would send a single-value changeset and Route53 would reject it.
Pushed 6e2579e. Added _with_record_set_metadata(), called from both update_record() and delete_record(): if _multi_value is absent from record.extra it looks the record set up in the zone and merges the metadata in, keeping any caller-supplied extras (ttl, priority). If the lookup fails or the record is not found it falls back to the old behaviour rather than raising.
I used list_records() rather than get_record() for the lookup, since get_record() only asks Route53 for maxitems=1 and then rejects the result when the returned member is not the one requested - for a multi value MX set that raises RecordDoesNotExistError before the metadata can be read.
New test test_delete_multi_value_record_without_record_set_metadata builds a bare Record with no metadata and asserts the DELETE changeset contains all five MX values. It fails without the driver change (only 1 ASPMX.L.GOOGLE.COM. is emitted). libcloud/test/dns/test_route53.py is 25 passed.
Records which did not come from list_records()/get_record() carry no _multi_value or _other_records metadata, so update_record() and delete_record() would build a changeset with a single value and Route53 would reject it. Look the record set up when the metadata is missing.
Closes #1831
Route53: fix deleting records which are part of a multi value record set
Description
delete_recordbuilt a DELETE changeset containing only the value of the recordbeing deleted. Route53 requires a DELETE to list every value in the record
set, so deleting any record belonging to a multi value set (e.g. one MX value)
was rejected as an invalid change batch and surfaced as
RecordDoesNotExistError.update_recordalready handles this correctly using the_multi_value/_other_recordsmetadata that_to_recordsattaches to each record.delete_recordnow uses the same metadata and includes the other values of theset in the changeset. The priority prefix that
_to_recordstrips out of MX/SRVvalues is restored so the emitted values match what Route53 stores.
Verified against the existing
list_recordsfixture: before the change theDELETE changeset for the 5-value MX set contained only
1 ASPMX.L.GOOGLE.COM.; after it contains all five values.This change was prepared with AI assistance; the regression test was run locally
and fails without the fix.
Status
done, ready for review
Checklist (tick everything that applies)