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

Since these worked in v23.08, we can't just rename them.  So if they are
used and unclaimed, we should rename them internally (if they're claimed,
it's probably clightning-rest, and we should *NOT* touch them!).

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Deprecated: Plugins: `clnrest` parameters `rest-port`, `rest-protocol`, `rest-host` and `rest-certs`: prefix `cln` to them

Author: rustyrussell

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}')