Mailing-List: contact dev-help@brooklyn.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@brooklyn.incubator.apache.org
From: aledsage <git@git.apache.org>
To: dev@brooklyn.incubator.apache.org
Reply-To: dev@brooklyn.incubator.apache.org
References: <git-pr-465-incubator-brooklyn@git.apache.org>
In-Reply-To: <git-pr-465-incubator-brooklyn@git.apache.org>
Subject: [GitHub] incubator-brooklyn pull request: Jclouds tidy
Content-Type: text/plain
Message-Id: <20150126103702.4B6ABE03F2@git1-us-west.apache.org>
Date: Mon, 26 Jan 2015 10:37:02 +0000 (UTC)

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

    https://github.com/apache/incubator-brooklyn/pull/465#discussion_r23521961
  
    --- 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);
    --- End diff --
    
    If Brooklyn makes up a new password, then should we log that? Looks like it's not logged anywhere currently. Most important to log it if the auto-generated password is the only subsequent login approach.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---