From 35810a569233a6e9313944f646571920b1234f07 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Wed, 31 Jan 2018 11:30:46 -0700
Subject: [PATCH 01/38] pull 'basedir' entirely into _Config

Put all config-related methods into _Config; change
code to ask config for paths instead of using basedir;
add some better docstrings
---
 src/allmydata/client.py                   | 135 +++++++------
 src/allmydata/control.py                  |   2 +-
 src/allmydata/introducer/server.py        |  18 +-
 src/allmydata/node.py                     | 222 ++++++++++++----------
 src/allmydata/test/no_network.py          |  12 +-
 src/allmydata/test/test_connections.py    |   8 +-
 src/allmydata/test/test_i2p_provider.py   |  44 ++---
 src/allmydata/test/test_node.py           |  67 ++++---
 src/allmydata/test/test_system.py         |   2 +-
 src/allmydata/test/test_tor_provider.py   |  57 +++---
 src/allmydata/test/web/test_grid.py       |   3 +-
 src/allmydata/test/web/test_introducer.py |   2 +-
 src/allmydata/util/i2p_provider.py        |   3 +-
 src/allmydata/util/tor_provider.py        |  11 +-
 14 files changed, 304 insertions(+), 282 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index d9b51a3d2..cb0978bc4 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -29,6 +29,7 @@ from allmydata.interfaces import IStatsProducer, SDMF_VERSION, MDMF_VERSION
 from allmydata.nodemaker import NodeMaker
 from allmydata.blacklist import Blacklist
 from allmydata.node import OldConfigOptionError, _common_config_sections
+from allmydata.node import read_config
 
 
 KiB=1024
@@ -156,12 +157,10 @@ class Terminator(service.Service):
 
 #@defer.inlineCallbacks
 def create_client(basedir=u"."):
-    from allmydata.node import read_config
     config = read_config(basedir, u"client.port", _valid_config_sections=_valid_config_sections)
     #defer.returnValue(
     return _Client(
             config,
-            basedir=basedir
         )
     #)
 
@@ -188,8 +187,8 @@ class _Client(node.Node, pollmixin.PollMixin):
                                    "max_segment_size": 128*KiB,
                                    }
 
-    def __init__(self, config, basedir=u"."):
-        node.Node.__init__(self, config, basedir=basedir)
+    def __init__(self, config):
+        node.Node.__init__(self, config)
         # All tub.registerReference must happen *after* we upcall, since
         # that's what does tub.setLocation()
         self._magic_folders = dict()
@@ -203,13 +202,13 @@ class _Client(node.Node, pollmixin.PollMixin):
         self.init_storage()
         self.init_control()
         self._key_generator = KeyGenerator()
-        key_gen_furl = self.get_config("client", "key_generator.furl", None)
+        key_gen_furl = config.get_config("client", "key_generator.furl", None)
         if key_gen_furl:
             log.msg("[client]key_generator.furl= is now ignored, see #2783")
         self.init_client()
         self.load_static_servers()
         self.helper = None
-        if self.get_config("helper", "enabled", False, boolean=True):
+        if config.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)")
@@ -221,8 +220,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         # If the node sees an exit_trigger file, it will poll every second to see
         # whether the file still exists, and what its mtime is. If the file does not
         # exist or has not been modified for a given timeout, the node will exit.
-        exit_trigger_file = os.path.join(self.basedir,
-                                         self.EXIT_TRIGGER_FILE)
+        exit_trigger_file = self.config.get_config_path(self.EXIT_TRIGGER_FILE)
         if os.path.exists(exit_trigger_file):
             age = time.time() - os.stat(exit_trigger_file)[stat.ST_MTIME]
             self.log("%s file noticed (%ds old), starting timer" % (self.EXIT_TRIGGER_FILE, age))
@@ -231,17 +229,17 @@ class _Client(node.Node, pollmixin.PollMixin):
 
         # this needs to happen last, so it can use getServiceNamed() to
         # acquire references to StorageServer and other web-statusable things
-        webport = self.get_config("node", "web.port", None)
+        webport = self.config.get_config("node", "web.port", None)
         if webport:
             self.init_web(webport) # strports string
 
     def _sequencer(self):
-        seqnum_s = self.get_config_from_file("announcement-seqnum")
+        seqnum_s = self.config.get_config_from_file("announcement-seqnum")
         if not seqnum_s:
             seqnum_s = "0"
         seqnum = int(seqnum_s.strip())
         seqnum += 1 # increment
-        self.write_config("announcement-seqnum", "%d\n" % seqnum)
+        self.config.write_config_file("announcement-seqnum", "%d\n" % seqnum)
         nonce = _make_secret().strip()
         return seqnum, nonce
 
@@ -249,7 +247,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         self.introducer_clients = []
         self.introducer_furls = []
 
-        introducers_yaml_filename = os.path.join(self.basedir, "private", "introducers.yaml")
+        introducers_yaml_filename = self.config.get_private_path("introducers.yaml")
         introducers_filepath = FilePath(introducers_yaml_filename)
 
         try:
@@ -265,7 +263,7 @@ class _Client(node.Node, pollmixin.PollMixin):
             raise ValueError("'default' introducer furl cannot be specified in introducers.yaml; please fix impossible configuration.")
 
         # read furl from tahoe.cfg
-        tahoe_cfg_introducer_furl = self.get_config("client", "introducer.furl", None)
+        tahoe_cfg_introducer_furl = self.config.get_config("client", "introducer.furl", None)
         if tahoe_cfg_introducer_furl == "None":
             raise ValueError("tahoe.cfg has invalid 'introducer.furl = None':"
                              " to disable it, use 'introducer.furl ='"
@@ -274,19 +272,19 @@ class _Client(node.Node, pollmixin.PollMixin):
             introducers[u'default'] = {'furl':tahoe_cfg_introducer_furl}
 
         for petname, introducer in introducers.items():
-            introducer_cache_filepath = FilePath(os.path.join(self.basedir, "private", "introducer_{}_cache.yaml".format(petname)))
+            introducer_cache_filepath = FilePath(self.config.get_private_path("introducer_{}_cache.yaml".format(petname)))
             ic = IntroducerClient(self.tub, introducer['furl'].encode("ascii"),
                                   self.nickname,
                                   str(allmydata.__full_version__),
                                   str(self.OLDEST_SUPPORTED_VERSION),
-                                  self.get_app_versions(), self._sequencer,
+                                  self.config.get_app_versions(), self._sequencer,
                                   introducer_cache_filepath)
             self.introducer_clients.append(ic)
             self.introducer_furls.append(introducer['furl'])
             ic.setServiceParent(self)
 
     def init_stats_provider(self):
-        gatherer_furl = self.get_config("client", "stats_gatherer.furl", None)
+        gatherer_furl = self.config.get_config("client", "stats_gatherer.furl", None)
         self.stats_provider = StatsProvider(self, gatherer_furl)
         self.add_service(self.stats_provider)
         self.stats_provider.register_producer(self)
@@ -295,10 +293,10 @@ class _Client(node.Node, pollmixin.PollMixin):
         return { 'node.uptime': time.time() - self.started_timestamp }
 
     def init_secrets(self):
-        lease_s = self.get_or_create_private_config("secret", _make_secret)
+        lease_s = self.config.get_or_create_private_config("secret", _make_secret)
         lease_secret = base32.a2b(lease_s)
-        convergence_s = self.get_or_create_private_config('convergence',
-                                                          _make_secret)
+        convergence_s = self.config.get_or_create_private_config('convergence',
+                                                                 _make_secret)
         self.convergence = base32.a2b(convergence_s)
         self._secret_holder = SecretHolder(lease_secret, self.convergence)
 
@@ -308,9 +306,9 @@ class _Client(node.Node, pollmixin.PollMixin):
         def _make_key():
             sk_vs,vk_vs = keyutil.make_keypair()
             return sk_vs+"\n"
-        sk_vs = self.get_or_create_private_config("node.privkey", _make_key)
+        sk_vs = self.config.get_or_create_private_config("node.privkey", _make_key)
         sk,vk_vs = keyutil.parse_privkey(sk_vs.strip())
-        self.write_config("node.pubkey", vk_vs+"\n")
+        self.config.write_config_file("node.pubkey", vk_vs+"\n")
         self._node_key = sk
 
     def get_long_nodeid(self):
@@ -322,7 +320,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         return idlib.nodeid_b2a(self.nodeid)
 
     def _init_permutation_seed(self, ss):
-        seed = self.get_config_from_file("permutation-seed")
+        seed = self.config.get_config_from_file("permutation-seed")
         if not seed:
             have_shares = ss.have_shares()
             if have_shares:
@@ -339,24 +337,24 @@ class _Client(node.Node, pollmixin.PollMixin):
                 # pubkey-based serverid
                 vk_bytes = self._node_key.get_verifying_key_bytes()
                 seed = base32.b2a(vk_bytes)
-            self.write_config("permutation-seed", seed+"\n")
+            self.config.write_config_file("permutation-seed", seed+"\n")
         return seed.strip()
 
     def init_storage(self):
         # should we run a storage server (and publish it for others to use)?
-        if not self.get_config("storage", "enabled", True, boolean=True):
+        if not self.config.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)
+        readonly = self.config.get_config("storage", "readonly", False, boolean=True)
 
         config_storedir = self.get_config(
             "storage", "storage_dir", self.STOREDIR,
         ).decode('utf-8')
-        storedir = os.path.join(self.basedir, config_storedir)
+        storedir = self.config.get_config_path(config_storedir)
 
-        data = self.get_config("storage", "reserved_space", None)
+        data = self.config.get_config("storage", "reserved_space", None)
         try:
             reserved = parse_abbreviated_size(data)
         except ValueError:
@@ -365,28 +363,28 @@ class _Client(node.Node, pollmixin.PollMixin):
             raise
         if reserved is None:
             reserved = 0
-        discard = self.get_config("storage", "debug_discard", False,
-                                  boolean=True)
+        discard = self.config.get_config("storage", "debug_discard", False,
+                                         boolean=True)
 
-        expire = self.get_config("storage", "expire.enabled", False, boolean=True)
+        expire = self.config.get_config("storage", "expire.enabled", False, boolean=True)
         if expire:
-            mode = self.get_config("storage", "expire.mode") # require a mode
+            mode = self.config.get_config("storage", "expire.mode") # require a mode
         else:
-            mode = self.get_config("storage", "expire.mode", "age")
+            mode = self.config.get_config("storage", "expire.mode", "age")
 
-        o_l_d = self.get_config("storage", "expire.override_lease_duration", None)
+        o_l_d = self.config.get_config("storage", "expire.override_lease_duration", None)
         if o_l_d is not None:
             o_l_d = parse_duration(o_l_d)
 
         cutoff_date = None
         if mode == "cutoff-date":
-            cutoff_date = self.get_config("storage", "expire.cutoff_date")
+            cutoff_date = self.config.get_config("storage", "expire.cutoff_date")
             cutoff_date = parse_date(cutoff_date)
 
         sharetypes = []
-        if self.get_config("storage", "expire.immutable", True, boolean=True):
+        if self.config.get_config("storage", "expire.immutable", True, boolean=True):
             sharetypes.append("immutable")
-        if self.get_config("storage", "expire.mutable", True, boolean=True):
+        if self.config.get_config("storage", "expire.mutable", True, boolean=True):
             sharetypes.append("mutable")
         expiration_sharetypes = tuple(sharetypes)
 
@@ -402,7 +400,7 @@ class _Client(node.Node, pollmixin.PollMixin):
                            expiration_sharetypes=expiration_sharetypes)
         self.add_service(ss)
 
-        furl_file = os.path.join(self.basedir, "private", "storage.furl").encode(get_filesystem_encoding())
+        furl_file = self.config.get_private_path("storage.furl").encode(get_filesystem_encoding())
         furl = self.tub.registerReference(ss, furlFile=furl_file)
         ann = {"anonymous-storage-FURL": furl,
                "permutation-seed-base32": self._init_permutation_seed(ss),
@@ -411,14 +409,14 @@ class _Client(node.Node, pollmixin.PollMixin):
             ic.publish("storage", ann, self._node_key)
 
     def init_client(self):
-        helper_furl = self.get_config("client", "helper.furl", None)
+        helper_furl = self.config.get_config("client", "helper.furl", None)
         if helper_furl in ("None", ""):
             helper_furl = None
 
         DEP = self.encoding_params
-        DEP["k"] = int(self.get_config("client", "shares.needed", DEP["k"]))
-        DEP["n"] = int(self.get_config("client", "shares.total", DEP["n"]))
-        DEP["happy"] = int(self.get_config("client", "shares.happy", DEP["happy"]))
+        DEP["k"] = int(self.config.get_config("client", "shares.needed", DEP["k"]))
+        DEP["n"] = int(self.config.get_config("client", "shares.total", DEP["n"]))
+        DEP["happy"] = int(self.config.get_config("client", "shares.happy", DEP["happy"]))
 
         # for the CLI to authenticate to local JSON endpoints
         self._create_auth_token()
@@ -441,7 +439,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         Currently only the URI '/magic' for magic-folder status; other
         endpoints are invited to include this as well, as appropriate.
         """
-        return self.get_private_config('api_auth_token')
+        return self.config.get_private_config('api_auth_token')
 
     def _create_auth_token(self):
         """
@@ -449,7 +447,7 @@ class _Client(node.Node, pollmixin.PollMixin):
 
         This is intentionally re-created every time the node starts.
         """
-        self.write_private_config(
+        self.config.write_private_config(
             'api_auth_token',
             urlsafe_b64encode(os.urandom(32)) + '\n',
         )
@@ -457,7 +455,7 @@ class _Client(node.Node, pollmixin.PollMixin):
     def init_client_storage_broker(self):
         # create a StorageFarmBroker object, for use by Uploader/Downloader
         # (and everybody else who wants to use storage servers)
-        ps = self.get_config("client", "peers.preferred", "").split(",")
+        ps = self.config.get_config("client", "peers.preferred", "").split(",")
         preferred_peers = tuple([p.strip() for p in ps if p != ""])
         sb = storage_client.StorageFarmBroker(permute_peers=True,
                                               tub_maker=self._create_tub,
@@ -476,7 +474,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         Load the servers.yaml file if it exists, and provide the static
         server data to the StorageFarmBroker.
         """
-        fn = os.path.join(self.basedir, "private", "servers.yaml")
+        fn = self.config.get_private_path("servers.yaml")
         servers_filepath = FilePath(fn)
         try:
             with servers_filepath.open() as f:
@@ -489,11 +487,11 @@ class _Client(node.Node, pollmixin.PollMixin):
             pass
 
     def init_blacklist(self):
-        fn = os.path.join(self.basedir, "access.blacklist")
+        fn = self.config.get_config_path("access.blacklist")
         self.blacklist = Blacklist(fn)
 
     def init_nodemaker(self):
-        default = self.get_config("client", "mutable.format", default="SDMF")
+        default = self.config.get_config("client", "mutable.format", default="SDMF")
         if default.upper() == "MDMF":
             self.mutable_file_default = MDMF_VERSION
         else:
@@ -515,10 +513,10 @@ class _Client(node.Node, pollmixin.PollMixin):
         c = ControlServer()
         c.setServiceParent(self)
         control_url = self.control_tub.registerReference(c)
-        self.write_private_config("control.furl", control_url + "\n")
+        self.config.write_private_config("control.furl", control_url + "\n")
 
     def init_helper(self):
-        self.helper = Helper(os.path.join(self.basedir, "helper"),
+        self.helper = Helper(self.config.get_config_path("helper"),
                              self.storage_broker, self._secret_holder,
                              self.stats_provider, self.history)
         # TODO: this is confusing. BASEDIR/private/helper.furl is created by
@@ -526,8 +524,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         # to use the helper. I like having the filename be the same, since
         # that makes 'cp' work smoothly, but the difference between config
         # inputs and generated outputs is hard to see.
-        helper_furlfile = os.path.join(self.basedir,
-                                       "private", "helper.furl").encode(get_filesystem_encoding())
+        helper_furlfile = self.config.get_private_path("helper.furl").encode(get_filesystem_encoding())
         self.tub.registerReference(self.helper, furlFile=helper_furlfile)
 
     def set_default_mutable_keysize(self, keysize):
@@ -537,35 +534,35 @@ class _Client(node.Node, pollmixin.PollMixin):
         self.log("init_web(webport=%s)", args=(webport,))
 
         from allmydata.webish import WebishServer
-        nodeurl_path = os.path.join(self.basedir, "node.url")
-        staticdir_config = self.get_config("node", "web.static", "public_html").decode("utf-8")
-        staticdir = abspath_expanduser_unicode(staticdir_config, base=self.basedir)
+        nodeurl_path = self.config.get_config_path("node.url")
+        staticdir_config = self.config.get_config("node", "web.static", "public_html").decode("utf-8")
+        staticdir = self.config.get_config_path(staticdir_config)
         ws = WebishServer(self, webport, nodeurl_path, staticdir)
         self.add_service(ws)
 
     def init_ftp_server(self):
-        if self.get_config("ftpd", "enabled", False, boolean=True):
+        if self.config.get_config("ftpd", "enabled", False, boolean=True):
             accountfile = from_utf8_or_none(
-                self.get_config("ftpd", "accounts.file", None))
+                self.config.get_config("ftpd", "accounts.file", None))
             if accountfile:
-                accountfile = abspath_expanduser_unicode(accountfile, base=self.basedir)
-            accounturl = self.get_config("ftpd", "accounts.url", None)
-            ftp_portstr = self.get_config("ftpd", "port", "8021")
+                accountfile = self.config.get_config_path(accountfile)
+            accounturl = self.config.get_config("ftpd", "accounts.url", None)
+            ftp_portstr = self.config.get_config("ftpd", "port", "8021")
 
             from allmydata.frontends import ftpd
             s = ftpd.FTPServer(self, accountfile, accounturl, ftp_portstr)
             s.setServiceParent(self)
 
     def init_sftp_server(self):
-        if self.get_config("sftpd", "enabled", False, boolean=True):
+        if self.config.get_config("sftpd", "enabled", False, boolean=True):
             accountfile = from_utf8_or_none(
-                self.get_config("sftpd", "accounts.file", None))
+                self.config.get_config("sftpd", "accounts.file", None))
             if accountfile:
