fixes for using hard-coded password, and loginUser.password, tested on vsphere (thanks @andreaturli #334); and warnings on misuse
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/e7fb0d40 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/e7fb0d40 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/e7fb0d40 Branch: refs/heads/0.4.0 Commit: e7fb0d402a500fa8e9dbb5223b7f8eae9bdf8f97 Parents: 31b5c07 Author: Alex Heneveld <[email protected]> Authored: Wed Oct 3 10:58:29 2012 +0100 Committer: Alex Heneveld <[email protected]> Committed: Wed Oct 3 12:30:06 2012 +0100 ---------------------------------------------------------------------- .../location/basic/SshMachineLocation.java | 5 +- .../location/basic/jclouds/JcloudsLocation.java | 54 ++++++++------------ .../brooklyn/util/internal/ssh/SshjTool.java | 45 ++++++++++++---- 3 files changed, 57 insertions(+), 47 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e7fb0d40/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java b/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java index 22b4c4d..f5a64a7 100644 --- a/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java +++ b/core/src/main/java/brooklyn/location/basic/SshMachineLocation.java @@ -92,8 +92,9 @@ public class SshMachineLocation extends AbstractLocation implements MachineLocat "noStdoutLogging", "noStderrLogging", "logPrefix", "out", "err", "password", "permissions", "sshTries", "env", "allocatePTY", "privateKeyPassphrase", "privateKeyFile", "privateKeyData", - // would like to deprecate these -- prefer privateKeyData/privateKeyFile (confusion about whether other holds a file or data) - // hard to warn for these however ... perhaps just remove early in 0.5.0 ? + // deprecated in 0.4.0 -- prefer privateKeyData/privateKeyFile + // (confusion about whether other holds a file or data; and public not useful here) + // they generate a warning where used "keyFiles", "publicKey", "privateKey"); //TODO remove once everything is prefixed SSHCONFIG_PREFIX or included above public static final Collection<String> NON_SSH_PROPS = ImmutableSet.of("latitude", "longitude", "backup", "sshPublicKeyData", "sshPrivateKeyData"); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e7fb0d40/core/src/main/java/brooklyn/location/basic/jclouds/JcloudsLocation.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/location/basic/jclouds/JcloudsLocation.java b/core/src/main/java/brooklyn/location/basic/jclouds/JcloudsLocation.java index b886e07..7a77d7f 100644 --- a/core/src/main/java/brooklyn/location/basic/jclouds/JcloudsLocation.java +++ b/core/src/main/java/brooklyn/location/basic/jclouds/JcloudsLocation.java @@ -386,6 +386,7 @@ public class JcloudsLocation extends AbstractLocation implements MachineProvisio public String setPrivateKeyData(String privateKeyData) { String oldPrivateKeyData = this.privateKeyData; this.privateKeyData = privateKeyData; + allconf.put("privateKeyData", privateKeyData); allconf.put("sshPrivateKeyData", privateKeyData); return oldPrivateKeyData; } @@ -520,10 +521,9 @@ public class JcloudsLocation extends AbstractLocation implements MachineProvisio } else { LoginCredentials.Builder expectedCredentialsBuilder = LoginCredentials.builder(). user(user); - if (pkd!=null) expectedCredentialsBuilder.noPassword().privateKey(pkd); - else expectedCredentialsBuilder.noPrivateKey().password(pwd); + if (pkd!=null) expectedCredentialsBuilder.privateKey(pkd); + if (pwd!=null) expectedCredentialsBuilder.password(pwd); expectedCredentials = expectedCredentialsBuilder.build(); -// LoginCredentials.fromCredentials(new Credentials(user, pkd!=null ? pkd : pwd)); } } if (expectedCredentials != null) @@ -724,36 +724,31 @@ public class JcloudsLocation extends AbstractLocation implements MachineProvisio private Map generateSshConfig(BrooklynJcloudsSetupHolder setup, NodeMetadata node) throws IOException { Map sshConfig = Maps.newLinkedHashMap(); - if (truth(setup.allconf.get("sshPrivateKeyData"))) { - sshConfig.put("privateKey", setup.privateKeyData); - // sshConfig.put("privateKeyData", setup.allconf.get("sshPrivateKeyData")); - // sshConfig.put("sshPrivateKeyData", setup.allconf.get("sshPrivateKeyData")); - } else if (truth(getPrivateKeyFile())) { - sshConfig.put("keyFiles", ImmutableList.of(getPrivateKeyFile().getCanonicalPath())); - } else if (truth(getPrivateKeyFile())) { - sshConfig.put("keyFiles", ImmutableList.of(getPrivateKeyFile().getCanonicalPath())); - } else if (setup.password != null) { + + if (setup.password != null) { sshConfig.put("password", setup.password); } else if (node!=null && node.getCredentials().getPassword() != null) { sshConfig.put("password", node.getCredentials().getPassword()); } + if (truth(setup.privateKeyData)) { + sshConfig.put("privateKeyData", setup.privateKeyData); + } else if (truth(setup.allconf.get("sshPrivateKeyData"))) { + LOG.warn("Using legacy sshPrivateKeyData but not privateKeyData"); + Object d = setup.allconf.get("sshPrivateKeyData"); + sshConfig.put("privateKeyData", d); + sshConfig.put("privateKey", d); + sshConfig.put("sshPrivateKeyData", d); + } else if (truth(getPrivateKeyFile())) { + LOG.warn("Using legacy keyFiles but not privateKeyData"); + sshConfig.put("keyFiles", ImmutableList.of(getPrivateKeyFile().getCanonicalPath())); + } + if (truth(setup.allconf.get("privateKeyPassphrase"))) { - // TODO do we set this up correctly for jclouds to use? + // NB: not supported in jclouds sshConfig.put("privateKeyPassphrase", setup.privateKeyPassphrase); } -// if (truth(setup.allconf.get("sshPublicKeyData"))) { -// sshConfig.put("sshPublicKeyData", setup.allconf.get("sshPublicKeyData")); -// } - -// if (truth(setup.allconf.get("sshPrivateKeyData"))) -// sshLocByHostname.configure(MutableMap.of("sshPrivateKeyData", setup.allconf.get("sshPrivateKeyData"))); -// if (truth(setup.allconf.get("sshPublicKeyData"))) -// sshLocByHostname.configure(MutableMap.of("sshPublicKeyData", setup.allconf.get("sshPublicKeyData"))); -// if (truth(setup.allconf.get("password"))) -// sshLocByHostname.configure(MutableMap.of("password", setup.allconf.get("password"))); - return sshConfig; } @@ -1088,16 +1083,7 @@ public class JcloudsLocation extends AbstractLocation implements MachineProvisio private String getPublicHostnameAws(String ip, BrooklynJcloudsSetupHolder setup) { try { - Map sshConfig = Maps.newLinkedHashMap(); - // TODO combine with above - if (truth(setup.password)) - sshConfig.put("password", setup.password); - if (truth(setup.privateKeyData)) - sshConfig.put("privateKeyData", setup.privateKeyData); - if (truth(setup.privateKeyPassphrase)) - sshConfig.put("privateKeyPassphrase", setup.privateKeyPassphrase); - if (truth(getPrivateKeyFile())) - sshConfig.put("keyFiles", ImmutableList.of(getPrivateKeyFile().getCanonicalPath())); + Map sshConfig = generateSshConfig(setup, null); // TODO messy way to get an SSH session SshMachineLocation sshLocByIp = new SshMachineLocation(MutableMap.of("address", ip, "user", setup.user, "config", sshConfig)); http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/e7fb0d40/core/src/main/java/brooklyn/util/internal/ssh/SshjTool.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/brooklyn/util/internal/ssh/SshjTool.java b/core/src/main/java/brooklyn/util/internal/ssh/SshjTool.java index dcd375e..8413cae 100644 --- a/core/src/main/java/brooklyn/util/internal/ssh/SshjTool.java +++ b/core/src/main/java/brooklyn/util/internal/ssh/SshjTool.java @@ -68,6 +68,7 @@ import brooklyn.util.text.StringEscapes.BashStringEscapes; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; +import com.google.common.base.Objects; import com.google.common.base.Predicate; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; @@ -139,13 +140,31 @@ public class SshjTool implements SshTool { return new Builder(); } + private static void warnOnDeprecated(Map<String, ?> props, String deprecatedKey, String correctKey) { + if (props.containsKey(deprecatedKey)) { + if (correctKey != null && props.containsKey(correctKey)) { + Object dv = props.get(deprecatedKey); + Object cv = props.get(correctKey); + if (!Objects.equal(cv, dv)) { + LOG.warn("SshjTool detected deprecated key '"+deprecatedKey+"' with different value ("+dv+") "+ + "than new key '"+correctKey+"' ("+cv+"); ambiguous which will be used"); + } else { + // ignore, the deprecated key populated for legacy reasons + } + } else { + Object dv = props.get(deprecatedKey); + LOG.warn("SshjTool detected deprecated key '"+deprecatedKey+"' used, with value ("+dv+")"); + } + } + } + public static class Builder { private String host; + private int port = 22; private String user = System.getProperty("user.name"); private String password; - private int port = 22; - public String privateKeyPassphrase; private String privateKeyData; + public String privateKeyPassphrase; private Set<String> privateKeyFiles = Sets.newLinkedHashSet(); private boolean strictHostKeyChecking = false; private boolean allocatePTY = false; @@ -159,23 +178,27 @@ public class SshjTool implements SshTool { host = getMandatoryVal(props, "host", String.class); port = getOptionalVal(props, "port", Integer.class, port); user = getOptionalVal(props, "user", String.class, user); + password = getOptionalVal(props, "password", String.class, password); - strictHostKeyChecking = getOptionalVal(props, "strictHostKeyChecking", Boolean.class, strictHostKeyChecking); - allocatePTY = getOptionalVal(props, "allocatePTY", Boolean.class, allocatePTY); - connectTimeout = getOptionalVal(props, "connectTimeout", Integer.class, connectTimeout); - sessionTimeout = getOptionalVal(props, "sessionTimeout", Integer.class, sessionTimeout); - sshTries = getOptionalVal(props, "sshTries", Integer.class, sshTries); - sshRetryDelay = getOptionalVal(props, "sshRetryDelay", Long.class, sshRetryDelay); - - privateKeyPassphrase = getOptionalVal(props, "privateKeyPassphrase", String.class, privateKeyPassphrase); + + warnOnDeprecated(props, "privateKey", "privateKeyData"); privateKeyData = getOptionalVal(props, "privateKey", String.class, privateKeyData); privateKeyData = getOptionalVal(props, "privateKeyData", String.class, privateKeyData); - + privateKeyPassphrase = getOptionalVal(props, "privateKeyPassphrase", String.class, privateKeyPassphrase); + // for backwards compatibility accept keyFiles and privateKey // but sshj accepts only a single privateKeyFile; leave blank to use defaults (i.e. ~/.ssh/id_rsa and id_dsa) + warnOnDeprecated(props, "keyFiles", null); privateKeyFiles.addAll(getOptionalVal(props, "keyFiles", List.class, Collections.emptyList())); String privateKeyFile = getOptionalVal(props, "privateKeyFile", String.class, null); if (privateKeyFile != null) privateKeyFiles.add(privateKeyFile); + + strictHostKeyChecking = getOptionalVal(props, "strictHostKeyChecking", Boolean.class, strictHostKeyChecking); + allocatePTY = getOptionalVal(props, "allocatePTY", Boolean.class, allocatePTY); + connectTimeout = getOptionalVal(props, "connectTimeout", Integer.class, connectTimeout); + sessionTimeout = getOptionalVal(props, "sessionTimeout", Integer.class, sessionTimeout); + sshTries = getOptionalVal(props, "sshTries", Integer.class, sshTries); + sshRetryDelay = getOptionalVal(props, "sshRetryDelay", Long.class, sshRetryDelay); return this; }
