Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/690#discussion_r118445857
--- Diff:
core/src/main/java/org/apache/brooklyn/location/byon/ByonLocationResolver.java
---
@@ -152,49 +153,55 @@ protected ConfigBag extractConfig(Map<?,?>
locationFlags, String spec, LocationR
String osFamily = (String)
machineConfig.remove(OS_FAMILY.getName());
String ssh = (String) machineConfig.remove("ssh");
- String winrm = (String) machineConfig.remove("winrm");
- Map<Integer, String> tcpPortMappings = (Map<Integer, String>)
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);
+ if (machineConfig.containsKey("winrm") &&
!(machineConfig.get("winrm") instanceof String)) {
--- End diff --
@iyovcheva The changes here break the support for specifying `winrm` as
`user@host:port`. Can you add it to the test cases.
On the other hand your approach is also interesting because it doesn't
resolve the value here - at the time of deploying the blueprint. Obtaining the
machine could happen much later in the lifecycle where the external values have
already changed.
A proper solution would involve moving all of non-essential parsing to the
machine location, leaving only minimal sanity checks here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
