fix: Update storage account naming convention (#190)

Ensures storage account names do not clash based on combination of prefix, region, stage & service name

- Ensures generated name is <= 24 chars
-  Ensures region names don't conflict
-  Generates short region part from full value
-  Generates short stage part from full value
-  Includes partial hash of service name in full name

Resolves AB#420

Author: wbreza

package-lock.json

@@ -46,6 +46,7 @@
     "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",

package.json

@@ -1,6 +1,10 @@
-import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../models/armTemplates";
+import {
+  ArmResourceTemplateGenerator,
+  ArmResourceTemplate
+} from "../models/armTemplates";
 import { Guard } from "../shared/guard";
 import { ServerlessAzureConfig } from "../models/serverless";
+import { Utils } from "../shared/utils";
 
 export class CompositeArmTemplate implements ArmResourceTemplateGenerator {
   public constructor(private childTemplates: ArmResourceTemplateGenerator[]) {
@@ -9,22 +13,23 @@ export class CompositeArmTemplate implements ArmResourceTemplateGenerator {
 
   public getTemplate(): ArmResourceTemplate {
     const template: ArmResourceTemplate = {
-      "$schema": "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
-      "contentVersion": "1.0.0.0",
-      "parameters": {},
-      "resources": [],
+      $schema:
+        "https://schema.management.azure.com/schemas/2015-01-01/deploymentTemplate.json#",
+      contentVersion: "1.0.0.0",
+      parameters: {},
+      resources: []
     };
 
-    this.childTemplates.forEach((resource) => {
+    this.childTemplates.forEach(resource => {
       const resourceTemplate = resource.getTemplate();
       template.parameters = {
         ...template.parameters,
-        ...resourceTemplate.parameters,
+        ...resourceTemplate.parameters
       };
 
       template.resources = [
         ...template.resources,
-        ...resourceTemplate.resources,
+        ...resourceTemplate.resources
       ];
     });
 
@@ -34,14 +39,14 @@ export class CompositeArmTemplate implements ArmResourceTemplateGenerator {
   public getParameters(config: ServerlessAzureConfig) {
     let parameters = {};
 
-    this.childTemplates.forEach((resource) => {
+    this.childTemplates.forEach(resource => {
       parameters = {
         ...parameters,
         ...resource.getParameters(config),
-        location: config.provider.region,
-      }
+        location: Utils.getNormalizedRegionName(config.provider.region)
+      };
     });
 
     return parameters;
   }
-}
+}

src/armTemplates/compositeArmTemplate.ts

@@ -1,12 +1,13 @@
 import { ServerlessAzureConfig } from "../../models/serverless";
 import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../../models/armTemplates";
 import { ApiManagementConfig } from "../../models/apiManagement";
+import { Utils } from "../../shared/utils";
 
 export class ApimResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
     return config.provider.apim && config.provider.apim.name
       ? config.provider.apim.name
-      : `${config.provider.prefix}-${config.provider.region}-${config.provider.stage}-apim`;
+      : `${config.provider.prefix}-${Utils.createShortAzureRegionName(config.provider.region)}-${Utils.createShortStageName(config.provider.stage)}-apim`;
   }
 
   public getTemplate(): ArmResourceTemplate {

src/armTemplates/resources/apim.ts

@@ -1,11 +1,12 @@
 import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../../models/armTemplates";
 import { ServerlessAzureConfig } from "../../models/serverless";
+import { Utils } from "../../shared/utils";
 
 export class AppInsightsResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
     return config.provider.appInsights && config.provider.appInsights.name
       ? config.provider.appInsights.name
-      : `${config.provider.prefix}-${config.provider.region}-${config.provider.stage}-appinsights`;
+      : `${config.provider.prefix}-${Utils.createShortAzureRegionName(config.provider.region)}-${Utils.createShortStageName(config.provider.stage)}-appinsights`;
   }
 
   public getTemplate(): ArmResourceTemplate {

src/armTemplates/resources/appInsights.ts

@@ -1,11 +1,12 @@
 import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../../models/armTemplates";
 import { ServerlessAzureConfig, ResourceConfig } from "../../models/serverless";
+import { Utils } from "../../shared/utils";
 
 export class AppServicePlanResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
     return config.provider.appServicePlan && config.provider.appServicePlan.name
       ? config.provider.appServicePlan.name
-      : `${config.provider.prefix}-${config.provider.region}-${config.provider.stage}-asp`;
+      : `${config.provider.prefix}-${Utils.createShortAzureRegionName(config.provider.region)}-${Utils.createShortStageName(config.provider.stage)}-asp`;
   }
 
   public getTemplate(): ArmResourceTemplate {

src/armTemplates/resources/appServicePlan.ts

@@ -1,11 +1,14 @@
 import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../../models/armTemplates";
 import { ServerlessAzureConfig, FunctionAppConfig } from "../../models/serverless";
+import { Utils } from "../../shared/utils";
 
 export class FunctionAppResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
+    const safeServiceName = config.service.replace(/\s/g, "-");
+
     return config.provider.functionApp && config.provider.functionApp.name
       ? config.provider.functionApp.name
-      : `${config.provider.prefix}-${config.provider.region}-${config.provider.stage}-${config.service}`;
+      : `${config.provider.prefix}-${Utils.createShortAzureRegionName(config.provider.region)}-${Utils.createShortStageName(config.provider.stage)}-${safeServiceName}`;
   }
 
   public getTemplate(): ArmResourceTemplate {

src/armTemplates/resources/functionApp.ts

@@ -1,11 +1,12 @@
 import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../../models/armTemplates";
 import { ServerlessAzureConfig } from "../../models/serverless";
+import { Utils } from "../../shared/utils";
 
 export class HostingEnvironmentResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
     return config.provider.hostingEnvironment && config.provider.hostingEnvironment.name
       ? config.provider.hostingEnvironment.name
-      : `${config.provider.prefix}-${config.provider.region}-${config.provider.stage}-ase`;
+      : `${config.provider.prefix}-${Utils.createShortAzureRegionName(config.provider.region)}-${Utils.createShortStageName(config.provider.stage)}-ase`;
   }
 
   public getTemplate(): ArmResourceTemplate {

src/armTemplates/resources/hostingEnvironment.ts

@@ -0,0 +1,154 @@
+import md5 from "md5";
+import { StorageAccountResource } from "./storageAccount";
+import { ServerlessAzureConfig } from "../../models/serverless";
+import { Utils } from "../../shared/utils";
+
+describe("Storage Account Resource", () => {
+  const config: ServerlessAzureConfig = {
+    functions: [],
+    plugins: [],
+    provider: {
+      prefix: "sls",
+      name: "azure",
+      region: "westus",
+      stage: "dev",
+    },
+    service: "test-api"
+  }
+
+  it("Generates safe storage account name with short parts", () => {
+    const testConfig: ServerlessAzureConfig = {
+      ...config,
+      service: "test-api",
+    };
+
+    const result = StorageAccountResource.getResourceName(testConfig);
+    assertValidStorageAccountName(testConfig, result);
+    expect(result.startsWith("slswusdev")).toBe(true);
+  });
+
+  it("Generates safe storage account names with long parts", () => {
+    const testConfig: ServerlessAzureConfig = {
+      ...config,
+      provider: {
+        ...config.provider,
+        prefix: "my-long-test-prefix-name",
+        region: "Australia Southeast",
+        stage: "development"
+      },
+      service: "my-long-test-api",
+    };
+
+    const result = StorageAccountResource.getResourceName(testConfig);
+    assertValidStorageAccountName(testConfig, result);
+    expect(result.startsWith("mylaussedev")).toBe(true);
+  });
+
+  it("Generating a storage account name is idempotent", () => {
+    const result1 = StorageAccountResource.getResourceName(config);
+    const result2 = StorageAccountResource.getResourceName(config);
+
+    expect(result1).toEqual(result2);
+  });
+
+  it("Generates distinct account names based on region", () => {
+    const regions = [
+      "eastasia",
+      "southeastasia",
+      "centralus",
+      "eastus",
+      "eastus2",
+      "westus",
+      "northcentralus",
+      "southcentralus",
+      "northeurope",
+      "westeurope",
+      "japanwest",
+      "japaneast",
+      "brazilsouth",
+      "australiaeast",
+      "australiasoutheast",
+      "southindia",
+      "centralindia",
+      "westindia",
+      "canadacentral",
+      "canadaeast",
+      "uksouth",
+      "ukwest",
+      "westcentralus",
+      "westus2",
+      "koreacentral",
+      "koreasouth",
+      "francecentral",
+      "francesouth",
+      "australiacentral",
+      "australiacentral2",
+      "uaecentral",
+      "uaenorth",
+      "southafricanorth",
+      "southafricawest"
+    ];
+
+    const regionConfigs = regions.map((region) => {
+      return {
+        ...config,
+        provider: {
+          ...config.provider,
+          region: region,
+        }
+      };
+    });
+
+    const results = {};
+    regionConfigs.forEach((config) => {
+      const result = StorageAccountResource.getResourceName(config);
+      assertValidStorageAccountName(config, result);
+      results[result] = config;
+    });
+
+    expect(Object.keys(results)).toHaveLength(regionConfigs.length);
+  });
+
+  it("Generates distinct account names based on stage", () => {
+    const stages = [
+      "dev",
+      "test",
+      "qa",
+      "uat",
+      "prod",
+      "preprod",
+    ];
+
+    const stageConfigs = stages.map((region) => {
+      return {
+        ...config,
+        provider: {
+          ...config.provider,
+          region: region,
+        }
+      };
+    });
+
+    const results = {};
+    stageConfigs.forEach((config) => {
+      const result = StorageAccountResource.getResourceName(config);
+      assertValidStorageAccountName(config, result);
+      results[result] = config;
+    });
+
+    expect(Object.keys(results)).toHaveLength(stageConfigs.length);
+  });
+
+  function assertValidStorageAccountName(config: ServerlessAzureConfig, value: string) {
+    expect(value.length).toBeLessThanOrEqual(24);
+    expect(value.match(/[a-z0-9]/g).length).toEqual(value.length);
+    expect(value).toContain(Utils.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);
+  };
+});

src/armTemplates/resources/storageAccount.test.ts

@@ -1,6 +1,7 @@
 import { ArmResourceTemplateGenerator, ArmResourceTemplate } from "../../models/armTemplates";
 import { ServerlessAzureConfig, ResourceConfig } from "../../models/serverless";
 import { Utils } from "../../shared/utils";
+import md5 from "md5";
 
 export class StorageAccountResource implements ArmResourceTemplateGenerator {
   public static getResourceName(config: ServerlessAzureConfig) {
@@ -67,19 +68,30 @@ export class StorageAccountResource implements ArmResourceTemplateGenerator {
   /**
    * Gets a default storage account name.
    * Storage account names can have at most 24 characters and can have only alpha-numerics
-   * Default naming convention:
-   * 
-   * "(first 3 of prefix)(first 3 of region)(first 3 of stage)(first 12 of service)sa"
-   * (Maximum of 23 characters)
    * @param config Serverless Azure Config
    */
   private static getDefaultStorageAccountName(config: ServerlessAzureConfig): string {
-    const prefix = Utils.appendSubstrings(
-      3,
-      config.provider.prefix,
-      config.provider.region,
-      config.provider.stage,
-    );
-    return `${prefix}${config.service.substr(0, 12)}sa`.replace("-", "").toLocaleLowerCase();
+    const maxAccountNameLength = 24;
+    const nameHash = md5(config.service);
+    const replacer = /\W+/g;
+
+    let safePrefix = config.provider.prefix.replace(replacer, "");
+    const safeRegion = Utils.createShortAzureRegionName(config.provider.region);
+    let safeStage = Utils.createShortStageName(config.provider.stage);
+    let safeNameHash = nameHash.substr(0, 6);
+
+    const remaining = maxAccountNameLength - (safePrefix.length + safeRegion.length + safeStage.length + safeNameHash.length);
+
+    // Dynamically adjust the substring based on space needed
+    if (remaining < 0) {
+      const partLength = Math.floor(Math.abs(remaining) / 3);
+      safePrefix = safePrefix.substr(0, partLength);
+      safeStage = safeStage.substr(0, partLength);
+      safeNameHash = safeNameHash.substr(0, partLength);
+    }
+
+    return [safePrefix, safeRegion, safeStage, safeNameHash]
+      .join("")
+      .toLocaleLowerCase();
   }
 }