Refactor create_main_tub to make testing tub location logic easier

Then take advantage of this and simplify the tub location logic test
This commit is contained in:
Jean-Paul Calderone 2020-12-11 10:34:30 -05:00
parent d29d9c57e7
commit 733223c8d7
2 changed files with 65 additions and 58 deletions

View File

@ -722,6 +722,10 @@ def _convert_tub_port(s):
return us
class PortAssignmentRequired(Exception):
pass
def _tub_portlocation(config, get_local_addresses_sync, allocate_tcp_port):
"""
Figure out the network location of the main tub for some configuration.
@ -778,7 +782,7 @@ def _tub_portlocation(config, get_local_addresses_sync, allocate_tcp_port):
for port in tubport.split(","):
if port in ("0", "tcp:0"):
raise ValueError("tub.port cannot be 0: you must choose")
raise PortAssignmentRequired()
if cfg_location is None:
cfg_location = "AUTO"
@ -821,6 +825,38 @@ def _tub_portlocation(config, get_local_addresses_sync, allocate_tcp_port):
return tubport, location
def set_tub_locations(i2p_provider, tor_provider, tub, portlocation):
"""
Assign a Tub its listener locations.
:param i2p_provider: See ``allmydata.util.i2p_provider.create``.
:param tor_provider: See ``allmydata.util.tor_provider.create``.
"""
if portlocation is None:
log.msg("Tub is not listening")
else:
tubport, location = portlocation
for port in tubport.split(","):
if port == "listen:i2p":
# the I2P provider will read its section of tahoe.cfg and
# return either a fully-formed Endpoint, or a descriptor
# that will create one, so we don't have to stuff all the
# options into the tub.port string (which would need a lot
# of escaping)
port_or_endpoint = i2p_provider.get_listener()
elif port == "listen:tor":
port_or_endpoint = tor_provider.get_listener()
else:
port_or_endpoint = port
# Foolscap requires native strings:
if isinstance(port_or_endpoint, (bytes, str)):
port_or_endpoint = ensure_str(port_or_endpoint)
tub.listenOn(port_or_endpoint)
tub.setLocation(location)
log.msg("Tub location set to %s" % (location,))
# the Tub is now ready for tub.registerReference()
def create_main_tub(config, tub_options,
default_connection_handlers, foolscap_connection_handlers,
i2p_provider, tor_provider,
@ -851,34 +887,17 @@ def create_main_tub(config, tub_options,
iputil.allocate_tcp_port,
)
certfile = config.get_private_path("node.pem") # FIXME? "node.pem" was the CERTFILE option/thing
tub = create_tub(tub_options, default_connection_handlers, foolscap_connection_handlers,
handler_overrides=handler_overrides, certFile=certfile)
if portlocation:
tubport, location = portlocation
for port in tubport.split(","):
if port == "listen:i2p":
# the I2P provider will read its section of tahoe.cfg and
# return either a fully-formed Endpoint, or a descriptor
# that will create one, so we don't have to stuff all the
# options into the tub.port string (which would need a lot
# of escaping)
port_or_endpoint = i2p_provider.get_listener()
elif port == "listen:tor":
port_or_endpoint = tor_provider.get_listener()
else:
port_or_endpoint = port
# Foolscap requires native strings:
if isinstance(port_or_endpoint, (bytes, str)):
port_or_endpoint = ensure_str(port_or_endpoint)
tub.listenOn(port_or_endpoint)
tub.setLocation(location)
log.msg("Tub location set to %s" % (location,))
# the Tub is now ready for tub.registerReference()
else:
log.msg("Tub is not listening")
# FIXME? "node.pem" was the CERTFILE option/thing
certfile = config.get_private_path("node.pem")
tub = create_tub(
tub_options,
default_connection_handlers,
foolscap_connection_handlers,
handler_overrides=handler_overrides,
certFile=certfile,
)
set_tub_locations(i2p_provider, tor_provider, tub, portlocation)
return tub

View File

@ -40,7 +40,9 @@ from foolscap.connections.tcp import default as make_tcp_handler
from twisted.application import service
from allmydata.node import (
PortAssignmentRequired,
PrivacyError,
set_tub_locations,
create_tub_options,
create_main_tub,
create_node_dir,
@ -528,6 +530,22 @@ class TestMissingPorts(unittest.TestCase):
def _alloc_port(self):
return 999
def test_listen_on_zero(self):
"""
``set_tub_locations`` raises ``PortAssignmentRequired`` called with a
listen address including port 0.
"""
config_data = (
"[node]\n"
"tub.port = tcp:0\n"
)
config = config_from_string(self.basedir, "portnum", config_data)
# XXX The implementation has some shortcomings. For example, how
# about tcp:localhost:0?
with self.assertRaises(PortAssignmentRequired):
_tub_portlocation(config, None, None)
def test_parsing_tcp(self):
"""
When ``tub.port`` is given and ``tub.location`` is **AUTO** the port
@ -767,36 +785,6 @@ class FakeTub(object):
class Listeners(unittest.TestCase):
def test_listen_on_zero(self):
"""
Trying to listen on port 0 should be an error
"""
basedir = self.mktemp()
create_node_dir(basedir, "testing")
with open(os.path.join(basedir, "tahoe.cfg"), "w") as f:
f.write(BASE_CONFIG)
f.write("[node]\n")
f.write("tub.port = tcp:0\n")
f.write("tub.location = AUTO\n")
config = client.read_config(basedir, "client.port")
fch = {"tcp": make_tcp_handler()}
dfh = create_default_connection_handlers(
None,
config,
fch,
)
tub_options = create_tub_options(config)
t = FakeTub()
with mock.patch("allmydata.node.Tub", return_value=t):
with self.assertRaises(ValueError) as ctx:
create_main_tub(config, tub_options, dfh, fch, None, None)
self.assertIn(
"you must choose",
str(ctx.exception),
)
# Randomly allocate a couple distinct port numbers to try out. The test
# never actually binds these port numbers so we don't care if they're "in
# use" on the system or not. We just want a couple distinct values we can