-                accountfile = abspath_expanduser_unicode(accountfile, base=self.basedir)
-            accounturl = self.get_config("sftpd", "accounts.url", None)
-            sftp_portstr = self.get_config("sftpd", "port", "8022")
-            pubkey_file = from_utf8_or_none(self.get_config("sftpd", "host_pubkey_file"))
-            privkey_file = from_utf8_or_none(self.get_config("sftpd", "host_privkey_file"))
+                accountfile = self.config.get_config_path(accountfile)
+            accounturl = self.config.get_config("sftpd", "accounts.url", None)
+            sftp_portstr = self.config.get_config("sftpd", "port", "8022")
+            pubkey_file = from_utf8_or_none(self.config.get_config("sftpd", "host_pubkey_file"))
+            privkey_file = from_utf8_or_none(self.config.get_config("sftpd", "host_privkey_file"))
 
             from allmydata.frontends import sftpd
             s = sftpd.SFTPServer(self, accountfile, accounturl,
@@ -574,15 +571,15 @@ class _Client(node.Node, pollmixin.PollMixin):
 
     def init_magic_folder(self):
         #print "init_magic_folder"
-        if self.get_config("drop_upload", "enabled", False, boolean=True):
+        if self.config.get_config("drop_upload", "enabled", False, boolean=True):
             raise OldConfigOptionError("The [drop_upload] section must be renamed to [magic_folder].\n"
                                        "See docs/frontends/magic-folder.rst for more information.")
 
-        if self.get_config("magic_folder", "enabled", False, boolean=True):
+        if self.config.get_config("magic_folder", "enabled", False, boolean=True):
             from allmydata.frontends import magic_folder
 
             try:
-                magic_folders = magic_folder.load_magic_folders(self.basedir)
+                magic_folders = magic_folder.load_magic_folders(self.config._basedir)
             except Exception as e:
                 log.msg("Error loading magic-folder config: {}".format(e))
                 raise
diff --git a/src/allmydata/control.py b/src/allmydata/control.py
index 568ebeaf5..33bbe22e6 100644
--- a/src/allmydata/control.py
+++ b/src/allmydata/control.py
@@ -150,7 +150,7 @@ class SpeedTest:
         self.size = size
         self.mutable_mode = mutable
         self.uris = {}
-        self.basedir = os.path.join(self.parent.basedir, "_speed_test_data")
+        self.basedir = self.parent.config.get_config_path("_speed_test_data")
 
     def run(self):
         self.create_data()
diff --git a/src/allmydata/introducer/server.py b/src/allmydata/introducer/server.py
index 6c3a45b8b..5886c991e 100644
--- a/src/allmydata/introducer/server.py
+++ b/src/allmydata/introducer/server.py
@@ -6,7 +6,6 @@ from foolscap.api import Referenceable
 import allmydata
 from allmydata import node
 from allmydata.util import log, rrefutil
-from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.introducer.interfaces import \
      RIIntroducerPublisherAndSubscriberService_v2
 from allmydata.introducer.common import unsign_from_foolscap, \
@@ -27,7 +26,6 @@ def create_introducer(basedir=u"."):
     #defer.returnValue(
     return _IntroducerNode(
             config,
-            basedir=basedir
         )
     #)
 
@@ -35,8 +33,8 @@ def create_introducer(basedir=u"."):
 class _IntroducerNode(node.Node):
     NODETYPE = "introducer"
 
-    def __init__(self, config, basedir=u"."):
-        node.Node.__init__(self, config, basedir=basedir)
+    def __init__(self, config):
+        node.Node.__init__(self, config)
         self.init_introducer()
         webport = self.get_config("node", "web.port", None)
         if webport:
@@ -46,11 +44,11 @@ class _IntroducerNode(node.Node):
         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)
+        introducerservice = IntroducerService()
         self.add_service(introducerservice)
 
-        old_public_fn = os.path.join(self.basedir, u"introducer.furl")
-        private_fn = os.path.join(self.basedir, u"private", u"introducer.furl")
+        old_public_fn = self.config.get_config_path(u"introducer.furl")
+        private_fn = self.config.get_private_path(u"introducer.furl")
 
         if os.path.exists(old_public_fn):
             if os.path.exists(private_fn):
@@ -73,9 +71,9 @@ class _IntroducerNode(node.Node):
         self.log("init_web(webport=%s)", args=(webport,), umid="2bUygA")
 
         from allmydata.webish import IntroducerWebishServer
-        nodeurl_path = os.path.join(self.basedir, u"node.url")
+        nodeurl_path = self.config.get_config_path(u"node.url")
         config_staticdir = self.get_config("node", "web.static", "public_html").decode('utf-8')
-        staticdir = abspath_expanduser_unicode(config_staticdir, base=self.basedir)
+        staticdir = self.config.get_config_path(config_staticdir)
         ws = IntroducerWebishServer(self, webport, nodeurl_path, staticdir)
         self.add_service(ws)
 
@@ -89,7 +87,7 @@ class IntroducerService(service.MultiService, Referenceable):
                 "application-version": str(allmydata.__full_version__),
                 }
 
-    def __init__(self, basedir="."):
+    def __init__(self):
         service.MultiService.__init__(self)
         self.introducer_url = None
         # 'index' is (service_name, key_s, tubid), where key_s or tubid is
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 05cf146bc..0d738cc6c 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -146,14 +146,19 @@ def read_config(basedir, portnumfile, generated_files=[], _valid_config_sections
     except EnvironmentError:
         if os.path.exists(config_fname):
             raise
+
     configutil.validate_config(config_fname, parser, _valid_config_sections())
-    return _Config(parser, portnumfile, config_fname)
+
+    # make sure we have a private configuration area
+    fileutil.make_dirs(os.path.join(basedir, "private"), 0o700)
+    return _Config(parser, portnumfile, basedir, config_fname)
 
 
-def config_from_string(config_str, portnumfile):
+def config_from_string(config_str, portnumfile, basedir):
+    # load configuration from in-memory string
     parser = ConfigParser.SafeConfigParser()
     parser.readfp(BytesIO(config_str))
-    return _Config(parser, portnumfile, '<in-memory>')
+    return _Config(parser, portnumfile, basedir, '<in-memory>')
 
 
 def _error_about_old_config_files(basedir, generated_files):
@@ -189,11 +194,11 @@ class _Config(object):
     as a helper instead.
     """
 
-    def __init__(self, configparser, portnum_fname, config_fname):
+    def __init__(self, configparser, portnum_fname, basedir, config_fname):
         # XXX I think this portnumfile thing is just legacy?
         self.portnum_fname = portnum_fname
-        self._config_fname = config_fname
-
+        self._basedir = abspath_expanduser_unicode(unicode(basedir))
+        self._config_fname = config_fname  # the actual fname "configparser" came from
         self.config = configparser
 
         nickname_utf8 = self.get_config("node", "nickname", "<unspecified>")
@@ -211,6 +216,22 @@ class _Config(object):
             if os.path.exists(self.config_fname):
                 raise
 
+    def get_app_versions(self):
+        # TODO: merge this with allmydata.get_package_versions
+        return dict(app_versions.versions)
+
+    def write_config_file(self, name, value, mode="w"):
+        """
+        writes the given 'value' into a file called 'name' in the config
+        directory
+        """
+        fn = os.path.join(self._basedir, name)
+        try:
+            fileutil.write(fn, value, mode)
+        except EnvironmentError as e:
+            log.msg("Unable to write config file '{}'".format(fn))
+            log.err(e)
+
     def get_config(self, section, option, default=_None, boolean=False):
         try:
             if boolean:
@@ -232,6 +253,87 @@ class _Config(object):
                 )
             return default
 
+    def get_config_from_file(self, name, required=False):
+        """Get the (string) contents of a config file, or None if the file
+        did not exist. If required=True, raise an exception rather than
+        returning None. Any leading or trailing whitespace will be stripped
+        from the data."""
+        fn = os.path.join(self._basedir, name)
+        try:
+            return fileutil.read(fn).strip()
+        except EnvironmentError:
+            if not required:
+                return None
+            raise
+
+    def get_or_create_private_config(self, name, default=_None):
+        """Try to get the (string) contents of a private config file (which
+        is a config file that resides within the subdirectory named
+        'private'), and return it. Any leading or trailing whitespace will be
+        stripped from the data.
+
+        If the file does not exist, and default is not given, report an error.
+        If the file does not exist and a default is specified, try to create
+        it using that default, and then return the value that was written.
+        If 'default' is a string, use it as a default value. If not, treat it
+        as a zero-argument callable that is expected to return a string.
+        """
+        privname = os.path.join(self._basedir, "private", name)
+        try:
+            value = fileutil.read(privname)
+        except EnvironmentError:
+            if os.path.exists(privname):
+                raise
+            if default is _None:
+                raise MissingConfigEntry("The required configuration file %s is missing."
+                                         % (quote_output(privname),))
+            if isinstance(default, basestring):
+                value = default
+            else:
+                value = default()
+            fileutil.write(privname, value)
+        return value.strip()
+
+    def write_private_config(self, name, value):
+        """Write the (string) contents of a private config file (which is a
+        config file that resides within the subdirectory named 'private'), and
+        return it.
+        """
+        privname = os.path.join(self._basedir, "private", name)
+        with open(privname, "w") as f:
+            f.write(value)
+
+    def get_private_config(self, name, default=_None):
+        """Read the (string) contents of a private config file (which is a
+        config file that resides within the subdirectory named 'private'),
+        and return it. Return a default, or raise an error if one was not
+        given.
+        """
+        privname = os.path.join(self._basedir, "private", name)
+        try:
+            return fileutil.read(privname).strip()
+        except EnvironmentError:
+            if os.path.exists(privname):
+                raise
+            if default is _None:
+                raise MissingConfigEntry("The required configuration file %s is missing."
+                                         % (quote_output(privname),))
+            return default
+
+    def get_private_path(self, *args):
+        """
+        returns an absolute path inside the 'private' directory with any
+        extra args join()-ed
+        """
+        return os.path.join(self._basedir, "private", *args)
+
+    def get_config_path(self, *args):
+        """
+        returns an absolute path inside the config directory with any
+        extra args join()-ed
+        """
+        return os.path.join(self._basedir, *args)
+
     @staticmethod
     def _contains_unescaped_hash(item):
         characters = iter(item)
@@ -253,17 +355,15 @@ class Node(service.MultiService):
     CERTFILE = "node.pem"
     GENERATED_FILES = []
 
-    def __init__(self, config, basedir=u"."):
+    def __init__(self, config):
         """
-        Initialize the node with the given configuration. It's base directory
+        Initialize the node with the given configuration. Its base directory
         is the current directory by default.
         """
         service.MultiService.__init__(self)
-        # ideally, this would only be in _Config (or otherwise abstracted)
-        self.basedir = abspath_expanduser_unicode(unicode(basedir))
         # XXX don't write files in ctor!
-        fileutil.make_dirs(os.path.join(self.basedir, "private"), 0700)
-        with open(os.path.join(self.basedir, "private", "README"), "w") as f:
+        fileutil.make_dirs(os.path.join(config._basedir, "private"), 0700)
+        with open(os.path.join(config._basedir, "private", "README"), "w") as f:
             f.write(PRIV_README)
 
         self.config = config
@@ -292,7 +392,9 @@ class Node(service.MultiService):
         Initialize/create a directory for temporary files.
         """
         tempdir_config = self.config.get_config("node", "tempdir", "tmp").decode('utf-8')
-        tempdir = abspath_expanduser_unicode(tempdir_config, base=self.basedir)
+        tempdir = self.config.get_config_path(tempdir_config)
+        tempdir0 = abspath_expanduser_unicode(tempdir_config, base=self.config._basedir)
+        assert tempdir == tempdir0
         if not os.path.exists(tempdir):
             fileutil.make_dirs(tempdir)
         tempfile.tempdir = tempdir
@@ -308,12 +410,12 @@ class Node(service.MultiService):
         self._reveal_ip = self.config.get_config("node", "reveal-IP-address", True,
                                                  boolean=True)
     def create_i2p_provider(self):
-        self._i2p_provider = i2p_provider.Provider(self.basedir, self.config, reactor)
+        self._i2p_provider = i2p_provider.Provider(self.config, reactor)
         self._i2p_provider.check_dest_config()
         self._i2p_provider.setServiceParent(self)
 
     def create_tor_provider(self):
-        self._tor_provider = tor_provider.Provider(self.basedir, self.config, reactor)
+        self._tor_provider = tor_provider.Provider(self.config, reactor)
         self._tor_provider.check_onion_config()
         self._tor_provider.setServiceParent(self)
 
@@ -475,11 +577,11 @@ class Node(service.MultiService):
         return tubport, location
 
     def create_main_tub(self):
-        certfile = os.path.join(self.basedir, "private", self.CERTFILE)
+        certfile = self.config.get_private_path(self.CERTFILE)
         self.tub = self._create_tub(certFile=certfile)
 
         self.nodeid = b32decode(self.tub.tubID.upper()) # binary format
-        self.write_config("my_nodeid", b32encode(self.nodeid).lower() + "\n")
+        self.config.write_config_file("my_nodeid", b32encode(self.nodeid).lower() + "\n")
         self.short_nodeid = b32encode(self.nodeid).lower()[:8] # for printing
         cfg_tubport = self.config.get_config("node", "tub.port", None)
         cfg_location = self.config.get_config("node", "tub.location", None)
@@ -538,86 +640,6 @@ class Node(service.MultiService):
         self.log("Log Tub location set to %s" % (location,))
         self.log_tub.setServiceParent(self)
 
-    def get_app_versions(self):
-        # TODO: merge this with allmydata.get_package_versions
-        return dict(app_versions.versions)
-
-    def get_config_from_file(self, name, required=False):
-        """Get the (string) contents of a config file, or None if the file
-        did not exist. If required=True, raise an exception rather than
-        returning None. Any leading or trailing whitespace will be stripped
-        from the data."""
-        fn = os.path.join(self.basedir, name)
-        try:
-            return fileutil.read(fn).strip()
-        except EnvironmentError:
-            if not required:
-                return None
-            raise
-
-    def write_private_config(self, name, value):
-        """Write the (string) contents of a private config file (which is a
-        config file that resides within the subdirectory named 'private'), and
-        return it.
-        """
-        privname = os.path.join(self.basedir, "private", name)
-        with open(privname, "w") as f:
-            f.write(value)
-
-    def get_private_config(self, name, default=_None):
-        """Read the (string) contents of a private config file (which is a
-        config file that resides within the subdirectory named 'private'),
-        and return it. Return a default, or raise an error if one was not
-        given.
-        """
-        privname = os.path.join(self.basedir, "private", name)
-        try:
-            return fileutil.read(privname).strip()
-        except EnvironmentError:
-            if os.path.exists(privname):
-                raise
-            if default is _None:
-                raise MissingConfigEntry("The required configuration file %s is missing."
-                                         % (quote_output(privname),))
-            return default
-
-    def get_or_create_private_config(self, name, default=_None):
-        """Try to get the (string) contents of a private config file (which
-        is a config file that resides within the subdirectory named
-        'private'), and return it. Any leading or trailing whitespace will be
-        stripped from the data.
-
-        If the file does not exist, and default is not given, report an error.
-        If the file does not exist and a default is specified, try to create
-        it using that default, and then return the value that was written.
-        If 'default' is a string, use it as a default value. If not, treat it
-        as a zero-argument callable that is expected to return a string.
-        """
-        privname = os.path.join(self.basedir, "private", name)
-        try:
-            value = fileutil.read(privname)
-        except EnvironmentError:
-            if os.path.exists(privname):
-                raise
-            if default is _None:
-                raise MissingConfigEntry("The required configuration file %s is missing."
-                                         % (quote_output(privname),))
-            if isinstance(default, basestring):
-                value = default
-            else:
-                value = default()
-            fileutil.write(privname, value)
-        return value.strip()
-
-    def write_config(self, name, value, mode="w"):
-        """Write a string to a config file."""
-        fn = os.path.join(self.basedir, name)
-        try:
-            fileutil.write(fn, value, mode)
-        except EnvironmentError, e:
-            self.log("Unable to write config file '%s'" % fn)
-            self.log(e)
-
     def startService(self):
         # Note: this class can be started and stopped at most once.
         self.log("Node.startService")
@@ -658,7 +680,7 @@ class Node(service.MultiService):
                     ob.formatTime = newmeth
         # TODO: twisted >2.5.0 offers maxRotatedFiles=50
 
-        lgfurl_file = os.path.join(self.basedir, "private", "logport.furl").encode(get_filesystem_encoding())
+        lgfurl_file = self.config.get_private_path("logport.furl").encode(get_filesystem_encoding())
         if os.path.exists(lgfurl_file):
             os.remove(lgfurl_file)
         self.log_tub.setOption("logport-furlfile", lgfurl_file)
@@ -667,9 +689,9 @@ class Node(service.MultiService):
             # this is in addition to the contents of log-gatherer-furlfile
             self.log_tub.setOption("log-gatherer-furl", lgfurl)
         self.log_tub.setOption("log-gatherer-furlfile",
-                               os.path.join(self.basedir, "log_gatherer.furl"))
+                               self.config.get_config_path("log_gatherer.furl"))
 
-        incident_dir = os.path.join(self.basedir, "logs", "incidents")
+        incident_dir = self.config.get_config_path("logs", "incidents")
         foolscap.logging.log.setLogDir(incident_dir.encode(get_filesystem_encoding()))
         twlog.msg("Foolscap logging initialized")
         twlog.msg("Note to developers: twistd.log does not receive very much.")
diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py
index 26199cd92..b95bd0be7 100644
--- a/src/allmydata/test/no_network.py
+++ b/src/allmydata/test/no_network.py
@@ -26,7 +26,8 @@ import treq
 from allmydata.util.assertutil import _assert
 
 from allmydata import uri as tahoe_uri
-from allmydata.client import _Client, _valid_config_sections
+from allmydata.client import _Client
+from allmydata.client import _valid_config_sections as client_valid_config_sections
 from allmydata.storage.server import StorageServer, storage_index_to_dir
 from allmydata.util import fileutil, idlib, hashutil
 from allmydata.util.hashutil import permute_server_hash
@@ -188,8 +189,8 @@ def NoNetworkClient(basedir):
     # XXX FIXME this is just to avoid massive search-replace for now;
     # should be create_nonetwork_client() or something...
     from allmydata.node import read_config
-    config = read_config(basedir, u'client.port', _valid_config_sections=_valid_config_sections)
-    return _NoNetworkClient(config, basedir=basedir)
+    config = read_config(basedir, u'client.port', _valid_config_sections=client_valid_config_sections)
+    return _NoNetworkClient(config)
 
 
 class _NoNetworkClient(_Client):
@@ -401,10 +402,7 @@ class GridTestMixin:
                                 for c in self.g.clients]
 
     def get_clientdir(self, i=0):
-        return self.g.clients[i].basedir
-
-    def set_clientdir(self, basedir, i=0):
-        self.g.clients[i].basedir = basedir
+        return self.g.clients[i].config._basedir
 
     def get_client(self, i=0):
         return self.g.clients[i]
diff --git a/src/allmydata/test/test_connections.py b/src/allmydata/test/test_connections.py
index 707c2f02c..99cce5f6b 100644
--- a/src/allmydata/test/test_connections.py
+++ b/src/allmydata/test/test_connections.py
@@ -10,8 +10,7 @@ from ..util import connection_status
 class FakeNode(Node):
     def __init__(self, config_str):
         from allmydata.node import config_from_string
-        self.config = config_from_string(config_str, "fake.port")
-        self.basedir = "BASEDIR"
+        self.config = config_from_string(config_str, "fake.port", "no-basedir")
         self._reveal_ip = True
         self.services = []
         self.create_i2p_provider()
@@ -60,7 +59,7 @@ class Tor(unittest.TestCase):
                         return_value=h1) as f:
             n = FakeNode(config)
             h = n._make_tor_handler()
-            private_dir = os.path.join(n.basedir, "private")
+            private_dir = n.config.get_config_path("private")
             exp = mock.call(n._tor_provider._make_control_endpoint,
                             takes_status=True)
             self.assertEqual(f.mock_calls, [exp])
