diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java index 681367d956..bf73dd83b7 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/WinRmMachineLocationExternalConfigYamlTest.java @@ -24,6 +24,7 @@ import org.apache.brooklyn.core.test.entity.LocalManagementContextForTests; import org.apache.brooklyn.entity.stock.BasicApplication; import org.apache.brooklyn.location.winrm.WinRmMachineLocation; +import org.apache.brooklyn.test.Asserts; import org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,6 +42,8 @@ protected LocalManagementContext newTestManagementContext() { BrooklynProperties props = BrooklynProperties.Factory.newEmpty(); props.put("brooklyn.external.inPlaceSupplier1", "org.apache.brooklyn.core.config.external.InPlaceExternalConfigSupplier"); props.put("brooklyn.external.inPlaceSupplier1.byonPassword", "passw0rd"); + props.put("brooklyn.external.inPlaceSupplier1.byonUser", "admin"); + props.put("brooklyn.external.inPlaceSupplier1.ip", "127.0.0.1"); return LocalManagementContextForTests.builder(true) .useProperties(props) @@ -49,13 +52,13 @@ protected LocalManagementContext newTestManagementContext() { } @Test() - public void testWindowsMachinesPasswordExternalProvider() throws Exception { + public void testWindowsMachinesExternalProvider() throws Exception { RecordingWinRmTool.constructorProps.clear(); final String yaml = Joiner.on("\n").join("location:", " byon:", " hosts:", - " - winrm: 127.0.0.1", - " user: admin", + " - winrm: $brooklyn:external(\"inPlaceSupplier1\", \"ip\")", + " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUser\")", " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPassword\")", " osFamily: windows", @@ -67,17 +70,19 @@ public void testWindowsMachinesPasswordExternalProvider() throws Exception { BasicApplication app = (BasicApplication) createAndStartApplication(yaml); waitForApplicationTasks(app); + assertEquals(RecordingWinRmTool.constructorProps.get(0).get("host"), "127.0.0.1"); + assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.USER.getName()), "admin"); assertEquals(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.PASSWORD.getName()), "passw0rd"); } @Test() - public void testWindowsMachinesNoPasswordExternalProvider() throws Exception { + public void testWindowsMachinesNoExternalProvider() throws Exception { RecordingWinRmTool.constructorProps.clear(); final String yaml = Joiner.on("\n").join("location:", " byon:", " hosts:", " - winrm: 127.0.0.1", - " user: admin", + " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUserEmpty\")", " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPasswordddd\")", " osFamily: windows", @@ -89,9 +94,36 @@ public void testWindowsMachinesNoPasswordExternalProvider() throws Exception { BasicApplication app = (BasicApplication) createAndStartApplication(yaml); waitForApplicationTasks(app); + assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.USER.getName())); assertNull(RecordingWinRmTool.constructorProps.get(0).get(WinRmMachineLocation.PASSWORD.getName())); } + @Test() + public void testWindowsMachinesNoExternalIPProvider() throws Exception { + RecordingWinRmTool.constructorProps.clear(); + final String yaml = Joiner.on("\n").join("location:", + " byon:", + " hosts:", + " - winrm: $brooklyn:external(\"inPlaceSupplier1\", \"ipEmpty\")", + " user: $brooklyn:external(\"inPlaceSupplier1\", \"byonUserEmpty\")", + " brooklyn.winrm.config.winrmToolClass: org.apache.brooklyn.util.core.internal.winrm.RecordingWinRmTool", + " password: $brooklyn:external(\"inPlaceSupplier1\", \"byonPasswordddd\")", + " osFamily: windows", + "services:", + "- type: org.apache.brooklyn.entity.software.base.VanillaWindowsProcess", + " brooklyn.config:", + " launch.command: echo launch", + " checkRunning.command: echo running"); + + try { + BasicApplication app = (BasicApplication) createAndStartApplication(yaml); + waitForApplicationTasks(app); + Asserts.shouldHaveFailedPreviously(); + } catch (Exception e) { + Asserts.expectedFailureContains(e, "Must specify exactly one of 'ssh' or 'winrm' for machine"); + } + } + @Override protected Logger getLogger() { return log; diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java index 156b720a01..009ee05eef 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java @@ -34,6 +34,7 @@ import org.apache.brooklyn.core.config.Sanitizer; import org.apache.brooklyn.core.location.AbstractLocationResolver; import org.apache.brooklyn.core.mgmt.internal.LocalLocationManager; +import org.apache.brooklyn.core.objs.BasicConfigurableObject; import org.apache.brooklyn.util.collections.MutableMap; import org.apache.brooklyn.util.core.ClassLoaderUtils; import org.apache.brooklyn.util.core.config.ConfigBag; @@ -133,7 +134,7 @@ protected ConfigBag extractConfig(Map locationFlags, String spec, LocationR if (host instanceof String) { machineSpec = parseMachine((String)host, locationClass, defaultProps, spec); } else if (host instanceof Map) { - machineSpec = parseMachine((Map)host, locationClass, defaultProps, spec); + machineSpec = parseMachine((Map)host, locationClass, defaultProps); } else { throw new IllegalArgumentException("Expected machine to be String or Map, but was "+host.getClass().getName()+" ("+host+")"); } @@ -146,62 +147,16 @@ protected ConfigBag extractConfig(Map locationFlags, String spec, LocationR return config; } - protected LocationSpec parseMachine(Map vals, Class locationClass, Map defaults, String specForErrMsg) { - Map valSanitized = Sanitizer.sanitize(vals); + protected LocationSpec parseMachine(Map vals, Class locationClass, Map defaults) { Map machineConfig = MutableMap.copyOf(vals); String osFamily = (String) machineConfig.remove(OS_FAMILY.getName()); - String ssh = (String) machineConfig.remove("ssh"); - String winrm = (String) machineConfig.remove("winrm"); - Map tcpPortMappings = (Map) machineConfig.get("tcpPortMappings"); - - checkArgument(ssh != null ^ winrm != null, "Must specify exactly one of 'ssh' or 'winrm' for machine: %s", valSanitized); - - UserAndHostAndPort userAndHostAndPort; - String host; - int port; - if (ssh != null) { - userAndHostAndPort = parseUserAndHostAndPort(ssh, 22); - } else { - // TODO set to null and rely on the MachineLocation. If not then make a dependency to WinRmMachineLocation and use its config key name. - userAndHostAndPort = parseUserAndHostAndPort(winrm, vals.get("winrm.useHttps") != null && (Boolean)vals.get("winrm.useHttps") ? 5986 : 5985); - } - - // If there is a tcpPortMapping defined for the connection-port, then use that for ssh/winrm machine - port = userAndHostAndPort.getHostAndPort().getPort(); - if (tcpPortMappings != null && tcpPortMappings.containsKey(port)) { - String override = tcpPortMappings.get(port); - HostAndPort hostAndPortOverride = HostAndPort.fromString(override); - if (!hostAndPortOverride.hasPort()) { - throw new IllegalArgumentException("Invalid portMapping ('"+override+"') for port "+port+" in "+specForErrMsg); - } - port = hostAndPortOverride.getPort(); - host = hostAndPortOverride.getHostText().trim(); - } else { - host = userAndHostAndPort.getHostAndPort().getHostText().trim(); - } - - machineConfig.put("address", host); - try { - InetAddress.getByName(host); - } catch (Exception e) { - throw new IllegalArgumentException("Invalid host '"+host+"' specified in '"+specForErrMsg+"': "+e); - } - - if (userAndHostAndPort.getUser() != null) { - checkArgument(!vals.containsKey("user"), "Must not specify user twice for machine: %s", valSanitized); - machineConfig.put("user", userAndHostAndPort.getUser()); - } - if (userAndHostAndPort.getHostAndPort().hasPort()) { - checkArgument(!vals.containsKey("port"), "Must not specify port twice for machine: %s", valSanitized); - machineConfig.put("port", port); - } for (Map.Entry entry : defaults.entrySet()) { if (!machineConfig.containsKey(entry.getKey())) { machineConfig.put(entry.getKey(), entry.getValue()); } } - + Class locationClassHere = locationClass; if (osFamily != null) { locationClassHere = getLocationClass(osFamily); diff --git a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java index 87b83f8928..648285a7d5 100644 --- a/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java +++ b/core/src/main/java/org/apache/brooklyn/location/byon/FixedListMachineProvisioningLocation.java @@ -22,12 +22,15 @@ import java.io.Closeable; import java.io.File; +import java.net.InetAddress; import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; import java.util.Set; +import com.google.common.base.Preconditions; +import com.google.common.net.HostAndPort; import org.apache.brooklyn.api.location.Location; import org.apache.brooklyn.api.location.LocationSpec; import org.apache.brooklyn.api.location.MachineLocation; @@ -45,6 +48,7 @@ import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.SetFromFlag; +import org.apache.brooklyn.util.net.UserAndHostAndPort; import org.apache.brooklyn.util.stream.Streams; import org.apache.brooklyn.util.text.WildcardGlobs; import org.apache.brooklyn.util.text.WildcardGlobs.PhraseTreatment; @@ -317,7 +321,7 @@ public T obtain(Map flags) throws NoMachinesAvailableException { T desiredMachine = (T) flags.get("desiredMachine"); ConfigBag allflags = ConfigBag.newInstanceExtending(config().getBag()).putAll(flags); Function, MachineLocation> chooser = allflags.get(MACHINE_CHOOSER); - + synchronized (lock) { Set a = getAvailable(); if (a.isEmpty()) { @@ -381,6 +385,10 @@ protected void updateMachineConfig(T machine, Map flags) { // For backwards compatibility, where peristed state did not have this. origConfigs = Maps.newLinkedHashMap(); } + if (((AbstractLocation) machine).config().getBag().getAllConfig().get("provider") != null && + ((AbstractLocation) machine).config().getBag().getAllConfig().get("provider").equals("byon")) { + parseMachineConfig((AbstractLocation) machine); + } Map strFlags = ConfigBag.newInstance(flags).getAllConfig(); Map origConfig = ((ConfigurationSupportInternal)machine.config()).getLocalBag().getAllConfig(); origConfigs.put(machine, origConfig); @@ -388,6 +396,78 @@ protected void updateMachineConfig(T machine, Map flags) { ((ConfigurationSupportInternal)machine.config()).putAll(strFlags); } + + private void parseMachineConfig(AbstractLocation machine) { + String ssh = machine.config().get(ConfigKeys.newStringConfigKey("ssh")); + machine.config().removeKey(ConfigKeys.newStringConfigKey("ssh")); + String winrm = machine.config().get(ConfigKeys.newStringConfigKey("winrm")); + machine.config().removeKey(ConfigKeys.newStringConfigKey("winrm")); + + if (ssh ==null && winrm == null && machine.config().get(ConfigKeys.newStringConfigKey("address")) != null) { + return; + } + + Preconditions.checkArgument(ssh != null || winrm != null, "Must specify exactly one of 'ssh' or 'winrm' for machine: " + machine); + + Map tcpPortMappings = (Map) machine.config().get(ConfigKeys.newConfigKey(Map.class, "tcpPortMappings")); + + UserAndHostAndPort userAndHostAndPort; + String host; + int port; + if (ssh != null) { + userAndHostAndPort = parseUserAndHostAndPort(ssh, 22); + } else { + // TODO set to null and rely on the MachineLocation. If not then make a dependency to WinRmMachineLocation and use its config key name. + Boolean useHttps = machine.config().get(ConfigKeys.newConfigKey(Boolean.class,"winrm.useHttps")); + userAndHostAndPort = parseUserAndHostAndPort(winrm, useHttps != null && useHttps ? 5986 : 5985); + } + + // If there is a tcpPortMapping defined for the connection-port, then use that for ssh/winrm machine + port = userAndHostAndPort.getHostAndPort().getPort(); + if (tcpPortMappings != null && tcpPortMappings.containsKey(port)) { + String override = tcpPortMappings.get(port); + HostAndPort hostAndPortOverride = HostAndPort.fromString(override); + if (!hostAndPortOverride.hasPort()) { + throw new IllegalArgumentException("Invalid portMapping ('"+override+"') for port "+port+" in machine configuration "+machine.config()); + } + port = hostAndPortOverride.getPort(); + host = hostAndPortOverride.getHostText().trim(); + } else { + host = userAndHostAndPort.getHostAndPort().getHostText().trim(); + } + + machine.config().set(ConfigKeys.newStringConfigKey("address"), host); + try { + InetAddress.getByName(host); + } catch (Exception e) { + throw new IllegalArgumentException("Invalid host '"+host+"' specified in '"+machine+"': "+e); + } + + if (userAndHostAndPort.getUser() != null) { + machine.config().set(ConfigKeys.newStringConfigKey("user"), userAndHostAndPort.getUser()); + } + if (userAndHostAndPort.getHostAndPort().hasPort()) { + machine.config().set(ConfigKeys.newConfigKey(Integer.class,"port"), port); + } + } + + private UserAndHostAndPort parseUserAndHostAndPort(String val) { + String userPart = null; + String hostPart = val; + if (val.contains("@")) { + userPart = val.substring(0, val.indexOf("@")); + hostPart = val.substring(val.indexOf("@")+1); + } + return UserAndHostAndPort.fromParts(userPart, HostAndPort.fromString(hostPart)); + } + + private UserAndHostAndPort parseUserAndHostAndPort(String val, int defaultPort) { + UserAndHostAndPort result = parseUserAndHostAndPort(val); + if (!result.getHostAndPort().hasPort()) { + result = UserAndHostAndPort.fromParts(result.getUser(), result.getHostAndPort().getHostText(), defaultPort); + } + return result; + } protected void restoreMachineConfig(MachineLocation machine) { if (origConfigs == null) {