Skip to content

ARTEMIS-5871 restrict reload of properties config to the set of reloa… #6205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gtully wants to merge 1 commit into
base: main
Choose a base branch
from
+59 −20
Open

ARTEMIS-5871 restrict reload of properties config to the set of reloa… #6205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions artemis-server/src/main/java/org/apache/activemq/artemis/core/config/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1545,4 +1545,5 @@ default boolean isUsingDatabasePersistence() {
}

Map<String, JaasAppConfiguration> getJaasConfigs();
Configuration setJaasConfigs(Map<String, JaasAppConfiguration> configs);
}
9 changes: 9 additions & 0 deletions ...-server/src/main/java/org/apache/activemq/artemis/core/config/impl/ConfigurationImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,15 @@ public void addJaasConfig(JaasAppConfiguration config) {
jaasConfigs.put(config.getName(), config);
}

@Override
public Configuration setJaasConfigs(Map<String, JaasAppConfiguration> 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());
Expand Down
30 changes: 18 additions & 12 deletions ...server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4682,22 +4682,28 @@ 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);
configuration.setStatus(config.getStatus());

Copy link
Contributor

@tabish121 tabish121 Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to disregard any configuration that was assigned programmatically via an initial configuration object when it sets the configuration from the reload of the possible XML file and then the properties file. It creates an empty configuration vessel which it might fill in from XML and or a properties file(s) set and then hard sets those entries into the existing configuration regardless of the differences between old and new which could lose original data. Its not clear when or why these differing configuration sets take precedence over each other.

Copy link
Contributor Author

@gtully gtully Feb 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is exactly the intent, the new config is just for the purpose of being reloaded by the reload logic. There is currently no callback for programatic config. Typically broker config is read once at startup. The reload logic is restricted to the set of accessors that support reload, the bits that are copied over after a config is reread. Then the deployReloadableConfigFromConfiguration method does the heavy lifting of applying any changes or not to individual components.

So the difference check is all in deployReloadableConfigFromConfiguration

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()); // needs reload logic
configuration.setJaasConfigs(config.getJaasConfigs());

updateStatus(ServerStatus.CONFIGURATION_COMPONENT, configuration.getStatus());
configurationReloadDeployed.set(false);
if (isActive()) {
Expand Down
39 changes: 31 additions & 8 deletions ...src/test/java/org/apache/activemq/artemis/tests/integration/server/ConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -36,13 +33,20 @@
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.assertTrue;

public class ConfigurationTest extends ActiveMQTestBase {

@Test
Expand Down Expand Up @@ -183,11 +187,15 @@ 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);
}
assertTrue(propsFile.exists());
properties.clear();

FileConfiguration fc = new FileConfiguration();
ActiveMQJAASSecurityManager sm = new ActiveMQJAASSecurityManager(InVMLoginModule.class.getName(), new SecurityConfiguration());
Expand All @@ -199,12 +207,13 @@ public void testPropertiesOnlyConfigReload() throws Exception {

assertEquals(1, server.getConfiguration().getConnectionRouters().size());
assertEquals("LF", server.getConfiguration().getConnectionRouters().get(0).getLocalTargetFilter());

properties.put("persistenceEnabled", "false");
properties.put("configurationFileRefreshPeriod", "100");

assertEquals(1, server.getActiveMQServerControl().getAcceptors().length);
// verify update
properties.put("configurationFileRefreshPeriod", "100");
properties.put("persistenceEnabled", "false");
properties.put("connectionRouters.joe.localTargetFilter", "UPDATED");

String startedStatus = server.getStatus();
try (FileOutputStream outStream = new FileOutputStream(propsFile)) {
properties.store(outStream, null);
}
Expand All @@ -213,6 +222,20 @@ public void testPropertiesOnlyConfigReload() throws Exception {
return "UPDATED".equals(server.getConfiguration().getConnectionRouters().get(0).getLocalTargetFilter());
});

// 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();
Expand Down