Skip to content

Commit 845fbfa

Browse files
cwperkscwperks
authored
Allow extended plugins to be optional (#16909)
* Make extended plugins optional Signed-off-by: Craig Perkins <cwperx@amazon.com> * Make extended plugins optional Signed-off-by: Craig Perkins <cwperx@amazon.com> * Load extensions for classpath plugins Signed-off-by: Craig Perkins <cwperx@amazon.com> * Ensure only single instance for each classpath extension Signed-off-by: Craig Perkins <cwperx@amazon.com> * Add test for classpath plugin extended plugin loading Signed-off-by: Craig Perkins <cwperx@amazon.com> * Modify test to allow optional extended plugin Signed-off-by: Craig Perkins <cwperx@amazon.com> * Only optional extended plugins Signed-off-by: Craig Perkins <cwperx@amazon.com> * Add additional warning message Signed-off-by: Craig Perkins <cwperx@amazon.com> * Add to CHANGELOG Signed-off-by: Craig Perkins <cwperx@amazon.com> * Add tag to make extended plugin optional Signed-off-by: Craig Perkins <cwperx@amazon.com> * Only send plugin names when serializing PluginInfo Signed-off-by: Craig Perkins <cwperx@amazon.com> * Keep track of optional extended plugins in separate set Signed-off-by: Craig Perkins <cwperx@amazon.com> * Include in ser/de of PluginInfo Signed-off-by: Craig Perkins <cwperx@amazon.com> * Change to 3_0_0 Signed-off-by: Craig Perkins <cwperx@amazon.com> --------- Signed-off-by: Craig Perkins <cwperx@amazon.com>
1 parent 703edda commit 845fbfa

5 files changed

Lines changed: 101 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5959
### Changed
6060
- Indexed IP field supports `terms_query` with more than 1025 IP masks [#16391](https://github.com/opensearch-project/OpenSearch/pull/16391)
6161
- Make entries for dependencies from server/build.gradle to gradle version catalog ([#16707](https://github.com/opensearch-project/OpenSearch/pull/16707))
62+
- Allow extended plugins to be optional ([#16909](https://github.com/opensearch-project/OpenSearch/pull/16909))
6263

6364
### Deprecated
6465
- Performing update operation with default pipeline or final pipeline is deprecated ([#16712](https://github.com/opensearch-project/OpenSearch/pull/16712))

‎server/src/main/java/org/opensearch/plugins/PluginInfo.java‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ public class PluginInfo implements Writeable, ToXContentObject {
8686
private final String classname;
8787
private final String customFolderName;
8888
private final List<String> extendedPlugins;
89+
// Optional extended plugins are a subset of extendedPlugins that only contains the optional extended plugins
90+
private final List<String> optionalExtendedPlugins;
8991
private final boolean hasNativeController;
9092

9193
/**
@@ -149,7 +151,11 @@ public PluginInfo(
149151
this.javaVersion = javaVersion;
150152
this.classname = classname;
151153
this.customFolderName = customFolderName;
152-
this.extendedPlugins = Collections.unmodifiableList(extendedPlugins);
154+
this.extendedPlugins = extendedPlugins.stream().map(s -> s.split(";")[0]).collect(Collectors.toUnmodifiableList());
155+
this.optionalExtendedPlugins = extendedPlugins.stream()
156+
.filter(PluginInfo::isOptionalExtension)
157+
.map(s -> s.split(";")[0])
158+
.collect(Collectors.toUnmodifiableList());
153159
this.hasNativeController = hasNativeController;
154160
}
155161

@@ -209,6 +215,16 @@ public PluginInfo(final StreamInput in) throws IOException {
209215
this.customFolderName = in.readString();
210216
this.extendedPlugins = in.readStringList();
211217
this.hasNativeController = in.readBoolean();
218+
if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
219+
this.optionalExtendedPlugins = in.readStringList();
220+
} else {
221+
this.optionalExtendedPlugins = new ArrayList<>();
222+
}
223+
}
224+
225+
static boolean isOptionalExtension(String extendedPlugin) {
226+
String[] dependency = extendedPlugin.split(";");
227+
return dependency.length > 1 && "optional=true".equals(dependency[1]);
212228
}
213229

214230
@Override
@@ -234,6 +250,9 @@ This works for currently supported range notations (=,~)
234250
}
235251
out.writeStringCollection(extendedPlugins);
236252
out.writeBoolean(hasNativeController);
253+
if (out.getVersion().onOrAfter(Version.V_3_0_0)) {
254+
out.writeStringCollection(optionalExtendedPlugins);
255+
}
237256
}
238257

239258
/**
@@ -417,8 +436,17 @@ public String getFolderName() {
417436
*
418437
* @return the names of the plugins extended
419438
*/
439+
public boolean isExtendedPluginOptional(String extendedPlugin) {
440+
return optionalExtendedPlugins.contains(extendedPlugin);
441+
}
442+
443+
/**
444+
* Other plugins this plugin extends through SPI
445+
*
446+
* @return the names of the plugins extended
447+
*/
420448
public List<String> getExtendedPlugins() {
421-
return extendedPlugins;
449+
return extendedPlugins.stream().map(s -> s.split(";")[0]).collect(Collectors.toUnmodifiableList());
422450
}
423451

424452
/**
@@ -493,6 +521,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
493521
builder.field("custom_foldername", customFolderName);
494522
builder.field("extended_plugins", extendedPlugins);
495523
builder.field("has_native_controller", hasNativeController);
524+
builder.field("optional_extended_plugins", optionalExtendedPlugins);
496525
}
497526
builder.endObject();
498527

‎server/src/main/java/org/opensearch/plugins/PluginsService.java‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,13 @@ private static void addSortedBundle(
524524
for (String dependency : bundle.plugin.getExtendedPlugins()) {
525525
Bundle depBundle = bundles.get(dependency);
526526
if (depBundle == null) {
527-
throw new IllegalArgumentException("Missing plugin [" + dependency + "], dependency of [" + name + "]");
527+
if (bundle.plugin.isExtendedPluginOptional(dependency)) {
528+
logger.warn("Missing plugin [" + dependency + "], dependency of [" + name + "]");
529+
logger.warn("Some features of this plugin may not function without the dependencies being installed.\n");
530+
continue;
531+
} else {
532+
throw new IllegalArgumentException("Missing plugin [" + dependency + "], dependency of [" + name + "]");
533+
}
528534
}
529535
addSortedBundle(depBundle, bundles, sortedBundles, dependencyStack);
530536
assert sortedBundles.contains(depBundle);
@@ -653,6 +659,9 @@ static void checkBundleJarHell(Set<URL> classpath, Bundle bundle, Map<String, Se
653659
Set<URL> urls = new HashSet<>();
654660
for (String extendedPlugin : exts) {
655661
Set<URL> pluginUrls = transitiveUrls.get(extendedPlugin);
662+
if (pluginUrls == null && bundle.plugin.isExtendedPluginOptional(extendedPlugin)) {
663+
continue;
664+
}
656665
assert pluginUrls != null : "transitive urls should have already been set for " + extendedPlugin;
657666

658667
Set<URL> intersection = new HashSet<>(urls);
@@ -704,6 +713,10 @@ private Plugin loadBundle(Bundle bundle, Map<String, Plugin> loaded) {
704713
List<ClassLoader> extendedLoaders = new ArrayList<>();
705714
for (String extendedPluginName : bundle.plugin.getExtendedPlugins()) {
706715
Plugin extendedPlugin = loaded.get(extendedPluginName);
716+
if (extendedPlugin == null && bundle.plugin.isExtendedPluginOptional(extendedPluginName)) {
717+
// extended plugin is optional and is not installed
718+
continue;
719+
}
707720
assert extendedPlugin != null;
708721
if (ExtensiblePlugin.class.isInstance(extendedPlugin) == false) {
709722
throw new IllegalStateException("Plugin [" + name + "] cannot extend non-extensible plugin [" + extendedPluginName + "]");

‎server/src/test/java/org/opensearch/plugins/PluginInfoTests.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.opensearch.semver.SemverRange;
4545
import org.opensearch.test.OpenSearchTestCase;
4646

47+
import java.io.IOException;
4748
import java.nio.ByteBuffer;
4849
import java.nio.file.Path;
4950
import java.util.ArrayList;
@@ -55,6 +56,7 @@
5556
import static org.hamcrest.Matchers.containsString;
5657
import static org.hamcrest.Matchers.empty;
5758
import static org.hamcrest.Matchers.equalTo;
59+
import static org.hamcrest.Matchers.is;
5860

5961
public class PluginInfoTests extends OpenSearchTestCase {
6062

@@ -281,6 +283,30 @@ public void testReadFromPropertiesJvmMissingClassname() throws Exception {
281283
assertThat(e.getMessage(), containsString("property [classname] is missing"));
282284
}
283285

286+
public void testExtendedPluginsSingleOptionalExtension() throws IOException {
287+
Path pluginDir = createTempDir().resolve("fake-plugin");
288+
PluginTestUtil.writePluginProperties(
289+
pluginDir,
290+
"description",
291+
"fake desc",
292+
"name",
293+
"my_plugin",
294+
"version",
295+
"1.0",
296+
"opensearch.version",
297+
Version.CURRENT.toString(),
298+
"java.version",
299+
System.getProperty("java.specification.version"),
300+
"classname",
301+
"FakePlugin",
302+
"extended.plugins",
303+
"foo;optional=true"
304+
);
305+
PluginInfo info = PluginInfo.readFromProperties(pluginDir);
306+
assertThat(info.getExtendedPlugins(), contains("foo"));
307+
assertThat(info.isExtendedPluginOptional("foo"), is(true));
308+
}
309+
284310
public void testExtendedPluginsSingleExtension() throws Exception {
285311
Path pluginDir = createTempDir().resolve("fake-plugin");
286312
PluginTestUtil.writePluginProperties(
@@ -302,6 +328,7 @@ public void testExtendedPluginsSingleExtension() throws Exception {
302328
);
303329
PluginInfo info = PluginInfo.readFromProperties(pluginDir);
304330
assertThat(info.getExtendedPlugins(), contains("foo"));
331+
assertThat(info.isExtendedPluginOptional("foo"), is(false));
305332
}
306333

307334
public void testExtendedPluginsMultipleExtensions() throws Exception {

‎server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ public void testSortBundlesNoDeps() throws Exception {
361361
assertThat(sortedBundles, Matchers.contains(bundle1, bundle2, bundle3));
362362
}
363363

364-
public void testSortBundlesMissingDep() throws Exception {
364+
public void testSortBundlesMissingRequiredDep() throws Exception {
365365
Path pluginDir = createTempDir();
366366
PluginInfo info = new PluginInfo("foo", "desc", "1.0", Version.CURRENT, "1.8", "MyPlugin", Collections.singletonList("dne"), false);
367367
PluginsService.Bundle bundle = new PluginsService.Bundle(info, pluginDir);
@@ -372,6 +372,33 @@ public void testSortBundlesMissingDep() throws Exception {
372372
assertEquals("Missing plugin [dne], dependency of [foo]", e.getMessage());
373373
}
374374

375+
public void testSortBundlesMissingOptionalDep() throws Exception {
376+
try (MockLogAppender mockLogAppender = MockLogAppender.createForLoggers(LogManager.getLogger(PluginsService.class))) {
377+
mockLogAppender.addExpectation(
378+
new MockLogAppender.SeenEventExpectation(
379+
"[.test] warning",
380+
"org.opensearch.plugins.PluginsService",
381+
Level.WARN,
382+
"Missing plugin [dne], dependency of [foo]"
383+
)
384+
);
385+
Path pluginDir = createTempDir();
386+
PluginInfo info = new PluginInfo(
387+
"foo",
388+
"desc",
389+
"1.0",
390+
Version.CURRENT,
391+
"1.8",
392+
"MyPlugin",
393+
Collections.singletonList("dne;optional=true"),
394+
false
395+
);
396+
PluginsService.Bundle bundle = new PluginsService.Bundle(info, pluginDir);
397+
PluginsService.sortBundles(Collections.singleton(bundle));
398+
mockLogAppender.assertAllExpectationsMatched();
399+
}
400+
}
401+
375402
public void testSortBundlesCommonDep() throws Exception {
376403
Path pluginDir = createTempDir();
377404
Set<PluginsService.Bundle> bundles = new LinkedHashSet<>(); // control iteration order

0 commit comments

Comments
 (0)