Merge bitcoin#10267: New -includeconf argument for including external…

… configuration files

25b7ab9 doc: Add release notes for -includeconf (Karl-Johan Alm)
0f0badd test: Test includeconf parameter. (Karl-Johan Alm)
629ff8c -includeconf=<path> support in config handler, for including external configuration files (Karl-Johan Alm)

Pull request description:

  Fixes: bitcoin#10071.

  Done:
  - adds `-includeconf=<path>`, where `<path>` is relative to `datadir` or to the path of the file being read, if in a file
  - protects against circular includes
  - updates help docs

  ~~~Thoughts:~~~
  - ~~~I am not sure how to test this in a neat manner. Feedback on this would be nice. Will dig/think though.~~~

Tree-SHA512: cb31f1b2f69fbc0890d264948eb2e501ac05cf12f5e06a5942f9c1539eb15ea8dc3cae817f4073aecb2fcc21d0386747f14f89d990772003a76e2a6d25642553

src/dash-cli.cpp

@@ -115,7 +115,7 @@ static int AppInitRPC(int argc, char* argv[])
         return EXIT_FAILURE;
     }
     try {
-        gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
+        gArgs.ReadConfigFiles();
     } catch (const std::exception& e) {
         fprintf(stderr,"Error reading configuration file: %s\n", e.what());
         return EXIT_FAILURE;

src/dashd.cpp

@@ -102,7 +102,7 @@ static bool AppInit(int argc, char* argv[])
         }
         try
         {
-            gArgs.ReadConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
+            gArgs.ReadConfigFiles();
         } catch (const std::exception& e) {
             fprintf(stderr,"Error reading configuration file: %s\n", e.what());
             return false;

src/init.cpp

@@ -483,6 +483,7 @@ void SetupServerArgs()
     gArgs.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), true, OptionsCategory::OPTIONS);
     gArgs.AddArg("-dbcache=<n>", strprintf("Set database cache size in megabytes (%d to %d, default: %d)", nMinDbCache, nMaxDbCache, nDefaultDbCache), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (0 to disable; default: %s)", DEFAULT_DEBUGLOGFILE), false, OptionsCategory::OPTIONS);
+    gArgs.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-loadblock=<file>", "Imports blocks from external blk000??.dat file on startup", false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-maxmempool=<n>", strprintf("Keep the transaction memory pool below <n> megabytes (default: %u)", DEFAULT_MAX_MEMPOOL_SIZE), false, OptionsCategory::OPTIONS);
     gArgs.AddArg("-maxorphantxsize=<n>", strprintf("Maximum total size of all orphan transactions in megabytes (default: %u)", DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE), false, OptionsCategory::OPTIONS);

src/interfaces/node.cpp

@@ -164,7 +164,7 @@ class NodeImpl : public Node
     {
         gArgs.ParseParameters(argc, argv);
     }
-    void readConfigFile(const std::string& conf_path) override { gArgs.ReadConfigFile(conf_path); }
+    void readConfigFiles() override { gArgs.ReadConfigFiles(); }
     bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
     bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
     void selectParams(const std::string& network) override { SelectParams(network); }

src/interfaces/node.h

@@ -102,7 +102,7 @@ class Node
     virtual bool softSetBoolArg(const std::string& arg, bool value) = 0;
 
     //! Load settings from configuration file.
-    virtual void readConfigFile(const std::string& conf_path) = 0;
+    virtual void readConfigFiles() = 0;
 
     //! Choose network parameters.
     virtual void selectParams(const std::string& network) = 0;

src/qt/dash.cpp

@@ -655,7 +655,7 @@ int main(int argc, char *argv[])
         return EXIT_FAILURE;
     }
     try {
-        node->readConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME));
+        node->readConfigFiles();
     } catch (const std::exception& e) {
         QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
                               QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what()));

src/util.cpp

@@ -456,6 +456,17 @@ void ArgsManager::ParseParameters(int argc, const char* const argv[])
             m_override_args[key].push_back(val);
         }
     }
+
+    // we do not allow -includeconf from command line, so we clear it here
+    auto it = m_override_args.find("-includeconf");
+    if (it != m_override_args.end()) {
+        if (it->second.size() > 0) {
+            for (const auto& ic : it->second) {
+                fprintf(stderr, "warning: -includeconf cannot be used from commandline; ignoring -includeconf=%s\n", ic.c_str());
+            }
+            m_override_args.erase(it);
+        }
+    }
 }
 
 std::vector<std::string> ArgsManager::GetArgs(const std::string& strArg) const
@@ -793,19 +804,41 @@ void ArgsManager::ReadConfigStream(std::istream& stream)
     }
 }
 
