Skip to content

Commit

Permalink
Updated feature flag name
Browse files Browse the repository at this point in the history 
Signed-off-by: Abhilasha Seth <[email protected]>
  • Loading branch information
@abseth-amzn
abseth-amzn committed Nov 28, 2023
1 parent f953b1e commit 4b2dffc
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 50 deletions.
10 changes: 6 additions & 4 deletions ...tion/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<Path, Environment> 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<Path, Environment> env = createEnv(fs, temp);
Path pluginDir = createPluginDir(temp);
Version coreVersion = Version.CURRENT;
Expand Down
8 changes: 4 additions & 4 deletions ...bution/tools/plugin-cli/src/test/java/org/opensearch/plugins/ListPluginsCommandTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
Expand All @@ -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()
);
Expand All @@ -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",
Expand All @@ -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()
);
Expand Down
42 changes: 23 additions & 19 deletions server/src/main/java/org/opensearch/plugins/PluginInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public class PluginInfo implements Writeable, ToXContentObject {
private final String customFolderName;
private final List<String> extendedPlugins;
private final boolean hasNativeController;
private final boolean isSemVerRangeCompatible;
private final boolean compatibleAcrossPatchVersions;

/**
* Construct plugin info.
Expand All @@ -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,
Expand All @@ -101,7 +101,7 @@ public PluginInfo(
String customFolderName,
List<String> extendedPlugins,
boolean hasNativeController,
boolean isSemVerRangeCompatible
boolean compatibleAcrossPatchVersions
) {
this.name = name;
this.description = description;
Expand All @@ -112,7 +112,7 @@ public PluginInfo(
this.customFolderName = customFolderName;
this.extendedPlugins = Collections.unmodifiableList(extendedPlugins);
this.hasNativeController = hasNativeController;
this.isSemVerRangeCompatible = isSemVerRangeCompatible;
this.compatibleAcrossPatchVersions = compatibleAcrossPatchVersions;
}

/**
Expand All @@ -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,
Expand All @@ -137,7 +137,7 @@ public PluginInfo(
String classname,
List<String> extendedPlugins,
boolean hasNativeController,
boolean isSemVerRangeCompatible
boolean compatibleAcrossPatchVersions
) {
this(
name,
Expand All @@ -149,7 +149,7 @@ public PluginInfo(
null /* customFolderName */,
extendedPlugins,
hasNativeController,
isSemVerRangeCompatible
compatibleAcrossPatchVersions
);
}

Expand All @@ -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
Expand All @@ -187,7 +187,7 @@ public void writeTo(final StreamOutput out) throws IOException {
}
out.writeStringCollection(extendedPlugins);
out.writeBoolean(hasNativeController);
out.writeBoolean(isSemVerRangeCompatible);
out.writeBoolean(compatibleAcrossPatchVersions);
}

/**
Expand Down Expand Up @@ -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());
Expand All @@ -269,7 +273,7 @@ public static PluginInfo readFromProperties(final Path path) throws IOException
customFolderName,
extendedPlugins,
hasNativeController,
isSemVerRangeCompatible
compatibleAcrossPatchVersions
);
}

Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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();

Expand Down Expand Up @@ -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();
}
}
4 changes: 2 additions & 2 deletions server/src/main/java/org/opensearch/plugins/PluginsService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 7 additions & 7 deletions server/src/test/java/org/opensearch/plugins/PluginInfoTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down
58 changes: 44 additions & 14 deletions server/src/test/java/org/opensearch/plugins/PluginsServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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(
Expand All @@ -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",
Expand Down

0 comments on commit 4b2dffc

Please sign in to comment.