diff --git a/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg b/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg index ae9b0ee9..aca1668f 100644 --- a/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg +++ b/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg @@ -6,7 +6,7 @@ # This user is equivelant to root. Must have '*' # permissions on the message bus. goss.system.manager = system -goss.system.manager.password = managera +goss.system.manager.password = manager # The following are used for the core-client connection. goss.openwire.uri = tcp://${activemq.host}:${openwire.port} diff --git a/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java b/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java index fe1657e2..cb0978b4 100644 --- a/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java +++ b/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java @@ -127,6 +127,9 @@ public class GridOpticsServer implements ServerControl { private static final String PROP_SYSTEM_MANAGER = "goss.system.manager"; private static final String PROP_SYSTEM_MANAGER_PASSWORD = "goss.system.manager.password"; + private static final String DEFAULT_SYSTEM_MANAGER_USER = "system"; + private static final String DEFAULT_SYSTEM_MANAGER_PASSWORD = "manager"; + private BrokerService broker; private Connection connection; private Session session; @@ -147,6 +150,13 @@ public class GridOpticsServer implements ServerControl { // Topic to listen on for receiving requests private String requestQueue = null; + // The broker connection principal used at createConnection(...). Defaults to + // system/manager (GADP-014 / issue #42) so a deployment that never sets + // goss.system.manager[.password] in pnnl.goss.core.server.cfg keeps today's + // behavior unchanged. + private String systemManagerUser = DEFAULT_SYSTEM_MANAGER_USER; + private String systemManagerPassword = DEFAULT_SYSTEM_MANAGER_PASSWORD; + // SSL Parameters private boolean sslEnabled = false; private String sslClientKeyStore = null; @@ -223,10 +233,10 @@ public synchronized void updated(Map properties) throws SystemEx if (properties != null) { - getProperty((String) properties.get(PROP_SYSTEM_MANAGER), - "system"); - getProperty((String) properties.get(PROP_SYSTEM_MANAGER_PASSWORD), - "manager"); + systemManagerUser = getProperty((String) properties.get(PROP_SYSTEM_MANAGER), + DEFAULT_SYSTEM_MANAGER_USER); + systemManagerPassword = getProperty((String) properties.get(PROP_SYSTEM_MANAGER_PASSWORD), + DEFAULT_SYSTEM_MANAGER_PASSWORD); shouldStartBroker = Boolean.parseBoolean( getProperty((String) properties.get(PROP_START_BROKER), "true")); @@ -449,10 +459,10 @@ public void start(Map properties) { connectionFactory = new ActiveMQConnectionFactory(openwireTransport); } - connection = connectionFactory.createConnection("system", "manager"); + connection = connectionFactory.createConnection(systemManagerUser, systemManagerPassword); connection.start(); } catch (Exception e) { - log.debug("Error Connecting to ActiveMQ", e); + log.warn("Error Connecting to ActiveMQ", e); if (shouldStartBroker) { try { if (broker != null) { diff --git a/pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java b/pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java new file mode 100644 index 00000000..7ed6f2bd --- /dev/null +++ b/pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java @@ -0,0 +1,96 @@ +package pnnl.goss.core.server.impl.test; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import pnnl.goss.core.server.impl.GridOpticsServer; + +/** + * GADP-014 / issue #42: the configured broker system-manager credential + * (goss.system.manager / goss.system.manager.password) was read via + * getProperty(...) but the returned values were discarded, so + * createConnection(...) always used the literal "system"/"manager" regardless + * of configuration. + * + * These tests assert the fix at the field level that createConnection(...) + * reads from: systemManagerUser / systemManagerPassword. GridOpticsServer has + * no OSGi container in a unit test, so updated(Map) is invoked directly (the + * same method @Activate's start(Map) delegates to before createConnection), and + * the resulting private fields are read via reflection since the class exposes + * no getters for them. + */ +public class GridOpticsServerBrokerCredentialTest { + + private static final String PROP_SYSTEM_MANAGER = "goss.system.manager"; + private static final String PROP_SYSTEM_MANAGER_PASSWORD = "goss.system.manager.password"; + + private String readSystemManagerUser(GridOpticsServer server) throws Exception { + return readPrivateField(server, "systemManagerUser"); + } + + private String readSystemManagerPassword(GridOpticsServer server) throws Exception { + return readPrivateField(server, "systemManagerPassword"); + } + + private String readPrivateField(GridOpticsServer server, String fieldName) throws Exception { + Field field = GridOpticsServer.class.getDeclaredField(fieldName); + field.setAccessible(true); + return (String) field.get(server); + } + + @Test + @DisplayName("Configured goss.system.manager / .password are honored when set") + public void configuredCredentialIsHonoredWhenSet() throws Exception { + GridOpticsServer server = new GridOpticsServer(); + Map properties = new HashMap<>(); + properties.put(PROP_SYSTEM_MANAGER, "configuredUser"); + properties.put(PROP_SYSTEM_MANAGER_PASSWORD, "configuredPassword"); + + server.updated(properties); + + assertThat(readSystemManagerUser(server)) + .as("the configured goss.system.manager value must reach the connection principal") + .isEqualTo("configuredUser"); + assertThat(readSystemManagerPassword(server)) + .as("the configured goss.system.manager.password value must reach the connection principal") + .isEqualTo("configuredPassword"); + } + + @Test + @DisplayName("system/manager defaults are used when the credential properties are absent") + public void defaultsToSystemManagerWhenPropertiesAbsent() throws Exception { + GridOpticsServer server = new GridOpticsServer(); + Map properties = new HashMap<>(); + // Deliberately omit PROP_SYSTEM_MANAGER / PROP_SYSTEM_MANAGER_PASSWORD: + // a deployment that never sets these must see unchanged behavior. + + server.updated(properties); + + assertThat(readSystemManagerUser(server)) + .as("absent goss.system.manager must default to 'system', preserving prior behavior") + .isEqualTo("system"); + assertThat(readSystemManagerPassword(server)) + .as("absent goss.system.manager.password must default to 'manager', preserving prior behavior") + .isEqualTo("manager"); + } + + @Test + @DisplayName("empty-string credential properties fall back to the system/manager default") + public void defaultsToSystemManagerWhenPropertiesEmpty() throws Exception { + GridOpticsServer server = new GridOpticsServer(); + Map properties = new HashMap<>(); + properties.put(PROP_SYSTEM_MANAGER, ""); + properties.put(PROP_SYSTEM_MANAGER_PASSWORD, ""); + + server.updated(properties); + + assertThat(readSystemManagerUser(server)).isEqualTo("system"); + assertThat(readSystemManagerPassword(server)).isEqualTo("manager"); + } +}