Support installing plugin SNAPSHOTs with SNASPHOT distribution (#16581)

* Support installing plugin SNAPSHOTs with SNASPHOT distribution Signed-off-by: Andriy Redko <[email protected]> * Use Build.CURRENT directly for snapshots Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]>
opensearch-project · Nov 14, 2024 · 3b9ca63 · 3b9ca63
1 parent ac3e4ff
commit 3b9ca63
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 175 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Switch from `buildSrc/version.properties` to Gradle version catalog (`gradle/libs.versions.toml`) to enable dependabot to perform automated upgrades on common libs ([#16284](https://github.com/opensearch-project/OpenSearch/pull/16284))
 - Increase segrep pressure checkpoint default limit to 30 ([#16577](https://github.com/opensearch-project/OpenSearch/pull/16577/files))
 - Add dynamic setting allowing size > 0 requests to be cached in the request cache ([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483))
+- Support installing plugin SNAPSHOTs with SNASPHOT distribution ([#16581](https://github.com/opensearch-project/OpenSearch/pull/16581))
 - Make IndexStoreListener a pluggable interface ([#16583](https://github.com/opensearch-project/OpenSearch/pull/16583))
 - Support for keyword fields in star-tree index ([#16233](https://github.com/opensearch-project/OpenSearch/pull/16233))
 - Add a flag in QueryShardContext to differentiate inner hit query ([#16600](https://github.com/opensearch-project/OpenSearch/pull/16600))

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
@@ -137,8 +137,6 @@
  */
 class InstallPluginCommand extends EnvironmentAwareCommand {
 
-    private static final String PROPERTY_STAGING_ID = "opensearch.plugins.staging";
-
     // exit codes for install
     /** A plugin with the same name is already installed. */
     static final int PLUGIN_EXISTS = 1;
@@ -307,14 +305,7 @@ void execute(Terminal terminal, List<String> pluginIds, boolean isBatch, Environ
     private Path download(Terminal terminal, String pluginId, Path tmpDir, boolean isBatch) throws Exception {
 
         if (OFFICIAL_PLUGINS.contains(pluginId)) {
-            final String url = getOpenSearchUrl(
-                terminal,
-                getStagingHash(),
-                Version.CURRENT,
-                isSnapshot(),
-                pluginId,
-                Platforms.PLATFORM_NAME
-            );
+            final String url = getOpenSearchUrl(terminal, Version.CURRENT, isSnapshot(), pluginId, Platforms.PLATFORM_NAME);
             terminal.println("-> Downloading " + pluginId + " from opensearch");
             return downloadAndValidate(terminal, url, tmpDir, true, isBatch);
         }
@@ -341,38 +332,25 @@ private Path download(Terminal terminal, String pluginId, Path tmpDir, boolean i
         return downloadZip(terminal, pluginId, tmpDir, isBatch);
     }
 
-    // pkg private so tests can override
-    String getStagingHash() {
-        return System.getProperty(PROPERTY_STAGING_ID);
-    }
-
     boolean isSnapshot() {
         return Build.CURRENT.isSnapshot();
     }
 
     /** Returns the url for an official opensearch plugin. */
     private String getOpenSearchUrl(
         final Terminal terminal,
-        final String stagingHash,
         final Version version,
         final boolean isSnapshot,
         final String pluginId,
         final String platform
     ) throws IOException, UserException {
         final String baseUrl;
-        if (isSnapshot && stagingHash == null) {
-            throw new UserException(
-                ExitCodes.CONFIG,
-                "attempted to install release build of official plugin on snapshot build of OpenSearch"
-            );
-        }
-        if (stagingHash != null) {
+        if (isSnapshot == true) {
             baseUrl = String.format(
                 Locale.ROOT,
-                "https://artifacts.opensearch.org/snapshots/plugins/%s/%s-%s",
+                "https://artifacts.opensearch.org/snapshots/plugins/%s/%s",
                 pluginId,
-                version,
-                stagingHash
+                Build.CURRENT.getQualifiedVersion()
             );
         } else {
             baseUrl = String.format(

diff --git a/...tion/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java b/...tion/tools/plugin-cli/src/test/java/org/opensearch/plugins/InstallPluginCommandTests.java
@@ -990,7 +990,6 @@ void assertInstallPluginFromUrl(
         final String pluginId,
         final String name,
         final String url,
-        final String stagingHash,
         final boolean isSnapshot,
         final String shaExtension,
         final Function<byte[], String> shaCalculator,
@@ -1065,11 +1064,6 @@ boolean urlExists(Terminal terminal, String urlString) throws IOException {
                 return urlString.equals(url);
             }
 
-            @Override
-            String getStagingHash() {
-                return stagingHash;
-            }
-
             @Override
             boolean isSnapshot() {
                 return isSnapshot;
@@ -1084,19 +1078,13 @@ void jarHellCheck(PluginInfo candidateInfo, Path candidate, Path pluginsDir, Pat
         assertPlugin(name, pluginDir, env.v2());
     }
 
-    public void assertInstallPluginFromUrl(
-        final String pluginId,
-        final String name,
-        final String url,
-        final String stagingHash,
-        boolean isSnapshot
-    ) throws Exception {
+    public void assertInstallPluginFromUrl(final String pluginId, final String name, final String url, boolean isSnapshot)
+        throws Exception {
         final MessageDigest digest = MessageDigest.getInstance("SHA-512");
         assertInstallPluginFromUrl(
             pluginId,
             name,
             url,
-            stagingHash,
             isSnapshot,
             ".sha512",
             checksumAndFilename(digest, url),
@@ -1111,42 +1099,17 @@ public void testOfficialPlugin() throws Exception {
             + "/analysis-icu-"
             + Build.CURRENT.getQualifiedVersion()
             + ".zip";
-        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, null, false);
+        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, false);
     }
 
     public void testOfficialPluginSnapshot() throws Exception {
         String url = String.format(
             Locale.ROOT,
-            "https://artifacts.opensearch.org/snapshots/plugins/analysis-icu/%s-abc123/analysis-icu-%s.zip",
-            Version.CURRENT,
-            Build.CURRENT.getQualifiedVersion()
-        );
-        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, "abc123", true);
-    }
-
-    public void testInstallReleaseBuildOfPluginOnSnapshotBuild() {
-        String url = String.format(
-            Locale.ROOT,
-            "https://artifacts.opensearch.org/snapshots/plugins/analysis-icu/%s-abc123/analysis-icu-%s.zip",
+            "https://artifacts.opensearch.org/snapshots/plugins/analysis-icu/%s-SNAPSHOT/analysis-icu-%s.zip",
             Version.CURRENT,
             Build.CURRENT.getQualifiedVersion()
         );
-        // attemping to install a release build of a plugin (no staging ID) on a snapshot build should throw a user exception
-        final UserException e = expectThrows(
-            UserException.class,
-            () -> assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, null, true)
-        );
-        assertThat(e.exitCode, equalTo(ExitCodes.CONFIG));
-        assertThat(e, hasToString(containsString("attempted to install release build of official plugin on snapshot build of OpenSearch")));
-    }
-
-    public void testOfficialPluginStaging() throws Exception {
-        String url = "https://artifacts.opensearch.org/snapshots/plugins/analysis-icu/"
-            + Version.CURRENT
-            + "-abc123/analysis-icu-"
-            + Build.CURRENT.getQualifiedVersion()
-            + ".zip";
-        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, "abc123", false);
+        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, true);
     }
 
     public void testOfficialPlatformPlugin() throws Exception {
@@ -1157,62 +1120,30 @@ public void testOfficialPlatformPlugin() throws Exception {
             + "-"
             + Build.CURRENT.getQualifiedVersion()
             + ".zip";
-        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, null, false);
-    }
-
-    public void testOfficialPlatformPluginSnapshot() throws Exception {
-        String url = String.format(
-            Locale.ROOT,
-            "https://artifacts.opensearch.org/snapshots/plugins/analysis-icu/%s-abc123/analysis-icu-%s-%s.zip",
-            Version.CURRENT,
-            Platforms.PLATFORM_NAME,
-            Build.CURRENT.getQualifiedVersion()
-        );
-        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, "abc123", true);
-    }
-
-    public void testOfficialPlatformPluginStaging() throws Exception {
-        String url = "https://artifacts.opensearch.org/snapshots/plugins/analysis-icu/"
-            + Version.CURRENT
-            + "-abc123/analysis-icu-"
-            + Platforms.PLATFORM_NAME
-            + "-"
-            + Build.CURRENT.getQualifiedVersion()
-            + ".zip";
-        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, "abc123", false);
+        assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, false);
     }
 
     public void testMavenPlugin() throws Exception {
         String url = "https://repo1.maven.org/maven2/mygroup/myplugin/1.0.0/myplugin-1.0.0.zip";
-        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, null, false);
+        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, false);
     }
 
     public void testMavenPlatformPlugin() throws Exception {
         String url = "https://repo1.maven.org/maven2/mygroup/myplugin/1.0.0/myplugin-" + Platforms.PLATFORM_NAME + "-1.0.0.zip";
-        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, null, false);
+        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, false);
     }
 
     public void testMavenSha1Backcompat() throws Exception {
         String url = "https://repo1.maven.org/maven2/mygroup/myplugin/1.0.0/myplugin-1.0.0.zip";
         MessageDigest digest = MessageDigest.getInstance("SHA-1");
-        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, null, false, ".sha1", checksum(digest), null, (b, p) -> null);
+        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, false, ".sha1", checksum(digest), null, (b, p) -> null);
         assertTrue(terminal.getOutput(), terminal.getOutput().contains("sha512 not found, falling back to sha1"));
     }
 
     public void testMavenChecksumWithoutFilename() throws Exception {
         String url = "https://repo1.maven.org/maven2/mygroup/myplugin/1.0.0/myplugin-1.0.0.zip";
         MessageDigest digest = MessageDigest.getInstance("SHA-512");
-        assertInstallPluginFromUrl(
-            "mygroup:myplugin:1.0.0",
-            "myplugin",
-            url,
-            null,
-            false,
-            ".sha512",
-            checksum(digest),
-            null,
-            (b, p) -> null
-        );
+        assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, false, ".sha512", checksum(digest), null, (b, p) -> null);
     }
 
     public void testOfficialChecksumWithoutFilename() throws Exception {
@@ -1224,17 +1155,7 @@ public void testOfficialChecksumWithoutFilename() throws Exception {
         MessageDigest digest = MessageDigest.getInstance("SHA-512");
         UserException e = expectThrows(
             UserException.class,
-            () -> assertInstallPluginFromUrl(
-                "analysis-icu",
-                "analysis-icu",
-                url,
-                null,
-                false,
-                ".sha512",
-                checksum(digest),
-                null,
-                (b, p) -> null
-            )
+            () -> assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, false, ".sha512", checksum(digest), null, (b, p) -> null)
         );
         assertEquals(ExitCodes.IO_ERROR, e.exitCode);
         assertThat(e.getMessage(), startsWith("Invalid checksum file"));
@@ -1249,17 +1170,7 @@ public void testOfficialShaMissing() throws Exception {
         MessageDigest digest = MessageDigest.getInstance("SHA-1");
         UserException e = expectThrows(
             UserException.class,
-            () -> assertInstallPluginFromUrl(
-                "analysis-icu",
-                "analysis-icu",
-                url,
-                null,
-                false,
-                ".sha1",
-                checksum(digest),
-                null,
-                (b, p) -> null
-            )
+            () -> assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, false, ".sha1", checksum(digest), null, (b, p) -> null)
         );
         assertEquals(ExitCodes.IO_ERROR, e.exitCode);
         assertEquals("Plugin checksum missing: " + url + ".sha512", e.getMessage());
@@ -1269,17 +1180,7 @@ public void testMavenShaMissing() throws Exception {
         String url = "https://repo1.maven.org/maven2/mygroup/myplugin/1.0.0/myplugin-1.0.0.zip";
         UserException e = expectThrows(
             UserException.class,
-            () -> assertInstallPluginFromUrl(
-                "mygroup:myplugin:1.0.0",
-                "myplugin",
-                url,
-                null,
-                false,
-                ".dne",
-                bytes -> null,
-                null,
-                (b, p) -> null
-            )
+            () -> assertInstallPluginFromUrl("mygroup:myplugin:1.0.0", "myplugin", url, false, ".dne", bytes -> null, null, (b, p) -> null)
         );
         assertEquals(ExitCodes.IO_ERROR, e.exitCode);
         assertEquals("Plugin checksum missing: " + url + ".sha1", e.getMessage());
@@ -1294,17 +1195,7 @@ public void testInvalidShaFileMissingFilename() throws Exception {
         MessageDigest digest = MessageDigest.getInstance("SHA-512");
         UserException e = expectThrows(
             UserException.class,
-            () -> assertInstallPluginFromUrl(
-                "analysis-icu",
-                "analysis-icu",
-                url,
-                null,
-                false,
-                ".sha512",
-                checksum(digest),
-                null,
-                (b, p) -> null
-            )
+            () -> assertInstallPluginFromUrl("analysis-icu", "analysis-icu", url, false, ".sha512", checksum(digest), null, (b, p) -> null)
         );
         assertEquals(ExitCodes.IO_ERROR, e.exitCode);
         assertTrue(e.getMessage(), e.getMessage().startsWith("Invalid checksum file"));
@@ -1323,7 +1214,6 @@ public void testInvalidShaFileMismatchFilename() throws Exception {
                 "analysis-icu",
                 "analysis-icu",
                 url,
-                null,
                 false,
                 ".sha512",
                 checksumAndString(digest, "  repository-s3-" + Build.CURRENT.getQualifiedVersion() + ".zip"),
@@ -1348,7 +1238,6 @@ public void testInvalidShaFileContainingExtraLine() throws Exception {
                 "analysis-icu",
                 "analysis-icu",
                 url,
-                null,
                 false,
                 ".sha512",
                 checksumAndString(digest, "  analysis-icu-" + Build.CURRENT.getQualifiedVersion() + ".zip\nfoobar"),
@@ -1372,7 +1261,6 @@ public void testSha512Mismatch() throws Exception {
                 "analysis-icu",
                 "analysis-icu",
                 url,
-                null,
                 false,
                 ".sha512",
                 bytes -> "foobar  analysis-icu-" + Build.CURRENT.getQualifiedVersion() + ".zip",
@@ -1392,7 +1280,6 @@ public void testSha1Mismatch() throws Exception {
                 "mygroup:myplugin:1.0.0",
                 "myplugin",
                 url,
-                null,
                 false,
                 ".sha1",
                 bytes -> "foobar",
@@ -1426,17 +1313,7 @@ public void testPublicKeyIdMismatchToExpectedPublicKeyId() throws Exception {
         final String expectedID = Long.toHexString(verifyingKey.getKeyID()).toUpperCase(Locale.ROOT);
         final IllegalStateException e = expectThrows(
             IllegalStateException.class,
-            () -> assertInstallPluginFromUrl(
-                icu,
-                icu,
-                url,
-                null,
-                false,
-                ".sha512",
-                checksumAndFilename(digest, url),
-                verifyingKey,
-                signature
-            )
+            () -> assertInstallPluginFromUrl(icu, icu, url, false, ".sha512", checksumAndFilename(digest, url), verifyingKey, signature)
         );
         assertThat(e, hasToString(containsString("key id [" + actualID + "] does not match expected key id [" + expectedID + "]")));
     }
@@ -1463,17 +1340,7 @@ public void testFailedSignatureVerification() throws Exception {
         };
         final IllegalStateException e = expectThrows(
             IllegalStateException.class,
-            () -> assertInstallPluginFromUrl(
-                icu,
-                icu,
-                url,
-                null,
-                false,
-                ".sha512",
-                checksumAndFilename(digest, url),
-                newSecretKey(),
-                signature
-            )
+            () -> assertInstallPluginFromUrl(icu, icu, url, false, ".sha512", checksumAndFilename(digest, url), newSecretKey(), signature)
         );
         assertThat(e, hasToString(equalTo("java.lang.IllegalStateException: signature verification for [" + url + "] failed")));
     }