You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrea Turli <no...@github.com> on 2016/07/13 12:43:41 UTC

[jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

You can view, comment on, or merge this pull request online at:

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

-- Commit Summary --

  * [azure-arm] add controlled storage account name generation

-- File Changes --

    M azurecompute-arm/src/main/java/org/jclouds/azurecompute/arm/util/DeploymentTemplateBuilder.java (86)

-- Patch Links --

https://github.com/jclouds/jclouds-labs/pull/301.patch
https://github.com/jclouds/jclouds-labs/pull/301.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/301

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Rita Zhang <no...@github.com>.
@andreaturli LGTM!

---
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/301#issuecomment-236643545

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Andrea Turli <no...@github.com>.
@ritazh can you have a look at this PR?

---
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/301#issuecomment-236138370

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Rita Zhang <no...@github.com>.
@andreaturli this is awesome! I will test it soon.

---
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/301#issuecomment-236297267

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -487,4 +496,37 @@ private boolean keyVaultInUse(){
>        return !Strings.isNullOrEmpty(options.getKeyVaultIdAndSecret());
>     }
>  
> +   /**
> +    * Generates a valid storage account
> +    *
> +    * Storage account names must be between 3 and 24 characters in length and may contain numbers and lowercase letters only.
> +    *
> +    * @param name the node name
> +    * @return the storage account name starting from a sanitized name (with only numbers and lowercase letters only ).
> +    * If sanitized name is between 3 and 24 characters, storage account name is equals to sanitized name.
> +    * If sanitized name is less than 3 characters, storage account is sanitized name plus 4 random chars.
> +    * If sanitized name is more than 24 characters, storage account is first 10 chars of sanitized name plus 4 random chars plus last 10 chars of sanitized name.
> +    */
> +   private static String generateStorageAccountName(String name) {

Please, add appropriate unit tests for this method so we don't break it unintentionally in the future. Tests should properly test the naming convention and the boundaries.

---
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/301/files/a34066c4331d7552669ed4fb005feec2374c442e#r70630490

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Andrea Turli <no...@github.com>.
@andreaturli pushed 1 commit.

1a2193f  add unit test for storage account name generation


---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/301/files/a34066c4331d7552669ed4fb005feec2374c442e..1a2193f8d6c767e668d6764f7493f513f54bdca9

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Andrea Turli <no...@github.com>.
Merged at [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/d0b07a66)

---
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/301#issuecomment-238022091

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Andrea Turli <no...@github.com>.
thx @ritazh merging it now

---
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/301#issuecomment-238021534

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Andrea Turli <no...@github.com>.
Closed #301.

---
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/301#event-747433245

Re: [jclouds/jclouds-labs] [azure-arm] add controlled storage account name generation (#301)

Posted by Ignasi Barrera <no...@github.com>.
> @@ -132,6 +132,41 @@ void testVirtualMachine() {
>     }
>  
>     @Test
> +   void testAddStorageResourceWhenNameIsLongerThan24Chars() {
> +      String name = "thishasmorethan24characters";
> +      DeploymentTemplateBuilder builder = getMockDeploymentTemplateBuilderWithEmptyOptions(name);
> +
> +      DeploymentBody deploymentBody = builder.getDeploymentTemplate();
> +      assertTrue(Iterables.contains(deploymentBody.template().variables().keySet(), "storageAccountName"));
> +      String storageAccountName = deploymentBody.template().variables().get("storageAccountName");
> +      assertEquals(storageAccountName.length(), 24);
> +      assertEquals(storageAccountName.substring(0, 10), name.replaceAll("[^a-z0-9]", "").substring(0, 10));
> +      assertEquals(storageAccountName.substring(storageAccountName.length() - 10, storageAccountName.length()), name.replaceAll("[^a-z0-9]", "").substring(name.length() - 10, name.length()));

Tests should not replicate the business logic (or they aren't actually testing that it produces the expected value; if the business logic is broken, the test is broken too).
Instead, make assertions against concrete values. Assert that the value is a concrete one, or starts with the expected String and has N prefix and N suffix characters, etc.

---
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/301/files/952392e682c37a0fd7bbe6513296faec3c49c719#r70884679