From d1fcfc9cbad12ea1238cf52b4329623f2884a70d Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Thu, 8 Aug 2019 13:03:40 -0700 Subject: [PATCH] fix: Removed service name from storage account name generator (#232) 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. --- package-lock.json | 20 ------- package.json | 1 - src/armTemplates/resources/apim.ts | 6 +- src/armTemplates/resources/appInsights.ts | 6 +- src/armTemplates/resources/appServicePlan.ts | 6 +- src/armTemplates/resources/functionApp.ts | 7 +-- .../resources/hostingEnvironment.ts | 6 +- .../resources/storageAccount.test.ts | 7 +-- src/armTemplates/resources/storageAccount.ts | 6 +- src/services/namingService.test.ts | 55 ++++++++++++++++++- src/services/namingService.ts | 50 +++++++++++------ 11 files changed, 95 insertions(+), 75 deletions(-) diff --git a/package-lock.json b/package-lock.json index e64a9f6d..fecdc6a5 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 6018b19a..8305eba4 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 5003361c..2bd66521 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 9426a0b6..ab72cc94 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 4485cb06..50d99e6d 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 1af37a3e..0263490f 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 7c8332bd..0cf86716 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.test.ts b/src/armTemplates/resources/storageAccount.test.ts index b398cda6..5d20643b 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/armTemplates/resources/storageAccount.ts b/src/armTemplates/resources/storageAccount.ts index 69dc7cd6..86832ae9 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.test.ts b/src/services/namingService.test.ts index 33749c45..d58fbc16 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); + }; }); diff --git a/src/services/namingService.ts b/src/services/namingService.ts index 10c37550..8d7df131 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);