From c4c33ff359b99e855306542d6cc077661e21383d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lorenzo=20Dematt=C3=A9?= Date: Mon, 28 Oct 2024 09:33:14 +0100 Subject: [PATCH] Fix NPE on plugin sync (#115640) --- .../plugins/cli/SyncPluginsAction.java | 8 ++++---- .../plugins/cli/SyncPluginsActionTests.java | 16 ++++++++++++++++ docs/changelog/115640.yaml | 6 ++++++ 3 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 docs/changelog/115640.yaml diff --git a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/SyncPluginsAction.java b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/SyncPluginsAction.java index 5394cb8f3d79..d6d061942277 100644 --- a/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/SyncPluginsAction.java +++ b/distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/cli/SyncPluginsAction.java @@ -25,6 +25,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Locale; @@ -60,7 +61,7 @@ public class SyncPluginsAction { * @throws UserException if a plugins config file is found. */ public static void ensureNoConfigFile(Environment env) throws UserException { - final Path pluginsConfig = env.configFile().resolve("elasticsearch-plugins.yml"); + final Path pluginsConfig = env.configFile().resolve(ELASTICSEARCH_PLUGINS_YML); if (Files.exists(pluginsConfig)) { throw new UserException( ExitCodes.USAGE, @@ -207,9 +208,8 @@ public class SyncPluginsAction { Optional cachedPluginsConfig, List existingPlugins ) { - final Map cachedPluginIdToLocation = cachedPluginsConfig.map( - config -> config.getPlugins().stream().collect(Collectors.toMap(InstallablePlugin::getId, InstallablePlugin::getLocation)) - ).orElse(Map.of()); + final Map cachedPluginIdToLocation = new HashMap<>(); + cachedPluginsConfig.ifPresent(config -> config.getPlugins().forEach(p -> cachedPluginIdToLocation.put(p.getId(), p.getLocation()))); return pluginsToMaybeUpgrade.stream().filter(eachPlugin -> { final String eachPluginId = eachPlugin.getId(); diff --git a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/SyncPluginsActionTests.java b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/SyncPluginsActionTests.java index 8ef44c8862e8..2d2336428a0a 100644 --- a/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/SyncPluginsActionTests.java +++ b/distribution/tools/plugin-cli/src/test/java/org/elasticsearch/plugins/cli/SyncPluginsActionTests.java @@ -157,6 +157,22 @@ public class SyncPluginsActionTests extends ESTestCase { assertThat(pluginChanges.upgrade.get(0).getId(), equalTo("analysis-icu")); } + /** + * Check that when there is an official plugin in the config file and in the cached config, then we + * calculate that the plugin does not need to be upgraded. + */ + public void test_getPluginChanges_withOfficialPluginCachedConfigAndNoChanges_returnsNoChanges() throws Exception { + createPlugin("analysis-icu"); + config.setPlugins(List.of(new InstallablePlugin("analysis-icu"))); + + final PluginsConfig cachedConfig = new PluginsConfig(); + cachedConfig.setPlugins(List.of(new InstallablePlugin("analysis-icu"))); + + final PluginChanges pluginChanges = action.getPluginChanges(config, Optional.of(cachedConfig)); + + assertThat(pluginChanges.isEmpty(), is(true)); + } + /** * Check that if an unofficial plugins' location has not changed in the cached config, then we * calculate that the plugin does not need to be upgraded. diff --git a/docs/changelog/115640.yaml b/docs/changelog/115640.yaml new file mode 100644 index 000000000000..5c4a943a9697 --- /dev/null +++ b/docs/changelog/115640.yaml @@ -0,0 +1,6 @@ +pr: 115640 +summary: Fix NPE on plugin sync +area: Infra/CLI +type: bug +issues: + - 114818