nresare commented 6 years ago

(This is a proposed solution to the issue that I encountered and described in https://github.com/SpamExperts/OrangeAssassin/issues/81)

While I was trying to get OrangeAssassin running on my OSX machine I noted that there were some strange namespace issues, caused by the fact that both py3dns and dnspython were installed in parallel. py3dns installs itself in top level module DNS and dnspython in dns, which turns out to be the same directory on a default install OSX machine due to filesystem case insensitivity.

I understand that pyspf has not seen a lot of development in the last few years, and my understanding is that dnspython is by far the more widely used library these days. It also happens to be the dns library used by other parts of OrangeAssassin.

I would be happy to create a diff that changes the dns library dependency in pyspf from py3dns to dnspython (along the lines of the code present in master) and also introduces a dependency declaration on dnspython (so that pip will handle the dependency for you) if you were interested in accepting it and releasing a new pyspf. What do you think?

sdgathman commented 6 years ago

My preferred solution is found in Milter/dns.py in the pymilter project. This is a simplified dns API that includes a short term cache suitable for email applications like pyspf, dkim, etc. dkimpy has a copy of it. It should be it's own module and shared, and that is what pyspf should use. Or we can just make another copy in pyspf...

Besides pydns and dnspython, there are other dns libraries that I'd like to use. Including a C library gaining popularity I want to wrap, but can't remember the name for the life of me. aarrrggghhh.

While dnspython is much more complete and supported, I still prefer pydns for my email applications because it is much more lightweight. So I do not want to hardwire dnspython. I do need to move pyspf to using the plugin that dkimpy and pymilter user, however.

I think one suggested name was dnsplug.

sdgathman commented 6 years ago

For your situation, if it was me I would actually rename the module from DNS to pydns, keeping the lightweight library but fixing the name conflict. I will talk to Fedora packagers about renaming and maybe including a symlink for backward compatibility.

nresare commented 6 years ago

Thank you for the feedback, Stuart.

Without having approached the py3dns maintainer, my assumption would be that it is a hard sell to have them make such a disruptive change to handle the admittedly weird semantics of the OSX file system, a problem that is invisible to most users. I will make an attempt though.

It is also unclear to me how the thin abstraction layer would resolve the underlying conflict. I guess one could provide a dnsplug pypi wheel that doesn't have any declared dependency on any concrete implementation and then have all users of that library also pick a dns implementation as a top level dependency, but this would add some complexity. In this case, having pip install pyspf in a virtualenv install pypsf and all deps needed to be able to use it.

sdgathman commented 6 years ago

The dnsplug.py implementation in dkimpy simply tries all supported libraries in order of preference:

try:
    # prefer dnspython (the more complete library)
    import dns
    import dns.resolver  # http://www.dnspython.org
    import dns.exception

    if not hasattr(dns.rdatatype,'SPF'):
      # patch in type99 support
      dns.rdatatype.SPF = 99
      dns.rdatatype._by_text['SPF'] = dns.rdatatype.SPF

    DNSLookup = DNSLookup_dnspython
except:
    import DNS    # http://pydns.sourceforge.net

    if not hasattr(DNS.Type, 'SPF'):
        # patch in type99 support
        DNS.Type.SPF = 99
        DNS.Type.typemap[99] = 'SPF'
        DNS.Lib.RRunpacker.getSPFdata = DNS.Lib.RRunpacker.getTXTdata

    # Fails on Mac OS X? Add domain to /etc/resolv.conf
    DNS.DiscoverNameServers()
    DNSLookup = DNSLookup_pydns

sdgathman commented 6 years ago

Also, I am the maintainer of py3dns :-)

nresare commented 6 years ago

Ah, that code would would neatly solve my problem. I'll come up with a PR to move to that.

For longer term reduction of confusing debug sessions, what would be the proper forum for bringing up a discussion about changing the namespace of py3dns? The code seems to be maintained on launchpad.net, but my assumption would be that https://bugs.launchpad.net/py3dns is fairly ubuntu-centric, right?

sdgathman commented 6 years ago

I moved py3dns to launchpad because Scott Kitterman is a Debian/Ubuntu maintainer, and a primary user of pydns. But he is now in the process of moving to git.

sdgathman commented 6 years ago

Ah, the resolver I was interested in trying with pyspf is python-unbound.

sdgathman commented 6 years ago

Scott and I hang out on #spf and #dkim on freenode.net to discuss pyspf/pydns/dkimpy issues.

kitterma commented 6 years ago

https://bugs.launchpad.net/py3dns is upstream. Launchpad is used for both Ubuntu and upstream projects. I use it since I prefer free software based systems. Also, it's on irc.perl.org (for historical reasons, I know it's odd).

kitterma commented 6 years ago

PyDNS goes back to roughly Python 1.6/2000. If a rename is needed, I think the newer project gets to do it (dnspython had it's 1.0 release in November 2012).

nresare commented 6 years ago

I have opened a task with dnspython here: https://github.com/rthalley/dnspython/issues/303

sdgathman commented 6 years ago

There is also the consideration of which project has the most users. My subjective impression, unsubstantiated by any survey, is that pydns has a loyal (myself included) but smaller following.

kitterma commented 6 years ago

This is only a problem on file systems which, IMO, are broken with respect to Python name spaces. In Python they are case sensitive and if the file system can't support that, then I don't think it properly supports Python.

nresare commented 6 years ago

So, it seems both maintainers of the underlying dns lookup implementations are in agreement that if any project should change it's the other one. Well, it was worth a try.

I agree that having a case insensitive file system was a bad engineering decision, but sometimes you are stuck with bad decisions for a long time. Obviously, it's any maintainer's prerogative to decide to not support any target platform for any reason, and I'm not going to argue with that.

Moving forward, I had a look at the dns module in pymilter and I think that your suggested approach of breaking it out into a separate module and have that one implement a fallback using dnspython if pydns is not available is a good one.

I created a repo at https://github.com/nresare/dnsplug that currently only holds a copy of dns.py and a single trivial test case. If you agree that this is a good way to proceed, I would be happy to clean up the code, fix the fallback to dnspython, comprehensive testing for several platforms, and modify pymilter, pyspf and any other project that might benefit.

nresare commented 6 years ago

Also, if you want to host the project under your github user, I'm happy to transfer the project to you. Some of the things I would like to do:

Implement fallback to dnspython
Create an easier to use interface. I'm thinking that having one function as the public interface of the module, named resolve() or similar, would be nice.
I think it would be neat to have a bounded cache. Sometimes the use case changes, and an unbound datatstructure can lead to all kinds of bad behaviour
Respecting TTL of cached entries seems easy and useful
mypy type annotations (while still keeping python 2.7 compatibility)

Please feel free to raise any questions or concerns :)

