Repository: serverless/serverless-azure-functions
Pull request #210 (2019-07-19 21:29:14)

feat: specify subscription ID in serverless yaml
New PR created because of merge issues due to one unsigned commit. 

#AB360

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

Coverage increased (+0.01%) to 84.315% when pulling **be9559dfba8baf09b083c849926afb439a0361e8 on t-nemand/azureSubscriptionID** into **2b97f75ca9dd5a6b29e823c4aa46e34bf3195d52 on dev**.


Patch:
diff --git a/package-lock.json b/package-lock.json
index 8f27d8f..0fb1b9b 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.0-6",
+  "version": "1.0.0-7",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
diff --git a/package.json b/package.json
index f8d38ce..7f84b17 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.0-6",
+  "version": "1.0.0-7",
   "description": "Provider plugin for the Serverless Framework v1.x which adds support for Azure Functions.",
   "license": "MIT",
   "main": "./lib/index.js",
diff --git a/src/plugins/deploy/azureDeployPlugin.ts b/src/plugins/deploy/azureDeployPlugin.ts
index 70f4822..8e38560 100644
--- a/src/plugins/deploy/azureDeployPlugin.ts
+++ b/src/plugins/deploy/azureDeployPlugin.ts
@@ -3,7 +3,6 @@ import Serverless from "serverless";
 import { FunctionAppService } from "../../services/functionAppService";
 import { AzureLoginOptions } from "../../services/loginService";
 import { ResourceService } from "../../services/resourceService";
