fix: honor configured broker manager credential (#42)

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}

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<String, Object> 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<String, Object> 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) {

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<String, Object> 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<String, Object> 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<String, Object> 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");
+    }
+}