sdgathman commented 6 years ago

Here are the reflections of the same concept that need to be reconciled:

1) dnsplug.py in the dkimpy project - has session caching and dynamic fallback from pydns to dnspython 2) Milter/dns.py in the pymilter project - has session caching but no fallback 3) DNSLookup in spf.py in pyspf project - has session caching no fallback to dnspython, BUT the test suite plugs in a DNSLookup implementation that pulls DNS data (and simulated TIMEOUTs and ERRORs) from the test suite.

Some milters import Milter.dns - but that can, of course, just import from dnsplug. The change will introduce a new dependency - unless I embed dnsplug.py as I did with dkimpy. At one point, I thought all users of dkimpy and pyspf would also be using pymilter - but that is of course silly in hind sight.

The pyspf test suite also exercises the Session caching - which is surprisingly non-trivial, and several bugs have fixed and tests added. So checking that pyspf still passes the test suite is important for testing dnsplug (the Session cache part).

Now, if I could see my way forward to have all those projects import dnsplug without breaking distro packages (maybe release notes would do the job), I would make dnsplug a package, and have plugin modules for each python dns implementation: pydns(py3dns), dnspython, testspf, python-unbound. That last one has not been used in any pymilter projects yet that I know of.

sdgathman commented 6 years ago

I guess the first step is a simple one file dnsplug.py that handles pydns/dnspython and testspf.

kitterma commented 6 years ago

I used the dnsplug.py from dkimpy in dkimpy-milter, which led to me having to do some unfun hackery in setup.py to get the install_requires correct. I'd love to see a dnsplug.py on pypi that I could just depend on and let the complexity of which DNS implementation is chosen hide behind it.

I think that the dnsplug.py from dkimpy is the best thing to start from. I would suggest (and am willing to help with):

Setup the basic repository with dnsplug.py and a setuptools based setup.py to handle dependncy resolution. I've worked through doing the latter for dkimpy-milter and will also include it in the next release of dkimpy (it's currently in git master).
Make sure that works for the existing dnsplug users (which as far as I know are dkimpy and dkimpy-milter) and release to pypi.
Update the existing users to depend on the pypi dnsplug and start to convert other users such as pyspf.

Last I checked (and it's been a few years) the Unbound python bindings did not support TXT. Someone will need to check on that and then we'll have to wait for an unbound release if it's still missing.

nresare commented 6 years ago

I hope you both have had a good easter weekend.

Circling back to this, I have now added support to fallback to dnspython, with tests for both protocol implementations set up with tox. I would like to hear if you think that I'm on the right track here. I'm not interested working on this unless we can get this code merged and released fairly soon.

The latest code is available here. Please have a look.

Some thoughts in bullet form:

If we want to change the API, now is the time to do it. Right now, keeping with the old interface of DNSLookup, AAAA records are returned as binary strings but A records are returned as ascii encoded dot separated octets. There is a suggestion in a comment to unify those two that proposes binary encoding for both, but I have preference for using a human readable format. Thoughts on that?
I have opted to make as few changes as possible to the existing code, but the current code is pretty far from PEP-8 and other coding conventions in use in the python community. Do you think that it would be worth the effort to move to a more commonly used naming standard etc?
I have not yet looked at the other use cases for this (dkimpy-milter, dkimpy). I wanted some input first.

sdgathman commented 6 years ago

I don't like using if-else to select between pydns_lookup and dnspython_lookup. There may be other choices. What was the reason for not simply assigning DNSLookup = [some implementation] ?

sdgathman commented 5 years ago

While fixing issue #10, I thought of calling using the name "anydns" instead of "dnsplug". For poorly understood and purely subjective reasons, I feel more motivated to work on "anydns" than "dnsplug". It may be because python has "anydbm" already, or because "plug" just feels "klunky".

lesterpotter commented 5 years ago

This is related I think. It occurs on Windows 10 Pro and not Windows 10 Home.

using a python2.7 virtualenv

pip install ipaddr pip install pydns pip install pyspf

Then a simple snippet like the following works on both...

test.py

import spf print (spf.DNSLookup('example.com','txt')

python test.py

But on Windows 10 Pro (Microsoft Windows [Version 10.0.17763.437]) continue with... pip install dkimpy

python test.py Traceback (most recent call last): File "test.py", line 1, in import spf File "{venv}\lib\site-packages\spf.py", line 111, in if not hasattr(DNS.Type, 'SPF'): AttributeError: 'module' object has no attribute 'Type'

This does not happen with Windows 10 Home (10.0.17134.706)

python test.py { good results }

Is this a known issue? Any suggestions on tracking it down or working around it?

sdgathman / pyspf

dependency on py3dns causes namespace issues on OSX #2

using a python2.7 virtualenv

test.py