From 6717cb6fb0decf1c07d89f077d3a3e9595184528 Mon Sep 17 00:00:00 2001 From: Marcus Bengtsson Date: Tue, 19 May 2026 10:42:06 +0200 Subject: [PATCH] ARTEMIS-6068 Broker plugins do not receive updated properties on broker.xml reload Broker plugins have no way to learn that their configured properties changed when broker.xml is reloaded. Store parsed class names and properties during reload. Notify matching running plugin instances via a new propertiesReloaded(Map) callback on ActiveMQServerBasePlugin. --- .../config/BrokerPluginConfiguration.java | 78 +++++++ .../artemis/core/config/Configuration.java | 7 + .../core/config/impl/ConfigurationImpl.java | 15 ++ .../impl/FileConfigurationParser.java | 15 +- .../core/server/impl/ActiveMQServerImpl.java | 18 +- .../plugin/ActiveMQServerBasePlugin.java | 10 + .../server/impl/BrokerPluginReloadTest.java | 215 ++++++++++++++++++ docs/user-manual/broker-plugins.adoc | 14 +- 8 files changed, 364 insertions(+), 8 deletions(-) create mode 100644 artemis-server/src/main/java/org/apache/activemq/artemis/core/config/BrokerPluginConfiguration.java create mode 100644 artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/BrokerPluginReloadTest.java diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/BrokerPluginConfiguration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/BrokerPluginConfiguration.java new file mode 100644 index 00000000000..512c53a7a5f --- /dev/null +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/BrokerPluginConfiguration.java @@ -0,0 +1,78 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.config; + +import java.io.Serializable; +import java.util.Map; +import java.util.Objects; + +public class BrokerPluginConfiguration implements Serializable { + + private static final long serialVersionUID = 1L; + + private String className; + + private Map properties; + + public BrokerPluginConfiguration() { + } + + public BrokerPluginConfiguration(String className, Map properties) { + this.className = className; + this.properties = properties; + } + + public String getClassName() { + return className; + } + + public BrokerPluginConfiguration setClassName(String className) { + this.className = className; + return this; + } + + public Map getProperties() { + return properties; + } + + public BrokerPluginConfiguration setProperties(Map properties) { + this.properties = properties; + return this; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof BrokerPluginConfiguration other)) { + return false; + } + + return Objects.equals(getClassName(), other.getClassName()) && Objects.equals(getProperties(), other.getProperties()); + } + + @Override + public int hashCode() { + return Objects.hash(getClassName(), getProperties()); + } + + @Override + public String toString() { + return "BrokerPluginConfiguration[className=" + className + ", properties=" + properties + "]"; + } +} diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java index 67aa0279cc2..1378af76174 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java @@ -1381,6 +1381,13 @@ default boolean isJDBC() { void unRegisterBrokerPlugin(ActiveMQServerBasePlugin plugin); + /** + * {@return the broker plugin configurations parsed from broker.xml} + */ + List getBrokerPluginConfigurations(); + + Configuration setBrokerPluginConfigurations(List configs); + Collection getLockCoordinatorConfigurations(); void addLockCoordinatorConfiguration(LockCoordinatorConfiguration configuration); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java index 76631124e06..7656dda9210 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java @@ -78,6 +78,7 @@ import org.apache.activemq.artemis.api.core.SimpleString; import org.apache.activemq.artemis.api.core.TransportConfiguration; import org.apache.activemq.artemis.core.config.BridgeConfiguration; +import org.apache.activemq.artemis.core.config.BrokerPluginConfiguration; import org.apache.activemq.artemis.core.config.ClusterConnectionConfiguration; import org.apache.activemq.artemis.core.config.Configuration; import org.apache.activemq.artemis.core.config.ConfigurationUtils; @@ -374,6 +375,7 @@ public class ConfigurationImpl extends javax.security.auth.login.Configuration i private MetricsConfiguration metricsConfiguration = null; private final List brokerPlugins = new CopyOnWriteArrayList<>(); + private List brokerPluginConfigurations = new ArrayList<>(); private final List brokerConnectionPlugins = new CopyOnWriteArrayList<>(); private final List brokerSessionPlugins = new CopyOnWriteArrayList<>(); private final List brokerConsumerPlugins = new CopyOnWriteArrayList<>(); @@ -1067,6 +1069,8 @@ private void writeProperties(FileWriter writer) throws Exception { "status", // we cannot import a map> property and this feature is only applied by the xml parser "securityRoleNameMappings", + // only used for reload matching, cannot be imported as plugins are instantiated via XML + "brokerPluginConfigurations", // using a deprecated config object "queueConfigurations", "queueConfigs", @@ -2684,6 +2688,17 @@ public List getBrokerPlugins() { return brokerPlugins; } + @Override + public List getBrokerPluginConfigurations() { + return brokerPluginConfigurations; + } + + @Override + public ConfigurationImpl setBrokerPluginConfigurations(final List configs) { + brokerPluginConfigurations = configs; + return this; + } + // for properties type inference public void addBrokerPlugin(ActiveMQServerBasePlugin type) { registerBrokerPlugin(type); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java index bd6c5ccc3ff..219a9144c55 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java @@ -45,6 +45,7 @@ import org.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory; import org.apache.activemq.artemis.api.core.client.ActiveMQClient; import org.apache.activemq.artemis.core.config.BridgeConfiguration; +import org.apache.activemq.artemis.core.config.BrokerPluginConfiguration; import org.apache.activemq.artemis.core.config.ClusterConnectionConfiguration; import org.apache.activemq.artemis.core.config.Configuration; import org.apache.activemq.artemis.core.config.ConfigurationUtils; @@ -1044,18 +1045,20 @@ private void parseBrokerPlugins(final Element e, final Configuration config) { if (brokerPlugins.getLength() != 0) { Element node = (Element) brokerPlugins.item(0); NodeList list = node.getElementsByTagName(BROKER_PLUGIN_ELEMENT_NAME); + List pluginConfigs = new ArrayList<>(); for (int i = 0; i < list.getLength(); i++) { - ActiveMQServerPlugin plugin = parseActiveMQServerPlugin(list.item(i)); + String className = list.item(i).getAttributes().getNamedItem("class-name").getNodeValue(); + Map properties = getMapOfChildPropertyElements(list.item(i)); + + ActiveMQServerPlugin plugin = parseActiveMQServerPlugin(className, properties); config.registerBrokerPlugin(plugin); + pluginConfigs.add(new BrokerPluginConfiguration(className, properties)); } + config.setBrokerPluginConfigurations(pluginConfigs); } } - private ActiveMQServerPlugin parseActiveMQServerPlugin(Node item) { - final String clazz = item.getAttributes().getNamedItem("class-name").getNodeValue(); - - Map properties = getMapOfChildPropertyElements(item); - + private ActiveMQServerPlugin parseActiveMQServerPlugin(String clazz, Map properties) { ActiveMQServerPlugin serverPlugin = SecurityManagerShim.doPrivileged((PrivilegedAction) () -> (ActiveMQServerPlugin) ClassloadingUtil.newInstanceFromClassLoader(FileConfigurationParser.class, clazz, ActiveMQServerPlugin.class)); serverPlugin.init(properties); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java index beb52d1624d..14845b12549 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java @@ -67,6 +67,7 @@ import org.apache.activemq.artemis.api.core.management.ResourceNames; import org.apache.activemq.artemis.core.client.impl.ClientSessionFactoryImpl; import org.apache.activemq.artemis.core.config.BridgeConfiguration; +import org.apache.activemq.artemis.core.config.BrokerPluginConfiguration; import org.apache.activemq.artemis.core.config.Configuration; import org.apache.activemq.artemis.core.config.ConfigurationUtils; import org.apache.activemq.artemis.core.config.CoreAddressConfiguration; @@ -4766,6 +4767,7 @@ private void updateReloadableConfigurationFrom(Configuration config) { configuration.setPurgePageFolders(config.isPurgePageFolders()); configuration.setConnectionRouters(config.getConnectionRouters()); configuration.setJaasConfigs(config.getJaasConfigs()); + configuration.setBrokerPluginConfigurations(config.getBrokerPluginConfigurations()); } private static boolean hasReloadableConfig(Configuration configuration) { @@ -4779,7 +4781,8 @@ private static boolean hasReloadableConfig(Configuration configuration) { !configuration.getAcceptorConfigurations().isEmpty() || !configuration.getAMQPConnection().isEmpty() || !configuration.getConnectionRouters().isEmpty() || - !configuration.getJaasConfigs().isEmpty(); + !configuration.getJaasConfigs().isEmpty() || + !configuration.getBrokerPluginConfigurations().isEmpty(); } private void deployReloadableConfigFromConfiguration() throws Exception { @@ -4887,6 +4890,19 @@ private void deployReloadableConfigFromConfiguration() throws Exception { ActiveMQServerLogger.LOGGER.reloadingConfiguration("protocol services"); updateProtocolServices(); + + ActiveMQServerLogger.LOGGER.reloadingConfiguration("broker plugins"); + for (BrokerPluginConfiguration pluginConfig : configuration.getBrokerPluginConfigurations()) { + getBrokerPlugins().stream() + .filter(p -> p.getClass().getName().equals(pluginConfig.getClassName())) + .forEach(p -> { + try { + p.propertiesReloaded(pluginConfig.getProperties()); + } catch (Throwable e) { + logger.warn("Error notifying plugin {} of property reload", p, e); + } + }); + } } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerBasePlugin.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerBasePlugin.java index cc0483a3cee..bcffec77f33 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerBasePlugin.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/plugin/ActiveMQServerBasePlugin.java @@ -46,4 +46,14 @@ default void registered(ActiveMQServer server) { */ default void unregistered(ActiveMQServer server) { } + + /** + * The broker configuration has been reloaded. Unlike {@link #init(Map)}, this is + * called only on an already registered and active plugin. Called on every reload + * regardless of whether properties actually changed. + * + * @param properties The new set of properties from the reloaded configuration + */ + default void propertiesReloaded(Map properties) { + } } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/BrokerPluginReloadTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/BrokerPluginReloadTest.java new file mode 100644 index 00000000000..c1cc5838c82 --- /dev/null +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/server/impl/BrokerPluginReloadTest.java @@ -0,0 +1,215 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.activemq.artemis.core.server.impl; + +import java.io.File; +import java.io.FileInputStream; +import java.nio.file.Files; +import java.util.Map; + +import org.apache.activemq.artemis.core.config.Configuration; +import org.apache.activemq.artemis.core.deployers.impl.FileConfigurationParser; +import org.apache.activemq.artemis.core.server.ActiveMQServer; +import org.apache.activemq.artemis.core.server.ActiveMQServers; +import org.apache.activemq.artemis.core.server.plugin.ActiveMQServerPlugin; +import org.apache.activemq.artemis.tests.util.ServerTestBase; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +@Timeout(10) +public class BrokerPluginReloadTest extends ServerTestBase { + + private static final String BROKER_XML_TEMPLATE = """ + + false + false + + + + + + + """; + + private static final String TWO_PLUGIN_BROKER_XML_TEMPLATE = """ + + false + false + + + + + + + + + + """; + + @Test + public void testPropertiesReloadedGetCorrectProperties() throws Exception { + File brokerXml = writeBrokerXml(ReloadAwarePlugin.class.getName(), "foo"); + + ActiveMQServer server = startServerFromBrokerXml(brokerXml); + ReloadAwarePlugin plugin = findPlugin(server, ReloadAwarePlugin.class); + assertEquals("foo", plugin.properties.get("key")); + + writeBrokerXml(brokerXml, ReloadAwarePlugin.class.getName(), "bar"); + server.reloadConfigurationFile(); + assertEquals(1, plugin.reloadCount); + assertEquals("bar", plugin.properties.get("key")); + + writeBrokerXml(brokerXml, ReloadAwarePlugin.class.getName(), "baz"); + server.reloadConfigurationFile(); + assertEquals(2, plugin.reloadCount); + assertEquals("baz", plugin.properties.get("key")); + } + + @Test + public void testPluginNotInBrokerXmlNotNotified() throws Exception { + File brokerXml = writeBrokerXml(TestPlugin.class.getName(), "foo"); + + ActiveMQServer server = startServerFromBrokerXml(brokerXml); + + // Manually register a plugin that does NOT appear in the broker.xml + ReloadAwarePlugin plugin = new ReloadAwarePlugin(); + server.registerBrokerPlugin(plugin); + + server.reloadConfigurationFile(); + + assertEquals(0, plugin.reloadCount); + } + + @Test + public void testExceptionInPluginNotBlockingOtherPlugins() throws Exception { + // ThrowingPlugin listed first - its exception must not block ReloadAwarePlugin + File brokerXml = writeBrokerXmlWithTwoPlugins(ThrowingPlugin.class.getName(), "foo1", ReloadAwarePlugin.class.getName(), "foo2"); + + ActiveMQServer server = startServerFromBrokerXml(brokerXml); + ReloadAwarePlugin plugin = findPlugin(server, ReloadAwarePlugin.class); + + writeBrokerXmlWithTwoPlugins(brokerXml, ThrowingPlugin.class.getName(), "bar1", ReloadAwarePlugin.class.getName(), "bar2"); + server.reloadConfigurationFile(); + + assertEquals(1, plugin.reloadCount); + assertEquals("bar2", plugin.properties.get("key")); + } + + @Test + public void testPluginNotifiedWhenPropertiesUnchanged() throws Exception { + File brokerXml = writeBrokerXml(ReloadAwarePlugin.class.getName(), "foo"); + + ActiveMQServer server = startServerFromBrokerXml(brokerXml); + ReloadAwarePlugin plugin = findPlugin(server, ReloadAwarePlugin.class); + + server.reloadConfigurationFile(); + assertEquals(1, plugin.reloadCount); + assertEquals("foo", plugin.properties.get("key")); + } + + @Test + public void testEachConfigEntryNotifiesMatchingPlugins() throws Exception { + // Two config entries with the same class - plugin should be notified once per entry + File brokerXml = writeBrokerXmlWithTwoPlugins(ReloadAwarePlugin.class.getName(), "foo1", ReloadAwarePlugin.class.getName(), "foo2"); + + ActiveMQServer server = startServerFromBrokerXml(brokerXml); + ReloadAwarePlugin plugin = findPlugin(server, ReloadAwarePlugin.class); + + writeBrokerXmlWithTwoPlugins(brokerXml, ReloadAwarePlugin.class.getName(), "bar1", ReloadAwarePlugin.class.getName(), "bar2"); + server.reloadConfigurationFile(); + + assertEquals(2, plugin.reloadCount); + // Last notification wins for properties + assertEquals("bar2", plugin.properties.get("key")); + } + + private ActiveMQServer startServerFromBrokerXml(File brokerXml) throws Exception { + Configuration config; + try (FileInputStream fis = new FileInputStream(brokerXml)) { + config = new FileConfigurationParser().parseMainConfig(fis); + } + config.setConfigurationUrl(brokerXml.toURI().toURL()); + config.setConfigurationFileRefreshPeriod(-1); + + ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(config)); + server.start(); + return server; + } + + @SuppressWarnings("unchecked") + private T findPlugin(ActiveMQServer server, Class type) { + return (T) server.getBrokerPlugins().stream().filter(type::isInstance).findFirst().orElseThrow(); + } + + private File writeBrokerXml(String className, String value) throws Exception { + File brokerXml = new File(temporaryFolder, "broker.xml"); + writeBrokerXml(brokerXml, className, value); + return brokerXml; + } + + private void writeBrokerXml(File file, String className, String value) throws Exception { + Files.writeString(file.toPath(), String.format(BROKER_XML_TEMPLATE, className, value)); + } + + private File writeBrokerXmlWithTwoPlugins(String pluginClass1, + String value1, + String pluginClass2, + String value2) throws Exception { + File brokerXml = new File(temporaryFolder, "broker.xml"); + writeBrokerXmlWithTwoPlugins(brokerXml, pluginClass1, value1, pluginClass2, value2); + return brokerXml; + } + + private void writeBrokerXmlWithTwoPlugins(File file, + String pluginClass1, + String value1, + String pluginClass2, + String value2) throws Exception { + Files.writeString(file.toPath(), String.format(TWO_PLUGIN_BROKER_XML_TEMPLATE, pluginClass1, value1, pluginClass2, value2)); + } + + public static class ReloadAwarePlugin implements ActiveMQServerPlugin { + + Map properties; + int reloadCount; + + @Override + public void init(Map properties) { + this.properties = properties; + } + + @Override + public void propertiesReloaded(Map properties) { + this.properties = properties; + this.reloadCount++; + } + } + + public static class TestPlugin implements ActiveMQServerPlugin { + + } + + public static class ThrowingPlugin implements ActiveMQServerPlugin { + + @Override + public void propertiesReloaded(Map properties) { + throw new RuntimeException("Simulated plugin failure"); + } + } +} diff --git a/docs/user-manual/broker-plugins.adoc b/docs/user-manual/broker-plugins.adoc index 3345b7032c5..7ea665202cf 100644 --- a/docs/user-manual/broker-plugins.adoc +++ b/docs/user-manual/broker-plugins.adoc @@ -205,4 +205,16 @@ The plugin can be configured via xml in the normal broker-plugin way: ----- \ No newline at end of file +---- + +== Reloaded Plugin Properties + +Broker plugins receive their initial configured properties through `init(Map)` when the plugin is created. + +When `broker.xml` is reloaded, already registered broker plugin instances are not replaced. +Instead, plugins may implement `propertiesReloaded(Map)` to receive the properties parsed from the reloaded configuration. + +The callback is invoked on each reload for matching plugin configuration entries, regardless of whether the property values changed. +Plugins that need change detection should compare against their current state. + +Broker plugin entries do not have a stable name/id attribute, so reload matching is performed by plugin class name.