diff --git a/buildSrc/src/main/groovy/org/opensearch/gradle/plugin/PluginBuildPlugin.groovy b/buildSrc/src/main/groovy/org/opensearch/gradle/plugin/PluginBuildPlugin.groovy index ad4bdb3258fcc..e82afdcf46288 100644 --- a/buildSrc/src/main/groovy/org/opensearch/gradle/plugin/PluginBuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/opensearch/gradle/plugin/PluginBuildPlugin.groovy @@ -113,8 +113,15 @@ class PluginBuildPlugin implements Plugin { 'customFolderName' : extension1.customFolderName, 'extendedPlugins' : extension1.extendedPlugins.join(','), 'hasNativeController' : extension1.hasNativeController, - 'requiresKeystore' : extension1.requiresKeystore + 'requiresKeystore' : extension1.requiresKeystore, + 'dependencies' : extension1.dependencies ] + + // Clear opensearch version if dependencies are specified + if (extension1.dependencies != null && !extension1.dependencies.trim().isEmpty()) { + properties.put('opensearchVersion', ''); + } + project.tasks.named('pluginProperties').configure { expand(properties) inputs.properties(properties) diff --git a/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java b/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java index d6117923973fa..da809a712cfc8 100644 --- a/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java +++ b/buildSrc/src/main/java/org/opensearch/gradle/plugin/PluginPropertiesExtension.java @@ -53,6 +53,8 @@ public class PluginPropertiesExtension { private String customFolderName = ""; + private String dependencies = ""; + /** Other plugins this plugin extends through SPI */ private List extendedPlugins = new ArrayList<>(); @@ -86,6 +88,14 @@ public void setCustomFolderName(String customFolderName) { this.customFolderName = customFolderName; } + public String getDependencies() { + return dependencies; + } + + public void setDependencies(String dependencies) { + this.dependencies = dependencies; + } + public String getName() { return name == null ? project.getName() : name; } diff --git a/buildSrc/src/main/resources/plugin-descriptor.properties b/buildSrc/src/main/resources/plugin-descriptor.properties index 885eb99ee3acb..525c7f65f9f87 100644 --- a/buildSrc/src/main/resources/plugin-descriptor.properties +++ b/buildSrc/src/main/resources/plugin-descriptor.properties @@ -59,3 +59,6 @@ extended.plugins=${extendedPlugins} # # 'has.native.controller': whether or not the plugin has a native controller has.native.controller=${hasNativeController} +# +# 'dependencies': any dependencies specified by the plugin +dependencies=${dependencies} diff --git a/buildSrc/src/test/java/org/opensearch/gradle/plugin/PluginBuildPluginTests.java b/buildSrc/src/test/java/org/opensearch/gradle/plugin/PluginBuildPluginTests.java index fa0693a258222..19a113a0c3178 100644 --- a/buildSrc/src/test/java/org/opensearch/gradle/plugin/PluginBuildPluginTests.java +++ b/buildSrc/src/test/java/org/opensearch/gradle/plugin/PluginBuildPluginTests.java @@ -39,8 +39,8 @@ import org.gradle.api.tasks.bundling.AbstractArchiveTask; import org.gradle.testfixtures.ProjectBuilder; import org.junit.Before; -import org.junit.Ignore; +import java.util.Map; import java.util.stream.Collectors; import org.mockito.Mockito; @@ -72,7 +72,6 @@ public void testApply() { }); } - @Ignore("https://github.com/elastic/elasticsearch/issues/47123") public void testApplyWithAfterEvaluate() { project.getExtensions().getExtraProperties().set("bwcVersions", Mockito.mock(BwcVersions.class)); project.getPlugins().apply(PluginBuildPlugin.class); @@ -88,5 +87,16 @@ public void testApplyWithAfterEvaluate() { "Task to generate notice not created: " + project.getTasks().stream().map(Task::getPath).collect(Collectors.joining(", ")), project.getTasks().findByName("generateNotice") ); + Map taskProperties = project.getTasks().findByName("pluginProperties").getInputs().getProperties(); + assertTrue(taskProperties.containsKey("description")); + assertTrue(taskProperties.containsKey("version")); + assertTrue(taskProperties.containsKey("name")); + assertTrue(taskProperties.containsKey("classname")); + assertTrue(taskProperties.containsKey("javaVersion")); + assertTrue(taskProperties.containsKey("opensearchVersion")); + assertTrue(taskProperties.containsKey("customFolderName")); + assertTrue(taskProperties.containsKey("extendedPlugins")); + assertTrue(taskProperties.containsKey("hasNativeController")); + assertTrue(taskProperties.containsKey("dependencies")); } } 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 e6c1070f36066..626c46547a33d 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 @@ -277,7 +277,9 @@ static void writePlugin(String name, Path structure, String... additionalProps) "java.version", System.getProperty("java.specification.version"), "classname", - "FakePlugin" + "FakePlugin", + "dependencies", + "" ), Arrays.stream(additionalProps) ).toArray(String[]::new); @@ -300,7 +302,9 @@ static void writePlugin(String name, Path structure, SemverRange opensearchVersi "java.version", System.getProperty("java.specification.version"), "classname", - "FakePlugin" + "FakePlugin", + "opensearch.version", + "" ), Arrays.stream(additionalProps) ).toArray(String[]::new); diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index 323e061aea567..374390bd13f7a 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -289,19 +289,19 @@ public static PluginInfo readFromProperties(final Path path) throws IOException final String opensearchVersionString = propsMap.remove("opensearch.version"); final String dependenciesValue = propsMap.remove("dependencies"); - if (opensearchVersionString == null && dependenciesValue == null) { + if (isBlank(opensearchVersionString) && isBlank(dependenciesValue)) { throw new IllegalArgumentException( "Either [opensearch.version] or [dependencies] property must be specified for the plugin [" + name + "]" ); } - if (opensearchVersionString != null && dependenciesValue != null) { + if (!isBlank(opensearchVersionString) && !isBlank(dependenciesValue)) { throw new IllegalArgumentException( "Only one of [opensearch.version] or [dependencies] property can be specified for the plugin [" + name + "]" ); } final List opensearchVersionRanges = new ArrayList<>(); - if (opensearchVersionString != null) { + if (!isBlank(opensearchVersionString)) { opensearchVersionRanges.add(SemverRange.fromString(opensearchVersionString)); } else { Map dependenciesMap; @@ -592,4 +592,8 @@ public String toString(String prefix) { .append(customFolderName); return information.toString(); } + + private static boolean isBlank(String s) { + return s == null || s.trim().isEmpty(); + } } diff --git a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java index 76294d85c64d4..697249c6c6d9a 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java @@ -176,6 +176,68 @@ public void testReadFromPropertiesOpenSearchVersionAndDependenciesMissing() thro ); } + public void testReadFromPropertiesOpenSearchVersionEmpty() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + PluginTestUtil.writePluginProperties( + pluginDir, + "description", + "fake desc", + "opensearch.version", + "", + "name", + "my_plugin", + "version", + "1.0" + ); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginInfo.readFromProperties(pluginDir)); + assertThat( + e.getMessage(), + containsString("Either [opensearch.version] or [dependencies] property must be specified for the plugin ") + ); + } + + public void testReadFromPropertiesDependenciesEmpty() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + PluginTestUtil.writePluginProperties( + pluginDir, + "description", + "fake desc", + "dependencies", + "", + "name", + "my_plugin", + "version", + "1.0" + ); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginInfo.readFromProperties(pluginDir)); + assertThat( + e.getMessage(), + containsString("Either [opensearch.version] or [dependencies] property must be specified for the plugin ") + ); + } + + public void testReadFromPropertiesOpenSearchVersionAndDependenciesEmpty() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + PluginTestUtil.writePluginProperties( + pluginDir, + "description", + "fake desc", + "opensearch.version", + "", + "dependencies", + "", + "name", + "my_plugin", + "version", + "1.0" + ); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginInfo.readFromProperties(pluginDir)); + assertThat( + e.getMessage(), + containsString("Either [opensearch.version] or [dependencies] property must be specified for the plugin ") + ); + } + public void testReadFromPropertiesWithDependenciesAndOpenSearchVersion() throws Exception { Path pluginDir = createTempDir().resolve("fake-plugin"); PluginTestUtil.writePluginProperties( @@ -202,6 +264,62 @@ public void testReadFromPropertiesWithDependenciesAndOpenSearchVersion() throws ); } + public void testReadFromPropertiesDependenciesEmptyOpenSearchVersionNotEmpty() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + PluginTestUtil.writePluginProperties( + pluginDir, + "description", + "fake desc", + "name", + "my_plugin", + "version", + "1.0", + "opensearch.version", + Version.CURRENT.toString(), + "java.version", + System.getProperty("java.specification.version"), + "classname", + "FakePlugin", + "dependencies", + "" + ); + PluginInfo info = PluginInfo.readFromProperties(pluginDir); + assertEquals("my_plugin", info.getName()); + assertEquals("fake desc", info.getDescription()); + assertEquals("1.0", info.getVersion()); + assertEquals("FakePlugin", info.getClassname()); + assertEquals(Version.CURRENT.toString(), info.getOpenSearchVersionRanges().get(0).toString()); + assertThat(info.getExtendedPlugins(), empty()); + } + + public void testReadFromPropertiesOpenSearchVersionEmptyDependenciesNotEmpty() throws Exception { + Path pluginDir = createTempDir().resolve("fake-plugin"); + PluginTestUtil.writePluginProperties( + pluginDir, + "description", + "fake desc", + "name", + "my_plugin", + "version", + "1.0", + "dependencies", + "{opensearch:\"~" + Version.CURRENT.toString() + "\"}", + "java.version", + System.getProperty("java.specification.version"), + "classname", + "FakePlugin", + "opensearch.version", + "" + ); + PluginInfo info = PluginInfo.readFromProperties(pluginDir); + assertEquals("my_plugin", info.getName()); + assertEquals("fake desc", info.getDescription()); + assertEquals("1.0", info.getVersion()); + assertEquals("FakePlugin", info.getClassname()); + assertEquals("~" + Version.CURRENT.toString(), info.getOpenSearchVersionRanges().get(0).toString()); + assertThat(info.getExtendedPlugins(), empty()); + } + public void testReadFromPropertiesJavaVersionMissing() throws Exception { Path pluginDir = createTempDir().resolve("fake-plugin"); PluginTestUtil.writePluginProperties( @@ -609,7 +727,7 @@ public void testInvalidRangeInDependencies() throws Exception { expectThrows(NumberFormatException.class, () -> PluginInfo.readFromProperties(pluginDir)); } - public void testhMultipleOpenSearchRangesInDependencies() throws Exception { + public void testMultipleOpenSearchRangesInDependencies() throws Exception { Path pluginDir = createTempDir().resolve("fake-plugin"); PluginTestUtil.writePluginProperties( pluginDir,