Fix handling of mandatory meta plugins

This commit fixes an issue with setting plugin.mandatory to include a
meta-plugin. The issue here is that the names that we collect are the
underlying plugins, not the meta-plugin. We should not use the
underlying plugins instead using the names of non-meta plugins and the
names of meta-plugins. This commit addresses this. The strategy here is
that when we look at the installed plugins on the filesystem, we keep
track of which ones are meta-plugins and carry this information up to
where check which plugins are installed against the mandatory plugins.

Relates #28710

server/src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -56,8 +56,8 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -65,6 +65,7 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
 
@@ -102,6 +103,8 @@ public class PluginsService extends AbstractComponent {
 
         List<Tuple<PluginInfo, Plugin>> pluginsLoaded = new ArrayList<>();
         List<PluginInfo> pluginsList = new ArrayList<>();
+        // we need to build a List of plugins for checking mandatory plugins
+        final List<String> pluginsNames = new ArrayList<>();
         // first we load plugins that are on the classpath. this is for tests and transport clients
         for (Class<? extends Plugin> pluginClass : classpathPlugins) {
             Plugin plugin = loadPlugin(pluginClass, settings, configPath);
@@ -112,6 +115,7 @@ public class PluginsService extends AbstractComponent {
             }
             pluginsLoaded.add(new Tuple<>(pluginInfo, plugin));
             pluginsList.add(pluginInfo);
+            pluginsNames.add(pluginInfo.getName());
         }
 
         Set<Bundle> seenBundles = new LinkedHashSet<>();
@@ -135,11 +139,15 @@ public class PluginsService extends AbstractComponent {
                 // TODO: remove this leniency, but tests bogusly rely on it
                 if (isAccessibleDirectory(pluginsDirectory, logger)) {
                     checkForFailedPluginRemovals(pluginsDirectory);
-                    Set<Bundle> plugins = getPluginBundles(pluginsDirectory);
+                    List<BundleCollection> plugins = getPluginBundleCollections(pluginsDirectory);
-                    for (Bundle bundle : plugins) {
+                    for (final BundleCollection plugin : plugins) {
-                        pluginsList.add(bundle.plugin);
+                        final Collection<Bundle> bundles = plugin.bundles();
+                        for (final Bundle bundle : bundles) {
+                            pluginsList.add(bundle.plugin);
+                        }
+                        seenBundles.addAll(bundles);
+                        pluginsNames.add(plugin.name());
                     }
-                    seenBundles.addAll(plugins);
                 }
             } catch (IOException ex) {
                 throw new IllegalStateException("Unable to initialize plugins", ex);
@@ -152,12 +160,6 @@ public class PluginsService extends AbstractComponent {
         this.info = new PluginsAndModules(pluginsList, modulesList);
         this.plugins = Collections.unmodifiableList(pluginsLoaded);
 
-        // We need to build a List of plugins for checking mandatory plugins
-        Set<String> pluginsNames = new HashSet<>();
-        for (Tuple<PluginInfo, Plugin> tuple : this.plugins) {
-            pluginsNames.add(tuple.v1().getName());
-        }
-
         // Checking expected plugins
         List<String> mandatoryPlugins = MANDATORY_SETTING.get(settings);
         if (mandatoryPlugins.isEmpty() == false) {
@@ -168,7 +170,11 @@ public class PluginsService extends AbstractComponent {
                 }
             }
             if (!missingPlugins.isEmpty()) {
-                throw new ElasticsearchException("Missing mandatory plugins [" + Strings.collectionToDelimitedString(missingPlugins, ", ") + "]");
+                final String message = String.format(
+                        Locale.ROOT,
+                        "missing mandatory plugins [%s]",
+                        Strings.collectionToDelimitedString(missingPlugins, ", "));
+                throw new IllegalStateException(message);
             }
         }
 
@@ -244,9 +250,17 @@ public class PluginsService extends AbstractComponent {
         return info;
     }
 
+    /**
+     * An abstraction over a single plugin and meta-plugins.
+     */
+    interface BundleCollection {
+        String name();
+        Collection<Bundle> bundles();
+    }
+
     // a "bundle" is a group of plugins in a single classloader
     // really should be 1-1, but we are not so fortunate
-    static class Bundle {
+    static class Bundle implements BundleCollection {
         final PluginInfo plugin;
         final Set<URL> urls;
 
@@ -266,6 +280,16 @@ public class PluginsService extends AbstractComponent {
             this.urls = Objects.requireNonNull(urls);
         }
 
+        @Override
+        public String name() {
+            return plugin.getName();
+        }
+
+        @Override
+        public Collection<Bundle> bundles() {
+            return Collections.singletonList(this);
+        }
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
@@ -281,35 +305,78 @@ public class PluginsService extends AbstractComponent {
     }
 
     /**
-     * Extracts all installed plugin directories from the provided {@code rootPath} expanding meta plugins if needed.
+     * Represents a meta-plugin and the {@link Bundle}s corresponding to its constituents.
+     */
+    static class MetaBundle implements BundleCollection {
+        private final String name;
+        private final List<Bundle> bundles;
+
+        MetaBundle(final String name, final List<Bundle> bundles) {
+            this.name = name;
+            this.bundles = bundles;
+        }
+
+        @Override
+        public String name() {
+            return name;
+        }
+
+        @Override
+        public Collection<Bundle> bundles() {
+            return bundles;
+        }
+        
+    }
+
+    /**
+     * Extracts all installed plugin directories from the provided {@code rootPath} expanding meta-plugins if needed.
+     *
      * @param rootPath the path where the plugins are installed
-     * @return A list of all plugin paths installed in the {@code rootPath}
+     * @return a list of all plugin paths installed in the {@code rootPath}
      * @throws IOException if an I/O exception occurred reading the directories
      */
     public static List<Path> findPluginDirs(final Path rootPath) throws IOException {
+        final Tuple<List<Path>, Map<String, List<Path>>> groupedPluginDirs = findGroupedPluginDirs(rootPath);
+        return Stream.concat(
+                groupedPluginDirs.v1().stream(),
+                groupedPluginDirs.v2().values().stream().flatMap(Collection::stream))
+                .collect(Collectors.toList());
+    }
+
+    /**
+     * Extracts all installed plugin directories from the provided {@code rootPath} expanding meta-plugins if needed. The plugins are
+     * grouped into plugins and meta-plugins. The meta-plugins are keyed by the meta-plugin name.
+     *
+     * @param rootPath the path where the plugins are installed
+     * @return a tuple of plugins as the first component and meta-plugins keyed by meta-plugin name as the second component
+     * @throws IOException if an I/O exception occurred reading the directories
+     */
+    private static Tuple<List<Path>, Map<String, List<Path>>> findGroupedPluginDirs(final Path rootPath) throws IOException {
         final List<Path> plugins = new ArrayList<>();
+        final Map<String, List<Path>> metaPlugins = new LinkedHashMap<>();
         final Set<String> seen = new HashSet<>();
         if (Files.exists(rootPath)) {
             try (DirectoryStream<Path> stream = Files.newDirectoryStream(rootPath)) {
                 for (Path plugin : stream) {
                     if (FileSystemUtils.isDesktopServicesStore(plugin) ||
-                        plugin.getFileName().toString().startsWith(".removing-")) {
+                            plugin.getFileName().toString().startsWith(".removing-")) {
                         continue;
                     }
                     if (seen.add(plugin.getFileName().toString()) == false) {
                         throw new IllegalStateException("duplicate plugin: " + plugin);
                     }
                     if (MetaPluginInfo.isMetaPlugin(plugin)) {
+                        final String name = plugin.getFileName().toString();
                         try (DirectoryStream<Path> subStream = Files.newDirectoryStream(plugin)) {
                             for (Path subPlugin : subStream) {
                                 if (MetaPluginInfo.isPropertiesFile(subPlugin) ||
-                                    FileSystemUtils.isDesktopServicesStore(subPlugin)) {
+                                        FileSystemUtils.isDesktopServicesStore(subPlugin)) {
                                     continue;
                                 }
                                 if (seen.add(subPlugin.getFileName().toString()) == false) {
                                     throw new IllegalStateException("duplicate plugin: " + subPlugin);
                                 }
-                                plugins.add(subPlugin);
+                                metaPlugins.computeIfAbsent(name, n -> new ArrayList<>()).add(subPlugin);
                             }
                         }
                     } else {
@@ -318,7 +385,7 @@ public class PluginsService extends AbstractComponent {
                 }
             }
         }
-        return plugins;
+        return Tuple.tuple(plugins, metaPlugins);
     }
 
     /**
@@ -380,26 +447,46 @@ public class PluginsService extends AbstractComponent {
      * @throws IOException if an I/O exception occurs reading the plugin bundles
      */
     static Set<Bundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
-        Logger logger = Loggers.getLogger(PluginsService.class);
+        return getPluginBundleCollections(pluginsDirectory).stream().flatMap(b -> b.bundles().stream()).collect(Collectors.toSet());
-        Set<Bundle> bundles = new LinkedHashSet<>();
+    }
 
-        List<Path> infos = findPluginDirs(pluginsDirectory);
+    private static List<BundleCollection> getPluginBundleCollections(final Path pluginsDirectory) throws IOException {
-        for (Path plugin : infos) {
+        final List<BundleCollection> bundles = new ArrayList<>();
-            logger.trace("--- adding plugin [{}]", plugin.toAbsolutePath());
+        final Set<Bundle> seenBundles = new HashSet<>();
-            final PluginInfo info;
+        final Tuple<List<Path>, Map<String, List<Path>>> groupedPluginDirs = findGroupedPluginDirs(pluginsDirectory);
-            try {
+        for (final Path plugin : groupedPluginDirs.v1()) {
-                info = PluginInfo.readFromProperties(plugin);
+            final Bundle bundle = bundle(seenBundles, plugin);
-            } catch (IOException e) {
+            bundles.add(bundle);
-                throw new IllegalStateException("Could not load plugin descriptor for existing plugin ["
-                        + plugin.getFileName() + "]. Was the plugin built before 2.0?", e);
-            }
-            if (bundles.add(new Bundle(info, plugin)) == false) {
-                throw new IllegalStateException("duplicate plugin: " + info);
-            }
         }
+        for (final Map.Entry<String, List<Path>> metaPlugin : groupedPluginDirs.v2().entrySet()) {
+            final List<Bundle> metaPluginBundles = new ArrayList<>();
+            for (final Path metaPluginPlugin : metaPlugin.getValue()) {
+                final Bundle bundle = bundle(seenBundles, metaPluginPlugin);
+                metaPluginBundles.add(bundle);
+            }
+            final MetaBundle metaBundle = new MetaBundle(metaPlugin.getKey(), metaPluginBundles);
+            bundles.add(metaBundle);
+        }
+
         return bundles;
     }
 
+    private static Bundle bundle(final Set<Bundle> bundles, final Path plugin) throws IOException {
+        Loggers.getLogger(PluginsService.class).trace("--- adding plugin [{}]", 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);
+        }
+        final Bundle bundle = new Bundle(info, plugin);
+        if (bundles.add(bundle) == false) {
+            throw new IllegalStateException("duplicate plugin: " + info);
+        }
+        return bundle;
+    }
+
     /**
      * Return the given bundles, sorted in dependency loading order.
      *

server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java

@@ -604,4 +604,123 @@ public class PluginsServiceTests extends ESTestCase {
         IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsService.verifyCompatibility(info));
         assertThat(e.getMessage(), containsString("my_plugin requires Java"));
     }
+
+    public void testFindPluginDirs() throws IOException {
+        final Path plugins = createTempDir();
+
+        final Path fake = plugins.resolve("fake");
+
+        PluginTestUtil.writePluginProperties(
+                fake,
+                "description", "description",
+                "name", "fake",
+                "version", "1.0.0",
+                "elasticsearch.version", Version.CURRENT.toString(),
+                "java.version", System.getProperty("java.specification.version"),
+                "classname", "test.DummyPlugin");
+
+        try (InputStream jar = PluginsServiceTests.class.getResourceAsStream("dummy-plugin.jar")) {
+            Files.copy(jar, fake.resolve("plugin.jar"));
+        }
+
+        final Path fakeMeta = plugins.resolve("fake-meta");
+
+        PluginTestUtil.writeMetaPluginProperties(fakeMeta, "description", "description", "name", "fake-meta");
+
+        final Path fakeMetaCore = fakeMeta.resolve("fake-meta-core");
+        PluginTestUtil.writePluginProperties(
+                fakeMetaCore,
+                "description", "description",
+                "name", "fake-meta-core",
+                "version", "1.0.0",
+                "elasticsearch.version", Version.CURRENT.toString(),
+                "java.version", System.getProperty("java.specification.version"),
+                "classname", "test.DummyPlugin");
+        try (InputStream jar = PluginsServiceTests.class.getResourceAsStream("dummy-plugin.jar")) {
+            Files.copy(jar, fakeMetaCore.resolve("plugin.jar"));
+        }
+
+        assertThat(PluginsService.findPluginDirs(plugins), containsInAnyOrder(fake, fakeMetaCore));
+    }
+
+    public void testMissingMandatoryPlugin() {
+        final Settings settings =
+                Settings.builder()
+                        .put("path.home", createTempDir())
+                        .put("plugin.mandatory", "fake")
+                        .build();
+        final IllegalStateException e = expectThrows(IllegalStateException.class, () -> newPluginsService(settings));
+        assertThat(e, hasToString(containsString("missing mandatory plugins [fake]")));
+    }
+
+    public void testExistingMandatoryClasspathPlugin() {
+        final Settings settings =
+                Settings.builder()
+                        .put("path.home", createTempDir())
+                        .put("plugin.mandatory", "org.elasticsearch.plugins.PluginsServiceTests$FakePlugin")
+                        .build();
+        newPluginsService(settings, FakePlugin.class);
+    }
+
+    public static class FakePlugin extends Plugin {
+
+        public FakePlugin() {
+
+        }
+
+    }
+
+    public void testExistingMandatoryInstalledPlugin() throws IOException {
+        final Path pathHome = createTempDir();
+        final Path plugins = pathHome.resolve("plugins");
+        final Path fake = plugins.resolve("fake");
+
+        PluginTestUtil.writePluginProperties(
+                fake,
+                "description", "description",
+                "name", "fake",
+                "version", "1.0.0",
+                "elasticsearch.version", Version.CURRENT.toString(),
+                "java.version", System.getProperty("java.specification.version"),
+                "classname", "test.DummyPlugin");
+        try (InputStream jar = PluginsServiceTests.class.getResourceAsStream("dummy-plugin.jar")) {
+            Files.copy(jar, fake.resolve("plugin.jar"));
+        }
+
+        final Settings settings =
+                Settings.builder()
+                        .put("path.home", pathHome)
+                        .put("plugin.mandatory", "fake")
+                        .build();
+        newPluginsService(settings);
+    }
+
+    public void testExistingMandatoryMetaPlugin() throws IOException {
+        final Path pathHome = createTempDir();
+        final Path plugins = pathHome.resolve("plugins");
+        final Path fakeMeta = plugins.resolve("fake-meta");
+
+        PluginTestUtil.writeMetaPluginProperties(fakeMeta, "description", "description", "name", "fake-meta");
+
+        final Path fake = fakeMeta.resolve("fake");
+        PluginTestUtil.writePluginProperties(
+                fake,
+                "description", "description",
+                "name", "fake",
+                "version", "1.0.0",
+                "elasticsearch.version", Version.CURRENT.toString(),
+                "java.version", System.getProperty("java.specification.version"),
+                "classname", "test.DummyPlugin");
+        try (InputStream jar = PluginsServiceTests.class.getResourceAsStream("dummy-plugin.jar")) {
+            Files.copy(jar, fake.resolve("plugin.jar"));
+        }
+
+        final Settings settings =
+                Settings.builder()
+                        .put("path.home", pathHome)
+                        .put("plugin.mandatory", "fake-meta")
+                        .build();
+        newPluginsService(settings);
+    }
+
 }