Add bwc tests and restrict range list size to 1

Signed-off-by: Abhilasha Seth <[email protected]>

libs/core/src/main/java/org/opensearch/semver/SemverRange.java

@@ -12,6 +12,7 @@
 import org.opensearch.common.Nullable;
 import org.opensearch.core.xcontent.ToXContentFragment;
 import org.opensearch.core.xcontent.XContentBuilder;
+import org.opensearch.semver.expr.Caret;
 import org.opensearch.semver.expr.Equal;
 import org.opensearch.semver.expr.Expression;
 import org.opensearch.semver.expr.Tilde;
@@ -42,7 +43,7 @@ public SemverRange(final Version rangeVersion, final RangeOperator rangeOperator
      */
     public static SemverRange fromString(final String range) {
         RangeOperator rangeOperator = RangeOperator.fromRange(range);
-        String version = range.replaceFirst(rangeOperator.asString(), "");
+        String version = range.replaceFirst(rangeOperator.asEscapedString(), "");
         if (!Version.stringHasLength(version)) {
             throw new IllegalArgumentException("Version cannot be empty");
         }
@@ -120,6 +121,7 @@ public enum RangeOperator {
 
         EQ("=", new Equal()),
         TILDE("~", new Tilde()),
+        CARET("^", new Caret()),
         DEFAULT("", new Equal());
 
         private final String operator;
@@ -139,6 +141,19 @@ public String asString() {
             return operator;
         }
 
+        /**
+         * Escaped string representation of the range operator,
+         * if operator is a regex character.
+         *
+         * @return range operator as escaped string, if operator is a regex character
+         */
+        public String asEscapedString() {
+            if (Objects.equals(operator, "^")) {
+                return "\\^";
+            }
+            return operator;
+        }
+
         public static RangeOperator fromRange(final String range) {
             Optional<RangeOperator> rangeOperator = stream(values()).filter(
                 operator -> operator != DEFAULT && range.startsWith(operator.asString())

libs/core/src/main/java/org/opensearch/semver/expr/Caret.java

@@ -0,0 +1,32 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.semver.expr;
+
+import org.opensearch.Version;
+
+/**
+ * Expression to evaluate version compatibility allowing for minor and patch version variability.
+ */
+public class Caret implements Expression {
+
+    /**
+     * Checks if the given version is compatible with the range version allowing for minor and
+     * patch version variability.
+     * Allows all versions starting from the rangeVersion upto next major version (exclusive).
+     * @param rangeVersion the version specified in range
+     * @param versionToEvaluate the version to evaluate
+     * @return {@code true} if the versions are compatible {@code false} otherwise
+     */
+    @Override
+    public boolean evaluate(final Version rangeVersion, final Version versionToEvaluate) {
+        Version lower = rangeVersion;
+        Version upper = Version.fromString((rangeVersion.major + 1) + ".0.0");
+        return versionToEvaluate.onOrAfter(lower) && versionToEvaluate.before(upper);
+    }
+}

libs/core/src/test/java/org/opensearch/semver/SemverRangeTests.java

@@ -43,6 +43,17 @@ public void testRangeWithTildeOperator() {
         assertFalse(range.isSatisfiedBy("3.0.0"));
     }
 
+    public void testRangeWithCaretOperator() {
+        SemverRange range = SemverRange.fromString("^2.3.4");
+        assertEquals(range.getRangeOperator(), SemverRange.RangeOperator.CARET);
+        assertTrue(range.isSatisfiedBy("2.3.4"));
+        assertTrue(range.isSatisfiedBy("2.3.5"));
+        assertTrue(range.isSatisfiedBy("2.4.12"));
+
+        assertFalse(range.isSatisfiedBy("2.3.3"));
+        assertFalse(range.isSatisfiedBy("3.0.0"));
+    }
+
     public void testInvalidRanges() {
         IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString(""));
         assertEquals("Version cannot be empty", ex.getMessage());
@@ -72,14 +83,23 @@ public void testInvalidRanges() {
         assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
 
         ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString("^"));
-        assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
+        assertEquals("Version cannot be empty", ex.getMessage());
 
         ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString("^1"));
         assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
 
         ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString("^1.2"));
         assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
 
-        expectThrows(NumberFormatException.class, () -> SemverRange.fromString("^1.2.3"));
+        ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString("$"));
+        assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
+
+        ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString("$1"));
+        assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
+
+        ex = expectThrows(IllegalArgumentException.class, () -> SemverRange.fromString("$1.2"));
+        assertTrue(ex.getMessage().contains("the version needs to contain major, minor, and revision, and optionally the build"));
+
+        expectThrows(NumberFormatException.class, () -> SemverRange.fromString("$1.2.3"));
     }
 }

libs/core/src/test/java/org/opensearch/semver/expr/CaretTests.java

