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 440abab3767..9d781e559aa 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 @@ -1545,4 +1545,5 @@ default boolean isUsingDatabasePersistence() { } Map getJaasConfigs(); + Configuration setJaasConfigs(Map configs); } 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 85e21296e95..9b6dc9cf3dc 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 @@ -1040,6 +1040,15 @@ public void addJaasConfig(JaasAppConfiguration config) { jaasConfigs.put(config.getName(), config); } + @Override + public Configuration setJaasConfigs(Map configs) { + jaasConfigs.putAll(configs); + // prune removed entries after update to retain existing entries, this is live config referenced by jaas + // see org.apache.activemq.artemis.core.config.impl.ConfigurationImpl.getAppConfigurationEntry + jaasConfigs.keySet().retainAll(configs.keySet()); + return this; + } + private void writeProperties(FileWriter writer) throws Exception { final BeanUtilsBean beanUtilsBean = new BeanUtilsBean(); beanUtilsBean.getPropertyUtils().addBeanIntrospector(new FluentPropertyBeanIntrospectorWithIgnores()); 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 336b81f43a4..a9aa48b7659 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 CriticalAnalyzer getCriticalAnalyzer() { } @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 @@ -4682,22 +4692,14 @@ public void reloadConfigurationFile() throws Exception { } private void reloadConfigurationFile(URL xmlConfigUri) throws Exception { + Configuration config = new ConfigurationImpl(); if (xmlConfigUri != null) { - Configuration config = new FileConfigurationParser().parseMainConfig(xmlConfigUri.openStream()); + config = new FileConfigurationParser().parseMainConfig(xmlConfigUri.openStream()); LegacyJMSConfiguration legacyJMSConfiguration = new LegacyJMSConfiguration(config); legacyJMSConfiguration.parseConfiguration(xmlConfigUri.openStream()); - configuration.setSecurityRoles(config.getSecurityRoles()); - configuration.setAddressSettings(config.getAddressSettings()); - configuration.setDivertConfigurations(config.getDivertConfigurations()); - configuration.setAddressConfigurations(config.getAddressConfigurations()); - configuration.setQueueConfigs(config.getQueueConfigs()); - configuration.setBridgeConfigurations(config.getBridgeConfigurations()); - configuration.setConnectorConfigurations(config.getConnectorConfigurations()); - configuration.setAcceptorConfigurations(config.getAcceptorConfigurations()); - configuration.setAMQPConnectionConfigurations(config.getAMQPConnection()); - configuration.setPurgePageFolders(config.isPurgePageFolders()); } - configuration.parseProperties(propertiesFileUrl); + config.parseProperties(propertiesFileUrl); + updateReloadableConfigurationFrom(config); updateStatus(ServerStatus.CONFIGURATION_COMPONENT, configuration.getStatus()); configurationReloadDeployed.set(false); if (isActive()) { @@ -4705,6 +4707,36 @@ private void reloadConfigurationFile(URL xmlConfigUri) throws Exception { } } + private void updateReloadableConfigurationFrom(Configuration config) { + configuration.setStatus(config.getStatus()); + configuration.setSecurityRoles(config.getSecurityRoles()); + configuration.setAddressSettings(config.getAddressSettings()); + configuration.setDivertConfigurations(config.getDivertConfigurations()); + configuration.setAddressConfigurations(config.getAddressConfigurations()); + configuration.setQueueConfigs(config.getQueueConfigs()); + configuration.setBridgeConfigurations(config.getBridgeConfigurations()); + configuration.setConnectorConfigurations(config.getConnectorConfigurations()); + configuration.setAcceptorConfigurations(config.getAcceptorConfigurations()); + configuration.setAMQPConnectionConfigurations(config.getAMQPConnection()); + configuration.setPurgePageFolders(config.isPurgePageFolders()); + configuration.setConnectionRouters(config.getConnectionRouters()); + configuration.setJaasConfigs(config.getJaasConfigs()); + } + + 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 { if (configurationReloadDeployed.compareAndSet(false, true)) { ActiveMQServerLogger.LOGGER.reloadingConfiguration("security settings"); diff --git a/docs/user-manual/config-reload.adoc b/docs/user-manual/config-reload.adoc index 66bf4fd63c1..b8c6e828e04 100644 --- a/docs/user-manual/config-reload.adoc +++ b/docs/user-manual/config-reload.adoc @@ -544,4 +544,7 @@ Adding, updating and removing an `` is supported, updating or removing === `` _(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 650599a56f2..41cc2771a8d 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 @@ private void prepareNodesAndStartCombinedHeadTail() throws Exception { .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 329f2cf5066..a61d24e875a 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 @@ -16,12 +16,9 @@ */ package org.apache.activemq.artemis.tests.integration.server; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertTrue; - import java.io.File; import java.io.FileOutputStream; +import java.io.StringReader; import java.util.Properties; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -36,13 +33,21 @@ import org.apache.activemq.artemis.core.server.ActiveMQServer; import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl; import org.apache.activemq.artemis.jms.server.config.impl.FileJMSConfiguration; +import org.apache.activemq.artemis.json.JsonObject; import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager; import org.apache.activemq.artemis.spi.core.security.jaas.InVMLoginModule; import org.apache.activemq.artemis.tests.util.ActiveMQTestBase; +import org.apache.activemq.artemis.utils.JsonLoader; import org.apache.activemq.artemis.utils.RandomUtil; import org.apache.activemq.artemis.tests.util.Wait; 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 { @Test @@ -183,6 +188,9 @@ public void testPropertiesOnlyConfigReload() throws Exception { properties.put("configurationFileRefreshPeriod", "100"); properties.put("persistenceEnabled", "false"); properties.put("connectionRouters.joe.localTargetFilter", "LF"); + properties.put("acceptorConfigurations.tcp.factoryClassName", NETTY_ACCEPTOR_FACTORY); + properties.put("acceptorConfigurations.tcp.params.HOST", "LOCALHOST"); + properties.put("acceptorConfigurations.tcp.params.PORT", "61616"); try (FileOutputStream outStream = new FileOutputStream(propsFile)) { properties.store(outStream, null); @@ -194,17 +202,20 @@ public void testPropertiesOnlyConfigReload() throws Exception { 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); + assertEquals(100, server.getConfiguration().getConfigurationFileRefreshPeriod()); - properties.put("persistenceEnabled", "false"); + // verify update and remove of tcp acceptor + properties.clear(); properties.put("configurationFileRefreshPeriod", "100"); - - // verify update + properties.put("persistenceEnabled", "false"); properties.put("connectionRouters.joe.localTargetFilter", "UPDATED"); + + String startedStatus = server.getStatus(); try (FileOutputStream outStream = new FileOutputStream(propsFile)) { properties.store(outStream, null); } @@ -213,6 +224,22 @@ public void testPropertiesOnlyConfigReload() throws Exception { return "UPDATED".equals(server.getConfiguration().getConnectionRouters().get(0).getLocalTargetFilter()); }); + // no change + assertEquals(100, server.getConfiguration().getConfigurationFileRefreshPeriod()); + // verify remove + assertEquals(0, server.getActiveMQServerControl().getAcceptors().length); + + // verify status json reflects update + String updatedStatus = server.getStatus(); + assertNotEquals(startedStatus, updatedStatus); + assertTrue(startedStatus.contains(propsFile.getName())); + assertTrue(updatedStatus.contains(propsFile.getName())); + JsonObject jsonStarted = JsonLoader.readObject(new StringReader(startedStatus)); + JsonObject jsonUpdated = JsonLoader.readObject(new StringReader(updatedStatus)); + String alder32Used = jsonStarted.getJsonObject("configuration").getJsonObject("properties").getJsonObject(propsFile.getName()).getString("fileAlder32"); + String alder32Updated = jsonUpdated.getJsonObject("configuration").getJsonObject("properties").getJsonObject(propsFile.getName()).getString("fileAlder32"); + assertNotEquals(alder32Used, alder32Updated); + } finally { try { server.stop(); @@ -350,6 +377,35 @@ public void testPropertiesDirWithFilterConfigReloadOnNewFileAfterGettingJournalL } } + @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();