Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

Commit

Permalink
fix: Append 6-char resource group hash to storage account name (#256)
Browse files Browse the repository at this point in the history
Because storage account names are global, the need to be unique across resource groups. This adds the first 6 characters of the `md5` resource group hash to the end of the storage account name.

Resolves AB#846
  • Loading branch information
tbarlow12 authored Aug 19, 2019
1 parent 660f8b7 commit 44185c5
Show file tree
Hide file tree
Showing 8 changed files with 88 additions and 18 deletions.
48 changes: 41 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,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",
Expand Down
16 changes: 13 additions & 3 deletions src/armTemplates/resources/storageAccount.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import { ServerlessAzureConfig } from "../../models/serverless";
import { AzureNamingService } from "../../services/namingService";
import { StorageAccountResource } from "./storageAccount";
import md5 from "md5";

describe("Storage Account Resource", () => {
const resourceGroup = "myResourceGroup";
const resourceGroupHash = md5(resourceGroup).substr(0, 6);

const config: ServerlessAzureConfig = {
functions: [],
plugins: [],
Expand All @@ -11,19 +15,24 @@ describe("Storage Account Resource", () => {
name: "azure",
region: "westus",
stage: "dev",
resourceGroup
},
service: "test-api"
}

it("Generates safe storage account name with short parts", () => {
const testConfig: ServerlessAzureConfig = {
...config,
provider: {
...config.provider,
resourceGroup
},
service: "test-api",
};

const result = StorageAccountResource.getResourceName(testConfig);
assertValidStorageAccountName(testConfig, result);
expect(result.startsWith("slswusdev")).toBe(true);
expect(result).toEqual(`slswusdev${resourceGroupHash}`);
});

it("Generates safe storage account names with long parts", () => {
Expand All @@ -33,13 +42,14 @@ describe("Storage Account Resource", () => {
...config.provider,
prefix: "my-long-test-prefix-name",
region: "Australia Southeast",
stage: "development"
stage: "development",
resourceGroup
}
};

const result = StorageAccountResource.getResourceName(testConfig);
assertValidStorageAccountName(testConfig, result);
expect(result.startsWith("mylaussedev")).toBe(true);
expect(result).toEqual(`mylaussedev${resourceGroupHash}`)
});

it("Generating a storage account name is idempotent", () => {
Expand Down
8 changes: 7 additions & 1 deletion src/armTemplates/resources/storageAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ 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,
"",
true
);
}

public getTemplate(): ArmResourceTemplate {
Expand Down
3 changes: 2 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export const configConstants = {
scmCommandApiPath: "/api/command",
scmDomain: ".scm.azurewebsites.net",
scmVfsPath: "/api/vfs/site/wwwroot/",
scmZipDeployApiPath: "/api/zipdeploy"
scmZipDeployApiPath: "/api/zipdeploy",
resourceGroupHashLength: 6,
};

export default configConstants;
1 change: 1 addition & 0 deletions src/services/baseService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export abstract class BaseService {
this.credentials = serverless.variables["azureCredentials"];
this.subscriptionId = serverless.variables["subscriptionId"];
this.resourceGroup = this.getResourceGroupName();
this.config.provider.resourceGroup = this.resourceGroup;
this.deploymentConfig = this.getDeploymentConfig();
this.deploymentName = this.getDeploymentName();
this.artifactName = this.getArtifactName(this.deploymentName);
Expand Down
5 changes: 5 additions & 0 deletions src/services/namingService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import { AzureNamingService } from "./namingService"
import { ServerlessAzureConfig } from "../models/serverless";

describe("Naming Service", () => {

const resourceGroup = "myResourceGroup";

it("Creates a short name for an azure region", () => {
const expected = "ausse";
const actual = AzureNamingService.createShortAzureRegionName("australiasoutheast");
Expand Down Expand Up @@ -96,6 +99,7 @@ describe("Naming Service", () => {
name: "azure",
region: "westus",
stage: "dev",
resourceGroup,
},
service: "test-api"
};
Expand All @@ -116,6 +120,7 @@ describe("Naming Service", () => {
name: "azure",
region: "westus",
stage: "multicloud",
resourceGroup,
},
service: "extra-long-service-name"
};
Expand Down
24 changes: 18 additions & 6 deletions src/services/namingService.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ServerlessAzureConfig, ResourceConfig } from "../models/serverless"
import { Guard } from "../shared/guard"
import configConstants from "../config";
import md5 from "md5";

export class AzureNamingService {

Expand Down Expand Up @@ -45,25 +46,28 @@ export class AzureNamingService {
* @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, suffix: string = "", forbidden: RegExp = /\W+/g, replaceWith: string = "") {
public static getSafeResourceName(config: ServerlessAzureConfig, maxLength: number, resourceConfig?: ResourceConfig, suffix: string = "", includeHash = false) {
const nonAlphaNumeric = /\W+/g;

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);
return name.replace(nonAlphaNumeric, "");
}

const { prefix, region, stage } = config.provider;

let safePrefix = prefix.replace(forbidden, replaceWith);
let safePrefix = prefix.replace(nonAlphaNumeric, "");
const safeRegion = this.createShortAzureRegionName(region);
let safeStage = this.createShortStageName(stage);
let safeSuffix = suffix.replace(forbidden, replaceWith);
let safeSuffix = suffix.replace(nonAlphaNumeric, "");

const remaining = maxLength - (safePrefix.length + safeRegion.length + safeStage.length + safeSuffix.length);
const hashLength = (includeHash) ? configConstants.resourceGroupHashLength : 0;
const remaining = maxLength - (safePrefix.length + safeRegion.length + safeStage.length + safeSuffix.length + hashLength);

// Dynamically adjust the substring based on space needed
if (remaining < 0) {
Expand All @@ -77,9 +81,17 @@ export class AzureNamingService {
safeSuffix = safeSuffix.substr(0, partLength);
}

return [safePrefix, safeRegion, safeStage, safeSuffix]
const safeHash = md5(config.provider.resourceGroup).substr(0, hashLength);

const name = [safePrefix, safeRegion, safeStage, safeHash, safeSuffix]
.join("")
.toLowerCase();

if (name.length > maxLength) {
throw new Error(`Name ${name} is too long. Should be shorter than ${maxLength} characters`)
}

return name;
}

/**
Expand Down

0 comments on commit 44185c5

Please sign in to comment.