diff --git a/lightningd/options.c b/lightningd/options.c index 3ace67d976a3..dddd5d736573 100644 --- a/lightningd/options.c +++ b/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); diff --git a/tests/plugins/clnrest-use-options.py b/tests/plugins/clnrest-use-options.py new file mode 100755 index 000000000000..1c520bcaf8e5 --- /dev/null +++ b/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() diff --git a/tests/test_clnrest.py b/tests/test_clnrest.py index a9b3daea975a..195880d026ea 100644 --- a/tests/test_clnrest.py +++ b/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}')