-void ArgsManager::ReadConfigFile(const std::string& confPath)
+void ArgsManager::ReadConfigFiles()
 {
     {
         LOCK(cs_args);
         m_config_args.clear();
     }
 
+    const std::string confPath = GetArg("-conf", BITCOIN_CONF_FILENAME);
     fs::ifstream stream(GetConfigFile(confPath));
 
     if (stream.good()) {
         ReadConfigStream(stream);
+        // if there is an -includeconf in the override args, but it is empty, that means the user
+        // passed '-noincludeconf' on the command line, in which case we should not include anything
+        if (m_override_args.count("-includeconf") == 0) {
+            std::vector<std::string> includeconf(GetArgs("-includeconf"));
+            {
+                // We haven't set m_network yet (that happens in SelectParams()), so manually check
+                // for network.includeconf args.
+                std::vector<std::string> includeconf_net(GetArgs(std::string("-") + GetChainName() + ".includeconf"));
+                includeconf.insert(includeconf.end(), includeconf_net.begin(), includeconf_net.end());
+            }
+
+            for (const std::string& to_include : includeconf) {
+                fs::ifstream include_config(GetConfigFile(to_include));
+                if (include_config.good()) {
+                    ReadConfigStream(include_config);
+                    LogPrintf("Included configuration file %s\n", to_include.c_str());
+                } else {
+                    fprintf(stderr, "Failed to include configuration file %s\n", to_include.c_str());
+                }
+            }
+        }
     } else {
-        // Create an empty dash.conf if it does not excist
+        // Create an empty dash.conf if it does not exist
         FILE* configFile = fopen(GetConfigFile(confPath).string().c_str(), "a");
         if (configFile != nullptr)
             fclose(configFile);

src/util.h

@@ -187,7 +187,7 @@ class ArgsManager
     void SelectConfigNetwork(const std::string& network);
 
     void ParseParameters(int argc, const char*const argv[]);
-    void ReadConfigFile(const std::string& confPath);
+    void ReadConfigFiles();
 
     /**
      * Log warnings for options in m_section_only_args when

test/functional/feature_includeconf.py

@@ -0,0 +1,78 @@
+#!/usr/bin/env python3
+# Copyright (c) 2018 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Tests the includeconf argument
+
+Verify that:
+
+1. adding includeconf to the configuration file causes the includeconf
+   file to be loaded in the correct order.
+2. includeconf cannot be used as a command line argument.
+3. includeconf cannot be used recursively (ie includeconf can only
+   be used from the base config file).
+4. multiple includeconf arguments can be specified in the main config
+   file.
+"""
+import os
+import tempfile
+
+from test_framework.test_framework import BitcoinTestFramework, assert_equal
+
+class IncludeConfTest(BitcoinTestFramework):
+    def set_test_params(self):
+        self.setup_clean_chain = False
+        self.num_nodes = 1
+
+    def setup_chain(self):
+        super().setup_chain()
+        # Create additional config files
+        # - tmpdir/node0/relative.conf
+        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+            f.write("uacomment=relative\n")
+        # - tmpdir/node0/relative2.conf
+        with open(os.path.join(self.options.tmpdir, "node0", "relative2.conf"), "w", encoding="utf8") as f:
+            f.write("uacomment=relative2\n")
+        with open(os.path.join(self.options.tmpdir, "node0", "dash.conf"), "a", encoding='utf8') as f:
+            f.write("uacomment=main\nincludeconf=relative.conf\n")
+
+    def run_test(self):
+        self.log.info("-includeconf works from config file. subversion should end with 'main; relative)/'")
+
+        subversion = self.nodes[0].getnetworkinfo()["subversion"]
+        assert subversion.endswith("main; relative)/")
+
+        self.log.info("-includeconf cannot be used as command-line arg. subversion should still end with 'main; relative)/'")
+        self.stop_node(0)
+        with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
+            self.start_node(0, extra_args=["-includeconf=relative2.conf"], stderr=log_stderr)
+
+            subversion = self.nodes[0].getnetworkinfo()["subversion"]
+            assert subversion.endswith("main; relative)/")
+            log_stderr.seek(0)
+            stderr = log_stderr.read().decode('utf-8').strip()
+            assert_equal(stderr, 'warning: -includeconf cannot be used from commandline; ignoring -includeconf=relative2.conf')
+
+        self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
+        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
+            f.write("includeconf=relative2.conf\n")
+
+        self.restart_node(0)
+
+        subversion = self.nodes[0].getnetworkinfo()["subversion"]
+        assert subversion.endswith("main; relative)/")
+
+        self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
+        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+            f.write("uacomment=relative\n")
+
+        with open(os.path.join(self.options.tmpdir, "node0", "dash.conf"), "a", encoding='utf8') as f:
+            f.write("includeconf=relative2.conf\n")
+
+        self.restart_node(0)
+
+        subversion = self.nodes[0].getnetworkinfo()["subversion"]
+        assert subversion.endswith("main; relative; relative2)/")
+
+if __name__ == '__main__':
+    IncludeConfTest().main()

test/functional/test_runner.py

@@ -170,6 +170,7 @@
     'feature_uacomment.py',
     'p2p_unrequested_blocks.py',
     'feature_asmap.py',
+    'feature_includeconf.py',
     'feature_logging.py',
     'p2p_node_network_limited.py',
     'feature_blocksdir.py',