diff --git a/doc/lightningd-config.5 b/doc/lightningd-config.5 index 0bfaa18f5..5c7ec02e8 100644 --- a/doc/lightningd-config.5 +++ b/doc/lightningd-config.5 @@ -528,9 +528,11 @@ normal effect\. \fBdisable-plugin\fR=\fIPLUGIN\fR -If \fIPLUGIN\fR contains a /, plugins with the same path as \fIPLUGIN\fR are -disabled\. Otherwise, any plugin with that base name is disabled, -whatever directory it is in\. +If \fIPLUGIN\fR contains a /, plugins with the same path as \fIPLUGIN\fR will +not be loaded at startup\. Otherwise, no plugin with that base name will +be loaded at startup, whatever directory it is in\. This option is useful for +disabling a single plugin inside a directory\. You can still explicitly +load plugins which have been disabled, using \fBlightning-plugin\fR(7) \fBstart\fR\. .SH BUGS diff --git a/doc/lightningd-config.5.md b/doc/lightningd-config.5.md index 9ddb1bb69..e79df85a9 100644 --- a/doc/lightningd-config.5.md +++ b/doc/lightningd-config.5.md @@ -434,9 +434,11 @@ including the default built-in plugin directory. You can still add normal effect. **disable-plugin**=*PLUGIN* -If *PLUGIN* contains a /, plugins with the same path as *PLUGIN* are -disabled. Otherwise, any plugin with that base name is disabled, -whatever directory it is in. +If *PLUGIN* contains a /, plugins with the same path as *PLUGIN* will +not be loaded at startup. Otherwise, no plugin with that base name will +be loaded at startup, whatever directory it is in. This option is useful for +disabling a single plugin inside a directory. You can still explicitly +load plugins which have been disabled, using lightning-plugin(7) `start`. BUGS ---- diff --git a/lightningd/options.c b/lightningd/options.c index ca08154f6..83cffbeaa 100644 --- a/lightningd/options.c +++ b/lightningd/options.c @@ -343,15 +343,18 @@ static char *opt_add_proxy_addr(const char *arg, struct lightningd *ld) static char *opt_add_plugin(const char *arg, struct lightningd *ld) { + if (plugin_blacklisted(ld->plugins, arg)) { + log_info(ld->log, "%s: disabled via disable-plugin", arg); + return NULL; + } plugin_register(ld->plugins, arg, NULL); return NULL; } static char *opt_disable_plugin(const char *arg, struct lightningd *ld) { - if (plugin_remove(ld->plugins, arg)) - return NULL; - return tal_fmt(NULL, "Could not find plugin %s", arg); + plugin_blacklist(ld->plugins, arg); + return NULL; } static char *opt_add_plugin_dir(const char *arg, struct lightningd *ld) diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 3460c89a6..b465afd0a 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -52,6 +52,7 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book, p->ld = ld; p->startup = true; p->json_cmds = tal_arr(p, struct command *, 0); + p->blacklist = tal_arr(p, const char *, 0); uintmap_init(&p->pending_requests); memleak_add_helper(p, memleak_help_pending_requests); @@ -175,19 +176,30 @@ bool plugin_paths_match(const char *cmd, const char *name) } } -bool plugin_remove(struct plugins *plugins, const char *name) +void plugin_blacklist(struct plugins *plugins, const char *name) { struct plugin *p, *next; - bool removed = false; list_for_each_safe(&plugins->plugins, p, next, list) { if (plugin_paths_match(p->cmd, name)) { + log_info(plugins->log, "%s: disabled via disable-plugin", + p->cmd); list_del_from(&plugins->plugins, &p->list); tal_free(p); - removed = true; } } - return removed; + + tal_arr_expand(&plugins->blacklist, + tal_strdup(plugins->blacklist, name)); +} + +bool plugin_blacklisted(struct plugins *plugins, const char *name) +{ + for (size_t i = 0; i < tal_count(plugins->blacklist); i++) + if (plugin_paths_match(name, plugins->blacklist[i])) + return true; + + return false; } void plugin_kill(struct plugin *plugin, const char *msg) @@ -1179,7 +1191,12 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, bool error_ok) if (streq(di->d_name, ".") || streq(di->d_name, "..")) continue; fullpath = plugin_fullpath(tmpctx, dir, di->d_name); - if (fullpath) { + if (!fullpath) + continue; + if (plugin_blacklisted(plugins, fullpath)) { + log_info(plugins->log, "%s: disabled via disable-plugin", + fullpath); + } else { p = plugin_register(plugins, fullpath, NULL); if (!p && !error_ok) return tal_fmt(NULL, "Failed to register %s: %s", diff --git a/lightningd/plugin.h b/lightningd/plugin.h index 4460324d6..63062bbf1 100644 --- a/lightningd/plugin.h +++ b/lightningd/plugin.h @@ -100,6 +100,9 @@ struct plugins { /* If there are json commands waiting for plugin resolutions. */ struct command **json_cmds; + + /* Blacklist of plugins from --disable-plugin */ + const char **blacklist; }; /* The value of a plugin option, which can have different types. @@ -191,10 +194,18 @@ bool plugin_paths_match(const char *cmd, const char *name); * @param plugins: Plugin context * @param arg: The basename or fullname of the executable for this plugin */ -bool plugin_remove(struct plugins *plugins, const char *name); +void plugin_blacklist(struct plugins *plugins, const char *name); + +/** + * Is a plugin disabled?. + * + * @param plugins: Plugin context + * @param arg: The basename or fullname of the executable for this plugin + */ +bool plugin_blacklisted(struct plugins *plugins, const char *name); /** - * Kick of initialization of a plugin. + * Kick off initialization of a plugin. * * Returns error string, or NULL. */ diff --git a/tests/test_plugin.py b/tests/test_plugin.py index f2359f7d0..a79674a6e 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -273,13 +273,14 @@ def test_plugin_command(node_factory): def test_plugin_disable(node_factory): """--disable-plugin works""" plugin_dir = os.path.join(os.getcwd(), 'contrib/plugins') - # We need plugin-dir before disable-plugin! + # We used to need plugin-dir before disable-plugin! n = node_factory.get_node(options=OrderedDict([('plugin-dir', plugin_dir), ('disable-plugin', '{}/helloworld.py' .format(plugin_dir))])) with pytest.raises(RpcError): n.rpc.hello(name='Sun') + assert n.daemon.is_in_log('helloworld.py: disabled via disable-plugin') # Also works by basename. n = node_factory.get_node(options=OrderedDict([('plugin-dir', plugin_dir), @@ -287,6 +288,39 @@ def test_plugin_disable(node_factory): 'helloworld.py')])) with pytest.raises(RpcError): n.rpc.hello(name='Sun') + assert n.daemon.is_in_log('helloworld.py: disabled via disable-plugin') + + # Other order also works! + n = node_factory.get_node(options=OrderedDict([('disable-plugin', + 'helloworld.py'), + ('plugin-dir', plugin_dir)])) + with pytest.raises(RpcError): + n.rpc.hello(name='Sun') + assert n.daemon.is_in_log('helloworld.py: disabled via disable-plugin') + + # Both orders of explicit specification work. + n = node_factory.get_node(options=OrderedDict([('disable-plugin', + 'helloworld.py'), + ('plugin', + '{}/helloworld.py' + .format(plugin_dir))])) + with pytest.raises(RpcError): + n.rpc.hello(name='Sun') + assert n.daemon.is_in_log('helloworld.py: disabled via disable-plugin') + + # Both orders of explicit specification work. + n = node_factory.get_node(options=OrderedDict([('plugin', + '{}/helloworld.py' + .format(plugin_dir)), + ('disable-plugin', + 'helloworld.py')])) + with pytest.raises(RpcError): + n.rpc.hello(name='Sun') + assert n.daemon.is_in_log('helloworld.py: disabled via disable-plugin') + + # Still disabled if we load directory. + n.rpc.plugin_startdir(directory=os.path.join(os.getcwd(), "contrib/plugins")) + n.daemon.wait_for_log('helloworld.py: disabled via disable-plugin') def test_plugin_hook(node_factory, executor):