@@ -78,7 +77,8 @@ class Tor(unittest.TestCase):
                 d = tp._make_control_endpoint(reactor,
                                               update_status=lambda status: None)
                 cep = self.successResultOf(d)
-        launch_tor.assert_called_with(reactor, executable, private_dir,
+        launch_tor.assert_called_with(reactor, executable,
+                                      os.path.abspath(private_dir),
                                       tp._txtorcon)
         cfs.assert_called_with(reactor, "ep_desc")
         self.assertIs(cep, tcep)
diff --git a/src/allmydata/test/test_i2p_provider.py b/src/allmydata/test/test_i2p_provider.py
index c6117ea0c..53eaa8096 100644
--- a/src/allmydata/test/test_i2p_provider.py
+++ b/src/allmydata/test/test_i2p_provider.py
@@ -193,16 +193,16 @@ class FakeConfig(dict):
 
 class Provider(unittest.TestCase):
     def test_build(self):
-        i2p_provider.Provider("basedir", FakeConfig(), "reactor")
+        i2p_provider.Provider(FakeConfig(), "reactor")
 
     def test_handler_disabled(self):
-        p = i2p_provider.Provider("basedir", FakeConfig(enabled=False),
+        p = i2p_provider.Provider(FakeConfig(enabled=False),
                                   "reactor")
         self.assertEqual(p.get_i2p_handler(), None)
 
     def test_handler_no_i2p(self):
         with mock_i2p(None):
-            p = i2p_provider.Provider("basedir", FakeConfig(), "reactor")
+            p = i2p_provider.Provider(FakeConfig(), "reactor")
         self.assertEqual(p.get_i2p_handler(), None)
 
     def test_handler_sam_endpoint(self):
@@ -213,8 +213,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir",
-                                      FakeConfig(**{"sam.port": "ep_desc"}),
+            p = i2p_provider.Provider(FakeConfig(**{"sam.port": "ep_desc"}),
                                       reactor)
             with mock.patch("allmydata.util.i2p_provider.clientFromString",
                             return_value=ep) as cfs:
@@ -230,7 +229,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir", FakeConfig(launch=True),
+            p = i2p_provider.Provider(FakeConfig(launch=True),
                                       reactor)
         h = p.get_i2p_handler()
         self.assertIs(h, handler)
@@ -243,8 +242,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir",
-                                      FakeConfig(launch=True,
+            p = i2p_provider.Provider(FakeConfig(launch=True,
                                                  **{"i2p.configdir": "configdir"}),
                                       reactor)
         h = p.get_i2p_handler()
@@ -258,8 +256,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir",
-                                      FakeConfig(launch=True,
+            p = i2p_provider.Provider(FakeConfig(launch=True,
                                                  **{"i2p.configdir": "configdir",
                                                     "i2p.executable": "myi2p",
                                                    }),
@@ -275,8 +272,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir",
-                                      FakeConfig(**{"i2p.configdir": "configdir"}),
+            p = i2p_provider.Provider(FakeConfig(**{"i2p.configdir": "configdir"}),
                                       reactor)
         h = p.get_i2p_handler()
         i2p.local_i2p.assert_called_with("configdir")
@@ -289,7 +285,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir", FakeConfig(), reactor)
+            p = i2p_provider.Provider(FakeConfig(), reactor)
         h = p.get_i2p_handler()
         self.assertIs(h, handler)
         i2p.default.assert_called_with(reactor, keyfile=None)
@@ -308,8 +304,7 @@ class ProviderListener(unittest.TestCase):
 
         privkeyfile = os.path.join("private", "i2p_dest.privkey")
         with mock_i2p(i2p):
-            p = i2p_provider.Provider("basedir",
-                                      FakeConfig(**{
+            p = i2p_provider.Provider(FakeConfig(**{
                                           "i2p.configdir": "configdir",
                                           "sam.port": "good:port",
                                           "dest": "true",
@@ -326,37 +321,36 @@ class Provider_CheckI2PConfig(unittest.TestCase):
         # default config doesn't start an I2P service, so it should be
         # happy both with and without txi2p
 
-        p = i2p_provider.Provider("basedir", FakeConfig(), "reactor")
+        p = i2p_provider.Provider(FakeConfig(), "reactor")
         p.check_dest_config()
 
         with mock_txi2p(None):
-            p = i2p_provider.Provider("basedir", FakeConfig(), "reactor")
+            p = i2p_provider.Provider(FakeConfig(), "reactor")
             p.check_dest_config()
 
     def test_no_txi2p(self):
         with mock_txi2p(None):
-            p = i2p_provider.Provider("basedir", FakeConfig(dest=True),
+            p = i2p_provider.Provider(FakeConfig(dest=True),
                                       "reactor")
             e = self.assertRaises(ValueError, p.check_dest_config)
             self.assertEqual(str(e), "Cannot create I2P Destination without txi2p. "
                              "Please 'pip install tahoe-lafs[i2p]' to fix.")
 
     def test_no_launch_no_control(self):
-        p = i2p_provider.Provider("basedir", FakeConfig(dest=True), "reactor")
+        p = i2p_provider.Provider(FakeConfig(dest=True), "reactor")
         e = self.assertRaises(ValueError, p.check_dest_config)
         self.assertEqual(str(e), "[i2p] dest = true, but we have neither "
                          "sam.port= nor launch=true nor configdir=")
 
     def test_missing_keys(self):
-        p = i2p_provider.Provider("basedir", FakeConfig(dest=True,
+        p = i2p_provider.Provider(FakeConfig(dest=True,
                                              **{"sam.port": "x",
                                                 }), "reactor")
         e = self.assertRaises(ValueError, p.check_dest_config)
         self.assertEqual(str(e), "[i2p] dest = true, "
                          "but dest.port= is missing")
 
-        p = i2p_provider.Provider("basedir",
-                                  FakeConfig(dest=True,
+        p = i2p_provider.Provider(FakeConfig(dest=True,
                                              **{"sam.port": "x",
                                                 "dest.port": "y",
                                                 }), "reactor")
@@ -365,8 +359,7 @@ class Provider_CheckI2PConfig(unittest.TestCase):
                          "but dest.private_key_file= is missing")
 
     def test_launch_not_implemented(self):
-        p = i2p_provider.Provider("basedir",
-                                  FakeConfig(dest=True, launch=True,
+        p = i2p_provider.Provider(FakeConfig(dest=True, launch=True,
                                              **{"dest.port": "x",
                                                 "dest.private_key_file": "y",
                                                 }), "reactor")
@@ -374,8 +367,7 @@ class Provider_CheckI2PConfig(unittest.TestCase):
         self.assertEqual(str(e), "[i2p] launch is under development.")
 
     def test_ok(self):
-        p = i2p_provider.Provider("basedir",
-                                  FakeConfig(dest=True,
+        p = i2p_provider.Provider(FakeConfig(dest=True,
                                              **{"sam.port": "x",
                                                 "dest.port": "y",
                                                 "dest.private_key_file": "z",
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index 99791db26..7eab84143 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -16,7 +16,8 @@ import foolscap.logging.log
 from twisted.application import service
 from allmydata.node import Node, formatTimeTahoeStyle, MissingConfigEntry, read_config, config_from_string
 from allmydata.introducer.server import create_introducer
-from allmydata.client import create_client, _valid_config_sections
+from allmydata.client import create_client
+from allmydata.client import _valid_config_sections as client_valid_config_sections
 from allmydata.util import fileutil, iputil
 from allmydata.util.namespace import Namespace
 from allmydata.util.configutil import UnknownConfigError
@@ -34,7 +35,7 @@ class TestNode(Node):
         config = read_config(
             basedir,
             'DEFAULT_PORTNUMFILE_BLANK',
-            _valid_config_sections=_valid_config_sections,
+            _valid_config_sections=client_valid_config_sections,
         )
         Node.__init__(self, config, basedir)
 
@@ -120,9 +121,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         f.write(u"nickname = \u2621\n".encode('utf-8'))
         f.close()
 
-        n = TestNode(basedir)
-        n.setServiceParent(self.parent)
-        self.failUnlessEqual(n.get_config("node", "nickname").decode('utf-8'),
+        config = read_config(basedir, "")
+        self.failUnlessEqual(config.get_config("node", "nickname").decode('utf-8'),
                              u"\u2621")
 
     def test_tahoe_cfg_hash_in_name(self):
@@ -133,8 +133,9 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         f.write("[node]\n")
         f.write("nickname = %s\n" % (nickname,))
         f.close()
-        n = TestNode(basedir)
-        self.failUnless(n.nickname == nickname)
+
+        config = read_config(basedir, "")
+        self.failUnless(config.nickname == nickname)
 
     def test_private_config(self):
         basedir = "test_node/test_private_config"
@@ -144,26 +145,44 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         f.write("secret")
         f.close()
 
-        n = TestNode(basedir)
-        self.failUnlessEqual(n.get_private_config("already"), "secret")
-        self.failUnlessEqual(n.get_private_config("not", "default"), "default")
-        self.failUnlessRaises(MissingConfigEntry, n.get_private_config, "not")
-        value = n.get_or_create_private_config("new", "start")
+        config = config_from_string("", "", basedir)
+
+        self.failUnlessEqual(config.get_private_config("already"), "secret")
+        self.failUnlessEqual(config.get_private_config("not", "default"), "default")
+        self.failUnlessRaises(MissingConfigEntry, config.get_private_config, "not")
+        value = config.get_or_create_private_config("new", "start")
         self.failUnlessEqual(value, "start")
-        self.failUnlessEqual(n.get_private_config("new"), "start")
+        self.failUnlessEqual(config.get_private_config("new"), "start")
         counter = []
         def make_newer():
             counter.append("called")
             return "newer"
-        value = n.get_or_create_private_config("newer", make_newer)
+        value = config.get_or_create_private_config("newer", make_newer)
         self.failUnlessEqual(len(counter), 1)
         self.failUnlessEqual(value, "newer")
-        self.failUnlessEqual(n.get_private_config("newer"), "newer")
+        self.failUnlessEqual(config.get_private_config("newer"), "newer")
 
-        value = n.get_or_create_private_config("newer", make_newer)
+        value = config.get_or_create_private_config("newer", make_newer)
         self.failUnlessEqual(len(counter), 1) # don't call unless necessary
         self.failUnlessEqual(value, "newer")
 
+    def test_write_config_unwritable_file(self):
+        basedir = "test_node/configdir"
+        fileutil.make_dirs(basedir)
+        config = config_from_string("", "", basedir)
+        with open(os.path.join(basedir, "bad"), "w") as f:
+            f.write("bad")
+        os.chmod(os.path.join(basedir, "bad"), 0o000)
+
+        config.write_config_file("bad", "some value")
+
+        errs = self.flushLoggedErrors()
+        self.assertEqual(1, len(errs))
+        self.assertIn(
+            "IOError",
+            str(errs[0])
+        )
+
     def test_timestamp(self):
         # this modified logger doesn't seem to get used during the tests,
         # probably because we don't modify the LogObserver that trial
@@ -177,8 +196,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
     def test_secrets_dir(self):
         basedir = "test_node/test_secrets_dir"
         fileutil.make_dirs(basedir)
-        n = TestNode(basedir)
-        self.failUnless(isinstance(n, TestNode))
+        read_config(basedir, "")
+
         self.failUnless(os.path.exists(os.path.join(basedir, "private")))
 
     def test_secrets_dir_protected(self):
@@ -189,8 +208,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             raise unittest.SkipTest("We don't know how to set permissions on Windows.")
         basedir = "test_node/test_secrets_dir_protected"
         fileutil.make_dirs(basedir)
-        n = TestNode(basedir)
-        self.failUnless(isinstance(n, TestNode))
+        read_config(basedir, "")
+
         privdir = os.path.join(basedir, "private")
         st = os.stat(privdir)
         bits = stat.S_IMODE(st[stat.ST_MODE])
@@ -212,8 +231,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
 class EmptyNode(Node):
     def __init__(self):
-        config = config_from_string("", "no portfile")
-        Node.__init__(self, config, 'no basedir')
+        config = config_from_string("", "no portfile", 'no basedir')
+        Node.__init__(self, config)
 
 EXPECTED = {
     # top-level key is tub.port category
@@ -389,7 +408,7 @@ class Listeners(unittest.TestCase):
         n.config = read_config(
             n.basedir,
             "client.port",
-            _valid_config_sections=_valid_config_sections,
+            _valid_config_sections=client_valid_config_sections,
         )
         n.check_privacy()
         n.services = []
@@ -419,7 +438,7 @@ class Listeners(unittest.TestCase):
         n.config = read_config(
             n.basedir,
             "client.port",
-            _valid_config_sections=_valid_config_sections,
+            _valid_config_sections=client_valid_config_sections,
         )
         n.check_privacy()
         n.services = []
diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py
index fc33651d5..28c494a4d 100644
--- a/src/allmydata/test/test_system.py
+++ b/src/allmydata/test/test_system.py
@@ -2120,7 +2120,7 @@ class SystemTest(SystemTestMixin, RunBinTahoeMixin, unittest.TestCase):
         # exercise the remote-control-the-client foolscap interfaces in
         # allmydata.control (mostly used for performance tests)
         c0 = self.clients[0]
-        control_furl_file = os.path.join(c0.basedir, "private", "control.furl")
+        control_furl_file = c0.config.get_private_path("control.furl")
         control_furl = open(control_furl_file, "r").read().strip()
         # it doesn't really matter which Tub we use to connect to the client,
         # so let's just use our IntroducerNode's
diff --git a/src/allmydata/test/test_tor_provider.py b/src/allmydata/test/test_tor_provider.py
index 21bc5878e..2cb5948a1 100644
--- a/src/allmydata/test/test_tor_provider.py
+++ b/src/allmydata/test/test_tor_provider.py
@@ -261,6 +261,7 @@ class CreateOnion(unittest.TestCase):
             privkey = f.read()
         self.assertEqual(privkey, "privkey")
 
+
 _None = object()
 class FakeConfig(dict):
     def get_config(self, section, option, default=_None, boolean=False):
@@ -271,6 +272,10 @@ class FakeConfig(dict):
             raise KeyError
         return value
 
+    def get_config_path(self, *args):
+        return os.path.join(self.get("basedir", "basedir"), *args)
+
+
 class EmptyContext(object):
     def __init__(self):
         pass
@@ -281,21 +286,21 @@ class EmptyContext(object):
 
 class Provider(unittest.TestCase):
     def test_build(self):
-        tor_provider.Provider("basedir", FakeConfig(), "reactor")
+        tor_provider.Provider(FakeConfig(), "reactor")
 
     def test_handler_disabled(self):
-        p = tor_provider.Provider("basedir", FakeConfig(enabled=False),
+        p = tor_provider.Provider(FakeConfig(enabled=False),
                                   "reactor")
         self.assertEqual(p.get_tor_handler(), None)
 
     def test_handler_no_tor(self):
         with mock_tor(None):
-            p = tor_provider.Provider("basedir", FakeConfig(), "reactor")
+            p = tor_provider.Provider(FakeConfig(), "reactor")
         self.assertEqual(p.get_tor_handler(), None)
 
     def test_handler_launch_no_txtorcon(self):
         with mock_txtorcon(None):
-            p = tor_provider.Provider("basedir", FakeConfig(launch=True),
+            p = tor_provider.Provider(FakeConfig(launch=True),
                                       "reactor")
         self.assertEqual(p.get_tor_handler(), None)
 
@@ -309,7 +314,7 @@ class Provider(unittest.TestCase):
         tor.add_context = mock.Mock(return_value=EmptyContext())
         with mock_tor(tor):
             with mock_txtorcon(txtorcon):
-                p = tor_provider.Provider("basedir", FakeConfig(launch=True),
+                p = tor_provider.Provider(FakeConfig(launch=True),
                                           reactor)
         h = p.get_tor_handler()
         self.assertIs(h, handler)
@@ -355,8 +360,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_tor(tor):
-            p = tor_provider.Provider("basedir",
-                                      FakeConfig(**{"socks.port": "ep_desc"}),
+            p = tor_provider.Provider(FakeConfig(**{"socks.port": "ep_desc"}),
                                       reactor)
             with mock.patch("allmydata.util.tor_provider.clientFromString", cfs):
                 h = p.get_tor_handler()
@@ -373,8 +377,7 @@ class Provider(unittest.TestCase):
         reactor = object()
 
         with mock_tor(tor):
-            p = tor_provider.Provider("basedir",
-                                      FakeConfig(**{"control.port": "ep_desc"}),
+            p = tor_provider.Provider(FakeConfig(**{"control.port": "ep_desc"}),
                                       reactor)
             with mock.patch("allmydata.util.tor_provider.clientFromString", cfs):
                 h = p.get_tor_handler()
@@ -388,7 +391,7 @@ class Provider(unittest.TestCase):
         tor.default_socks = mock.Mock(return_value=handler)
 
         with mock_tor(tor):
-            p = tor_provider.Provider("basedir", FakeConfig(), "reactor")
+            p = tor_provider.Provider(FakeConfig(), "reactor")
             h = p.get_tor_handler()
         self.assertIs(h, handler)
         tor.default_socks.assert_called_with()
@@ -405,8 +408,7 @@ class ProviderListener(unittest.TestCase):
         reactor = object()
 
         with mock_tor(tor):
-            p = tor_provider.Provider("basedir",
-                                      FakeConfig(**{"onion.local_port": "321"}),
+            p = tor_provider.Provider(FakeConfig(**{"onion.local_port": "321"}),
                                       reactor)
         fake_ep = object()
         with mock.patch("allmydata.util.tor_provider.TCP4ServerEndpoint",
@@ -421,44 +423,42 @@ class Provider_CheckOnionConfig(unittest.TestCase):
         # default config doesn't start an onion service, so it should be
         # happy both with and without txtorcon
 
-        p = tor_provider.Provider("basedir", FakeConfig(), "reactor")
+        p = tor_provider.Provider(FakeConfig(), "reactor")
         p.check_onion_config()
 
         with mock_txtorcon(None):
-            p = tor_provider.Provider("basedir", FakeConfig(), "reactor")
+            p = tor_provider.Provider(FakeConfig(), "reactor")
             p.check_onion_config()
 
     def test_no_txtorcon(self):
         with mock_txtorcon(None):
-            p = tor_provider.Provider("basedir", FakeConfig(onion=True),
+            p = tor_provider.Provider(FakeConfig(onion=True),
                                       "reactor")
             e = self.assertRaises(ValueError, p.check_onion_config)
             self.assertEqual(str(e), "Cannot create onion without txtorcon. "
                              "Please 'pip install tahoe-lafs[tor]' to fix.")
 
     def test_no_launch_no_control(self):
-        p = tor_provider.Provider("basedir", FakeConfig(onion=True), "reactor")
+        p = tor_provider.Provider(FakeConfig(onion=True), "reactor")
         e = self.assertRaises(ValueError, p.check_onion_config)
         self.assertEqual(str(e), "[tor] onion = true, but we have neither "
                          "launch=true nor control.port=")
 
     def test_missing_keys(self):
-        p = tor_provider.Provider("basedir", FakeConfig(onion=True,
-                                                        launch=True), "reactor")
+        p = tor_provider.Provider(FakeConfig(onion=True,
+                                             launch=True), "reactor")
         e = self.assertRaises(ValueError, p.check_onion_config)
         self.assertEqual(str(e), "[tor] onion = true, "
                          "but onion.local_port= is missing")
 
-        p = tor_provider.Provider("basedir",
-                                  FakeConfig(onion=True, launch=True,
+        p = tor_provider.Provider(FakeConfig(onion=True, launch=True,
                                              **{"onion.local_port": "x",
                                                 }), "reactor")
         e = self.assertRaises(ValueError, p.check_onion_config)
         self.assertEqual(str(e), "[tor] onion = true, "
                          "but onion.external_port= is missing")
 
-        p = tor_provider.Provider("basedir",
-                                  FakeConfig(onion=True, launch=True,
+        p = tor_provider.Provider(FakeConfig(onion=True, launch=True,
                                              **{"onion.local_port": "x",
                                                 "onion.external_port": "y",
                                                 }), "reactor")
@@ -467,8 +467,7 @@ class Provider_CheckOnionConfig(unittest.TestCase):
                          "but onion.private_key_file= is missing")
 
     def test_ok(self):
-        p = tor_provider.Provider("basedir",
-                                  FakeConfig(onion=True, launch=True,
+        p = tor_provider.Provider(FakeConfig(onion=True, launch=True,
                                              **{"onion.local_port": "x",
                                                 "onion.external_port": "y",
                                                 "onion.private_key_file": "z",
@@ -478,7 +477,7 @@ class Provider_CheckOnionConfig(unittest.TestCase):
 class Provider_Service(unittest.TestCase):
     def test_no_onion(self):
         reactor = object()
-        p = tor_provider.Provider("basedir", FakeConfig(onion=False), reactor)
+        p = tor_provider.Provider(FakeConfig(onion=False), reactor)
         with mock.patch("allmydata.util.tor_provider.Provider._start_onion") as s:
             p.startService()
         self.assertEqual(s.mock_calls, [])
@@ -495,7 +494,7 @@ class Provider_Service(unittest.TestCase):
         with open(fn, "w") as f:
             f.write("private key")
         reactor = object()
-        cfg = FakeConfig(onion=True, launch=True,
+        cfg = FakeConfig(basedir=basedir, onion=True, launch=True,
                          **{"onion.local_port": 123,
                             "onion.external_port": 456,
                             "onion.private_key_file": "keyfile",
@@ -503,7 +502,7 @@ class Provider_Service(unittest.TestCase):
 
         txtorcon = mock.Mock()
         with mock_txtorcon(txtorcon):
-            p = tor_provider.Provider(basedir, cfg, reactor)
+            p = tor_provider.Provider(cfg, reactor)
         tor_state = mock.Mock()
         tor_state.protocol = object()
         ehs = mock.Mock()
@@ -535,7 +534,7 @@ class Provider_Service(unittest.TestCase):
         with open(fn, "w") as f:
             f.write("private key")
         reactor = object()
-        cfg = FakeConfig(onion=True,
+        cfg = FakeConfig(basedir=basedir, onion=True,
                          **{"control.port": "ep_desc",
                             "onion.local_port": 123,
                             "onion.external_port": 456,
@@ -544,7 +543,7 @@ class Provider_Service(unittest.TestCase):
 
         txtorcon = mock.Mock()
         with mock_txtorcon(txtorcon):
-            p = tor_provider.Provider(basedir, cfg, reactor)
+            p = tor_provider.Provider(cfg, reactor)
         tor_state = mock.Mock()
         tor_state.protocol = object()
         txtorcon.build_tor_connection = mock.Mock(return_value=tor_state)
diff --git a/src/allmydata/test/web/test_grid.py b/src/allmydata/test/web/test_grid.py
index 208c03881..8c07a0dd1 100644
--- a/src/allmydata/test/web/test_grid.py
+++ b/src/allmydata/test/web/test_grid.py
@@ -1238,8 +1238,7 @@ class Grid(GridTestMixin, WebErrorMixin, ShouldFailMixin, testutil.ReallyEqualMi
         self.basedir = "web/Grid/blacklist"
         self.set_up_grid(oneshare=True)
         c0 = self.g.clients[0]
-        c0_basedir = c0.basedir
-        fn = os.path.join(c0_basedir, "access.blacklist")
+        fn = c0.config.get_config_path("access.blacklist")
         self.uris = {}
         DATA = "off-limits " * 50
 
diff --git a/src/allmydata/test/web/test_introducer.py b/src/allmydata/test/web/test_introducer.py
index d3848baf0..80b69f15f 100644
--- a/src/allmydata/test/web/test_introducer.py
+++ b/src/allmydata/test/web/test_introducer.py
@@ -25,7 +25,7 @@ class IntroducerWeb(unittest.TestCase):
         )
         from allmydata.node import config_from_string
         self.node = IntroducerNode(
-            config_from_string(config, portnumfile="introducer.port"),
+            config_from_string(config, "introducer.port", "no-basedir"),
         )
         self.ws = self.node.getServiceNamed("webish")
 
diff --git a/src/allmydata/util/i2p_provider.py b/src/allmydata/util/i2p_provider.py
index 96dc11ce2..be706efcb 100644
--- a/src/allmydata/util/i2p_provider.py
+++ b/src/allmydata/util/i2p_provider.py
@@ -124,9 +124,8 @@ def create_config(reactor, cli_config):
 # a nice error, and startService will throw an ugly error.
 
 class Provider(service.MultiService):
-    def __init__(self, basedir, config, reactor):
+    def __init__(self, config, reactor):
         service.MultiService.__init__(self)
-        self._basedir = basedir
         self._config = config
         self._i2p = _import_i2p()
         self._txi2p = _import_txi2p()
diff --git a/src/allmydata/util/tor_provider.py b/src/allmydata/util/tor_provider.py
index 14d871cd0..bfdde3643 100644
--- a/src/allmydata/util/tor_provider.py
+++ b/src/allmydata/util/tor_provider.py
@@ -198,10 +198,9 @@ def create_config(reactor, cli_config):
 # nice error, and startService will throw an ugly error.
 
 class Provider(service.MultiService):
-    def __init__(self, basedir, node_for_config, reactor):
+    def __init__(self, config, reactor):
         service.MultiService.__init__(self)
-        self._basedir = basedir
-        self._node_for_config = node_for_config
+        self._config = config
         self._tor_launched = None
         self._onion_ehs = None
         self._onion_tor_control_proto = None
@@ -210,7 +209,7 @@ class Provider(service.MultiService):
         self._reactor = reactor
 
     def _get_tor_config(self, *args, **kwargs):
-        return self._node_for_config.get_config("tor", *args, **kwargs)
+        return self._config.get_config("tor", *args, **kwargs)
 
     def get_listener(self):
         local_port = int(self._get_tor_config("onion.local_port"))
@@ -255,7 +254,7 @@ class Provider(service.MultiService):
         # this fires with a tuple of (control_endpoint, tor_protocol)
         if not self._tor_launched:
             self._tor_launched = OneShotObserverList()
-            private_dir = os.path.join(self._basedir, "private")
+            private_dir = self._config.get_config_path("private")
             tor_binary = self._get_tor_config("tor.executable", None)
             d = _launch_tor(reactor, tor_binary, private_dir, self._txtorcon)
             d.addBoth(self._tor_launched.fire)
@@ -298,7 +297,7 @@ class Provider(service.MultiService):
         external_port = int(self._get_tor_config("onion.external_port"))
 
         fn = self._get_tor_config("onion.private_key_file")
-        privkeyfile = os.path.join(self._basedir, fn)
+        privkeyfile = self._config.get_config_path(fn)
         with open(privkeyfile, "rb") as f:
             privkey = f.read()
         ehs = self._txtorcon.EphemeralHiddenService(

From f37ab3b12d62ab94143fa0db422f02041a1210ac Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Tue, 13 Feb 2018 18:57:49 -0700
Subject: [PATCH 02/38] get_app_version -> global function

---
 src/allmydata/client.py | 3 ++-
 src/allmydata/node.py   | 9 +++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index cb0978bc4..390327345 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -30,6 +30,7 @@ from allmydata.nodemaker import NodeMaker
 from allmydata.blacklist import Blacklist
 from allmydata.node import OldConfigOptionError, _common_config_sections
 from allmydata.node import read_config
+from allmydata.node import get_app_versions
 
 
 KiB=1024
@@ -277,7 +278,7 @@ class _Client(node.Node, pollmixin.PollMixin):
                                   self.nickname,
                                   str(allmydata.__full_version__),
                                   str(self.OLDEST_SUPPORTED_VERSION),
-                                  self.config.get_app_versions(), self._sequencer,
+                                  get_app_versions(), self._sequencer,
                                   introducer_cache_filepath)
             self.introducer_clients.append(ic)
             self.introducer_furls.append(introducer['furl'])
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 0d738cc6c..aa00deb48 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -161,6 +161,11 @@ def config_from_string(config_str, portnumfile, basedir):
     return _Config(parser, portnumfile, basedir, '<in-memory>')
 
 
+def get_app_versions():
+    # TODO: merge this with allmydata.get_package_versions
+    return dict(app_versions.versions)
+
+
 def _error_about_old_config_files(basedir, generated_files):
     """
     If any old configuration files are detected, raise
@@ -216,10 +221,6 @@ class _Config(object):
             if os.path.exists(self.config_fname):
                 raise
 
-    def get_app_versions(self):
-        # TODO: merge this with allmydata.get_package_versions
-        return dict(app_versions.versions)
-
     def write_config_file(self, name, value, mode="w"):
         """
         writes the given 'value' into a file called 'name' in the config

From 38063037c198588257d5b6b6eee51c4fc4163061 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 22 Feb 2018 14:39:35 -0700
Subject: [PATCH 03/38] add documentation

---
 src/allmydata/node.py | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index aa00deb48..18fae7c38 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -193,17 +193,30 @@ def _error_about_old_config_files(basedir, generated_files):
 
 class _Config(object):
     """
-    FIXME better name
+    Manages configuration of a Tahoe 'node directory'.
 
-    pulling out all the 'config' stuff from Node, so we can pass it in
-    as a helper instead.
+    Note: all this code and functionality was formerly in the Node
+    class; names and funtionality have been kept the same while moving
+    the code. It probably makes sense for several of these APIs to
+    have better names.
     """
 
     def __init__(self, configparser, portnum_fname, basedir, config_fname):
-        # XXX I think this portnumfile thing is just legacy?
+        """
+        :param configparser: a ConfigParser instance
+
+        :param portnum_fname: filename to use for the port-number file
+           (a relative path inside basedir)
+
+        :param basedir: path to our "node directory", inside which all
+           configuration is managed
+
+        :param config_fname: the pathname actually used to create the
+            configparser (might be 'fake' if using in-memory data)
+        """
         self.portnum_fname = portnum_fname
         self._basedir = abspath_expanduser_unicode(unicode(basedir))
-        self._config_fname = config_fname  # the actual fname "configparser" came from
+        self._config_fname = config_fname
         self.config = configparser
 
         nickname_utf8 = self.get_config("node", "nickname", "<unspecified>")

From 990f23d5c7715773df36da458270fc7029124f65 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 22 Feb 2018 15:16:01 -0700
Subject: [PATCH 04/38] _Config does this for us

---
 src/allmydata/node.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 18fae7c38..98244bd74 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -375,8 +375,6 @@ class Node(service.MultiService):
         is the current directory by default.
         """
         service.MultiService.__init__(self)
-        # XXX don't write files in ctor!
-        fileutil.make_dirs(os.path.join(config._basedir, "private"), 0700)
         with open(os.path.join(config._basedir, "private", "README"), "w") as f:
             f.write(PRIV_README)
 

From 79756c088e6ace4ad1d95d1db8c4e81a3aa288ac Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sun, 28 Jan 2018 22:48:18 -0700
Subject: [PATCH 05/38] split client, introducer READMEs

---
 src/allmydata/client.py            | 12 ++++++++++++
 src/allmydata/introducer/server.py | 18 ++++++++++++++++--
 src/allmydata/node.py              |  4 +---
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 390327345..02e869e03 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -96,6 +96,15 @@ def _valid_config_sections():
     })
     return cfg
 
+# this is put into README in new node-directories
+CLIENT_README = """
+This directory contains files which contain private data for the Tahoe node,
+such as private keys.  On Unix-like systems, the permissions on this directory
+are set to disallow users other than its owner from reading the contents of
+the files.   See the 'configuration.rst' documentation file for details.
+"""
+
+
 
 def _make_secret():
     return base32.b2a(os.urandom(hashutil.CRYPTO_VAL_SIZE)) + "\n"
@@ -158,7 +167,10 @@ class Terminator(service.Service):
 
 #@defer.inlineCallbacks
 def create_client(basedir=u"."):
+    # load configuration
     config = read_config(basedir, u"client.port", _valid_config_sections=_valid_config_sections)
+    config.write_private_config("README", CLIENT_README)
+
     #defer.returnValue(
     return _Client(
             config,
diff --git a/src/allmydata/introducer/server.py b/src/allmydata/introducer/server.py
index 5886c991e..bb49a1df6 100644
--- a/src/allmydata/introducer/server.py
+++ b/src/allmydata/introducer/server.py
@@ -11,6 +11,15 @@ from allmydata.introducer.interfaces import \
 from allmydata.introducer.common import unsign_from_foolscap, \
      SubscriberDescriptor, AnnouncementDescriptor
 
+# this is put into README in new node-directories
+INTRODUCER_README = """
+This directory contains files which contain private data for the Tahoe node,
+such as private keys.  On Unix-like systems, the permissions on this directory
+are set to disallow users other than its owner from reading the contents of
+the files.   See the 'configuration.rst' documentation file for details.
+"""
+
+
 def _valid_config_sections():
     return node._common_config_sections()
 
@@ -21,8 +30,13 @@ class FurlFileConflictError(Exception):
 #@defer.inlineCallbacks
 def create_introducer(basedir=u"."):
     from allmydata.node import read_config
-    config = read_config(basedir, u"client.port", generated_files=["introducer.furl"])
-    config.validate(_valid_config_sections())
+
+    config = read_config(
+        basedir, u"client.port",
+        generated_files=["introducer.furl"],
+        _valid_config_secionts=_valid_config_sections,
+    )
+    config.write_private_config("README", INTRODUCER_README)
     #defer.returnValue(
     return _IntroducerNode(
             config,
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 98244bd74..b8b968b12 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -94,7 +94,7 @@ such as private keys.  On Unix-like systems, the permissions on this directory
 are set to disallow users other than its owner from reading the contents of
 the files.   See the 'configuration.rst' documentation file for details."""
 
-class _None(object):
+class _None(object):  # used as a marker in get_config()
     """
     This class is to be used as a marker in get_config()
     """
@@ -375,8 +375,6 @@ class Node(service.MultiService):
         is the current directory by default.
         """
         service.MultiService.__init__(self)
-        with open(os.path.join(config._basedir, "private", "README"), "w") as f:
-            f.write(PRIV_README)
 
         self.config = config
         self.get_config = config.get_config # XXX stopgap

From f68a0ab74c431e7e5915d2abf98771fbbea9c56b Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 22 Feb 2018 15:25:38 -0700
Subject: [PATCH 06/38] remove debug

---
 src/allmydata/node.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index b8b968b12..a9d0128f6 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -403,8 +403,6 @@ class Node(service.MultiService):
         """
         tempdir_config = self.config.get_config("node", "tempdir", "tmp").decode('utf-8')
         tempdir = self.config.get_config_path(tempdir_config)
-        tempdir0 = abspath_expanduser_unicode(tempdir_config, base=self.config._basedir)
-        assert tempdir == tempdir0
         if not os.path.exists(tempdir):
             fileutil.make_dirs(tempdir)
         tempfile.tempdir = tempdir

From 22e2d0a41789ba43b1f6278fb897ac1c74606948 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 22 Feb 2018 15:32:00 -0700
Subject: [PATCH 07/38] re-expand path because user input

---
 src/allmydata/node.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index a9d0128f6..9010fedcd 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -346,7 +346,12 @@ class _Config(object):
         returns an absolute path inside the config directory with any
         extra args join()-ed
         """
-        return os.path.join(self._basedir, *args)
+        # note: we re-expand here (_basedir already went through this
+        # expanduser function) in case the path we're being asked for
+        # has embedded ".."'s in it
+        return abspath_expanduser_unicode(
+            os.path.join(self._basedir, *args)
+        )
 
     @staticmethod
     def _contains_unescaped_hash(item):

From 86a9ce57934942ddd98d3356b887fab62815ae2c Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 22 Feb 2018 15:42:44 -0700
Subject: [PATCH 08/38] create _NoNetworkClient using same code as _Client

---
 src/allmydata/client.py            | 14 ++++++++++++--
 src/allmydata/introducer/server.py |  2 +-
 src/allmydata/node.py              |  1 +
 src/allmydata/test/no_network.py   | 12 ++++--------
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 02e869e03..6b0818aff 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -166,13 +166,23 @@ class Terminator(service.Service):
 
 
 #@defer.inlineCallbacks
-def create_client(basedir=u"."):
+def create_client(basedir=u".", _client_factory=None):
+    """
+    Creates a new client instance (a subclass of Node).
+
+    :param basedir: the node directory
+
+    :param _client_factory: for testing; the class to instantiate
+    """
     # load configuration
     config = read_config(basedir, u"client.port", _valid_config_sections=_valid_config_sections)
     config.write_private_config("README", CLIENT_README)
 
+    if _client_factory is None:
+        _client_factory = _Client
+
     #defer.returnValue(
-    return _Client(
+    return _client_factory(
             config,
         )
     #)
diff --git a/src/allmydata/introducer/server.py b/src/allmydata/introducer/server.py
index bb49a1df6..860f9ecf7 100644
--- a/src/allmydata/introducer/server.py
+++ b/src/allmydata/introducer/server.py
@@ -34,7 +34,7 @@ def create_introducer(basedir=u"."):
     config = read_config(
         basedir, u"client.port",
         generated_files=["introducer.furl"],
-        _valid_config_secionts=_valid_config_sections,
+        _valid_config_sections=_valid_config_sections,
     )
     config.write_private_config("README", INTRODUCER_README)
     #defer.returnValue(
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 9010fedcd..f71bd2608 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -313,6 +313,7 @@ class _Config(object):
         config file that resides within the subdirectory named 'private'), and
         return it.
         """
+        fileutil.make_dirs(os.path.join(self._basedir, "private"), 0700)
         privname = os.path.join(self._basedir, "private", name)
         with open(privname, "w") as f:
             f.write(value)
diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py
index b95bd0be7..fd8e74f4b 100644
--- a/src/allmydata/test/no_network.py
+++ b/src/allmydata/test/no_network.py
@@ -27,7 +27,7 @@ from allmydata.util.assertutil import _assert
 
 from allmydata import uri as tahoe_uri
 from allmydata.client import _Client
-from allmydata.client import _valid_config_sections as client_valid_config_sections
+from allmydata.client import create_client
 from allmydata.storage.server import StorageServer, storage_index_to_dir
 from allmydata.util import fileutil, idlib, hashutil
 from allmydata.util.hashutil import permute_server_hash
@@ -185,12 +185,8 @@ class NoNetworkStorageBroker(object):
         return []  # FIXME?
 
 
-def NoNetworkClient(basedir):
-    # XXX FIXME this is just to avoid massive search-replace for now;
-    # should be create_nonetwork_client() or something...
-    from allmydata.node import read_config
-    config = read_config(basedir, u'client.port', _valid_config_sections=client_valid_config_sections)
-    return _NoNetworkClient(config)
+def create_no_network_client(basedir):
+    return create_client(basedir, _client_factory=_NoNetworkClient)
 
 
 class _NoNetworkClient(_Client):
@@ -293,7 +289,7 @@ class NoNetworkGrid(service.MultiService):
             c = self.client_config_hooks[i](clientdir)
 
         if not c:
-            c = NoNetworkClient(clientdir)
+            c = create_no_network_client(clientdir)
             c.set_default_mutable_keysize(TEST_RSA_KEY_SIZE)
 
         c.nodeid = clientid

From f7f3c54f9371647e6708f29cae9b8fbf3368e1da Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Mon, 26 Feb 2018 15:12:46 -0700
Subject: [PATCH 09/38] dead code

---
 src/allmydata/node.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index f71bd2608..816b4d4c0 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -226,14 +226,6 @@ class _Config(object):
     def validate(self, valid_config_sections):
         configutil.validate_config(self._config_fname, self.config, valid_config_sections)
 
-    def read_config(self):
-
-        try:
-            self.config = configutil.get_config(self.config_fname)
-        except EnvironmentError:
-            if os.path.exists(self.config_fname):
-                raise
-
     def write_config_file(self, name, value, mode="w"):
         """
         writes the given 'value' into a file called 'name' in the config

From c93ee4f867458a9d9f68345baa4cdfc681f82bc6 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Mon, 26 Feb 2018 18:25:55 -0700
Subject: [PATCH 10/38] document a test-method

---
 src/allmydata/test/test_node.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index 7eab84143..65491d9d5 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -167,6 +167,11 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         self.failUnlessEqual(value, "newer")
 
     def test_write_config_unwritable_file(self):
+        """
+        Existing behavior merely logs any errors upon writing
+        configuration; this should probably be fixed to do something
+        better (like fail entirely). See #2905
+        """
         basedir = "test_node/configdir"
         fileutil.make_dirs(basedir)
         config = config_from_string("", "", basedir)

From d544284f9299f991d093f1ce4af939af9c7ca76d Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Tue, 27 Feb 2018 15:00:31 -0700
Subject: [PATCH 11/38] introduce create_node_dir

---
 src/allmydata/client.py                   |   3 +-
 src/allmydata/introducer/server.py        |   7 +-
 src/allmydata/node.py                     |  18 +++-
 src/allmydata/test/test_introducer.py     |   5 +-
 src/allmydata/test/test_node.py           | 119 +++++++++++-----------
 src/allmydata/test/web/test_introducer.py |   6 +-
 6 files changed, 93 insertions(+), 65 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 6b0818aff..554ab5b91 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -174,9 +174,10 @@ def create_client(basedir=u".", _client_factory=None):
 
     :param _client_factory: for testing; the class to instantiate
     """
+    node.create_node_dir(basedir, CLIENT_README)
+
     # load configuration
     config = read_config(basedir, u"client.port", _valid_config_sections=_valid_config_sections)
-    config.write_private_config("README", CLIENT_README)
 
     if _client_factory is None:
         _client_factory = _Client
diff --git a/src/allmydata/introducer/server.py b/src/allmydata/introducer/server.py
index 860f9ecf7..e51b706e5 100644
--- a/src/allmydata/introducer/server.py
+++ b/src/allmydata/introducer/server.py
@@ -29,14 +29,15 @@ class FurlFileConflictError(Exception):
 
 #@defer.inlineCallbacks
 def create_introducer(basedir=u"."):
-    from allmydata.node import read_config
+    from allmydata import node
+    if not os.path.exists(basedir):
+        node.create_node_dir(basedir, INTRODUCER_README)
 
-    config = read_config(
+    config = node.read_config(
         basedir, u"client.port",
         generated_files=["introducer.furl"],
         _valid_config_sections=_valid_config_sections,
     )
-    config.write_private_config("README", INTRODUCER_README)
     #defer.returnValue(
     return _IntroducerNode(
             config,
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 816b4d4c0..695d77c49 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -127,6 +127,23 @@ class PrivacyError(Exception):
     that the IP address could be revealed"""
 
 
+def create_node_dir(basedir, readme_text):
+    """
+    Create new new 'node directory' at 'basedir'. This includes a
+    'private' subdirectory. If basedir (and privdir) already exists,
+    nothing is done.
+
+    :param readme_text: text to put in <basedir>/private/README
+    """
+    if not os.path.exists(basedir):
+        fileutil.make_dirs(basedir)
+    privdir = os.path.join(basedir, "private")
+    if not os.path.exists(privdir):
+        fileutil.make_dirs(privdir, 0700)
+        with open(os.path.join(privdir, 'README'), 'w') as f:
+            f.write(readme_text)
+
+
 def read_config(basedir, portnumfile, generated_files=[], _valid_config_sections=None):
     basedir = abspath_expanduser_unicode(unicode(basedir))
     if _valid_config_sections is None:
@@ -305,7 +322,6 @@ class _Config(object):
         config file that resides within the subdirectory named 'private'), and
         return it.
         """
-        fileutil.make_dirs(os.path.join(self._basedir, "private"), 0700)
         privname = os.path.join(self._basedir, "private", name)
         with open(privname, "w") as f:
             f.write(value)
diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py
index 3d3d9e88f..0697379d4 100644
--- a/src/allmydata/test/test_introducer.py
+++ b/src/allmydata/test/test_introducer.py
@@ -20,6 +20,7 @@ from allmydata.introducer.server import IntroducerService, FurlFileConflictError
 from allmydata.introducer.common import get_tubid_string_from_ann, \
      get_tubid_string, sign_to_foolscap, unsign_from_foolscap, \
      UnknownKeyError
+from allmydata.node import create_node_dir
 # the "new way" to create introducer node instance
 from allmydata.introducer.server import create_introducer
 from allmydata.web import introweb
@@ -42,7 +43,7 @@ class Node(testutil.SignalMixin, testutil.ReallyEqualMixin, unittest.TestCase):
 
     def test_furl(self):
         basedir = "introducer.IntroducerNode.test_furl"
-        os.mkdir(basedir)
+        create_node_dir(basedir, "testing")
         public_fn = os.path.join(basedir, "introducer.furl")
         private_fn = os.path.join(basedir, "private", "introducer.furl")
 
@@ -75,7 +76,7 @@ class Node(testutil.SignalMixin, testutil.ReallyEqualMixin, unittest.TestCase):
 
     def test_web_static(self):
         basedir = u"introducer.Node.test_web_static"
-        os.mkdir(basedir)
+        create_node_dir(basedir, "testing")
         fileutil.write(os.path.join(basedir, "tahoe.cfg"),
                        "[node]\n" +
                        "web.port = tcp:0:interface=127.0.0.1\n" +
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index 65491d9d5..cf3fdc8c7 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -14,7 +14,7 @@ from foolscap.api import flushEventualQueue
 import foolscap.logging.log
 
 from twisted.application import service
-from allmydata.node import Node, formatTimeTahoeStyle, MissingConfigEntry, read_config, config_from_string
+from allmydata.node import Node, formatTimeTahoeStyle, MissingConfigEntry, read_config, config_from_string, create_node_dir
 from allmydata.introducer.server import create_introducer
 from allmydata.client import create_client
 from allmydata.client import _valid_config_sections as client_valid_config_sections
@@ -58,14 +58,13 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         return d
 
     def _test_location(self, basedir, expected_addresses, tub_port=None, tub_location=None, local_addresses=None):
-        fileutil.make_dirs(basedir)
-        f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
-        f.write("[node]\n")
-        if tub_port:
-            f.write("tub.port = %d\n" % (tub_port,))
-        if tub_location is not None:
-            f.write("tub.location = %s\n" % (tub_location,))
-        f.close()
+        create_node_dir(basedir, "testing")
+        with open(os.path.join(basedir, 'tahoe.cfg'), 'wt') as f:
+            f.write("[node]\n")
+            if tub_port:
+                f.write("tub.port = %d\n" % (tub_port,))
+            if tub_location is not None:
+                f.write("tub.location = %s\n" % (tub_location,))
 
         if local_addresses:
             self.patch(iputil, 'get_local_addresses_sync',
@@ -200,9 +199,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
     def test_secrets_dir(self):
         basedir = "test_node/test_secrets_dir"
-        fileutil.make_dirs(basedir)
-        read_config(basedir, "")
-
+        create_node_dir(basedir, "testing")
         self.failUnless(os.path.exists(os.path.join(basedir, "private")))
 
     def test_secrets_dir_protected(self):
@@ -212,9 +209,9 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             # that unprivileged users can't read this thing.)
             raise unittest.SkipTest("We don't know how to set permissions on Windows.")
         basedir = "test_node/test_secrets_dir_protected"
-        fileutil.make_dirs(basedir)
-        read_config(basedir, "")
+        create_node_dir(basedir, "nothing to see here")
 
+        # make sure private dir was created with correct modes
         privdir = os.path.join(basedir, "private")
         st = os.stat(privdir)
         bits = stat.S_IMODE(st[stat.ST_MODE])
@@ -222,7 +219,6 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
     def test_logdir_is_str(self):
         basedir = "test_node/test_logdir_is_str"
-        fileutil.make_dirs(basedir)
 
         ns = Namespace()
         ns.called = False
@@ -231,14 +227,18 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             self.failUnless(isinstance(logdir, str), logdir)
         self.patch(foolscap.logging.log, 'setLogDir', call_setLogDir)
 
+        create_node_dir(basedir, "nothing to see here")
         TestNode(basedir)
         self.failUnless(ns.called)
 
+
 class EmptyNode(Node):
-    def __init__(self):
-        config = config_from_string("", "no portfile", 'no basedir')
+    def __init__(self, config=None):
+        if config is None:
+            config = config_from_string("", "no portfile", 'no basedir')
         Node.__init__(self, config)
 
+
 EXPECTED = {
     # top-level key is tub.port category
     "missing": {
@@ -288,14 +288,14 @@ class PortLocation(unittest.TestCase):
                         "hintstring": "tcp:HOST:888,AUTO",
                         }[tl]
 
-        n = EmptyNode()
         basedir = os.path.join("test_node/portlocation/%s/%s" % (tp, tl))
-        fileutil.make_dirs(basedir)
-        config = n.config = read_config(
+        create_node_dir(basedir, "testing")
+        config = read_config(
             basedir,
             "node.port",
-            _valid_config_sections=_valid_config_sections,
+            _valid_config_sections=client_valid_config_sections,
         )
+        n = EmptyNode(config)
         n._reveal_ip = True
 
         if exp in ("ERR1", "ERR2", "ERR3", "ERR4"):
@@ -394,27 +394,27 @@ class FakeTub:
 
 class Listeners(unittest.TestCase):
     def test_multiple_ports(self):
-        n = EmptyNode()
-        n.basedir = self.mktemp()
-        n.config_fname = os.path.join(n.basedir, "tahoe.cfg")
-        os.mkdir(n.basedir)
-        os.mkdir(os.path.join(n.basedir, "private"))
+        basedir = self.mktemp()
+        create_node_dir(basedir, "testing")
+
         port1 = iputil.allocate_tcp_port()
         port2 = iputil.allocate_tcp_port()
         port = ("tcp:%d:interface=127.0.0.1,tcp:%d:interface=127.0.0.1" %
                 (port1, port2))
         location = "tcp:localhost:%d,tcp:localhost:%d" % (port1, port2)
-        with open(n.config_fname, "w") as f:
+        with open(os.path.join(basedir, "tahoe.cfg"), "w") as f:
             f.write(BASE_CONFIG)
             f.write("tub.port = %s\n" % port)
             f.write("tub.location = %s\n" % location)
+
         # we're doing a lot of calling-into-setup-methods here, it might be
         # better to just create a real Node instance, I'm not sure.
-        n.config = read_config(
-            n.basedir,
+        config = read_config(
+            basedir,
             "client.port",
             _valid_config_sections=client_valid_config_sections,
         )
+        n = EmptyNode(config)
         n.check_privacy()
         n.services = []
         n.create_i2p_provider()
@@ -429,43 +429,49 @@ class Listeners(unittest.TestCase):
                           "tcp:%d:interface=127.0.0.1" % port2])
 
     def test_tor_i2p_listeners(self):
-        n = EmptyNode()
-        n.basedir = self.mktemp()
-        n.config_fname = os.path.join(n.basedir, "tahoe.cfg")
-        os.mkdir(n.basedir)
-        os.mkdir(os.path.join(n.basedir, "private"))
-        with open(n.config_fname, "w") as f:
+        basedir = self.mktemp()
+        config_fname = os.path.join(basedir, "tahoe.cfg")
+        os.mkdir(basedir)
+        os.mkdir(os.path.join(basedir, "private"))
+        with open(config_fname, "w") as f:
             f.write(BASE_CONFIG)
             f.write("tub.port = listen:i2p,listen:tor\n")
             f.write("tub.location = tcp:example.org:1234\n")
         # we're doing a lot of calling-into-setup-methods here, it might be
         # better to just create a real Node instance, I'm not sure.
-        n.config = read_config(
+        config = read_config(
             n.basedir,
             "client.port",
             _valid_config_sections=client_valid_config_sections,
         )
-        n.check_privacy()
-        n.services = []
+
         i2p_ep = object()
+        i2p_prov = i2p_provider.Provider(config, mock.Mock())
+        i2p_prov.get_listener = mock.Mock(return_value=i2p_ep)
+        i2p_x = mock.Mock()
+        i2p_x.Provider = lambda c, r: i2p_prov
+        i2p_mock = mock.patch('allmydata.node.i2p_provider', new=i2p_x)
+
         tor_ep = object()
-        n._i2p_provider = mock.Mock()
-        n._i2p_provider.get_listener = mock.Mock(return_value=i2p_ep)
-        n._tor_provider = mock.Mock()
-        n._tor_provider.get_listener = mock.Mock(return_value=tor_ep)
-        n.init_connections()
-        n.set_tub_options()
-        t = FakeTub()
-        with mock.patch("allmydata.node.Tub", return_value=t):
-            n.create_main_tub()
+        tor_prov = tor_provider.Provider(config, mock.Mock())
+        tor_prov.get_listener = mock.Mock(return_value=tor_ep)
+        tor_x = mock.Mock()
+        tor_x.Provider = lambda c, r: tor_prov
+        tor_mock = mock.patch('allmydata.node.tor_provider', new=tor_x)
+
+        tub_mock = mock.patch("allmydata.node.Tub", return_value=FakeTub())
+        with i2p_mock, tor_mock, tub_mock:
+            n = EmptyNode(config)
+
         self.assertEqual(n._i2p_provider.get_listener.mock_calls, [mock.call()])
         self.assertEqual(n._tor_provider.get_listener.mock_calls, [mock.call()])
-        self.assertEqual(t.listening_ports, [i2p_ep, tor_ep])
+        self.assertIn(i2p_ep, n.tub.listening_ports)
+        self.assertIn(tor_ep, n.tub.listening_ports)
 
 class ClientNotListening(unittest.TestCase):
     def test_disabled(self):
         basedir = "test_node/test_disabled"
-        fileutil.make_dirs(basedir)
+        create_node_dir(basedir, "testing")
         f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
         f.write(BASE_CONFIG)
         f.write(NOLISTEN)
@@ -476,7 +482,7 @@ class ClientNotListening(unittest.TestCase):
 
     def test_disabled_but_storage(self):
         basedir = "test_node/test_disabled_but_storage"
-        fileutil.make_dirs(basedir)
+        create_node_dir(basedir, "testing")
         f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
         f.write(BASE_CONFIG)
         f.write(NOLISTEN)
@@ -487,7 +493,7 @@ class ClientNotListening(unittest.TestCase):
 
     def test_disabled_but_helper(self):
         basedir = "test_node/test_disabled_but_helper"
-        fileutil.make_dirs(basedir)
+        create_node_dir(basedir, "testing")
         f = open(os.path.join(basedir, 'tahoe.cfg'), 'wt')
         f.write(BASE_CONFIG)
         f.write(NOLISTEN)
@@ -500,12 +506,11 @@ class ClientNotListening(unittest.TestCase):
 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 = disabled\n")
-        f.write("tub.location = disabled\n")
-        f.close()
+        create_node_dir(basedir, "testing")
+        with open(os.path.join(basedir, 'tahoe.cfg'), 'wt') as f:
+            f.write("[node]\n")
+            f.write("tub.port = disabled\n")
+            f.write("tub.location = disabled\n")
         e = self.assertRaises(ValueError, create_introducer, basedir)
         self.assertIn("we are Introducer, but tub is not listening", str(e))
 
diff --git a/src/allmydata/test/web/test_introducer.py b/src/allmydata/test/web/test_introducer.py
index 80b69f15f..2efe17fcd 100644
--- a/src/allmydata/test/web/test_introducer.py
+++ b/src/allmydata/test/web/test_introducer.py
@@ -2,6 +2,7 @@ from twisted.trial import unittest
 from foolscap.api import fireEventually, flushEventualQueue
 from twisted.internet import defer
 from allmydata.introducer import IntroducerNode
+from allmydata import node
 from .common import FAVICON_MARKUP
 from ..common_web import do_http
 
@@ -23,9 +24,12 @@ class IntroducerWeb(unittest.TestCase):
             "tub.location = 127.0.0.1:1\n"
             "web.port = tcp:0\n"
         )
+        basedir = self.mktemp()
+        node.create_node_dir(basedir, "testing")
+
         from allmydata.node import config_from_string
         self.node = IntroducerNode(
-            config_from_string(config, "introducer.port", "no-basedir"),
+            config_from_string(config, "introducer.port", basedir),
         )
         self.ws = self.node.getServiceNamed("webish")
 

From 107ddcd1bacf70848976f244a4b6e31e4d5a13f7 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Tue, 27 Feb 2018 15:01:02 -0700
Subject: [PATCH 12/38] get rid of get_clientdir

---
 src/allmydata/test/no_network.py      | 4 ++--
 src/allmydata/test/test_configutil.py | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py
index fd8e74f4b..51cee7e2e 100644
--- a/src/allmydata/test/no_network.py
+++ b/src/allmydata/test/no_network.py
@@ -397,8 +397,8 @@ class GridTestMixin:
         self.client_baseurls = [c.getServiceNamed("webish").getURL()
                                 for c in self.g.clients]
 
-    def get_clientdir(self, i=0):
-        return self.g.clients[i].config._basedir
+    def get_client_config(self, i=0):
+        return self.g.clients[i].config
 
     def get_client(self, i=0):
         return self.g.clients[i]
diff --git a/src/allmydata/test/test_configutil.py b/src/allmydata/test/test_configutil.py
index 5786be8e0..ab40d1e6b 100644
--- a/src/allmydata/test/test_configutil.py
+++ b/src/allmydata/test/test_configutil.py
@@ -13,10 +13,9 @@ class ConfigUtilTests(GridTestMixin, unittest.TestCase):
     def test_config_utils(self):
         self.basedir = "cli/ConfigUtilTests/test-config-utils"
         self.set_up_grid(oneshare=True)
-        tahoe_cfg = os.path.join(self.get_clientdir(i=0), "tahoe.cfg")
 
         # test that at least one option was read correctly
-        config = configutil.get_config(tahoe_cfg)
+        config = self.get_client_config(i=0)
         self.failUnlessEqual(config.get("node", "nickname"), "client-0")
 
         # test that set_config can mutate an existing option

From 4c7f60f42a6843bc59ddbe39746988cfc26a3e22 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Tue, 27 Feb 2018 15:01:18 -0700
Subject: [PATCH 13/38] test fixups from review

---
 src/allmydata/test/test_node.py | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index cf3fdc8c7..43f6019e3 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -19,6 +19,7 @@ from allmydata.introducer.server import create_introducer
 from allmydata.client import create_client
 from allmydata.client import _valid_config_sections as client_valid_config_sections
 from allmydata.util import fileutil, iputil
+from allmydata.util import i2p_provider, tor_provider
 from allmydata.util.namespace import Namespace
 from allmydata.util.configutil import UnknownConfigError
 import allmydata.test.common_util as testutil
@@ -168,8 +169,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
     def test_write_config_unwritable_file(self):
         """
         Existing behavior merely logs any errors upon writing
-        configuration; this should probably be fixed to do something
-        better (like fail entirely). See #2905
+        configuration files; this bad behavior should probably be
+        fixed to do something better (like fail entirely). See #2905
         """
         basedir = "test_node/configdir"
         fileutil.make_dirs(basedir)
@@ -180,12 +181,8 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
         config.write_config_file("bad", "some value")
 
-        errs = self.flushLoggedErrors()
+        errs = self.flushLoggedErrors(IOError)
         self.assertEqual(1, len(errs))
-        self.assertIn(
-            "IOError",
-            str(errs[0])
-        )
 
     def test_timestamp(self):
         # this modified logger doesn't seem to get used during the tests,

From 2937c729e30c1217769c0e323c49c7b41507078a Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Wed, 28 Feb 2018 11:13:14 -0700
Subject: [PATCH 14/38] change imports; introducer client.read_config

---
 src/allmydata/client.py         | 26 ++++++++++++++++----------
 src/allmydata/test/test_node.py | 12 ++++++------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 554ab5b91..4d853da43 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -28,9 +28,7 @@ from allmydata.history import History
 from allmydata.interfaces import IStatsProducer, SDMF_VERSION, MDMF_VERSION
 from allmydata.nodemaker import NodeMaker
 from allmydata.blacklist import Blacklist
-from allmydata.node import OldConfigOptionError, _common_config_sections
-from allmydata.node import read_config
-from allmydata.node import get_app_versions
+from allmydata import node
 
 
 KiB=1024
@@ -40,7 +38,7 @@ TiB=1024*GiB
 PiB=1024*TiB
 
 def _valid_config_sections():
-    cfg = _common_config_sections()
+    cfg = node._common_config_sections()
     cfg.update({
         "client": (
             "helper.furl",
@@ -165,6 +163,14 @@ class Terminator(service.Service):
         return service.Service.stopService(self)
 
 
+def read_config(basedir, portnumfile, generated_files=[]):
+    return node.read_config(
+        basedir, portnumfile,
+        generated_files=generated_files,
+        _valid_config_sections=_valid_config_sections,
+    )
+
+
 #@defer.inlineCallbacks
 def create_client(basedir=u".", _client_factory=None):
     """
@@ -175,9 +181,7 @@ def create_client(basedir=u".", _client_factory=None):
     :param _client_factory: for testing; the class to instantiate
     """
     node.create_node_dir(basedir, CLIENT_README)
-
-    # load configuration
-    config = read_config(basedir, u"client.port", _valid_config_sections=_valid_config_sections)
+    config = read_config(basedir, u"client.port")
 
     if _client_factory is None:
         _client_factory = _Client
@@ -301,7 +305,7 @@ class _Client(node.Node, pollmixin.PollMixin):
                                   self.nickname,
                                   str(allmydata.__full_version__),
                                   str(self.OLDEST_SUPPORTED_VERSION),
-                                  get_app_versions(), self._sequencer,
+                                  node.get_app_versions(), self._sequencer,
                                   introducer_cache_filepath)
             self.introducer_clients.append(ic)
             self.introducer_furls.append(introducer['furl'])
@@ -596,8 +600,10 @@ class _Client(node.Node, pollmixin.PollMixin):
     def init_magic_folder(self):
         #print "init_magic_folder"
         if self.config.get_config("drop_upload", "enabled", False, boolean=True):
-            raise OldConfigOptionError("The [drop_upload] section must be renamed to [magic_folder].\n"
-                                       "See docs/frontends/magic-folder.rst for more information.")
+            raise node.OldConfigOptionError(
+                "The [drop_upload] section must be renamed to [magic_folder].\n"
+                "See docs/frontends/magic-folder.rst for more information."
+            )
 
         if self.config.get_config("magic_folder", "enabled", False, boolean=True):
             from allmydata.frontends import magic_folder
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index 43f6019e3..b2110f1dc 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -16,8 +16,8 @@ import foolscap.logging.log
 from twisted.application import service
 from allmydata.node import Node, formatTimeTahoeStyle, MissingConfigEntry, read_config, config_from_string, create_node_dir
 from allmydata.introducer.server import create_introducer
-from allmydata.client import create_client
-from allmydata.client import _valid_config_sections as client_valid_config_sections
+from allmydata import client
+
 from allmydata.util import fileutil, iputil
 from allmydata.util import i2p_provider, tor_provider
 from allmydata.util.namespace import Namespace
@@ -436,7 +436,7 @@ class Listeners(unittest.TestCase):
             f.write("tub.location = tcp:example.org:1234\n")
         # we're doing a lot of calling-into-setup-methods here, it might be
         # better to just create a real Node instance, I'm not sure.
-        config = read_config(
+        config = client.read_config(
             n.basedir,
             "client.port",
             _valid_config_sections=client_valid_config_sections,
@@ -474,7 +474,7 @@ class ClientNotListening(unittest.TestCase):
         f.write(NOLISTEN)
         f.write(DISABLE_STORAGE)
         f.close()
-        n = create_client(basedir)
+        n = client.create_client(basedir)
         self.assertEqual(n.tub.getListeners(), [])
 
     def test_disabled_but_storage(self):
@@ -485,7 +485,7 @@ class ClientNotListening(unittest.TestCase):
         f.write(NOLISTEN)
         f.write(ENABLE_STORAGE)
         f.close()
-        e = self.assertRaises(ValueError, create_client, basedir)
+        e = self.assertRaises(ValueError, client.create_client, basedir)
         self.assertIn("storage is enabled, but tub is not listening", str(e))
 
     def test_disabled_but_helper(self):
@@ -497,7 +497,7 @@ class ClientNotListening(unittest.TestCase):
         f.write(DISABLE_STORAGE)
         f.write(ENABLE_HELPER)
         f.close()
-        e = self.assertRaises(ValueError, create_client, basedir)
+        e = self.assertRaises(ValueError, client.create_client, basedir)
         self.assertIn("helper is enabled, but tub is not listening", str(e))
 
 class IntroducerNotListening(unittest.TestCase):

From 4d7f8ec9dd3accc8bf237b4cf6f09c8094985e97 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Wed, 28 Feb 2018 11:13:54 -0700
Subject: [PATCH 15/38] keep clientdir for now, improve how we access it

---
 src/allmydata/test/cli/test_backup.py | 3 +--
 src/allmydata/test/no_network.py      | 5 +++++
 src/allmydata/test/test_configutil.py | 3 ++-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/test/cli/test_backup.py b/src/allmydata/test/cli/test_backup.py
index 1e24193e1..4c8f164cf 100644
--- a/src/allmydata/test/cli/test_backup.py
+++ b/src/allmydata/test/cli/test_backup.py
@@ -193,8 +193,7 @@ class Backup(GridTestMixin, CLITestMixin, StallMixin, unittest.TestCase):
         # sneak into the backupdb, crank back the "last checked"
         # timestamp to force a check on all files
         def _reset_last_checked(res):
-            dbfile = os.path.join(self.get_clientdir(),
-                                  "private", "backupdb.sqlite")
+            dbfile = self.get_client_config().get_private_path("backupdb.sqlite")
             self.failUnless(os.path.exists(dbfile), dbfile)
             bdb = backupdb.get_backupdb(dbfile)
             bdb.cursor.execute("UPDATE last_upload SET last_checked=0")
diff --git a/src/allmydata/test/no_network.py b/src/allmydata/test/no_network.py
index 51cee7e2e..a005b689b 100644
--- a/src/allmydata/test/no_network.py
+++ b/src/allmydata/test/no_network.py
@@ -400,6 +400,11 @@ class GridTestMixin:
     def get_client_config(self, i=0):
         return self.g.clients[i].config
 
+    def get_clientdir(self, i=0):
+        # ideally, use something get_client_config() only, we
+        # shouldn't need to manipulate raw paths..
+        return self.get_client_config(i).get_config_path()
+
     def get_client(self, i=0):
         return self.g.clients[i]
 
diff --git a/src/allmydata/test/test_configutil.py b/src/allmydata/test/test_configutil.py
index ab40d1e6b..5786be8e0 100644
--- a/src/allmydata/test/test_configutil.py
+++ b/src/allmydata/test/test_configutil.py
@@ -13,9 +13,10 @@ class ConfigUtilTests(GridTestMixin, unittest.TestCase):
     def test_config_utils(self):
         self.basedir = "cli/ConfigUtilTests/test-config-utils"
         self.set_up_grid(oneshare=True)
+        tahoe_cfg = os.path.join(self.get_clientdir(i=0), "tahoe.cfg")
 
         # test that at least one option was read correctly
-        config = self.get_client_config(i=0)
+        config = configutil.get_config(tahoe_cfg)
         self.failUnlessEqual(config.get("node", "nickname"), "client-0")
 
         # test that set_config can mutate an existing option

From ab947704f0690df7fcbc439ba6d84b9e2316bfb1 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Tue, 27 Feb 2018 15:56:58 -0700
Subject: [PATCH 16/38] replace PortLocations tests

The old tests were hard to read due to all the "if"
statements; these might be slightly more verbose but
also more explicit
---
 src/allmydata/test/test_node.py | 224 +++++++++++++++++++++-----------
 1 file changed, 149 insertions(+), 75 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index b2110f1dc..c02c2e8b8 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -265,86 +265,160 @@ EXPECTED = {
         },
     }
 
-class PortLocation(unittest.TestCase):
-    def test_all(self):
-        for tp in EXPECTED.keys():
-            for tl in EXPECTED[tp].keys():
-                exp = EXPECTED[tp][tl]
-                self._try(tp, tl, exp)
 
-    def _try(self, tp, tl, exp):
-        log.msg("PortLocation._try:", tp, tl, exp)
-        cfg_tubport = {"missing": None,
-                       "empty": "",
-                       "disabled": "disabled",
-                       "endpoint": "tcp:777",
-                       }[tp]
-        cfg_location = {"missing": None,
-                        "empty": "",
-                        "disabled": "disabled",
-                        "hintstring": "tcp:HOST:888,AUTO",
-                        }[tl]
+class TestMissingPorts(unittest.TestCase):
+    """
+    Test certain error-cases for ports setup
+    """
 
-        basedir = os.path.join("test_node/portlocation/%s/%s" % (tp, tl))
-        create_node_dir(basedir, "testing")
-        config = read_config(
-            basedir,
-            "node.port",
-            _valid_config_sections=client_valid_config_sections,
+    def setUp(self):
+        self.basedir = self.mktemp()
+        create_node_dir(self.basedir, "testing")
+
+    def test_0(self):
+        get_addr = mock.patch(
+            "allmydata.util.iputil.get_local_addresses_sync",
+            return_value=["LOCAL"],
         )
-        n = EmptyNode(config)
-        n._reveal_ip = True
+        alloc_port = mock.patch(
+            "allmydata.util.iputil.allocate_tcp_port",
+            return_value=999,
+        )
+        config = read_config(self.basedir, "portnum")
 
-        if exp in ("ERR1", "ERR2", "ERR3", "ERR4"):
-            e = self.assertRaises(ValueError, n.get_tub_portlocation,
-                                  cfg_tubport, cfg_location)
-            if exp == "ERR1":
-                self.assertEqual("tub.port must not be empty", str(e))
-            elif exp == "ERR2":
-                self.assertEqual("tub.location must not be empty", str(e))
-            elif exp == "ERR3":
-                self.assertEqual("tub.port is disabled, but not tub.location",
-                                 str(e))
-            elif exp == "ERR4":
-                self.assertEqual("tub.location is disabled, but not tub.port",
-                                 str(e))
-            else:
-                self.assert_(False)
-        elif exp == "no-listen":
+        with get_addr, alloc_port:
+            n = Node(config)
+            # could probably refactor this get_tub_portlocation into a
+            # bare helper instead of method.
+            cfg_tubport = "tcp:777"
+            cfg_location = "AUTO"
+            tubport, tublocation = n.get_tub_portlocation(cfg_tubport, cfg_location)
+        self.assertEqual(tubport, "tcp:777")
+        self.assertEqual(tublocation, "tcp:LOCAL:777")
+
+    def test_1(self):
+        get_addr = mock.patch(
+            "allmydata.util.iputil.get_local_addresses_sync",
+            return_value=["LOCAL"],
+        )
+        alloc_port = mock.patch(
+            "allmydata.util.iputil.allocate_tcp_port",
+            return_value=999,
+        )
+        config = read_config(self.basedir, "portnum")
+
+        with get_addr, alloc_port:
+            n = Node(config)
+            # could probably refactor this get_tub_portlocation into a
+            # bare helper instead of method.
+            cfg_tubport = None
+            cfg_location = None
+            tubport, tublocation = n.get_tub_portlocation(cfg_tubport, cfg_location)
+        self.assertEqual(tubport, "tcp:999")
+        self.assertEqual(tublocation, "tcp:LOCAL:999")
+
+    def test_2(self):
+        get_addr = mock.patch(
+            "allmydata.util.iputil.get_local_addresses_sync",
+            return_value=["LOCAL"],
+        )
+        alloc_port = mock.patch(
+            "allmydata.util.iputil.allocate_tcp_port",
+            return_value=999,
+        )
+        config = read_config(self.basedir, "portnum")
+
+        with get_addr, alloc_port:
+            n = Node(config)
+            # could probably refactor this get_tub_portlocation into a
+            # bare helper instead of method.
+            cfg_tubport = None
+            cfg_location = "tcp:HOST:888,AUTO"
+            tubport, tublocation = n.get_tub_portlocation(cfg_tubport, cfg_location)
+        self.assertEqual(tubport, "tcp:999")
+        self.assertEqual(tublocation, "tcp:HOST:888,tcp:LOCAL:999")
+
+    def test_3(self):
+        get_addr = mock.patch(
+            "allmydata.util.iputil.get_local_addresses_sync",
+            return_value=["LOCAL"],
+        )
+        alloc_port = mock.patch(
+            "allmydata.util.iputil.allocate_tcp_port",
+            return_value=999,
+        )
+        config = read_config(self.basedir, "portnum")
+
+        with get_addr, alloc_port:
+            n = Node(config)
+            # could probably refactor this get_tub_portlocation into a
+            # bare helper instead of method.
+            cfg_tubport = "disabled"
+            cfg_location = "disabled"
             res = n.get_tub_portlocation(cfg_tubport, cfg_location)
-            self.assertEqual(res, None)
-        elif exp in ("alloc/auto", "alloc/file", "auto", "manual"):
-            with mock.patch("allmydata.util.iputil.get_local_addresses_sync",
-                            return_value=["LOCAL"]):
-                with mock.patch("allmydata.util.iputil.allocate_tcp_port",
-                                return_value=999):
-                    port, location = n.get_tub_portlocation(cfg_tubport,
-                                                            cfg_location)
-            try:
-                with open(config.portnum_fname, "r") as f:
-                    saved_port = f.read().strip()
-            except EnvironmentError:
-                saved_port = None
-            if exp == "alloc/auto":
-                self.assertEqual(port, "tcp:999")
-                self.assertEqual(location, "tcp:LOCAL:999")
-                self.assertEqual(saved_port, "tcp:999")
-            elif exp == "alloc/file":
-                self.assertEqual(port, "tcp:999")
-                self.assertEqual(location, "tcp:HOST:888,tcp:LOCAL:999")
-                self.assertEqual(saved_port, "tcp:999")
-            elif exp == "auto":
-                self.assertEqual(port, "tcp:777")
-                self.assertEqual(location, "tcp:LOCAL:777")
-                self.assertEqual(saved_port, None)
-            elif exp == "manual":
-                self.assertEqual(port, "tcp:777")
-                self.assertEqual(location, "tcp:HOST:888,tcp:LOCAL:777")
-                self.assertEqual(saved_port, None)
-            else:
-                self.assert_(False)
-        else:
-            self.assert_(False)
+        self.assertTrue(res is None)
+
+    def test_empty_tub_port(self):
+        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
+            f.write(
+                "[node]\n"
+                "tub.port = \n"
+            )
+        config = read_config(self.basedir, "portnum")
+
+        with self.assertRaises(ValueError) as ctx:
+            Node(config)
+        self.assertIn(
+            "tub.port must not be empty",
+            str(ctx.exception)
+        )
+
+    def test_empty_tub_location(self):
+        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
+            f.write(
+                "[node]\n"
+                "tub.location = \n"
+            )
+        config = read_config(self.basedir, "portnum")
+
+        with self.assertRaises(ValueError) as ctx:
+            Node(config)
+        self.assertIn(
+            "tub.location must not be empty",
+            str(ctx.exception)
+        )
+
+    def test_disabled_port_not_tub(self):
+        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
+            f.write(
+                "[node]\n"
+                "tub.port = disabled\n"
+                "tub.location = not_disabled\n"
+            )
+        config = read_config(self.basedir, "portnum")
+
+        with self.assertRaises(ValueError) as ctx:
+            Node(config)
+        self.assertIn(
+            "tub.port is disabled, but not tub.location",
+            str(ctx.exception)
+        )
+
+    def test_disabled_tub_not_port(self):
+        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
+            f.write(
+                "[node]\n"
+                "tub.port = not_disabled\n"
+                "tub.location = disabled\n"
+            )
+        config = read_config(self.basedir, "portnum")
+
+        with self.assertRaises(ValueError) as ctx:
+            Node(config)
+        self.assertIn(
+            "tub.location is disabled, but not tub.port",
+            str(ctx.exception)
+        )
 
 BASE_CONFIG = """
 [client]

From 6c388b9d58919acd958fc48ca47a471bd9a445e3 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Mar 2018 15:45:10 -0700
Subject: [PATCH 17/38] test for authtoken API

---
 src/allmydata/test/test_client.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py
index 77cda2d73..18b6c67d9 100644
--- a/src/allmydata/test/test_client.py
+++ b/src/allmydata/test/test_client.py
@@ -7,7 +7,7 @@ import allmydata
 import allmydata.frontends.magic_folder
 import allmydata.util.log
 
-from allmydata.node import OldConfigError, OldConfigOptionError, UnescapedHashError, _Config, read_config
+from allmydata.node import OldConfigError, OldConfigOptionError, UnescapedHashError, _Config, read_config, create_node_dir
 from allmydata.frontends.auth import NeedRootcapLookupScheme
 from allmydata import client
 from allmydata.storage_client import StorageFarmBroker
@@ -212,6 +212,20 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                            "reserved_space = bogus\n")
         self.failUnlessRaises(ValueError, client.create_client, basedir)
 
+    def test_web_apiauthtoken(self):
+        basedir = u"client.Basic.test_web_apiauthtoken"
+        create_node_dir(basedir, "testing")
+        config = read_config(basedir, "portnum")
+
+        c = client.create_client(basedir)
+        # this must come after we create the client, as it will create
+        # a new, random authtoken itself
+        with open(os.path.join(basedir, "private", "api_auth_token"), "w") as f:
+            f.write("deadbeef")
+
+        token = c.get_auth_token()
+        self.assertEqual("deadbeef", token)
+
     def test_web_staticdir(self):
         basedir = u"client.Basic.test_web_staticdir"
         os.mkdir(basedir)

From dd2209a96c70b30875f89c72e74f52e7b0c93bdf Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Mar 2018 16:07:52 -0700
Subject: [PATCH 18/38] tests to cover config changes

---
 src/allmydata/test/test_introducer.py |  5 ++++
 src/allmydata/test/test_node.py       | 38 +++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py
index 0697379d4..6f907a876 100644
--- a/src/allmydata/test/test_introducer.py
+++ b/src/allmydata/test/test_introducer.py
@@ -41,6 +41,11 @@ class Node(testutil.SignalMixin, testutil.ReallyEqualMixin, unittest.TestCase):
         from allmydata.introducer import IntroducerNode
         IntroducerNode  # pyflakes
 
+    def test_create(self):
+        basedir = "introducer.IntroducerNode.test_create"
+        create_introducer(basedir)
+        self.assertTrue(os.path.exists(basedir))
+
     def test_furl(self):
         basedir = "introducer.IntroducerNode.test_furl"
         create_node_dir(basedir, "testing")
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index c02c2e8b8..dde499d7c 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -137,6 +137,44 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         config = read_config(basedir, "")
         self.failUnless(config.nickname == nickname)
 
+    def test_config_required(self):
+        basedir = u"test_node/test_config_required"
+        config = read_config(basedir, "portnum")
+
+        with self.assertRaises(Exception) as ctx:
+            config.get_config_from_file("it_does_not_exist", required=True)
+
+    def test_private_config_unreadable(self):
+        basedir = u"test_node/test_private_config_unreadable"
+        create_node_dir(basedir, "testing")
+        config = read_config(basedir, "portnum")
+        config.get_or_create_private_config("foo", "contents")
+        fname = os.path.join(basedir, "private", "foo")
+        os.chmod(fname, 0)
+
+        with self.assertRaises(Exception) as ctx:
+            config.get_or_create_private_config("foo")
+
+    def test_private_config_unreadable_preexisting(self):
+        basedir = u"test_node/test_private_config_unreadable_preexisting"
+        create_node_dir(basedir, "testing")
+        config = read_config(basedir, "portnum")
+        fname = os.path.join(basedir, "private", "foo")
+        with open(fname, "w") as f:
+            f.write("stuff")
+        os.chmod(fname, 0)
+
+        with self.assertRaises(Exception) as ctx:
+            config.get_private_config("foo")
+
+    def test_private_config_missing(self):
+        basedir = u"test_node/test_private_config_missing"
+        create_node_dir(basedir, "testing")
+        config = read_config(basedir, "portnum")
+
+        with self.assertRaises(MissingConfigEntry) as ctx:
+            config.get_or_create_private_config("foo")
+
     def test_private_config(self):
         basedir = "test_node/test_private_config"
         privdir = os.path.join(basedir, "private")

From abd7b638b88a796570c9a4e4d896389fa279287b Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Mar 2018 16:38:42 -0700
Subject: [PATCH 19/38] basic stfpd setup test

---
 src/allmydata/test/test_client.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py
index 18b6c67d9..0178c7e6c 100644
--- a/src/allmydata/test/test_client.py
+++ b/src/allmydata/test/test_client.py
@@ -2,6 +2,7 @@ import os, sys
 import twisted
 from twisted.trial import unittest
 from twisted.application import service
+import mock
 
 import allmydata
 import allmydata.frontends.magic_folder
@@ -242,6 +243,21 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
 
     # TODO: also test config options for SFTP.
 
+    def test_ftp_create(self):
+        basedir = u"client.Basic.test_ftp_create"
+        create_node_dir(basedir, "testing")
+        with open(os.path.join(basedir, "tahoe.cfg"), "w") as f:
+            f.write(
+                '[sftpd]\n'
+                'enabled = true\n'
+                'accounts.file = foo\n'
+                'host_pubkey_file = pubkey\n'
+                'host_privkey_file = privkey\n'
+            )
+        with mock.patch('allmydata.frontends.sftpd.SFTPServer') as p:
+            c = client.create_client(basedir)
+        self.assertTrue(p.called)
+
     def test_ftp_auth_keyfile(self):
         basedir = u"client.Basic.test_ftp_auth_keyfile"
         os.mkdir(basedir)

From 27ea11d164c722bc1fcbc81ebf4cd074dc194e63 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Mar 2018 20:39:11 -0700
Subject: [PATCH 20/38] dead code

---
 src/allmydata/test/test_node.py | 39 +--------------------------------
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index dde499d7c..b0df47f95 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -267,43 +267,6 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         self.failUnless(ns.called)
 
 
-class EmptyNode(Node):
-    def __init__(self, config=None):
-        if config is None:
-            config = config_from_string("", "no portfile", 'no basedir')
-        Node.__init__(self, config)
-
-
-EXPECTED = {
-    # top-level key is tub.port category
-    "missing": {
-        # 2nd-level key is tub.location category
-        "missing": "alloc/auto",
-        "empty": "ERR2",
-        "disabled": "ERR4",
-        "hintstring": "alloc/file",
-        },
-    "empty": {
-        "missing": "ERR1",
-        "empty": "ERR1",
-        "disabled": "ERR1",
-        "hintstring": "ERR1",
-        },
-    "disabled": {
-        "missing": "ERR3",
-        "empty": "ERR2",
-        "disabled": "no-listen",
-        "hintstring": "ERR3",
-        },
-    "endpoint": {
-        "missing": "auto",
-        "empty": "ERR2",
-        "disabled": "ERR4",
-        "hintstring": "manual",
-        },
-    }
-
-
 class TestMissingPorts(unittest.TestCase):
     """
     Test certain error-cases for ports setup
@@ -570,7 +533,7 @@ class Listeners(unittest.TestCase):
 
         tub_mock = mock.patch("allmydata.node.Tub", return_value=FakeTub())
         with i2p_mock, tor_mock, tub_mock:
-            n = EmptyNode(config)
+            n = Node(config)
 
         self.assertEqual(n._i2p_provider.get_listener.mock_calls, [mock.call()])
         self.assertEqual(n._tor_provider.get_listener.mock_calls, [mock.call()])

From 5000787c185718ec3096bb6473cadeaa409a8278 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Mar 2018 20:51:35 -0700
Subject: [PATCH 21/38] flake8

---
 src/allmydata/client.py           | 1 -
 src/allmydata/test/test_client.py | 3 +--
 src/allmydata/test/test_node.py   | 8 ++++----
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 4d853da43..77643d205 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -28,7 +28,6 @@ from allmydata.history import History
 from allmydata.interfaces import IStatsProducer, SDMF_VERSION, MDMF_VERSION
 from allmydata.nodemaker import NodeMaker
 from allmydata.blacklist import Blacklist
-from allmydata import node
 
 
 KiB=1024
diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py
index 0178c7e6c..d1caeb1f3 100644
--- a/src/allmydata/test/test_client.py
+++ b/src/allmydata/test/test_client.py
@@ -216,7 +216,6 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
     def test_web_apiauthtoken(self):
         basedir = u"client.Basic.test_web_apiauthtoken"
         create_node_dir(basedir, "testing")
-        config = read_config(basedir, "portnum")
 
         c = client.create_client(basedir)
         # this must come after we create the client, as it will create
@@ -255,7 +254,7 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
                 'host_privkey_file = privkey\n'
             )
         with mock.patch('allmydata.frontends.sftpd.SFTPServer') as p:
-            c = client.create_client(basedir)
+            client.create_client(basedir)
         self.assertTrue(p.called)
 
     def test_ftp_auth_keyfile(self):
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index b0df47f95..f81869536 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -141,7 +141,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         basedir = u"test_node/test_config_required"
         config = read_config(basedir, "portnum")
 
-        with self.assertRaises(Exception) as ctx:
+        with self.assertRaises(Exception):
             config.get_config_from_file("it_does_not_exist", required=True)
 
     def test_private_config_unreadable(self):
@@ -152,7 +152,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         fname = os.path.join(basedir, "private", "foo")
         os.chmod(fname, 0)
 
-        with self.assertRaises(Exception) as ctx:
+        with self.assertRaises(Exception):
             config.get_or_create_private_config("foo")
 
     def test_private_config_unreadable_preexisting(self):
@@ -164,7 +164,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             f.write("stuff")
         os.chmod(fname, 0)
 
-        with self.assertRaises(Exception) as ctx:
+        with self.assertRaises(Exception):
             config.get_private_config("foo")
 
     def test_private_config_missing(self):
@@ -172,7 +172,7 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         create_node_dir(basedir, "testing")
         config = read_config(basedir, "portnum")
 
-        with self.assertRaises(MissingConfigEntry) as ctx:
+        with self.assertRaises(MissingConfigEntry):
             config.get_or_create_private_config("foo")
 
     def test_private_config(self):

From 629185d98ff0b41a7cab00ff9705e1040c47754b Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Thu, 1 Mar 2018 21:42:08 -0700
Subject: [PATCH 22/38] skip some tests on windows (permissions)

---
 src/allmydata/test/test_node.py | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index f81869536..ac8346989 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -145,6 +145,11 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             config.get_config_from_file("it_does_not_exist", required=True)
 
     def test_private_config_unreadable(self):
+        if "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower():
+            # We don't know how to test that unprivileged users can't read this
+            # thing.  (Also we don't know exactly how to set the permissions so
+            # that unprivileged users can't read this thing.)
+            raise unittest.SkipTest("We don't know how to set permissions on Windows.")
         basedir = u"test_node/test_private_config_unreadable"
         create_node_dir(basedir, "testing")
         config = read_config(basedir, "portnum")
@@ -156,6 +161,11 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             config.get_or_create_private_config("foo")
 
     def test_private_config_unreadable_preexisting(self):
+        if "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower():
+            # We don't know how to test that unprivileged users can't read this
+            # thing.  (Also we don't know exactly how to set the permissions so
+            # that unprivileged users can't read this thing.)
+            raise unittest.SkipTest("We don't know how to set permissions on Windows.")
         basedir = u"test_node/test_private_config_unreadable_preexisting"
         create_node_dir(basedir, "testing")
         config = read_config(basedir, "portnum")

From a432fc35da03eaa75c074f58ba44f0573c768d39 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 15:41:31 -0600
Subject: [PATCH 23/38] docstring improvements

---
 src/allmydata/client.py | 16 ++++++++++++++--
 src/allmydata/node.py   | 18 +++++++++++++++++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 77643d205..a9135470a 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -163,6 +163,13 @@ class Terminator(service.Service):
 
 
 def read_config(basedir, portnumfile, generated_files=[]):
+    """
+    Read and validate configuration for a client-style Node. See
+    :method:`allmydata.node.read_config` for parameter meanings (the
+    only difference here is we pass different validation data)
+
+    :returns: :class:`allmydata.node._Config` instance
+    """
     return node.read_config(
         basedir, portnumfile,
         generated_files=generated_files,
@@ -175,9 +182,14 @@ def create_client(basedir=u".", _client_factory=None):
     """
     Creates a new client instance (a subclass of Node).
 
-    :param basedir: the node directory
+    :param unicode basedir: the node directory (which may not exist yet)
 
-    :param _client_factory: for testing; the class to instantiate
+    :param _client_factory: (for testing) a callable that returns an
+        instance of :class:`allmydata.node.Node` (or a subclass). By default
+        this is :class:`allmydata.client._Client`
+
+    :returns: :class:`allmydata.client._Client` instance (or whatever
+        `_client_factory` returns)
     """
     node.create_node_dir(basedir, CLIENT_README)
     config = read_config(basedir, u"client.port")
diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 695d77c49..132521f48 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -94,7 +94,7 @@ such as private keys.  On Unix-like systems, the permissions on this directory
 are set to disallow users other than its owner from reading the contents of
 the files.   See the 'configuration.rst' documentation file for details."""
 
-class _None(object):  # used as a marker in get_config()
+class _None(object):
     """
     This class is to be used as a marker in get_config()
     """
@@ -145,6 +145,22 @@ def create_node_dir(basedir, readme_text):
 
 
 def read_config(basedir, portnumfile, generated_files=[], _valid_config_sections=None):
+    """
+    Read and validate configuration.
+
+    :param unicode basedir: directory where configuration data begins
+
+    :param unicode portnumfile: filename fragment for "port number" files
+
+    :param list generated_files: a list of automatically-generated
+        configuration files.
+
+    :param dict _valid_config_sections: (internal use, optional) a
+        dict-of-dicts structure defining valid configuration sections and
+        keys
+
+    :returns: :class:`allmydata.node._Config` instance
+    """
     basedir = abspath_expanduser_unicode(unicode(basedir))
     if _valid_config_sections is None:
         _valid_config_sections = _common_config_sections

From 1b30e9edfc90fefac42a6290ed77e6765e2e0dd4 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 15:41:44 -0600
Subject: [PATCH 24/38] consistently use 'config' not 'self.config' in __init__

---
 src/allmydata/client.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index a9135470a..84461a089 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -259,7 +259,7 @@ class _Client(node.Node, pollmixin.PollMixin):
         # If the node sees an exit_trigger file, it will poll every second to see
         # whether the file still exists, and what its mtime is. If the file does not
         # exist or has not been modified for a given timeout, the node will exit.
-        exit_trigger_file = self.config.get_config_path(self.EXIT_TRIGGER_FILE)
+        exit_trigger_file = config.get_config_path(self.EXIT_TRIGGER_FILE)
         if os.path.exists(exit_trigger_file):
             age = time.time() - os.stat(exit_trigger_file)[stat.ST_MTIME]
             self.log("%s file noticed (%ds old), starting timer" % (self.EXIT_TRIGGER_FILE, age))
@@ -268,7 +268,7 @@ class _Client(node.Node, pollmixin.PollMixin):
 
         # this needs to happen last, so it can use getServiceNamed() to
         # acquire references to StorageServer and other web-statusable things
-        webport = self.config.get_config("node", "web.port", None)
+        webport = config.get_config("node", "web.port", None)
         if webport:
             self.init_web(webport) # strports string
 

From 37d4b59a39caf2336f20dd946705200a9e11dad1 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 15:42:00 -0600
Subject: [PATCH 25/38] modern syntax

---
 src/allmydata/node.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 132521f48..e3dab73cc 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -139,7 +139,7 @@ def create_node_dir(basedir, readme_text):
         fileutil.make_dirs(basedir)
     privdir = os.path.join(basedir, "private")
     if not os.path.exists(privdir):
-        fileutil.make_dirs(privdir, 0700)
+        fileutil.make_dirs(privdir, 0o700)
         with open(os.path.join(privdir, 'README'), 'w') as f:
             f.write(readme_text)
 

From c0772cdd5f0c745db20fc1d0060657f2858cec54 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 15:44:17 -0600
Subject: [PATCH 26/38] improve docstring

---
 src/allmydata/node.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index e3dab73cc..058e4ddf7 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -195,7 +195,9 @@ def config_from_string(config_str, portnumfile, basedir):
 
 
 def get_app_versions():
-    # TODO: merge this with allmydata.get_package_versions
+    """
+    :returns: dict of versions important to Foolscap
+    """
     return dict(app_versions.versions)
 
 

From 536ccf8b6df0b5e17e639826fad8cfa1b236159f Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 15:50:55 -0600
Subject: [PATCH 27/38] better 'file not found' handling

---
 src/allmydata/node.py | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 058e4ddf7..803a0227b 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -6,6 +6,7 @@ import datetime
 import os.path
 import re
 import types
+import errno
 import ConfigParser
 import tempfile
 from io import BytesIO
@@ -14,6 +15,7 @@ from base64 import b32decode, b32encode
 from twisted.internet import reactor
 from twisted.python import log as twlog
 from twisted.application import service
+from twisted.python.failure import Failure
 from foolscap.api import Tub, app_versions
 import foolscap.logging.log
 from allmydata import get_package_versions, get_package_versions_string
@@ -176,8 +178,8 @@ def read_config(basedir, portnumfile, generated_files=[], _valid_config_sections
     parser = ConfigParser.SafeConfigParser()
     try:
         parser = configutil.get_config(config_fname)
-    except EnvironmentError:
-        if os.path.exists(config_fname):
+    except EnvironmentError as e:
+        if e.errno != errno.ENOENT:
             raise
 
     configutil.validate_config(config_fname, parser, _valid_config_sections())
@@ -269,9 +271,11 @@ class _Config(object):
         fn = os.path.join(self._basedir, name)
         try:
             fileutil.write(fn, value, mode)
-        except EnvironmentError as e:
-            log.msg("Unable to write config file '{}'".format(fn))
-            log.err(e)
+        except EnvironmentError:
+            log.err(
+                Failure(),
+                "Unable to write config file '{}'".format(fn),
+            )
 
     def get_config(self, section, option, default=_None, boolean=False):
         try:
@@ -302,7 +306,9 @@ class _Config(object):
         fn = os.path.join(self._basedir, name)
         try:
             return fileutil.read(fn).strip()
-        except EnvironmentError:
+        except EnvironmentError as e:
+            if e.errno != errno.ENOENT:
+                raise  # we only care about "file doesn't exist"
             if not required:
                 return None
             raise
@@ -322,9 +328,9 @@ class _Config(object):
         privname = os.path.join(self._basedir, "private", name)
         try:
             value = fileutil.read(privname)
-        except EnvironmentError:
-            if os.path.exists(privname):
-                raise
+        except EnvironmentError as e:
+            if e.errno != errno.ENOENT:
+                raise  # we only care about "file doesn't exist"
             if default is _None:
                 raise MissingConfigEntry("The required configuration file %s is missing."
                                          % (quote_output(privname),))
@@ -353,9 +359,9 @@ class _Config(object):
         privname = os.path.join(self._basedir, "private", name)
         try:
             return fileutil.read(privname).strip()
-        except EnvironmentError:
-            if os.path.exists(privname):
-                raise
+        except EnvironmentError as e:
+            if e.errno != errno.ENOENT:
+                raise  # we only care about "file doesn't exist"
             if default is _None:
                 raise MissingConfigEntry("The required configuration file %s is missing."
                                          % (quote_output(privname),))

From d478cf38313677290a4e61b9fad26957b1f7dfbd Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 16:50:04 -0600
Subject: [PATCH 28/38] add some docstrings, fix comments

---
 src/allmydata/test/test_client.py     | 6 ++++++
 src/allmydata/test/test_introducer.py | 3 +++
 src/allmydata/test/test_node.py       | 6 ++++++
 3 files changed, 15 insertions(+)

diff --git a/src/allmydata/test/test_client.py b/src/allmydata/test/test_client.py
index d1caeb1f3..c75cf91da 100644
--- a/src/allmydata/test/test_client.py
+++ b/src/allmydata/test/test_client.py
@@ -214,6 +214,9 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
         self.failUnlessRaises(ValueError, client.create_client, basedir)
 
     def test_web_apiauthtoken(self):
+        """
+        Client loads the proper API auth token from disk
+        """
         basedir = u"client.Basic.test_web_apiauthtoken"
         create_node_dir(basedir, "testing")
 
@@ -243,6 +246,9 @@ class Basic(testutil.ReallyEqualMixin, testutil.NonASCIIPathMixin, unittest.Test
     # TODO: also test config options for SFTP.
 
     def test_ftp_create(self):
+        """
+        configuration for sftpd results in it being started
+        """
         basedir = u"client.Basic.test_ftp_create"
         create_node_dir(basedir, "testing")
         with open(os.path.join(basedir, "tahoe.cfg"), "w") as f:
diff --git a/src/allmydata/test/test_introducer.py b/src/allmydata/test/test_introducer.py
index 6f907a876..dc5761643 100644
--- a/src/allmydata/test/test_introducer.py
+++ b/src/allmydata/test/test_introducer.py
@@ -42,6 +42,9 @@ class Node(testutil.SignalMixin, testutil.ReallyEqualMixin, unittest.TestCase):
         IntroducerNode  # pyflakes
 
     def test_create(self):
+        """
+        A brand new introducer creates its config dir
+        """
         basedir = "introducer.IntroducerNode.test_create"
         create_introducer(basedir)
         self.assertTrue(os.path.exists(basedir))
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index ac8346989..da2eaf136 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -138,6 +138,9 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         self.failUnless(config.nickname == nickname)
 
     def test_config_required(self):
+        """
+        Asking for missing (but required) configuration is an error
+        """
         basedir = u"test_node/test_config_required"
         config = read_config(basedir, "portnum")
 
@@ -145,6 +148,9 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             config.get_config_from_file("it_does_not_exist", required=True)
 
     def test_private_config_unreadable(self):
+        """
+        Asking for inaccessible private config is an error
+        """
         if "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower():
             # We don't know how to test that unprivileged users can't read this
             # thing.  (Also we don't know exactly how to set the permissions so

From 38dac24b2bd8dd2dc9db0f95efbad528845eeb9c Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Fri, 24 Aug 2018 16:51:47 -0600
Subject: [PATCH 29/38] use skipIf decorator, not inline logic

---
 src/allmydata/test/test_node.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index da2eaf136..ae578824f 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -5,6 +5,7 @@ import sys
 import time
 import mock
 
+from unittest import skipIf
 
 from twisted.trial import unittest
 from twisted.internet import defer
@@ -147,15 +148,14 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         with self.assertRaises(Exception):
             config.get_config_from_file("it_does_not_exist", required=True)
 
+    @skipIf(
+        "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower(),
+        "We don't know how to set permissions on Windows.",
+    )
     def test_private_config_unreadable(self):
         """
         Asking for inaccessible private config is an error
         """
-        if "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower():
-            # We don't know how to test that unprivileged users can't read this
-            # thing.  (Also we don't know exactly how to set the permissions so
-            # that unprivileged users can't read this thing.)
-            raise unittest.SkipTest("We don't know how to set permissions on Windows.")
         basedir = u"test_node/test_private_config_unreadable"
         create_node_dir(basedir, "testing")
         config = read_config(basedir, "portnum")

From 89872b832ccb7f11226f1afaff3c067c7f2ff9ca Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 25 Aug 2018 00:31:16 -0600
Subject: [PATCH 30/38] docstring, naming improvements

---
 src/allmydata/test/test_node.py | 83 +++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 30 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index ae578824f..32cbe3dff 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -184,6 +184,9 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
             config.get_private_config("foo")
 
     def test_private_config_missing(self):
+        """
+        a missing config with no default is an error
+        """
         basedir = u"test_node/test_private_config_missing"
         create_node_dir(basedir, "testing")
         config = read_config(basedir, "portnum")
@@ -292,7 +295,10 @@ class TestMissingPorts(unittest.TestCase):
         self.basedir = self.mktemp()
         create_node_dir(self.basedir, "testing")
 
-    def test_0(self):
+    def test_parsing_tcp(self):
+        """
+        parse explicit tub.port with explicitly-default tub.location
+        """
         get_addr = mock.patch(
             "allmydata.util.iputil.get_local_addresses_sync",
             return_value=["LOCAL"],
@@ -313,7 +319,10 @@ class TestMissingPorts(unittest.TestCase):
         self.assertEqual(tubport, "tcp:777")
         self.assertEqual(tublocation, "tcp:LOCAL:777")
 
-    def test_1(self):
+    def test_parsing_defaults(self):
+        """
+        parse empty config, check defaults
+        """
         get_addr = mock.patch(
             "allmydata.util.iputil.get_local_addresses_sync",
             return_value=["LOCAL"],
@@ -334,7 +343,10 @@ class TestMissingPorts(unittest.TestCase):
         self.assertEqual(tubport, "tcp:999")
         self.assertEqual(tublocation, "tcp:LOCAL:999")
 
-    def test_2(self):
+    def test_parsing_location_complex(self):
+        """
+        location with two options (including defaults)
+        """
         get_addr = mock.patch(
             "allmydata.util.iputil.get_local_addresses_sync",
             return_value=["LOCAL"],
@@ -355,7 +367,10 @@ class TestMissingPorts(unittest.TestCase):
         self.assertEqual(tubport, "tcp:999")
         self.assertEqual(tublocation, "tcp:HOST:888,tcp:LOCAL:999")
 
-    def test_3(self):
+    def test_parsing_all_disabled(self):
+        """
+        parse config with both port + location disabled
+        """
         get_addr = mock.patch(
             "allmydata.util.iputil.get_local_addresses_sync",
             return_value=["LOCAL"],
@@ -376,12 +391,14 @@ class TestMissingPorts(unittest.TestCase):
         self.assertTrue(res is None)
 
     def test_empty_tub_port(self):
-        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
-            f.write(
-                "[node]\n"
-                "tub.port = \n"
-            )
-        config = read_config(self.basedir, "portnum")
+        """
+        port povided, but empty is an error
+        """
+        config_data = (
+            "[node]\n"
+            "tub.port = \n"
+        )
+        config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)
@@ -391,12 +408,14 @@ class TestMissingPorts(unittest.TestCase):
         )
 
     def test_empty_tub_location(self):
-        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
-            f.write(
-                "[node]\n"
-                "tub.location = \n"
-            )
-        config = read_config(self.basedir, "portnum")
+        """
+        location povided, but empty is an error
+        """
+        config_data = (
+            "[node]\n"
+            "tub.location = \n"
+        )
+        config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)
@@ -406,13 +425,15 @@ class TestMissingPorts(unittest.TestCase):
         )
 
     def test_disabled_port_not_tub(self):
-        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
-            f.write(
-                "[node]\n"
-                "tub.port = disabled\n"
-                "tub.location = not_disabled\n"
-            )
-        config = read_config(self.basedir, "portnum")
+        """
+        error to disable port but not location
+        """
+        config_data = (
+            "[node]\n"
+            "tub.port = disabled\n"
+            "tub.location = not_disabled\n"
+        )
+        config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)
@@ -422,13 +443,15 @@ class TestMissingPorts(unittest.TestCase):
         )
 
     def test_disabled_tub_not_port(self):
-        with open(os.path.join(self.basedir, "tahoe.cfg"), "w") as f:
-            f.write(
-                "[node]\n"
-                "tub.port = not_disabled\n"
-                "tub.location = disabled\n"
-            )
-        config = read_config(self.basedir, "portnum")
+        """
+        error to disable location but not port
+        """
+        config_data = (
+            "[node]\n"
+            "tub.port = not_disabled\n"
+            "tub.location = disabled\n"
+        )
+        config = config_from_string(self.basedir, "portnum", config_data)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)

From 7db48e3677ba68cc1edbd139b5e7586015d7e358 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 25 Aug 2018 00:36:58 -0600
Subject: [PATCH 31/38] skip using decorator

---
 src/allmydata/test/test_node.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index 32cbe3dff..d791161a4 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -166,12 +166,14 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
         with self.assertRaises(Exception):
             config.get_or_create_private_config("foo")
 
+    @skipIf(
+        "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower(),
+        "We don't know how to set permissions on Windows.",
+    )
     def test_private_config_unreadable_preexisting(self):
-        if "win32" in sys.platform.lower() or "cygwin" in sys.platform.lower():
-            # We don't know how to test that unprivileged users can't read this
-            # thing.  (Also we don't know exactly how to set the permissions so
-            # that unprivileged users can't read this thing.)
-            raise unittest.SkipTest("We don't know how to set permissions on Windows.")
+        """
+        error if reading private config data fails
+        """
         basedir = u"test_node/test_private_config_unreadable_preexisting"
         create_node_dir(basedir, "testing")
         config = read_config(basedir, "portnum")

From 74b560c3c2cae6f08ef56d7ce7ffddbff72ad1cb Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 25 Aug 2018 00:37:11 -0600
Subject: [PATCH 32/38] failUnless -> assert

---
 src/allmydata/test/test_node.py | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index d791161a4..01700f61e 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -206,24 +206,24 @@ class TestCase(testutil.SignalMixin, unittest.TestCase):
 
         config = config_from_string("", "", basedir)
 
-        self.failUnlessEqual(config.get_private_config("already"), "secret")
-        self.failUnlessEqual(config.get_private_config("not", "default"), "default")
-        self.failUnlessRaises(MissingConfigEntry, config.get_private_config, "not")
+        self.assertEqual(config.get_private_config("already"), "secret")
+        self.assertEqual(config.get_private_config("not", "default"), "default")
+        self.assertRaises(MissingConfigEntry, config.get_private_config, "not")
         value = config.get_or_create_private_config("new", "start")
-        self.failUnlessEqual(value, "start")
-        self.failUnlessEqual(config.get_private_config("new"), "start")
+        self.assertEqual(value, "start")
+        self.assertEqual(config.get_private_config("new"), "start")
         counter = []
         def make_newer():
             counter.append("called")
             return "newer"
         value = config.get_or_create_private_config("newer", make_newer)
-        self.failUnlessEqual(len(counter), 1)
-        self.failUnlessEqual(value, "newer")
-        self.failUnlessEqual(config.get_private_config("newer"), "newer")
+        self.assertEqual(len(counter), 1)
+        self.assertEqual(value, "newer")
+        self.assertEqual(config.get_private_config("newer"), "newer")
 
         value = config.get_or_create_private_config("newer", make_newer)
-        self.failUnlessEqual(len(counter), 1) # don't call unless necessary
-        self.failUnlessEqual(value, "newer")
+        self.assertEqual(len(counter), 1) # don't call unless necessary
+        self.assertEqual(value, "newer")
 
     def test_write_config_unwritable_file(self):
         """

From dab291c72fdb4a1979fbab244612fafdfc0fb167 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 25 Aug 2018 01:26:49 -0600
Subject: [PATCH 33/38] docstring

---
 src/allmydata/test/test_system.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py
index 28c494a4d..ed2878199 100644
--- a/src/allmydata/test/test_system.py
+++ b/src/allmydata/test/test_system.py
@@ -536,6 +536,9 @@ class SystemTestMixin(pollmixin.PollMixin, testutil.StallMixin):
         return s
 
     def _create_introducer(self):
+        """
+        :returns: (via Deferred) an Introducer instance
+        """
         iv_dir = self.getdir("introducer")
         if not os.path.isdir(iv_dir):
             introducer_config = (

From cce4a92900ec311f50917bfdd65a1f78533a8f7b Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Mon, 20 Aug 2018 17:40:53 -0600
Subject: [PATCH 34/38] fix path-creation cases after merge

---
 src/allmydata/frontends/magic_folder.py | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/allmydata/frontends/magic_folder.py b/src/allmydata/frontends/magic_folder.py
index 27543dc5f..1c6c60e23 100644
--- a/src/allmydata/frontends/magic_folder.py
+++ b/src/allmydata/frontends/magic_folder.py
@@ -309,11 +309,7 @@ class MagicFolder(service.MultiService):
         :param dict config: Magic-folder configuration like that in the list
             returned by ``load_magic_folders``.
         """
-        db_filename = os.path.join(
-            client_node.basedir,
-            "private",
-            "magicfolder_{}.sqlite".format(name),
-        )
+        db_filename = client_node.config.get_private_path("magicfolder_{}.sqlite".format(name))
         local_dir_config = config['directory']
         try:
             poll_interval = int(config["poll_interval"])
@@ -324,9 +320,10 @@ class MagicFolder(service.MultiService):
             client=client_node,
             upload_dircap=config["upload_dircap"],
             collective_dircap=config["collective_dircap"],
+            # XXX surely a better way for this local_path_u business
             local_path_u=abspath_expanduser_unicode(
                 local_dir_config,
-                base=client_node.basedir,
+                base=client_node.config.get_config_path(),
             ),
             dbfile=abspath_expanduser_unicode(db_filename),
             umask=config["umask"],

From cc4e6b988fd47996026ed18ff8ad7306000b4764 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Mon, 20 Aug 2018 18:12:15 -0600
Subject: [PATCH 35/38] add newsfragment

---
 newsfragments/2936.bugfix | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 newsfragments/2936.bugfix

diff --git a/newsfragments/2936.bugfix b/newsfragments/2936.bugfix
new file mode 100644
index 000000000..8bca69813
--- /dev/null
+++ b/newsfragments/2936.bugfix
@@ -0,0 +1 @@
+refactor configuration handling out of Node into _Config
\ No newline at end of file

From 91517bbad08914dc767d9251d781e9ec1c16ff97 Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Mon, 20 Aug 2018 18:13:00 -0600
Subject: [PATCH 36/38] unused import

---
 src/allmydata/client.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/allmydata/client.py b/src/allmydata/client.py
index 84461a089..ebf55f076 100644
--- a/src/allmydata/client.py
+++ b/src/allmydata/client.py
@@ -20,7 +20,6 @@ from allmydata.util import (hashutil, base32, pollmixin, log, keyutil, idlib,
                             yamlutil)
 from allmydata.util.encodingutil import (get_filesystem_encoding,
                                          from_utf8_or_none)
-from allmydata.util.fileutil import abspath_expanduser_unicode
 from allmydata.util.abbreviate import parse_abbreviated_size
 from allmydata.util.time_format import parse_duration, parse_date
 from allmydata.stats import StatsProvider

From 65ebde6f9d18734db23c1cac1bafa747a20676ba Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Mon, 20 Aug 2018 23:41:55 -0600
Subject: [PATCH 37/38] fix incorrect rebase resolutions

---
 src/allmydata/test/test_node.py | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index 01700f61e..b11587740 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -37,9 +37,8 @@ class TestNode(Node):
         config = read_config(
             basedir,
             'DEFAULT_PORTNUMFILE_BLANK',
-            _valid_config_sections=client_valid_config_sections,
         )
-        Node.__init__(self, config, basedir)
+        Node.__init__(self, config)
 
 
 class TestCase(testutil.SignalMixin, unittest.TestCase):
@@ -522,12 +521,11 @@ class Listeners(unittest.TestCase):
 
         # we're doing a lot of calling-into-setup-methods here, it might be
         # better to just create a real Node instance, I'm not sure.
-        config = read_config(
+        config = client.read_config(
             basedir,
             "client.port",
-            _valid_config_sections=client_valid_config_sections,
         )
-        n = EmptyNode(config)
+        n = Node(config)
         n.check_privacy()
         n.services = []
         n.create_i2p_provider()
@@ -553,9 +551,8 @@ class Listeners(unittest.TestCase):
         # we're doing a lot of calling-into-setup-methods here, it might be
         # better to just create a real Node instance, I'm not sure.
         config = client.read_config(
-            n.basedir,
+            basedir,
             "client.port",
-            _valid_config_sections=client_valid_config_sections,
         )
 
         i2p_ep = object()
@@ -643,7 +640,6 @@ class Configuration(unittest.TestCase):
             read_config(
                 self.basedir,
                 "client.port",
-                _valid_config_sections=_valid_config_sections,
             )
 
         self.assertIn(
@@ -658,7 +654,7 @@ class Configuration(unittest.TestCase):
                 'foo = bar\n'
             )
         with self.assertRaises(UnknownConfigError) as ctx:
-            create_client(self.basedir)
+            client.create_client(self.basedir)
 
         self.assertIn(
             "invalid section",

From 0f22b9bad06a70d56b327dcaa4973abafce3e30b Mon Sep 17 00:00:00 2001
From: meejah <meejah@meejah.ca>
Date: Sat, 25 Aug 2018 02:23:58 -0600
Subject: [PATCH 38/38] fixups after rebase

---
 src/allmydata/node.py           | 4 +++-
 src/allmydata/test/test_node.py | 8 ++++----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/allmydata/node.py b/src/allmydata/node.py
index 803a0227b..671b922cc 100644
--- a/src/allmydata/node.py
+++ b/src/allmydata/node.py
@@ -190,7 +190,9 @@ def read_config(basedir, portnumfile, generated_files=[], _valid_config_sections
 
 
 def config_from_string(config_str, portnumfile, basedir):
-    # load configuration from in-memory string
+    """
+    load configuration from in-memory string
+    """
     parser = ConfigParser.SafeConfigParser()
     parser.readfp(BytesIO(config_str))
     return _Config(parser, portnumfile, basedir, '<in-memory>')
diff --git a/src/allmydata/test/test_node.py b/src/allmydata/test/test_node.py
index b11587740..7e0b103d4 100644
--- a/src/allmydata/test/test_node.py
+++ b/src/allmydata/test/test_node.py
@@ -399,7 +399,7 @@ class TestMissingPorts(unittest.TestCase):
             "[node]\n"
             "tub.port = \n"
         )
-        config = config_from_string(self.basedir, "portnum", config_data)
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)
@@ -416,7 +416,7 @@ class TestMissingPorts(unittest.TestCase):
             "[node]\n"
             "tub.location = \n"
         )
-        config = config_from_string(self.basedir, "portnum", config_data)
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)
@@ -434,7 +434,7 @@ class TestMissingPorts(unittest.TestCase):
             "tub.port = disabled\n"
             "tub.location = not_disabled\n"
         )
-        config = config_from_string(self.basedir, "portnum", config_data)
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)
@@ -452,7 +452,7 @@ class TestMissingPorts(unittest.TestCase):
             "tub.port = not_disabled\n"
             "tub.location = disabled\n"
         )
-        config = config_from_string(self.basedir, "portnum", config_data)
+        config = config_from_string(config_data, "portnum", self.basedir)
 
         with self.assertRaises(ValueError) as ctx:
             Node(config)