lightningd: fix up deprecated rest-port, rest-protocol, rest-host and rest-certs option if we would otherwise fail.

lightningd/options.c

@@ -1811,8 +1811,43 @@ void handle_early_opts(struct lightningd *ld, int argc, char *argv[])
 	logging_options_parsed(ld->log_book);
 }
 
+/* Free *str, set *str to copy with `cln` prepended */
+static void prefix_cln(const char **str STEALS)
+{
+	const char *newstr = tal_fmt(tal_parent(*str), "cln%s", *str);
+	tal_free(*str);
+	*str = newstr;
+}
+
+/* Due to a conflict between the widely-deployed clightning-rest plugin and
+ * our own clnrest plugin, and people wanting to run both, in v23.11 we
+ * renamed some options.  This breaks perfectly working v23.08 deployments who
+ * don't care about clightning-rest, so we work around it here. */
+static void fixup_clnrest_options(struct lightningd *ld)
+{
+	for (size_t i = 0; i < tal_count(ld->configvars); i++) {
+		struct configvar *cv = ld->configvars[i];
+
+		/* These worked for v23.08 */
+		if (!strstarts(cv->configline, "rest-port=")
+		    && !strstarts(cv->configline, "rest-protocol=")
+		    && !strstarts(cv->configline, "rest-host=")
+		    && !strstarts(cv->configline, "rest-certs="))
+			continue;
+		/* Did some (plugin) claim it? */
+		if (opt_find_long(cv->configline, &cv->optarg))
+			continue;
+		log_unusual(ld->log, "Option %s deprecated in v23.11, renaming to cln%s",
+			    cv->configline, cv->configline);
+		prefix_cln(&cv->configline);
+	}
+}
+
 void handle_opts(struct lightningd *ld)
 {
+	if (ld->deprecated_apis)
+		fixup_clnrest_options(ld);
+
 	/* Now we know all the options, finish parsing and finish
 	 * populating ld->configvars with cmdline. */
 	parse_configvars_final(ld->configvars, true, ld->developer);

tests/plugins/clnrest-use-options.py

@@ -0,0 +1,18 @@
+#!/usr/bin/env python3
+"""Register rest-port to test that we don't "fix" it."""
+
+from pyln.client import Plugin
+
+
+plugin = Plugin()
+
+
+@plugin.init()
+def init(configuration, options, plugin):
+    print(f"rest-port is {plugin.get_option('rest-port')}")
+
+
+plugin.add_option('rest-port', None, "Parameter to clash with clnrest.py deprecated one")
+
+
+plugin.run()

tests/test_clnrest.py

@@ -3,6 +3,7 @@
 from pyln.testing.utils import env, TEST_NETWORK
 from pyln.client import Millisatoshi
 import unittest
+import os
 import requests
 from pathlib import Path
 from requests.adapters import HTTPAdapter
@@ -433,3 +434,35 @@ def test_clnrest_http_headers(node_factory):
                                 headers={'Origin': 'http://192.168.1.10:1010'},
                                 verify=ca_cert)
     assert response.headers['Access-Control-Allow-Origin'] == 'http://192.168.1.10:1010'
+
+
+def test_clnrest_old_params(node_factory):
+    """Test that we handle the v23.08-style parameters"""
+    rest_port = str(reserve())
+    rest_host = '127.0.0.1'
+    base_url = f'https://{rest_host}:{rest_port}'
+    l1 = node_factory.get_node(options={'rest-port': rest_port,
+                                        'rest-host': rest_host,
+                                        'allow-deprecated-apis': True})
+    # This might happen really early!
+    l1.daemon.logsearch_start = 0
+    l1.daemon.wait_for_logs([r'UNUSUAL lightningd: Option rest-port=.* deprecated in v23\.11, renaming to clnrest-port',
+                             r'UNUSUAL lightningd: Option rest-host=.* deprecated in v23\.11, renaming to clnrest-host'])
+    l1.daemon.wait_for_log(r'plugin-clnrest.py: REST server running at ' + base_url)
+
+    # Now try one where a plugin (e.g. clightning-rest) registers the option.
+    plugin = os.path.join(os.path.dirname(__file__), 'plugins/clnrest-use-options.py')
+    l2 = node_factory.get_node(options={'rest-port': rest_port,
+                                        'rest-host': rest_host,
+                                        'plugin': plugin,
+                                        'allow-deprecated-apis': True})
+
+    l2.daemon.logsearch_start = 0
+    # We still rename this one, since it's for clnrest.
+    assert l2.daemon.is_in_log(r'UNUSUAL lightningd: Option rest-host=.* deprecated in v23\.11, renaming to clnrest-host')
+
+    # This one does not get renamed!
+    assert not l2.daemon.is_in_log(r'UNUSUAL lightningd: Option rest-port=.* deprecated in v23\.11, renaming to clnrest-host')
+    assert [p for p in l2.rpc.plugin('list')['plugins'] if 'clnrest.py' in p['name']] == []
+    assert l2.daemon.is_in_log(r'plugin-clnrest.py: Killing plugin: disabled itself at init: `clnrest-port` option is not configured')
+    assert l2.daemon.is_in_log(rf'clnrest-use-options.py: rest-port is {rest_port}')