From 4b2dffc5d1688bcf7056de3dff28f761b8668c64 Mon Sep 17 00:00:00 2001 From: Abhilasha Seth Date: Thu, 3 Aug 2023 17:35:17 +0530 Subject: [PATCH] Updated feature flag name Signed-off-by: Abhilasha Seth --- .../plugins/InstallPluginCommandTests.java | 10 ++-- .../plugins/ListPluginsCommandTests.java | 8 +-- .../org/opensearch/plugins/PluginInfo.java | 42 ++++++++------ .../opensearch/plugins/PluginsService.java | 4 +- .../opensearch/plugins/PluginInfoTests.java | 14 ++--- .../plugins/PluginsServiceTests.java | 58 ++++++++++++++----- 6 files changed, 86 insertions(+), 50 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 4e82a91c68663..4ed4006a5132e 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 @@ -877,18 +877,20 @@ public void testInstallMisspelledOfficialPlugins() throws Exception { assertThat(e.getMessage(), containsString("Unknown plugin unknown_plugin")); } - public void testInstallPluginDifferingInPatchVersionAllowed() throws Exception { + public void testInstallPluginWithDifferentPatchVersionIfPluginOptsIn() throws Exception { Tuple env = createEnv(fs, temp); Path pluginDir = createPluginDir(temp); Version coreVersion = Version.CURRENT; Version pluginVersion = VersionUtils.getVersion(coreVersion.major, coreVersion.minor, (byte) (coreVersion.revision + 1)); - // Plugin explicitly specifies semVer range compatibility so patch version is not checked - String pluginZip = createPlugin("fake", pluginDir, pluginVersion, "is.semVer.range.compatible", "true").toUri().toURL().toString(); + // Plugin explicitly specifies compatibility across patch versions + String pluginZip = createPlugin("fake", pluginDir, pluginVersion, "compatible.across.patch.versions", "true").toUri() + .toURL() + .toString(); skipJarHellCommand.execute(terminal, Collections.singletonList(pluginZip), false, env.v2()); assertThat(terminal.getOutput(), containsString("100%")); } - public void testInstallPluginDifferingInPatchVersionNotAllowed() throws Exception { + public void testInstallPluginWithDifferentPatchVersionNotAllowedByDefault() throws Exception { Tuple env = createEnv(fs, temp); Path pluginDir = createPluginDir(temp); Version coreVersion = Version.CURRENT; diff --git a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/ListPluginsCommandTests.java b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/ListPluginsCommandTests.java index 864dbc3482c0e..f63ed9f62c4d1 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/ListPluginsCommandTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/opensearch/plugins/ListPluginsCommandTests.java @@ -173,7 +173,7 @@ public void testPluginWithVerbose() throws Exception { "Extended Plugins: []", " * Classname: org.fake", "Folder name: custom-folder", - "SemVer Range Compatible: false" + "Compatible across patch versions: false" ), terminal.getOutput() ); @@ -197,7 +197,7 @@ public void testPluginWithNativeController() throws Exception { "Extended Plugins: []", " * Classname: org.fake", "Folder name: custom-folder", - "SemVer Range Compatible: false" + "Compatible across patch versions: false" ), terminal.getOutput() ); @@ -222,7 +222,7 @@ public void testPluginWithVerboseMultiplePlugins() throws Exception { "Extended Plugins: []", " * Classname: org.fake", "Folder name: custom-folder", - "SemVer Range Compatible: false", + "Compatible across patch versions: false", "fake_plugin2", "- Plugin information:", "Name: fake_plugin2", @@ -234,7 +234,7 @@ public void testPluginWithVerboseMultiplePlugins() throws Exception { "Extended Plugins: []", " * Classname: org.fake2", "Folder name: custom-folder", - "SemVer Range Compatible: false" + "Compatible across patch versions: false" ), terminal.getOutput() ); diff --git a/server/src/main/java/org/opensearch/plugins/PluginInfo.java b/server/src/main/java/org/opensearch/plugins/PluginInfo.java index e5749cdd21cd6..3df073e6f1c5f 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginInfo.java +++ b/server/src/main/java/org/opensearch/plugins/PluginInfo.java @@ -75,7 +75,7 @@ public class PluginInfo implements Writeable, ToXContentObject { private final String customFolderName; private final List extendedPlugins; private final boolean hasNativeController; - private final boolean isSemVerRangeCompatible; + private final boolean compatibleAcrossPatchVersions; /** * Construct plugin info. @@ -89,7 +89,7 @@ public class PluginInfo implements Writeable, ToXContentObject { * @param customFolderName the custom folder name for the plugin * @param extendedPlugins other plugins this plugin extends through SPI * @param hasNativeController whether or not the plugin has a native controller - * @param isSemVerRangeCompatible whether or not the plugin specifies a semVer range of compatible versions + * @param compatibleAcrossPatchVersions whether or not the plugin is compatible across patch versions */ public PluginInfo( String name, @@ -101,7 +101,7 @@ public PluginInfo( String customFolderName, List extendedPlugins, boolean hasNativeController, - boolean isSemVerRangeCompatible + boolean compatibleAcrossPatchVersions ) { this.name = name; this.description = description; @@ -112,7 +112,7 @@ public PluginInfo( this.customFolderName = customFolderName; this.extendedPlugins = Collections.unmodifiableList(extendedPlugins); this.hasNativeController = hasNativeController; - this.isSemVerRangeCompatible = isSemVerRangeCompatible; + this.compatibleAcrossPatchVersions = compatibleAcrossPatchVersions; } /** @@ -126,7 +126,7 @@ public PluginInfo( * @param classname the entry point to the plugin * @param extendedPlugins other plugins this plugin extends through SPI * @param hasNativeController whether or not the plugin has a native controller - * @param isSemVerRangeCompatible whether or not the plugin specifies a semVer range of compatible versions + * @param compatibleAcrossPatchVersions whether or not the plugin is compatible with all patch versions */ public PluginInfo( String name, @@ -137,7 +137,7 @@ public PluginInfo( String classname, List extendedPlugins, boolean hasNativeController, - boolean isSemVerRangeCompatible + boolean compatibleAcrossPatchVersions ) { this( name, @@ -149,7 +149,7 @@ public PluginInfo( null /* customFolderName */, extendedPlugins, hasNativeController, - isSemVerRangeCompatible + compatibleAcrossPatchVersions ); } @@ -169,7 +169,7 @@ public PluginInfo(final StreamInput in) throws IOException { this.customFolderName = in.readString(); this.extendedPlugins = in.readStringList(); this.hasNativeController = in.readBoolean(); - this.isSemVerRangeCompatible = in.readBoolean(); + this.compatibleAcrossPatchVersions = in.readBoolean(); } @Override @@ -187,7 +187,7 @@ public void writeTo(final StreamOutput out) throws IOException { } out.writeStringCollection(extendedPlugins); out.writeBoolean(hasNativeController); - out.writeBoolean(isSemVerRangeCompatible); + out.writeBoolean(compatibleAcrossPatchVersions); } /** @@ -252,8 +252,12 @@ public static PluginInfo readFromProperties(final Path path) throws IOException final String hasNativeControllerValue = propsMap.remove("has.native.controller"); final boolean hasNativeController = getBooleanProperty("has.native.controller", hasNativeControllerValue, false); - final String isSemVerRangeCompatibleValue = propsMap.remove("is.semVer.range.compatible"); - final boolean isSemVerRangeCompatible = getBooleanProperty("is.semVer.range.compatible", isSemVerRangeCompatibleValue, false); + final String compatibleAcrossPatchVersionsValue = propsMap.remove("compatible.across.patch.versions"); + final boolean compatibleAcrossPatchVersions = getBooleanProperty( + "compatible.across.patch.versions", + compatibleAcrossPatchVersionsValue, + false + ); if (propsMap.isEmpty() == false) { throw new IllegalArgumentException("Unknown properties in plugin descriptor: " + propsMap.keySet()); @@ -269,7 +273,7 @@ public static PluginInfo readFromProperties(final Path path) throws IOException customFolderName, extendedPlugins, hasNativeController, - isSemVerRangeCompatible + compatibleAcrossPatchVersions ); } @@ -377,11 +381,11 @@ public boolean hasNativeController() { } /** - * Whether or not the plugin specifies a semVer range of compatible versions. - * @return {@code true} if the plugin specifies a semVer range of compatible versions, {@code false} otherwise. + * Whether or not the plugin is compatible with all patch versions for given opensearch version + * @return {@code true} if the plugin is compatible with all patch versions, {@code false} otherwise. */ - public boolean isSemVerRangeCompatible() { - return isSemVerRangeCompatible; + public boolean compatibleAcrossPatchVersions() { + return compatibleAcrossPatchVersions; } /** @@ -406,7 +410,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field("custom_foldername", customFolderName); builder.field("extended_plugins", extendedPlugins); builder.field("has_native_controller", hasNativeController); - builder.field("is_semVer_range_compatible", isSemVerRangeCompatible); + builder.field("compatible_across_patch_versions", compatibleAcrossPatchVersions); } builder.endObject(); @@ -477,8 +481,8 @@ public String toString(String prefix) { .append(customFolderName) .append("\n") .append(prefix) - .append("SemVer Range Compatible: ") - .append(isSemVerRangeCompatible); + .append("Compatible across patch versions: ") + .append(compatibleAcrossPatchVersions); return information.toString(); } } diff --git a/server/src/main/java/org/opensearch/plugins/PluginsService.java b/server/src/main/java/org/opensearch/plugins/PluginsService.java index 55e0914aaa66c..ea38a37d4b186 100644 --- a/server/src/main/java/org/opensearch/plugins/PluginsService.java +++ b/server/src/main/java/org/opensearch/plugins/PluginsService.java @@ -410,8 +410,8 @@ static void verifyCompatibility(PluginInfo info) { */ static boolean isPluginVersionCompatible(final PluginInfo pluginInfo, final Version coreVersion) { final Version pluginVersion = pluginInfo.getOpenSearchVersion(); - if (pluginInfo.isSemVerRangeCompatible()) { - // Ignore patch version if plugin is specifying semVer range compatibility + if (pluginInfo.compatibleAcrossPatchVersions()) { + // Ignore patch version if plugin is compatible across patch versions return pluginVersion.major == coreVersion.major && pluginVersion.minor == coreVersion.minor; } return pluginVersion.equals(coreVersion); diff --git a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java index 435ffce672656..e6432574f9179 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginInfoTests.java @@ -75,7 +75,7 @@ public void testReadFromProperties() throws Exception { assertEquals("1.0", info.getVersion()); assertEquals("FakePlugin", info.getClassname()); assertThat(info.getExtendedPlugins(), empty()); - assertEquals(info.isSemVerRangeCompatible(), false); + assertEquals(info.compatibleAcrossPatchVersions(), false); } public void testReadFromPropertiesWithFolderNameAndVersionAfter() throws Exception { @@ -288,7 +288,7 @@ public void testExtendedPluginsEmpty() throws Exception { assertThat(info.getExtendedPlugins(), empty()); } - public void testReadFromPropertiesSemVerRangeCompatible() throws Exception { + public void testReadFromPropertiesPluginCompatibleAcrossPatchVersions() throws Exception { Path pluginDir = createTempDir().resolve("fake-plugin"); PluginTestUtil.writePluginProperties( pluginDir, @@ -304,14 +304,14 @@ public void testReadFromPropertiesSemVerRangeCompatible() throws Exception { System.getProperty("java.specification.version"), "classname", "FakePlugin", - "is.semVer.range.compatible", + "compatible.across.patch.versions", "true" ); PluginInfo info = PluginInfo.readFromProperties(pluginDir); - assertEquals(true, info.isSemVerRangeCompatible()); + assertEquals(true, info.compatibleAcrossPatchVersions()); } - public void testReadFromPropertiesSemVerRangeNotCompatible() throws Exception { + public void testReadFromPropertiesPluginNotCompatibleAcrossPatchVersions() throws Exception { Path pluginDir = createTempDir().resolve("fake-plugin"); PluginTestUtil.writePluginProperties( pluginDir, @@ -327,11 +327,11 @@ public void testReadFromPropertiesSemVerRangeNotCompatible() throws Exception { System.getProperty("java.specification.version"), "classname", "FakePlugin", - "is.semVer.range.compatible", + "compatible.across.patch.versions", "false" ); PluginInfo info = PluginInfo.readFromProperties(pluginDir); - assertEquals(false, info.isSemVerRangeCompatible()); + assertEquals(false, info.compatibleAcrossPatchVersions()); } public void testSerialize() throws Exception { diff --git a/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java index c9ae3ebaac934..23b5ecc88248a 100644 --- a/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java @@ -790,8 +790,8 @@ public void testIncompatibleOpenSearchVersion() throws Exception { assertThat(e.getMessage(), containsString("was built for OpenSearch version 6.0.0")); } - public void testPluginCompatibleWhenSemVerRangeNotSpecified() throws Exception { - // When semVer range is not specified, plugin is required to have same version as the core (Version.CURRENT) + public void testPluginCompatibilityDefaultBehavior() throws Exception { + // Plugin is required to have same version as the core (Version.CURRENT) PluginInfo info = new PluginInfo( "my_plugin", "desc", @@ -806,8 +806,8 @@ public void testPluginCompatibleWhenSemVerRangeNotSpecified() throws Exception { PluginsService.verifyCompatibility(info); } - public void testPluginIncompatibleWhenSemVerRangeNotSpecified() throws Exception { - // When semVer range is not specified, differing patch version is not allowed + public void testPluginIncompatibleIfPatchVersionMismatches() throws Exception { + // Differing patch version is not allowed by default Version coreVersion = Version.CURRENT; Version pluginVersion = VersionUtils.getVersion(coreVersion.major, coreVersion.minor, (byte) (coreVersion.revision + 1)); PluginInfo info = new PluginInfo( @@ -825,34 +825,64 @@ public void testPluginIncompatibleWhenSemVerRangeNotSpecified() throws Exception assertThat(e.getMessage(), containsString("was built for OpenSearch version")); } - public void testPluginCompatibilityWhenSemVerRangeSpecified() { + public void testPluginCompatibilityWhenPluginOptsIntoPatchVersionVariability() { // Compatible plugin and core versions - assertTrue(PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.0.0"), Version.fromString("1.0.0"))); - assertTrue(PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.0.0"), Version.fromString("1.0.1"))); - assertTrue(PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.0.1"), Version.fromString("1.0.0"))); + assertTrue( + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.0.0"), + Version.fromString("1.0.0") + ) + ); + assertTrue( + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.0.0"), + Version.fromString("1.0.1") + ) + ); + assertTrue( + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.0.1"), + Version.fromString("1.0.0") + ) + ); // Incompatible plugin and core versions // Different minor versions assertFalse( - PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.0.0"), Version.fromString("1.1.0")) + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.0.0"), + Version.fromString("1.1.0") + ) ); assertFalse( - PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.1.0"), Version.fromString("1.0.0")) + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.1.0"), + Version.fromString("1.0.0") + ) ); // Different major versions assertFalse( - PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.0.0"), Version.fromString("2.0.0")) + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.0.0"), + Version.fromString("2.0.0") + ) ); assertFalse( - PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("2.0.0"), Version.fromString("1.0.0")) + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("2.0.0"), + Version.fromString("1.0.0") + ) ); // Different major and minor versions assertFalse( - PluginsService.isPluginVersionCompatible(getSemverCompatiblePluginInfoForVersion("1.2.0"), Version.fromString("2.1.0")) + PluginsService.isPluginVersionCompatible( + getPluginInfoWithCompatibilityAcrossPatchVersions("1.2.0"), + Version.fromString("2.1.0") + ) ); } - private PluginInfo getSemverCompatiblePluginInfoForVersion(String version) { + private PluginInfo getPluginInfoWithCompatibilityAcrossPatchVersions(String version) { return new PluginInfo( "my_plugin", "desc",