diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index 3ed4374ca2ac3..766d171752c16 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -140,7 +140,8 @@ public PluginsService(Settings settings, Path configPath, Path modulesDirectory, // TODO: remove this leniency, but tests bogusly rely on it if (isAccessibleDirectory(pluginsDirectory, logger)) { checkForFailedPluginRemovals(pluginsDirectory); - List plugins = getPluginBundleCollections(pluginsDirectory); + // call findBundles directly to get the meta plugin names + List plugins = findBundles(pluginsDirectory, "plugin"); for (final BundleCollection plugin : plugins) { final Collection bundles = plugin.bundles(); for (final Bundle bundle : bundles) { @@ -173,8 +174,9 @@ public PluginsService(Settings settings, Path configPath, Path modulesDirectory, if (!missingPlugins.isEmpty()) { final String message = String.format( Locale.ROOT, - "missing mandatory plugins [%s]", - Strings.collectionToDelimitedString(missingPlugins, ", ")); + "missing mandatory plugins [%s], found plugins [%s]", + Strings.collectionToDelimitedString(missingPlugins, ", "), + Strings.collectionToDelimitedString(pluginsNames, ", ")); throw new IllegalStateException(message); } } @@ -400,25 +402,6 @@ static void verifyCompatibility(PluginInfo info) { JarHell.checkJavaVersion(info.getName(), info.getJavaVersion()); } - // similar in impl to getPluginBundles, but DO NOT try to make them share code. - // we don't need to inherit all the leniency, and things are different enough. - static Set getModuleBundles(Path modulesDirectory) throws IOException { - // damn leniency - if (Files.notExists(modulesDirectory)) { - return Collections.emptySet(); - } - Set bundles = new LinkedHashSet<>(); - try (DirectoryStream stream = Files.newDirectoryStream(modulesDirectory)) { - for (Path module : stream) { - PluginInfo info = PluginInfo.readFromProperties(module); - if (bundles.add(new Bundle(info, module)) == false) { - throw new IllegalStateException("duplicate module: " + info); - } - } - } - return bundles; - } - static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOException { /* * Check for the existence of a marker file that indicates any plugins are in a garbage state from a failed attempt to remove the @@ -440,29 +423,29 @@ static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOE } } - /** - * Get the plugin bundles from the specified directory. - * - * @param pluginsDirectory the directory - * @return the set of plugin bundles in the specified directory - * @throws IOException if an I/O exception occurs reading the plugin bundles - */ + /** Get bundles for plugins installed in the given modules directory. */ + static Set getModuleBundles(Path modulesDirectory) throws IOException { + return findBundles(modulesDirectory, "module").stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet()); + } + + /** Get bundles for plugins installed in the given plugins directory. */ static Set getPluginBundles(final Path pluginsDirectory) throws IOException { - return getPluginBundleCollections(pluginsDirectory).stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet()); + return findBundles(pluginsDirectory, "plugin").stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet()); } - private static List getPluginBundleCollections(final Path pluginsDirectory) throws IOException { + // searches subdirectories under the given directory for plugin directories + private static List findBundles(final Path directory, String type) throws IOException { final List bundles = new ArrayList<>(); final Set seenBundles = new HashSet<>(); - final Tuple, Map>> groupedPluginDirs = findGroupedPluginDirs(pluginsDirectory); + final Tuple, Map>> groupedPluginDirs = findGroupedPluginDirs(directory); for (final Path plugin : groupedPluginDirs.v1()) { - final Bundle bundle = bundle(seenBundles, plugin); + final Bundle bundle = readPluginBundle(seenBundles, plugin, type); bundles.add(bundle); } for (final Map.Entry> metaPlugin : groupedPluginDirs.v2().entrySet()) { final List metaPluginBundles = new ArrayList<>(); for (final Path metaPluginPlugin : metaPlugin.getValue()) { - final Bundle bundle = bundle(seenBundles, metaPluginPlugin); + final Bundle bundle = readPluginBundle(seenBundles, metaPluginPlugin, type); metaPluginBundles.add(bundle); } final MetaBundle metaBundle = new MetaBundle(metaPlugin.getKey(), metaPluginBundles); @@ -472,18 +455,19 @@ private static List getPluginBundleCollections(final Path plug return bundles; } - private static Bundle bundle(final Set bundles, final Path plugin) throws IOException { - Loggers.getLogger(PluginsService.class).trace("--- adding plugin [{}]", plugin.toAbsolutePath()); + // get a bundle for a single plugin dir + private static Bundle readPluginBundle(final Set bundles, final Path plugin, String type) throws IOException { + Loggers.getLogger(PluginsService.class).trace("--- adding [{}] [{}]", type, plugin.toAbsolutePath()); final PluginInfo info; try { info = PluginInfo.readFromProperties(plugin); } catch (final IOException e) { - throw new IllegalStateException("Could not load plugin descriptor for existing plugin [" - + plugin.getFileName() + "]. Was the plugin built before 2.0?", e); + throw new IllegalStateException("Could not load plugin descriptor for " + type + + " directory [" + plugin.getFileName() + "]", e); } final Bundle bundle = new Bundle(info, plugin); if (bundles.add(bundle) == false) { - throw new IllegalStateException("duplicate plugin: " + info); + throw new IllegalStateException("duplicate " + type + ": " + info); } return bundle; } diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 36e1266c51118..4f0a73ca44ca6 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -107,12 +107,9 @@ public void testAdditionalSettingsClash() { public void testExistingPluginMissingDescriptor() throws Exception { Path pluginsDir = createTempDir(); Files.createDirectory(pluginsDir.resolve("plugin-missing-descriptor")); - try { - PluginsService.getPluginBundles(pluginsDir); - fail(); - } catch (IllegalStateException e) { - assertTrue(e.getMessage(), e.getMessage().contains("Could not load plugin descriptor for existing plugin [plugin-missing-descriptor]")); - } + IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsService.getPluginBundles(pluginsDir)); + assertThat(e.getMessage(), + containsString("Could not load plugin descriptor for plugin directory [plugin-missing-descriptor]")); } public void testFilterPlugins() { @@ -139,7 +136,7 @@ public void testHiddenFiles() throws IOException { IllegalStateException.class, () -> newPluginsService(settings)); - final String expected = "Could not load plugin descriptor for existing plugin [.hidden]"; + final String expected = "Could not load plugin descriptor for plugin directory [.hidden]"; assertThat(e, hasToString(containsString(expected))); } @@ -158,7 +155,7 @@ public void testDesktopServicesStoreFiles() throws IOException { assertNotNull(pluginsService); } else { final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings)); - assertThat(e, hasToString(containsString("Could not load plugin descriptor for existing plugin [.DS_Store]"))); + assertThat(e.getMessage(), containsString("Could not load plugin descriptor for plugin directory [.DS_Store]")); assertNotNull(e.getCause()); assertThat(e.getCause(), instanceOf(FileSystemException.class)); if (Constants.WINDOWS) {