From 7b24470089d298168739a5803ee5ad2dda16c57e Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Wed, 6 Nov 2024 09:12:57 -0500 Subject: [PATCH 1/2] Support installing plugin SNAPSHOTs with SNASPHOT distribution Signed-off-by: Andriy Redko --- CHANGELOG.md | 1 + .../plugins/InstallPluginCommand.java | 30 +--- .../plugins/InstallPluginCommandTests.java | 165 ++---------------- 3 files changed, 21 insertions(+), 175 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60535b2cca895..55c10cad5c2f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add Setting to adjust the primary constraint weights ([#16471](https://github.com/opensearch-project/OpenSearch/pull/16471)) - 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)) - 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)) ### Dependencies 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 index 838d6e22a37bd..6e56d811c781e 100644 --- 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 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,11 +332,6 @@ 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(); } @@ -353,26 +339,18 @@ boolean 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 + version + "-SNAPSHOT" ); } else { baseUrl = String.format( 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 c264788df20e8..e6c1070f36066 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 @@ -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 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"))); } From 42c6f65e5f7058b59c059c4309bbf28376c65198 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Thu, 14 Nov 2024 11:07:14 -0500 Subject: [PATCH 2/2] Use Build.CURRENT directly for snapshots Signed-off-by: Andriy Redko --- .../main/java/org/opensearch/plugins/InstallPluginCommand.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 6e56d811c781e..511d6974085aa 100644 --- 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 @@ -350,7 +350,7 @@ private String getOpenSearchUrl( Locale.ROOT, "https://artifacts.opensearch.org/snapshots/plugins/%s/%s", pluginId, - version + "-SNAPSHOT" + Build.CURRENT.getQualifiedVersion() ); } else { baseUrl = String.format(