From 6db323164e5373a3d5bfb35a41c2cb3005aed241 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 14 Jun 2016 20:01:34 -0700 Subject: [PATCH 1/2] Plugins: Emit nicer error message when trying to install unknown plugin When installing plugins, we first try the elastic download service for official plugins, then try maven coordinates, and finally try the argument as a url. This can lead to confusing error messages about unknown protocols when eg an official plugin name is mispelled. This change adds a heuristic for determining if the argument in the final case is in fact a url that we should try, and gives a simplified error message in the case it is definitely not a url. closes #17226 --- .../org/elasticsearch/plugins/InstallPluginCommand.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 9e17eec2e93..75ab95414a2 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -134,6 +134,9 @@ class InstallPluginCommand extends SettingCommand { } } + // protocols allowed for direct url installation + private static final List URL_PROTOCOLS = Arrays.asList("http", "https", "file"); + private final OptionSpec batchOption; private final OptionSpec arguments; @@ -237,6 +240,10 @@ class InstallPluginCommand extends SettingCommand { } // fall back to plain old URL + if (pluginId.contains("://") == false) { + // definitely not a valid url, so assume it is a plugin name + throw new UserError(ExitCodes.USAGE, "Unknown plugin " + pluginId); + } terminal.println("-> Downloading " + URLDecoder.decode(pluginId, "UTF-8")); return downloadZip(terminal, pluginId, tmpDir); } From 1ecf14cee052eb30e46003c37bf411fd63ef890e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 14 Jun 2016 23:01:37 -0700 Subject: [PATCH 2/2] Add test for plugin install heuristic --- .../org/elasticsearch/plugins/InstallPluginCommand.java | 3 --- .../elasticsearch/plugins/InstallPluginCommandTests.java | 6 ++++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index 75ab95414a2..aa224b3c7c3 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -134,9 +134,6 @@ class InstallPluginCommand extends SettingCommand { } } - // protocols allowed for direct url installation - private static final List URL_PROTOCOLS = Arrays.asList("http", "https", "file"); - private final OptionSpec batchOption; private final OptionSpec arguments; diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java index 5c766018f13..fdf52da2634 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java @@ -307,6 +307,12 @@ public class InstallPluginCommandTests extends ESTestCase { assertTrue(e.getMessage(), e.getMessage().contains("no protocol")); } + public void testUnknownPlugin() throws Exception { + Tuple env = createEnv(fs, temp); + UserError e = expectThrows(UserError.class, () -> installPlugin("foo", env.v1())); + assertTrue(e.getMessage(), e.getMessage().contains("Unknown plugin foo")); + } + public void testPluginsDirMissing() throws Exception { Tuple env = createEnv(fs, temp); Files.delete(env.v2().pluginsFile());