Use Windows API to get dns nameservers

[blink1073]

trafficstars

Fixes #1191

As far as I can tell the only two ways of getting the search list are using COM or the registry, so I subclassed the registry getter.

Testing with a PR against my fork so I can run actions: https://github.com/blink1073/dnspython/pull/1

• [x] Get it to pass in GitHub Actions
• [x] Get it to pass in PyMongo CI
• [ ] Establish how to configure the usage of this getter

[blink1073]

@jaraco might you be able to verify that this fixes the issue for you as well? For PyMongo the relevant change was that we needed to ignore a loopback adapter.

[rthalley]

This patch is looking good so far. Ideally we want all three DnsInfo attributes. You've gotten the nameservers attribute. The DnsSuffix field is the domain attribute of DnsInfo (if not NULL or an empty string); if NULL or empty, the DnsInfo value should be None. The list at FirstDnsSuffix at the very bottom of the structure is what should go in the search field (or empty list if not there).

[blink1073]

I did some more digging, and the FirstDnsSuffix is only set if the adapter is configured directly. I'm getting a null pointer for the value. If it is null, we need to fall back to using the registry. Here's what ipconfig looks like on our EC2 host:

$ ipconfig /all

Windows IP Configuration

   Host Name . . . . . . . . . . . . : <snip>
   Primary Dns Suffix  . . . . . . . :
   Node Type . . . . . . . . . . . . : Hybrid
   IP Routing Enabled. . . . . . . . : No
   WINS Proxy Enabled. . . . . . . . : No
   DNS Suffix Search List. . . . . . : us-east-1.ec2-utilities.amazonaws.com
                                       ec2.internal

Ethernet adapter Ethernet 4:

   Connection-specific DNS Suffix  . : ec2.internal
   Description . . . . . . . . . . . : Amazon Elastic Network Adapter #2
  <snip>
  DNS Servers . . . . . . . . . . . : 10.122.0.2

[blink1073]

Actually, I can't find any indication that FirstDnsSuffix is what we want, but we know that the registry approach works, so I think the best compromise is to use the registry for the search list unconditionally.

[rthalley]

Are you still working on this (still marked as draft)? No rush, just making sure you're not blocked on me or something! I'm ok with your registry approach if you are unsure about FirstDnsSuffix.

[blink1073]

Hi @rthalley, I'm blocked on two decisions:

• Is it okay if I refactor a bit so we can re-use the logic to read the search field from the system registry without the awkward subclassing that I'm doing currently?
• How would you like to enable the opt-in? I'm currently using an environment variable, DNSPYTHON_PREFER_CTYPES.

[rthalley]

I'm ok if you split out the search logic. For enabling the opt-in, I think maybe having an enum ConfigMethod with values like Registry, WMI, and Win32 would be good. There would be a module level variable _config_method that was an instance of the enum, and a set_config_method() API for setting it. Default is registry. No environment variable lookups.

[blink1073]

Thanks @rthalley, I'm picking this back and up and testing now.

[jaraco]

Sorry for the delay. Been busy. Will test this soon.

[blink1073]

Thanks @jaraco, there's no rush from my end. I've done the following:

• Updated the configuration setting method
• Added a test for setting the configuration method
• Added a test using wmi
• Added a whatsnew entry
• Tested using the pymongo test suite

[rthalley]

looking good!

[jaraco]

I'm sad to report that the behavior is still undesirable when running out of the box:

.venv/Scripts/python -m pip install git+https://github.com/blink1073/dnspython@windows-ctypes
Collecting git+https://github.com/blink1073/dnspython@windows-ctypes
  Cloning https://github.com/blink1073/dnspython (to revision windows-ctypes) to c:\users\jaraco\appdata\local\temp\pip-req-build-pczvej2q
  Running command git clone --filter=blob:none --quiet https://github.com/blink1073/dnspython 'C:\Users\jaraco\AppData\Local\Temp\pip-req-build-pczvej2q'
  Running command git checkout -b windows-ctypes --track origin/windows-ctypes
  branch 'windows-ctypes' set up to track 'origin/windows-ctypes'.
  Switched to a new branch 'windows-ctypes'
  Resolved https://github.com/blink1073/dnspython to commit a713d982fed1e8980dd6889ea2651c1179ce346c
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done

.venv/Scripts/python -c "import dns.resolver; print(dns.resolver.get_default_resolver().nameservers)"
['10.50.10.50', '10.50.50.50', '157.58.217.23', '65.54.13.169']

I see now, looking at the implementation, that the default behavior is degraded and one has to opt-in to the fix. I guess I don't understand why we would default to a degraded behavior and require opt-in for a more appropriate behavior. At the very least, if the default behavior is expected to be degraded, that should be called out in the user guide (something like "All callers should consider calling set_config_method(win32) to avoid degraded behavior on Windows.").

My preference would be to offer the optimal behavior by default and have a fallback for those wishing to opt out. Maybe that's the long term plan.

Ultimately, if PyMongo adopts this change and implements the opt-in, that will address my motivation for filing the ticket, but it will leave the failure mode open for any other usage of dnspython. I'd prefer to see the issue addressed systemically so it's not a latent risk for the next consumer.

I'll add a few other less crucial comments to the review.

[blink1073]

@jaraco were you able to confirm that using set_config_method addresses the undesirable behavior?

To answer your question, yes, we (PyMongo) intend to detect the presence of set_config_method and use this new functionality when available if it were to get merged as-is.

[rthalley]

We don't prefer degraded behavior, but we are cautious about breaking existing code, so I my plan is to put out a release where pymongo and others can use the new stuff more widely, and then if all is still good, we will change the default in the release after that.

[jaraco]

@jaraco were you able to confirm that using set_config_method addresses the undesirable behavior?

I only got as far as confirming that using set_config_method has the intended effect on the nameservers in my environment (before the change, inactive nameservers were present and after the change only active nameservers were present).

I did try replicating the original issue with PyMongo, but it wasn't occurring, maybe because of the luck of ordering in the nameservers or perhaps some other factor (maybe wmi was somehow implicitly present).

Regardless, I'm confident the approach addresses the issue.

[rthalley]

Are we ready to leave draft state then?

[blink1073]

Are we ready to leave draft state then?

Done!

[rthalley]

In case you're continuing to use your code while waiting for the next dnspython release, I fixed a few bugs related to IPv6 extraction and also an infinite loop if there was a non-operational interface. Everything else worked great when I tested in on my local Windows box, thanks!

[blink1073]

Great, thank you!