From 931bdef2a2b6fe6ca5d124911c3bf5cc73dd65a9 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:20:54 -0400 Subject: [PATCH 1/6] Get rid of the old implementation and related unused code Also put in the new implementation, though now it needs tests because *there were no direct tests for the old one*. --- setup.py | 3 + src/allmydata/test/test_iputil.py | 167 +------------------------ src/allmydata/util/iputil.py | 197 +++--------------------------- 3 files changed, 22 insertions(+), 345 deletions(-) diff --git a/setup.py b/setup.py index 874cc1258..212e6c369 100644 --- a/setup.py +++ b/setup.py @@ -126,6 +126,9 @@ install_requires = [ # Support for Python 3 transition "future >= 0.18.2", + # Discover local network configuration + "netifaces", + # Utility code: "pyutil >= 3.3.0", diff --git a/src/allmydata/test/test_iputil.py b/src/allmydata/test/test_iputil.py index f403de35b..6f6100410 100644 --- a/src/allmydata/test/test_iputil.py +++ b/src/allmydata/test/test_iputil.py @@ -13,7 +13,7 @@ from future.utils import PY2, native_str if PY2: from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 -import re, errno, subprocess, os, socket +import re, os, socket import gc from twisted.trial import unittest @@ -23,171 +23,6 @@ from tenacity import retry, stop_after_attempt from foolscap.api import Tub from allmydata.util import iputil, gcutil -import allmydata.test.common_util as testutil -from allmydata.util.namespace import Namespace - - -DOTTED_QUAD_RE=re.compile(r"^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$") - -# Mock output from subprocesses should be bytes, that's what happens on both -# Python 2 and Python 3: -MOCK_IPADDR_OUTPUT = b"""\ -1: lo: mtu 16436 qdisc noqueue state UNKNOWN \n\ - link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 - inet 127.0.0.1/8 scope host lo - inet6 ::1/128 scope host \n\ - valid_lft forever preferred_lft forever -2: eth1: mtu 1500 qdisc pfifo_fast state UP qlen 1000 - link/ether d4:3d:7e:01:b4:3e brd ff:ff:ff:ff:ff:ff - inet 192.168.0.6/24 brd 192.168.0.255 scope global eth1 - inet6 fe80::d63d:7eff:fe01:b43e/64 scope link \n\ - valid_lft forever preferred_lft forever -3: wlan0: mtu 1500 qdisc mq state UP qlen 1000 - link/ether 90:f6:52:27:15:0a brd ff:ff:ff:ff:ff:ff - inet 192.168.0.2/24 brd 192.168.0.255 scope global wlan0 - inet6 fe80::92f6:52ff:fe27:150a/64 scope link \n\ - valid_lft forever preferred_lft forever -""" - -MOCK_IFCONFIG_OUTPUT = b"""\ -eth1 Link encap:Ethernet HWaddr d4:3d:7e:01:b4:3e \n\ - inet addr:192.168.0.6 Bcast:192.168.0.255 Mask:255.255.255.0 - inet6 addr: fe80::d63d:7eff:fe01:b43e/64 Scope:Link - UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 - RX packets:154242234 errors:0 dropped:0 overruns:0 frame:0 - TX packets:155461891 errors:0 dropped:0 overruns:0 carrier:0 - collisions:0 txqueuelen:1000 \n\ - RX bytes:84367213640 (78.5 GiB) TX bytes:73401695329 (68.3 GiB) - Interrupt:20 Memory:f4f00000-f4f20000 \n\ - -lo Link encap:Local Loopback \n\ - inet addr:127.0.0.1 Mask:255.0.0.0 - inet6 addr: ::1/128 Scope:Host - UP LOOPBACK RUNNING MTU:16436 Metric:1 - RX packets:27449267 errors:0 dropped:0 overruns:0 frame:0 - TX packets:27449267 errors:0 dropped:0 overruns:0 carrier:0 - collisions:0 txqueuelen:0 \n\ - RX bytes:192643017823 (179.4 GiB) TX bytes:192643017823 (179.4 GiB) - -wlan0 Link encap:Ethernet HWaddr 90:f6:52:27:15:0a \n\ - inet addr:192.168.0.2 Bcast:192.168.0.255 Mask:255.255.255.0 - inet6 addr: fe80::92f6:52ff:fe27:150a/64 Scope:Link - UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 - RX packets:12352750 errors:0 dropped:0 overruns:0 frame:0 - TX packets:4501451 errors:0 dropped:0 overruns:0 carrier:0 - collisions:0 txqueuelen:1000 \n\ - RX bytes:3916475942 (3.6 GiB) TX bytes:458353654 (437.1 MiB) -""" - -# This is actually from a VirtualBox VM running XP. -MOCK_ROUTE_OUTPUT = b"""\ -=========================================================================== -Interface List -0x1 ........................... MS TCP Loopback interface -0x2 ...08 00 27 c3 80 ad ...... AMD PCNET Family PCI Ethernet Adapter - Packet Scheduler Miniport -=========================================================================== -=========================================================================== -Active Routes: -Network Destination Netmask Gateway Interface Metric - 0.0.0.0 0.0.0.0 10.0.2.2 10.0.2.15 20 - 10.0.2.0 255.255.255.0 10.0.2.15 10.0.2.15 20 - 10.0.2.15 255.255.255.255 127.0.0.1 127.0.0.1 20 - 10.255.255.255 255.255.255.255 10.0.2.15 10.0.2.15 20 - 127.0.0.0 255.0.0.0 127.0.0.1 127.0.0.1 1 - 224.0.0.0 240.0.0.0 10.0.2.15 10.0.2.15 20 - 255.255.255.255 255.255.255.255 10.0.2.15 10.0.2.15 1 -Default Gateway: 10.0.2.2 -=========================================================================== -Persistent Routes: - None -""" - -UNIX_TEST_ADDRESSES = set(["127.0.0.1", "192.168.0.6", "192.168.0.2", "192.168.0.10"]) -WINDOWS_TEST_ADDRESSES = set(["127.0.0.1", "10.0.2.15", "192.168.0.10"]) -CYGWIN_TEST_ADDRESSES = set(["127.0.0.1", "192.168.0.10"]) - - -class FakeProcess(object): - def __init__(self, output, err): - self.output = output - self.err = err - def communicate(self): - return (self.output, self.err) - - -class ListAddresses(testutil.SignalMixin, unittest.TestCase): - def test_get_local_ip_for(self): - addr = iputil.get_local_ip_for('127.0.0.1') - self.failUnless(DOTTED_QUAD_RE.match(addr)) - # Bytes can be taken as input: - bytes_addr = iputil.get_local_ip_for(b'127.0.0.1') - self.assertEqual(addr, bytes_addr) - # The output is a native string: - self.assertIsInstance(addr, native_str) - - def test_list_async(self): - d = iputil.get_local_addresses_async() - def _check(addresses): - self.failUnlessIn("127.0.0.1", addresses) - self.failIfIn("0.0.0.0", addresses) - d.addCallbacks(_check) - return d - # David A.'s OpenSolaris box timed out on this test one time when it was at 2s. - test_list_async.timeout=4 - - def _test_list_async_mock(self, command, output, expected): - ns = Namespace() - ns.first = True - - def call_Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, - preexec_fn=None, close_fds=False, shell=False, cwd=None, env=None, - universal_newlines=False, startupinfo=None, creationflags=0): - if ns.first: - ns.first = False - e = OSError("EINTR") - e.errno = errno.EINTR - raise e - elif os.path.basename(args[0]) == command: - return FakeProcess(output, "") - else: - e = OSError("[Errno 2] No such file or directory") - e.errno = errno.ENOENT - raise e - self.patch(subprocess, 'Popen', call_Popen) - self.patch(os.path, 'isfile', lambda x: True) - - def call_get_local_ip_for(target): - if target in ("localhost", "127.0.0.1"): - return "127.0.0.1" - else: - return "192.168.0.10" - self.patch(iputil, 'get_local_ip_for', call_get_local_ip_for) - - def call_which(name): - return [name] - self.patch(iputil, 'which', call_which) - - d = iputil.get_local_addresses_async() - def _check(addresses): - self.failUnlessEquals(set(addresses), set(expected)) - d.addCallbacks(_check) - return d - - def test_list_async_mock_ip_addr(self): - self.patch(iputil, 'platform', "linux2") - return self._test_list_async_mock("ip", MOCK_IPADDR_OUTPUT, UNIX_TEST_ADDRESSES) - - def test_list_async_mock_ifconfig(self): - self.patch(iputil, 'platform', "linux2") - return self._test_list_async_mock("ifconfig", MOCK_IFCONFIG_OUTPUT, UNIX_TEST_ADDRESSES) - - def test_list_async_mock_route(self): - self.patch(iputil, 'platform', "win32") - return self._test_list_async_mock("route.exe", MOCK_ROUTE_OUTPUT, WINDOWS_TEST_ADDRESSES) - - def test_list_async_mock_cygwin(self): - self.patch(iputil, 'platform', "cygwin") - return self._test_list_async_mock(None, None, CYGWIN_TEST_ADDRESSES) class ListenOnUsed(unittest.TestCase): diff --git a/src/allmydata/util/iputil.py b/src/allmydata/util/iputil.py index bd5ea7e78..cbb4922e4 100644 --- a/src/allmydata/util/iputil.py +++ b/src/allmydata/util/iputil.py @@ -13,19 +13,19 @@ from future.utils import PY2, native_str if PY2: from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 -import os, re, socket, subprocess, errno -from sys import platform +import os, socket from zope.interface import implementer import attr +from netifaces import ( + interfaces, + ifaddresses, +) + # from Twisted from twisted.python.reflect import requireModule -from twisted.internet import defer, threads, reactor -from twisted.internet.protocol import DatagramProtocol -from twisted.internet.error import CannotListenError -from twisted.python.procutils import which from twisted.python import log from twisted.internet.endpoints import AdoptedStreamServerEndpoint from twisted.internet.interfaces import ( @@ -101,180 +101,21 @@ except ImportError: # since one might be shadowing the other. This hack appeases pyflakes. increase_rlimits = _increase_rlimits + def get_local_addresses_sync(): """ - Return a list of IPv4 addresses (as dotted-quad native strings) that are - currently configured on this host, sorted in descending order of how likely - we think they are to work. + Get locally assigned addresses as dotted-quad native strings. + + :return [str]: A list of IPv4 addresses which are assigned to interfaces + on the local system. """ - return [native_str(a) for a in _synchronously_find_addresses_via_config()] - -def get_local_addresses_async(target="198.41.0.4"): # A.ROOT-SERVERS.NET - """ - Return a Deferred that fires with a list of IPv4 addresses (as dotted-quad - native strings) that are currently configured on this host, sorted in - descending order of how likely we think they are to work. - - @param target: we want to learn an IP address they could try using to - connect to us; The default value is fine, but it might help if you - pass the address of a host that you are actually trying to be - reachable to. - """ - addresses = [] - local_ip = get_local_ip_for(target) - if local_ip is not None: - addresses.append(local_ip) - - if platform == "cygwin": - d = _cygwin_hack_find_addresses() - else: - d = _find_addresses_via_config() - - def _collect(res): - for addr in res: - if addr != "0.0.0.0" and not addr in addresses: - addresses.append(addr) - return addresses - d.addCallback(_collect) - d.addCallback(lambda addresses: [native_str(s) for s in addresses]) - return d - -def get_local_ip_for(target): - """Find out what our IP address is for use by a given target. - - @return: the IP address as a dotted-quad native string which could be used - to connect to us. It might work for them, it might not. If - there is no suitable address (perhaps we don't currently have an - externally-visible interface), this will return None. - """ - - try: - target_ipaddr = socket.gethostbyname(target) - except socket.gaierror: - # DNS isn't running, or somehow we encountered an error - - # note: if an interface is configured and up, but nothing is - # connected to it, gethostbyname("A.ROOT-SERVERS.NET") will take 20 - # seconds to raise socket.gaierror . This is synchronous and occurs - # for each node being started, so users of - # test.common.SystemTestMixin (like test_system) will see something - # like 120s of delay, which may be enough to hit the default trial - # timeouts. For that reason, get_local_addresses_async() was changed - # to default to the numerical ip address for A.ROOT-SERVERS.NET, to - # avoid this DNS lookup. This also makes node startup fractionally - # faster. - return None - - try: - udpprot = DatagramProtocol() - port = reactor.listenUDP(0, udpprot) - try: - # connect() will fail if we're offline (e.g. running tests on a - # disconnected laptop), which is fine (localip=None), but we must - # still do port.stopListening() or we'll get a DirtyReactorError - udpprot.transport.connect(target_ipaddr, 7) - localip = udpprot.transport.getHost().host - return localip - finally: - d = port.stopListening() - d.addErrback(log.err) - except (socket.error, CannotListenError): - # no route to that host - localip = None - return native_str(localip) - - -# Wow, I'm really amazed at home much mileage we've gotten out of calling -# the external route.exe program on windows... It appears to work on all -# versions so far. -# ... thus wrote Greg Smith in time immemorial... -# Also, the Win32 APIs for this are really klunky and error-prone. --Daira - -_win32_re = re.compile(br'^\s*\d+\.\d+\.\d+\.\d+\s.+\s(?P
\d+\.\d+\.\d+\.\d+)\s+(?P\d+)\s*$', flags=re.M|re.I|re.S) -_win32_commands = (('route.exe', ('print',), _win32_re),) - -# These work in most Unices. -_addr_re = re.compile(br'^\s*inet [a-zA-Z]*:?(?P
\d+\.\d+\.\d+\.\d+)[\s/].+$', flags=re.M|re.I|re.S) -_unix_commands = (('/bin/ip', ('addr',), _addr_re), - ('/sbin/ip', ('addr',), _addr_re), - ('/sbin/ifconfig', ('-a',), _addr_re), - ('/usr/sbin/ifconfig', ('-a',), _addr_re), - ('/usr/etc/ifconfig', ('-a',), _addr_re), - ('ifconfig', ('-a',), _addr_re), - ('/sbin/ifconfig', (), _addr_re), - ) - - -def _find_addresses_via_config(): - return threads.deferToThread(_synchronously_find_addresses_via_config) - -def _synchronously_find_addresses_via_config(): - # originally by Greg Smith, hacked by Zooko and then Daira - - # We don't reach here for cygwin. - if platform == 'win32': - commands = _win32_commands - else: - commands = _unix_commands - - for (pathtotool, args, regex) in commands: - # If pathtotool is a fully qualified path then we just try that. - # If it is merely an executable name then we use Twisted's - # "which()" utility and try each executable in turn until one - # gives us something that resembles a dotted-quad IPv4 address. - - if os.path.isabs(pathtotool): - exes_to_try = [pathtotool] - else: - exes_to_try = which(pathtotool) - - subprocess_error = getattr( - subprocess, "SubprocessError", subprocess.CalledProcessError - ) - for exe in exes_to_try: - try: - addresses = _query(exe, args, regex) - except (IOError, OSError, ValueError, subprocess_error): - addresses = [] - if addresses: - return addresses - - return [] - -def _query(path, args, regex): - if not os.path.isfile(path): - return [] - env = {native_str('LANG'): native_str('en_US.UTF-8')} - TRIES = 5 - for trial in range(TRIES): - try: - p = subprocess.Popen([path] + list(args), stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env) - (output, err) = p.communicate() - break - except OSError as e: - if e.errno == errno.EINTR and trial < TRIES-1: - continue - raise - - addresses = [] - outputsplit = output.split(b'\n') - for outline in outputsplit: - m = regex.match(outline) - if m: - addr = m.group('address') - if addr not in addresses: - addresses.append(addr.decode("utf-8")) - - return addresses - -def _cygwin_hack_find_addresses(): - addresses = [] - for h in ["localhost", "127.0.0.1",]: - addr = get_local_ip_for(h) - if addr is not None and addr not in addresses: - addresses.append(addr) - - return defer.succeed(addresses) + return list( + native_str(address[native_str("addr")]) + for iface_name + in interfaces() + for address + in ifaddresses(iface_name).get(socket.AF_INET) + ) def _foolscapEndpointForPortNumber(portnum): @@ -382,7 +223,5 @@ def listenOnUnused(tub, portnum=None): __all__ = ["allocate_tcp_port", "increase_rlimits", "get_local_addresses_sync", - "get_local_addresses_async", - "get_local_ip_for", "listenOnUnused", ] From c60d62d85832e137b6df07c63a76ff349e296eb4 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:32:13 -0400 Subject: [PATCH 2/6] Direct test for the new implementation --- src/allmydata/test/test_iputil.py | 40 +++++++++++++++++++++++++++++++ src/allmydata/util/iputil.py | 2 +- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/allmydata/test/test_iputil.py b/src/allmydata/test/test_iputil.py index 6f6100410..54f414bfe 100644 --- a/src/allmydata/test/test_iputil.py +++ b/src/allmydata/test/test_iputil.py @@ -16,6 +16,13 @@ if PY2: import re, os, socket import gc +from testtools.matchers import ( + MatchesAll, + IsInstance, + AllMatch, + MatchesPredicate, +) + from twisted.trial import unittest from tenacity import retry, stop_after_attempt @@ -24,6 +31,13 @@ from foolscap.api import Tub from allmydata.util import iputil, gcutil +from ..util.iputil import ( + get_local_addresses_sync, +) + +from .common import ( + SyncTestCase, +) class ListenOnUsed(unittest.TestCase): """Tests for listenOnUnused.""" @@ -96,3 +110,29 @@ class GcUtil(unittest.TestCase): self.assertEqual(len(collections), 0) tracker.allocate() self.assertEqual(len(collections), 1) + + +class GetLocalAddressesSyncTests(SyncTestCase): + """ + Tests for ``get_local_addresses_sync``. + """ + def test_some_ipv4_addresses(self): + """ + ``get_local_addresses_sync`` returns a list of IPv4 addresses as native + strings. + """ + self.assertThat( + get_local_addresses_sync(), + MatchesAll( + IsInstance(list), + AllMatch( + MatchesAll( + IsInstance(native_str), + MatchesPredicate( + lambda addr: socket.inet_pton(socket.AF_INET, addr), + "%r is not an IPv4 address.", + ), + ), + ), + ), + ) diff --git a/src/allmydata/util/iputil.py b/src/allmydata/util/iputil.py index cbb4922e4..fd3e88c7f 100644 --- a/src/allmydata/util/iputil.py +++ b/src/allmydata/util/iputil.py @@ -114,7 +114,7 @@ def get_local_addresses_sync(): for iface_name in interfaces() for address - in ifaddresses(iface_name).get(socket.AF_INET) + in ifaddresses(iface_name).get(socket.AF_INET, []) ) From e170d8b881676b546e5e7f08cecdbd4580447018 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:33:20 -0400 Subject: [PATCH 3/6] Remove nettools (iproute2, ifconfig) from the NixOS packaging --- nix/tahoe-lafs.nix | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index a7f8fcbf7..302e1ff84 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -1,5 +1,5 @@ { fetchFromGitHub, lib -, nettools, python +, python , twisted, foolscap, zfec , setuptools, setuptoolsTrial, pyasn1, zope_interface , service-identity, pyyaml, magic-wormhole, treq, appdirs @@ -41,10 +41,6 @@ python.pkgs.buildPythonPackage rec { ''; - propagatedNativeBuildInputs = [ - nettools - ]; - propagatedBuildInputs = with python.pkgs; [ twisted foolscap zfec appdirs setuptoolsTrial pyasn1 zope_interface From 682e91382fda7c9416c9c374b46bd81934d84439 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:36:26 -0400 Subject: [PATCH 4/6] *Add* the new dep to the NixOS packaging too --- nix/tahoe-lafs.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nix/tahoe-lafs.nix b/nix/tahoe-lafs.nix index 302e1ff84..a862781cf 100644 --- a/nix/tahoe-lafs.nix +++ b/nix/tahoe-lafs.nix @@ -3,7 +3,7 @@ , twisted, foolscap, zfec , setuptools, setuptoolsTrial, pyasn1, zope_interface , service-identity, pyyaml, magic-wormhole, treq, appdirs -, beautifulsoup4, eliot, autobahn, cryptography +, beautifulsoup4, eliot, autobahn, cryptography, netifaces , html5lib, pyutil, distro }: python.pkgs.buildPythonPackage rec { @@ -45,7 +45,7 @@ python.pkgs.buildPythonPackage rec { twisted foolscap zfec appdirs setuptoolsTrial pyasn1 zope_interface service-identity pyyaml magic-wormhole treq - eliot autobahn cryptography setuptools + eliot autobahn cryptography netifaces setuptools future pyutil distro ]; From 96c848b2ad7413ef0a86bdb85f9675edbbbbc4a5 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:50:31 -0400 Subject: [PATCH 5/6] flakes --- src/allmydata/test/test_iputil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/test/test_iputil.py b/src/allmydata/test/test_iputil.py index 54f414bfe..081c80ee3 100644 --- a/src/allmydata/test/test_iputil.py +++ b/src/allmydata/test/test_iputil.py @@ -13,7 +13,7 @@ from future.utils import PY2, native_str if PY2: from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 -import re, os, socket +import os, socket import gc from testtools.matchers import ( From ed5d472209f63d83cfe5fc0bf8bb1e9ff4dbbacf Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 23 Oct 2020 09:52:07 -0400 Subject: [PATCH 6/6] news fragment --- newsfragments/3486.installation | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3486.installation diff --git a/newsfragments/3486.installation b/newsfragments/3486.installation new file mode 100644 index 000000000..7b24956b2 --- /dev/null +++ b/newsfragments/3486.installation @@ -0,0 +1 @@ +Tahoe-LAFS now requires the `netifaces` Python package and no longer requires the external `ip`, `ifconfig`, or `route.exe` executables.