Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Plugins: Separate plugin semantic validation from properties format v…
…alidation (#28581)

This commit moves the semantic validation (like which version a plugin
was built for or which java version it is compatible with) from reading
a plugin descriptor, leaving the checks on the format of the descriptor
intact.

relates #28540
  • Loading branch information
rjernst committed Feb 13, 2018
1 parent 53e6f00 commit 9c828e7
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 129 deletions.
Expand Up @@ -565,6 +565,7 @@ private void verifyPluginName(Path pluginPath, String pluginName, Path candidate
/** Load information about the plugin, and verify it can be installed with no errors. */
private PluginInfo loadPluginInfo(Terminal terminal, Path pluginRoot, boolean isBatch, Environment env) throws Exception {
final PluginInfo info = PluginInfo.readFromProperties(pluginRoot);
PluginsService.verifyCompatibility(info);

// checking for existing version of the plugin
verifyPluginName(env.pluginsFile(), info.getName(), pluginRoot);
Expand Down Expand Up @@ -649,6 +650,7 @@ private void installMetaPlugin(Terminal terminal, boolean isBatch, Path tmpRoot,
continue;
}
final PluginInfo info = PluginInfo.readFromProperties(plugin);
PluginsService.verifyCompatibility(info);
verifyPluginName(env.pluginsFile(), info.getName(), plugin);
pluginPaths.add(plugin);
}
Expand Down
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.plugins;

import joptsimple.OptionSet;
import org.elasticsearch.Version;
import org.elasticsearch.cli.EnvironmentAwareCommand;
import org.elasticsearch.cli.Terminal;
import org.elasticsearch.common.Nullable;
Expand Down Expand Up @@ -84,15 +85,11 @@ protected void execute(Terminal terminal, OptionSet options, Environment env) th

private void printPlugin(Environment env, Terminal terminal, Path plugin, String prefix) throws IOException {
terminal.println(Terminal.Verbosity.SILENT, prefix + plugin.getFileName().toString());
try {
PluginInfo info = PluginInfo.readFromProperties(env.pluginsFile().resolve(plugin.toAbsolutePath()));
terminal.println(Terminal.Verbosity.VERBOSE, info.toString(prefix));
} catch (IllegalArgumentException e) {
if (e.getMessage().contains("incompatible with version")) {
terminal.println("WARNING: " + e.getMessage());
} else {
throw e;
}
PluginInfo info = PluginInfo.readFromProperties(env.pluginsFile().resolve(plugin.toAbsolutePath()));
terminal.println(Terminal.Verbosity.VERBOSE, info.toString(prefix));
if (info.getElasticsearchVersion().equals(Version.CURRENT) == false) {
terminal.println("WARNING: plugin [" + info.getName() + "] was built for Elasticsearch version " + info.getVersion() +
" but version " + Version.CURRENT + " is required");
}
}
}
Expand Up @@ -86,7 +86,7 @@ void execute(Terminal terminal, Environment env, String pluginName, boolean purg

// first make sure nothing extends this plugin
List<String> usedBy = new ArrayList<>();
Set<PluginsService.Bundle> bundles = PluginsService.getPluginBundles(env.pluginsFile(), false);
Set<PluginsService.Bundle> bundles = PluginsService.getPluginBundles(env.pluginsFile());
for (PluginsService.Bundle bundle : bundles) {
for (String extendedPlugin : bundle.plugin.getExtendedPlugins()) {
if (extendedPlugin.equals(pluginName)) {
Expand Down
Expand Up @@ -363,11 +363,7 @@ public void testExistingIncompatiblePlugin() throws Exception {
buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");

MockTerminal terminal = listPlugins(home);
final String message = String.format(Locale.ROOT,
"plugin [%s] is incompatible with version [%s]; was designed for version [%s]",
"fake_plugin1",
Version.CURRENT.toString(),
"1.0.0");
String message = "plugin [fake_plugin1] was built for Elasticsearch version 1.0 but version " + Version.CURRENT + " is required";
assertEquals(
"fake_plugin1\n" + "WARNING: " + message + "\n" + "fake_plugin2\n",
terminal.getOutput());
Expand All @@ -389,11 +385,7 @@ public void testExistingIncompatibleMetaPlugin() throws Exception {
buildFakePlugin(env, "fake desc 2", "fake_plugin2", "org.fake2");

MockTerminal terminal = listPlugins(home);
final String message = String.format(Locale.ROOT,
"plugin [%s] is incompatible with version [%s]; was designed for version [%s]",
"fake_plugin1",
Version.CURRENT.toString(),
"1.0.0");
String message = "plugin [fake_plugin1] was built for Elasticsearch version 1.0 but version " + Version.CURRENT + " is required";
assertEquals(
"fake_plugin2\nmeta_plugin\n\tfake_plugin1\n" + "WARNING: " + message + "\n",
terminal.getOutput());
Expand Down
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.http.HttpTransportSettings;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.plugins.PluginsService;
import org.elasticsearch.secure_sm.SecureSM;
import org.elasticsearch.transport.TcpTransport;

Expand Down Expand Up @@ -161,7 +162,7 @@ static Map<String, URL> getCodebaseJarMap(Set<URL> urls) {
static Map<String,Policy> getPluginPermissions(Environment environment) throws IOException, NoSuchAlgorithmException {
Map<String,Policy> map = new HashMap<>();
// collect up set of plugins and modules by listing directories.
Set<Path> pluginsAndModules = new LinkedHashSet<>(PluginInfo.extractAllPlugins(environment.pluginsFile()));
Set<Path> pluginsAndModules = new LinkedHashSet<>(PluginsService.findPluginDirs(environment.pluginsFile()));

if (Files.exists(environment.modulesFile())) {
try (DirectoryStream<Path> stream = Files.newDirectoryStream(environment.modulesFile())) {
Expand Down
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.env.Environment;
import org.elasticsearch.plugins.Platforms;
import org.elasticsearch.plugins.PluginInfo;
import org.elasticsearch.plugins.PluginsService;

import java.io.Closeable;
import java.io.IOException;
Expand Down Expand Up @@ -70,7 +71,7 @@ void spawnNativePluginControllers(final Environment environment) throws IOExcept
* For each plugin, attempt to spawn the controller daemon. Silently ignore any plugin that
* don't include a controller for the correct platform.
*/
List<Path> paths = PluginInfo.extractAllPlugins(pluginsFile);
List<Path> paths = PluginsService.findPluginDirs(pluginsFile);
for (Path plugin : paths) {
final PluginInfo info = PluginInfo.readFromProperties(plugin);
final Path spawnPath = Platforms.nativeControllerPath(plugin);
Expand Down
68 changes: 2 additions & 66 deletions server/src/main/java/org/elasticsearch/plugins/PluginInfo.java
Expand Up @@ -150,67 +150,13 @@ public void writeTo(final StreamOutput out) throws IOException {
}

/**
* Extracts all {@link PluginInfo} 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}
* @throws IOException if an I/O exception occurred reading the plugin descriptors
*/
public static List<Path> extractAllPlugins(final Path rootPath) throws IOException {
final List<Path> plugins = new LinkedList<>(); // order is already lost, but some filesystems have it
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-")) {
continue;
}
if (seen.add(plugin.getFileName().toString()) == false) {
throw new IllegalStateException("duplicate plugin: " + plugin);
}
if (MetaPluginInfo.isMetaPlugin(plugin)) {
try (DirectoryStream<Path> subStream = Files.newDirectoryStream(plugin)) {
for (Path subPlugin : subStream) {
if (MetaPluginInfo.isPropertiesFile(subPlugin) ||
FileSystemUtils.isDesktopServicesStore(subPlugin)) {
continue;
}
if (seen.add(subPlugin.getFileName().toString()) == false) {
throw new IllegalStateException("duplicate plugin: " + subPlugin);
}
plugins.add(subPlugin);
}
}
} else {
plugins.add(plugin);
}
}
}
}
return plugins;
}

/**
* Reads and validates the plugin descriptor file.
*
* @param path the path to the root directory for the plugin
* @return the plugin info
* @throws IOException if an I/O exception occurred reading the plugin descriptor
*/
public static PluginInfo readFromProperties(final Path path) throws IOException {
return readFromProperties(path, true);
}

/**
* Reads and validates the plugin descriptor file. If {@code enforceVersion} is false then version enforcement for the plugin descriptor
* is skipped.
* Reads the plugin descriptor file.
*
* @param path the path to the root directory for the plugin
* @param enforceVersion whether or not to enforce the version when reading plugin descriptors
* @return the plugin info
* @throws IOException if an I/O exception occurred reading the plugin descriptor
*/
static PluginInfo readFromProperties(final Path path, final boolean enforceVersion) throws IOException {
public static PluginInfo readFromProperties(final Path path) throws IOException {
final Path descriptor = path.resolve(ES_PLUGIN_PROPERTIES);

final Map<String, String> propsMap;
Expand Down Expand Up @@ -244,22 +190,12 @@ static PluginInfo readFromProperties(final Path path, final boolean enforceVersi
"property [elasticsearch.version] is missing for plugin [" + name + "]");
}
final Version esVersion = Version.fromString(esVersionString);
if (enforceVersion && esVersion.equals(Version.CURRENT) == false) {
final String message = String.format(
Locale.ROOT,
"plugin [%s] is incompatible with version [%s]; was designed for version [%s]",
name,
Version.CURRENT.toString(),
esVersionString);
throw new IllegalArgumentException(message);
}
final String javaVersionString = propsMap.remove("java.version");
if (javaVersionString == null) {
throw new IllegalArgumentException(
"property [java.version] is missing for plugin [" + name + "]");
}
JarHell.checkVersionFormat(javaVersionString);
JarHell.checkJavaVersion(name, javaVersionString);
final String classname = propsMap.remove("classname");
if (classname == null) {
throw new IllegalArgumentException(
Expand Down
74 changes: 59 additions & 15 deletions server/src/main/java/org/elasticsearch/plugins/PluginsService.java
Expand Up @@ -35,6 +35,7 @@
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.component.LifecycleComponent;
import org.elasticsearch.common.inject.Module;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
Expand All @@ -56,6 +57,7 @@
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -278,6 +280,59 @@ public int hashCode() {
}
}

/**
* 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}
* @throws IOException if an I/O exception occurred reading the directories
*/
public static List<Path> findPluginDirs(final Path rootPath) throws IOException {
final List<Path> plugins = new ArrayList<>();
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(".installing-")) {
continue;
}
if (seen.add(plugin.getFileName().toString()) == false) {
throw new IllegalStateException("duplicate plugin: " + plugin);
}
if (MetaPluginInfo.isMetaPlugin(plugin)) {
try (DirectoryStream<Path> subStream = Files.newDirectoryStream(plugin)) {
for (Path subPlugin : subStream) {
if (MetaPluginInfo.isPropertiesFile(subPlugin) ||
FileSystemUtils.isDesktopServicesStore(subPlugin)) {
continue;
}
if (seen.add(subPlugin.getFileName().toString()) == false) {
throw new IllegalStateException("duplicate plugin: " + subPlugin);
}
plugins.add(subPlugin);
}
}
} else {
plugins.add(plugin);
}
}
}
}
return plugins;
}

/**
* Verify the given plugin is compatible with the current Elasticsearch installation.
*/
static void verifyCompatibility(PluginInfo info) {
if (info.getElasticsearchVersion().equals(Version.CURRENT) == false) {
throw new IllegalArgumentException("Plugin [" + info.getName() + "] was built for Elasticsearch version "
+ info.getElasticsearchVersion() + " but version " + Version.CURRENT + " is running");
}
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<Bundle> getModuleBundles(Path modulesDirectory) throws IOException {
Expand Down Expand Up @@ -326,28 +381,15 @@ static void checkForFailedPluginRemovals(final Path pluginsDirectory) throws IOE
* @throws IOException if an I/O exception occurs reading the plugin bundles
*/
static Set<Bundle> getPluginBundles(final Path pluginsDirectory) throws IOException {
return getPluginBundles(pluginsDirectory, true);
}

/**
* Get the plugin bundles from the specified directory. If {@code enforceVersion} is true, then the version in each plugin descriptor
* must match the current version.
*
* @param pluginsDirectory the directory
* @param enforceVersion whether or not to enforce the version when reading plugin descriptors
* @return the set of plugin bundles in the specified directory
* @throws IOException if an I/O exception occurs reading the plugin bundles
*/
static Set<Bundle> getPluginBundles(final Path pluginsDirectory, final boolean enforceVersion) throws IOException {
Logger logger = Loggers.getLogger(PluginsService.class);
Set<Bundle> bundles = new LinkedHashSet<>();

List<Path> infos = PluginInfo.extractAllPlugins(pluginsDirectory);
List<Path> infos = findPluginDirs(pluginsDirectory);
for (Path plugin : infos) {
logger.trace("--- adding plugin [{}]", plugin.toAbsolutePath());
final PluginInfo info;
try {
info = PluginInfo.readFromProperties(plugin, enforceVersion);
info = PluginInfo.readFromProperties(plugin);
} catch (IOException e) {
throw new IllegalStateException("Could not load plugin descriptor for existing plugin ["
+ plugin.getFileName() + "]. Was the plugin built before 2.0?", e);
Expand Down Expand Up @@ -480,6 +522,8 @@ static void checkBundleJarHell(Bundle bundle, Map<String, Set<URL>> transitiveUr
private Plugin loadBundle(Bundle bundle, Map<String, Plugin> loaded) {
String name = bundle.plugin.getName();

verifyCompatibility(bundle.plugin);

// collect loaders of extended plugins
List<ClassLoader> extendedLoaders = new ArrayList<>();
for (String extendedPluginName : bundle.plugin.getExtendedPlugins()) {
Expand Down
Expand Up @@ -113,7 +113,7 @@ public void testExtractAllPluginsWithDuplicates() throws Exception {
"classname", "FakePlugin");

IllegalStateException exc =
expectThrows(IllegalStateException.class, () -> PluginInfo.extractAllPlugins(pluginDir));
expectThrows(IllegalStateException.class, () -> PluginsService.findPluginDirs(pluginDir));
assertThat(exc.getMessage(), containsString("duplicate plugin"));
assertThat(exc.getMessage(), endsWith("plugin1"));
}
Expand Down
Expand Up @@ -103,20 +103,6 @@ public void testReadFromPropertiesJavaVersionMissing() throws Exception {
assertThat(e.getMessage(), containsString("[java.version] is missing"));
}

public void testReadFromPropertiesJavaVersionIncompatible() throws Exception {
String pluginName = "fake-plugin";
Path pluginDir = createTempDir().resolve(pluginName);
PluginTestUtil.writePluginProperties(pluginDir,
"description", "fake desc",
"name", pluginName,
"elasticsearch.version", Version.CURRENT.toString(),
"java.version", "1000000.0",
"classname", "FakePlugin",
"version", "1.0");
IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginInfo.readFromProperties(pluginDir));
assertThat(e.getMessage(), containsString(pluginName + " requires Java"));
}

public void testReadFromPropertiesBadJavaVersionFormat() throws Exception {
String pluginName = "fake-plugin";
Path pluginDir = createTempDir().resolve(pluginName);
Expand All @@ -143,17 +129,6 @@ public void testReadFromPropertiesBogusElasticsearchVersion() throws Exception {
assertThat(e.getMessage(), containsString("version needs to contain major, minor, and revision"));
}

public void testReadFromPropertiesOldElasticsearchVersion() throws Exception {
Path pluginDir = createTempDir().resolve("fake-plugin");
PluginTestUtil.writePluginProperties(pluginDir,
"description", "fake desc",
"name", "my_plugin",
"version", "1.0",
"elasticsearch.version", Version.V_5_0_0.toString());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginInfo.readFromProperties(pluginDir));
assertThat(e.getMessage(), containsString("was designed for version [5.0.0]"));
}

public void testReadFromPropertiesJvmMissingClassname() throws Exception {
Path pluginDir = createTempDir().resolve("fake-plugin");
PluginTestUtil.writePluginProperties(pluginDir,
Expand Down

0 comments on commit 9c828e7

Please sign in to comment.