Fix the ability to remove old plugin

We now read the plugin descriptor when removing an old plugin. This is
to check if we are removing a plugin that is extended by another
plugin. However, when reading the descriptor we enforce that it is of
the same version that we are. This is not the case when a user has
upgraded Elasticsearch and is now trying to remove an old plugin. This
commit fixes this by skipping the version enforcement when reading the
plugin descriptor only when removing a plugin.

Relates #28540
This commit is contained in:
Jason Tedor 2018-02-06 17:38:26 -05:00 committed by GitHub
parent 64adaffe11
commit c2fcf15d9d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 76 additions and 21 deletions

View File

@ -86,7 +86,7 @@ class RemovePluginCommand extends EnvironmentAwareCommand {
// first make sure nothing extends this plugin
List<String> usedBy = new ArrayList<>();
Set<PluginsService.Bundle> bundles = PluginsService.getPluginBundles(env.pluginsFile());
Set<PluginsService.Bundle> bundles = PluginsService.getPluginBundles(env.pluginsFile(), false);
for (PluginsService.Bundle bundle : bundles) {
for (String extendedPlugin : bundle.plugin.getExtendedPlugins()) {
if (extendedPlugin.equals(pluginName)) {

View File

@ -29,6 +29,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.VersionUtils;
import org.junit.Before;
import java.io.BufferedReader;
@ -41,6 +42,7 @@ import java.util.Map;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasToString;
@LuceneTestCase.SuppressFileSystems("*")
@ -78,19 +80,27 @@ public class RemovePluginCommandTests extends ESTestCase {
env = TestEnvironment.newEnvironment(settings);
}
void createPlugin(String name) throws Exception {
void createPlugin(String name) throws IOException {
createPlugin(env.pluginsFile(), name);
}
void createPlugin(Path path, String name) throws Exception {
void createPlugin(String name, Version version) throws IOException {
createPlugin(env.pluginsFile(), name, version);
}
void createPlugin(Path path, String name) throws IOException {
createPlugin(path, name, Version.CURRENT);
}
void createPlugin(Path path, String name, Version version) throws IOException {
PluginTestUtil.writePluginProperties(
path.resolve(name),
"description", "dummy",
"name", name,
"version", "1.0",
"elasticsearch.version", Version.CURRENT.toString(),
"java.version", System.getProperty("java.specification.version"),
"classname", "SomeClass");
path.resolve(name),
"description", "dummy",
"name", name,
"version", "1.0",
"elasticsearch.version", version.toString(),
"java.version", System.getProperty("java.specification.version"),
"classname", "SomeClass");
}
void createMetaPlugin(String name, String... plugins) throws Exception {
@ -137,6 +147,18 @@ public class RemovePluginCommandTests extends ESTestCase {
assertRemoveCleaned(env);
}
public void testRemoveOldVersion() throws Exception {
createPlugin(
"fake",
VersionUtils.randomVersionBetween(
random(),
Version.CURRENT.minimumIndexCompatibilityVersion(),
VersionUtils.getPreviousVersion()));
removePlugin("fake", home, randomBoolean());
assertThat(Files.exists(env.pluginsFile().resolve("fake")), equalTo(false));
assertRemoveCleaned(env);
}
public void testBasicMeta() throws Exception {
createMetaPlugin("meta", "fake1");
createPlugin("other");

View File

@ -181,6 +181,19 @@ public class PluginInfo implements Writeable, ToXContentObject {
* @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.
*
* @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 {
final Path descriptor = path.resolve(ES_PLUGIN_PROPERTIES);
final Map<String, String> propsMap;
@ -214,7 +227,7 @@ public class PluginInfo implements Writeable, ToXContentObject {
"property [elasticsearch.version] is missing for plugin [" + name + "]");
}
final Version esVersion = Version.fromString(esVersionString);
if (esVersion.equals(Version.CURRENT) == false) {
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]",
@ -258,12 +271,12 @@ public class PluginInfo implements Writeable, ToXContentObject {
break;
default:
final String message = String.format(
Locale.ROOT,
"property [%s] must be [%s], [%s], or unspecified but was [%s]",
"has_native_controller",
"true",
"false",
hasNativeControllerValue);
Locale.ROOT,
"property [%s] must be [%s], [%s], or unspecified but was [%s]",
"has_native_controller",
"true",
"false",
hasNativeControllerValue);
throw new IllegalArgumentException(message);
}
}
@ -277,7 +290,7 @@ public class PluginInfo implements Writeable, ToXContentObject {
requiresKeystore = Booleans.parseBoolean(requiresKeystoreValue);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException("property [requires.keystore] must be [true] or [false]," +
" but was [" + requiresKeystoreValue + "]", e);
" but was [" + requiresKeystoreValue + "]", e);
}
if (propsMap.isEmpty() == false) {

View File

@ -317,7 +317,27 @@ public class PluginsService extends AbstractComponent {
}
}
static Set<Bundle> getPluginBundles(Path pluginsDirectory) throws IOException {
/**
* 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
*/
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<>();
@ -326,10 +346,10 @@ public class PluginsService extends AbstractComponent {
logger.trace("--- adding plugin [{}]", plugin.toAbsolutePath());
final PluginInfo info;
try {
info = PluginInfo.readFromProperties(plugin);
info = PluginInfo.readFromProperties(plugin, enforceVersion);
} catch (IOException e) {
throw new IllegalStateException("Could not load plugin descriptor for existing plugin ["
+ plugin.getFileName() + "]. Was the plugin built before 2.0?", e);
+ plugin.getFileName() + "]. Was the plugin built before 2.0?", e);
}
if (bundles.add(new Bundle(info, plugin)) == false) {
throw new IllegalStateException("duplicate plugin: " + info);