# HG changeset patch # User Severin Gehwolf # Date 1479143241 -3600 # Node ID 92de1fab95ec64145d08c04412bc199170a148f2 # Parent f09130ac15b7fa08a3f6365284ae2485102d9ad0 Make published agent address configurable. For hosts sitting behind a router, the config listen address of the agent's command channel might not be resolvable by Thermostat clients in front of the router. In those cases it's useful to be able to configure the address Thermostat shall publish to storage for clients to use for establishing command channel connections to agents. Reviewed-by: neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-November/021689.html PR3231 diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/main/java/com/redhat/thermostat/agent/Agent.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/Agent.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/Agent.java Mon Nov 14 18:07:21 2016 +0100 @@ -158,8 +158,10 @@ AgentInformation agentInfo = new AgentInformation(writerId); agentInfo.setStartTime(config.getStartTime()); agentInfo.setAlive(true); - HostPortPair hostPort = config.getConfigListenAddress(); - agentInfo.setConfigListenAddress(hostPort.toExternalForm()); + // Report the configured publish address if any. Otherwise, + // defaults to (agent-local) configured listen address. + HostPortPair publishAddress = config.getConfigPublishAddress(); + agentInfo.setConfigListenAddress(publishAddress.toExternalForm()); return agentInfo; } diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentConfigsUtils.java Mon Nov 14 18:07:21 2016 +0100 @@ -100,6 +100,11 @@ // accept any connection configuration.setConfigListenAddress("127.0.0.1:12000"); } + + String configPublishAddress = properties.getProperty(AgentProperties.CONFIG_PUBLISH_ADDRESS.name()); + if (configPublishAddress != null) { + configuration.setConfigPublishAddress(configPublishAddress); + } } } diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentProperties.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentProperties.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentProperties.java Mon Nov 14 18:07:21 2016 +0100 @@ -41,5 +41,6 @@ DB_URL, SAVE_ON_EXIT, CONFIG_LISTEN_ADDRESS, + CONFIG_PUBLISH_ADDRESS, } diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/config/AgentStartupConfiguration.java Mon Nov 14 18:07:21 2016 +0100 @@ -47,7 +47,8 @@ private boolean purge; private String url; private long startTime; - private HostPortPair hostPort; + private HostPortPair listenAddr; + private HostPortPair publishAddr; AgentStartupConfiguration() { } @@ -79,17 +80,34 @@ } public void setConfigListenAddress(String address) { + this.listenAddr = parseAddress(address); + } + + public HostPortPair getConfigListenAddress() { + return listenAddr; + } + + public void setConfigPublishAddress(String address) { + this.publishAddr = parseAddress(address); + } + + public HostPortPair getConfigPublishAddress() { + if (publishAddr != null) { + return publishAddr; + } + // Otherwise default to configured listen address + // as the publish address for backwards compat reasons. + return listenAddr; + } + + private HostPortPair parseAddress(String address) throws AssertionError { HostPortsParser parser = new HostPortsParser(address); parser.parse(); List list = parser.getHostsPorts(); if (list.size() != 1) { throw new AssertionError("Multiple listen addresses not supported! Got: " + address); } - this.hostPort = parser.getHostsPorts().get(0); - } - - public HostPortPair getConfigListenAddress() { - return hostPort; + return parser.getHostsPorts().get(0); } } diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/test/java/com/redhat/thermostat/agent/AgentTest.java --- a/agent/core/src/test/java/com/redhat/thermostat/agent/AgentTest.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/AgentTest.java Mon Nov 14 18:07:21 2016 +0100 @@ -82,7 +82,7 @@ config = mock(AgentStartupConfiguration.class); when(config.getStartTime()).thenReturn(123L); when(config.purge()).thenReturn(true); - when(config.getConfigListenAddress()).thenReturn(new HostPortPair("foo", 23)); + when(config.getConfigPublishAddress()).thenReturn(new HostPortPair("foo", 23)); storage = mock(Storage.class); agentInfoDao = mock(AgentInfoDAO.class); @@ -199,7 +199,7 @@ AgentStartupConfiguration config = mock(AgentStartupConfiguration.class); when(config.getStartTime()).thenReturn(123L); when(config.purge()).thenReturn(false); - when(config.getConfigListenAddress()).thenReturn(new HostPortPair("foo", 23)); + when(config.getConfigPublishAddress()).thenReturn(new HostPortPair("foo", 23)); WriterID id = mock(WriterID.class); Agent agent = new Agent(backendRegistry, config, storage, agentInfoDao, backendInfoDao, id); diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java --- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentConfigsUtilsTest.java Mon Nov 14 18:07:21 2016 +0100 @@ -88,6 +88,11 @@ HostPortPair hostPorts = config.getConfigListenAddress(); Assert.assertEquals("42.42.42.42", hostPorts.getHost()); Assert.assertEquals(42, hostPorts.getPort()); + + // Not explicitly set, should default to config listen address + HostPortPair publishAddr = config.getConfigPublishAddress(); + Assert.assertEquals("42.42.42.42", publishAddr.getHost()); + Assert.assertEquals(42, publishAddr.getPort()); } @Test @@ -120,6 +125,11 @@ HostPortPair hostPorts = config.getConfigListenAddress(); Assert.assertEquals("24.24.24.24", hostPorts.getHost()); Assert.assertEquals(24, hostPorts.getPort()); + + // Not explicitly set, should default to config listen address + HostPortPair publishAddr = config.getConfigPublishAddress(); + Assert.assertEquals("24.24.24.24", publishAddr.getHost()); + Assert.assertEquals(24, publishAddr.getPort()); } @Test @@ -129,9 +139,43 @@ setConfigs(sysProps, new Properties()); AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); - HostPortPair hostPorts = config.getConfigListenAddress(); - Assert.assertEquals("::1", hostPorts.getHost()); - Assert.assertEquals(12000, hostPorts.getPort()); + HostPortPair listenAddr = config.getConfigListenAddress(); + Assert.assertEquals("::1", listenAddr.getHost()); + Assert.assertEquals(12000, listenAddr.getPort()); + } + + @Test + public void canOptionallySetSystemPublishAddress() { + Properties sysProps = createSystemProperties(); + sysProps.put("CONFIG_PUBLISH_ADDRESS", "foo.example.com:9999"); + setConfigs(sysProps, new Properties()); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + HostPortPair listenAddr = config.getConfigListenAddress(); + Assert.assertEquals("42.42.42.42", listenAddr.getHost()); + Assert.assertEquals(42, listenAddr.getPort()); + + HostPortPair publishAddr = config.getConfigPublishAddress(); + Assert.assertEquals("foo.example.com", publishAddr.getHost()); + Assert.assertEquals(9999, publishAddr.getPort()); + } + + @Test + public void canOptionallySetUserPublishAddress() { + Properties sysProps = createSystemProperties(); + sysProps.put("CONFIG_PUBLISH_ADDRESS", "foo.example.com:9999"); + Properties userProps = createUserProperties(); + userProps.put("CONFIG_PUBLISH_ADDRESS", "33.33.33.33:9333"); + setConfigs(sysProps, userProps); + AgentStartupConfiguration config = AgentConfigsUtils.createAgentConfigs(); + + HostPortPair listenAddr = config.getConfigListenAddress(); + Assert.assertEquals("24.24.24.24", listenAddr.getHost()); + Assert.assertEquals(24, listenAddr.getPort()); + + HostPortPair publishAddr = config.getConfigPublishAddress(); + Assert.assertEquals("33.33.33.33", publishAddr.getHost()); + Assert.assertEquals(9333, publishAddr.getPort()); } private Properties createSystemProperties(String configListenAddress) { diff -r f09130ac15b7 -r 92de1fab95ec agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStartupConfigurationTest.java --- a/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStartupConfigurationTest.java Mon Nov 14 15:19:00 2016 +0100 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/config/AgentStartupConfigurationTest.java Mon Nov 14 18:07:21 2016 +0100 @@ -37,6 +37,8 @@ package com.redhat.thermostat.agent.config; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import org.junit.Before; import org.junit.Test; @@ -70,4 +72,25 @@ assertEquals("::1", hostPort.getHost()); assertEquals(12000, hostPort.getPort()); } + + @Test + public void noPublishAddressDefaultsToListenAddress() { + config.setConfigListenAddress(IP_V6_LISTEN_ADDRESS); + HostPortPair publishAddr = config.getConfigPublishAddress(); + assertNotNull(publishAddr); + assertEquals("::1", publishAddr.getHost()); + assertEquals(12000, publishAddr.getPort()); + } + + @Test + public void canSetPublishAddressIndependentOfListenAddress() { + config.setConfigListenAddress(IP_V6_LISTEN_ADDRESS); + config.setConfigPublishAddress("foo.example.com:9212"); + HostPortPair publishAddr = config.getConfigPublishAddress(); + assertNotNull(publishAddr); + assertEquals("foo.example.com", publishAddr.getHost()); + assertEquals(9212, publishAddr.getPort()); + HostPortPair listenAddr = config.getConfigListenAddress(); + assertFalse(listenAddr.toExternalForm().equals(publishAddr.toExternalForm())); + } } diff -r f09130ac15b7 -r 92de1fab95ec distribution/config/agent.properties --- a/distribution/config/agent.properties Mon Nov 14 15:19:00 2016 +0100 +++ b/distribution/config/agent.properties Mon Nov 14 18:07:21 2016 +0100 @@ -10,6 +10,14 @@ # [1fff:0:a88:85a3::ac1f]:12000 CONFIG_LISTEN_ADDRESS=127.0.0.1:12000 +# Related to CONFIG_LISTEN_ADDRESS. CONFIG_PUBLISH_ADDRESS will be used +# as an address advertised to clients to reach the agent. If not specified +# defaults to CONFIG_LISTEN_ADDRESS. This config value is useful if the +# listen address of the agent is some local-only resolvable address. +# In that case, specify the globally reachable address of the agent via +# CONFIG_PUBLISH_ADDRESS. See CONFIG_LISTEN_ADDRESS for supported formats. +#CONFIG_PUBLISH_ADDRESS=agent.example.com:12000 + # Connection URL to storage. This can be overridden with the -d option # on the command line. DB_URL=http://127.0.0.1:8999/thermostat/storage