Enforce isolated mode for all plugins

This commit removes the isolated option, each plugin have its own classloader.

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesExtension.groovy

@@ -39,9 +39,6 @@ class PluginPropertiesExtension {
     @Input
     String classname
 
-    @Input
-    boolean isolated = true
-
     PluginPropertiesExtension(Project project) {
         name = project.name
         version = project.version

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginPropertiesTask.groovy

@@ -54,12 +54,6 @@ class PluginPropertiesTask extends Copy {
             if (extension.classname == null) {
                 throw new InvalidUserDataException('classname is a required setting for esplugin')
             }
-            doFirst {
-                if (extension.isolated == false) {
-                    String warning = "WARNING: Disabling plugin isolation in ${project.path} is deprecated and will be removed in the future"
-                    logger.warn("${'=' * warning.length()}\n${warning}\n${'=' * warning.length()}")
-                }
-            }
             // configure property substitution
             from(templateFile)
             into(generatedResourcesDir)
@@ -80,7 +74,6 @@ class PluginPropertiesTask extends Copy {
             'version': stringSnap(extension.version),
             'elasticsearchVersion': stringSnap(VersionProperties.elasticsearch),
             'javaVersion': project.targetCompatibility as String,
-            'isolated': extension.isolated as String,
             'classname': extension.classname
         ]
     }

buildSrc/src/main/resources/plugin-descriptor.properties

@@ -38,12 +38,3 @@ java.version=${javaVersion}
 #
 # 'elasticsearch.version' version of elasticsearch compiled against
 elasticsearch.version=${elasticsearchVersion}
-#
-### deprecated elements for jvm plugins :
-#
-# 'isolated': true if the plugin should have its own classloader.
-# passing false is deprecated, and only intended to support plugins
-# that have hard dependencies against each other. If this is
-# not specified, then the plugin is isolated by default.
-isolated=${isolated}
-#

core/src/main/java/org/elasticsearch/plugins/DummyPluginInfo.java

@@ -20,9 +20,9 @@
 
 public class DummyPluginInfo extends PluginInfo {
 
-    private DummyPluginInfo(String name, String description, String version, String classname, boolean isolated) {
-        super(name, description, version, classname, isolated);
+    private DummyPluginInfo(String name, String description, String version, String classname) {
+        super(name, description, version, classname);
     }
 
-    public static final DummyPluginInfo INSTANCE = new DummyPluginInfo("dummy_plugin_name", "dummy plugin description", "dummy_plugin_version", "DummyPluginName", true);
+    public static final DummyPluginInfo INSTANCE = new DummyPluginInfo("dummy_plugin_name", "dummy plugin description", "dummy_plugin_version", "DummyPluginName");
 }

core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

@@ -342,7 +342,7 @@ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) t
         }
 
         // check for jar hell before any copying
-        jarHellCheck(pluginRoot, env.pluginsFile(), info.isIsolated());
+        jarHellCheck(pluginRoot, env.pluginsFile());
 
         // read optional security policy (extra permissions)
         // if it exists, confirm or warn the user
@@ -355,19 +355,13 @@ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch) t
     }
 
     /** check a candidate plugin for jar hell before installing it */
