You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ds...@apache.org on 2024/01/24 18:00:12 UTC
(solr) branch branch_9x updated: SOLR-17119: Fix exception swallowing in /cluster/plugins (#2202)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new f705f1058eb SOLR-17119: Fix exception swallowing in /cluster/plugins (#2202)
f705f1058eb is described below
commit f705f1058ebbe5d592ae66db60151ec499a182e3
Author: Yohann Callea <yo...@gmail.com>
AuthorDate: Wed Jan 24 15:46:20 2024 +0100
SOLR-17119: Fix exception swallowing in /cluster/plugins (#2202)
When registering or updating a ConfigurablePlugin through the `/cluster/plugin` API,
config validation exceptions are now propagated to the callers.
---
solr/CHANGES.txt | 3 +-
.../solr/handler/admin/ContainerPluginsApi.java | 39 ++++++++----------
.../apache/solr/handler/TestContainerPlugin.java | 46 ++++++++++++++++++++++
3 files changed, 64 insertions(+), 24 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 425c9f1e744..b06caf88132 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -12,7 +12,8 @@ New Features
Improvements
---------------------
-(No changes)
+* SOLR-17119: When registering or updating a ConfigurablePlugin through the `/cluster/plugin` API,
+ config validation exceptions are now propagated to the callers. (Yohann Callea)
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java b/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
index 910964783f9..0de5abc86a5 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/ContainerPluginsApi.java
@@ -17,15 +17,12 @@
package org.apache.solr.handler.admin;
-import static org.apache.lucene.util.IOUtils.closeWhileHandlingException;
-
import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import org.apache.solr.api.AnnotatedApi;
import org.apache.solr.api.ClusterPluginsSource;
import org.apache.solr.api.Command;
import org.apache.solr.api.ContainerPluginsRegistry;
@@ -130,28 +127,24 @@ public class ContainerPluginsApi {
}
private void validateConfig(PayloadObj<PluginMeta> payload, PluginMeta info) throws IOException {
- if (info.klass.indexOf(':') > 0) {
- if (info.version == null) {
- payload.addError("Using package. must provide a packageVersion");
- return;
- }
- }
- List<String> errs = new ArrayList<>();
- ContainerPluginsRegistry.ApiInfo apiInfo =
- coreContainer.getContainerPluginsRegistry().createInfo(payload.getDataMap(), errs);
- if (!errs.isEmpty()) {
- for (String err : errs) payload.addError(err);
+ if (info.klass.indexOf(':') > 0 && info.version == null) {
+ payload.addError("Using package. must provide a packageVersion");
return;
}
- AnnotatedApi api = null;
- try {
- apiInfo.init();
- } catch (Exception e) {
- log.error("Error instantiating plugin ", e);
- errs.add(e.getMessage());
- return;
- } finally {
- closeWhileHandlingException(api);
+
+ final List<String> errs = new ArrayList<>();
+ final ContainerPluginsRegistry.ApiInfo apiInfo =
+ coreContainer.getContainerPluginsRegistry().createInfo(payload.getDataMap(), errs);
+
+ if (errs.isEmpty()) {
+ try {
+ apiInfo.init();
+ } catch (Exception e) {
+ log.error("Error instantiating plugin ", e);
+ errs.add(e.getMessage());
+ }
}
+
+ errs.forEach(payload::addError);
}
}
diff --git a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
index f7864c28ef6..eea4177ff01 100644
--- a/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
+++ b/solr/core/src/test/org/apache/solr/handler/TestContainerPlugin.java
@@ -47,6 +47,7 @@ import org.apache.solr.client.solrj.request.beans.PluginMeta;
import org.apache.solr.client.solrj.response.V2Response;
import org.apache.solr.cloud.ClusterSingleton;
import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.annotation.JsonProperty;
import org.apache.solr.common.util.ReflectMapWriter;
import org.apache.solr.core.CoreContainer;
@@ -378,6 +379,37 @@ public class TestContainerPlugin extends SolrCloudTestCase {
assertEquals(1452, C5.classData.limit());
}
+ @Test
+ public void testPluginConfigValidation() throws Exception {
+ final Callable<V2Response> readPluginState = getPlugin("/cluster/plugin");
+
+ // Register a plugin with an invalid configuration
+ final ValidatableConfig config = new ValidatableConfig();
+ config.willPassValidation = false;
+
+ final PluginMeta plugin = new PluginMeta();
+ plugin.name = "validatableplugin";
+ plugin.klass = ConfigurablePluginWithValidation.class.getName();
+ plugin.config = config;
+
+ final V2Request addPlugin = postPlugin(singletonMap("add", plugin));
+
+ // Verify that the expected error is thrown and the plugin is not registered
+ expectError(addPlugin, "invalid config");
+ V2Response response = readPluginState.call();
+ assertNull(response._getStr("/plugin/validatableplugin/class", null));
+
+ // Now register it with a valid configuration
+ config.willPassValidation = true;
+ addPlugin.process(cluster.getSolrClient());
+
+ // Verify that the plugin is properly registered
+ response = readPluginState.call();
+ assertEquals(
+ ConfigurablePluginWithValidation.class.getName(),
+ response._getStr("/plugin/validatableplugin/class", null));
+ }
+
public static class CC1 extends CC {}
public static class CC2 extends CC1 {}
@@ -510,6 +542,20 @@ public class TestContainerPlugin extends SolrCloudTestCase {
}
}
+ public static class ConfigurablePluginWithValidation
+ implements ConfigurablePlugin<ValidatableConfig> {
+ @Override
+ public void configure(ValidatableConfig cfg) {
+ if (!cfg.willPassValidation) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "invalid config");
+ }
+ }
+ }
+
+ public static class ValidatableConfig implements ReflectMapWriter {
+ @JsonProperty public boolean willPassValidation;
+ }
+
private Callable<V2Response> getPlugin(String path) {
V2Request req = new V2Request.Builder(path).forceV2(forceV2).GET().build();
return () -> {