Fix DNS driver API inconsistencies by aaron-sierra · Pull Request #2066 · apache/libcloud

aaron-sierra · 2025-04-03T00:50:48Z

Fix DNS driver API inconsistencies

Description

Implement #2065

Summary

In general, commit messages provide necessary background regarding the changes made and why.

Drop Zerigo DNS
Deprecate outdated DNSimple DNS driver
Deprecate outdated RcodeZero DNS driver
Ignore AuroraDNS (see Remove unavailable cloud vendors: auroracompute; InternetSolutions; dimension data #2057)
Add unittest API request checking to remaining drivers
Add and leverage common code for domain/host name manipulation
Add and leverage common code for <type>[:<name>]-style record IDs
Change Record.id format for NS1 and Zonomi providers
Change Record.id interpretation for World Wide DNS

Status

Ready for review

Checklist (tick everything that applies)

Code linting (required, can be done after the PR checks)
Documentation
Tests
ICLA (required for bigger changes)

The Zerigo DNS service was permanently shut down on April 30, 2017.

The DNSimple v1 API was shutdown on May 31, 2018: https://blog.dnsimple.com/2018/03/api-v1-shutdown-notice/

The RcodeZero v1 API has been shutdown: https://my.rcodezero.at/openapi/

aaron-sierra · 2025-04-03T19:35:29Z

The latest batch of updates includes changes from black and isort.

Enable unit tests to verify HTTP requests they submit.

* Fix test_create_zone_success * Fix test_get_record_success

* Fix test_update_zone

Leverage the existing get_record.json for this test.

Previously, test_get_record_success() succeeded when receiving a record completely unrelated to the originating .get_record() request. Update the request to align with the expected response.

Previously, test_list_records_success() succeeded when receiving records completely unrelated to the originating .list_records() request. Update the request to align with the expected response.

Make NSOneResponse.errors and NSOneResponse.objects instance variables, rather than class variables. This was found due to errors in NSOneDNSDriver unit testing that only appeared when tests were run together, not when tests were run individually.

Add .hostname(), .fqdn(), and .prefix() helpers to the Zone class. Each of these functions accepts a host prefix, complete name (unrooted hostname), or FQDN (rooted hostname) associated with the Zone object and returns the format implied by the function name.

Add .to_default_id() and .from_default_id() functions to the DNSDriver class. These functions support the <type>[:<name>] (e.g. "A:www") record ID format used by many of the drivers.

Previously, our NsOneDNSDriver.get_record() function performed lookups using a `record_id` format that differed from the ID embedded within the returned Record instance, so the driver leveraged two independent record ID concepts simultaneously. The NS1 API returns an ID when records are created, but that ID has no local relevance. For example, it cannot be used to request the record from the NS1 API. Make the Record class .id value consistent between create_record() and get_record(z.id, r.id). | zone_id | z.id | r.id | record_id | ---------|----------|----------|---------------|---------------| previous | z.domain | random* | random* | r.type | next | z.domain | z.domain | r.type:r.name | r.type:r.name | * no meaning, effectively random

* Fix TTL in test_create_record_success() request

* Align record update with parent zone in test_update_record_success()

* Fix record_id in test_get_record_success() request

DNS AAAA records are associated with IPv6 addresses.

* Fix "ttle" typo in test_update_record()

Previously, the Zonomi DNS driver could only return the first record for a given host, because it used the name of the host as Record.id. Now, use the new default, <type>[:<name>], format to allow various records to be differentiated for a given host.

DNS drivers share the need to manipulate host/domain names, so let's add utility functions to the common Record and Zone classes to allow redundant code scattered throughout the various drivers to be consolidated into common and consistent implementations. * Record.name - no longer driver-dependent, now the host prefix or "" * Record.hostname - the complete host name, including domain part * Record.fqdn - the fully qualified domain name, including root dot * Zone.domain - no longer driver-dependent, now the unrooted domain * Zone.rooted() - format a (un)rooted domain as a rooted domain * Zone.rooted() - format a (un)rooted domain as an unrooted domain

Update testing inputs to conform to spec'ed usage.

Leverage common code from DNSDriver, Record, and Zone classes for host name and record ID handling.

Drop RackspaceDNSDriver._to_partial_record_name() and RackspaceDNSDriver._to_full_record_name() in favor of the common support provided by the Record and Zone classes.

aaron-sierra · 2025-04-03T20:06:57Z

The latest changes have been through black, isort, lint, and pylint.

micafer

Thanks for the extensive work on making the DNS driver APIs more consistent. The additional HTTP request assertions are particularly valuable.

I found an issue in the new Zone.prefix() implementation that should be addressed before merging.

The expression:

subname.rstrip(".").removesuffix(self.domain).rstrip(".")

removes the zone domain even when it is not preceded by a DNS label separator.

For example, with self.domain == "example.com", calling prefix("notexample.com") returns "not", even though notexample.com is not a hostname within the example.com zone.

Since Zone.hostname() and Zone.fqdn() depend on this method, the issue can silently produce incorrect DNS names.

I suggest checking for an exact domain match or the suffix "." + self.domain before stripping it, and adding regression tests for hostnames that merely share a textual suffix with the zone domain.

Additionally, this PR changes public-facing Record.id, Record.name, and Zone.domain semantics across multiple providers. As we are going to merge this to the new major version these compatibility changes will be explicitly documented and it is not majot issue.

aaron-sierra · 2026-11-06T19:44:58Z

Hi Miguel,

Thanks for your review. In the example that you described regarding Zone.prefix() for a "sub-domain" that isn't actually related to the Zone instance, what would you suggest as reasonable output? A few options come to mind:

Return None to indicate "no relationship", since the empty string is used to indicate "related, but no prefix"
- Both None and "" are falsy, which could make it a source of error.
Raise a ValueError exception (to aggressively highlight bugs?)
Something else

Presumably all three of the Zone helpers that operate on sub-domains should behave similarly when offered an unrelated "sub-domain" (i.e. Zone.prefix(), Zone.hostname(), and Zone.fqdn()). Does that sound right to you?

micafer · 2026-11-08T08:37:35Z

I probably prefer to return None when the domain is not related, as in the example notexample.com.
"" should only be returned when using the same domain as a prefix.
What do you think?

micafer · 2026-11-10T15:10:39Z

I probably prefer to return None when the domain is not related, as in the example notexample.com. "" should only be returned when using the same domain as a prefix. What do you think?

I thought a bit on this and probably returning the full DNS name can be the best solution: as in the example notexample.com will return notexample.com.

micafer · 2026-11-10T15:47:10Z

I'm still checking this and I found two issues:

Google Cloud DNS — get_record() uses a relative name in the API query.

After parsing the normalized record ID, parts.name contains a relative name such as foo. However, the Cloud DNS API expects a fully qualified name such as foo.example.com. in the name query parameter.

I believe this should use zone.fqdn(parts.name) instead. The corresponding test currently checks the endpoint URL but does not assert the query parameters.

World Wide DNS — update_record() fails when extra=None.

The new condition if "entry" in extra raises TypeError when the optional extra argument is omitted.

Please guard against None and add a regression test covering an update without extra.

aaron-sierra · 2026-11-10T17:43:35Z

Google Cloud DNS — get_record() uses a relative name in the API query.

After parsing the normalized record ID, parts.name contains a relative name such as foo. However, the Cloud DNS API expects a fully qualified name such as foo.example.com. in the name query parameter.

I believe this should use zone.fqdn(parts.name) instead. The corresponding test currently checks the endpoint URL but does not assert the query parameters.

You're right! I've added API parameter checking to test_get_record(), so this doesn't slip through again:

        self.assertEqual(len(sent.query["type"]), 1)
        self.assertEqual(sent.query["type"].pop(), "A")
        self.assertEqual(len(sent.query["name"]), 1)
        self.assertEqual(sent.query["name"].pop(), "foo.example.com.")

aaron-sierra · 2026-11-10T18:30:53Z

I probably prefer to return None when the domain is not related, as in the example notexample.com. "" should only be returned when using the same domain as a prefix. What do you think?

I thought a bit on this and probably returning the full DNS name can be the best solution: as in the example notexample.com will return notexample.com.

I think we need to clarify what should happen when a hostname (unrooted) and an FQDN (rooted) are passed into these functions. In the following examples, zone.domain == "example.com".

In the case of an unrelated hostname ("notexample.com") , these functions can't really do much more than treat the input as a prefix and output garbage:
- zone.prefix("notexample.com") == "notexample.com"
- zone.hostname("notexample.com") == "notexample.com.example.com"
- zone.fqdn("notexample.com") == "notexample.com.example.com."
In the case of an unrelated FQDN ("notexample.com.") , these functions know that the input is garbage and can return None (or raise an exception):
- zone.prefix("notexample.com.") == None
- zone.hostname("notexample.com.") == None
- zone.fqdn("notexample.com.") == None

I think that I prefer raising an exception in the unrelated FQDN case, considering how #2065 notes inconsistent usage of None and the empty string by various DNS driver implementations.

micafer · 2026-11-11T07:37:21Z

I think that I prefer raising an exception in the unrelated FQDN case, considering how #2065 notes inconsistent usage of None and the empty string by various DNS driver implementations.

Ok, so let's go in that direction. But we can create an specific exception (something like ZoneNameMismatchError) instead of using the generic ValueError.

micafer · 2026-11-11T07:38:44Z

So please make this changes and merge the trunk branch.

Hisernberg · 2026-11-11T20:43:06Z

FYI from #2216 (micafer's review thread): your NsOneResponse hunk and mine are semantically identical (class-level errors/objects → per-instance lists, set before super().__init__() since it calls success()). Whichever PR merges first, the other rebases trivially; the regression tests in #2216 (test_nsone.py) pin the shared behavior either way. No action needed from you now — just flagging so the eventual rebase is a no-op decision-wise.

aaron-sierra · 2026-11-11T22:25:06Z

I think that I prefer raising an exception in the unrelated FQDN case, considering how #2065 notes inconsistent usage of None and the empty string by various DNS driver implementations.

Ok, so let's go in that direction. But we can create an specific exception (something like ZoneNameMismatchError) instead of using the generic ValueError.

How's ZoneNotRelatedError for clarity and specificity?

Aaron Sierra added 3 commits March 26, 2025 18:31

Drop Zerigo DNS support

0cf61ca

The Zerigo DNS service was permanently shut down on April 30, 2017.

Deprecate DNSimple DNS driver

26cc840

The DNSimple v1 API was shutdown on May 31, 2018: https://blog.dnsimple.com/2018/03/api-v1-shutdown-notice/

Deprecate RcodeZero DNS driver

8b6b835

The RcodeZero v1 API has been shutdown: https://my.rcodezero.at/openapi/

aaron-sierra force-pushed the dns-consistent-api branch 2 times, most recently from 0fe8ec0 to 46c0e4f Compare April 3, 2025 19:34

Aaron Sierra added 24 commits April 3, 2025 15:04

test: Add optional request history to MockHttp

d3581ab

Enable unit tests to verify HTTP requests they submit.

dns: buddyns: Add API request checking

d5297ac

dns: cloudflare: Add API request checking

dde99d3

fixup: cloudflare: f-string

72c3999

dns: digitalocean: Add API request checking

d0aee37

dns: dnspod: Add API request checking

20fb38f

* Fix test_create_zone_success * Fix test_get_record_success

dns: durabledns: Add API request checking

f6dd97d

dns: gandi: Add API request checking

ef33880

* Fix test_update_zone

dns: godaddy: Add API request checking

25c7764

dns: google: Add API request checking

b40c1c2

dns: liquidweb: Implement test_create_record_success()

728fd5f

Leverage the existing get_record.json for this test.

dns: liquidweb: Add API request checking

dd0c4c4

dns: luadns: Add API request checking

7e461ae

dns: nsone: Fix test_get_record_success()

098a504

Previously, test_get_record_success() succeeded when receiving a record completely unrelated to the originating .get_record() request. Update the request to align with the expected response.

dns: nsone: Fix test_list_records_success()

7f11904

Previously, test_list_records_success() succeeded when receiving records completely unrelated to the originating .list_records() request. Update the request to align with the expected response.

dns: Add {to,from}_default_id() to DNSDriver class

d0bb79c

Add .to_default_id() and .from_default_id() functions to the DNSDriver class. These functions support the <type>[:<name>] (e.g. "A:www") record ID format used by many of the drivers.

dns: onapp: Add API request checking

72d4571

* Fix TTL in test_create_record_success() request

dns: pointdns: Add API request checking

5915071

* Align record update with parent zone in test_update_record_success()

dns: rackspace: Add API request checking

7271045

* Fix record_id in test_get_record_success() request

dns: route53: Fix test_update_record() record type

4b9d364

DNS AAAA records are associated with IPv6 addresses.

dns: route53: Add API request checking

47f9ca5

* Fix "ttle" typo in test_update_record()

Aaron Sierra added 11 commits April 3, 2025 15:04

dns: gandi_live: Add API request checking

b9a323d

dns: pointdns: Make records agree with zone

1a988f0

dns: luadns: Make records agree with zones

aa3d081

dns: google: Leverage {to,from}_default_id()

cf68aa8

dns: google: Drop FQDN from inputs

59e3125

Update testing inputs to conform to spec'ed usage.

dns: gandi: Leverage {to,from}_default_id()

f21850b

dns: route53: Leverage common code

2ec65db

Leverage common code from DNSDriver, Record, and Zone classes for host name and record ID handling.

dns: gandi_live: Leverage {to,from}_default_id()

3a7c27f

dns: rackspace: Leverage common host naming code

fcaf113

Drop RackspaceDNSDriver._to_partial_record_name() and RackspaceDNSDriver._to_full_record_name() in favor of the common support provided by the Record and Zone classes.

aaron-sierra force-pushed the dns-consistent-api branch from 46c0e4f to fcaf113 Compare April 3, 2025 20:04

aaron-sierra marked this pull request as ready for review April 3, 2025 20:07

tisonkun mentioned this pull request Jan 27, 2026

Tracking issue for the next release #2107

Closed

micafer requested changes Nov 6, 2026

View reviewed changes

micafer added the api: dns label Nov 6, 2026

micafer mentioned this pull request Nov 10, 2026

Tracking issue for the 4.0.0 release #2215

Open

2 of 7 tasks

micafer requested a review from tisonkun November 10, 2026 09:51

micafer mentioned this pull request Nov 11, 2026

Fix shared class-level errors/objects lists in 4 DNS response classes #2216

Merged

Conversation

aaron-sierra commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!