@@ -0,0 +1,30 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.semver.expr;
+
+import org.opensearch.Version;
+import org.opensearch.test.OpenSearchTestCase;
+
+public class CaretTests extends OpenSearchTestCase {
+
+    public void testMinorAndPatchVersionVariability() {
+        Caret caretExpr = new Caret();
+        Version rangeVersion = Version.fromString("1.2.3");
+
+        // Compatible versions
+        assertTrue(caretExpr.evaluate(rangeVersion, Version.fromString("1.2.3")));
+        assertTrue(caretExpr.evaluate(rangeVersion, Version.fromString("1.2.4")));
+        assertTrue(caretExpr.evaluate(rangeVersion, Version.fromString("1.3.3")));
+        assertTrue(caretExpr.evaluate(rangeVersion, Version.fromString("1.9.9")));
+
+        // Incompatible versions
+        assertFalse(caretExpr.evaluate(rangeVersion, Version.fromString("1.2.2")));
+        assertFalse(caretExpr.evaluate(rangeVersion, Version.fromString("2.0.0")));
+    }
+}

qa/full-cluster-restart/src/test/java/org/opensearch/upgrades/PluginInfoIT.java

@@ -0,0 +1,27 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.upgrades;
+
+import org.opensearch.client.Request;
+import org.opensearch.client.Response;
+import org.opensearch.test.rest.yaml.ObjectPath;
+
+import java.util.Map;
+
+public class PluginInfoIT extends AbstractFullClusterRestartTestCase {
+    public void testPluginInfoSerialization() throws Exception {
+        // Ensure all nodes are able to come up, validate with GET _nodes.
+        Response response = client().performRequest(new Request("GET", "_nodes"));
+        ObjectPath objectPath = ObjectPath.createFromResponse(response);
+        final Map<String, Object> nodeMap = objectPath.evaluate("nodes");
+        // Any issue in PluginInfo serialization logic will result into connection failures
+        // and hence reduced number of nodes.
+        assertEquals(2, nodeMap.keySet().size());
+    }
+}

qa/mixed-cluster/src/test/java/org/opensearch/backwards/PluginInfoIT.java

@@ -0,0 +1,32 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.backwards;
+
+import org.opensearch.Version;
+import org.opensearch.client.Request;
+import org.opensearch.client.Response;
+import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.common.settings.Settings;
+
+import org.opensearch.test.rest.OpenSearchRestTestCase;
+import org.opensearch.test.rest.yaml.ObjectPath;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+
+public class PluginInfoIT extends OpenSearchRestTestCase {
+    public void testPluginInfoSerialization() throws Exception {
+        // Ensure all nodes are able to come up
+        Response response = client().performRequest(new Request("GET", "_nodes"));
+        ObjectPath objectPath = ObjectPath.createFromResponse(response);
+        final Map<String, Object> nodeMap = objectPath.evaluate("nodes");
+        assertEquals(4, nodeMap.keySet().size());
+    }
+}

server/src/internalClusterTest/java/org/opensearch/plugins/PluginsServiceIT.java

