This is an automated email from the ASF dual-hosted git repository. gtully pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/artemis.git
commit 9fe80fe5b0696370a830df1edf60fe9eaae14444 Author: Gary Tully <[email protected]> AuthorDate: Tue Feb 10 13:54:08 2026 +0000 ARTEMIS-5871 make it illegal to provide non xml reloadable config and a properties source when reload is enabled --- .../core/server/impl/ActiveMQServerImpl.java | 46 +++++++++++++++++----- docs/user-manual/config-reload.adoc | 5 ++- .../integration/routing/ElasticQueueTest.java | 1 + .../integration/server/ConfigurationTest.java | 39 ++++++++++++++++-- 4 files changed, 77 insertions(+), 14 deletions(-) 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 32c20c9138..a9aa48b765 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 @@ -308,7 +308,7 @@ public class ActiveMQServerImpl implements ActiveMQServer { protected volatile ExecutorFactory ioExecutorFactory; /** - * This is a thread pool for page only tasks only. This is because we have to limit parallel reads on paging. + * This is a thread pool for page tasks only. This is because we have to limit parallel reads on paging. */ protected volatile ExecutorFactory pageExecutorFactory; @@ -679,8 +679,18 @@ public class ActiveMQServerImpl implements ActiveMQServer { } @Override - public void setProperties(String fileUrltoBrokerProperties) { - propertiesFileUrl = fileUrltoBrokerProperties; + public void setProperties(String fileUrlToBrokerProperties) { + throwIfReloadableConfigProvidedWithoutFileAndBrokerPropertiesUrlNonNullAndReload(configuration, fileUrlToBrokerProperties); + propertiesFileUrl = fileUrlToBrokerProperties; + } + + private void throwIfReloadableConfigProvidedWithoutFileAndBrokerPropertiesUrlNonNullAndReload(Configuration configuration, String propertiesFileUrl) { + if (configuration.getConfigurationUrl() == null && configuration.getConfigurationFileRefreshPeriod() > 0 && configuration.resolvePropertiesSources(propertiesFileUrl) != null) { + // if any non xml (programmatic) provided config is reloadable, on the first properties source reload it will whack that config as the reload has the source of truth for reloadable attributes + if (hasReloadableConfig(configuration)) { + throw new IllegalStateException(String.format("a properties source (%s) is illegal, programmatic config contains reloadable elements and configurationFileRefreshPeriod > 0; your programmatic config will be replaced on reload of the properties source", propertiesFileUrl)); + } + } } @Override @@ -4689,8 +4699,16 @@ public class ActiveMQServerImpl implements ActiveMQServer { legacyJMSConfiguration.parseConfiguration(xmlConfigUri.openStream()); } config.parseProperties(propertiesFileUrl); - configuration.setStatus(config.getStatus()); + updateReloadableConfigurationFrom(config); + updateStatus(ServerStatus.CONFIGURATION_COMPONENT, configuration.getStatus()); + configurationReloadDeployed.set(false); + if (isActive()) { + deployReloadableConfigFromConfiguration(); + } + } + private void updateReloadableConfigurationFrom(Configuration config) { + configuration.setStatus(config.getStatus()); configuration.setSecurityRoles(config.getSecurityRoles()); configuration.setAddressSettings(config.getAddressSettings()); configuration.setDivertConfigurations(config.getDivertConfigurations()); @@ -4701,14 +4719,22 @@ public class ActiveMQServerImpl implements ActiveMQServer { configuration.setAcceptorConfigurations(config.getAcceptorConfigurations()); configuration.setAMQPConnectionConfigurations(config.getAMQPConnection()); configuration.setPurgePageFolders(config.isPurgePageFolders()); - configuration.setConnectionRouters(config.getConnectionRouters()); // needs reload logic + configuration.setConnectionRouters(config.getConnectionRouters()); configuration.setJaasConfigs(config.getJaasConfigs()); + } - updateStatus(ServerStatus.CONFIGURATION_COMPONENT, configuration.getStatus()); - configurationReloadDeployed.set(false); - if (isActive()) { - deployReloadableConfigFromConfiguration(); - } + private static boolean hasReloadableConfig(Configuration configuration) { + return !configuration.getSecurityRoles().isEmpty() || + !configuration.getAddressSettings().isEmpty() || + !configuration.getDivertConfigurations().isEmpty() || + !configuration.getAddressConfigurations().isEmpty() || + !configuration.getQueueConfigs().isEmpty() || + !configuration.getBridgeConfigurations().isEmpty() || + !configuration.getConnectorConfigurations().isEmpty() || + !configuration.getAcceptorConfigurations().isEmpty() || + !configuration.getAMQPConnection().isEmpty() || + !configuration.getConnectionRouters().isEmpty() || + !configuration.getJaasConfigs().isEmpty(); } private void deployReloadableConfigFromConfiguration() throws Exception { diff --git a/docs/user-manual/config-reload.adoc b/docs/user-manual/config-reload.adoc index 66bf4fd63c..b8c6e828e0 100644 --- a/docs/user-manual/config-reload.adoc +++ b/docs/user-manual/config-reload.adoc @@ -544,4 +544,7 @@ Adding, updating and removing an `<acceptor>` is supported, updating or removing === `<queues>` _(Deprecated)_ == Broker Properties -The location of xref:configuration-index.adoc#broker-properties[brokerProperties] files will be tracked for reload. Any property values that relfect reloadable parameters will take effect after the `configuration-file-refresh-period`. +The location of xref:configuration-index.adoc#broker-properties[brokerProperties] files will be tracked for reload. Any property values that reflect reloadable parameters will take effect after the `configuration-file-refresh-period`. + +[NOTE] +If programmatic configuration is in play, any configuration object passed to the broker must not contain reloadable config *and* have a broker properties source. The broker does not have any config reload callbacks at this time to protect the programmatic configuration. An IllegalStateException will be thrown if this arises. diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/routing/ElasticQueueTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/routing/ElasticQueueTest.java index 650599a56f..41cc2771a8 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/routing/ElasticQueueTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/routing/ElasticQueueTest.java @@ -427,6 +427,7 @@ public class ElasticQueueTest extends ActiveMQTestBase { .setAutoDeleteQueues(false).setAutoDeleteAddresses(false); // so slow consumer can kick in! Configuration baseConfig = new ConfigurationImpl(); + baseConfig.setConfigurationFileRefreshPeriod(-1); // the classpath has broker.properties we don't want to reload baseConfig.getAddressSettings().put(qName, blockingQueue); ConnectionRouterConfiguration connectionRouterConfiguration = new ConnectionRouterConfiguration(); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java index 1c08ce33da..a61d24e875 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java @@ -45,6 +45,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class ConfigurationTest extends ActiveMQTestBase { @@ -195,20 +196,21 @@ public class ConfigurationTest extends ActiveMQTestBase { properties.store(outStream, null); } assertTrue(propsFile.exists()); - properties.clear(); FileConfiguration fc = new FileConfiguration(); ActiveMQJAASSecurityManager sm = new ActiveMQJAASSecurityManager(InVMLoginModule.class.getName(), new SecurityConfiguration()); ActiveMQServer server = addServer(new ActiveMQServerImpl(fc, sm)); server.setProperties(propsFile.getAbsolutePath()); // no xml config try { - server.start(); assertEquals(1, server.getConfiguration().getConnectionRouters().size()); assertEquals("LF", server.getConfiguration().getConnectionRouters().get(0).getLocalTargetFilter()); assertEquals(1, server.getActiveMQServerControl().getAcceptors().length); - // verify update + assertEquals(100, server.getConfiguration().getConfigurationFileRefreshPeriod()); + + // verify update and remove of tcp acceptor + properties.clear(); properties.put("configurationFileRefreshPeriod", "100"); properties.put("persistenceEnabled", "false"); properties.put("connectionRouters.joe.localTargetFilter", "UPDATED"); @@ -222,6 +224,8 @@ public class ConfigurationTest extends ActiveMQTestBase { return "UPDATED".equals(server.getConfiguration().getConnectionRouters().get(0).getLocalTargetFilter()); }); + // no change + assertEquals(100, server.getConfiguration().getConfigurationFileRefreshPeriod()); // verify remove assertEquals(0, server.getActiveMQServerControl().getAcceptors().length); @@ -373,6 +377,35 @@ public class ConfigurationTest extends ActiveMQTestBase { } } + @Test + public void testPropertiesAndProgrammaticReloadableConfigArg() throws Exception { + + File propsFile = new File(getTestDirfile(), "somemore.props"); + propsFile.createNewFile(); + + + Properties properties = new ConfigurationImpl.InsertionOrderedProperties(); + properties.put("configurationFileRefreshPeriod", "100"); + properties.put("persistenceEnabled", "false"); + properties.put("acceptorConfigurations.reloadable.factoryClassName", NETTY_ACCEPTOR_FACTORY); + properties.put("acceptorConfigurations.reloadable.params.HOST", "LOCALHOST"); + properties.put("acceptorConfigurations.reloadable.params.PORT", "61616"); + + try (FileOutputStream outStream = new FileOutputStream(propsFile)) { + properties.store(outStream, null); + } + assertTrue(propsFile.exists()); + + ConfigurationImpl programmatic = new ConfigurationImpl(); + programmatic.addAcceptorConfiguration("tcp", "tcp://localhost:62618"); + ActiveMQJAASSecurityManager sm = new ActiveMQJAASSecurityManager(InVMLoginModule.class.getName(), new SecurityConfiguration()); + ActiveMQServer server = addServer(new ActiveMQServerImpl(programmatic, sm)); + + assertThrows(IllegalStateException.class, () -> { + server.setProperties(propsFile.getAbsolutePath()); + }); + } + protected ActiveMQServer getActiveMQServer(String brokerConfig) throws Exception { FileConfiguration fc = new FileConfiguration(); FileJMSConfiguration fileConfiguration = new FileJMSConfiguration(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
