Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/465#discussion_r23522150
  
    --- Diff: 
locations/jclouds/src/main/java/brooklyn/location/jclouds/JcloudsLocation.java 
---
    @@ -1305,118 +1326,148 @@ public UserCreation(LoginCredentials creds, 
List<Statement> statements) {
          *   
          * @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,
    +     * or null if we are just going to use those from jclouds.
          */
         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).
    +        //NB: private key is not installed remotely, just used to 
get/validate the public key
             
    -        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);
    +        credential.checkNoErrors().logAnyWarnings();
    +        String passwordToSet = 
Strings.isNonBlank(credential.getPassword()) ? credential.getPassword() : 
Identifiers.makeRandomId(12);
             List<Statement> statements = Lists.newArrayList();
             
             if (groovyTruth(dontCreateUser)) {
    -            // TODO For dontCreateUser, we probably only want to treat it 
special if user was explicitly supplied
    -            // (rather than it just being the default config key value). 
If user was explicit, then should
    -            // set the password + authorize the key for that user. 
Presumably the caller knows that this
    -            // user pre-exists on the given VM image.
    +            // dontCreateUser:
    +            // if caller has not specified a user, we'll just continue to 
use the loginUser;
    +            // if caller *has*, we set up our credentials assuming that 
user and credentials already exist
    +            
                 if (!groovyTruth(user)) {
    -                // loginCreds result will be null; use creds returned by 
jclouds on the node
    +                // createdUserCreds returned from this method will be 
null; 
    +                // we will use the creds returned by jclouds on the node
                     LOG.info("Not setting up any user (subsequently using 
loginUser {})", user, loginUser);
                     config.put(USER, loginUser);
                     
                 } 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();
    +                LOG.info("Not creating user {}, and not installing its 
password or authorizing keys (assuming it exists)", user);
    +
    +                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(passwordToSet)) {
    +                statements.add(new 
ReplaceShadowPasswordEntry(Sha512Crypt.function(), user, passwordToSet));
    +                createdUserCreds = 
LoginCredentials.builder().user(user).password(passwordToSet).build();
                 }
    -            if (publicKeyData!=null) {
    -                statements.add(new 
AuthorizeRSAPublicKeys("~"+loginUser+"/.ssh", ImmutableList.of(publicKeyData)));
    -                if (privateKeyData != null) {
    -                    loginCreds = 
LoginCredentials.builder().user(loginUser).privateKey(privateKeyData).build();
    +            if (Strings.isNonBlank(credential.getPublicKeyData())) {
    +                statements.add(new 
AuthorizeRSAPublicKeys("~"+user+"/.ssh", 
ImmutableList.of(credential.getPublicKeyData())));
    +                if (!credential.isUsingPassword() && 
Strings.isNonBlank(credential.getPrivateKeyData())) {
    +                    createdUserCreds = 
LoginCredentials.builder().user(user).privateKey(credential.getPrivateKeyData()).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();
    +        } else {
    +            String pubKey = credential.getPublicKeyData();
    +            String privKey = credential.getPrivateKeyData();
    +            
    +            if (credential.get()==null) {
    +                if (!loggedSshKeysHint && 
!config.containsKey(PRIVATE_KEY_FILE)) {
    +                    loggedSshKeysHint = true;
    +                    LOG.info("Default SSH keys not found or not usable; 
will create new keys for each machine. "
    +                        + "Create ~/.ssh/id_rsa or "
    +                        + "set "+PRIVATE_KEY_FILE.getName()+" / 
"+PRIVATE_KEY_PASSPHRASE.getName()+" / "+PASSWORD.getName()+" "
    +                        + "as appropriate for this location if you wish to 
be able to log in without Brooklyn.");
                     }
    +                KeyPair newKeyPair = SecureKeys.newKeyPair();
    +                pubKey = SecureKeys.toPub(newKeyPair);
    +                privKey = SecureKeys.toPem(newKeyPair);
    +                LOG.debug("Brooklyn key being created for "+user+" at new 
machine "+this+" is:\n"+privKey);
                 }
    +            // ensure credential is not used any more, as we have 
extracted al useful info
    +            credential = null;
                 
    -        } else {
                 // Create the user
                 // 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)
    -                    .grantSudoToAdminUser(groovyTruth(grantUserSudo))
    -                    .resetLoginPassword(true)
    -                    .loginPassword(password);
    -
    -            if (publicKeyData!=null) {
    -                
adminBuilder.authorizeAdminPublicKey(true).adminPublicKey(publicKeyData);
    +                    .grantSudoToAdminUser(groovyTruth(grantUserSudo));
    +                    // login user's password now set to something random
    +                    // (could explicitly set with loginPassword)
    +            
    +            Boolean useKey = null;
    +            if (passwordToSet!=null) {
    +                adminBuilder.adminPassword(passwordToSet);
    +                useKey = false;
    --- End diff --
    
    This looks wrong, so not sure how it works. Presumably we'll always go into 
this if branch because `passwordToSet` is never null. Therefore we'll always 
set `userKey = false`.


---
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.
---

Reply via email to