Repository: serverless/serverless-azure-functions
Pull request #232 (2019-08-08 01:14:01)

fix: Removed service name from storage account name generator
When the storage account name is generated it includes a hash of the sls service name.  By default if you deploy multiple services/apps into the same resource group this causes multiple storage accounts to be created vs. reusing the same one.  By removing the service name from the storage account generation logic it simplifies the case to reuse the same account.  This value can still be overridden by specifying your own name in the sls yaml configuration.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/25050715/badge)](https://coveralls.io/builds/25050715)

Coverage decreased (-0.05%) to 84.246% when pulling **bcaf8c2899a9c9aea215ab428d0f869279b0b205 on wabrez/storage-name-refactor** into **4e46a0314bb63eebf9c20673d394a66942cebfc0 on dev**.


Patch:
diff --git a/package-lock.json b/package-lock.json
index e64a9f6..fecdc6a 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -2852,11 +2852,6 @@
       "integrity": "sha512-mT8iDcrh03qDGRRmoA2hmBJnxpllMR+0/0qlzjqZES6NdiWDcZkCNAk4rPFZ9Q85r27unkiNNg8ZOiwZXBHwcA==",
       "dev": true
     },
-    "charenc": {
-      "version": "0.0.2",
-      "resolved": "https://registry.npmjs.org/charenc/-/charenc-0.0.2.tgz",
-      "integrity": "sha1-wKHS86cJLgN3S/qD8UwPxXkKhmc="
-    },
     "chokidar": {
       "version": "2.1.6",
       "resolved": "https://registry.npmjs.org/chokidar/-/chokidar-2.1.6.tgz",
@@ -3306,11 +3301,6 @@
         "which": "^1.2.9"
       }
     },
-    "crypt": {
-      "version": "0.0.2",
-      "resolved": "https://registry.npmjs.org/crypt/-/crypt-0.0.2.tgz",
-      "integrity": "sha1-iNf/fsDfuG9xPch7u0LQRNPmxBs="
-    },
     "crypto-browserify": {
       "version": "3.12.0",
       "resolved": "https://registry.npmjs.org/crypto-browserify/-/crypto-browserify-3.12.0.tgz",
@@ -7573,16 +7563,6 @@
         "object-visit": "^1.0.0"
       }
     },
