From 83db7e8b430a6ab35b5cd9d12a1c56cd4ee021e3 Mon Sep 17 00:00:00 2001 From: David Stainton Date: Tue, 6 Sep 2016 15:03:31 +0000 Subject: [PATCH 1/9] Add create node args: listen, port, hostname, location fixes ticket: 2773 --- src/allmydata/scripts/create_node.py | 94 +++++++++++++--- src/allmydata/test/cli/test_create.py | 151 ++++++++++++++++++++++++++ src/allmydata/test/test_runner.py | 6 +- 3 files changed, 234 insertions(+), 17 deletions(-) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 891c03a82..6d98ba731 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -1,9 +1,10 @@ import os +from twisted.python import usage from allmydata.scripts.common import BasedirOptions, NoDefaultBasedirOptions from allmydata.scripts.default_nodedir import _default_nodedir from allmydata.util.assertutil import precondition from allmydata.util.encodingutil import listdir_unicode, argv_to_unicode, quote_local_unicode_path -from allmydata.util import fileutil +from allmydata.util import fileutil, iputil dummy_tac = """ @@ -17,10 +18,38 @@ def write_tac(basedir, nodetype): fileutil.write(os.path.join(basedir, "tahoe-%s.tac" % (nodetype,)), dummy_tac) +WHERE_OPTS = [ + ("location", None, None, "Specify the location to advertise for this node."), + ("port", None, None, "Specify the server endpoint to listen on for this node."), +] + +def validate_where_options(options): + if options['hostname'] and options['port']: + raise usage.UsageError("The --hostname option cannot be used with the --port option.") + if options['hostname'] and options['location']: + raise usage.UsageError("The --hostname option cannot be used with the --location option.") + if not options['hostname'] and (options['location'] and not options['port']): + raise usage.UsageError("The --location option must be used with the --port option.") + if not options['hostname'] and (options['port'] and not options['location']): + raise usage.UsageError("The --port option must be used with the --location option.") + if (options['listen'] != "tcp") and options['hostname']: + raise usage.UsageError("The listener type must be TCP to use --hostname option.") + class _CreateBaseOptions(BasedirOptions): optFlags = [ ("hide-ip", None, "prohibit any configuration that would reveal the node's IP address"), ] + + # This is overridden in order to ensure we get a "Wrong number of + # arguments." error when more than one argument is given. + def parseArgs(self, basedir=None): + BasedirOptions.parseArgs(self, basedir) + + +class CreateClientOptions(_CreateBaseOptions): + synopsis = "[options] [NODEDIR]" + description = "Create a client-only Tahoe-LAFS node (no storage server)." + optParameters = [ # we provide 'create-node'-time options for the most common # configuration knobs. The rest can be controlled by editing @@ -31,32 +60,48 @@ class _CreateBaseOptions(BasedirOptions): "Specify which TCP port to run the HTTP interface on. Use 'none' to disable."), ("basedir", "C", None, "Specify which Tahoe base directory should be used. This has the same effect as the global --node-directory option. [default: %s]" % quote_local_unicode_path(_default_nodedir)), - ] - # This is overridden in order to ensure we get a "Wrong number of - # arguments." error when more than one argument is given. def parseArgs(self, basedir=None): BasedirOptions.parseArgs(self, basedir) -class CreateClientOptions(_CreateBaseOptions): - synopsis = "[options] [NODEDIR]" - description = "Create a client-only Tahoe-LAFS node (no storage server)." - class CreateNodeOptions(CreateClientOptions): optFlags = [ ("no-storage", None, "Do not offer storage service to other nodes."), ] synopsis = "[options] [NODEDIR]" description = "Create a full Tahoe-LAFS node (client+server)." + optParameters = WHERE_OPTS + [ + # we provide 'create-node'-time options for the most common + # configuration knobs. The rest can be controlled by editing + # tahoe.cfg before node startup. + ("hostname", None, None, "Specify the hostname for listening and advertising for this node."), + ("listen", None, "tcp", "Specify the listener type for this node."), + ("nickname", "n", None, "Specify the nickname for this node."), + ("introducer", "i", None, "Specify the introducer FURL to use."), + ("webport", "p", "tcp:3456:interface=127.0.0.1", + "Specify which TCP port to run the HTTP interface on. Use 'none' to disable."), + ("basedir", "C", None, "Specify which Tahoe base directory should be used. This has the same effect as the global --node-directory option. [default: %s]" + % quote_local_unicode_path(_default_nodedir)), + + ] + + def parseArgs(self, basedir=None): + CreateClientOptions.parseArgs(self, basedir) + validate_where_options(self) class CreateIntroducerOptions(NoDefaultBasedirOptions): subcommand_name = "create-introducer" description = "Create a Tahoe-LAFS introducer." optFlags = [ ("hide-ip", None, "prohibit any configuration that would reveal the node's IP address"), - ] - + ] + optParameters = WHERE_OPTS + [("listen", None, "tcp", "Specify the listener type for this node."), + ("hostname", None, None, "Specify the hostname for listening and advertising for this node."), + ] + def parseArgs(self, basedir=None): + NoDefaultBasedirOptions.parseArgs(self, basedir) + validate_where_options(self) def write_node_config(c, config): # this is shared between clients and introducers @@ -83,11 +128,30 @@ def write_node_config(c, config): webport = "" c.write("web.port = %s\n" % (webport.encode('utf-8'),)) c.write("web.static = public_html\n") - c.write("# to prevent the Tub from listening at all, use this:\n") - c.write("# tub.port = disabled\n") - c.write("# tub.location = disabled\n") - c.write("#tub.port =\n") - c.write("#tub.location = \n") + + if 'hostname' in config and config['hostname'] is not None: + print "HOSTNAME" + new_port = iputil.allocate_tcp_port() + c.write("tub.port = tcp:%s\n" % new_port) + c.write("tub.location = tcp:%s:%s\n" % (config.get('hostname').encode('utf-8'), new_port)) + elif 'listen' in config and config['listen'] == "tor": + raise NotImplementedError("This feature addition is being tracked by this ticket:" + + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") + elif 'listen' in config and config['listen'] == "i2p": + raise NotImplementedError("This feature addition is being tracked by this ticket:" + + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") + elif config.get('port') is not None: + c.write("tub.port = %s\n" % config.get('port').encode('utf-8')) + c.write("tub.location = %s\n" % config.get('location').encode('utf-8')) + else: + c.write("tub.port = disabled\n") + c.write("tub.location = disabled\n") + + if ('hostname' in config and config['hostname']) or ('listen' in config and config['listen']): + c.write("# to prevent the Tub from listening at all, use this:\n") + c.write("# tub.port = disabled\n") + c.write("# tub.location = disabled\n") + c.write("#log_gatherer.furl =\n") c.write("#timeout.keepalive =\n") c.write("#timeout.disconnect =\n") diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index c10271a8b..df9a4d637 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -1,6 +1,7 @@ import os from twisted.trial import unittest from twisted.internet import defer +from twisted.python import usage from allmydata.util import configutil from ..common_util import run_cli @@ -16,6 +17,81 @@ class Config(unittest.TestCase): rc, out, err = yield run_cli("create-client", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True) + self.assertEqual(cfg.get("node", "tub.port"), "disabled") + self.assertEqual(cfg.get("node", "tub.location"), "disabled") + + @defer.inlineCallbacks + def test_client_hostname(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", "--hostname=computer", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --hostname not recognized") + else: + self.fail("UsageError expected to be raised") + + @defer.inlineCallbacks + def test_client_port_location(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", + "--port=unix:/var/tahoe/socket", + "--location=tor:myservice.onion:12345", + basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --port not recognized") + else: + self.fail("UsageError expected to be raised") + + @defer.inlineCallbacks + def test_client_port_only(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", "--port=unix:/var/tahoe/socket", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --port not recognized") + else: + self.fail("UsageError expected to be raised") + + @defer.inlineCallbacks + def test_client_location_only(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", "--location=tor:myservice.onion:12345", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --location not recognized") + else: + self.fail("UsageError expected to be raised") + + @defer.inlineCallbacks + def test_client_listen_tcp(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", "--listen=tcp", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --listen not recognized") + else: + self.fail("UsageError expected to be raised)") + + @defer.inlineCallbacks + def test_client_listen_tor(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", "--listen=tor", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --listen not recognized") + else: + self.fail("UsageError expected to be raised)") + + @defer.inlineCallbacks + def test_client_listen_i2p(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-client", "--listen=i2p", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option --listen not recognized") + else: + self.fail("UsageError expected to be raised") @defer.inlineCallbacks def test_client_hide_ip(self): @@ -30,6 +106,7 @@ class Config(unittest.TestCase): rc, out, err = yield run_cli("create-node", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True) + self.assertEqual(cfg.get("node", "listen"), "tcp") @defer.inlineCallbacks def test_node_hide_ip(self): @@ -38,6 +115,73 @@ class Config(unittest.TestCase): cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False) + @defer.inlineCallbacks + def test_node_hostname(self): + basedir = self.mktemp() + rc, out, err = yield run_cli("create-node", "--hostname=computer", basedir) + cfg = self.read_config(basedir) + self.assertTrue("computer" in cfg.get("node", "tub.location")) + + @defer.inlineCallbacks + def test_node_port_location(self): + basedir = self.mktemp() + rc, out, err = yield run_cli("create-node", + "--port=unix:/var/tahoe/socket", + "--location=tor:myservice.onion:12345", + basedir) + cfg = self.read_config(basedir) + self.assertEqual(cfg.get("node", "tub.location"), "tor:myservice.onion:12345") + self.assertEqual(cfg.get("node", "tub.port"), "unix:/var/tahoe/socket") + + @defer.inlineCallbacks + def test_node_listen_tcp(self): + basedir = self.mktemp() + rc, out, err = yield run_cli("create-node", "--listen=tcp", basedir) + cfg = self.read_config(basedir) + self.assertEqual(cfg.get("node", "listen"), "tcp") + + @defer.inlineCallbacks + def test_node_listen_tor(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-node", "--listen=tor", basedir) + except NotImplementedError, e: + self.failUnlessEqual(str(e), "This feature addition is being tracked by this ticket:" + + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") + else: + self.fail("NotImplementedError expected to be raised") + + @defer.inlineCallbacks + def test_node_listen_i2p(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-node", "--listen=i2p", basedir) + except NotImplementedError, e: + self.failUnlessEqual(str(e), "This feature addition is being tracked by this ticket:" + + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") + else: + self.fail("NotImplementedError expected to be raised") + + @defer.inlineCallbacks + def test_node_port_only(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-node", "--port=unix:/var/tahoe/socket", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "The --port option must be used with the --location option.") + else: + self.fail("UsageError expected to be raised") + + @defer.inlineCallbacks + def test_node_location_only(self): + basedir = self.mktemp() + try: + rc, out, err = yield run_cli("create-node", "--location=tor:myservice.onion:12345", basedir) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "The --location option must be used with the --port option.") + else: + self.fail("UsageError expected to be raised") + @defer.inlineCallbacks def test_introducer(self): basedir = self.mktemp() @@ -51,3 +195,10 @@ class Config(unittest.TestCase): rc, out, err = yield run_cli("create-introducer", "--hide-ip", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False) + + @defer.inlineCallbacks + def test_introducer_hostname(self): + basedir = self.mktemp() + rc, out, err = yield run_cli("create-introducer", "--hostname=computer", basedir) + cfg = self.read_config(basedir) + self.assertTrue("computer" in cfg.get("node", "tub.location")) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index e8468ca46..2c29598d6 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -344,7 +344,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, node_url_file = os.path.join(c1, "node.url") config_file = os.path.join(c1, "tahoe.cfg") - d = self.run_bintahoe(["--quiet", "create-introducer", "--basedir", c1]) + d = self.run_bintahoe(["--quiet", "create-introducer", "--basedir", c1, "--hostname", "localhost"]) def _cb(res): out, err, rc_or_sig = res self.failUnlessEqual(rc_or_sig, 0) @@ -373,6 +373,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) self.failUnlessEqual(rc_or_sig, 0, errstr) self.failUnlessEqual(out, "", errstr) + print errstr # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. # the parent (twistd) has exited. However, twistd writes the pid @@ -393,6 +394,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # read the introducer.furl and introducer.port files so we can # check that their contents don't change on restart self.furl = fileutil.read(introducer_furl_file) + print "portnum_file " + portnum_file self.failUnless(os.path.exists(portnum_file)) self.portnum = fileutil.read(portnum_file) @@ -531,7 +533,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, node_url_file = os.path.join(c1, "node.url") config_file = os.path.join(c1, "tahoe.cfg") - d = self.run_bintahoe(["--quiet", "create-node", "--basedir", c1, "--webport", "0"]) + d = self.run_bintahoe(["--quiet", "create-node", "--basedir", c1, "--webport", "0", "--hostname", "localhost"]) def _cb(res): out, err, rc_or_sig = res self.failUnlessEqual(rc_or_sig, 0) From 31dfee2dcd9e5598201f8410f6e3d6c22bf9c0c4 Mon Sep 17 00:00:00 2001 From: David Stainton Date: Thu, 8 Sep 2016 15:02:09 +0000 Subject: [PATCH 2/9] fix cli test create tests --- src/allmydata/test/cli/test_create.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index df9a4d637..319a9f7db 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -106,7 +106,6 @@ class Config(unittest.TestCase): rc, out, err = yield run_cli("create-node", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True) - self.assertEqual(cfg.get("node", "listen"), "tcp") @defer.inlineCallbacks def test_node_hide_ip(self): @@ -138,7 +137,6 @@ class Config(unittest.TestCase): basedir = self.mktemp() rc, out, err = yield run_cli("create-node", "--listen=tcp", basedir) cfg = self.read_config(basedir) - self.assertEqual(cfg.get("node", "listen"), "tcp") @defer.inlineCallbacks def test_node_listen_tor(self): From e9c1075ac5003a580d9c3f42bb23e6b2e675c7cc Mon Sep 17 00:00:00 2001 From: David Stainton Date: Fri, 9 Sep 2016 09:22:00 +0000 Subject: [PATCH 3/9] Make corrections from Daira's code review --- src/allmydata/scripts/create_node.py | 12 ++-- src/allmydata/test/cli/test_create.py | 100 +++++++------------------- 2 files changed, 32 insertions(+), 80 deletions(-) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 6d98ba731..69625e96e 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -34,18 +34,14 @@ def validate_where_options(options): raise usage.UsageError("The --port option must be used with the --location option.") if (options['listen'] != "tcp") and options['hostname']: raise usage.UsageError("The listener type must be TCP to use --hostname option.") + if options['listen'] not in ["tcp", "tor", "i2p"]: + raise usage.UsageError("The listener type must set to one of: tcp, tor, i2p.") class _CreateBaseOptions(BasedirOptions): optFlags = [ ("hide-ip", None, "prohibit any configuration that would reveal the node's IP address"), ] - # This is overridden in order to ensure we get a "Wrong number of - # arguments." error when more than one argument is given. - def parseArgs(self, basedir=None): - BasedirOptions.parseArgs(self, basedir) - - class CreateClientOptions(_CreateBaseOptions): synopsis = "[options] [NODEDIR]" description = "Create a client-only Tahoe-LAFS node (no storage server)." @@ -62,6 +58,8 @@ class CreateClientOptions(_CreateBaseOptions): % quote_local_unicode_path(_default_nodedir)), ] + # This is overridden in order to ensure we get a "Wrong number of + # arguments." error when more than one argument is given. def parseArgs(self, basedir=None): BasedirOptions.parseArgs(self, basedir) @@ -147,7 +145,7 @@ def write_node_config(c, config): c.write("tub.port = disabled\n") c.write("tub.location = disabled\n") - if ('hostname' in config and config['hostname']) or ('listen' in config and config['listen']): + if config.get('hostname', None) or config.get('listen', None): c.write("# to prevent the Tub from listening at all, use this:\n") c.write("# tub.port = disabled\n") c.write("# tub.location = disabled\n") diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index 319a9f7db..718be1371 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -11,6 +11,33 @@ class Config(unittest.TestCase): config = configutil.get_config(tahoe_cfg) return config + @defer.inlineCallbacks + def _test_option_not_recognized(self, option, *args): + basedir = self.mktemp() + try: + params = args[0] + (basedir,) + rc, out, err = yield run_cli(*params) + except usage.UsageError, e: + self.failUnlessEqual(str(e), "option %s not recognized" % (option,)) + else: + self.fail("UsageError expected to be raised") + + @defer.inlineCallbacks + def test_client_unrecognized_options(self): + basedir = self.mktemp() + tests = [ + ("--listen", ("create-client", "--listen=tcp")), + ("--hostname", ("create-client", "--hostname=computer")), + ("--port", ("create-client", "--port=unix:/var/tahoe/socket", + "--location=tor:myservice.onion:12345")), + ("--port", ("create-client", "--port=unix:/var/tahoe/socket")), + ("--location", ("create-client", "--location=tor:myservice.onion:12345")), + ("--listen", ("create-client", "--listen=tor")), + ("--listen", ("create-client", "--listen=i2p")), + ] + for test in tests: + yield self._test_option_not_recognized(test[0], test[1]) + @defer.inlineCallbacks def test_client(self): basedir = self.mktemp() @@ -20,79 +47,6 @@ class Config(unittest.TestCase): self.assertEqual(cfg.get("node", "tub.port"), "disabled") self.assertEqual(cfg.get("node", "tub.location"), "disabled") - @defer.inlineCallbacks - def test_client_hostname(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", "--hostname=computer", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --hostname not recognized") - else: - self.fail("UsageError expected to be raised") - - @defer.inlineCallbacks - def test_client_port_location(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", - "--port=unix:/var/tahoe/socket", - "--location=tor:myservice.onion:12345", - basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --port not recognized") - else: - self.fail("UsageError expected to be raised") - - @defer.inlineCallbacks - def test_client_port_only(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", "--port=unix:/var/tahoe/socket", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --port not recognized") - else: - self.fail("UsageError expected to be raised") - - @defer.inlineCallbacks - def test_client_location_only(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", "--location=tor:myservice.onion:12345", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --location not recognized") - else: - self.fail("UsageError expected to be raised") - - @defer.inlineCallbacks - def test_client_listen_tcp(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", "--listen=tcp", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --listen not recognized") - else: - self.fail("UsageError expected to be raised)") - - @defer.inlineCallbacks - def test_client_listen_tor(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", "--listen=tor", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --listen not recognized") - else: - self.fail("UsageError expected to be raised)") - - @defer.inlineCallbacks - def test_client_listen_i2p(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-client", "--listen=i2p", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option --listen not recognized") - else: - self.fail("UsageError expected to be raised") - @defer.inlineCallbacks def test_client_hide_ip(self): basedir = self.mktemp() From 09ce7963c60056a33cef7f29bf3a516581eda3aa Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 9 Sep 2016 17:04:55 -0700 Subject: [PATCH 4/9] fixes --- src/allmydata/test/cli/test_create.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index 718be1371..37b791ad6 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -24,7 +24,6 @@ class Config(unittest.TestCase): @defer.inlineCallbacks def test_client_unrecognized_options(self): - basedir = self.mktemp() tests = [ ("--listen", ("create-client", "--listen=tcp")), ("--hostname", ("create-client", "--hostname=computer")), @@ -73,7 +72,10 @@ class Config(unittest.TestCase): basedir = self.mktemp() rc, out, err = yield run_cli("create-node", "--hostname=computer", basedir) cfg = self.read_config(basedir) - self.assertTrue("computer" in cfg.get("node", "tub.location")) + port = cfg.get("node", "tub.port") + location = cfg.get("node", "tub.location") + self.assertRegex(port, r'^tcp:\d+$') + self.assertRegex(location, r'^tcp:computer:\d+$') @defer.inlineCallbacks def test_node_port_location(self): @@ -87,10 +89,11 @@ class Config(unittest.TestCase): self.assertEqual(cfg.get("node", "tub.port"), "unix:/var/tahoe/socket") @defer.inlineCallbacks - def test_node_listen_tcp(self): + def test_node_listen_tcp_no_hostname(self): basedir = self.mktemp() - rc, out, err = yield run_cli("create-node", "--listen=tcp", basedir) - cfg = self.read_config(basedir) + d = run_cli("create-node", "--listen=tcp", basedir) + e = yield self.assertFailure(d, usage.UsageError) + self.assertIn("--listen=tcp requires --hostname=", str(e)) @defer.inlineCallbacks def test_node_listen_tor(self): From 7d3b4149ae17f4649e660866f7a8daf511292eb8 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 9 Sep 2016 17:55:27 -0700 Subject: [PATCH 5/9] remove leftover debug prints --- src/allmydata/scripts/create_node.py | 1 - src/allmydata/test/test_runner.py | 2 -- 2 files changed, 3 deletions(-) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 69625e96e..1cd04584f 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -128,7 +128,6 @@ def write_node_config(c, config): c.write("web.static = public_html\n") if 'hostname' in config and config['hostname'] is not None: - print "HOSTNAME" new_port = iputil.allocate_tcp_port() c.write("tub.port = tcp:%s\n" % new_port) c.write("tub.location = tcp:%s:%s\n" % (config.get('hostname').encode('utf-8'), new_port)) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index 2c29598d6..ccbb9d40c 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -373,7 +373,6 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, errstr = "rc=%d, OUT: '%s', ERR: '%s'" % (rc_or_sig, out, err) self.failUnlessEqual(rc_or_sig, 0, errstr) self.failUnlessEqual(out, "", errstr) - print errstr # self.failUnlessEqual(err, "", errstr) # See test_client_no_noise -- for now we ignore noise. # the parent (twistd) has exited. However, twistd writes the pid @@ -394,7 +393,6 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # read the introducer.furl and introducer.port files so we can # check that their contents don't change on restart self.furl = fileutil.read(introducer_furl_file) - print "portnum_file " + portnum_file self.failUnless(os.path.exists(portnum_file)) self.portnum = fileutil.read(portnum_file) From 9a1a18619775df36e91a2222b915af9b441ba835 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 9 Sep 2016 17:55:58 -0700 Subject: [PATCH 6/9] simplify tests: use parse_cli, assertRaises, assertFailure parse_cli() got added during the async-CLI-dispatch work assertRaises/assertFailure have been in Twisted for a while, but I only learned about them recently. Over time I'm looking forward to changing all tahoe tests to use them (and getting rid of ShouldFailMixin/etc). --- src/allmydata/test/cli/test_create.py | 90 +++++++++++---------------- 1 file changed, 35 insertions(+), 55 deletions(-) diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index 37b791ad6..f08444916 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -3,7 +3,7 @@ from twisted.trial import unittest from twisted.internet import defer from twisted.python import usage from allmydata.util import configutil -from ..common_util import run_cli +from ..common_util import run_cli, parse_cli class Config(unittest.TestCase): def read_config(self, basedir): @@ -11,31 +11,25 @@ class Config(unittest.TestCase): config = configutil.get_config(tahoe_cfg) return config - @defer.inlineCallbacks - def _test_option_not_recognized(self, option, *args): - basedir = self.mktemp() - try: - params = args[0] + (basedir,) - rc, out, err = yield run_cli(*params) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "option %s not recognized" % (option,)) - else: - self.fail("UsageError expected to be raised") - - @defer.inlineCallbacks def test_client_unrecognized_options(self): tests = [ - ("--listen", ("create-client", "--listen=tcp")), - ("--hostname", ("create-client", "--hostname=computer")), - ("--port", ("create-client", "--port=unix:/var/tahoe/socket", - "--location=tor:myservice.onion:12345")), - ("--port", ("create-client", "--port=unix:/var/tahoe/socket")), - ("--location", ("create-client", "--location=tor:myservice.onion:12345")), - ("--listen", ("create-client", "--listen=tor")), - ("--listen", ("create-client", "--listen=i2p")), + ("--listen", "create-client", "--listen=tcp"), + ("--hostname", "create-client", "--hostname=computer"), + ("--port", + "create-client", "--port=unix:/var/tahoe/socket", + "--location=tor:myservice.onion:12345"), + ("--port", "create-client", "--port=unix:/var/tahoe/socket"), + ("--location", + "create-client", "--location=tor:myservice.onion:12345"), + ("--listen", "create-client", "--listen=tor"), + ("--listen", "create-client", "--listen=i2p"), ] for test in tests: - yield self._test_option_not_recognized(test[0], test[1]) + option = test[0] + verb = test[1] + args = test[2:] + e = self.assertRaises(usage.UsageError, parse_cli, verb, *args) + self.assertIn("option %s not recognized" % (option,), str(e)) @defer.inlineCallbacks def test_client(self): @@ -88,54 +82,40 @@ class Config(unittest.TestCase): self.assertEqual(cfg.get("node", "tub.location"), "tor:myservice.onion:12345") self.assertEqual(cfg.get("node", "tub.port"), "unix:/var/tahoe/socket") - @defer.inlineCallbacks def test_node_listen_tcp_no_hostname(self): basedir = self.mktemp() - d = run_cli("create-node", "--listen=tcp", basedir) - e = yield self.assertFailure(d, usage.UsageError) + e = self.assertRaises(usage.UsageError, + parse_cli, + "create-node", "--listen=tcp", basedir) self.assertIn("--listen=tcp requires --hostname=", str(e)) @defer.inlineCallbacks def test_node_listen_tor(self): basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-node", "--listen=tor", basedir) - except NotImplementedError, e: - self.failUnlessEqual(str(e), "This feature addition is being tracked by this ticket:" + - "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") - else: - self.fail("NotImplementedError expected to be raised") + d = run_cli("create-node", "--listen=tor", basedir) + e = yield self.assertFailure(d, NotImplementedError) + self.assertEqual(str(e), "This feature addition is being tracked by this ticket:" + + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") @defer.inlineCallbacks def test_node_listen_i2p(self): basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-node", "--listen=i2p", basedir) - except NotImplementedError, e: - self.failUnlessEqual(str(e), "This feature addition is being tracked by this ticket:" + - "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") - else: - self.fail("NotImplementedError expected to be raised") + d = run_cli("create-node", "--listen=i2p", basedir) + e = yield self.assertFailure(d, NotImplementedError) + self.failUnlessEqual(str(e), "This feature addition is being tracked by this ticket:" + + "https://tahoe-lafs.org/trac/tahoe-lafs/ticket/2490") - @defer.inlineCallbacks def test_node_port_only(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-node", "--port=unix:/var/tahoe/socket", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "The --port option must be used with the --location option.") - else: - self.fail("UsageError expected to be raised") + e = self.assertRaises(usage.UsageError, + parse_cli, + "create-node", "--port=unix:/var/tahoe/socket") + self.assertEqual(str(e), "The --port option must be used with the --location option.") - @defer.inlineCallbacks def test_node_location_only(self): - basedir = self.mktemp() - try: - rc, out, err = yield run_cli("create-node", "--location=tor:myservice.onion:12345", basedir) - except usage.UsageError, e: - self.failUnlessEqual(str(e), "The --location option must be used with the --port option.") - else: - self.fail("UsageError expected to be raised") + e = self.assertRaises(usage.UsageError, + parse_cli, + "create-node", "--location=tor:myservice.onion:12345") + self.assertEqual(str(e), "The --location option must be used with the --port option.") @defer.inlineCallbacks def test_introducer(self): From 38ebdfac20969864d3126e52f059fda5c5d42a61 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 9 Sep 2016 17:56:24 -0700 Subject: [PATCH 7/9] create-node: reject --listen=tcp without --hostname= --- src/allmydata/scripts/create_node.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index 1cd04584f..a68b1f94b 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -34,6 +34,8 @@ def validate_where_options(options): raise usage.UsageError("The --port option must be used with the --location option.") if (options['listen'] != "tcp") and options['hostname']: raise usage.UsageError("The listener type must be TCP to use --hostname option.") + if options['listen'] == "tcp" and not options['hostname']: + raise usage.UsageError("--listen=tcp requires --hostname=") if options['listen'] not in ["tcp", "tor", "i2p"]: raise usage.UsageError("The listener type must set to one of: tcp, tor, i2p.") From 229e306e9d63be46cc5650558dd6def4fec6d5dc Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 9 Sep 2016 18:49:37 -0700 Subject: [PATCH 8/9] test_runner: fix to use new conventions create-node needs --hostname=, and we can't expect the node to write client.port at startup (because tahoe.cfg now has tub.port= set properly) --- src/allmydata/test/test_runner.py | 48 +++++++++++++++---------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/allmydata/test/test_runner.py b/src/allmydata/test/test_runner.py index ccbb9d40c..3dd32444a 100644 --- a/src/allmydata/test/test_runner.py +++ b/src/allmydata/test/test_runner.py @@ -262,14 +262,14 @@ class CreateNode(unittest.TestCase): command) def test_node(self): - self.do_create("node") + self.do_create("node", "--hostname=127.0.0.1") def test_client(self): # create-client should behave like create-node --no-storage. self.do_create("client") def test_introducer(self): - self.do_create("introducer") + self.do_create("introducer", "--hostname=127.0.0.1") def test_stats_gatherer(self): self.do_create("stats-gatherer", "--hostname=127.0.0.1") @@ -340,7 +340,6 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, exit_trigger_file = os.path.join(c1, Client.EXIT_TRIGGER_FILE) twistd_pid_file = os.path.join(c1, "twistd.pid") introducer_furl_file = os.path.join(c1, "private", "introducer.furl") - portnum_file = os.path.join(c1, "introducer.port") node_url_file = os.path.join(c1, "node.url") config_file = os.path.join(c1, "tahoe.cfg") @@ -390,11 +389,9 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, d.addCallback(lambda res: self.poll(_node_has_started)) def _started(res): - # read the introducer.furl and introducer.port files so we can - # check that their contents don't change on restart + # read the introducer.furl file so we can check that the contents + # don't change on restart self.furl = fileutil.read(introducer_furl_file) - self.failUnless(os.path.exists(portnum_file)) - self.portnum = fileutil.read(portnum_file) fileutil.write(exit_trigger_file, "") self.failUnless(os.path.exists(twistd_pid_file)) @@ -418,14 +415,13 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # so poll until it is. This time introducer_furl_file already # exists, so we check for the existence of node_url_file instead. def _node_has_restarted(): - return os.path.exists(node_url_file) and os.path.exists(portnum_file) + return os.path.exists(node_url_file) d.addCallback(lambda res: self.poll(_node_has_restarted)) - def _check_same_furl_and_port(res): + def _check_same_furl(res): self.failUnless(os.path.exists(introducer_furl_file)) self.failUnlessEqual(self.furl, fileutil.read(introducer_furl_file)) - self.failUnlessEqual(self.portnum, fileutil.read(portnum_file)) - d.addCallback(_check_same_furl_and_port) + d.addCallback(_check_same_furl) # Now we can kill it. TODO: On a slow machine, the node might kill # itself before we get a chance to, especially if spawning the @@ -463,7 +459,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, c1 = os.path.join(basedir, "c1") exit_trigger_file = os.path.join(c1, Client.EXIT_TRIGGER_FILE) twistd_pid_file = os.path.join(c1, "twistd.pid") - portnum_file = os.path.join(c1, "client.port") + node_url_file = os.path.join(c1, "node.url") d = self.run_bintahoe(["--quiet", "create-client", "--basedir", c1, "--webport", "0"]) def _cb(res): @@ -506,7 +502,7 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, d.addCallback(_cb2) def _node_has_started(): - return os.path.exists(portnum_file) + return os.path.exists(node_url_file) d.addCallback(lambda res: self.poll(_node_has_started)) # now we can kill it. TODO: On a slow machine, the node might kill @@ -527,11 +523,13 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, c1 = os.path.join(basedir, "c1") exit_trigger_file = os.path.join(c1, Client.EXIT_TRIGGER_FILE) twistd_pid_file = os.path.join(c1, "twistd.pid") - portnum_file = os.path.join(c1, "client.port") node_url_file = os.path.join(c1, "node.url") + storage_furl_file = os.path.join(c1, "private", "storage.furl") config_file = os.path.join(c1, "tahoe.cfg") - d = self.run_bintahoe(["--quiet", "create-node", "--basedir", c1, "--webport", "0", "--hostname", "localhost"]) + d = self.run_bintahoe(["--quiet", "create-node", "--basedir", c1, + "--webport", "0", + "--hostname", "localhost"]) def _cb(res): out, err, rc_or_sig = res self.failUnlessEqual(rc_or_sig, 0) @@ -540,9 +538,9 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, config = fileutil.read(config_file) self.failUnlessIn('\nweb.port = 0\n', config) - # By writing this file, we get two minutes before the client will exit. This ensures - # that even if the 'stop' command doesn't work (and the test fails), the client should - # still terminate. + # By writing this file, we get two minutes before the client will + # exit. This ensures that even if the 'stop' command doesn't work + # (and the test fails), the client should still terminate. fileutil.write(exit_trigger_file, "") # now it's safe to start the node d.addCallback(_cb) @@ -570,14 +568,13 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, d.addCallback(_cb2) def _node_has_started(): - # this depends upon both files being created atomically - return os.path.exists(node_url_file) and os.path.exists(portnum_file) + return os.path.exists(node_url_file) d.addCallback(lambda res: self.poll(_node_has_started)) def _started(res): - # read the client.port file so we can check that its contents + # read the storage.furl file so we can check that its contents # don't change on restart - self.portnum = fileutil.read(portnum_file) + self.storage_furl = fileutil.read(storage_furl_file) fileutil.write(exit_trigger_file, "") self.failUnless(os.path.exists(twistd_pid_file)) @@ -601,9 +598,10 @@ class RunNode(common_util.SignalMixin, unittest.TestCase, pollmixin.PollMixin, # so poll until it is d.addCallback(lambda res: self.poll(_node_has_started)) - def _check_same_port(res): - self.failUnlessEqual(self.portnum, fileutil.read(portnum_file)) - d.addCallback(_check_same_port) + def _check_same_furl(res): + self.failUnlessEqual(self.storage_furl, + fileutil.read(storage_furl_file)) + d.addCallback(_check_same_furl) # now we can kill it. TODO: On a slow machine, the node might kill # itself before we get a chance to, especially if spawning the From 3b172895693a982ff0b6464c475280abdc3fd25e Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 9 Sep 2016 18:50:51 -0700 Subject: [PATCH 9/9] create_node: simplify validation, clean up tests --- src/allmydata/scripts/create_node.py | 46 +++++++++++++++++---------- src/allmydata/test/cli/test_create.py | 27 +++++++++------- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/src/allmydata/scripts/create_node.py b/src/allmydata/scripts/create_node.py index a68b1f94b..cc9ec834c 100644 --- a/src/allmydata/scripts/create_node.py +++ b/src/allmydata/scripts/create_node.py @@ -1,5 +1,5 @@ import os -from twisted.python import usage +from twisted.python.usage import UsageError from allmydata.scripts.common import BasedirOptions, NoDefaultBasedirOptions from allmydata.scripts.default_nodedir import _default_nodedir from allmydata.util.assertutil import precondition @@ -23,21 +23,35 @@ WHERE_OPTS = [ ("port", None, None, "Specify the server endpoint to listen on for this node."), ] -def validate_where_options(options): - if options['hostname'] and options['port']: - raise usage.UsageError("The --hostname option cannot be used with the --port option.") - if options['hostname'] and options['location']: - raise usage.UsageError("The --hostname option cannot be used with the --location option.") - if not options['hostname'] and (options['location'] and not options['port']): - raise usage.UsageError("The --location option must be used with the --port option.") - if not options['hostname'] and (options['port'] and not options['location']): - raise usage.UsageError("The --port option must be used with the --location option.") - if (options['listen'] != "tcp") and options['hostname']: - raise usage.UsageError("The listener type must be TCP to use --hostname option.") - if options['listen'] == "tcp" and not options['hostname']: - raise usage.UsageError("--listen=tcp requires --hostname=") - if options['listen'] not in ["tcp", "tor", "i2p"]: - raise usage.UsageError("The listener type must set to one of: tcp, tor, i2p.") +def validate_where_options(o): + # --location and --port: overrides all others, rejects all others + if o['location'] and not o['port']: + raise UsageError("--location must be used with --port") + if o['port'] and not o['location']: + raise UsageError("--port must be used with --location") + + if o['location'] and o['port']: + if o['hostname']: + raise UsageError("--hostname cannot be used with --location/--port") + # TODO: really, we should reject an explicit --listen= option (we + # want them to omit it entirely, because --location/--port would + # override anything --listen= might allocate). For now, just let it + # pass, because that allows us to use --listen=tcp as the default in + # optParameters, which (I think) gets included in the rendered --help + # output, which is useful. In the future, let's reconsider the value + # of that --help text (or achieve that documentation in some other + # way), change the default to None, complain here if it's not None, + # then change parseArgs() to transform the None into "tcp" + else: + # no --location and --port? expect --listen= (maybe the default), and --listen=tcp requires --hostname + listeners = o['listen'].split(",") + if 'tcp' in listeners and not o['hostname']: + raise UsageError("--listen=tcp requires --hostname=") + if 'tcp' not in listeners and o['hostname']: + raise UsageError("--listen= must be tcp to use --hostname") + for l in listeners: + if l not in ["tcp", "tor", "i2p"]: + raise UsageError("--listen= must be: tcp, tor, i2p") class _CreateBaseOptions(BasedirOptions): optFlags = [ diff --git a/src/allmydata/test/cli/test_create.py b/src/allmydata/test/cli/test_create.py index f08444916..08ce2d83e 100644 --- a/src/allmydata/test/cli/test_create.py +++ b/src/allmydata/test/cli/test_create.py @@ -50,14 +50,15 @@ class Config(unittest.TestCase): @defer.inlineCallbacks def test_node(self): basedir = self.mktemp() - rc, out, err = yield run_cli("create-node", basedir) + rc, out, err = yield run_cli("create-node", "--hostname=foo", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True) @defer.inlineCallbacks def test_node_hide_ip(self): basedir = self.mktemp() - rc, out, err = yield run_cli("create-node", "--hide-ip", basedir) + rc, out, err = yield run_cli("create-node", "--hide-ip", + "--hostname=foo", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False) @@ -109,31 +110,33 @@ class Config(unittest.TestCase): e = self.assertRaises(usage.UsageError, parse_cli, "create-node", "--port=unix:/var/tahoe/socket") - self.assertEqual(str(e), "The --port option must be used with the --location option.") + self.assertEqual(str(e), "--port must be used with --location") def test_node_location_only(self): e = self.assertRaises(usage.UsageError, parse_cli, "create-node", "--location=tor:myservice.onion:12345") - self.assertEqual(str(e), "The --location option must be used with the --port option.") + self.assertEqual(str(e), "--location must be used with --port") - @defer.inlineCallbacks - def test_introducer(self): + def test_introducer_no_hostname(self): basedir = self.mktemp() - rc, out, err = yield run_cli("create-introducer", basedir) - cfg = self.read_config(basedir) - self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True) + e = self.assertRaises(usage.UsageError, parse_cli, + "create-introducer", basedir) + self.assertEqual(str(e), "--listen=tcp requires --hostname=") @defer.inlineCallbacks def test_introducer_hide_ip(self): basedir = self.mktemp() - rc, out, err = yield run_cli("create-introducer", "--hide-ip", basedir) + rc, out, err = yield run_cli("create-introducer", "--hide-ip", + "--hostname=foo", basedir) cfg = self.read_config(basedir) self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), False) @defer.inlineCallbacks def test_introducer_hostname(self): basedir = self.mktemp() - rc, out, err = yield run_cli("create-introducer", "--hostname=computer", basedir) + rc, out, err = yield run_cli("create-introducer", + "--hostname=foo", basedir) cfg = self.read_config(basedir) - self.assertTrue("computer" in cfg.get("node", "tub.location")) + self.assertTrue("foo" in cfg.get("node", "tub.location")) + self.assertEqual(cfg.getboolean("node", "reveal-IP-address"), True)