Skip to content

Paginate DigitalOcean destroy_dns_records and drop Py2 decode (Fixes #55143)#69766

Open
ggiesen wants to merge 1 commit into
saltstack:3006.xfrom
ggiesen:fix-55143
Open

Paginate DigitalOcean destroy_dns_records and drop Py2 decode (Fixes #55143)#69766
ggiesen wants to merge 1 commit into
saltstack:3006.xfrom
ggiesen:fix-55143

Conversation

@ggiesen

@ggiesen ggiesen commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Makes the DigitalOcean cloud driver's destroy_dns_records() walk every page of a domain's DNS records instead of requesting only the first page, and removes a leftover Python 2 .decode() call that crashed record matching on Python 3.

The pagination loop reuses the same fetch/page + "next" in response["links"]["pages"] idiom already used by avail_images, list_keypairs, list_floating_ips, and _list_nodes in this driver, requesting records?page=<n>&per_page=200 until the API stops returning a next link.

What issues does this PR fix or reference?

Fixes #55143

Previous Behavior

destroy_dns_records() issued a single unpaginated request:

response = query(method="domains", droplet_id=domain, command="records")

The DigitalOcean API returns at most 20 DNS records per page by default, so any record beyond the first page was never seen and therefore never deleted. In addition, the match used r["name"].decode(), a Python 2 leftover; on Python 3 str has no .decode(), so the comprehension raised AttributeError for any domain that has records (real domains always have SOA/NS), meaning the function crashed before deletion could even be attempted.

New Behavior

destroy_dns_records() accumulates domain_records across every page before matching, so records past page 1 are found and their IDs deleted. Record names are compared directly (r["name"] == hostname), which works on Python 3. Non-matching records are still left untouched, and a failed domain lookup still returns False without attempting any deletion.

Merge requirements satisfied?

  • Docs -- N/A (internal bug fix, no doc-facing behaviour change)
  • Changelog -- changelog/55143.fixed.md added
  • Tests written/updated -- tests/pytests/unit/cloud/clouds/test_digitalocean.py:
    • test_destroy_dns_records_pagination_55143 -- patches query with a paged side effect; the matching record lives on page 2, asserts the loop requested records?page=2 and issued the records/42 delete, and that no page-1 non-matching record was deleted.
    • test_destroy_dns_records_no_matching_records_55143 -- inverse case: an empty record set produces zero deletions (passes with and without the fix; guards against over-deletion).
    • test_destroy_dns_records_domain_lookup_failure_55143 -- a SaltCloudSystemExit from the lookup returns False with no delete attempted.

Commits signed with GPG?

No

destroy_dns_records issued a single unpaginated request for a domain's
DNS records, so the DigitalOcean API returned only the first page (20
records by default) and any matching record beyond page 1 was never
deleted. Walk every page and accumulate the records before matching,
mirroring the pagination idiom already used elsewhere in this driver.

Also drop the leftover Python 2 r["name"].decode() call, which raised
AttributeError on Python 3 for every domain that has records.

Fixes saltstack#55143
@ggiesen ggiesen requested a review from a team as a code owner July 10, 2026 17:55
@charzl charzl self-assigned this Jul 10, 2026
@dwoz dwoz added the test:full Run the full test suite label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants