Opened 11 months ago

Last modified 11 months ago

#6840 enhancement new

t.n.client.Resolver should allow alternative DNS protocol factories to be used

Reported by: rwall Owned by: rwall
Priority: normal Milestone: EDNS0
Component: names Keywords:
Cc: Branch: branches/resolver-protocol-factory-6840
(diff, github, buildbot, log)
Author: rwall Launchpad Bug:

Description

client.Resolver is currently hard coded to use dns.DNSDatagramProtocol and
dns.DNSProtocol (TCP) via client.DNSClientFactory.

This makes it difficult to integrate with the EDNS message parser from #5675 and potential new EDNS protocols which may arise from #6839

Allow a caller to supply alternative stream and datagram protocol factories.

Requirements:

  1. client.Resolver constructor will accept a "datagramProtocolFactory" and "streamProtocolFactory" arguments, which will default to existing factories but which will allow dns.EDNSDatagramProtocol to be supplied instead.

See:wiki:EDNS0#client.Resolverprotocoloverrideoptions

Change History (7)

comment:1 Changed 11 months ago by rwall

  • Author set to rwall
  • Branch set to branches/resolver-protocol-factory-6840

(In [40755]) Branching to 'resolver-protocol-factory-6840'

comment:2 Changed 11 months ago by rwall

  • Keywords review added

Ready for review in log:branches/resolver-protocol-factory-6840

  • Added parameterized datagram and stream protocol factories
  • Also added a parameterized AXFRController factory -- purely for testing, but it may be a useful option for people wanting to customise AXFR handling.

Build Results:

comment:3 in reply to: ↑ description ; follow-up: Changed 11 months ago by thijs

Replying to rwall:

client.Resolver is currently hard coded to use dns.DNSDatagramProtocol and
dns.DNSProtocol (TCP) via client.DNSClientFactory.

This adds datagramProtocolFactory, streamProtocolFactory, and axfrControllerFactory arguments.. is this necessary or is it possible to move these extra arguments elsewhere, because it seems you're moving one hardcoded thing to another. These new arguments also aren't documented, what's a axfrControllerFactory?

comment:4 in reply to: ↑ 3 Changed 11 months ago by rwall

Replying to thijs:

Replying to rwall:

client.Resolver is currently hard coded to use dns.DNSDatagramProtocol and
dns.DNSProtocol (TCP) via client.DNSClientFactory.

This adds datagramProtocolFactory, streamProtocolFactory, and axfrControllerFactory arguments.. is this necessary or is it possible to move these extra arguments elsewhere, because it seems you're moving one hardcoded thing to another. These new arguments also aren't documented, what's a axfrControllerFactory?

Hey Thijs,

I hope it's going to make it easy to create an EDNSResolver class which wraps rather than subclasses Resolver. To avoid eg:

I could just make the default factories public class attributes, and expect the user to modify the instance attribute if they need to. But I'd have to refactor a bit, because the streamProtocolFactory gets called in the constructor.

Or I could add a new classmethod as an alternative constructor?

What do you think? What is your main objection to adding the new arguments in init?

Thanks for spotting the missing docstrings. I'll add those.

-RichardW.

comment:5 Changed 11 months ago by rwall

(In [40779]) add missing docstrings. refs #6840

comment:6 Changed 11 months ago by rwall

Here's a branch which combines all the EDNS and DNSSEC changes that
are currently up for review.

I've hacked together some new EDNS versions of protocol, client and
server. Implemented as proxies for the existing classes.

I've also added some examples which demo the new APIs.

  • twistd -noy doc/names/examples/edns_server.tac.py
  • TARGET=127.0.0.1,10053,twistd ./bin/trial

doc/names/examples/test_edns_compliance.py

  • python doc/names/examples/txdig.py -s 8.8.8.8 com DNSKEY

Hopefully this gives some context for the changes in this branch.

comment:7 Changed 11 months ago by tom.prince

  • Keywords review removed
  • Owner set to rwall

What will the difference between the current factories and the new factories? Will they just pass a different messageFactory to the create protocols? If that is the case, perhaps those factories could be changed to taken an optional messageFactory argument, and Resolver could pass that on.

But, what is the final use interface going to look like? The user probably just wants to say "use EDNS". (They certainly *don't* want to say, use the edns version of the 'datagramProtocolFactory, streamProtocolFactory and axfrControllerFactory`.)

I'm not sure what the final interface for client.Resolver should be, but DNSClientFactory will probably want to take a messageFactory argument however it ends up being passed.

I think perhaps this ticket should wait until it is clearer what the interface should be.

Note: See TracTickets for help on using tickets.