-    void jarHellCheck(Path candidate, Path pluginsDir, boolean isolated) throws Exception {
+    void jarHellCheck(Path candidate, Path pluginsDir) throws Exception {
         // create list of current jars in classpath
         final List<URL> jars = new ArrayList<>();
         jars.addAll(Arrays.asList(JarHell.parseClassPath()));
 
         // read existing bundles. this does some checks on the installation too.
-        List<PluginsService.Bundle> bundles = PluginsService.getPluginBundles(pluginsDir);
-
-        // if we aren't isolated, we need to jarhellcheck against any other non-isolated plugins
-        // that's always the first bundle
-        if (isolated == false) {
-            jars.addAll(bundles.get(0).urls);
-        }
+        PluginsService.getPluginBundles(pluginsDir);
 
         // add plugin jars to the list
         Path pluginJars[] = FileSystemUtils.files(candidate, "*.jar");

core/src/main/java/org/elasticsearch/plugins/PluginInfo.java

@@ -44,14 +44,12 @@ static final class Fields {
         static final XContentBuilderString URL = new XContentBuilderString("url");
         static final XContentBuilderString VERSION = new XContentBuilderString("version");
         static final XContentBuilderString CLASSNAME = new XContentBuilderString("classname");
-        static final XContentBuilderString ISOLATED = new XContentBuilderString("isolated");
     }
 
     private String name;
     private String description;
     private String version;
     private String classname;
-    private boolean isolated;
 
     public PluginInfo() {
     }
@@ -63,12 +61,11 @@ public PluginInfo() {
      * @param description Its description
      * @param version     Version number
      */
-    PluginInfo(String name, String description, String version, String classname, boolean isolated) {
+    PluginInfo(String name, String description, String version, String classname) {
         this.name = name;
         this.description = description;
         this.version = version;
         this.classname = classname;
-        this.isolated = isolated;
     }
 
     /** reads (and validates) plugin metadata descriptor file */
@@ -106,13 +103,12 @@ public static PluginInfo readFromProperties(Path dir) throws IOException {
         }
         JarHell.checkVersionFormat(javaVersionString);
         JarHell.checkJavaVersion(name, javaVersionString);
-        boolean isolated = Boolean.parseBoolean(props.getProperty("isolated", "true"));
         String classname = props.getProperty("classname");
         if (classname == null) {
             throw new IllegalArgumentException("Property [classname] is missing for plugin [" + name + "]");
         }
 
-        return new PluginInfo(name, description, version, classname, isolated);
+        return new PluginInfo(name, description, version, classname);
     }
 
     /**
@@ -129,13 +125,6 @@ public String getDescription() {
         return description;
     }
 
-    /**
-     * @return true if plugin has isolated classloader
-     */
-    public boolean isIsolated() {
-        return isolated;
-    }
-
     /**
      * @return plugin's classname
      */
@@ -162,7 +151,6 @@ public void readFrom(StreamInput in) throws IOException {
         this.description = in.readString();
         this.version = in.readString();
         this.classname = in.readString();
-        this.isolated = in.readBoolean();
     }
 
     @Override
@@ -171,7 +159,6 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeString(description);
         out.writeString(version);
         out.writeString(classname);
-        out.writeBoolean(isolated);
     }
 
     @Override
@@ -181,7 +168,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         builder.field(Fields.VERSION, version);
         builder.field(Fields.DESCRIPTION, description);
         builder.field(Fields.CLASSNAME, classname);
-        builder.field(Fields.ISOLATED, isolated);
         builder.endObject();
 
         return builder;
@@ -212,8 +198,7 @@ public String toString() {
                 .append("Name: ").append(name).append("\n")
                 .append("Description: ").append(description).append("\n")
                 .append("Version: ").append(version).append("\n")
-                .append(" * Classname: ").append(classname).append("\n")
-                .append(" * Isolated: ").append(isolated);
+                .append(" * Classname: ").append(classname);
 
         return information.toString();
     }

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

@@ -103,7 +103,7 @@ public PluginsService(Settings settings, Path modulesDirectory, Path pluginsDire
         // 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);
-            PluginInfo pluginInfo = new PluginInfo(plugin.name(), plugin.description(), "NA", pluginClass.getName(), false);
+            PluginInfo pluginInfo = new PluginInfo(plugin.name(), plugin.description(), "NA", pluginClass.getName());
             if (logger.isTraceEnabled()) {
                 logger.trace("plugin loaded from classpath [{}]", pluginInfo);
             }
@@ -302,9 +302,6 @@ static List<Bundle> getModuleBundles(Path modulesDirectory) throws IOException {
                     continue; // skip over .DS_Store etc
                 }
                 PluginInfo info = PluginInfo.readFromProperties(module);
-                if (!info.isIsolated()) {
-                    throw new IllegalStateException("modules must be isolated: " + info);
-                }
                 Bundle bundle = new Bundle();
                 bundle.plugins.add(info);
                 // gather urls for jar files
@@ -329,8 +326,6 @@ static List<Bundle> getPluginBundles(Path pluginsDirectory) throws IOException {
         }
 
         List<Bundle> bundles = new ArrayList<>();
-        // a special purgatory for plugins that directly depend on each other
-        bundles.add(new Bundle());
 
         try (DirectoryStream<Path> stream = Files.newDirectoryStream(pluginsDirectory)) {
             for (Path plugin : stream) {
@@ -354,13 +349,8 @@ static List<Bundle> getPluginBundles(Path pluginsDirectory) throws IOException {
                         urls.add(jar.toRealPath().toUri().toURL());
                     }
                 }
-                final Bundle bundle;
-                if (info.isIsolated() == false) {
-                    bundle = bundles.get(0); // purgatory
-                } else {
-                    bundle = new Bundle();
-                    bundles.add(bundle);
-                }
+                final Bundle bundle = new Bundle();
+                bundles.add(bundle);
                 bundle.plugins.add(info);
                 bundle.urls.addAll(urls);
             }

core/src/test/java/org/elasticsearch/plugins/PluginInfoTests.java

@@ -46,7 +46,6 @@ public void testReadFromProperties() throws Exception {
         assertEquals("fake desc", info.getDescription());
         assertEquals("1.0", info.getVersion());
         assertEquals("FakePlugin", info.getClassname());
-        assertTrue(info.isIsolated());
     }
 
     public void testReadFromPropertiesNameMissing() throws Exception {
@@ -203,11 +202,11 @@ public void testReadFromPropertiesJvmMissingClassname() throws Exception {
 
     public void testPluginListSorted() {
         PluginsAndModules pluginsInfo = new PluginsAndModules();
-        pluginsInfo.addPlugin(new PluginInfo("c", "foo", "dummy", "dummyclass", true));
-        pluginsInfo.addPlugin(new PluginInfo("b", "foo", "dummy", "dummyclass", true));
-        pluginsInfo.addPlugin(new PluginInfo("e", "foo", "dummy", "dummyclass", true));
-        pluginsInfo.addPlugin(new PluginInfo("a", "foo", "dummy", "dummyclass", true));
-        pluginsInfo.addPlugin(new PluginInfo("d", "foo", "dummy", "dummyclass", true));
+        pluginsInfo.addPlugin(new PluginInfo("c", "foo", "dummy", "dummyclass"));
+        pluginsInfo.addPlugin(new PluginInfo("b", "foo", "dummy", "dummyclass"));
+        pluginsInfo.addPlugin(new PluginInfo("e", "foo", "dummy", "dummyclass"));
+        pluginsInfo.addPlugin(new PluginInfo("a", "foo", "dummy", "dummyclass"));
+        pluginsInfo.addPlugin(new PluginInfo("d", "foo", "dummy", "dummyclass"));
 
         final List<PluginInfo> infos = pluginsInfo.getPluginInfos();
         List<String> names = infos.stream().map((input) -> input.getName()).collect(Collectors.toList());

docs/reference/migration/migrate_5_0/plugins.asciidoc

@@ -6,6 +6,10 @@ structure of the plugin ZIP archive has changed. All the plugin files must be
 contained in a top-level directory called `elasticsearch`. If you use the
 gradle build, this structure is automatically generated.
 
+==== Plugins isolation
+
+`isolated` option has been removed. Each plugin will have its own classloader.
+
 ==== Site plugins removed
 
 Site plugins have been removed. Site plugins should be reimplemented as Kibana

modules/build.gradle

@@ -35,12 +35,6 @@ subprojects {
     throw new InvalidModelException("Modules cannot contain config files") 
   }
 
-  project.afterEvaluate {
-    if (esplugin.isolated == false) {
-      throw new InvalidModelException("Modules cannot disable isolation")
-    }
-  }
-
   // these are implementation details of our build, no need to publish them!
   install.enabled = false
   uploadArchives.enabled = false

qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

@@ -193,9 +193,9 @@ static MockTerminal installPlugin(String pluginUrl, Environment env, boolean jar
         MockTerminal terminal = new MockTerminal();
         new InstallPluginCommand(env) {
             @Override
-            void jarHellCheck(Path candidate, Path pluginsDir, boolean isolated) throws Exception {
+            void jarHellCheck(Path candidate, Path pluginsDir) throws Exception {
                 if (jarHellCheck) {
-                    super.jarHellCheck(candidate, pluginsDir, isolated);
+                    super.jarHellCheck(candidate, pluginsDir);
                 }
             }
         }.execute(terminal, pluginUrl, true);
@@ -369,40 +369,6 @@ public void testIsolatedPlugins() throws Exception {
         assertPlugin("fake2", pluginDir2, env);
     }
 
-    public void testPurgatoryJarHell() throws Exception {
-        assumeTrue("real filesystem", isReal);
-        Environment environment = createEnv(fs, temp);
-        Path pluginDir1 = createPluginDir(temp);
-        PluginTestUtil.writeProperties(pluginDir1,
-            "description", "fake desc",
-            "name", "fake1",
-            "version", "1.0",
-            "elasticsearch.version", Version.CURRENT.toString(),
-            "java.version", System.getProperty("java.specification.version"),
-            "classname", "FakePlugin",
-            "isolated", "false");
-        writeJar(pluginDir1.resolve("plugin.jar"), "FakePlugin");
-        String pluginZip1 = writeZip(pluginDir1, "elasticsearch");
-        installPlugin(pluginZip1, environment);
-
-        Path pluginDir2 = createPluginDir(temp);
-        PluginTestUtil.writeProperties(pluginDir2,
-            "description", "fake desc",
-            "name", "fake2",
-            "version", "1.0",
-            "elasticsearch.version", Version.CURRENT.toString(),
-            "java.version", System.getProperty("java.specification.version"),
-            "classname", "FakePlugin",
-            "isolated", "false");
-        writeJar(pluginDir2.resolve("plugin.jar"), "FakePlugin");
-        String pluginZip2 = writeZip(pluginDir2, "elasticsearch");
-        IllegalStateException e = expectThrows(IllegalStateException.class, () -> {
-            installPlugin(pluginZip2, environment, true);
-        });
-        assertTrue(e.getMessage(), e.getMessage().contains("jar hell"));
-        assertInstallCleaned(environment);
-    }
-
     public void testExistingPlugin() throws Exception {
         Environment env = createEnv(fs, temp);
         Path pluginDir = createPluginDir(temp);