-
Notifications
You must be signed in to change notification settings - Fork 926
Fix DNS driver API inconsistencies #2066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
aaron-sierra
wants to merge
52
commits into
apache:trunk
Choose a base branch
from
aaron-sierra:dns-consistent-api
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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/
0fe8ec0
to
46c0e4f
Compare
The latest batch of updates includes changes from |
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()
The "DYN" parameter is only defined for non-resellers: https://www.worldwidedns.net/dns_api_protocol_new_domain.asp The "ID" parameter is only defined for resellers: https://www.worldwidedns.net/dns_api_protocol_new_domain_reseller.asp
Include "ID" parameter when reseller_id is set: https://www.worldwidedns.net/dns_api_protocol_modify_reseller.asp
The update_record() and delete_record() functions require a Record instance in their respective argument lists to identify the DNS record to manipulate. Record objects instantiated by this driver contain the assocated "slot" ID within the .id field, so: 1. There is no need for the user to specify the "slot" ID via .update_record(..., extra={"entry": ID}). However, since it was previously required, an "entry" key will be accepted as long as it is consistent with the Record instance. 2. There is no need for .delete_record() to search the Record's zone for the "slot" ID of the Record.
The API key is automatically inserted into the request by the ZonomiConnection class.
The MockHttp function was defined, but the corresponding test was not.
The v3 API became completely unsupported by Linode on July 31, 2023. https://www.linode.com/community/questions/25142/status-of-v3-api-eol
The v1 API became completely unsupported by Vultr on August 14, 2023. https://docs.vultr.com/vultr-api-v1-to-v2-transition-strategies
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.
46c0e4f
to
fcaf113
Compare
The latest changes have been through |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix DNS driver API inconsistencies
Description
Implement #2065
Summary
In general, commit messages provide necessary background regarding the changes made and why.
<type>[:<name>]
-style record IDsRecord.id
format for NS1 and Zonomi providersRecord.id
interpretation for World Wide DNSStatus
Checklist (tick everything that applies)