if tub.port is empty, don't listen

Updated config docs. Added errors if we're not listening but were told
to enable storage, helper, or if we're the Introducer server.

closes ticket:2816

docs/configuration.rst

@@ -152,14 +152,21 @@ set the ``tub.location`` option described below.
     willing to ask Twisted to allocate port numbers in this way). To
     automatically allocate a TCP port, leave ``tub.port`` blank.
 
-    If the ``tub.port`` config key is not provided, the node will look in
+    If the ``tub.port`` key is empty (i.e. ``tub.port =``), the node will not
-    ``BASEDIR/client.port`` (or ``BASEDIR/introducer.port``, for introducers)
+    listen at all, and thus cannot accept connections from other nodes. If
-    for the descriptor that was used last time.
+    ``[storage] enabled = true``, or ``[helper] enabled = true``, or the node
+    is an Introducer, then it is an error to have ``tub.port`` be empty.
 
-    If neither is available, the node will ask the kernel for any available
+    If the ``tub.port`` config key is not provided (e.g. ``tub.port`` appears
-    port (the moral equivalent of ``tcp:0``). The allocated port number will
+    nowhere in the ``[node]`` section, or is commented out), the node will
-    be written into a descriptor string in ``BASEDIR/client.port`` (or
+    look in ``BASEDIR/client.port`` (or ``BASEDIR/introducer.port``, for
-    ``introducer.port``), so that subsequent runs will re-use the same port.
+    introducers) for the descriptor that was used last time.
+
+    If neither ``tub.port`` nor the port file is available, the node will ask
+    the kernel to allocate any available port (the moral equivalent of
+    ``tcp:0``). The allocated port number will be written into a descriptor
+    string in ``BASEDIR/client.port`` (or ``introducer.port``), so that
+    subsequent runs will re-use the same port.
 
 ``tub.location = (string, optional)``
 

src/allmydata/client.py

@@ -141,6 +141,9 @@ class Client(node.Node, pollmixin.PollMixin):
         self.load_static_servers()
         self.helper = None
         if self.get_config("helper", "enabled", False, boolean=True):
+            if not self._tub_is_listening:
+                raise ValueError("config error: helper is enabled, but tub "
+                                 "is not listening ('tub.port=' is empty)")
             self.init_helper()
         self.init_ftp_server()
         self.init_sftp_server()
@@ -246,6 +249,9 @@ class Client(node.Node, pollmixin.PollMixin):
         # should we run a storage server (and publish it for others to use)?
         if not self.get_config("storage", "enabled", True, boolean=True):
             return
+        if not self._tub_is_listening:
+            raise ValueError("config error: storage is enabled, but tub "
+                             "is not listening ('tub.port=' is empty)")
         readonly = self.get_config("storage", "readonly", False, boolean=True)
 
         storedir = os.path.join(self.basedir, self.STOREDIR)

src/allmydata/introducer/server.py

@@ -29,6 +29,9 @@ class IntroducerNode(node.Node):
             self.init_web(webport) # strports string
 
     def init_introducer(self):
+        if not self._tub_is_listening:
+            raise ValueError("config error: we are Introducer, but tub "
+                             "is not listening ('tub.port=' is empty)")
         introducerservice = IntroducerService(self.basedir)
         self.add_service(introducerservice)
 

src/allmydata/node.py

@@ -300,8 +300,11 @@ class Node(service.MultiService):
 
     def get_tub_port(self):
         # return a descriptor string
-        cfg_tubport = self.get_config("node", "tub.port", "")
+        MISSING = object()
-        if cfg_tubport:
+        cfg_tubport = self.get_config("node", "tub.port", MISSING)
+        if cfg_tubport is not MISSING:
+            if cfg_tubport.strip() == "":
+                return None # don't listen at all
             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.
@@ -338,15 +341,19 @@ class Node(service.MultiService):
         self.write_config("my_nodeid", b32encode(self.nodeid).lower() + "\n")
         self.short_nodeid = b32encode(self.nodeid).lower()[:8] # ready for printing
         tubport = self.get_tub_port()
+        if tubport:
             if tubport in ("0", "tcp:0"):
                 raise ValueError("tub.port cannot be 0: you must choose")
             self.tub.listenOn(tubport)
-
             location = self.get_tub_location(tubport)
             self.tub.setLocation(location)
+            self._tub_is_listening = True
             self.log("Tub location set to %s" % (location,))
-
             # the Tub is now ready for tub.registerReference()
+        else:
+            self._tub_is_listening = False
+            self.log("Tub is not listening")
+
         self.tub.setServiceParent(self)
 
     def create_control_tub(self):

src/allmydata/test/test_node.py

@@ -10,6 +10,8 @@ import foolscap.logging.log
 
 from twisted.application import service
 from allmydata.node import Node, formatTimeTahoeStyle, MissingConfigEntry
+from allmydata.introducer.server import IntroducerNode
+from allmydata.client import Client
 from allmydata.util import fileutil, iputil
 from allmydata.util.namespace import Namespace
 import allmydata.test.common_util as testutil
@@ -178,3 +180,71 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
         TestNode(basedir)
         self.failUnless(ns.called)
+
+NO_LISTEN_CONFIG = """
+[node]
+tub.port =
+#tub.location =
+[client]
+introducer.furl = empty
+"""
+
+class ClientNotListening(unittest.TestCase):
+    def test_port_none(self):
+        basedir = "test_node/test_port_none"
+        fileutil.make_dirs(basedir)
+        f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
+        f.write(NO_LISTEN_CONFIG)
+        f.write("[storage]\n")
+        f.write("enabled = false\n")
+        f.close()
+        n = Client(basedir)
+        self.assertEqual(n.tub.getListeners(), [])
+
+    def test_port_none_location_none(self):
+        basedir = "test_node/test_port_none_location_none"
+        fileutil.make_dirs(basedir)
+        f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
+        f.write(NO_LISTEN_CONFIG)
+        f.write("tub.location =\n")
+        f.write("[storage]\n")
+        f.write("enabled = false\n")
+        f.close()
+        n = Client(basedir)
+        self.assertEqual(n.tub.getListeners(), [])
+
+    def test_port_none_storage(self):
+        basedir = "test_node/test_port_none_storage"
+        fileutil.make_dirs(basedir)
+        f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
+        f.write(NO_LISTEN_CONFIG)
+        f.write("[storage]\n")
+        f.write("enabled = true")
+        f.close()
+        e = self.assertRaises(ValueError, Client, basedir)
+        self.assertIn("storage is enabled, but tub is not listening", str(e))
+
+    def test_port_none_helper(self):
+        basedir = "test_node/test_port_none_helper"
+        fileutil.make_dirs(basedir)
+        f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
+        f.write(NO_LISTEN_CONFIG)
+        f.write("[storage]\n")
+        f.write("enabled = false\n")
+        f.write("[helper]\n")
+        f.write("enabled = true")
+        f.close()
+        e = self.assertRaises(ValueError, Client, basedir)
+        self.assertIn("helper is enabled, but tub is not listening", str(e))
+
+class IntroducerNotListening(unittest.TestCase):
+    def test_port_none_introducer(self):
+        basedir = "test_node/test_port_none_introducer"
+        fileutil.make_dirs(basedir)
+        f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
+        f.write("[node]\n")
+        f.write("tub.port = \n")
+        f.write("#tub.location = \n")
+        f.close()
+        e = self.assertRaises(ValueError, IntroducerNode, basedir)
+        self.assertIn("we are Introducer, but tub is not listening", str(e))