@@ -57,9 +57,6 @@ public void testNodeBootstrapWithRangeCompatiblePlugin() throws IOException {
         // Prepare the plugins directory and then start a node
         Path baseDir = createTempDir();
         Path pluginDir = baseDir.resolve("plugins/dummy-plugin");
-        String range1 = "~" + Version.CURRENT;
-        String range2 = "=" + Version.CURRENT;
-        String ranges = "\"" + range1 + "," + range2 + "\"";
         PluginTestUtil.writePluginProperties(
             pluginDir,
             "description",
@@ -69,7 +66,7 @@ public void testNodeBootstrapWithRangeCompatiblePlugin() throws IOException {
             "version",
             "1.0",
             "dependencies",
-            "{opensearch:" + ranges + "}",
+            "{opensearch:" + "~" + Version.CURRENT + "}",
             "java.version",
             System.getProperty("java.specification.version"),
             "classname",

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

@@ -130,6 +130,12 @@ public PluginInfo(
         this.name = name;
         this.description = description;
         this.version = version;
+        // Ensure only one range is specified (for now)
+        if (opensearchVersionRanges.size() != 1) {
+            throw new IllegalArgumentException(
+                "Exactly one range is allowed to be specified in dependencies for the plugin [" + name + "]"
+            );
+        }
         this.opensearchVersionRanges = opensearchVersionRanges;
         this.javaVersion = javaVersion;
         this.classname = classname;
@@ -280,12 +286,12 @@ public static PluginInfo readFromProperties(final Path path) throws IOException
                 throw new IllegalArgumentException("Only opensearch is allowed to be specified as a plugin dependency: " + dependenciesMap);
             }
             String[] ranges = dependenciesMap.get("opensearch").split(",");
-            for (String range : ranges) {
-                opensearchVersionRanges.add(SemverRange.fromString(range.trim()));
-            }
-            if (opensearchVersionRanges.isEmpty()) {
-                throw new IllegalArgumentException("Invalid version specified in dependencies for the plugin [" + name + "]");
+            if (ranges.length != 1) {
+                throw new IllegalArgumentException(
+                    "Exactly one range is allowed to be specified in dependencies for the plugin [\" + name + \"]"
+                );
             }
+            opensearchVersionRanges.add(SemverRange.fromString(ranges[0].trim()));
         }
 
         final String javaVersionString = propsMap.remove("java.version");

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

@@ -403,7 +403,7 @@ static void verifyCompatibility(PluginInfo info) {
     }
 
     public static boolean isPluginVersionCompatible(final PluginInfo pluginInfo, final Version coreVersion) {
-        // Core version must satisfy all ranges of opensearch version
+        // Core version must satisfy the semver range in plugin info
         for (SemverRange range : pluginInfo.getOpenSearchVersionRanges()) {
             if (!range.isSatisfiedBy(coreVersion)) {
                 return false;

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

@@ -37,6 +37,7 @@
 import org.opensearch.action.admin.cluster.node.info.PluginsAndModules;
 import org.opensearch.common.io.stream.BytesStreamOutput;
 import org.opensearch.core.common.io.stream.ByteBufferStreamInput;
+import org.opensearch.semver.SemverRange;
 import org.opensearch.test.OpenSearchTestCase;
 
 import java.nio.ByteBuffer;
@@ -105,32 +106,6 @@ public void testReadFromPropertiesWithSingleOpenSearchRange() throws Exception {
         assertThat(info.getExtendedPlugins(), empty());
     }
 
-    public void testReadFromPropertiesWithMultipleOpenSearchRanges() throws Exception {
-        Path pluginDir = createTempDir().resolve("fake-plugin");
-        PluginTestUtil.writePluginProperties(
-            pluginDir,
-            "description",
-            "fake desc",
-            "name",
-            "my_plugin",
-            "version",
-            "1.0",
-            "dependencies",
-            "{opensearch:\"~1.2.3, =1.2.3\"}",
-            "java.version",
-            System.getProperty("java.specification.version"),
-            "classname",
-            "FakePlugin"
-        );
-        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("[~1.2.3,=1.2.3]", info.getOpenSearchVersionRangesString());
-        assertThat(info.getExtendedPlugins(), empty());
-    }
-
     public void testReadFromPropertiesWithFolderNameAndVersionAfter() throws Exception {
         Path pluginDir = createTempDir().resolve("fake-plugin");
         PluginTestUtil.writePluginProperties(
@@ -569,12 +544,54 @@ public void testInvalidRangesInDependencies() throws Exception {
             "version",
             "1.0",
             "dependencies",
-            "{opensearch:\"1.2.3,~1.2.3,<2.2.0\"}",
+            "{opensearch:\"<2.2.0\"}",
             "java.version",
             System.getProperty("java.specification.version"),
             "classname",
             "FakePlugin"
         );
         expectThrows(NumberFormatException.class, () -> PluginInfo.readFromProperties(pluginDir));
     }
+
+    public void testhMultipleOpenSearchRangesInDependencies() throws Exception {
+        Path pluginDir = createTempDir().resolve("fake-plugin");
+        PluginTestUtil.writePluginProperties(
+            pluginDir,
+            "description",
+            "fake desc",
+            "name",
+            "my_plugin",
+            "version",
+            "1.0",
+            "dependencies",
+            "{opensearch:\"~1.2.3, =1.2.3\"}",
+            "java.version",
+            System.getProperty("java.specification.version"),
+            "classname",
+            "FakePlugin"
+        );
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> PluginInfo.readFromProperties(pluginDir));
+        assertThat(e.getMessage(), containsString("Exactly one range is allowed to be specified in dependencies for the plugin"));
+    }
+
+    public void testhMultipleOpenSearchRangesInConstructor() throws Exception {
+        IllegalArgumentException e = expectThrows(
+            IllegalArgumentException.class,
+            () -> new PluginInfo(
+                "plugin_name",
+                "foo",
+                "dummy",
+                List.of(
+                    new SemverRange(Version.CURRENT, SemverRange.RangeOperator.EQ),
+                    new SemverRange(Version.CURRENT, SemverRange.RangeOperator.DEFAULT)
+                ),
+                "1.8",
+                "dummyclass",
+                null,
+                Collections.emptyList(),
+                randomBoolean()
+            )
+        );
+        assertThat(e.getMessage(), containsString("Exactly one range is allowed to be specified in dependencies for the plugin"));
+    }
 }