Repository: incubator-brooklyn
Updated Branches:
  refs/heads/master 98f7667de -> ec7448947


tidy up how os credentials are inferred

share lookup routines in preparation for adding validation


Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/fa79e20d
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/fa79e20d
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/fa79e20d

Branch: refs/heads/master
Commit: fa79e20dbf895520b411965779034f0baee01f30
Parents: c563e65
Author: Alex Heneveld <[email protected]>
Authored: Thu Jan 22 10:33:16 2015 +0000
Committer: Alex Heneveld <[email protected]>
Committed: Thu Jan 22 11:29:49 2015 +0000

----------------------------------------------------------------------
 .../location/basic/LocationConfigKeys.java      |   2 +-
 .../location/basic/LocationConfigUtils.java     | 217 ++++++++++++++++++-
 .../location/basic/LocationConfigUtilsTest.java |  40 ++--
 .../brooklyn/location/basic/sample_id_rsa       |  27 +++
 .../brooklyn/location/basic/sample_id_rsa.pub   |   1 +
 .../location/jclouds/JcloudsLocation.java       | 150 ++++++-------
 .../main/java/brooklyn/util/text/Strings.java   |   7 +
 7 files changed, 331 insertions(+), 113 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java 
b/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
index ded3a0a..074bf8e 100644
--- a/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
+++ b/core/src/main/java/brooklyn/location/basic/LocationConfigKeys.java
@@ -54,7 +54,7 @@ public class LocationConfigKeys {
             "user account for normal access to the remote machine, defaulting 
to local user", System.getProperty("user.name"));
     
     public static final ConfigKey<String> PASSWORD = 
ConfigKeys.newStringConfigKey("password");
-    public static final ConfigKey<String> PUBLIC_KEY_FILE = 
ConfigKeys.newStringConfigKey("publicKeyFile", "colon-separated list of ssh 
public key file(s) to use; if blank will infer from privateKeyFile by appending 
\".pub\"");
+    public static final ConfigKey<String> PUBLIC_KEY_FILE = 
ConfigKeys.newStringConfigKey("publicKeyFile", "ssh public key file to use; if 
blank will infer from privateKeyFile by appending \".pub\"");
     public static final ConfigKey<String> PUBLIC_KEY_DATA = 
ConfigKeys.newStringConfigKey("publicKeyData", "ssh public key string to use 
(takes precedence over publicKeyFile)");
     public static final ConfigKey<String> PRIVATE_KEY_FILE = 
ConfigKeys.newStringConfigKey("privateKeyFile", "a '" + File.pathSeparator + "' 
separated list of ssh private key files; uses first in list that can be read",
                                                                                
            Os.fromHome(".ssh/id_rsa") + File.pathSeparator + 
Os.fromHome(".ssh/id_dsa"));

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java 
b/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
index 1ce8542..0fac5e3 100644
--- a/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
+++ b/core/src/main/java/brooklyn/location/basic/LocationConfigUtils.java
@@ -23,6 +23,7 @@ import static brooklyn.util.JavaGroovyEquivalents.groovyTruth;
 import java.io.File;
 import java.io.IOException;
 import java.util.Arrays;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -32,8 +33,10 @@ import org.slf4j.LoggerFactory;
 import brooklyn.config.ConfigKey;
 import brooklyn.entity.basic.ConfigKeys;
 import brooklyn.management.ManagementContext;
+import brooklyn.util.ResourceUtils;
 import brooklyn.util.collections.MutableMap;
 import brooklyn.util.config.ConfigBag;
+import brooklyn.util.exceptions.Exceptions;
 import brooklyn.util.os.Os;
 import brooklyn.util.text.StringFunctions;
 import brooklyn.util.text.Strings;
@@ -48,11 +51,210 @@ import com.google.common.io.Files;
 public class LocationConfigUtils {
 
     private static final Logger log = 
LoggerFactory.getLogger(LocationConfigUtils.class);
+
+    /** Creates an instance of {@link OsCredential} by inspecting {@link 
LocationConfigKeys#PASSWORD}; 
+     * {@link LocationConfigKeys#PRIVATE_KEY_DATA} and {@link 
LocationConfigKeys#PRIVATE_KEY_FILE};
+     * {@link LocationConfigKeys#PRIVATE_KEY_PASSPHRASE} if needed, and
+     * {@link LocationConfigKeys#PRIVATE_KEY_DATA} and {@link 
LocationConfigKeys#PRIVATE_KEY_FILE}
+     * (defaulting to the private key file + ".pub"). 
+     **/
+    public static OsCredential getOsCredential(ConfigBag config) {
+        return OsCredential.newInstance(config);
+    }
     
+    /** Convenience class for holding private/public keys and passwords, 
inferring from config keys.
+     * See {@link LocationConfigUtils#getOsCredential(ConfigBag)}. */
+    public static class OsCredential {
+        private final ConfigBag config;
+        private boolean preferPassword = false;
+        private boolean tryDefaultKeys = true;
+        private boolean requirePublicKey = true;
+        private boolean doKeyValidation = true;
+        private boolean warnOnErrors = true;
+        private boolean throwOnErrors = false;
+        
+        private boolean dirty = true;;
+        
+        private String privateKeyData;
+        private String publicKeyData;
+        private String password;
+        
+        private OsCredential(ConfigBag config) {
+            this.config = config;
+        }
+
+        /** throws if there are any problems */
+        public OsCredential check() {
+            throwOnErrors(true);
+            infer();
+            return this;
+        }
+
+        /** returns either the key or password or null; if both a key and a 
password this prefers the key unless otherwise set
+         * via {@link #preferPassword()} */
+        public synchronized String get() {
+            infer();
+            
+            if (isUsingPassword()) return password;
+            if (hasKey()) return privateKeyData;
+            return null;
+        }
+
+        /** if there is no credential (ignores public key) */
+        public boolean isEmpty() {
+            return !hasKey() && !hasPassword();
+        }
+        public boolean hasKey() {
+            infer();
+            // key has stricter non-blank check than password
+            return Strings.isNonBlank(privateKeyData);
+        }
+        public boolean hasPassword() {
+            infer();
+            // blank, even empty passwords are allowed
+            return password!=null;
+        }
+        /** if a password is available, and either this is preferred over a 
key or there is no key */
+        public boolean isUsingPassword() {
+            return hasPassword() && (!hasKey() || preferPassword);
+        }
+        
+        public String getPrivateKeyData() {
+            infer();
+            return privateKeyData;
+        }
+        public String getPublicKeyData() {
+            infer();
+            return publicKeyData;
+        }
+        public String getPassword() {
+            infer();
+            return password;
+        }
+        
+        /** if both key and password supplied, prefer the key; the default */
+        public OsCredential preferKey() { preferPassword = false; return 
dirty(); }
+        /** if both key and password supplied, prefer the password; see {@link 
#preferKey()} */
+        public OsCredential preferPassword() { preferPassword = true; return 
dirty(); }
+        
+        /** if false, do not mind if there is no public key corresponding to 
any private key;
+         * defaults to true; only applies if a private key is set */
+        public OsCredential requirePublicKey(boolean requirePublicKey) {
+            this.requirePublicKey = requirePublicKey;
+            return dirty(); 
+        }
+        /** whether to check the private/public keys and passphrase are 
coherent; default true */
+        public OsCredential doKeyValidation(boolean doKeyValidation) {
+            this.doKeyValidation = doKeyValidation;
+            return dirty();
+        }
+        /** if true (the default) this will look at default locations set on 
keys */
+        public OsCredential useDefaultKeys(boolean tryDefaultKeys) {
+            this.tryDefaultKeys = tryDefaultKeys;
+            return dirty(); 
+        }
+        /** whether to log warnings on problems */
+        public OsCredential warnOnErrors(boolean warnOnErrors) {
+            this.warnOnErrors = warnOnErrors;
+            return dirty(); 
+        }
+        /** whether to throw on problems */
+        public OsCredential throwOnErrors(boolean throwOnErrors) {
+            this.throwOnErrors = throwOnErrors;
+            return dirty(); 
+        }
+        
+        private OsCredential dirty() { dirty = true; return this; }
+            
+        public static OsCredential newInstance(ConfigBag config) {
+            return new OsCredential(config);
+        }
+        
+        private synchronized void infer() {
+            if (!dirty) return;
+            
+            log.debug("Inferring OS credentials");
+            privateKeyData = config.get(LocationConfigKeys.PRIVATE_KEY_DATA);
+            password = config.get(LocationConfigKeys.PASSWORD);
+            publicKeyData = getKeyDataFromDataKeyOrFileKey(config, 
LocationConfigKeys.PUBLIC_KEY_DATA, LocationConfigKeys.PUBLIC_KEY_FILE);
+
+            if (Strings.isBlank(privateKeyData)) {
+                // look up private key files
+                String privateKeyFiles = null;
+                if (tryDefaultKeys || 
config.containsKey(LocationConfigKeys.PRIVATE_KEY_FILE)) 
+                    privateKeyFiles = 
config.get(LocationConfigKeys.PRIVATE_KEY_FILE);
+                if (Strings.isNonBlank(privateKeyFiles)) {
+                    Iterator<String> fi = 
Arrays.asList(privateKeyFiles.split(File.pathSeparator)).iterator();
+                    while (fi.hasNext()) {
+                        String file = fi.next();
+                        if (Strings.isNonBlank(file)) {
+                            try {
+                                // real URL's won't actual work, due to use of 
path separator above 
+                                // not real important, but we get it for free 
if "files" is a list instead.
+                                // using ResourceUtils is useful for classpath 
resources
+                                privateKeyData = 
ResourceUtils.create().getResourceAsString(file);
+                                if (Strings.isNonBlank(publicKeyData)) {
+                                    log.debug("Loaded private key data from 
"+file+" (public key data from explicit config)");
+                                } else {
+                                    try {
+                                        file = file+".pub";
+                                        publicKeyData = 
ResourceUtils.create().getResourceAsString(file);
+                                        log.debug("Loaded private key data 
from "+Strings.removeFromEnd(file, ".pub")+
+                                            " and public key data from "+file);
+                                        break;
+                                    } catch (Exception e) {
+                                        Exceptions.propagateIfFatal(e);
+                                        log.debug("No public key file "+file+" 
; " + 
+                                            (!requirePublicKey ? "this is 
allowed in this case" : !fi.hasNext() ? "no more files to try" : "trying next 
file"), e);
+                                        if (requirePublicKey) {
+                                            privateKeyData = null;
+                                        } else {
+                                            // look for a different private key
+                                            break;
+                                        }
+                                    }
+                                }
+                                
+                                // TODO check passphrase public key, 
validation, etc
+                                
+                            } catch (Exception e) {
+                                Exceptions.propagateIfFatal(e);
+                                log.debug("No private key file "+file+" ; " + 
(!fi.hasNext() ? "no more files to try" : "trying next file"), e);
+                            }
+                        }
+                    }
+                }
+            }
+            
+            if (privateKeyData!=null) {
+                if (requirePublicKey && Strings.isBlank(publicKeyData)) {
+                    String message = "If explicit 
"+LocationConfigKeys.PRIVATE_KEY_DATA.getName()+" is supplied, then "
+                        + "the corresponding 
"+LocationConfigKeys.PUBLIC_KEY_DATA.getName()+" must also be supplied.";
+                    if (warnOnErrors) log.warn(message);
+                    if (throwOnErrors) throw new 
IllegalStateException(message);
+                }
+            }
+
+            log.debug("OS credential inference: "+this);
+            dirty = false;
+        }
+        
+        @Override
+        public String toString() {
+            // TODO print hash of key?
+            return getClass().getSimpleName()+"["+
+                (Strings.isNonBlank(publicKeyData) ? "public-key" : 
"public-key")+","+
+                (Strings.isNonBlank(privateKeyData) ? "private-key" : 
"private-key")+","+
+                (password!=null ? "password" : "no-password")+"]";
+        }
+    }
+
+    /** @deprecated since 0.7.0, use #getOsCredential(ConfigBag) */ @Deprecated
     public static String getPrivateKeyData(ConfigBag config) {
         return getKeyData(config, LocationConfigKeys.PRIVATE_KEY_DATA, 
LocationConfigKeys.PRIVATE_KEY_FILE);
     }
     
+    /** @deprecated since 0.7.0, use #getOsCredential(ConfigBag) */ @Deprecated
     public static String getPublicKeyData(ConfigBag config) {
         String data = getKeyData(config, LocationConfigKeys.PUBLIC_KEY_DATA, 
LocationConfigKeys.PUBLIC_KEY_FILE);
         if (groovyTruth(data)) return data;
@@ -76,7 +278,12 @@ public class LocationConfigUtils {
         return null;
     }
 
+    /** @deprecated since 0.7.0, use #getOsCredential(ConfigBag) */ @Deprecated
     public static String getKeyData(ConfigBag config, ConfigKey<String> 
dataKey, ConfigKey<String> fileKey) {
+        return getKeyDataFromDataKeyOrFileKey(config, dataKey, fileKey);
+    }
+    
+    private static String getKeyDataFromDataKeyOrFileKey(ConfigBag config, 
ConfigKey<String> dataKey, ConfigKey<String> fileKey) {
         boolean unused = config.isUnused(dataKey);
         String data = config.get(dataKey);
         if (groovyTruth(data) && !unused) {
@@ -112,19 +319,17 @@ public class LocationConfigUtils {
      * @param files             list of file paths
      */
     private static String getFileContents(Iterable<String> files) {
-        int size = Iterables.size(files);
-        int i = 0;
-        
-        for (String file : files) {
+        Iterator<String> fi = files.iterator();
+        while (fi.hasNext()) {
+            String file = fi.next();
             if (groovyTruth(file)) {
                 try {
                     File f = new File(file);
                     return Files.toString(f, Charsets.UTF_8);
                 } catch (IOException e) {
-                    log.debug("Invalid file "+file+" ; " + (i >= (size-1) ? 
"no more files to try" : "trying next file"), e);
+                    log.debug("Invalid file "+file+" ; " + (!fi.hasNext() ? 
"no more files to try" : "trying next file"), e);
                 }
             }
-            i++;
         }
         return null;
     }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java 
b/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
index 08a4705..46a96a1 100644
--- a/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
+++ b/core/src/test/java/brooklyn/location/basic/LocationConfigUtilsTest.java
@@ -27,71 +27,69 @@ import brooklyn.util.config.ConfigBag;
 
 public class LocationConfigUtilsTest {
 
-    public static final String SSH_PRIVATE_KEY_FILE = 
System.getProperty("sshPrivateKey", "~/.ssh/id_rsa");
-    public static final String SSH_PUBLIC_KEY_FILE = 
System.getProperty("sshPrivateKey", "~/.ssh/id_rsa.pub");
+    // set these system properties differently if needed to fix your tests
+    public static final String SSH_PRIVATE_KEY_FILE_WITH_TILDE = 
System.getProperty("sshPrivateKey", "~/.ssh/id_rsa");
+    public static final String SSH_PUBLIC_KEY_FILE_WITH_TILDE = 
System.getProperty("sshPublicKey", "~/.ssh/id_rsa.pub");
+    public static final String SSH_PRIVATE_KEY_FILE = 
System.getProperty("sshPrivateKeySample", 
"/brooklyn/location/basic/sample_id_rsa");
+    public static final String SSH_PUBLIC_KEY_FILE = 
System.getProperty("sshPublicKeySample", 
"/brooklyn/location/basic/sample_id_rsa.pub");
     
-    @Test(groups="Integration")
     public void testPreferPrivateKeyDataOverFile() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PRIVATE_KEY_DATA, "mydata");
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, SSH_PRIVATE_KEY_FILE);
         
-        String data = LocationConfigUtils.getPrivateKeyData(config);
+        String data = 
LocationConfigUtils.getOsCredential(config).getPrivateKeyData();
         assertEquals(data, "mydata");
     }
     
-    @Test(groups="Integration")
     public void testPreferPubilcKeyDataOverFile() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PUBLIC_KEY_DATA, "mydata");
         config.put(LocationConfigKeys.PUBLIC_KEY_FILE, SSH_PUBLIC_KEY_FILE);
         
-        String data = LocationConfigUtils.getPublicKeyData(config);
+        String data = 
LocationConfigUtils.getOsCredential(config).getPublicKeyData();
         assertEquals(data, "mydata");
     }
     
-    @Test(groups="Integration")
-    public void testReadsPrivateKeyFileWithTildaPath() throws Exception {
+    @Test(groups="Integration")  // requires ~/.ssh/id_rsa
+    public void testReadsPrivateKeyFileWithTildePath() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
-        config.put(LocationConfigKeys.PRIVATE_KEY_FILE, SSH_PRIVATE_KEY_FILE);
+        config.put(LocationConfigKeys.PRIVATE_KEY_FILE, 
SSH_PRIVATE_KEY_FILE_WITH_TILDE);
         
-        String data = LocationConfigUtils.getPrivateKeyData(config);
+        String data = 
LocationConfigUtils.getOsCredential(config).skipPassphraseValidation().get();
         assertTrue(data != null && data.length() > 0);
     }
     
-    @Test(groups="Integration")
     public void 
testReadsPrivateKeyFileWithMultipleColonSeparatedFilesWithGoodLast() throws 
Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, 
"/path/does/not/exist:"+SSH_PRIVATE_KEY_FILE);
         
-        String data = LocationConfigUtils.getPrivateKeyData(config);
+        String data = LocationConfigUtils.getOsCredential(config).get();
         assertTrue(data != null && data.length() > 0);
     }
     
-    @Test(groups="Integration")
     public void 
testReadsPrivateKeyFileWithMultipleColonSeparatedFilesWithGoodFirst() throws 
Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, 
SSH_PRIVATE_KEY_FILE+":/path/does/not/exist");
-        
-        String data = LocationConfigUtils.getPrivateKeyData(config);
+
+        String data = LocationConfigUtils.getOsCredential(config).get();
         assertTrue(data != null && data.length() > 0);
     }
     
-    @Test(groups="Integration")
-    public void testReadsPublicKeyFileWithTildaPath() throws Exception {
+    @Test(groups="Integration")  // requires ~/.ssh/id_rsa
+    public void testReadsPublicKeyFileWithTildePath() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
-        config.put(LocationConfigKeys.PUBLIC_KEY_FILE, SSH_PUBLIC_KEY_FILE);
+        config.put(LocationConfigKeys.PUBLIC_KEY_FILE, 
SSH_PUBLIC_KEY_FILE_WITH_TILDE);
         
-        String data = LocationConfigUtils.getPublicKeyData(config);
+        String data = 
LocationConfigUtils.getOsCredential(config).skipPassphraseValidation().getPublicKeyData();
         assertTrue(data != null && data.length() > 0);
     }
     
-    @Test(groups="Integration")
     public void testInfersPublicKeyFileFromPrivateKeyFile() throws Exception {
         ConfigBag config = ConfigBag.newInstance();
         config.put(LocationConfigKeys.PRIVATE_KEY_FILE, SSH_PRIVATE_KEY_FILE);
         
-        String data = LocationConfigUtils.getPublicKeyData(config);
+        String data = 
LocationConfigUtils.getOsCredential(config).getPublicKeyData();
         assertTrue(data != null && data.length() > 0);
     }
 }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/core/src/test/resources/brooklyn/location/basic/sample_id_rsa
----------------------------------------------------------------------
diff --git a/core/src/test/resources/brooklyn/location/basic/sample_id_rsa 
b/core/src/test/resources/brooklyn/location/basic/sample_id_rsa
new file mode 100644
index 0000000..bfeef4a
--- /dev/null
+++ b/core/src/test/resources/brooklyn/location/basic/sample_id_rsa
@@ -0,0 +1,27 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEA0rPOuv1rVXhrSaZSnA2fShSFp8/3knG/Uuss8ZojUW4k+ETj
+dVuYzoH/tvAaYqTUuxqB5OpNgOgUpS02hqOWtN/3iYcfJtBHe4eu/dMSfcQXKasU
+ZalQtv2zF955EMH7jZ0q4TPikOS/tR/gPBDe6xHGjwPpB7Jd2daU0VgJD7M9SOIL
+bRDbilw1XpgyOs4C6I3ftlcH1nA9tQKcdFQ0wmWAXr0ulizYMoAy3VOm34XAwT/o
+w1sgpGK7YwPLbBv1QI4SKfvHi9OhcNBBOoXTE3G3vg6EiKGaam3Kp17EO60iq4CE
+Ec+3FBk5JQ0pAX2SsfxqASHY7QcpbQphF+jlRwIDAQABAoIBAQCqjnZXku+hfhqK
+waG5RKWeV8JhNs0WtBDFVC1LXRQdxGUUut7MjtrAvyZ5tR4Gn5q74hcncCpQoIyl
+sFWk4yMJQwqjPseOqaZTbl/Og19CgsqlJiEasdXuaqrgNWwWjo/L8F9XcKKD20b7
+nNPsi1OHQRpThjzJyC6EOVi5pOOg2mL+cWBJEJy6AIOWVkidw/x+IX6P/6eppc+C
+JM9AVRvSkCYBmTXTCM4OZq6WV53V/m/SaAlG/LcC4ykZBDdhq8T3lNp/sACvWVzL
+JKVcQuBAv7ABBbMcyTgIFpKrx7siUTA+7QGkdG1IbR55S57iSgGGytUV4qv02pBr
+bCJUokZRAoGBAPx/XZ0ig9lYVF7Z4ayASS7j5na4A/MZyHeBZ8hPynIHpjbGvRrG
++r61XEcSsS4vj6ptCBhuxzYRq6t1yX/6wecFmWdSxcjoS4grMKdGPoWSoOBnV85I
+4/Ctd+NiFtXDqd1abGH3NKtkSwwKr/FX3IK2ePiGpoBNYWbGFMRO2LJrAoGBANWg
+BlLjnZwZ71+8cKb3ozCaLmE8fMG/vw6idTEPRk6iyvto9rpjnpz7Sd12z/d35U32
+6QM+3MsGZ3B1NTJoba7l7cuVtwfIHBc0Q3VQSHBp5PDuM5bNhDb38jt0/Dtv/bMD
+NMVletmrc2Hfx8uqUdr21LKrWO42AwYsLFsMAGeVAoGAObXWrKqN3ihdKEy+UtID
+aA84xpuqc27KLd5K3TK3f7aV2+EyqaMe/mWvUKNKEddXC8nd1s/DAm2pggfq5TBo
+Dyhtdnspr5DAasAMX78jXR41XPTh0clBJ+pOA4+QzozpDymyqfV5eU70BC2RJyVA
+xjN0lMEZ3ytQfs/5QSEQUD8CgYBwipOKS3uW51riVsYKUF/alP9mHpWjBL9EmHWg
+2OkzODQzasLAwwamsQPi9lrthm55OmDbYtyy4LbR2g2idr2B7IPwQvlf0h5qYxA+
+14KyJjeEbhkjkzXaN5mXlTPkpEVFb6T3cVTdI6PvphL9ysbA0lSPpBF/vViugcsE
+VDhKWQKBgQCwaNQ07sHKRqc22SFNgcOv0KOMx7CbaSRxGoEwtpvvdAPlUXuIYMs+
+pueK5scHKJvtcHZQb82j6o84OC37ut+8tUe2XkasyAbONZqqm+Oes1xseKrNcF8J
++thFmGvZc8O3xdflT87OXDLGzufGJXUDDvZctJwrC5AYPClwj6wd0A==
+-----END RSA PRIVATE KEY-----

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/core/src/test/resources/brooklyn/location/basic/sample_id_rsa.pub
----------------------------------------------------------------------
diff --git a/core/src/test/resources/brooklyn/location/basic/sample_id_rsa.pub 
b/core/src/test/resources/brooklyn/location/basic/sample_id_rsa.pub
new file mode 100644
index 0000000..f137be8
--- /dev/null
+++ b/core/src/test/resources/brooklyn/location/basic/sample_id_rsa.pub
@@ -0,0 +1 @@
+ssh-rsa 
AAAAB3NzaC1yc2EAAAADAQABAAABAQDSs866/WtVeGtJplKcDZ9KFIWnz/eScb9S6yzxmiNRbiT4RON1W5jOgf+28BpipNS7GoHk6k2A6BSlLTaGo5a03/eJhx8m0Ed7h6790xJ9xBcpqxRlqVC2/bMX3nkQwfuNnSrhM+KQ5L+1H+A8EN7rEcaPA+kHsl3Z1pTRWAkPsz1I4gttENuKXDVemDI6zgLojd+2VwfWcD21Apx0VDTCZYBevS6WLNgygDLdU6bfhcDBP+jDWyCkYrtjA8tsG/VAjhIp+8eL06Fw0EE6hdMTcbe+DoSIoZpqbcqnXsQ7rSKrgIQRz7cUGTklDSkBfZKx/GoBIdjtByltCmEX6OVH
 [email protected]

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
----------------------------------------------------------------------
diff --git 
a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
 
b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
index a5f32a0..db9c9d6 100644
--- 
a/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
+++ 
b/locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java
@@ -99,6 +99,7 @@ import brooklyn.location.access.PortForwardManager;
 import brooklyn.location.basic.BasicMachineMetadata;
 import brooklyn.location.basic.LocationConfigKeys;
 import brooklyn.location.basic.LocationConfigUtils;
+import brooklyn.location.basic.LocationConfigUtils.OsCredential;
 import brooklyn.location.basic.SshMachineLocation;
 import brooklyn.location.cloud.AbstractCloudMachineProvisioningLocation;
 import brooklyn.location.cloud.AvailabilityZoneExtension;
@@ -575,14 +576,14 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             Stopwatch provisioningStopwatch = Stopwatch.createStarted();
             Duration templateTimestamp, provisionTimestamp, usableTimestamp, 
customizedTimestamp;
 
-            LoginCredentials initialCredentials = null;
+            LoginCredentials userCredentials = null;
             Set<? extends NodeMetadata> nodes;
             Template template;
             try {
                 // Setup the template
                 template = buildTemplate(computeService, setup);
                 if (waitForSshable && !skipJcloudsSshing) {
-                    initialCredentials = initTemplateForCreateUser(template, 
setup);
+                    userCredentials = initTemplateForCreateUser(template, 
setup);
                 }
 
                 //FIXME initialCredentials = initUserTemplateOptions(template, 
setup);
@@ -627,39 +628,39 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             if (waitForSshable && skipJcloudsSshing) {
                 // once that host:port is definitely reachable, we can create 
the user
                 waitForReachable(computeService, node, sshHostAndPortOverride, 
node.getCredentials(), setup);
-                initialCredentials = createUser(computeService, node, 
sshHostAndPortOverride, setup);
+                userCredentials = createUser(computeService, node, 
sshHostAndPortOverride, setup);
             }
             
             // Figure out which login-credentials to use
             LoginCredentials customCredentials = setup.get(CUSTOM_CREDENTIALS);
             if (customCredentials != null) {
-                initialCredentials = customCredentials;
+                userCredentials = customCredentials;
                 //set userName and other data, from these credentials
                 Object oldUsername = setup.put(USER, 
customCredentials.getUser());
                 LOG.debug("node {} username {} / {} (customCredentials)", new 
Object[] { node, customCredentials.getUser(), oldUsername });
                 if (customCredentials.getOptionalPassword().isPresent()) 
setup.put(PASSWORD, customCredentials.getOptionalPassword().get());
                 if (customCredentials.getOptionalPrivateKey().isPresent()) 
setup.put(PRIVATE_KEY_DATA, customCredentials.getOptionalPrivateKey().get());
             }
-            if (initialCredentials == null) {
-                initialCredentials = extractVmCredentials(setup, node);
+            if (userCredentials == null) {
+                userCredentials = extractVmCredentials(setup, node);
             }
-            if (initialCredentials != null) {
-                node = 
NodeMetadataBuilder.fromNodeMetadata(node).credentials(initialCredentials).build();
+            if (userCredentials != null) {
+                node = 
NodeMetadataBuilder.fromNodeMetadata(node).credentials(userCredentials).build();
             } else {
                 // only happens if something broke above...
-                initialCredentials = 
LoginCredentials.fromCredentials(node.getCredentials());
+                userCredentials = 
LoginCredentials.fromCredentials(node.getCredentials());
             }
             
             // Wait for the VM to be reachable over SSH
             if (waitForSshable) {
-                waitForReachable(computeService, node, sshHostAndPortOverride, 
initialCredentials, setup);
+                waitForReachable(computeService, node, sshHostAndPortOverride, 
userCredentials, setup);
             } else {
                 LOG.debug("Skipping ssh check for {} ({}) due to config 
waitForSshable=false", node, setup.getDescription());
             }
             usableTimestamp = Duration.of(provisioningStopwatch);
             
             // Create a JcloudsSshMachineLocation, and register it
-            sshMachineLocation = 
registerJcloudsSshMachineLocation(computeService, node, initialCredentials, 
sshHostAndPortOverride, setup);
+            sshMachineLocation = 
registerJcloudsSshMachineLocation(computeService, node, userCredentials, 
sshHostAndPortOverride, setup);
             if (template!=null && sshMachineLocation.getTemplate()==null) {
                 sshMachineLocation.template = template;
             }
@@ -767,8 +768,8 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             LOG.info("Finished VM "+setup.getDescription()+" creation:"
                     + " 
"+sshMachineLocation.getUser()+"@"+sshMachineLocation.getAddress()+":"+sshMachineLocation.getPort()
                     + (Boolean.TRUE.equals(setup.get(LOG_CREDENTIALS))
-                              ? "password=" + 
(initialCredentials.getOptionalPassword().isPresent() ? 
initialCredentials.getOptionalPassword() : "<absent>")
-                                      + " && key=" + 
(initialCredentials.getOptionalPrivateKey().isPresent() ? 
initialCredentials.getOptionalPrivateKey() : "<absent>")
+                              ? "password=" + 
(userCredentials.getOptionalPassword().isPresent() ? 
userCredentials.getOptionalPassword() : "<absent>")
+                                      + " && key=" + 
(userCredentials.getOptionalPrivateKey().isPresent() ? 
userCredentials.getOptionalPrivateKey() : "<absent>")
                               : "")
                     + " ready after 
"+Duration.of(provisioningStopwatch).toStringRounded()
                     + " ("+template+" template built in 
"+Duration.of(templateTimestamp).toStringRounded()+";"
@@ -1232,7 +1233,7 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             }
         }
 
-        return userCreation.loginCredentials;
+        return userCreation.createdUserCredentials;
     }
     
     /**
@@ -1246,15 +1247,15 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             options.runScript(new StatementList(userCreation.statements));
         }
 
-        return userCreation.loginCredentials;
+        return userCreation.createdUserCredentials;
     }
     
     protected static class UserCreation {
-        public final LoginCredentials loginCredentials;
+        public final LoginCredentials createdUserCredentials;
         public final List<Statement> statements;
         
         public UserCreation(LoginCredentials creds, List<Statement> 
statements) {
-            this.loginCredentials = creds;
+            this.createdUserCredentials = creds;
             this.statements = statements;
         }
     }
@@ -1305,23 +1306,21 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
      *   
      * @param image  The image being used to create the VM
      * @param config Configuration for creating the VM
-     * @return       The commands required to create the user, along with the 
expected login credentials.
+     * @return       The commands required to create the user, along with the 
expected login credentials for that user.
      */
     protected UserCreation createUserStatements(@Nullable Image image, 
ConfigBag config) {
         //NB: we ignore private key here because, by default we probably 
should not be installing it remotely;
         //also, it may not be valid for first login (it is created before 
login e.g. on amazon, so valid there;
         //but not elsewhere, e.g. on rackspace).
         
-        LoginCredentials loginCreds = null;
+        LoginCredentials createdUserCreds = null;
         String user = getUser(config);
         String explicitLoginUser = config.get(LOGIN_USER);
         String loginUser = groovyTruth(explicitLoginUser) ? explicitLoginUser 
: (image != null && image.getDefaultCredentials() != null) ? 
image.getDefaultCredentials().identity : null;
         Boolean dontCreateUser = config.get(DONT_CREATE_USER);
         Boolean grantUserSudo = config.get(GRANT_USER_SUDO);
-        String publicKeyData = LocationConfigUtils.getPublicKeyData(config);
-        String privateKeyData = LocationConfigUtils.getPrivateKeyData(config);
-        String explicitPassword = config.get(PASSWORD);
-        String password = groovyTruth(explicitPassword) ? explicitPassword : 
Identifiers.makeRandomId(12);
+        OsCredential credential = LocationConfigUtils.getOsCredential(config);
+        String newPassword = Strings.isNonBlank(credential.getPassword()) ? 
credential.getPassword() : Identifiers.makeRandomId(12);
         List<Statement> statements = Lists.newArrayList();
         
         if (groovyTruth(dontCreateUser)) {
@@ -1336,42 +1335,30 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
                 
             } else {
                 LOG.info("Not creating user {}, and not setting its password 
or authorizing keys", user);
-                
-                if (privateKeyData != null) {
-                    loginCreds = 
LoginCredentials.builder().user(user).privateKey(privateKeyData).build();
-                } else if (explicitPassword != null) {
-                    loginCreds = 
LoginCredentials.builder().user(user).password(password).build();
+
+                if (credential.isUsingPassword()) {
+                    createdUserCreds = 
LoginCredentials.builder().user(user).password(credential.get()).build();
+                } else if (credential.hasKey()) {
+                    createdUserCreds = 
LoginCredentials.builder().user(user).privateKey(credential.get()).build();
                 }
             }
             
-        } else if (!groovyTruth(user) || user.equals(loginUser)) {
+        } else if (Strings.isBlank(user) || user.equals(loginUser) || 
user.equals(ROOT_USERNAME)) {
             // For subsequent ssh'ing, we'll be using the loginUser
-            if (!groovyTruth(user)) {
-                config.put(USER, loginUser);
+            if (Strings.isBlank(user)) {
+                user = loginUser;
+                config.put(USER, user);
             }
 
             // Using the pre-existing loginUser; setup the publicKey/password 
so can login as expected
-            if (password != null) {
-                statements.add(new 
ReplaceShadowPasswordEntry(Sha512Crypt.function(), loginUser, password));
-                loginCreds = 
LoginCredentials.builder().user(loginUser).password(password).build();
+            if (Strings.isNonBlank(newPassword)) {
+                statements.add(new 
ReplaceShadowPasswordEntry(Sha512Crypt.function(), user, newPassword));
+                createdUserCreds = 
LoginCredentials.builder().user(user).password(newPassword).build();
             }
-            if (publicKeyData!=null) {
-                statements.add(new 
AuthorizeRSAPublicKeys("~"+loginUser+"/.ssh", ImmutableList.of(publicKeyData)));
-                if (privateKeyData != null) {
-                    loginCreds = 
LoginCredentials.builder().user(loginUser).privateKey(privateKeyData).build();
-                }
-            }
-            
-        } else if (user.equals(ROOT_USERNAME)) {
-            // Authorizes the public-key and sets password for the root user, 
so can login as expected
-            if (password != null) {
-                statements.add(new 
ReplaceShadowPasswordEntry(Sha512Crypt.function(), ROOT_USERNAME, password));
-                loginCreds = 
LoginCredentials.builder().user(user).password(password).build();
-            }
-            if (publicKeyData!=null) {
-                statements.add(new 
AuthorizeRSAPublicKeys("~"+ROOT_USERNAME+"/.ssh", 
ImmutableList.of(publicKeyData)));
-                if (privateKeyData != null) {
-                    loginCreds = 
LoginCredentials.builder().user(user).privateKey(privateKeyData).build();
+            if (Strings.isNonBlank(credential.getPublicKeyData())) {
+                statements.add(new AuthorizeRSAPublicKeys("~"+user+"/.ssh", 
ImmutableList.of(credential.getPublicKeyData())));
+                if (Strings.isNonBlank(credential.getPrivateKeyData()) && 
createdUserCreds==null) {
+                    createdUserCreds = 
LoginCredentials.builder().user(user).privateKey(credential.getPrivateKeyData()).build();
                 }
             }
             
@@ -1380,13 +1367,13 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             // note AdminAccess requires _all_ fields set, due to 
http://code.google.com/p/jclouds/issues/detail?id=1095
             AdminAccess.Builder adminBuilder = AdminAccess.builder()
                     .adminUsername(user)
-                    .adminPassword(password)
+                    .adminPassword(newPassword)
                     .grantSudoToAdminUser(groovyTruth(grantUserSudo))
                     .resetLoginPassword(true)
-                    .loginPassword(password);
+                    .loginPassword(newPassword);
 
-            if (publicKeyData!=null) {
-                
adminBuilder.authorizeAdminPublicKey(true).adminPublicKey(publicKeyData);
+            if (Strings.isNonBlank(credential.getPublicKeyData())) {
+                
adminBuilder.authorizeAdminPublicKey(true).adminPublicKey(credential.getPublicKeyData());
             } else {
                 
adminBuilder.authorizeAdminPublicKey(false).adminPublicKey("ignored");
             }
@@ -1396,27 +1383,20 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
             // then authorizeAdminPublicKey is reset to null!
             
adminBuilder.installAdminPrivateKey(false).adminPrivateKey("ignore");
             
-            if (groovyTruth(explicitPassword)) {
-                adminBuilder.lockSsh(false);
-            } else if (publicKeyData != null) {
-                adminBuilder.lockSsh(true);
-            } else {
-                // no keys or passwords supplied; using only defaults!
-                adminBuilder.lockSsh(false);
-            }
-
+            // lock SSH (key only) iff there is a public key and no password 
supplied
+            boolean useKey = !credential.isUsingPassword() && 
Strings.isNonBlank(credential.getPublicKeyData()) && 
Strings.isNonBlank(credential.getPrivateKeyData());
+            adminBuilder.lockSsh(useKey);
             
             statements.add(adminBuilder.build());
             
-            if (groovyTruth(publicKeyData) && groovyTruth(privateKeyData)) {
-                // assume have uploaded corresponding .pub file
-                loginCreds = 
LoginCredentials.builder().user(user).privateKey(privateKeyData).build();
+            if (useKey) {
+                createdUserCreds = 
LoginCredentials.builder().user(user).privateKey(credential.getPrivateKeyData()).build();
             } else {
-                loginCreds = 
LoginCredentials.builder().user(user).password(password).build();
+                createdUserCreds = 
LoginCredentials.builder().user(user).password(newPassword).build();
             }
         }
         
-        return new UserCreation(loginCreds, statements);
+        return new UserCreation(createdUserCreds, statements);
     }
 
 
@@ -1490,8 +1470,8 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
                 throw new IllegalArgumentException("Jclouds node for rebind 
matching multiple, looking for id="+rawId+" and hostname="+rawHostname+": 
"+candidateNodes);
 
             NodeMetadata node = Iterables.getOnlyElement(candidateNodes);
-            String pkd = LocationConfigUtils.getPrivateKeyData(setup);
-            if (groovyTruth(pkd)) {
+            String pkd = 
LocationConfigUtils.getOsCredential(setup).getPrivateKeyData();
+            if (Strings.isNonBlank(pkd)) {
                 LoginCredentials expectedCredentials = 
LoginCredentials.fromCredentials(new Credentials(user, pkd));
                 //override credentials
                 node = 
NodeMetadataBuilder.fromNodeMetadata(node).credentials(expectedCredentials).build();
@@ -1710,33 +1690,33 @@ public class JcloudsLocation extends 
AbstractCloudMachineProvisioningLocation im
      */
     protected LoginCredentials extractVmCredentials(ConfigBag setup, 
NodeMetadata node) {
         String user = getUser(setup);
-        String localPrivateKeyData = 
LocationConfigUtils.getPrivateKeyData(setup);
-        String localPassword = setup.get(PASSWORD);
+        OsCredential localCredentials = 
LocationConfigUtils.getOsCredential(setup);
         LoginCredentials nodeCredentials = 
LoginCredentials.fromCredentials(node.getCredentials());
 
-        LOG.debug("node {} username {} / {} (jclouds)", new Object[] { node, 
user, nodeCredentials.getUser() });
+        LOG.debug("Credentials extracted for {}: {}/{} with {}/{}", new 
Object[] { node, 
+            user, nodeCredentials.getUser(), localCredentials, nodeCredentials 
});
         
-        if (groovyTruth(nodeCredentials.getUser())) {
-            if (user==null) {
+        if (Strings.isNonBlank(nodeCredentials.getUser())) {
+            if (Strings.isBlank(user)) {
                 setup.put(USER, user = nodeCredentials.getUser());
-            } else if ("root".equals(user) && 
ROOT_ALIASES.contains(nodeCredentials.getUser())) {
+            } else if (ROOT_USERNAME.equals(user) && 
ROOT_ALIASES.contains(nodeCredentials.getUser())) {
                 // deprecated, we used to default username to 'root'; now we 
leave null, then use autodetected credentials if no user specified
-                // 
                 LOG.warn("overriding username 'root' in favour of 
'"+nodeCredentials.getUser()+"' at {}; this behaviour may be removed in 
future", node);
                 setup.put(USER, user = nodeCredentials.getUser());
             }
             
-            String pkd = elvis(localPrivateKeyData, 
nodeCredentials.getPrivateKey());
-            String pwd = elvis(localPassword, nodeCredentials.getPassword());
-            if (user==null || (pkd==null && pwd==null)) {
+            String pkd = 
Strings.maybeNonBlank(localCredentials.getPrivateKeyData()).or(nodeCredentials.getOptionalPrivateKey().orNull());
+            String pwd = 
Strings.maybeNonBlank(localCredentials.getPassword()).or(nodeCredentials.getOptionalPassword().orNull());
+            if (Strings.isBlank(user) || (Strings.isBlank(pkd) && pwd==null)) {
                 String missing = (user==null ? "user" : "credential");
                 LOG.warn("Not able to determine "+missing+" for "+this+" at 
"+node+"; will likely fail subsequently");
                 return null;
             } else {
-                LoginCredentials.Builder resultBuilder = 
LoginCredentials.builder()
-                        .user(user);
-                if (pkd!=null) resultBuilder.privateKey(pkd);
-                if (pwd!=null && pkd==null) resultBuilder.password(pwd);
+                LoginCredentials.Builder resultBuilder = 
LoginCredentials.builder().user(user);
+                if (pwd!=null && (Strings.isBlank(pkd) || 
localCredentials.isUsingPassword())) 
+                    resultBuilder.password(pwd);
+                else // pkd guaranteed non-blank due to above  
+                    resultBuilder.privateKey(pkd);
                 return resultBuilder.build();        
             }
         }

http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/fa79e20d/utils/common/src/main/java/brooklyn/util/text/Strings.java
----------------------------------------------------------------------
diff --git a/utils/common/src/main/java/brooklyn/util/text/Strings.java 
b/utils/common/src/main/java/brooklyn/util/text/Strings.java
index 3117d7d..d874783 100644
--- a/utils/common/src/main/java/brooklyn/util/text/Strings.java
+++ b/utils/common/src/main/java/brooklyn/util/text/Strings.java
@@ -33,6 +33,7 @@ import javax.annotation.Nullable;
 
 import brooklyn.util.collections.MutableList;
 import brooklyn.util.collections.MutableMap;
+import brooklyn.util.guava.Maybe;
 import brooklyn.util.time.Time;
 
 import com.google.common.base.CharMatcher;
@@ -108,6 +109,12 @@ public class Strings {
         return !isBlank(s);
     }
 
+    /** @return a {@link Maybe} object which is absent if the argument {@link 
#isBlank(CharSequence)} */
+    public static <T extends CharSequence> Maybe<T> maybeNonBlank(T s) {
+        if (isNonBlank(s)) return Maybe.of(s);
+        return Maybe.absent();
+    }
+
     /** throws IllegalArgument if string not empty; cf. guava 
Preconditions.checkXxxx */
     public static void checkNonEmpty(CharSequence s) {
         if (s==null) throw new IllegalArgumentException("String must not be 
null");

Reply via email to