clean up tub.port processing, reject tcp:0

This is the first step towards making node startup be synchronous: the
tub.port is entirely determined (including any TCP port allocation that
might be necessary) before creating the Tub, so the portnumber part of
FURLs can be determined earlier.

docs/configuration.rst

@@ -49,13 +49,22 @@ The item descriptions below use the following types:
     not specified, Tahoe-LAFS will attempt to bind the port specified on all
     interfaces.
 
+``endpoint specification string``
+
+    a Twisted Endpoint specification string, like "``tcp:80``" or
+    "``tcp:3456:interface=127.0.0.1``". These are replacing strports strings.
+    For a full description of the format, see `the Twisted Endpoints
+    documentation`_. Please note, if interface= is not specified, Tahoe-LAFS
+    will attempt to bind the port specified on all interfaces. Also note that
+    ``tub.port`` only works with TCP endpoints right now.
+
 ``FURL string``
 
     a Foolscap endpoint identifier, like
     ``pb://soklj4y7eok5c3xkmjeqpw@192.168.69.247:44801/eqpwqtzm``
 
 .. _the Twisted strports documentation: https://twistedmatrix.com/documents/current/api/twisted.application.strports.html
-
+.. _the Twisted Endpoints documentation: http://twistedmatrix.com/documents/current/core/howto/endpoints.html#endpoint-types-included-with-twisted
 
 Node Types
 ==========
@@ -128,13 +137,28 @@ set the ``tub.location`` option described below.
     ``http://127.0.0.1:3456/static/foo.html`` will serve the contents of
     ``BASEDIR/public_html/foo.html`` .
 
-``tub.port = (integer, optional)``
+``tub.port = (endpoints specification string, optional)``
 
-    This controls which port the node uses to accept Foolscap connections
-    from other nodes. If not provided, the node will ask the kernel for any
-    available port. The port will be written to a separate file (named
+    This controls the node's "listening port", through which it accepts
+    Foolscap connections from other nodes. If not provided, the node will ask
+    the kernel to allocate an available port, then saves it to a file (named
     ``client.port`` or ``introducer.port``), so that subsequent runs will
-    re-use the same port.
+    re-use the same port. The value should be ``tcp:`` followed by a port
+    number, e.g. ``tcp:3457``.
+
+    Note that the node can advertise an entirely different host+port (with
+    ``tub.location``, below) than the port that it listens on (controlled by
+    ``tub.port``). This is most useful when the node is a storage server and
+    lives behind a firewall that has been configured to forward a TCP port.
+    ``tub.location`` would be set to ``tcp:ADDR1:PORT1`` (where ADDR1 is the
+    external hostname or IP address of the firewall box, and PORT1 is the
+    externally-visible port), while ``tub.port`` would be ``tcp:PORT2``
+    (where the firewall is forwarding external PORT1 to internal PORT2).
+
+    ``tub.port`` cannot be ``tcp:0`` or ``0``: older versions accepted this,
+    but the node is no longer willing to ask Twisted to allocate port numbers
+    in this way. For backwards compatibility, a ``tub.port`` that is an
+    integer (other than 0) will be given a ``tcp:`` prefix.
 
 ``tub.location = (string, optional)``
 

src/allmydata/node.py

@@ -144,18 +144,6 @@ class Node(service.MultiService):
             if os.path.exists(tahoe_cfg):
                 raise
 
-        cfg_tubport = self.get_config("node", "tub.port", "")
-        if not cfg_tubport:
-            # For 'tub.port', tahoe.cfg overrides the individual file on
-            # disk. So only read self._portnumfile if tahoe.cfg doesn't
-            # provide a value.
-            try:
-                file_tubport = fileutil.read(self._portnumfile).strip()
-                configutil.set_config(self.config, "node", "tub.port", file_tubport)
-            except EnvironmentError:
-                if os.path.exists(self._portnumfile):
-                    raise
-
     def error_about_old_config_files(self):
         """ If any old configuration files are detected, raise OldConfigError. """
 
@@ -175,6 +163,25 @@ class Node(service.MultiService):
             twlog.msg(e)
             raise e
 
+    def _convert_tub_port(self, s):
+        if re.search(r'^\d+$', s):
+            return "tcp:%d" % int(s)
+        return s
+
+    def get_tub_port(self):
+        # return a descriptor string
+        cfg_tubport = self.get_config("node", "tub.port", "")
+        if cfg_tubport:
+            return self._convert_tub_port(cfg_tubport)
+        # For 'tub.port', tahoe.cfg overrides the individual file on disk. So
+        # only read self._portnumfile if tahoe.cfg doesn't provide a value.
+        if os.path.exists(self._portnumfile):
+            file_tubport = fileutil.read(self._portnumfile).strip()
+            return self._convert_tub_port(file_tubport)
+        tubport = "tcp:%d" % iputil.allocate_tcp_port()
+        fileutil.write_atomically(self._portnumfile, tubport + "\n", mode="")
+        return tubport
+
     def create_tub(self):
         certfile = os.path.join(self.basedir, "private", self.CERTFILE)
         self.tub = Tub(certFile=certfile)
@@ -194,8 +201,9 @@ class Node(service.MultiService):
         self.nodeid = b32decode(self.tub.tubID.upper()) # binary format
         self.write_config("my_nodeid", b32encode(self.nodeid).lower() + "\n")
         self.short_nodeid = b32encode(self.nodeid).lower()[:8] # ready for printing
-
-        tubport = self.get_config("node", "tub.port", "tcp:0")
+        tubport = self.get_tub_port()
+        if tubport in ("0", "tcp:0"):
+            raise ValueError("tub.port cannot be 0: you must choose")
         self.tub.listenOn(tubport)
         # we must wait until our service has started before we can find out
         # our IP address and thus do tub.setLocation, and we can't register
@@ -369,9 +377,6 @@ class Node(service.MultiService):
         # running, which means after startService.
         l = self.tub.getListeners()[0]
         portnum = l.getPortnum()
-        # record which port we're listening on, so we can grab the same one
-        # next time
-        fileutil.write_atomically(self._portnumfile, "%d\n" % portnum, mode="")
 
         location = self.get_config("node", "tub.location", "AUTO")
 

topfiles/2491.config

@@ -0,0 +1,14 @@
+* tub.port is now an Endpoint server specification string (which is pretty
+  much just like a strports string, but can be extended by plugins). It now
+  rejects "tcp:0" and "0". The tahoe.cfg value overrides anything stored on
+  disk (in client.port). This should have no effect on most nodes, which do
+  not set tub.port in tahoe.cfg, and wrote an allocated port number to
+  client.port the first time they launched. Folks who want to listen on a
+  specific port number typically set tub.port to "tcp:12345" or "12345", not
+  "0".
+* The "portnumfile" (e.g. NODEDIR/client.port) is written as soon as the port
+  is allocated, before the tub is created, and only if "tub.port" was empty.
+  The old code wrote to it unconditionally, and after Tub startup. So if the
+  user allows NODEDIR/client.port to be written, then later modifies
+  tahoe.cfg to set "tub.port" to a different value, this difference will
+  persist (and the node will honor tahoe.cfg "tub.port" exclusively).