You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Svet <no...@github.com> on 2017/07/25 12:10:00 UTC

[jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Depends on https://github.com/jclouds/jclouds/pull/1123
As discussed in https://github.com/jclouds/jclouds-labs/pull/402
You can view, comment on, or merge this pull request online at:

  https://github.com/jclouds/jclouds-labs/pull/404

-- Commit Summary --

  * Re-use the just added Passwords from jclouds-core

-- File Changes --

    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/compute/strategy/CreateResourcesThenCreateNodes.java (2)
    D azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/Passwords.java (32)
    M oneandone/src/main/java/org/apache/jclouds/oneandone/rest/compute/OneandoneComputeServiceAdapter.java (2)
    D oneandone/src/main/java/org/apache/jclouds/oneandone/rest/util/Passwords.java (64)
    M profitbricks-rest/src/main/java/org/apache/jclouds/profitbricks/rest/compute/ProfitBricksComputeServiceAdapter.java (2)
    M profitbricks-rest/src/main/java/org/apache/jclouds/profitbricks/rest/domain/Volume.java (2)
    D profitbricks-rest/src/main/java/org/apache/jclouds/profitbricks/rest/util/Passwords.java (64)
    D profitbricks-rest/src/test/java/org/apache/jclouds/profitbricks/rest/util/PasswordsTest.java (53)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/404.patch
https://github.com/jclouds/jclouds-labs/pull/404.diff

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404

Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Posted by Svet <no...@github.com>.
Merged in [master](https://git1-us-west.apache.org/repos/asf?p=jclouds-labs.git;a=commit;h=c9af1b248e3cfd0ce312823436e986726e826ccd).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404#issuecomment-319347263

Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Posted by Ignasi Barrera <no...@github.com>.
nacx commented on this pull request.



> @@ -50,7 +50,7 @@
 import org.jclouds.azurecompute.arm.domain.Subnet.SubnetProperties;
 import org.jclouds.azurecompute.arm.domain.VirtualNetwork.AddressSpace;
 import org.jclouds.azurecompute.arm.domain.VirtualNetwork.VirtualNetworkProperties;
-import org.jclouds.azurecompute.arm.util.Passwords;
+import org.jclouds.util.Passwords;
 import org.jclouds.compute.config.CustomizationResponse;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.Template;

Does this default implementations generate passwords that are valid for Azure windows VMs?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404#pullrequestreview-52068773

Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Posted by Svet <no...@github.com>.
retest this please
Dependent changes merged in jclouds, next build should be successful.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404#issuecomment-319082215

Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Posted by Svet <no...@github.com>.
neykov commented on this pull request.



> @@ -50,7 +50,7 @@
 import org.jclouds.azurecompute.arm.domain.Subnet.SubnetProperties;
 import org.jclouds.azurecompute.arm.domain.VirtualNetwork.AddressSpace;
 import org.jclouds.azurecompute.arm.domain.VirtualNetwork.VirtualNetworkProperties;
-import org.jclouds.azurecompute.arm.util.Passwords;
+import org.jclouds.util.Passwords;
 import org.jclouds.compute.config.CustomizationResponse;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.Template;

Short answer: yes
Couldn't find authoritative docs on what the exact requirements for the password are. The generator is pretty conservative with the characters used and testing the output with Azure works fine.
I'd do one more change with the password generator, but in a separate PR - use a fixed max length for the password. Currently it will pick a length between 8 and 50 characters. Don't think it adds any security. I'd go with the MAX always for consistency. It will also bring out any problems with character length support in the providers (say if tests have been passing, but the password being generated is not in the extremes).

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404#discussion_r129312641

Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Posted by Svet <no...@github.com>.
Tested azure with the max password length (50), merging.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404#issuecomment-319327205

Re: [jclouds/jclouds-labs] Re-use the just added Passwords from jclouds-core (#404)

Posted by Ignasi Barrera <no...@github.com>.
nacx approved this pull request.

LGTM so far. Just one comment about making sure the default implementation works for Azure.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/404#pullrequestreview-52068735