From 739aaa3ef9a292a6a2bafd12fa12c53bbb3af189 Mon Sep 17 00:00:00 2001 From: meejah Date: Mon, 29 Jan 2018 23:51:49 -0700 Subject: [PATCH] put create() methods in i2p_, tor_provider Also Provider -> _Provider, improve docs and update tests --- src/allmydata/client.py | 4 +- src/allmydata/introducer/server.py | 5 +- src/allmydata/node.py | 13 --- src/allmydata/test/test_i2p_provider.py | 143 ++++++++++++++---------- src/allmydata/test/test_tor_provider.py | 121 +++++++++++--------- src/allmydata/util/i2p_provider.py | 22 +++- src/allmydata/util/tor_provider.py | 22 +++- 7 files changed, 188 insertions(+), 142 deletions(-) diff --git a/src/allmydata/client.py b/src/allmydata/client.py index 1c5b68eb9..94f65808b 100644 --- a/src/allmydata/client.py +++ b/src/allmydata/client.py @@ -22,6 +22,8 @@ from allmydata.util.encodingutil import (get_filesystem_encoding, from_utf8_or_none) from allmydata.util.abbreviate import parse_abbreviated_size from allmydata.util.time_format import parse_duration, parse_date +from allmydata.util.i2p_provider import create as create_i2p_provider +from allmydata.util.tor_provider import create as create_tor_provider from allmydata.stats import StatsProvider from allmydata.history import History from allmydata.interfaces import IStatsProducer, SDMF_VERSION, MDMF_VERSION @@ -210,8 +212,6 @@ def create_client(basedir=u".", _client_factory=None): # this can/should be async # @defer.inlineCallbacks def create_client_from_config(basedir, config): - from twisted.internet import reactor - from allmydata.node import create_i2p_provider, create_tor_provider i2p_provider = create_i2p_provider(reactor, basedir, config) tor_provider = create_tor_provider(reactor, basedir, config) handlers = create_connection_handlers(reactor, basedir, config, i2p_provider, tor_provider) diff --git a/src/allmydata/introducer/server.py b/src/allmydata/introducer/server.py index 54309b8fa..f62548ebb 100644 --- a/src/allmydata/introducer/server.py +++ b/src/allmydata/introducer/server.py @@ -7,6 +7,10 @@ from foolscap.api import Referenceable import allmydata from allmydata import node from allmydata.util import log, rrefutil +from allmydata.util import fileutil +from allmydata.util.fileutil import abspath_expanduser_unicode +from allmydata.util.i2p_provider import create as create_i2p_provider +from allmydata.util.tor_provider import create as create_tor_provider from allmydata.introducer.interfaces import \ RIIntroducerPublisherAndSubscriberService_v2 from allmydata.introducer.common import unsign_from_foolscap, \ @@ -48,7 +52,6 @@ def create_introducer(basedir=u"."): ) # XXX fix up imports etc (also: reactor) - from allmydata.node import create_i2p_provider, create_tor_provider i2p_provider = create_i2p_provider(reactor, basedir, config) tor_provider = create_tor_provider(reactor, basedir, config) diff --git a/src/allmydata/node.py b/src/allmydata/node.py index 4b8e0cf76..5503180fb 100644 --- a/src/allmydata/node.py +++ b/src/allmydata/node.py @@ -24,7 +24,6 @@ from allmydata.util.assertutil import _assert from allmydata.util.fileutil import abspath_expanduser_unicode from allmydata.util.encodingutil import get_filesystem_encoding, quote_output from allmydata.util import configutil -from allmydata.util import i2p_provider, tor_provider def _common_config_sections(): return { @@ -430,18 +429,6 @@ def create_tub_options(config): return tub_options -def create_i2p_provider(reactor, basedir, config): - provider = i2p_provider.Provider(basedir, config, reactor) - provider.check_dest_config() - return provider - - -def create_tor_provider(reactor, basedir, config): - provider = tor_provider.Provider(basedir, config, reactor) - provider.check_onion_config() - return provider - - def _make_tcp_handler(): # this is always available from foolscap.connections.tcp import default diff --git a/src/allmydata/test/test_i2p_provider.py b/src/allmydata/test/test_i2p_provider.py index 53eaa8096..0f11c3d09 100644 --- a/src/allmydata/test/test_i2p_provider.py +++ b/src/allmydata/test/test_i2p_provider.py @@ -193,16 +193,15 @@ class FakeConfig(dict): class Provider(unittest.TestCase): def test_build(self): - i2p_provider.Provider(FakeConfig(), "reactor") + i2p_provider.create("reactor", FakeConfig()) def test_handler_disabled(self): - p = i2p_provider.Provider(FakeConfig(enabled=False), - "reactor") + p = i2p_provider.create("reactor", FakeConfig(enabled=False)) self.assertEqual(p.get_i2p_handler(), None) def test_handler_no_i2p(self): with mock_i2p(None): - p = i2p_provider.Provider(FakeConfig(), "reactor") + p = i2p_provider.create("reactor", FakeConfig()) self.assertEqual(p.get_i2p_handler(), None) def test_handler_sam_endpoint(self): @@ -213,8 +212,8 @@ class Provider(unittest.TestCase): reactor = object() with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(**{"sam.port": "ep_desc"}), - reactor) + p = i2p_provider.create(reactor, + FakeConfig(**{"sam.port": "ep_desc"})) with mock.patch("allmydata.util.i2p_provider.clientFromString", return_value=ep) as cfs: h = p.get_i2p_handler() @@ -229,8 +228,8 @@ class Provider(unittest.TestCase): reactor = object() with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(launch=True), - reactor) + p = i2p_provider.create(reactor, + FakeConfig(launch=True)) h = p.get_i2p_handler() self.assertIs(h, handler) i2p.launch.assert_called_with(i2p_configdir=None, i2p_binary=None) @@ -242,9 +241,9 @@ class Provider(unittest.TestCase): reactor = object() with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(launch=True, - **{"i2p.configdir": "configdir"}), - reactor) + p = i2p_provider.create(reactor, + FakeConfig(launch=True, + **{"i2p.configdir": "configdir"})) h = p.get_i2p_handler() self.assertIs(h, handler) i2p.launch.assert_called_with(i2p_configdir="configdir", i2p_binary=None) @@ -256,11 +255,11 @@ class Provider(unittest.TestCase): reactor = object() with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(launch=True, - **{"i2p.configdir": "configdir", - "i2p.executable": "myi2p", - }), - reactor) + p = i2p_provider.create(reactor, + FakeConfig(launch=True, + **{"i2p.configdir": "configdir", + "i2p.executable": "myi2p", + })) h = p.get_i2p_handler() self.assertIs(h, handler) i2p.launch.assert_called_with(i2p_configdir="configdir", i2p_binary="myi2p") @@ -272,8 +271,8 @@ class Provider(unittest.TestCase): reactor = object() with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(**{"i2p.configdir": "configdir"}), - reactor) + p = i2p_provider.create(reactor, + FakeConfig(**{"i2p.configdir": "configdir"})) h = p.get_i2p_handler() i2p.local_i2p.assert_called_with("configdir") self.assertIs(h, handler) @@ -285,7 +284,7 @@ class Provider(unittest.TestCase): reactor = object() with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(), reactor) + p = i2p_provider.create(reactor, FakeConfig()) h = p.get_i2p_handler() self.assertIs(h, handler) i2p.default.assert_called_with(reactor, keyfile=None) @@ -304,14 +303,14 @@ class ProviderListener(unittest.TestCase): privkeyfile = os.path.join("private", "i2p_dest.privkey") with mock_i2p(i2p): - p = i2p_provider.Provider(FakeConfig(**{ - "i2p.configdir": "configdir", - "sam.port": "good:port", - "dest": "true", - "dest.port": "3457", - "dest.private_key_file": privkeyfile, - }), - reactor) + p = i2p_provider.create(reactor, + FakeConfig(**{ + "i2p.configdir": "configdir", + "sam.port": "good:port", + "dest": "true", + "dest.port": "3457", + "dest.private_key_file": privkeyfile, + })) endpoint_or_description = p.get_listener() self.assertEqual(endpoint_or_description, "i2p:%s:3457:api=SAM:apiEndpoint=good\\:port" % privkeyfile) @@ -321,55 +320,75 @@ class Provider_CheckI2PConfig(unittest.TestCase): # default config doesn't start an I2P service, so it should be # happy both with and without txi2p - p = i2p_provider.Provider(FakeConfig(), "reactor") + p = i2p_provider.create("reactor", FakeConfig()) p.check_dest_config() with mock_txi2p(None): - p = i2p_provider.Provider(FakeConfig(), "reactor") + p = i2p_provider.create("reactor", FakeConfig()) p.check_dest_config() def test_no_txi2p(self): with mock_txi2p(None): - p = i2p_provider.Provider(FakeConfig(dest=True), - "reactor") - e = self.assertRaises(ValueError, p.check_dest_config) - self.assertEqual(str(e), "Cannot create I2P Destination without txi2p. " - "Please 'pip install tahoe-lafs[i2p]' to fix.") + with self.assertRaises(ValueError) as ctx: + i2p_provider.create("reactor", FakeConfig(dest=True)) + self.assertEqual( + str(ctx.exception), + "Cannot create I2P Destination without txi2p. " + "Please 'pip install tahoe-lafs[i2p]' to fix." + ) def test_no_launch_no_control(self): - p = i2p_provider.Provider(FakeConfig(dest=True), "reactor") - e = self.assertRaises(ValueError, p.check_dest_config) - self.assertEqual(str(e), "[i2p] dest = true, but we have neither " - "sam.port= nor launch=true nor configdir=") + with self.assertRaises(ValueError) as ctx: + i2p_provider.create("reactor", FakeConfig(dest=True)) + self.assertEqual( + str(ctx.exception), + "[i2p] dest = true, but we have neither " + "sam.port= nor launch=true nor configdir=" + ) def test_missing_keys(self): - p = i2p_provider.Provider(FakeConfig(dest=True, - **{"sam.port": "x", - }), "reactor") - e = self.assertRaises(ValueError, p.check_dest_config) - self.assertEqual(str(e), "[i2p] dest = true, " + with self.assertRaises(ValueError) as ctx: + i2p_provider.create("reactor", + FakeConfig( + dest=True, + **{"sam.port": "x", + } + )) + self.assertEqual(str(ctx.exception), "[i2p] dest = true, " "but dest.port= is missing") - p = i2p_provider.Provider(FakeConfig(dest=True, - **{"sam.port": "x", - "dest.port": "y", - }), "reactor") - e = self.assertRaises(ValueError, p.check_dest_config) - self.assertEqual(str(e), "[i2p] dest = true, " - "but dest.private_key_file= is missing") + with self.assertRaises(ValueError) as ctx: + i2p_provider.create("reactor", + FakeConfig(dest=True, + **{"sam.port": "x", + "dest.port": "y", + })) + self.assertEqual( + str(ctx.exception), + "[i2p] dest = true, " + "but dest.private_key_file= is missing" + ) def test_launch_not_implemented(self): - p = i2p_provider.Provider(FakeConfig(dest=True, launch=True, - **{"dest.port": "x", - "dest.private_key_file": "y", - }), "reactor") - e = self.assertRaises(NotImplementedError, p.check_dest_config) - self.assertEqual(str(e), "[i2p] launch is under development.") + with self.assertRaises(NotImplementedError) as ctx: + i2p_provider.create("reactor", + FakeConfig(dest=True, launch=True, + **{"dest.port": "x", + "dest.private_key_file": "y", + })) + self.assertEqual( + str(ctx.exception), + "[i2p] launch is under development." + ) def test_ok(self): - p = i2p_provider.Provider(FakeConfig(dest=True, - **{"sam.port": "x", - "dest.port": "y", - "dest.private_key_file": "z", - }), "reactor") - p.check_dest_config() + i2p_provider.create( + "reactor", + FakeConfig( + dest=True, **{ + "sam.port": "x", + "dest.port": "y", + "dest.private_key_file": "z", + } + ) + ) diff --git a/src/allmydata/test/test_tor_provider.py b/src/allmydata/test/test_tor_provider.py index 2cb5948a1..68f3f09f0 100644 --- a/src/allmydata/test/test_tor_provider.py +++ b/src/allmydata/test/test_tor_provider.py @@ -286,22 +286,20 @@ class EmptyContext(object): class Provider(unittest.TestCase): def test_build(self): - tor_provider.Provider(FakeConfig(), "reactor") + tor_provider.create("reactor", FakeConfig()) def test_handler_disabled(self): - p = tor_provider.Provider(FakeConfig(enabled=False), - "reactor") + p = tor_provider.create("reactor", FakeConfig(enabled=False)) self.assertEqual(p.get_tor_handler(), None) def test_handler_no_tor(self): with mock_tor(None): - p = tor_provider.Provider(FakeConfig(), "reactor") + p = tor_provider.create("reactor", FakeConfig()) self.assertEqual(p.get_tor_handler(), None) def test_handler_launch_no_txtorcon(self): with mock_txtorcon(None): - p = tor_provider.Provider(FakeConfig(launch=True), - "reactor") + p = tor_provider.create("reactor", FakeConfig(launch=True)) self.assertEqual(p.get_tor_handler(), None) @defer.inlineCallbacks @@ -314,8 +312,7 @@ class Provider(unittest.TestCase): tor.add_context = mock.Mock(return_value=EmptyContext()) with mock_tor(tor): with mock_txtorcon(txtorcon): - p = tor_provider.Provider(FakeConfig(launch=True), - reactor) + p = tor_provider.create(reactor, FakeConfig(launch=True)) h = p.get_tor_handler() self.assertIs(h, handler) tor.control_endpoint_maker.assert_called_with(p._make_control_endpoint, @@ -360,8 +357,8 @@ class Provider(unittest.TestCase): reactor = object() with mock_tor(tor): - p = tor_provider.Provider(FakeConfig(**{"socks.port": "ep_desc"}), - reactor) + p = tor_provider.create(reactor, + FakeConfig(**{"socks.port": "ep_desc"})) with mock.patch("allmydata.util.tor_provider.clientFromString", cfs): h = p.get_tor_handler() cfs.assert_called_with(reactor, "ep_desc") @@ -377,8 +374,8 @@ class Provider(unittest.TestCase): reactor = object() with mock_tor(tor): - p = tor_provider.Provider(FakeConfig(**{"control.port": "ep_desc"}), - reactor) + p = tor_provider.create(reactor, + FakeConfig(**{"control.port": "ep_desc"})) with mock.patch("allmydata.util.tor_provider.clientFromString", cfs): h = p.get_tor_handler() self.assertIs(h, handler) @@ -391,7 +388,7 @@ class Provider(unittest.TestCase): tor.default_socks = mock.Mock(return_value=handler) with mock_tor(tor): - p = tor_provider.Provider(FakeConfig(), "reactor") + p = tor_provider.create("reactor", FakeConfig()) h = p.get_tor_handler() self.assertIs(h, handler) tor.default_socks.assert_called_with() @@ -408,8 +405,8 @@ class ProviderListener(unittest.TestCase): reactor = object() with mock_tor(tor): - p = tor_provider.Provider(FakeConfig(**{"onion.local_port": "321"}), - reactor) + p = tor_provider.create(reactor, + FakeConfig(**{"onion.local_port": "321"})) fake_ep = object() with mock.patch("allmydata.util.tor_provider.TCP4ServerEndpoint", return_value=fake_ep) as e: @@ -423,62 +420,78 @@ class Provider_CheckOnionConfig(unittest.TestCase): # default config doesn't start an onion service, so it should be # happy both with and without txtorcon - p = tor_provider.Provider(FakeConfig(), "reactor") + p = tor_provider.create("reactor", FakeConfig()) p.check_onion_config() with mock_txtorcon(None): - p = tor_provider.Provider(FakeConfig(), "reactor") + p = tor_provider.create("reactor", FakeConfig()) p.check_onion_config() def test_no_txtorcon(self): with mock_txtorcon(None): - p = tor_provider.Provider(FakeConfig(onion=True), - "reactor") - e = self.assertRaises(ValueError, p.check_onion_config) - self.assertEqual(str(e), "Cannot create onion without txtorcon. " - "Please 'pip install tahoe-lafs[tor]' to fix.") + with self.assertRaises(ValueError) as ctx: + tor_provider.create("reactor", FakeConfig(onion=True)) + self.assertEqual( + str(ctx.exception), + "Cannot create onion without txtorcon. " + "Please 'pip install tahoe-lafs[tor]' to fix." + ) def test_no_launch_no_control(self): - p = tor_provider.Provider(FakeConfig(onion=True), "reactor") - e = self.assertRaises(ValueError, p.check_onion_config) - self.assertEqual(str(e), "[tor] onion = true, but we have neither " - "launch=true nor control.port=") + with self.assertRaises(ValueError) as ctx: + tor_provider.create("reactor", FakeConfig(onion=True)) + self.assertEqual( + str(ctx.exception), + "[tor] onion = true, but we have neither " + "launch=true nor control.port=" + ) - def test_missing_keys(self): - p = tor_provider.Provider(FakeConfig(onion=True, - launch=True), "reactor") - e = self.assertRaises(ValueError, p.check_onion_config) - self.assertEqual(str(e), "[tor] onion = true, " - "but onion.local_port= is missing") + def test_missing_keys0(self): + with self.assertRaises(ValueError) as ctx: + tor_provider.create("reactor", FakeConfig(onion=True, launch=True)) + self.assertEqual( + str(ctx.exception), + "[tor] onion = true, " + "but onion.local_port= is missing" + ) - p = tor_provider.Provider(FakeConfig(onion=True, launch=True, - **{"onion.local_port": "x", - }), "reactor") - e = self.assertRaises(ValueError, p.check_onion_config) - self.assertEqual(str(e), "[tor] onion = true, " - "but onion.external_port= is missing") + def test_missing_keys1(self): + with self.assertRaises(ValueError) as ctx: + tor_provider.create("reactor", + FakeConfig(onion=True, launch=True, + **{"onion.local_port": "x", + })) + self.assertEqual( + str(ctx.exception), + "[tor] onion = true, but onion.external_port= is missing" + ) - p = tor_provider.Provider(FakeConfig(onion=True, launch=True, - **{"onion.local_port": "x", - "onion.external_port": "y", - }), "reactor") - e = self.assertRaises(ValueError, p.check_onion_config) - self.assertEqual(str(e), "[tor] onion = true, " - "but onion.private_key_file= is missing") + def test_missing_keys2(self): + with self.assertRaises(ValueError) as ctx: + tor_provider.create("reactor", + FakeConfig(onion=True, launch=True, + **{"onion.local_port": "x", + "onion.external_port": "y", + })) + self.assertEqual( + str(ctx.exception), + "[tor] onion = true, but onion.private_key_file= is missing" + ) def test_ok(self): - p = tor_provider.Provider(FakeConfig(onion=True, launch=True, - **{"onion.local_port": "x", - "onion.external_port": "y", - "onion.private_key_file": "z", - }), "reactor") + p = tor_provider.create("reactor", + FakeConfig(onion=True, launch=True, + **{"onion.local_port": "x", + "onion.external_port": "y", + "onion.private_key_file": "z", + })) p.check_onion_config() class Provider_Service(unittest.TestCase): def test_no_onion(self): reactor = object() - p = tor_provider.Provider(FakeConfig(onion=False), reactor) - with mock.patch("allmydata.util.tor_provider.Provider._start_onion") as s: + p = tor_provider.create(reactor, FakeConfig(onion=False)) + with mock.patch("allmydata.util.tor_provider._Provider._start_onion") as s: p.startService() self.assertEqual(s.mock_calls, []) self.assertEqual(p.running, True) @@ -502,7 +515,7 @@ class Provider_Service(unittest.TestCase): txtorcon = mock.Mock() with mock_txtorcon(txtorcon): - p = tor_provider.Provider(cfg, reactor) + p = tor_provider.create(reactor, basedir, cfg) tor_state = mock.Mock() tor_state.protocol = object() ehs = mock.Mock() @@ -543,7 +556,7 @@ class Provider_Service(unittest.TestCase): txtorcon = mock.Mock() with mock_txtorcon(txtorcon): - p = tor_provider.Provider(cfg, reactor) + p = tor_provider.create(reactor, basedir, cfg) tor_state = mock.Mock() tor_state.protocol = object() txtorcon.build_tor_connection = mock.Mock(return_value=tor_state) diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py index be706efcb..c9cd69ed3 100644 --- a/src/allmydata/util/i2p_provider.py +++ b/src/allmydata/util/i2p_provider.py @@ -7,6 +7,22 @@ from twisted.internet.endpoints import clientFromString from twisted.internet.error import ConnectionRefusedError, ConnectError from twisted.application import service + +def create(reactor, basedir, config): + """ + Create a new Provider service (this is an IService so must be + hooked up to a parent or otherwise started). + + If foolscap.connections.i2p or txi2p are not installed, then + Provider.get_i2p_handler() will return None. If 'tahoe.cfg' wants + to start an I2P Destination too, then this `create()` method will + throw a nice error (and startService will throw an ugly error). + """ + provider = _Provider(basedir, config, reactor) + provider.check_dest_config() + return provider + + def _import_i2p(): # this exists to be overridden by unit tests try: @@ -118,12 +134,8 @@ def create_config(reactor, cli_config): returnValue((tahoe_config_i2p, i2p_port, i2p_location)) -# we can always create a Provider. If foolscap.connections.i2p or txi2p -# are not installed, then get_i2p_handler() will return None. If tahoe.cfg -# wants to start an I2P Destination too, then check_dest_config() will throw -# a nice error, and startService will throw an ugly error. -class Provider(service.MultiService): +class _Provider(service.MultiService): def __init__(self, config, reactor): service.MultiService.__init__(self) self._config = config diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py index bfdde3643..dc9e411ce 100644 --- a/src/allmydata/util/tor_provider.py +++ b/src/allmydata/util/tor_provider.py @@ -10,6 +10,22 @@ from twisted.application import service from .observer import OneShotObserverList from .iputil import allocate_tcp_port + +def create(reactor, basedir, config): + """ + Create a new _Provider service (this is an IService so must be + hooked up to a parent or otherwise started). + + If foolscap.connections.tor or txtorcon are not installed, then + Provider.get_tor_handler() will return None. If tahoe.cfg wants + to start an onion service too, then this `create()` method will + throw a nice error (and startService will throw an ugly error). + """ + provider = _Provider(basedir, config, reactor) + provider.check_onion_config() + return provider + + def _import_tor(): # this exists to be overridden by unit tests try: @@ -192,12 +208,8 @@ def create_config(reactor, cli_config): returnValue((tahoe_config_tor, tor_port, tor_location)) -# we can always create a Provider. If foolscap.connections.tor or txtorcon -# are not installed, then get_tor_handler() will return None. If tahoe.cfg -# wants to start an onion service too, then check_onion_config() will throw a -# nice error, and startService will throw an ugly error. -class Provider(service.MultiService): +class _Provider(service.MultiService): def __init__(self, config, reactor): service.MultiService.__init__(self) self._config = config