-    "md5": {
-      "version": "2.2.1",
-      "resolved": "https://registry.npmjs.org/md5/-/md5-2.2.1.tgz",
-      "integrity": "sha1-U6s41f48iJG6RlMp6iP6wFQBJvk=",
-      "requires": {
-        "charenc": "~0.0.1",
-        "crypt": "~0.0.1",
-        "is-buffer": "~1.1.1"
-      }
-    },
     "md5.js": {
       "version": "1.3.5",
       "resolved": "https://registry.npmjs.org/md5.js/-/md5.js-1.3.5.tgz",
diff --git a/package.json b/package.json
index 6018b19..8305eba 100644
--- a/package.json
+++ b/package.json
@@ -50,7 +50,6 @@
     "js-yaml": "^3.13.1",
     "jsonpath": "^1.0.1",
     "lodash": "^4.16.6",
-    "md5": "^2.2.1",
     "open": "^6.3.0",
     "request": "^2.81.0",
     "rimraf": "^2.6.3",
diff --git a/src/armTemplates/resources/apim.ts b/src/armTemplates/resources/apim.ts
index 5003361..2bd6652 100644
--- a/src/armTemplates/resources/apim.ts
+++ b/src/armTemplates/resources/apim.ts
@@ -5,11 +5,7 @@ import { AzureNamingService } from "../../services/namingService";
 
 export class ApimResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
-    return AzureNamingService.getResourceName(
-      config,
-      config.provider.apim,
-      "apim"
-    );
+    return AzureNamingService.getResourceName(config, config.provider.apim, "apim");
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/armTemplates/resources/appInsights.ts b/src/armTemplates/resources/appInsights.ts
index 9426a0b..ab72cc9 100644
--- a/src/armTemplates/resources/appInsights.ts
+++ b/src/armTemplates/resources/appInsights.ts
@@ -4,11 +4,7 @@ import { AzureNamingService } from "../../services/namingService";
 
 export class AppInsightsResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
-    return AzureNamingService.getResourceName(
-      config,
-      config.provider.appInsights,
-      "appinsights"
-    );
+    return AzureNamingService.getResourceName(config, config.provider.appInsights, "appinsights");
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/armTemplates/resources/appServicePlan.ts b/src/armTemplates/resources/appServicePlan.ts
index 4485cb0..50d99e6 100644
--- a/src/armTemplates/resources/appServicePlan.ts
+++ b/src/armTemplates/resources/appServicePlan.ts
@@ -4,11 +4,7 @@ import { AzureNamingService } from "../../services/namingService";
 
 export class AppServicePlanResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
-    return AzureNamingService.getResourceName(
-      config,
-      config.provider.appInsights,
-      "asp"
-    );
+    return AzureNamingService.getResourceName(config, config.provider.appInsights, "asp");
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/armTemplates/resources/functionApp.ts b/src/armTemplates/resources/functionApp.ts
index 1af37a3..0263490 100644
--- a/src/armTemplates/resources/functionApp.ts
+++ b/src/armTemplates/resources/functionApp.ts
@@ -5,11 +5,8 @@ import { AzureNamingService } from "../../services/namingService";
 export class FunctionAppResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
     const safeServiceName = config.service.replace(/\s/g, "-");
-    return AzureNamingService.getResourceName(
-      config,
-      config.provider.appInsights,
-      safeServiceName
-    );
+
+    return AzureNamingService.getResourceName(config, config.provider.appInsights, safeServiceName);
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/armTemplates/resources/hostingEnvironment.ts b/src/armTemplates/resources/hostingEnvironment.ts
index 7c8332b..0cf8671 100644
--- a/src/armTemplates/resources/hostingEnvironment.ts
+++ b/src/armTemplates/resources/hostingEnvironment.ts
@@ -4,11 +4,7 @@ import { AzureNamingService } from "../../services/namingService";
 
 export class HostingEnvironmentResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
-    return AzureNamingService.getResourceName(
-      config,
-      config.provider.hostingEnvironment,
-      "ase"
-    );
+    return AzureNamingService.getResourceName(config, config.provider.hostingEnvironment, "ase");
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/armTemplates/resources/storageAccount.ts b/src/armTemplates/resources/storageAccount.ts
index 69dc7cd..86832ae 100644
--- a/src/armTemplates/resources/storageAccount.ts
+++ b/src/armTemplates/resources/storageAccount.ts
@@ -5,11 +5,7 @@ import configConstants from "../../config";
 
 export class StorageAccountResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
-    return AzureNamingService.getSafeResourceName(
-      config,
-      configConstants.naming.maxLength.storageAccount,
-      config.provider.storageAccount
-    );
+    return AzureNamingService.getSafeResourceName(config, configConstants.naming.maxLength.storageAccount, config.provider.storageAccount);
   }
 
   public getTemplate(): ArmResourceTemplate {
diff --git a/src/services/namingService.ts b/src/services/namingService.ts
index 10c3755..8d7df13 100644
--- a/src/services/namingService.ts
+++ b/src/services/namingService.ts
@@ -1,6 +1,5 @@
 import { ServerlessAzureConfig, ResourceConfig } from "../models/serverless"
 import { Guard } from "../shared/guard"
-import md5 from "md5";
 import configConstants from "../config";
 
 export class AzureNamingService {
@@ -11,23 +10,26 @@ export class AzureNamingService {
    * {prefix}-{shortRegionName}-{shortStageName}(optionally: -{suffix})
    *
    * @param config Serverless Azure Config for service (serverless.service)
-   * @param resourceConfig
-   * @param suffix
+   * @param resourceConfig The serverless resource configuration
+   * @param suffix Optional suffix to append on the end of the generated name
    */
   public static getResourceName(config: ServerlessAzureConfig, resourceConfig?: ResourceConfig, suffix?: string) {
     if (resourceConfig && resourceConfig.name) {
       return resourceConfig.name;
     }
+
     const { prefix, region, stage } = config.provider
     let name = [
       prefix,
       this.createShortAzureRegionName(region),
       this.createShortStageName(stage),
     ].join("-");
+
     if (suffix) {
       name += `-${suffix}`;
     }
-    return name.toLocaleLowerCase();
+
+    return name.toLowerCase();
   }
 
   /**
@@ -38,43 +40,53 @@ export class AzureNamingService {
    *
    * @param config Serverless Azure Config for service (serverless.service)
    * @param maxLength Maximum length of name for resource
-   * @param resourceConfig Configuration for resource from serverless configuration
+   * @param resourceConfig The serverless resource configuration
+   * @param suffix Optional suffix to append on the end of the generated name
    * @param forbidden Regex for characters to remove from name. Defaults to non-alpha-numerics
    * @param replaceWith String to replace forbidden characters. Defaults to empty string
    */
-  public static getSafeResourceName(config: ServerlessAzureConfig, maxLength: number, resourceConfig?: ResourceConfig, forbidden = /\W+/g, replaceWith = "") {
+  public static getSafeResourceName(config: ServerlessAzureConfig, maxLength: number, resourceConfig?: ResourceConfig, suffix: string = "", forbidden: RegExp = /\W+/g, replaceWith: string = "") {
     if (resourceConfig && resourceConfig.name) {
       const { name } = resourceConfig;
+
       if (name.length > maxLength) {
         throw new Error(`Name '${name}' invalid. Should be shorter than ${maxLength} characters`);
       }
+
       return name.replace(forbidden, replaceWith);
     }
 
     const { prefix, region, stage } = config.provider;
 
-    const nameHash = md5(config.service);
-
     let safePrefix = prefix.replace(forbidden, replaceWith);
     const safeRegion = this.createShortAzureRegionName(region);
     let safeStage = this.createShortStageName(stage);
-    let safeNameHash = nameHash.substr(0, 6);
+    let safeSuffix = suffix.replace(forbidden, replaceWith);
 
-    const remaining = maxLength - (safePrefix.length + safeRegion.length + safeStage.length + safeNameHash.length);
+    const remaining = maxLength - (safePrefix.length + safeRegion.length + safeStage.length + safeSuffix.length);
 
     // Dynamically adjust the substring based on space needed
     if (remaining < 0) {
-      const partLength = Math.floor(Math.abs(remaining) / 3);
+      let partLength = Math.floor(Math.abs(remaining) / 4);
+      if (partLength < 3) {
+        partLength = 3;
+      }
+
       safePrefix = safePrefix.substr(0, partLength);
       safeStage = safeStage.substr(0, partLength);
-      safeNameHash = safeNameHash.substr(0, partLength);
+      safeSuffix = safeSuffix.substr(0, partLength);
     }
 
-    return [safePrefix, safeRegion, safeStage, safeNameHash]
+    return [safePrefix, safeRegion, safeStage, safeSuffix]
       .join("")
-      .toLocaleLowerCase();
+      .toLowerCase();
   }
 
+  /**
+   * Creates a deployment name from the serverless configuration
+   * @param config The serverless azure config
+   * @param timestamp The timestamp of the deployment
+   */
   public static getDeploymentName(config: ServerlessAzureConfig, timestamp?: string) {
     let maxLength = configConstants.naming.maxLength.deploymentName;
     const suffix = configConstants.naming.suffix.deployment;
@@ -82,11 +94,13 @@ export class AzureNamingService {
     const { deploymentName } = config.provider
 
     if (timestamp) {
-      maxLength -= timestamp.length + suffix.length;;
+      maxLength -= timestamp.length + suffix.length;
+
+      const name = (deploymentName)
+        ? deploymentName.substr(0, maxLength)
+        : [AzureNamingService.getSafeResourceName(config, maxLength, null, config.service), suffix].join("-");
 
-      const name = (deploymentName) ? deploymentName.substr(0, maxLength)
-        : [ AzureNamingService.getSafeResourceName(config, maxLength), suffix ].join("-");
-      return [ name, timestamp ].join("-");
+      return [name, timestamp].join("-");
     }
 
     return deploymentName.substr(0, maxLength);


Test patch:
diff --git a/src/armTemplates/resources/storageAccount.test.ts b/src/armTemplates/resources/storageAccount.test.ts
index b398cda..5d20643 100644
--- a/src/armTemplates/resources/storageAccount.test.ts
+++ b/src/armTemplates/resources/storageAccount.test.ts
@@ -1,4 +1,3 @@
-import md5 from "md5";
 import { ServerlessAzureConfig } from "../../models/serverless";
 import { AzureNamingService } from "../../services/namingService";
 import { StorageAccountResource } from "./storageAccount";
@@ -35,8 +34,7 @@ describe("Storage Account Resource", () => {
         prefix: "my-long-test-prefix-name",
         region: "Australia Southeast",
         stage: "development"
-      },
-      service: "my-long-test-api",
+      }
     };
 
     const result = StorageAccountResource.getResourceName(testConfig);
@@ -145,10 +143,9 @@ describe("Storage Account Resource", () => {
     expect(value).toContain(AzureNamingService.createShortAzureRegionName(config.provider.region));
     expect(value).toContain(createSafeString(config.provider.prefix));
     expect(value).toContain(createSafeString(config.provider.stage));
-    expect(value).toContain(md5(config.service).substr(0, 3));
   }
 
   function createSafeString(value: string) {
-    return value.replace(/\W+/g, "").toLocaleLowerCase().substr(0, 3);
+    return value.replace(/\W+/g, "").toLowerCase().substr(0, 3);
   };
 });
diff --git a/src/services/namingService.test.ts b/src/services/namingService.test.ts
index 33749c4..d58fbc1 100644
--- a/src/services/namingService.test.ts
+++ b/src/services/namingService.test.ts
@@ -1,7 +1,7 @@
 import { AzureNamingService } from "./namingService"
+import { ServerlessAzureConfig } from "../models/serverless";
 
 describe("Naming Service", () => {
-
   it("Creates a short name for an azure region", () => {
     const expected = "ausse";
     const actual = AzureNamingService.createShortAzureRegionName("australiasoutheast");
@@ -86,4 +86,57 @@ describe("Naming Service", () => {
     const actual = AzureNamingService.getNormalizedRegionName(expected);
     expect(actual).toEqual(expected);
   });
+
+  it("deployment name is generated correctly", () => {
+    const config: ServerlessAzureConfig = {
+      functions: [],
+      plugins: [],
+      provider: {
+        prefix: "sls",
+        name: "azure",
+        region: "westus",
+        stage: "dev",
+      },
+      service: "test-api"
+    };
+
+    const timestamp = Date.now();
+    const deploymentName = AzureNamingService.getDeploymentName(config, `t${timestamp}`);
+
+    expect(deploymentName).toEqual(`slswusdevtestapi-DEPLOYMENT-t${timestamp}`);
+    assertValidDeploymentName(config, deploymentName, timestamp);
+  });
+
+  it("deployment name with long suffix or service name generated correctly", () => {
+    const config: ServerlessAzureConfig = {
+      functions: [],
+      plugins: [],
+      provider: {
+        prefix: "sls-long-prefix-name",
+        name: "azure",
+        region: "westus",
+        stage: "multicloud",
+      },
+      service: "extra-long-service-name"
+    };
+
+    const timestamp = Date.now();
+    const deploymentName = AzureNamingService.getDeploymentName(config, `t${timestamp}`);
+
+    expect(deploymentName).toEqual(`slswusmulext-DEPLOYMENT-t${timestamp}`);
+    assertValidDeploymentName(config, deploymentName, timestamp);
+  });
+
+  function assertValidDeploymentName(config: ServerlessAzureConfig, value: string, timestamp: number) {
+    expect(value.length).toBeLessThanOrEqual(64);
+    expect(value).toContain(timestamp);
+    expect(value).toContain(AzureNamingService.createShortAzureRegionName(config.provider.region));
+    expect(value).toContain(createSafeString(config.provider.prefix));
+    expect(value).toContain(createSafeString(config.provider.stage));
+    expect(value).toContain(createSafeString(config.service));
+  }
+
+  function createSafeString(value: string) {
+    return value.replace(/\W+/g, "").toLowerCase().substr(0, 3);
+  };
 });