fix: Removed service name from storage account name generator (#232)

wbreza · tbarlow12 · commit d1fcfc9cbad1 · 2019-09-13T14:07:02.000-07:00
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.
diff --git a/package-lock.json b/package-lock.json
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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.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/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.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);
+  };
 });
diff --git 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,55 +40,67 @@ 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;
 
     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);