-import { Utils } from "../../shared/utils";
 import { AzureBasePlugin } from "../azureBasePlugin";
 
 export class AzureDeployPlugin extends AzureBasePlugin<AzureLoginOptions> {
@@ -49,25 +48,7 @@ export class AzureDeployPlugin extends AzureBasePlugin<AzureLoginOptions> {
   private async list() {
     this.log("Listing deployments");
     const resourceService = new ResourceService(this.serverless, this.options);
-    const deployments = await resourceService.getDeployments();
-    if (!deployments || deployments.length === 0) {
-      this.log(`No deployments found for resource group '${resourceService.getResourceGroupName()}'`);
-      return;
-    }
-    let stringDeployments = "\n\nDeployments";
-
-    for (const dep of deployments) {
-      stringDeployments += "\n-----------\n"
-      stringDeployments += `Name: ${dep.name}\n`
-      const timestampFromName = Utils.getTimestampFromName(dep.name);
-      stringDeployments += `Timestamp: ${(timestampFromName) ? timestampFromName : "None"}\n`;
-
-      const dateTime = timestampFromName ? new Date(+timestampFromName).toISOString() : "None";
-      stringDeployments += `Datetime: ${dateTime}\n`
-    }
-
-    stringDeployments += "-----------\n"
-    this.log(stringDeployments);
+    this.log(await resourceService.listDeployments());
   }
 
   private async deploy() {
diff --git a/src/plugins/login/azureLoginPlugin.ts b/src/plugins/login/azureLoginPlugin.ts
index 77311ae..14e12a1 100644
--- a/src/plugins/login/azureLoginPlugin.ts
+++ b/src/plugins/login/azureLoginPlugin.ts
@@ -1,23 +1,18 @@
 import Serverless from "serverless";
-import AzureProvider from "../../provider/azureProvider";
-import { AzureLoginService } from "../../services/loginService";
+import { AzureLoginOptions, AzureLoginService } from "../../services/loginService";
 import { AzureBasePlugin } from "../azureBasePlugin";
-import { AzureLoginOptions } from "../../services/loginService";
+import { loginHooks } from "./loginHooks";
 
 export class AzureLoginPlugin extends AzureBasePlugin<AzureLoginOptions> {
-  private provider: AzureProvider;
   public hooks: { [eventName: string]: Promise<any> };
 
   public constructor(serverless: Serverless, options: AzureLoginOptions) {
     super(serverless, options);
-    this.provider = (this.serverless.getProvider("azure") as any) as AzureProvider;
 
-    this.hooks = {
-      "before:deploy:deploy": this.login.bind(this),
-      "before:deploy:list:list": this.login.bind(this),
-      "before:invoke:invoke": this.login.bind(this),
-      "before:rollback:rollback": this.login.bind(this),
-    };
+    this.hooks = {};
+    for (const h of loginHooks) {
+      this.hooks[`before:${h}`] = this.login.bind(this);
+    }
   }
 
   private async login() {
@@ -33,7 +28,7 @@ export class AzureLoginPlugin extends AzureBasePlugin<AzureLoginOptions> {
       this.serverless.variables["azureCredentials"] = authResult.credentials;
       // Use environment variable for sub ID or use the first subscription in the list (service principal can
       // have access to more than one subscription)
-      this.serverless.variables["subscriptionId"] = this.options.subscriptionId || process.env.azureSubId || authResult.subscriptions[0].id;
+      this.serverless.variables["subscriptionId"] = this.options.subscriptionId || process.env.azureSubId || this.serverless.service.provider["subscriptionId"] || authResult.subscriptions[0].id;
       this.serverless.cli.log(`Using subscription ID: ${this.serverless.variables["subscriptionId"]}`);
     }
     catch (e) {
diff --git a/src/plugins/login/loginHooks.ts b/src/plugins/login/loginHooks.ts
new file mode 100644
index 0000000..4840de9
--- /dev/null
+++ b/src/plugins/login/loginHooks.ts
@@ -0,0 +1,11 @@
+/**
+ * Hooks that require authentication before execution
+ */
+export const loginHooks = [
+  "package:initialize",
+  "deploy:list:list",
+  "deploy:deploy",
+  "invoke:invoke",
+  "rollback:rollback",
+  "remove:remove",
+]
diff --git a/src/services/apimService.ts b/src/services/apimService.ts
index 6126a70..9257b13 100644
--- a/src/services/apimService.ts
+++ b/src/services/apimService.ts
@@ -10,6 +10,7 @@ import {
 } from "@azure/arm-apimanagement/esm/models";
 import { Site } from "@azure/arm-appservice/esm/models";
 import { Guard } from "../shared/guard";
+import { ApimResource } from "../armTemplates/resources/apim";
 
 /**
  * APIM Service handles deployment and integration with Azure API Management
@@ -28,7 +29,7 @@ export class ApimService extends BaseService {
     }
 
     if (!this.apimConfig.name) {
-      this.apimConfig.name = `${this.config.provider.prefix}-${this.config.provider.region}-${this.config.provider.stage}-apim`
+      this.apimConfig.name = ApimResource.getResourceName(this.config);
     }
 
     if (!this.apimConfig.backend) {
@@ -325,4 +326,4 @@ export class ApimService extends BaseService {
 
     return xml(policy, { indent: "\t" });
   }
-}
\ No newline at end of file
+}
diff --git a/src/services/resourceService.ts b/src/services/resourceService.ts
index 4b7dca0..2d75181 100644
--- a/src/services/resourceService.ts
+++ b/src/services/resourceService.ts
@@ -20,6 +20,31 @@ export class ResourceService extends BaseService {
     return await this.resourceClient.deployments.listByResourceGroup(this.resourceGroup);
   }
 
+  /**
+   * Returns stringified list of deployments with timestamps
+   */
+  public async listDeployments(): Promise<string> {
+    const deployments = await this.getDeployments()
+    if (!deployments || deployments.length === 0) {
+      this.log(`No deployments found for resource group '${this.getResourceGroupName()}'`);
+      return;
+    }
+    let stringDeployments = "\n\nDeployments";
+
+    for (const dep of deployments) {
+      stringDeployments += "\n-----------\n"
+      stringDeployments += `Name: ${dep.name}\n`
+      const timestampFromName = Utils.getTimestampFromName(dep.name);
+      stringDeployments += `Timestamp: ${(timestampFromName) ? timestampFromName : "None"}\n`;
+
+      const dateTime = timestampFromName ? new Date(+timestampFromName).toISOString() : "None";
+      stringDeployments += `Datetime: ${dateTime}\n`
+    }
+
+    stringDeployments += "-----------\n"
+    return stringDeployments
+  }
+
   /**
    * Get ARM template for previous deployment
    * @param deploymentName Name of deployment
@@ -45,4 +70,4 @@ export class ResourceService extends BaseService {
     this.log(`Deleting resource group: ${this.resourceGroup}`);
     return await this.resourceClient.resourceGroups.deleteMethod(this.resourceGroup);
   }
-}
\ No newline at end of file
+}
diff --git a/src/services/rollbackService.ts b/src/services/rollbackService.ts
index 0adb0bd..a857693 100644
--- a/src/services/rollbackService.ts
+++ b/src/services/rollbackService.ts
@@ -102,12 +102,14 @@ export class RollbackService extends BaseService {
 
   /**
    * Get deployment specified by timestamp in Serverless options
+   * Lists deployments if no timestamp is provided
    */
   private async getDeployment(): Promise<DeploymentExtended> {
     let timestamp = Utils.get(this.options, "timestamp");
     if (!timestamp) {
-      this.log("Need to specify a timestamp for rollback. Run `sls deploy list` to see timestamps of deployments");
+      this.log("Need to specify a timestamp for rollback.");
       this.log("Example usage:\n\nsls rollback -t 1562014362");
+      this.log(await this.resourceService.listDeployments());
       return null;
     }
     const deployments = await this.getArmDeploymentsByTimestamp();


Test patch:
diff --git a/src/plugins/deploy/azureDeployPlugin.test.ts b/src/plugins/deploy/azureDeployPlugin.test.ts
index 52b98fb..7b1f3e0 100644
--- a/src/plugins/deploy/azureDeployPlugin.test.ts
+++ b/src/plugins/deploy/azureDeployPlugin.test.ts
@@ -57,75 +57,11 @@ describe("Deploy plugin", () => {
     expect(uploadFunctions).toBeCalledWith(functionAppStub);
   });
 
-  it("does not call deploy if zip does not exist", async () => {
-    const deployResourceGroup = jest.fn();
-    const functionAppStub: Site = MockFactory.createTestSite();
-    const deploy = jest.fn(() => Promise.resolve(functionAppStub));
-    const uploadFunctions = jest.fn();
-
-    const zipFile = "fake.zip";
-
-    FunctionAppService.prototype.getFunctionZipFile = (() => zipFile);
-    ResourceService.prototype.deployResourceGroup = deployResourceGroup;
-    FunctionAppService.prototype.deploy = deploy;
-    FunctionAppService.prototype.uploadFunctions = uploadFunctions;
-
-    await invokeHook(plugin, "deploy:deploy");
-
-    expect(deployResourceGroup).not.toBeCalled();
-    expect(deploy).not.toBeCalled();
-    expect(uploadFunctions).not.toBeCalled();
-    expect(sls.cli.log).lastCalledWith(`Function app zip file '${zipFile}' does not exist`);
-  });
-
-  it("lists deployments with timestamps", async () => {
-    const deployments = MockFactory.createTestDeployments(5, true);
-    ResourceService.prototype.getDeployments = jest.fn(() => Promise.resolve(deployments));
-
-    await invokeHook(plugin, "deploy:list:list");
-    expect(ResourceService.prototype.getDeployments).toBeCalled();
-
-    let expectedLogStatement = "\n\nDeployments";
-    const originalTimestamp = +MockFactory.createTestTimestamp();
-    let i = 0
-    for (const dep of deployments) {
-      const timestamp = originalTimestamp + i
-      expectedLogStatement += "\n-----------\n"
-      expectedLogStatement += `Name: ${dep.name}\n`
-      expectedLogStatement += `Timestamp: ${timestamp}\n`;
-      expectedLogStatement += `Datetime: ${new Date(timestamp).toISOString()}\n`
-      i++
-    }
-    expectedLogStatement += "-----------\n"
-    expect(sls.cli.log).lastCalledWith(expectedLogStatement);
-  });
-
-  it("lists deployments without timestamps", async () => {
-    const deployments = MockFactory.createTestDeployments();
-    ResourceService.prototype.getDeployments = jest.fn(() => Promise.resolve(deployments));
-
+  it("lists deployments", async () => {
+    const deploymentString = "deployments";
+    ResourceService.prototype.listDeployments = jest.fn(() => Promise.resolve(deploymentString));
     await invokeHook(plugin, "deploy:list:list");
-    expect(ResourceService.prototype.getDeployments).toBeCalled();
-
-    let expectedLogStatement = "\n\nDeployments";
-    for (const dep of deployments) {
-      expectedLogStatement += "\n-----------\n"
-      expectedLogStatement += `Name: ${dep.name}\n`
-      expectedLogStatement += "Timestamp: None\n";
-      expectedLogStatement += "Datetime: None\n"
-    }
-    expectedLogStatement += "-----------\n"
-    expect(sls.cli.log).lastCalledWith(expectedLogStatement);
-  });
-
-  it("logs empty deployment list", async () => {
-    const resourceGroup = "rg1";
-    ResourceService.prototype.getDeployments = jest.fn(() => Promise.resolve([])) as any;
-    ResourceService.prototype.getResourceGroupName = jest.fn(() => resourceGroup);
-
-    await invokeHook(plugin, "deploy:list:list");
-    expect(ResourceService.prototype.getDeployments).toBeCalled();
-
-    expect(sls.cli.log).lastCalledWith(`No deployments found for resource group '${resourceGroup}'`);
+    expect(ResourceService.prototype.listDeployments).toBeCalled();
+    expect(sls.cli.log).lastCalledWith(deploymentString);
   });
 });
diff --git a/src/plugins/login/azureLoginPlugin.test.ts b/src/plugins/login/azureLoginPlugin.test.ts
index 1838318..9e71595 100644
--- a/src/plugins/login/azureLoginPlugin.test.ts
+++ b/src/plugins/login/azureLoginPlugin.test.ts
@@ -3,6 +3,7 @@ import { AzureLoginService } from "../../services/loginService";
 import { MockFactory } from "../../test/mockFactory";
 import { invokeHook, setEnvVariables, unsetEnvVariables } from "../../test/utils";
 import { AzureLoginPlugin } from "./azureLoginPlugin";
+import { loginHooks } from "./loginHooks";
 
 describe("Login Plugin", () => {
 
@@ -32,7 +33,7 @@ describe("Login Plugin", () => {
 
   async function invokeLoginHook(hasCreds = false, serverless?: Serverless, options?: Serverless.Options) {
     const plugin = createPlugin(hasCreds, serverless, options);
-    await invokeHook(plugin, "before:deploy:deploy");
+    await invokeHook(plugin, `before:${loginHooks[0]}`);
   }
 
   beforeEach(() => {
@@ -40,6 +41,10 @@ describe("Login Plugin", () => {
     AzureLoginService.servicePrincipalLogin = createMockLoginFunction();
   });
 
+  it("contains the hooks as contained in loginHooks", () => {
+    expect(Object.keys(createPlugin().hooks)).toEqual(loginHooks.map((hook) => `before:${hook}`));
+  });
+
   it("returns if azure credentials are set", async () => {
     await invokeLoginHook(true);
     expect(AzureLoginService.interactiveLogin).not.toBeCalled();
@@ -100,7 +105,7 @@ describe("Login Plugin", () => {
     expect(AzureLoginService.interactiveLogin).toBeCalled()
     expect(sls.variables["subscriptionId"]).toEqual("test-subs-id");
     expect(sls.cli.log).toBeCalledWith("Using subscription ID: test-subs-id");
-  })
+  });
 
   it("Uses the default subscription ID" , async () => {
     const sls = MockFactory.createTestServerless();
@@ -110,4 +115,14 @@ describe("Login Plugin", () => {
     expect(sls.variables["subscriptionId"]).toEqual("azureSubId");
     expect(sls.cli.log).toBeCalledWith("Using subscription ID: azureSubId");
   });
+  
+  it("Uses the subscription ID specified in serverless yaml", async () => {
+    const sls = MockFactory.createTestServerless();
+    const opt = MockFactory.createTestServerlessOptions();
+    sls.service.provider["subscriptionId"] = "test-subs-id";
+    await invokeLoginHook(false, sls, opt);
+    expect(AzureLoginService.interactiveLogin).toBeCalled()
+    expect(sls.variables["subscriptionId"]).toEqual("test-subs-id");
+    expect(sls.cli.log).toBeCalledWith("Using subscription ID: test-subs-id");
+  });
 });
diff --git a/src/services/apimService.test.ts b/src/services/apimService.test.ts
index 028b8ec..50131f2 100644
--- a/src/services/apimService.test.ts
+++ b/src/services/apimService.test.ts
@@ -19,6 +19,7 @@ import {
   OperationContract,
   ApiPolicyCreateOrUpdateResponse,
 } from "@azure/arm-apimanagement/esm/models";
+import { Utils } from "../shared/utils";
 
 describe("APIM Service", () => {
   const apimConfig = MockFactory.createTestApimConfig();
@@ -46,7 +47,6 @@ describe("APIM Service", () => {
       subscriptionId: "ABC123",
     };
   });
-
   it("is defined", () => {
     expect(ApimService).toBeDefined();
   });
@@ -56,6 +56,16 @@ describe("APIM Service", () => {
     expect(service).not.toBeNull();
   });
 
+  it("when generated, has a shorted region name in the resource name", () => {
+    // if APIM name is ommited, it should auto-generate one that contains a shortened region name
+    const apimConfigName = MockFactory.createTestApimConfig(true);
+    (serverless.service.provider as any).apim = apimConfigName;
+
+    const service = new ApimService(serverless);
+    const expectedRegionName = Utils.createShortAzureRegionName(service.getRegion());
+    expect(apimConfigName.name.includes(expectedRegionName)).toBeTruthy();
+  });
+
   describe("Get service reference", () => {
     it("returns null when service doesn't exist", async () => {
       axios.request = jest.fn((requestConfig) => MockFactory.createTestAxiosResponse(requestConfig, apimGetService404, 404));
diff --git a/src/services/resourceService.test.ts b/src/services/resourceService.test.ts
index bd22314..c78df52 100644
--- a/src/services/resourceService.test.ts
+++ b/src/services/resourceService.test.ts
@@ -1,16 +1,17 @@
+import { DeploymentsListByResourceGroupResponse } from "@azure/arm-resources/esm/models";
+import { Utils } from "../shared/utils";
 import { MockFactory } from "../test/mockFactory";
 import { ResourceService } from "./resourceService";
 
-
 jest.mock("@azure/arm-resources")
 import { ResourceManagementClient } from "@azure/arm-resources";
-import { Utils } from "../shared/utils";
 
 describe("Resource Service", () => {
-  const deployments = MockFactory.createTestDeployments();
+  let deployments: DeploymentsListByResourceGroupResponse;
   const template = "myTemplate";
 
-  beforeAll(() => {
+  beforeEach(() => {
+    deployments = MockFactory.createTestDeployments(5, true);
     ResourceManagementClient.prototype.resourceGroups = {
       createOrUpdate: jest.fn(),
       deleteMethod: jest.fn(),
@@ -80,7 +81,7 @@ describe("Resource Service", () => {
       .toBeCalledWith(resourceGroup);
   });
 
-  it("lists deployments", async () => {
+  it("gets deployments", async () => {
     const sls = MockFactory.createTestServerless();
     const resourceGroup = "myResourceGroup";
     sls.service.provider["resourceGroup"] = resourceGroup
@@ -91,6 +92,53 @@ describe("Resource Service", () => {
     expect(deps).toEqual(deployments);
   });
 
+  it("lists deployments as string with timestamps", async () => {
+    const sls = MockFactory.createTestServerless();
+    const resourceGroup = "myResourceGroup";
+    sls.service.provider["resourceGroup"] = resourceGroup
+    sls.variables["azureCredentials"] = "fake credentials"
+    const options = MockFactory.createTestServerlessOptions();
+    const service = new ResourceService(sls, options);
+    const deploymentString = await service.listDeployments();
+    let expectedDeploymentString = "\n\nDeployments";
+    const originalTimestamp = +MockFactory.createTestTimestamp();
+    let i = 0
+    for (const dep of deployments) {
+      const timestamp = originalTimestamp + i
+      expectedDeploymentString += "\n-----------\n"
+      expectedDeploymentString += `Name: ${dep.name}\n`
+      expectedDeploymentString += `Timestamp: ${timestamp}\n`;
+      expectedDeploymentString += `Datetime: ${new Date(timestamp).toISOString()}\n`
+      i++
+    }
+    expectedDeploymentString += "-----------\n"
+    expect(deploymentString).toEqual(expectedDeploymentString);
+  });
+
+  it("lists deployments as string without timestamps", async () => {
+    deployments = MockFactory.createTestDeployments();
+    ResourceManagementClient.prototype.deployments = {
+      listByResourceGroup: jest.fn(() => Promise.resolve(deployments)),
+    } as any;
+
+    const sls = MockFactory.createTestServerless();
+    const resourceGroup = "myResourceGroup";
+    sls.service.provider["resourceGroup"] = resourceGroup
+    sls.variables["azureCredentials"] = "fake credentials"
+    const options = MockFactory.createTestServerlessOptions();
+    const service = new ResourceService(sls, options);
+    const deploymentString = await service.listDeployments();
+    let expectedDeploymentString = "\n\nDeployments";
+    for (const dep of deployments) {
+      expectedDeploymentString += "\n-----------\n"
+      expectedDeploymentString += `Name: ${dep.name}\n`
+      expectedDeploymentString += "Timestamp: None\n";
+      expectedDeploymentString += "Datetime: None\n"
+    }
+    expectedDeploymentString += "-----------\n"
+    expect(deploymentString).toEqual(expectedDeploymentString);
+  });
+
   it("gets deployment template",async () => {
     const sls = MockFactory.createTestServerless();
     const resourceGroup = "myResourceGroup";
@@ -107,4 +155,4 @@ describe("Resource Service", () => {
       );
     expect(result).toEqual(template);
   });
-});
\ No newline at end of file
+});
diff --git a/src/services/rollbackService.test.ts b/src/services/rollbackService.test.ts
index 2b0e51d..547bbe2 100644
--- a/src/services/rollbackService.test.ts
+++ b/src/services/rollbackService.test.ts
@@ -31,6 +31,7 @@ describe("Rollback Service", () => {
   const artifactName = MockFactory.createTestDeployment().name.replace("deployment", "artifact") + ".zip";
   const artifactPath = `.serverless${path.sep}${artifactName}`
   const armDeployment: ArmDeployment = { template, parameters };
+  const deploymentString = "deployments";
 
   function createOptions(timestamp?: string): Serverless.Options {
     return {
@@ -58,6 +59,7 @@ describe("Rollback Service", () => {
     ResourceService.prototype.getDeploymentTemplate = jest.fn(
       () => Promise.resolve({ template })
     ) as any;
+    ResourceService.prototype.listDeployments = jest.fn(() => Promise.resolve(deploymentString))
     AzureBlobStorageService.prototype.generateBlobSasTokenUrl = jest.fn(() => sasURL) as any;
     FunctionAppService.prototype.get = jest.fn(() => appStub) as any;
   });
@@ -72,8 +74,7 @@ describe("Rollback Service", () => {
     const options = {} as any;
     const service = createService(sls, options);
     await service.rollback();
-    const calls = (sls.cli.log as any).mock.calls;
-    expect(calls[0][0]).toEqual("Need to specify a timestamp for rollback. Run `sls deploy list` to see timestamps of deployments");
+    expect(sls.cli.log).lastCalledWith(deploymentString);
   });
 
   it("should return early with invalid timestamp", async () => {
diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts
index e453e20..2d48e2f 100644
--- a/src/test/mockFactory.ts
+++ b/src/test/mockFactory.ts
@@ -267,9 +267,9 @@ export class MockFactory {
     return (asYaml) ? yaml.dump(data) : data;
   }
 
-  public static createTestApimConfig(): ApiManagementConfig {
+  public static createTestApimConfig(generateName: boolean = false): ApiManagementConfig {
     return {
-      name: "test-apim-resource",
+      name: generateName ? null : "test-apim-resource",
       api: {
         name: "test-apim-api1",
         subscriptionRequired: false,