From 43e8974fce02ad33597bee629b2aabc148012a68 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 4 Sep 2024 23:43:59 -0400 Subject: [PATCH 01/18] WIP on requesting perms and displaying on install Signed-off-by: Craig Perkins --- .../plugins/InstallPluginCommand.java | 3 + .../plugins/IdentityAwarePlugin.java | 22 ++++++ .../opensearch/plugins/PluginsService.java | 68 ++++++++++++++++++- 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java index 838d6e22a37bd..13edb60b28d77 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java @@ -60,6 +60,7 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.collect.Tuple; import org.opensearch.common.hash.MessageDigests; +import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.env.Environment; @@ -893,6 +894,8 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo env.configDir().resolve(targetFolderName), deleteOnFailure ); + final PluginsService.Bundle bundle = new PluginsService.Bundle(info, env.pluginsDir()); + final Plugin plugin = PluginsService.loadBundle(bundle, Settings.EMPTY, env.configDir(), tmpRoot); movePlugin(tmpRoot, destination); return info; } diff --git a/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java b/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java index b19dcfe5544ec..5732038439cd0 100644 --- a/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java +++ b/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java @@ -12,6 +12,10 @@ import org.opensearch.identity.PluginSubject; import org.opensearch.identity.Subject; +import java.util.Collections; +import java.util.Map; +import java.util.Set; + /** * Plugin that performs transport actions with a plugin system context. IdentityAwarePlugins are initialized * with a {@link Subject} that they can utilize to perform transport actions outside the default subject. @@ -31,4 +35,22 @@ public interface IdentityAwarePlugin { * interaction */ default void assignSubject(PluginSubject pluginSubject) {} + + /** + * Returns a set of cluster actions this plugin can perform within a pluginSubject.runAs(() -> { ... }) block. + * + * @return Set of cluster actions + */ + default Set getClusterActions() { + return Collections.emptySet(); + } + + /** + * Returns a map of index pattern -> allowed index actions this plugin can perform within a pluginSubject.runAs(() -> { ... }) block. + * + * @return Map of index pattern -> allowed index actions + */ + default Map> getIndexActions() { + return Collections.emptyMap(); + } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index f08c9c738f1b4..303e4e67f01f5 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -694,6 +694,68 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map jarPaths = Files.list(pluginPath) + .filter((fileName) -> fileName.toString().toLowerCase(Locale.ROOT).endsWith(".jar")) + .collect(Collectors.toList()); + + if (jarPaths.isEmpty()) { + throw new IllegalStateException("No JAR files found"); + } + + URL[] urls = new URL[jarPaths.size()]; + for (int i = 0; i < jarPaths.size(); i++) { + urls[i] = jarPaths.get(i).toUri().toURL(); + } + URLClassLoader loader = new URLClassLoader(urls, parentLoader); + + // reload SPI with any new services from the plugin + reloadLuceneSPI(loader); + + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + try { + // Set context class loader to plugin's class loader so that plugins + // that have dependencies with their own SPI endpoints have a chance to load + // and initialize them appropriately. + AccessController.doPrivileged((PrivilegedAction) () -> { + Thread.currentThread().setContextClassLoader(loader); + return null; + }); + + logger.debug("Loading plugin [" + name + "]..."); + Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), loader); + if (loader != pluginClass.getClassLoader()) { + throw new IllegalStateException( + "Plugin [" + + name + + "] must reference a class loader local Plugin class [" + + bundle.plugin.getClassname() + + "] (class loader [" + + pluginClass.getClassLoader() + + "])" + ); + } + Plugin plugin = loadPlugin(pluginClass, settings, configPath); + return plugin; + } finally { + AccessController.doPrivileged((PrivilegedAction) () -> { + Thread.currentThread().setContextClassLoader(cl); + return null; + }); + } + } + @SuppressWarnings("removal") private Plugin loadBundle(Bundle bundle, Map loaded) { String name = bundle.plugin.getName(); @@ -767,7 +829,7 @@ static void reloadLuceneSPI(ClassLoader loader) { Codec.reloadCodecs(loader); } - private Class loadPluginClass(String className, ClassLoader loader) { + private static Class loadPluginClass(String className, ClassLoader loader) { try { return Class.forName(className, false, loader).asSubclass(Plugin.class); } catch (Throwable t) { @@ -775,7 +837,7 @@ private Class loadPluginClass(String className, ClassLoader lo } } - private Plugin loadPlugin(Class pluginClass, Settings settings, Path configPath) { + private static Plugin loadPlugin(Class pluginClass, Settings settings, Path configPath) { final Constructor[] constructors = pluginClass.getConstructors(); if (constructors.length == 0) { throw new IllegalStateException("no public constructor for [" + pluginClass.getName() + "]"); @@ -806,7 +868,7 @@ private Plugin loadPlugin(Class pluginClass, Settings settings } } - private String signatureMessage(final Class clazz) { + private static String signatureMessage(final Class clazz) { return String.format( Locale.ROOT, "no public constructor of correct signature for [%s]; must be [%s], [%s], or [%s]", From a841f0613d362e80309f70da2f1aa172a848a964 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 5 Sep 2024 16:44:27 -0400 Subject: [PATCH 02/18] Actually compile a fake plugin in InstallPluginCommandTests Signed-off-by: Craig Perkins --- distribution/tools/plugin-cli/build.gradle | 1 + .../plugins/InstallPluginCommand.java | 19 ++++- .../plugins/InstallPluginCommandTests.java | 81 +++++++++++++++++-- .../opensearch/plugins/PluginSecurity.java | 61 ++++++++++++-- .../opensearch/plugins/PluginsService.java | 9 ++- 5 files changed, 153 insertions(+), 18 deletions(-) diff --git a/distribution/tools/plugin-cli/build.gradle b/distribution/tools/plugin-cli/build.gradle index 784cdc457a1a9..df6c4cb693be4 100644 --- a/distribution/tools/plugin-cli/build.gradle +++ b/distribution/tools/plugin-cli/build.gradle @@ -39,6 +39,7 @@ dependencies { compileOnly project(":libs:opensearch-cli") api "org.bouncycastle:bcpg-fips:2.0.9" api "org.bouncycastle:bc-fips:2.0.0" + testRuntimeOnly project(':libs:opensearch-plugin-classloader') testImplementation project(":test:framework") testImplementation 'com.google.jimfs:jimfs:1.3.0' testRuntimeOnly("com.google.guava:guava:${versions.guava}") { diff --git a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java index 13edb60b28d77..a590e9ad63ad7 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java @@ -881,7 +881,22 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo } else { permissions = Collections.emptySet(); } - PluginSecurity.confirmPolicyExceptions(terminal, permissions, isBatch); + final PluginsService.Bundle bundle = new PluginsService.Bundle(info, env.pluginsDir()); + + final Set requestedClusterActions = new HashSet<>(); + final Map> requestedIndexActions = new HashMap<>(); + + final IdentityAwarePlugin plugin = PluginsService.maybeLoadIdentityAwarePluginFromBundle( + bundle, + Settings.EMPTY, + env.configDir(), + tmpRoot + ); + if (plugin != null) { + requestedClusterActions.addAll(plugin.getClusterActions()); + requestedIndexActions.putAll(plugin.getIndexActions()); + } + PluginSecurity.confirmPolicyExceptions(terminal, permissions, requestedClusterActions, requestedIndexActions, isBatch); String targetFolderName = info.getTargetFolderName(); final Path destination = env.pluginsDir().resolve(targetFolderName); @@ -894,8 +909,6 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo env.configDir().resolve(targetFolderName), deleteOnFailure ); - final PluginsService.Bundle bundle = new PluginsService.Bundle(info, env.pluginsDir()); - final Plugin plugin = PluginsService.loadBundle(bundle, Settings.EMPTY, env.configDir(), tmpRoot); movePlugin(tmpRoot, destination); return info; } diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java index c264788df20e8..879b36b2efcd2 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java @@ -77,12 +77,22 @@ import org.junit.After; import org.junit.Before; +import javax.tools.FileObject; +import javax.tools.ForwardingJavaFileManager; +import javax.tools.JavaCompiler; +import javax.tools.JavaFileManager; +import javax.tools.JavaFileObject; +import javax.tools.SimpleJavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; + import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.io.StringReader; import java.net.MalformedURLException; import java.net.URI; @@ -237,7 +247,13 @@ static Path createPluginDir(Function temp) throws IOException { static void writeJar(Path jar, String... classes) throws IOException { try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(jar))) { for (String clazz : classes) { - stream.putNextEntry(new ZipEntry(clazz + ".class")); // no package names, just support simple classes + clazz = clazz.replace('.', '/'); + ZipEntry entry = new ZipEntry(clazz + ".class"); + stream.putNextEntry(entry); // no package names, just support simple classes + Path compiledClassPath = jar.getParent().resolve(clazz + ".class"); + if (Files.exists(compiledClassPath)) { + Files.copy(compiledClassPath, stream); + } } } } @@ -263,7 +279,60 @@ static String createPluginUrl(String name, Path structure, String... additionalP return createPlugin(name, structure, additionalProps).toUri().toURL().toString(); } + static class JavaSourceFromString extends SimpleJavaFileObject { + private final String code; + + public JavaSourceFromString(String className, String code) { + super(URI.create("string:///" + className.replace('.', '/') + Kind.SOURCE.extension), Kind.SOURCE); + this.code = code; + } + + @Override + public CharSequence getCharContent(boolean ignoreEncodingErrors) { + return code; + } + } + + private static String compileFakePlugin(Path structure) throws IOException { + String pluginClassName = "org.opensearch.plugins.FakePlugin"; + String javaSourceCode = "package org.opensearch.plugins;\n" + "\n" + "class FakePlugin extends Plugin {}\n"; + if (Files.notExists(structure)) { + Files.createDirectories(structure); + } + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + + StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, null); + JavaFileManager fileManager = new ForwardingJavaFileManager(standardFileManager) { + @Override + public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind, FileObject sibling) { + Path classFile = structure.resolve(className.replace('.', '/') + ".class"); + if (Files.notExists(classFile.getParent())) { + try { + Files.createDirectories(classFile.getParent()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return new SimpleJavaFileObject(classFile.toUri(), kind) { + @Override + public OutputStream openOutputStream() throws IOException { + return Files.newOutputStream(classFile); + } + }; + } + }; + + JavaFileObject javaFileObject = new JavaSourceFromString(pluginClassName, javaSourceCode); + Iterable options = Arrays.asList("-d", structure.toUri().toString()); + JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, null, Arrays.asList(javaFileObject)); + boolean success = task.call(); + // Close the file manager + fileManager.close(); + return pluginClassName; + } + static void writePlugin(String name, Path structure, String... additionalProps) throws IOException { + String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( Stream.of( "description", @@ -277,16 +346,18 @@ static void writePlugin(String name, Path structure, String... additionalProps) "java.version", System.getProperty("java.specification.version"), "classname", - "FakePlugin" + pluginClassName ), Arrays.stream(additionalProps) ).toArray(String[]::new); + PluginTestUtil.writePluginProperties(structure, properties); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; - writeJar(structure.resolve("plugin.jar"), className); + writeJar(structure.resolve("plugin.jar"), className, pluginClassName); } static void writePlugin(String name, Path structure, SemverRange opensearchVersionRange, String... additionalProps) throws IOException { + String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( Stream.of( "description", @@ -300,13 +371,13 @@ static void writePlugin(String name, Path structure, SemverRange opensearchVersi "java.version", System.getProperty("java.specification.version"), "classname", - "FakePlugin" + pluginClassName ), Arrays.stream(additionalProps) ).toArray(String[]::new); PluginTestUtil.writePluginProperties(structure, properties); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; - writeJar(structure.resolve("plugin.jar"), className); + writeJar(structure.resolve("plugin.jar"), className, pluginClassName); } static Path createPlugin(String name, Path structure, SemverRange opensearchVersionRange, String... additionalProps) diff --git a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java index 1bf8642d1112f..d422906613215 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java +++ b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java @@ -51,6 +51,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; @@ -64,9 +65,15 @@ class PluginSecurity { /** * prints/confirms policy exceptions with the user */ - static void confirmPolicyExceptions(Terminal terminal, Set permissions, boolean batch) throws UserException { + static void confirmPolicyExceptions( + Terminal terminal, + Set permissions, + Set requestedClusterActions, + Map> requestedIndexActions, + boolean batch + ) throws UserException { List requested = new ArrayList<>(permissions); - if (requested.isEmpty()) { + if (requested.isEmpty() && requestedClusterActions.isEmpty() && requestedIndexActions.isEmpty()) { terminal.println(Verbosity.VERBOSE, "plugin has a policy file with no additional permissions"); } else { @@ -76,12 +83,52 @@ static void confirmPolicyExceptions(Terminal terminal, Set permissions, terminal.errorPrintln(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); terminal.errorPrintln(Verbosity.NORMAL, "@ WARNING: plugin requires additional permissions @"); terminal.errorPrintln(Verbosity.NORMAL, "@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@@"); - // print all permissions: - for (String permission : requested) { - terminal.errorPrintln(Verbosity.NORMAL, "* " + permission); + if (!requested.isEmpty()) { + // print all permissions: + for (String permission : requested) { + terminal.errorPrintln(Verbosity.NORMAL, "* " + permission); + } + terminal.errorPrintln( + Verbosity.NORMAL, + "See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html" + ); + terminal.errorPrintln(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks."); + terminal.errorPrintln(Verbosity.NORMAL, ""); + terminal.errorPrintln(Verbosity.NORMAL, "Plugin requests permission to perform the following transport actions. Any index"); + terminal.errorPrintln( + Verbosity.NORMAL, + "pattern that appears below is a default value and may change depending on plugin settings." + ); } - terminal.errorPrintln(Verbosity.NORMAL, "See http://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html"); - terminal.errorPrintln(Verbosity.NORMAL, "for descriptions of what these permissions allow and the associated risks."); + if (!requestedClusterActions.isEmpty() || !requestedIndexActions.isEmpty()) { + terminal.errorPrintln(Verbosity.NORMAL, ""); + terminal.errorPrintln(Verbosity.NORMAL, "Cluster Actions"); + terminal.errorPrintln(Verbosity.NORMAL, "---------------"); + terminal.errorPrintln(Verbosity.NORMAL, ""); + if (requestedClusterActions.isEmpty()) { + terminal.errorPrintln(Verbosity.NORMAL, "None"); + } else { + for (String clusterAction : requestedClusterActions) { + terminal.errorPrintln(Verbosity.NORMAL, "* " + clusterAction); + } + } + terminal.errorPrintln(Verbosity.NORMAL, ""); + terminal.errorPrintln(Verbosity.NORMAL, "Index Actions"); + terminal.errorPrintln(Verbosity.NORMAL, "-------------"); + terminal.errorPrintln(Verbosity.NORMAL, ""); + if (requestedIndexActions.isEmpty()) { + terminal.errorPrintln(Verbosity.NORMAL, "None"); + } else { + for (Map.Entry> entry : requestedIndexActions.entrySet()) { + terminal.errorPrintln(Verbosity.NORMAL, "Index Pattern: " + entry.getKey()); + terminal.errorPrintln(Verbosity.NORMAL, ""); + for (String indexAction : entry.getValue()) { + terminal.errorPrintln(Verbosity.NORMAL, "* " + indexAction); + } + } + } + } + prompt(terminal, batch); } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 303e4e67f01f5..7070651e533b5 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -695,7 +695,8 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map) () -> { Thread.currentThread().setContextClassLoader(cl); From 58216320899cfec5e8e197f6a50f3d8196ab5fe4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 16 Sep 2024 15:35:25 -0400 Subject: [PATCH 03/18] Add test for prompt when installing IdentityAwarePlugin Signed-off-by: Craig Perkins --- .../plugins/InstallPluginCommandTests.java | 150 ++++++++++++++---- .../opensearch/plugins/PluginsService.java | 11 -- 2 files changed, 119 insertions(+), 42 deletions(-) diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java index 879b36b2efcd2..f0c58a78fcf57 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java @@ -279,6 +279,11 @@ static String createPluginUrl(String name, Path structure, String... additionalP return createPlugin(name, structure, additionalProps).toUri().toURL().toString(); } + /** creates a plugin .zip and returns the url for testing */ + static String createIdentityAwarePluginUrl(String name, Path structure, String... additionalProps) throws IOException { + return createIdentityAwarePlugin(name, structure, additionalProps).toUri().toURL().toString(); + } + static class JavaSourceFromString extends SimpleJavaFileObject { private final String code; @@ -331,6 +336,56 @@ public OutputStream openOutputStream() throws IOException { return pluginClassName; } + private static String compileIdentityAwareFakePlugin(Path structure) throws IOException { + String pluginClassName = "org.opensearch.plugins.FakePlugin"; + String javaSourceCode = "package org.opensearch.plugins;\n" + + "\n" + + "import java.util.Set;\n" + + "\n" + + "public class FakePlugin extends Plugin implements IdentityAwarePlugin {\n" + + "\n" + + " public FakePlugin() {}\n" + + "\n" + + " @Override\n" + + " public Set getClusterActions() {\n" + + " return Set.of(\"cluster:monitor/health\");\n" + + " }\n" + + "}\n"; + if (Files.notExists(structure)) { + Files.createDirectories(structure); + } + JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); + + StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, null); + JavaFileManager fileManager = new ForwardingJavaFileManager(standardFileManager) { + @Override + public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind, FileObject sibling) { + Path classFile = structure.resolve(className.replace('.', '/') + ".class"); + if (Files.notExists(classFile.getParent())) { + try { + Files.createDirectories(classFile.getParent()); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return new SimpleJavaFileObject(classFile.toUri(), kind) { + @Override + public OutputStream openOutputStream() throws IOException { + return Files.newOutputStream(classFile); + } + }; + } + }; + + JavaFileObject javaFileObject = new JavaSourceFromString(pluginClassName, javaSourceCode); + Iterable options = Arrays.asList("-d", structure.toUri().toString()); + JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, null, Arrays.asList(javaFileObject)); + boolean success = task.call(); + // Close the file manager + fileManager.close(); + return pluginClassName; + } + static void writePlugin(String name, Path structure, String... additionalProps) throws IOException { String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( @@ -356,6 +411,31 @@ static void writePlugin(String name, Path structure, String... additionalProps) writeJar(structure.resolve("plugin.jar"), className, pluginClassName); } + static void writeIdentityAwarePlugin(String name, Path structure, String... additionalProps) throws IOException { + String pluginClassName = compileIdentityAwareFakePlugin(structure); + String[] properties = Stream.concat( + Stream.of( + "description", + "fake desc", + "name", + name, + "version", + "1.0", + "opensearch.version", + Version.CURRENT.toString(), + "java.version", + System.getProperty("java.specification.version"), + "classname", + pluginClassName + ), + Arrays.stream(additionalProps) + ).toArray(String[]::new); + + PluginTestUtil.writePluginProperties(structure, properties); + String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; + writeJar(structure.resolve("plugin.jar"), className, pluginClassName); + } + static void writePlugin(String name, Path structure, SemverRange opensearchVersionRange, String... additionalProps) throws IOException { String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( @@ -402,6 +482,11 @@ static Path createPlugin(String name, Path structure, String... additionalProps) return writeZip(structure, null); } + static Path createIdentityAwarePlugin(String name, Path structure, String... additionalProps) throws IOException { + writeIdentityAwarePlugin(name, structure, additionalProps); + return writeZip(structure, null); + } + void installPlugin(String pluginUrl, Path home) throws Exception { installPlugin(pluginUrl, home, skipJarHellCommand); } @@ -1611,43 +1696,37 @@ private String signature(final byte[] bytes, final PGPSecretKey secretKey) { // checks the plugin requires a policy confirmation, and does not install when that is rejected by the user // the plugin is installed after this method completes private void assertPolicyConfirmation(Tuple env, String pluginZip, String... warnings) throws Exception { - for (int i = 0; i < warnings.length; ++i) { - String warning = warnings[i]; - for (int j = 0; j < i; ++j) { - terminal.addTextInput("y"); // accept warnings we have already tested - } - // default answer, does not install - terminal.addTextInput(""); - UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1())); - assertEquals("installation aborted by user", e.getMessage()); - - assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning)); - try (Stream fileStream = Files.list(env.v2().pluginsDir())) { - assertThat(fileStream.collect(Collectors.toList()), empty()); - } + // default answer, does not install + terminal.addTextInput(""); + UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1())); + assertEquals("installation aborted by user", e.getMessage()); - // explicitly do not install - terminal.reset(); - for (int j = 0; j < i; ++j) { - terminal.addTextInput("y"); // accept warnings we have already tested - } - terminal.addTextInput("n"); - e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1())); - assertEquals("installation aborted by user", e.getMessage()); - assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning)); - try (Stream fileStream = Files.list(env.v2().pluginsDir())) { - assertThat(fileStream.collect(Collectors.toList()), empty()); - } + try (Stream fileStream = Files.list(env.v2().pluginsDir())) { + assertThat(fileStream.collect(Collectors.toList()), empty()); } - // allow installation + for (String warning : warnings) { + assertThat(terminal.getErrorOutput(), containsString(warning)); + } + + // explicitly do not install terminal.reset(); - for (int j = 0; j < warnings.length; ++j) { - terminal.addTextInput("y"); + terminal.addTextInput("n"); + e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1())); + assertEquals("installation aborted by user", e.getMessage()); + try (Stream fileStream = Files.list(env.v2().pluginsDir())) { + assertThat(fileStream.collect(Collectors.toList()), empty()); } + for (String warning : warnings) { + assertThat(terminal.getErrorOutput(), containsString(warning)); + } + + // allow installation + terminal.reset(); + terminal.addTextInput("y"); installPlugin(pluginZip, env.v1()); for (String warning : warnings) { - assertThat(terminal.getErrorOutput(), containsString("WARNING: " + warning)); + assertThat(terminal.getErrorOutput(), containsString(warning)); } } @@ -1657,7 +1736,16 @@ public void testPolicyConfirmation() throws Exception { writePluginSecurityPolicy(pluginDir, "setAccessible", "setFactory"); String pluginZip = createPluginUrl("fake", pluginDir); - assertPolicyConfirmation(env, pluginZip, "plugin requires additional permissions"); + assertPolicyConfirmation(env, pluginZip, "WARNING: plugin requires additional permissions"); + assertPlugin("fake", pluginDir, env.v2()); + } + + public void testRequestedActionsConfirmation() throws Exception { + Tuple env = createEnv(fs, temp); + Path pluginDir = createPluginDir(temp); + String pluginZip = createIdentityAwarePluginUrl("fake", pluginDir); + + assertPolicyConfirmation(env, pluginZip, "WARNING: plugin requires additional permissions", "Cluster Actions", "Index Actions"); assertPlugin("fake", pluginDir, env.v2()); } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 7070651e533b5..42236f12ce6e5 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -736,17 +736,6 @@ static IdentityAwarePlugin maybeLoadIdentityAwarePluginFromBundle(Bundle bundle, logger.debug("Loading plugin [" + name + "]..."); Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), loader); - if (loader != pluginClass.getClassLoader()) { - throw new IllegalStateException( - "Plugin [" - + name - + "] must reference a class loader local Plugin class [" - + bundle.plugin.getClassname() - + "] (class loader [" - + pluginClass.getClassLoader() - + "])" - ); - } if (!IdentityAwarePlugin.class.isAssignableFrom(pluginClass)) { return null; } From 11069785af5bd1c9276b9e39777fc03caab4067e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 16 Sep 2024 16:10:49 -0400 Subject: [PATCH 04/18] Update warning message Signed-off-by: Craig Perkins --- server/src/main/java/org/opensearch/plugins/PluginSecurity.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java index d422906613215..1cbabde9d5659 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java +++ b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java @@ -74,7 +74,7 @@ static void confirmPolicyExceptions( ) throws UserException { List requested = new ArrayList<>(permissions); if (requested.isEmpty() && requestedClusterActions.isEmpty() && requestedIndexActions.isEmpty()) { - terminal.println(Verbosity.VERBOSE, "plugin has a policy file with no additional permissions"); + terminal.println(Verbosity.VERBOSE, "plugin has not requested any additional permissions"); } else { // sort permissions in a reasonable order From 4a29367c516bedf3d60014ea2333df269356f95f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 17 Sep 2024 14:51:41 -0400 Subject: [PATCH 05/18] Get requested actions from plugin-permissions.yml Signed-off-by: Craig Perkins --- .../plugins/InstallPluginCommand.java | 38 +++++++--- .../plugins/InstallPluginCommandTests.java | 74 +++++-------------- .../opensearch/common/settings/Setting.java | 39 ++++++++++ .../org/opensearch/plugins/PluginInfo.java | 1 + .../opensearch/plugins/PluginSecurity.java | 15 +++- .../opensearch/plugins/PluginsService.java | 54 -------------- 6 files changed, 95 insertions(+), 126 deletions(-) diff --git a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java index a590e9ad63ad7..34e0e605ee017 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java @@ -60,6 +60,7 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.collect.Tuple; import org.opensearch.common.hash.MessageDigests; +import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.env.Environment; @@ -102,6 +103,7 @@ import java.util.Objects; import java.util.Set; import java.util.TreeSet; +import java.util.function.Function; import java.util.stream.Collectors; import static org.opensearch.cli.Terminal.Verbosity.VERBOSE; @@ -194,6 +196,14 @@ class InstallPluginCommand extends EnvironmentAwareCommand { static final Set PLUGIN_DIR_PERMS; static final Set PLUGIN_FILES_PERMS; + static final Setting> CLUSTER_ACTIONS_SETTING = Setting.listSetting( + "cluster.actions", + Collections.emptyList(), + Function.identity() + ); + + static final Setting INDEX_ACTIONS_SETTING = Setting.groupSetting("index.actions."); + static { // Bin directory get chmod 755 BIN_DIR_PERMS = Collections.unmodifiableSet(PosixFilePermissions.fromString("rwxr-xr-x")); @@ -881,21 +891,25 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo } else { permissions = Collections.emptySet(); } - final PluginsService.Bundle bundle = new PluginsService.Bundle(info, env.pluginsDir()); - final Set requestedClusterActions = new HashSet<>(); - final Map> requestedIndexActions = new HashMap<>(); + Path actions = tmpRoot.resolve(PluginInfo.OPENSEARCH_PLUGIN_ACTIONS); + Settings requestedActions = Settings.EMPTY; - final IdentityAwarePlugin plugin = PluginsService.maybeLoadIdentityAwarePluginFromBundle( - bundle, - Settings.EMPTY, - env.configDir(), - tmpRoot - ); - if (plugin != null) { - requestedClusterActions.addAll(plugin.getClusterActions()); - requestedIndexActions.putAll(plugin.getIndexActions()); + if (Files.exists(actions)) { + requestedActions = PluginSecurity.parseRequestedActions(actions); + } + + final Map> requestedIndexActions = new HashMap<>(); + + final List requestedClusterActions = CLUSTER_ACTIONS_SETTING.get(requestedActions); + final Settings requestedIndexActionsGroup = INDEX_ACTIONS_SETTING.get(requestedActions); + if (!requestedIndexActionsGroup.keySet().isEmpty()) { + for (String indexPattern : requestedIndexActionsGroup.keySet()) { + List indexActionsForPattern = requestedIndexActionsGroup.getAsList(indexPattern); + requestedIndexActions.put(indexPattern, indexActionsForPattern); + } } + PluginSecurity.confirmPolicyExceptions(terminal, permissions, requestedClusterActions, requestedIndexActions, isBatch); String targetFolderName = info.getTargetFolderName(); diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java index f0c58a78fcf57..4a36947e508c1 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java @@ -280,8 +280,8 @@ static String createPluginUrl(String name, Path structure, String... additionalP } /** creates a plugin .zip and returns the url for testing */ - static String createIdentityAwarePluginUrl(String name, Path structure, String... additionalProps) throws IOException { - return createIdentityAwarePlugin(name, structure, additionalProps).toUri().toURL().toString(); + static String createPluginWithRequestedActionsUrl(String name, Path structure, String... additionalProps) throws IOException { + return createPluginWithRequestedActions(name, structure, additionalProps).toUri().toURL().toString(); } static class JavaSourceFromString extends SimpleJavaFileObject { @@ -336,56 +336,6 @@ public OutputStream openOutputStream() throws IOException { return pluginClassName; } - private static String compileIdentityAwareFakePlugin(Path structure) throws IOException { - String pluginClassName = "org.opensearch.plugins.FakePlugin"; - String javaSourceCode = "package org.opensearch.plugins;\n" - + "\n" - + "import java.util.Set;\n" - + "\n" - + "public class FakePlugin extends Plugin implements IdentityAwarePlugin {\n" - + "\n" - + " public FakePlugin() {}\n" - + "\n" - + " @Override\n" - + " public Set getClusterActions() {\n" - + " return Set.of(\"cluster:monitor/health\");\n" - + " }\n" - + "}\n"; - if (Files.notExists(structure)) { - Files.createDirectories(structure); - } - JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); - - StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, null); - JavaFileManager fileManager = new ForwardingJavaFileManager(standardFileManager) { - @Override - public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind, FileObject sibling) { - Path classFile = structure.resolve(className.replace('.', '/') + ".class"); - if (Files.notExists(classFile.getParent())) { - try { - Files.createDirectories(classFile.getParent()); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - return new SimpleJavaFileObject(classFile.toUri(), kind) { - @Override - public OutputStream openOutputStream() throws IOException { - return Files.newOutputStream(classFile); - } - }; - } - }; - - JavaFileObject javaFileObject = new JavaSourceFromString(pluginClassName, javaSourceCode); - Iterable options = Arrays.asList("-d", structure.toUri().toString()); - JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, null, Arrays.asList(javaFileObject)); - boolean success = task.call(); - // Close the file manager - fileManager.close(); - return pluginClassName; - } - static void writePlugin(String name, Path structure, String... additionalProps) throws IOException { String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( @@ -411,8 +361,8 @@ static void writePlugin(String name, Path structure, String... additionalProps) writeJar(structure.resolve("plugin.jar"), className, pluginClassName); } - static void writeIdentityAwarePlugin(String name, Path structure, String... additionalProps) throws IOException { - String pluginClassName = compileIdentityAwareFakePlugin(structure); + static void writePluginWithRequestedActions(String name, Path structure, String... additionalProps) throws IOException { + String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( Stream.of( "description", @@ -432,6 +382,7 @@ static void writeIdentityAwarePlugin(String name, Path structure, String... addi ).toArray(String[]::new); PluginTestUtil.writePluginProperties(structure, properties); + writePluginPermissionsYaml(structure); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; writeJar(structure.resolve("plugin.jar"), className, pluginClassName); } @@ -477,13 +428,22 @@ static void writePluginSecurityPolicy(Path pluginDir, String... permissions) thr Files.write(pluginDir.resolve("plugin-security.policy"), securityPolicyContent.toString().getBytes(StandardCharsets.UTF_8)); } + static void writePluginPermissionsYaml(Path pluginDir, String... permissions) throws IOException { + String permissionsYamlContent = "cluster.actions:\n" + + " - cluster:monitor/health\n" + + "indices.actions:\n" + + " example-index*:\n" + + " - indices:data/write/index*"; + Files.write(pluginDir.resolve("plugin-permissions.yml"), permissionsYamlContent.getBytes(StandardCharsets.UTF_8)); + } + static Path createPlugin(String name, Path structure, String... additionalProps) throws IOException { writePlugin(name, structure, additionalProps); return writeZip(structure, null); } - static Path createIdentityAwarePlugin(String name, Path structure, String... additionalProps) throws IOException { - writeIdentityAwarePlugin(name, structure, additionalProps); + static Path createPluginWithRequestedActions(String name, Path structure, String... additionalProps) throws IOException { + writePluginWithRequestedActions(name, structure, additionalProps); return writeZip(structure, null); } @@ -1743,7 +1703,7 @@ public void testPolicyConfirmation() throws Exception { public void testRequestedActionsConfirmation() throws Exception { Tuple env = createEnv(fs, temp); Path pluginDir = createPluginDir(temp); - String pluginZip = createIdentityAwarePluginUrl("fake", pluginDir); + String pluginZip = createPluginWithRequestedActionsUrl("fake", pluginDir); assertPolicyConfirmation(env, pluginZip, "WARNING: plugin requires additional permissions", "Cluster Actions", "Index Actions"); assertPlugin("fake", pluginDir, env.v2()); diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index fea4c165809ba..2f1c4cbfa47e9 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -2345,6 +2345,20 @@ public static Setting> listSetting( return new ListSetting<>(key, fallbackSetting, defaultStringValue, parser, validator, properties); } + public static Setting>>> listSetting( + final String key, + final List defaultStringValue, + final Property... properties + ) { + Function> defaultStringValueFn = (s) -> defaultStringValue; + if (defaultStringValueFn.apply(Settings.EMPTY) == null) { + throw new IllegalArgumentException("default value function must not return null"); + } + Function>>> parser = (s) -> new ArrayList<>(parseableMapToList(s)); + + return new ListSetting<>(key, null, defaultStringValueFn, parser, v -> {}, properties); + } + private static List parseableStringToList(String parsableString) { // fromXContent doesn't use named xcontent or deprecation. try ( @@ -2368,6 +2382,31 @@ private static List parseableStringToList(String parsableString) { } } + private static List>> parseableMapToList(String parsableString) { + // fromXContent doesn't use named xcontent or deprecation. + try ( + XContentParser xContentParser = MediaTypeRegistry.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, parsableString) + ) { + XContentParser.Token token = xContentParser.nextToken(); + if (token != XContentParser.Token.START_ARRAY) { + throw new IllegalArgumentException("expected START_ARRAY but got " + token); + } + ArrayList>> list = new ArrayList<>(); + while ((token = xContentParser.nextToken()) != XContentParser.Token.END_ARRAY) { + list.add( + xContentParser.map( + HashMap::new, + p -> p.list().stream().map(object -> Objects.toString(object, null)).collect(Collectors.toSet()) + ) + ); + } + return list; + } catch (IOException e) { + throw new IllegalArgumentException("failed to parse array", e); + } + } + private static String arrayToParsableString(List array) { try { XContentBuilder builder = XContentBuilder.builder(MediaTypeRegistry.JSON.xContent()); diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index b6030f4ded5e5..0b3a8bd8c8c70 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -73,6 +73,7 @@ public class PluginInfo implements Writeable, ToXContentObject { public static final String OPENSEARCH_PLUGIN_PROPERTIES = "plugin-descriptor.properties"; public static final String OPENSEARCH_PLUGIN_POLICY = "plugin-security.policy"; + public static final String OPENSEARCH_PLUGIN_ACTIONS = "plugin-permissions.yml"; private static final JsonFactory jsonFactory = new JsonFactory().configure( JsonReadFeature.ALLOW_UNQUOTED_FIELD_NAMES.mappedFeature(), true diff --git a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java index 1cbabde9d5659..a4d83b84aa17a 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java +++ b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java @@ -36,6 +36,7 @@ import org.opensearch.cli.Terminal; import org.opensearch.cli.Terminal.Verbosity; import org.opensearch.cli.UserException; +import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import java.io.IOException; @@ -68,8 +69,8 @@ class PluginSecurity { static void confirmPolicyExceptions( Terminal terminal, Set permissions, - Set requestedClusterActions, - Map> requestedIndexActions, + List requestedClusterActions, + Map> requestedIndexActions, boolean batch ) throws UserException { List requested = new ArrayList<>(permissions); @@ -119,7 +120,7 @@ static void confirmPolicyExceptions( if (requestedIndexActions.isEmpty()) { terminal.errorPrintln(Verbosity.NORMAL, "None"); } else { - for (Map.Entry> entry : requestedIndexActions.entrySet()) { + for (Map.Entry> entry : requestedIndexActions.entrySet()) { terminal.errorPrintln(Verbosity.NORMAL, "Index Pattern: " + entry.getKey()); terminal.errorPrintln(Verbosity.NORMAL, ""); for (String indexAction : entry.getValue()) { @@ -218,4 +219,12 @@ public static Set parsePermissions(Path file, Path tmpDir) throws IOExce } return Collections.list(actualPermissions.elements()).stream().map(PluginSecurity::formatPermission).collect(Collectors.toSet()); } + + /** + * Parses plugin-permissions.yml file. + */ + @SuppressWarnings("removal") + public static Settings parseRequestedActions(Path file) throws IOException { + return Settings.builder().loadFromPath(file).build(); + } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 42236f12ce6e5..2669ecd24235c 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -694,60 +694,6 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map jarPaths = Files.list(pluginPath) - .filter((fileName) -> fileName.toString().toLowerCase(Locale.ROOT).endsWith(".jar")) - .collect(Collectors.toList()); - - if (jarPaths.isEmpty()) { - throw new IllegalStateException("No JAR files found"); - } - - URL[] urls = new URL[jarPaths.size()]; - for (int i = 0; i < jarPaths.size(); i++) { - urls[i] = jarPaths.get(i).toUri().toURL(); - } - URLClassLoader loader = new URLClassLoader(urls, parentLoader); - - // reload SPI with any new services from the plugin - reloadLuceneSPI(loader); - - ClassLoader cl = Thread.currentThread().getContextClassLoader(); - try { - // Set context class loader to plugin's class loader so that plugins - // that have dependencies with their own SPI endpoints have a chance to load - // and initialize them appropriately. - AccessController.doPrivileged((PrivilegedAction) () -> { - Thread.currentThread().setContextClassLoader(loader); - return null; - }); - - logger.debug("Loading plugin [" + name + "]..."); - Class pluginClass = loadPluginClass(bundle.plugin.getClassname(), loader); - if (!IdentityAwarePlugin.class.isAssignableFrom(pluginClass)) { - return null; - } - return (IdentityAwarePlugin) loadPlugin(pluginClass, settings, configPath); - } finally { - AccessController.doPrivileged((PrivilegedAction) () -> { - Thread.currentThread().setContextClassLoader(cl); - return null; - }); - } - } - @SuppressWarnings("removal") private Plugin loadBundle(Bundle bundle, Map loaded) { String name = bundle.plugin.getName(); From f4bd92123749d2982d38423705b3fb3e7a1f8d6f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 17 Sep 2024 15:21:02 -0400 Subject: [PATCH 06/18] Allow plugin dev to configure a description to describe why plugin is requesting permissions Signed-off-by: Craig Perkins --- .../plugins/InstallPluginCommand.java | 12 +++++- .../opensearch/common/settings/Setting.java | 39 ------------------- .../plugins/IdentityAwarePlugin.java | 22 ----------- .../opensearch/plugins/PluginSecurity.java | 5 +++ .../opensearch/plugins/PluginsService.java | 6 +-- 5 files changed, 19 insertions(+), 65 deletions(-) diff --git a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java index 34e0e605ee017..979327014efb0 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java @@ -204,6 +204,8 @@ class InstallPluginCommand extends EnvironmentAwareCommand { static final Setting INDEX_ACTIONS_SETTING = Setting.groupSetting("index.actions."); + static final Setting DESCRIPTION_SETTING = Setting.simpleString("description"); + static { // Bin directory get chmod 755 BIN_DIR_PERMS = Collections.unmodifiableSet(PosixFilePermissions.fromString("rwxr-xr-x")); @@ -903,6 +905,7 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo final List requestedClusterActions = CLUSTER_ACTIONS_SETTING.get(requestedActions); final Settings requestedIndexActionsGroup = INDEX_ACTIONS_SETTING.get(requestedActions); + final String pluginActionDescription = DESCRIPTION_SETTING.get(requestedActions); if (!requestedIndexActionsGroup.keySet().isEmpty()) { for (String indexPattern : requestedIndexActionsGroup.keySet()) { List indexActionsForPattern = requestedIndexActionsGroup.getAsList(indexPattern); @@ -910,7 +913,14 @@ private PluginInfo installPlugin(Terminal terminal, boolean isBatch, Path tmpRoo } } - PluginSecurity.confirmPolicyExceptions(terminal, permissions, requestedClusterActions, requestedIndexActions, isBatch); + PluginSecurity.confirmPolicyExceptions( + terminal, + permissions, + pluginActionDescription, + requestedClusterActions, + requestedIndexActions, + isBatch + ); String targetFolderName = info.getTargetFolderName(); final Path destination = env.pluginsDir().resolve(targetFolderName); diff --git a/server/src/main/java/org/opensearch/common/settings/Setting.java b/server/src/main/java/org/opensearch/common/settings/Setting.java index 2f1c4cbfa47e9..fea4c165809ba 100644 --- a/server/src/main/java/org/opensearch/common/settings/Setting.java +++ b/server/src/main/java/org/opensearch/common/settings/Setting.java @@ -2345,20 +2345,6 @@ public static Setting> listSetting( return new ListSetting<>(key, fallbackSetting, defaultStringValue, parser, validator, properties); } - public static Setting>>> listSetting( - final String key, - final List defaultStringValue, - final Property... properties - ) { - Function> defaultStringValueFn = (s) -> defaultStringValue; - if (defaultStringValueFn.apply(Settings.EMPTY) == null) { - throw new IllegalArgumentException("default value function must not return null"); - } - Function>>> parser = (s) -> new ArrayList<>(parseableMapToList(s)); - - return new ListSetting<>(key, null, defaultStringValueFn, parser, v -> {}, properties); - } - private static List parseableStringToList(String parsableString) { // fromXContent doesn't use named xcontent or deprecation. try ( @@ -2382,31 +2368,6 @@ private static List parseableStringToList(String parsableString) { } } - private static List>> parseableMapToList(String parsableString) { - // fromXContent doesn't use named xcontent or deprecation. - try ( - XContentParser xContentParser = MediaTypeRegistry.JSON.xContent() - .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, parsableString) - ) { - XContentParser.Token token = xContentParser.nextToken(); - if (token != XContentParser.Token.START_ARRAY) { - throw new IllegalArgumentException("expected START_ARRAY but got " + token); - } - ArrayList>> list = new ArrayList<>(); - while ((token = xContentParser.nextToken()) != XContentParser.Token.END_ARRAY) { - list.add( - xContentParser.map( - HashMap::new, - p -> p.list().stream().map(object -> Objects.toString(object, null)).collect(Collectors.toSet()) - ) - ); - } - return list; - } catch (IOException e) { - throw new IllegalArgumentException("failed to parse array", e); - } - } - private static String arrayToParsableString(List array) { try { XContentBuilder builder = XContentBuilder.builder(MediaTypeRegistry.JSON.xContent()); diff --git a/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java b/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java index 5732038439cd0..b19dcfe5544ec 100644 --- a/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java +++ b/server/src/main/java/org/opensearch/plugins/IdentityAwarePlugin.java @@ -12,10 +12,6 @@ import org.opensearch.identity.PluginSubject; import org.opensearch.identity.Subject; -import java.util.Collections; -import java.util.Map; -import java.util.Set; - /** * Plugin that performs transport actions with a plugin system context. IdentityAwarePlugins are initialized * with a {@link Subject} that they can utilize to perform transport actions outside the default subject. @@ -35,22 +31,4 @@ public interface IdentityAwarePlugin { * interaction */ default void assignSubject(PluginSubject pluginSubject) {} - - /** - * Returns a set of cluster actions this plugin can perform within a pluginSubject.runAs(() -> { ... }) block. - * - * @return Set of cluster actions - */ - default Set getClusterActions() { - return Collections.emptySet(); - } - - /** - * Returns a map of index pattern -> allowed index actions this plugin can perform within a pluginSubject.runAs(() -> { ... }) block. - * - * @return Map of index pattern -> allowed index actions - */ - default Map> getIndexActions() { - return Collections.emptyMap(); - } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java index a4d83b84aa17a..3420dd5d7f8ed 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginSecurity.java +++ b/server/src/main/java/org/opensearch/plugins/PluginSecurity.java @@ -69,6 +69,7 @@ class PluginSecurity { static void confirmPolicyExceptions( Terminal terminal, Set permissions, + String description, List requestedClusterActions, Map> requestedIndexActions, boolean batch @@ -103,6 +104,10 @@ static void confirmPolicyExceptions( } if (!requestedClusterActions.isEmpty() || !requestedIndexActions.isEmpty()) { terminal.errorPrintln(Verbosity.NORMAL, ""); + if (description != null) { + terminal.errorPrintln(Verbosity.NORMAL, description); + terminal.errorPrintln(Verbosity.NORMAL, ""); + } terminal.errorPrintln(Verbosity.NORMAL, "Cluster Actions"); terminal.errorPrintln(Verbosity.NORMAL, "---------------"); terminal.errorPrintln(Verbosity.NORMAL, ""); diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 2669ecd24235c..f08c9c738f1b4 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -767,7 +767,7 @@ static void reloadLuceneSPI(ClassLoader loader) { Codec.reloadCodecs(loader); } - private static Class loadPluginClass(String className, ClassLoader loader) { + private Class loadPluginClass(String className, ClassLoader loader) { try { return Class.forName(className, false, loader).asSubclass(Plugin.class); } catch (Throwable t) { @@ -775,7 +775,7 @@ private static Class loadPluginClass(String className, ClassLo } } - private static Plugin loadPlugin(Class pluginClass, Settings settings, Path configPath) { + private Plugin loadPlugin(Class pluginClass, Settings settings, Path configPath) { final Constructor[] constructors = pluginClass.getConstructors(); if (constructors.length == 0) { throw new IllegalStateException("no public constructor for [" + pluginClass.getName() + "]"); @@ -806,7 +806,7 @@ private static Plugin loadPlugin(Class pluginClass, Settings s } } - private static String signatureMessage(final Class clazz) { + private String signatureMessage(final Class clazz) { return String.format( Locale.ROOT, "no public constructor of correct signature for [%s]; must be [%s], [%s], or [%s]", From 454a5dd498935445052ef5d7a8f6e43f0888ce8e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 18 Sep 2024 20:38:17 -0400 Subject: [PATCH 07/18] Add requested actions to PluginInfo and pass PluginInfo IdentityPlugin.getPluginSubject Signed-off-by: Craig Perkins --- .../identity/shiro/ShiroIdentityPlugin.java | 3 +- .../opensearch/identity/IdentityService.java | 10 +++-- .../identity/noop/NoopIdentityPlugin.java | 4 +- .../main/java/org/opensearch/node/Node.java | 6 ++- .../opensearch/plugins/IdentityPlugin.java | 4 +- .../org/opensearch/plugins/PluginInfo.java | 40 ++++++++++++++++--- .../opensearch/plugins/PluginsService.java | 17 +++++++- .../identity/noop/NoopPluginSubjectTests.java | 18 ++++++++- .../nodesinfo/NodeInfoStreamingTests.java | 6 ++- .../opensearch/plugins/PluginInfoTests.java | 10 +++-- .../plugins/PluginsServiceTests.java | 9 +++-- 11 files changed, 101 insertions(+), 26 deletions(-) diff --git a/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java b/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java index af802596ebaa7..ee7b0c2b600e6 100644 --- a/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java +++ b/plugins/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java @@ -26,6 +26,7 @@ import org.opensearch.identity.tokens.TokenManager; import org.opensearch.plugins.IdentityPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; import org.opensearch.threadpool.ThreadPool; @@ -101,7 +102,7 @@ public TokenManager getTokenManager() { } @Override - public PluginSubject getPluginSubject(Plugin plugin) { + public PluginSubject getPluginSubject(PluginInfo pluginInfo) { return new ShiroPluginSubject(threadPool); } } diff --git a/server/src/main/java/org/opensearch/identity/IdentityService.java b/server/src/main/java/org/opensearch/identity/IdentityService.java index 03f937180f4ba..279c0672a9ffb 100644 --- a/server/src/main/java/org/opensearch/identity/IdentityService.java +++ b/server/src/main/java/org/opensearch/identity/IdentityService.java @@ -8,12 +8,14 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.opensearch.OpenSearchException; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.identity.noop.NoopIdentityPlugin; import org.opensearch.identity.tokens.TokenManager; import org.opensearch.plugins.IdentityAwarePlugin; import org.opensearch.plugins.IdentityPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.threadpool.ThreadPool; import java.util.List; @@ -61,11 +63,11 @@ public TokenManager getTokenManager() { return identityPlugin.getTokenManager(); } - public void initializeIdentityAwarePlugins(final List identityAwarePlugins) { + public void initializeIdentityAwarePlugins(final List> identityAwarePlugins) { if (identityAwarePlugins != null) { - for (IdentityAwarePlugin plugin : identityAwarePlugins) { - PluginSubject pluginSubject = identityPlugin.getPluginSubject((Plugin) plugin); - plugin.assignSubject(pluginSubject); + for (Tuple pluginTuple : identityAwarePlugins) { + PluginSubject pluginSubject = identityPlugin.getPluginSubject(pluginTuple.v1()); + ((IdentityAwarePlugin) pluginTuple.v2()).assignSubject(pluginSubject); } } } diff --git a/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java b/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java index 6279388c76f96..977f88c1a24d0 100644 --- a/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java +++ b/server/src/main/java/org/opensearch/identity/noop/NoopIdentityPlugin.java @@ -12,7 +12,7 @@ import org.opensearch.identity.Subject; import org.opensearch.identity.tokens.TokenManager; import org.opensearch.plugins.IdentityPlugin; -import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.threadpool.ThreadPool; /** @@ -49,7 +49,7 @@ public TokenManager getTokenManager() { } @Override - public PluginSubject getPluginSubject(Plugin plugin) { + public PluginSubject getPluginSubject(PluginInfo pluginInfo) { return new NoopPluginSubject(threadPool); } } diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index a8d4ebcf23dab..0982d296158df 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -91,6 +91,7 @@ import org.opensearch.common.StopWatch; import org.opensearch.common.cache.module.CacheModule; import org.opensearch.common.cache.service.CacheService; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.inject.Injector; import org.opensearch.common.inject.Key; import org.opensearch.common.inject.Module; @@ -211,6 +212,7 @@ import org.opensearch.plugins.NetworkPlugin; import org.opensearch.plugins.PersistentTaskPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.plugins.PluginsService; import org.opensearch.plugins.RepositoryPlugin; import org.opensearch.plugins.ScriptPlugin; @@ -1023,8 +1025,8 @@ protected Node( // Add the telemetryAwarePlugin components to the existing pluginComponents collection. pluginComponents.addAll(telemetryAwarePluginComponents); - List identityAwarePlugins = pluginsService.filterPlugins(IdentityAwarePlugin.class); - identityService.initializeIdentityAwarePlugins(identityAwarePlugins); + List> identityAwarePluginTuples = pluginsService.filterPluginTuples(IdentityAwarePlugin.class); + identityService.initializeIdentityAwarePlugins(identityAwarePluginTuples); final QueryGroupService queryGroupService = new QueryGroupService(); // We will need to replace this with actual instance of the // queryGroupService diff --git a/server/src/main/java/org/opensearch/plugins/IdentityPlugin.java b/server/src/main/java/org/opensearch/plugins/IdentityPlugin.java index b40af14231fb9..b77a4cafd0c6b 100644 --- a/server/src/main/java/org/opensearch/plugins/IdentityPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/IdentityPlugin.java @@ -38,8 +38,8 @@ public interface IdentityPlugin { * Gets a subject corresponding to the passed plugin that can be utilized to perform transport actions * in the plugin system context * - * @param plugin The corresponding plugin + * @param pluginInfo The corresponding pluginInfo * @return Subject corresponding to the plugin */ - PluginSubject getPluginSubject(Plugin plugin); + PluginSubject getPluginSubject(PluginInfo pluginInfo); } diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index 0b3a8bd8c8c70..cec0b0559ac8c 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -38,6 +38,7 @@ import org.opensearch.Version; import org.opensearch.bootstrap.JarHell; import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.json.JsonXContentParser; import org.opensearch.core.common.Strings; import org.opensearch.core.common.io.stream.StreamInput; @@ -88,6 +89,7 @@ public class PluginInfo implements Writeable, ToXContentObject { private final String customFolderName; private final List extendedPlugins; private final boolean hasNativeController; + private final Settings requestedActions; /** * Construct plugin info. @@ -111,7 +113,8 @@ public PluginInfo( String classname, String customFolderName, List extendedPlugins, - boolean hasNativeController + boolean hasNativeController, + Settings requestedActions ) { this( name, @@ -122,7 +125,8 @@ public PluginInfo( classname, customFolderName, extendedPlugins, - hasNativeController + hasNativeController, + requestedActions ); } @@ -135,7 +139,8 @@ public PluginInfo( String classname, String customFolderName, List extendedPlugins, - boolean hasNativeController + boolean hasNativeController, + Settings requestedActions ) { this.name = name; this.description = description; @@ -152,6 +157,7 @@ public PluginInfo( this.customFolderName = customFolderName; this.extendedPlugins = Collections.unmodifiableList(extendedPlugins); this.hasNativeController = hasNativeController; + this.requestedActions = requestedActions; } /** @@ -185,7 +191,8 @@ public PluginInfo( classname, null /* customFolderName */, extendedPlugins, - hasNativeController + hasNativeController, + Settings.EMPTY /* requestedActions */ ); } @@ -210,6 +217,7 @@ public PluginInfo(final StreamInput in) throws IOException { this.customFolderName = in.readString(); this.extendedPlugins = in.readStringList(); this.hasNativeController = in.readBoolean(); + this.requestedActions = Settings.readSettingsFromStream(in); } @Override @@ -235,6 +243,7 @@ This works for currently supported range notations (=,~) } out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); + Settings.writeSettingsToStream(requestedActions, out); } /** @@ -364,6 +373,17 @@ public static PluginInfo readFromProperties(final Path path) throws IOException throw new IllegalArgumentException("Unknown properties in plugin descriptor: " + propsMap.keySet()); } + Settings requestedActions = Settings.EMPTY; + Path actions = path.resolve(PluginInfo.OPENSEARCH_PLUGIN_ACTIONS); + + if (Files.exists(actions)) { + try { + requestedActions = PluginSecurity.parseRequestedActions(actions); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + return new PluginInfo( name, description, @@ -373,7 +393,8 @@ public static PluginInfo readFromProperties(final Path path) throws IOException classname, customFolderName, extendedPlugins, - hasNativeController + hasNativeController, + requestedActions ); } @@ -481,6 +502,15 @@ public String getTargetFolderName() { return (this.customFolderName == null || this.customFolderName.isEmpty()) ? this.name : this.customFolderName; } + /** + * Returns actions requested by this plugin in the plugin-permissions.yml file + * + * @return A settings object containing the contents of plugin-permissions.yml file + */ + public Settings getRequestedActions() { + return requestedActions; + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index f08c9c738f1b4..cab25d57b23e3 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -151,7 +151,8 @@ public PluginsService( pluginClass.getName(), null, Collections.emptyList(), - false + false, + null ); if (logger.isTraceEnabled()) { logger.trace("plugin loaded from classpath [{}]", pluginInfo); @@ -324,10 +325,12 @@ public PluginsAndModules info() { // a "bundle" is a group of jars in a single classloader static class Bundle { final PluginInfo plugin; + final Path pluginDir; final Set urls; Bundle(PluginInfo plugin, Path dir) throws IOException { this.plugin = Objects.requireNonNull(plugin); + this.pluginDir = dir; Set urls = new LinkedHashSet<>(); // gather urls for jar files try (DirectoryStream jarStream = Files.newDirectoryStream(dir, "*.jar")) { @@ -817,6 +820,18 @@ private String signatureMessage(final Class clazz) { ); } + public Settings getRequestedActionsForPlugin(Plugin plugin) { + Optional> tuple = plugins.stream().filter(x -> x.v2().equals(plugin)).findFirst(); + if (tuple.isEmpty()) { + throw new IllegalStateException("Unable to find plugin in loaded plugins"); + } + return tuple.get().v1().getRequestedActions(); + } + + public List> filterPluginTuples(Class type) { + return plugins.stream().filter(x -> type.isAssignableFrom(x.v2().getClass())).collect(Collectors.toList()); + } + public List filterPlugins(Class type) { return plugins.stream().filter(x -> type.isAssignableFrom(x.v2().getClass())).map(p -> ((T) p.v2())).collect(Collectors.toList()); } diff --git a/server/src/test/java/org/opensearch/identity/noop/NoopPluginSubjectTests.java b/server/src/test/java/org/opensearch/identity/noop/NoopPluginSubjectTests.java index 79c26a7eb790d..0d14a34e24770 100644 --- a/server/src/test/java/org/opensearch/identity/noop/NoopPluginSubjectTests.java +++ b/server/src/test/java/org/opensearch/identity/noop/NoopPluginSubjectTests.java @@ -8,16 +8,20 @@ package org.opensearch.identity.noop; +import org.opensearch.Version; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.identity.IdentityService; import org.opensearch.identity.NamedPrincipal; import org.opensearch.identity.PluginSubject; import org.opensearch.plugins.IdentityAwarePlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.PluginInfo; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.TestThreadPool; import org.opensearch.threadpool.ThreadPool; +import java.util.Collections; import java.util.List; import static org.hamcrest.Matchers.equalTo; @@ -41,7 +45,19 @@ public void testInitializeIdentityAwarePlugin() throws Exception { IdentityService identityService = new IdentityService(Settings.EMPTY, threadPool, List.of()); TestPlugin testPlugin = new TestPlugin(); - identityService.initializeIdentityAwarePlugins(List.of(testPlugin)); + final PluginInfo info = new PluginInfo( + "fake", + "foo", + "x.y.z", + Version.CURRENT, + "1.8", + testPlugin.getClass().getCanonicalName(), + "folder", + Collections.emptyList(), + false, + Settings.EMPTY + ); + identityService.initializeIdentityAwarePlugins(List.of(Tuple.tuple(info, testPlugin))); PluginSubject testPluginSubject = new NoopPluginSubject(threadPool); assertThat(testPlugin.getSubject().getPrincipal().getName(), equalTo(NamedPrincipal.UNAUTHENTICATED.getName())); diff --git a/server/src/test/java/org/opensearch/nodesinfo/NodeInfoStreamingTests.java b/server/src/test/java/org/opensearch/nodesinfo/NodeInfoStreamingTests.java index fba26b0c72e0e..c4f74e7e19ebf 100644 --- a/server/src/test/java/org/opensearch/nodesinfo/NodeInfoStreamingTests.java +++ b/server/src/test/java/org/opensearch/nodesinfo/NodeInfoStreamingTests.java @@ -179,7 +179,8 @@ private static NodeInfo createNodeInfo() { randomAlphaOfLengthBetween(3, 10), name, Collections.emptyList(), - randomBoolean() + randomBoolean(), + Settings.EMPTY ) ); } @@ -197,7 +198,8 @@ private static NodeInfo createNodeInfo() { randomAlphaOfLengthBetween(3, 10), name, Collections.emptyList(), - randomBoolean() + randomBoolean(), + Settings.EMPTY ) ); } diff --git a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java index 12c7dc870c104..8e0932889ea95 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java @@ -37,6 +37,7 @@ import org.opensearch.Version; import org.opensearch.action.admin.cluster.node.info.PluginsAndModules; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.common.io.stream.ByteBufferStreamInput; import org.opensearch.core.xcontent.ToXContent; @@ -360,7 +361,8 @@ public void testSerialize() throws Exception { "dummyclass", "c", Collections.singletonList("foo"), - randomBoolean() + randomBoolean(), + Settings.EMPTY ); BytesStreamOutput output = new BytesStreamOutput(); info.writeTo(output); @@ -380,7 +382,8 @@ public void testToXContent() throws Exception { "dummyClass", "folder", Collections.emptyList(), - false + false, + Settings.EMPTY ); XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); String prettyPrint = info.toXContent(builder, ToXContent.EMPTY_PARAMS).prettyPrint().toString(); @@ -618,7 +621,8 @@ public void testhMultipleOpenSearchRangesInConstructor() throws Exception { "dummyclass", null, Collections.emptyList(), - randomBoolean() + randomBoolean(), + Settings.EMPTY ) ); assertThat(e.getMessage(), containsString("Exactly one range is allowed to be specified in dependencies for the plugin")); diff --git a/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java index bd9ee33856f14..2698471f9c635 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java @@ -730,7 +730,8 @@ public void testCompatibleOpenSearchVersionRange() { "FakePlugin", null, Collections.emptyList(), - false + false, + Settings.EMPTY ); PluginsService.verifyCompatibility(info); } @@ -752,7 +753,8 @@ public void testIncompatibleOpenSearchVersionRange() { "FakePlugin", null, Collections.emptyList(), - false + false, + Settings.EMPTY ); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginsService.verifyCompatibility(info)); assertThat(e.getMessage(), containsString("was built for OpenSearch version ")); @@ -1113,7 +1115,8 @@ private PluginInfo getPluginInfoWithWithSemverRange(String semverRange) { "FakePlugin", null, Collections.emptyList(), - false + false, + Settings.EMPTY ); } From a2fb0fd08983355a57d3594519410052903250a9 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 19 Sep 2024 10:16:28 -0400 Subject: [PATCH 08/18] Add null check Signed-off-by: Craig Perkins --- server/src/main/java/org/opensearch/plugins/PluginInfo.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index cec0b0559ac8c..d39067c976807 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -243,7 +243,11 @@ This works for currently supported range notations (=,~) } out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); - Settings.writeSettingsToStream(requestedActions, out); + if (requestedActions != null) { + Settings.writeSettingsToStream(requestedActions, out); + } else { + Settings.writeSettingsToStream(Settings.EMPTY, out); + } } /** From 758b671ca07617cee3eef8497897d260aab1bea6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 19 Sep 2024 11:59:20 -0400 Subject: [PATCH 09/18] Check stream version Signed-off-by: Craig Perkins --- server/src/main/java/org/opensearch/plugins/PluginInfo.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index d39067c976807..7adefb8c7b1bb 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -217,7 +217,11 @@ public PluginInfo(final StreamInput in) throws IOException { this.customFolderName = in.readString(); this.extendedPlugins = in.readStringList(); this.hasNativeController = in.readBoolean(); - this.requestedActions = Settings.readSettingsFromStream(in); + if (in.getVersion().onOrAfter(Version.V_2_18_0)) { + this.requestedActions = Settings.readSettingsFromStream(in); + } else { + this.requestedActions = Settings.EMPTY; + } } @Override From 4cdd593d4f1126ad033fd3da3093868fcea2e8a9 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 19 Sep 2024 14:08:26 -0400 Subject: [PATCH 10/18] Check version when serializing Signed-off-by: Craig Perkins --- .../src/main/java/org/opensearch/plugins/PluginInfo.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index 7adefb8c7b1bb..c00fe0f92933e 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -217,7 +217,8 @@ public PluginInfo(final StreamInput in) throws IOException { this.customFolderName = in.readString(); this.extendedPlugins = in.readStringList(); this.hasNativeController = in.readBoolean(); - if (in.getVersion().onOrAfter(Version.V_2_18_0)) { + // TODO switch this to 2.X version this change will be released in after backport + if (in.getVersion().onOrAfter(Version.CURRENT)) { this.requestedActions = Settings.readSettingsFromStream(in); } else { this.requestedActions = Settings.EMPTY; @@ -247,10 +248,8 @@ This works for currently supported range notations (=,~) } out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); - if (requestedActions != null) { + if (out.getVersion().onOrAfter(Version.CURRENT)) { Settings.writeSettingsToStream(requestedActions, out); - } else { - Settings.writeSettingsToStream(Settings.EMPTY, out); } } From 394c47a59a390a949586d1c147e3cdeeeb6a22ec Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 19 Sep 2024 14:47:34 -0400 Subject: [PATCH 11/18] Handle case where requestedActions is null Signed-off-by: Craig Perkins --- server/src/main/java/org/opensearch/plugins/PluginInfo.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index c00fe0f92933e..34df3a21f4705 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -249,7 +249,11 @@ This works for currently supported range notations (=,~) out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); if (out.getVersion().onOrAfter(Version.CURRENT)) { - Settings.writeSettingsToStream(requestedActions, out); + if (requestedActions != null) { + Settings.writeSettingsToStream(requestedActions, out); + } else { + Settings.writeSettingsToStream(Settings.EMPTY, out); + } } } From 157740b2f0179581875c32c7becf8db4b35f4a14 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Sep 2024 10:09:58 -0400 Subject: [PATCH 12/18] Ensure requestedActions is non-null Signed-off-by: Craig Perkins --- server/src/main/java/org/opensearch/plugins/PluginInfo.java | 6 +----- .../main/java/org/opensearch/plugins/PluginsService.java | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index 34df3a21f4705..c00fe0f92933e 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -249,11 +249,7 @@ This works for currently supported range notations (=,~) out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); if (out.getVersion().onOrAfter(Version.CURRENT)) { - if (requestedActions != null) { - Settings.writeSettingsToStream(requestedActions, out); - } else { - Settings.writeSettingsToStream(Settings.EMPTY, out); - } + Settings.writeSettingsToStream(requestedActions, out); } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index cab25d57b23e3..2207e2aa20e5a 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -152,7 +152,7 @@ public PluginsService( null, Collections.emptyList(), false, - null + Settings.EMPTY ); if (logger.isTraceEnabled()) { logger.trace("plugin loaded from classpath [{}]", pluginInfo); From bf44a29a41db5bad1dddde58c1fd35338d22ff43 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Sep 2024 10:23:37 -0400 Subject: [PATCH 13/18] Simplify tests Signed-off-by: Craig Perkins --- distribution/build.gradle | 3 +- .../plugins/InstallPluginCommandTests.java | 69 ++----------------- .../opensearch/plugins/PluginsService.java | 10 --- 3 files changed, 8 insertions(+), 74 deletions(-) diff --git a/distribution/build.gradle b/distribution/build.gradle index 36efe2e0d45e8..7fb07bd08d87d 100644 --- a/distribution/build.gradle +++ b/distribution/build.gradle @@ -327,7 +327,6 @@ configure(subprojects.findAll { ['archives', 'packages'].contains(it.name) }) { dependencies { libs project(':server') - libs project(':libs:opensearch-plugin-classloader') libs project(':distribution:tools:java-version-checker') libs project(':distribution:tools:launchers') @@ -481,7 +480,7 @@ configure(subprojects.findAll { ['archives', 'packages'].contains(it.name) }) { } } } - + jreFiles = { Project project, String platform, String architecture -> return copySpec { /* diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java index 4a36947e508c1..c065d52c2c89f 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java @@ -77,14 +77,7 @@ import org.junit.After; import org.junit.Before; -import javax.tools.FileObject; -import javax.tools.ForwardingJavaFileManager; -import javax.tools.JavaCompiler; -import javax.tools.JavaFileManager; -import javax.tools.JavaFileObject; import javax.tools.SimpleJavaFileObject; -import javax.tools.StandardJavaFileManager; -import javax.tools.ToolProvider; import java.io.BufferedReader; import java.io.ByteArrayInputStream; @@ -92,7 +85,6 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.io.OutputStream; import java.io.StringReader; import java.net.MalformedURLException; import java.net.URI; @@ -247,13 +239,7 @@ static Path createPluginDir(Function temp) throws IOException { static void writeJar(Path jar, String... classes) throws IOException { try (ZipOutputStream stream = new ZipOutputStream(Files.newOutputStream(jar))) { for (String clazz : classes) { - clazz = clazz.replace('.', '/'); - ZipEntry entry = new ZipEntry(clazz + ".class"); - stream.putNextEntry(entry); // no package names, just support simple classes - Path compiledClassPath = jar.getParent().resolve(clazz + ".class"); - if (Files.exists(compiledClassPath)) { - Files.copy(compiledClassPath, stream); - } + stream.putNextEntry(new ZipEntry(clazz + ".class")); // no package names, just support simple classes } } } @@ -298,46 +284,7 @@ public CharSequence getCharContent(boolean ignoreEncodingErrors) { } } - private static String compileFakePlugin(Path structure) throws IOException { - String pluginClassName = "org.opensearch.plugins.FakePlugin"; - String javaSourceCode = "package org.opensearch.plugins;\n" + "\n" + "class FakePlugin extends Plugin {}\n"; - if (Files.notExists(structure)) { - Files.createDirectories(structure); - } - JavaCompiler compiler = ToolProvider.getSystemJavaCompiler(); - - StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(null, null, null); - JavaFileManager fileManager = new ForwardingJavaFileManager(standardFileManager) { - @Override - public JavaFileObject getJavaFileForOutput(Location location, String className, JavaFileObject.Kind kind, FileObject sibling) { - Path classFile = structure.resolve(className.replace('.', '/') + ".class"); - if (Files.notExists(classFile.getParent())) { - try { - Files.createDirectories(classFile.getParent()); - } catch (IOException e) { - throw new RuntimeException(e); - } - } - return new SimpleJavaFileObject(classFile.toUri(), kind) { - @Override - public OutputStream openOutputStream() throws IOException { - return Files.newOutputStream(classFile); - } - }; - } - }; - - JavaFileObject javaFileObject = new JavaSourceFromString(pluginClassName, javaSourceCode); - Iterable options = Arrays.asList("-d", structure.toUri().toString()); - JavaCompiler.CompilationTask task = compiler.getTask(null, fileManager, null, options, null, Arrays.asList(javaFileObject)); - boolean success = task.call(); - // Close the file manager - fileManager.close(); - return pluginClassName; - } - static void writePlugin(String name, Path structure, String... additionalProps) throws IOException { - String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( Stream.of( "description", @@ -351,18 +298,17 @@ static void writePlugin(String name, Path structure, String... additionalProps) "java.version", System.getProperty("java.specification.version"), "classname", - pluginClassName + "FakePlugin" ), Arrays.stream(additionalProps) ).toArray(String[]::new); PluginTestUtil.writePluginProperties(structure, properties); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; - writeJar(structure.resolve("plugin.jar"), className, pluginClassName); + writeJar(structure.resolve("plugin.jar"), className); } static void writePluginWithRequestedActions(String name, Path structure, String... additionalProps) throws IOException { - String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( Stream.of( "description", @@ -376,7 +322,7 @@ static void writePluginWithRequestedActions(String name, Path structure, String. "java.version", System.getProperty("java.specification.version"), "classname", - pluginClassName + "FakePlugin" ), Arrays.stream(additionalProps) ).toArray(String[]::new); @@ -384,11 +330,10 @@ static void writePluginWithRequestedActions(String name, Path structure, String. PluginTestUtil.writePluginProperties(structure, properties); writePluginPermissionsYaml(structure); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; - writeJar(structure.resolve("plugin.jar"), className, pluginClassName); + writeJar(structure.resolve("plugin.jar"), className); } static void writePlugin(String name, Path structure, SemverRange opensearchVersionRange, String... additionalProps) throws IOException { - String pluginClassName = compileFakePlugin(structure); String[] properties = Stream.concat( Stream.of( "description", @@ -402,13 +347,13 @@ static void writePlugin(String name, Path structure, SemverRange opensearchVersi "java.version", System.getProperty("java.specification.version"), "classname", - pluginClassName + "FakePlugin" ), Arrays.stream(additionalProps) ).toArray(String[]::new); PluginTestUtil.writePluginProperties(structure, properties); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; - writeJar(structure.resolve("plugin.jar"), className, pluginClassName); + writeJar(structure.resolve("plugin.jar"), className); } static Path createPlugin(String name, Path structure, SemverRange opensearchVersionRange, String... additionalProps) diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 2207e2aa20e5a..3833d69792332 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -325,12 +325,10 @@ public PluginsAndModules info() { // a "bundle" is a group of jars in a single classloader static class Bundle { final PluginInfo plugin; - final Path pluginDir; final Set urls; Bundle(PluginInfo plugin, Path dir) throws IOException { this.plugin = Objects.requireNonNull(plugin); - this.pluginDir = dir; Set urls = new LinkedHashSet<>(); // gather urls for jar files try (DirectoryStream jarStream = Files.newDirectoryStream(dir, "*.jar")) { @@ -820,14 +818,6 @@ private String signatureMessage(final Class clazz) { ); } - public Settings getRequestedActionsForPlugin(Plugin plugin) { - Optional> tuple = plugins.stream().filter(x -> x.v2().equals(plugin)).findFirst(); - if (tuple.isEmpty()) { - throw new IllegalStateException("Unable to find plugin in loaded plugins"); - } - return tuple.get().v1().getRequestedActions(); - } - public List> filterPluginTuples(Class type) { return plugins.stream().filter(x -> type.isAssignableFrom(x.v2().getClass())).collect(Collectors.toList()); } From 8b1605130bd023a8e9cd9902f1ee97ef5720491b Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Sep 2024 10:37:29 -0400 Subject: [PATCH 14/18] modify correct build.gradle Signed-off-by: Craig Perkins --- distribution/build.gradle | 1 + distribution/tools/plugin-cli/build.gradle | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/build.gradle b/distribution/build.gradle index 7fb07bd08d87d..131d6e8d27ffd 100644 --- a/distribution/build.gradle +++ b/distribution/build.gradle @@ -327,6 +327,7 @@ configure(subprojects.findAll { ['archives', 'packages'].contains(it.name) }) { dependencies { libs project(':server') + libs project(':libs:opensearch-plugin-classloader') libs project(':distribution:tools:java-version-checker') libs project(':distribution:tools:launchers') diff --git a/distribution/tools/plugin-cli/build.gradle b/distribution/tools/plugin-cli/build.gradle index df6c4cb693be4..784cdc457a1a9 100644 --- a/distribution/tools/plugin-cli/build.gradle +++ b/distribution/tools/plugin-cli/build.gradle @@ -39,7 +39,6 @@ dependencies { compileOnly project(":libs:opensearch-cli") api "org.bouncycastle:bcpg-fips:2.0.9" api "org.bouncycastle:bc-fips:2.0.0" - testRuntimeOnly project(':libs:opensearch-plugin-classloader') testImplementation project(":test:framework") testImplementation 'com.google.jimfs:jimfs:1.3.0' testRuntimeOnly("com.google.guava:guava:${versions.guava}") { From c12df2d72a772af1eee6779bf3a9918857518228 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Sep 2024 10:40:40 -0400 Subject: [PATCH 15/18] Remove unused code Signed-off-by: Craig Perkins --- distribution/build.gradle | 2 +- .../plugins/InstallPluginCommandTests.java | 15 --------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/distribution/build.gradle b/distribution/build.gradle index 131d6e8d27ffd..36efe2e0d45e8 100644 --- a/distribution/build.gradle +++ b/distribution/build.gradle @@ -481,7 +481,7 @@ configure(subprojects.findAll { ['archives', 'packages'].contains(it.name) }) { } } } - + jreFiles = { Project project, String platform, String architecture -> return copySpec { /* diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java index c065d52c2c89f..45ac16a4dcddf 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java @@ -270,20 +270,6 @@ static String createPluginWithRequestedActionsUrl(String name, Path structure, S return createPluginWithRequestedActions(name, structure, additionalProps).toUri().toURL().toString(); } - static class JavaSourceFromString extends SimpleJavaFileObject { - private final String code; - - public JavaSourceFromString(String className, String code) { - super(URI.create("string:///" + className.replace('.', '/') + Kind.SOURCE.extension), Kind.SOURCE); - this.code = code; - } - - @Override - public CharSequence getCharContent(boolean ignoreEncodingErrors) { - return code; - } - } - static void writePlugin(String name, Path structure, String... additionalProps) throws IOException { String[] properties = Stream.concat( Stream.of( @@ -302,7 +288,6 @@ static void writePlugin(String name, Path structure, String... additionalProps) ), Arrays.stream(additionalProps) ).toArray(String[]::new); - PluginTestUtil.writePluginProperties(structure, properties); String className = name.substring(0, 1).toUpperCase(Locale.ENGLISH) + name.substring(1) + "Plugin"; writeJar(structure.resolve("plugin.jar"), className); From abbb6f0a65def2e71ac7d8fe09e80fb6dfad6b25 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 20 Sep 2024 11:14:53 -0400 Subject: [PATCH 16/18] Remove unused import Signed-off-by: Craig Perkins --- .../java/org/opensearch/plugins/InstallPluginCommandTests.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java index 45ac16a4dcddf..268beb228ec19 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java @@ -77,8 +77,6 @@ import org.junit.After; import org.junit.Before; -import javax.tools.SimpleJavaFileObject; - import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; From 5b8c07990e702b4eedce0c06b8425ef3099548af Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 29 Oct 2024 15:30:37 -0400 Subject: [PATCH 17/18] Update ToXContent Signed-off-by: Craig Perkins --- .../plugins/InstallPluginCommand.java | 15 ++---- .../org/opensearch/plugins/PluginInfo.java | 47 ++++++++++++++++--- .../opensearch/plugins/PluginInfoTests.java | 36 ++++++++++++++ 3 files changed, 79 insertions(+), 19 deletions(-) diff --git a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java index 979327014efb0..b4e568313f7b4 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java +++ b/distribution/tools/plugin-cli/src/main/java/org/opensearch/plugins/InstallPluginCommand.java @@ -60,7 +60,6 @@ import org.opensearch.common.SuppressForbidden; import org.opensearch.common.collect.Tuple; import org.opensearch.common.hash.MessageDigests; -import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.io.IOUtils; import org.opensearch.env.Environment; @@ -103,10 +102,12 @@ import java.util.Objects; import java.util.Set; import java.util.TreeSet; -import java.util.function.Function; import java.util.stream.Collectors; import static org.opensearch.cli.Terminal.Verbosity.VERBOSE; +import static org.opensearch.plugins.PluginInfo.CLUSTER_ACTIONS_SETTING; +import static org.opensearch.plugins.PluginInfo.DESCRIPTION_SETTING; +import static org.opensearch.plugins.PluginInfo.INDEX_ACTIONS_SETTING; /** * A command for the plugin cli to install a plugin into opensearch. @@ -196,16 +197,6 @@ class InstallPluginCommand extends EnvironmentAwareCommand { static final Set PLUGIN_DIR_PERMS; static final Set PLUGIN_FILES_PERMS; - static final Setting> CLUSTER_ACTIONS_SETTING = Setting.listSetting( - "cluster.actions", - Collections.emptyList(), - Function.identity() - ); - - static final Setting INDEX_ACTIONS_SETTING = Setting.groupSetting("index.actions."); - - static final Setting DESCRIPTION_SETTING = Setting.simpleString("description"); - static { // Bin directory get chmod 755 BIN_DIR_PERMS = Collections.unmodifiableSet(PosixFilePermissions.fromString("rwxr-xr-x")); diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index c00fe0f92933e..7817dd0b9b6d8 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -38,6 +38,7 @@ import org.opensearch.Version; import org.opensearch.bootstrap.JarHell; import org.opensearch.common.annotation.PublicApi; +import org.opensearch.common.settings.Setting; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.json.JsonXContentParser; import org.opensearch.core.common.Strings; @@ -57,6 +58,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -80,6 +82,16 @@ public class PluginInfo implements Writeable, ToXContentObject { true ); + public static final Setting> CLUSTER_ACTIONS_SETTING = Setting.listSetting( + "cluster.actions", + Collections.emptyList(), + Function.identity() + ); + + public static final Setting INDEX_ACTIONS_SETTING = Setting.groupSetting("index.actions."); + + public static final Setting DESCRIPTION_SETTING = Setting.simpleString("description"); + private final String name; private final String description; private final String version; @@ -217,8 +229,7 @@ public PluginInfo(final StreamInput in) throws IOException { this.customFolderName = in.readString(); this.extendedPlugins = in.readStringList(); this.hasNativeController = in.readBoolean(); - // TODO switch this to 2.X version this change will be released in after backport - if (in.getVersion().onOrAfter(Version.CURRENT)) { + if (in.getVersion().onOrAfter(Version.V_2_19_0)) { this.requestedActions = Settings.readSettingsFromStream(in); } else { this.requestedActions = Settings.EMPTY; @@ -248,7 +259,7 @@ This works for currently supported range notations (=,~) } out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); - if (out.getVersion().onOrAfter(Version.CURRENT)) { + if (out.getVersion().onOrAfter(Version.V_2_19_0)) { Settings.writeSettingsToStream(requestedActions, out); } } @@ -510,12 +521,30 @@ public String getTargetFolderName() { } /** - * Returns actions requested by this plugin in the plugin-permissions.yml file + * Returns cluster actions requested by this plugin in the plugin-permissions.yml file + * + * @return A list of cluster actions contained within the plugin-permissions.yml file + */ + public List getClusterActions() { + return CLUSTER_ACTIONS_SETTING.get(requestedActions); + } + + /** + * Returns index actions requested by this plugin in the plugin-permissions.yml file * - * @return A settings object containing the contents of plugin-permissions.yml file + * @return A list of index actions contained within the plugin-permissions.yml file. This method returns a map + * of index pattern -> list of actions that apply on the index pattern */ - public Settings getRequestedActions() { - return requestedActions; + public Map> getIndexActions() { + final Map> indexActions = new HashMap<>(); + final Settings requestedIndexActionsGroup = INDEX_ACTIONS_SETTING.get(requestedActions); + if (!requestedIndexActionsGroup.keySet().isEmpty()) { + for (String indexPattern : requestedIndexActionsGroup.keySet()) { + List indexActionsForPattern = requestedIndexActionsGroup.getAsList(indexPattern); + indexActions.put(indexPattern, indexActionsForPattern); + } + } + return indexActions; } @Override @@ -531,6 +560,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field("custom_foldername", customFolderName); builder.field("extended_plugins", extendedPlugins); builder.field("has_native_controller", hasNativeController); + if (!Settings.EMPTY.equals(requestedActions)) { + builder.field("cluster.actions", getClusterActions()); + builder.field("index.actions", getIndexActions()); + } } builder.endObject(); diff --git a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java index 8e0932889ea95..192f8eda305a0 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java @@ -35,7 +35,9 @@ import com.fasterxml.jackson.core.JsonParseException; import org.opensearch.Version; +import org.opensearch.action.admin.cluster.health.ClusterHealthAction; import org.opensearch.action.admin.cluster.node.info.PluginsAndModules; +import org.opensearch.action.index.IndexAction; import org.opensearch.common.io.stream.BytesStreamOutput; import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.json.JsonXContent; @@ -48,6 +50,7 @@ import java.nio.ByteBuffer; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -387,6 +390,7 @@ public void testToXContent() throws Exception { ); XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); String prettyPrint = info.toXContent(builder, ToXContent.EMPTY_PARAMS).prettyPrint().toString(); + prettyPrint = Arrays.stream(prettyPrint.split("\n")).map(String::trim).collect(Collectors.joining("")); assertTrue(prettyPrint.contains("\"name\" : \"fake\"")); assertTrue(prettyPrint.contains("\"version\" : \"dummy\"")); assertTrue(prettyPrint.contains("\"opensearch_version\" : \"" + Version.CURRENT)); @@ -398,6 +402,38 @@ public void testToXContent() throws Exception { assertTrue(prettyPrint.contains("\"has_native_controller\" : false")); } + public void testToXContentWithRequestedActions() throws Exception { + PluginInfo info = new PluginInfo( + "fake", + "foo", + "dummy", + Version.CURRENT, + "1.8", + "dummyClass", + "folder", + Collections.emptyList(), + false, + Settings.builder() + .putList("cluster.actions", List.of(ClusterHealthAction.NAME)) + .putList("index.actions.my-index", List.of(IndexAction.NAME)) + .build() + ); + XContentBuilder builder = JsonXContent.contentBuilder().prettyPrint(); + String prettyPrint = info.toXContent(builder, ToXContent.EMPTY_PARAMS).prettyPrint().toString(); + prettyPrint = Arrays.stream(prettyPrint.split("\n")).map(String::trim).collect(Collectors.joining("")); + assertTrue(prettyPrint.contains("\"name\" : \"fake\"")); + assertTrue(prettyPrint.contains("\"version\" : \"dummy\"")); + assertTrue(prettyPrint.contains("\"opensearch_version\" : \"" + Version.CURRENT)); + assertTrue(prettyPrint.contains("\"java_version\" : \"1.8\"")); + assertTrue(prettyPrint.contains("\"description\" : \"foo\"")); + assertTrue(prettyPrint.contains("\"classname\" : \"dummyClass\"")); + assertTrue(prettyPrint.contains("\"custom_foldername\" : \"folder\"")); + assertTrue(prettyPrint.contains("\"extended_plugins\" : [ ]")); + assertTrue(prettyPrint.contains("\"has_native_controller\" : false")); + assertTrue(prettyPrint.contains("\"cluster.actions\" : [\"" + ClusterHealthAction.NAME + "\"]")); + assertTrue(prettyPrint.contains("\"index.actions\" : {\"my-index\" : [\"" + IndexAction.NAME + "\"]}")); + } + public void testPluginListSorted() { List plugins = new ArrayList<>(); plugins.add(new PluginInfo("c", "foo", "dummy", Version.CURRENT, "1.8", "dummyclass", Collections.emptyList(), randomBoolean())); From ddeb16bfdd4d3ecc1172747cb0ad3dcf6eca1b3a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 29 Oct 2024 16:56:16 -0400 Subject: [PATCH 18/18] Set to CURRENT until backport Signed-off-by: Craig Perkins --- server/src/main/java/org/opensearch/plugins/PluginInfo.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index 7817dd0b9b6d8..35ebd3d82f852 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -229,7 +229,7 @@ public PluginInfo(final StreamInput in) throws IOException { this.customFolderName = in.readString(); this.extendedPlugins = in.readStringList(); this.hasNativeController = in.readBoolean(); - if (in.getVersion().onOrAfter(Version.V_2_19_0)) { + if (in.getVersion().onOrAfter(Version.CURRENT)) { this.requestedActions = Settings.readSettingsFromStream(in); } else { this.requestedActions = Settings.EMPTY; @@ -259,7 +259,7 @@ This works for currently supported range notations (=,~) } out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); - if (out.getVersion().onOrAfter(Version.V_2_19_0)) { + if (out.getVersion().onOrAfter(Version.CURRENT)) { Settings.writeSettingsToStream(requestedActions, out); } }