Repository: serverless/serverless-azure-functions
Pull request #308 (2019-09-05 02:45:24)

fix: Updates default http output binding
This updates the default value for the name of the HTTP output binding from `$return` to `$res` to work around an issue that existing within the Azure core tools http runtime..

A bug exists in the Azure core tools http runtime that evaluates `$return` binding incorrectly which causes serialization issues when returning arrays within an HTTP response.
https://github.com/Azure/azure-functions-nodejs-worker/issues/228


Patch:
diff --git a/package-lock.json b/package-lock.json
index c7ed0d6..efa405c 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.0-35",
+  "version": "1.0.0-36",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {
diff --git a/package.json b/package.json
index 7188059..9d6c4d0 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-azure-functions",
-  "version": "1.0.0-35",
+  "version": "1.0.0-36",
   "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/func/azureFuncPlugin.ts b/src/plugins/func/azureFuncPlugin.ts
index 3a7c48c..7c3d79a 100644
--- a/src/plugins/func/azureFuncPlugin.ts
+++ b/src/plugins/func/azureFuncPlugin.ts
@@ -3,7 +3,6 @@ import { FuncService } from "../../services/funcService";
 import { AzureBasePlugin } from "../azureBasePlugin";
 
 export class AzureFuncPlugin extends AzureBasePlugin {
-  private service: FuncService;
 
   public constructor(serverless: Serverless, options: Serverless.Options) {
     super(serverless, options);
@@ -48,8 +47,6 @@ export class AzureFuncPlugin extends AzureBasePlugin {
         }
       }
     }
-
-    this.service = new FuncService(serverless, options);
   }
 
   private async func() {
@@ -57,10 +54,12 @@ export class AzureFuncPlugin extends AzureBasePlugin {
   }
 
   private async add() {
-    this.service.add();
+    const service = new FuncService(this.serverless, this.options);
+    service.add();
   }
 
   private async remove() {
-    this.service.remove();
+    const service = new FuncService(this.serverless, this.options);
+    service.remove();
   }
 }
diff --git a/src/plugins/invoke/azureInvokePlugin.ts b/src/plugins/invoke/azureInvokePlugin.ts
index 17a0f9b..ae2f117 100644
--- a/src/plugins/invoke/azureInvokePlugin.ts
+++ b/src/plugins/invoke/azureInvokePlugin.ts
@@ -6,8 +6,6 @@ import { AzureBasePlugin } from "../azureBasePlugin";
 
 export class AzureInvokePlugin extends AzureBasePlugin {
 
-  private invokeService: InvokeService;
-
   public constructor(serverless: Serverless, options: Serverless.Options) {
     super(serverless, options);
     const path = this.options["path"];
@@ -117,8 +115,8 @@ export class AzureInvokePlugin extends AzureBasePlugin {
       return;
     }
 
-    this.invokeService = new InvokeService(this.serverless, this.options, local);
-    const response = await this.invokeService.invoke(method, functionName, data);
+    const invokeService = new InvokeService(this.serverless, this.options, local);
+    const response = await invokeService.invoke(method, functionName, data);
     if (response) {
       this.log(JSON.stringify(response.data));
     }
diff --git a/src/plugins/offline/azureOfflinePlugin.ts b/src/plugins/offline/azureOfflinePlugin.ts
index 699419b..129d2ee 100644
--- a/src/plugins/offline/azureOfflinePlugin.ts
+++ b/src/plugins/offline/azureOfflinePlugin.ts
@@ -3,11 +3,9 @@ import { OfflineService } from "../../services/offlineService";
 import { AzureBasePlugin } from "../azureBasePlugin";
 
 export class AzureOfflinePlugin extends AzureBasePlugin {
-  private offlineService: OfflineService;
 
   public constructor(serverless: Serverless, options: Serverless.Options) {
     super(serverless, options);
-    this.offlineService = new OfflineService(this.serverless, this.options);
 
     this.hooks = {
       "before:offline:offline": this.azureOfflineBuild.bind(this),
@@ -60,14 +58,17 @@ export class AzureOfflinePlugin extends AzureBasePlugin {
   }
 
   private async azureOfflineBuild(){
-    await this.offlineService.build();
+    const offlineService = new OfflineService(this.serverless, this.options);
+    await offlineService.build();
   }
 
   private async azureOfflineStart(){
-    await this.offlineService.start();
+    const offlineService = new OfflineService(this.serverless, this.options);
+    await offlineService.start();
   }
 
   private async azureOfflineCleanup(){
-    await this.offlineService.cleanup();
+    const offlineService = new OfflineService(this.serverless, this.options);
+    await offlineService.cleanup();
   }
 }
diff --git a/src/plugins/package/azurePackagePlugin.ts b/src/plugins/package/azurePackagePlugin.ts
index 2c30567..91c7de6 100644
--- a/src/plugins/package/azurePackagePlugin.ts
+++ b/src/plugins/package/azurePackagePlugin.ts
@@ -7,7 +7,6 @@ import { ServerlessCliCommand } from "../../models/serverless";
 
 export class AzurePackagePlugin extends AzureBasePlugin {
   private bindingsCreated: boolean = false;
-  private packageService: PackageService;
   public provider: AzureProvider;
 
   public constructor(serverless: Serverless, options: Serverless.Options) {
@@ -17,11 +16,11 @@ export class AzurePackagePlugin extends AzureBasePlugin {
       "before:webpack:package:packageModules": this.webpack.bind(this),
       "after:package:finalize": this.finalize.bind(this),
     };
-
-    this.packageService = new PackageService(this.serverless, this.options);
   }
 
   private async setupProviderConfiguration(): Promise<void> {
+    const packageService = new PackageService(this.serverless, this.options);
+
     if (this.processedCommands[0] === ServerlessCliCommand.DEPLOY && this.getOption("package")) {
       this.log("Deploying pre-built package. No need to create bindings");
       return;
@@ -30,14 +29,16 @@ export class AzurePackagePlugin extends AzureBasePlugin {
       throw new Error("Cannot package Azure Functions individually. " +
         "Remove `individually` attribute from the `package` section of the serverless config");
     }
-    this.packageService.cleanUpServerlessDir();
-    await this.packageService.createBindings();
+    packageService.cleanUpServerlessDir();
+    await packageService.createBindings();
     this.bindingsCreated = true;
 
     return Promise.resolve();
   }
 
   private async webpack(): Promise<void> {
+    const packageService = new PackageService(this.serverless, this.options);
+
     if (this.getOption("package")) {
       this.log("No need to perform webpack. Using pre-existing package");
       return Promise.resolve();
@@ -46,17 +47,19 @@ export class AzurePackagePlugin extends AzureBasePlugin {
       await this.setupProviderConfiguration();
     }
 
-    await this.packageService.prepareWebpack();
+    await packageService.prepareWebpack();
   }
 
   /**
    * Cleans up generated folders & files after packaging is complete
    */
   private async finalize(): Promise<void> {
+    const packageService = new PackageService(this.serverless, this.options);
+
     if (this.getOption("package")) {
       this.log("No need to clean up generated folders & files. Using pre-existing package");
       return Promise.resolve();
     }
-    await this.packageService.cleanUp();
+    await packageService.cleanUp();
   }
 }
diff --git a/src/services/azureKeyVaultService.ts b/src/services/azureKeyVaultService.ts
index a8056f5..069ed19 100644
--- a/src/services/azureKeyVaultService.ts
+++ b/src/services/azureKeyVaultService.ts
@@ -2,7 +2,7 @@ import Serverless from "serverless";
 import { BaseService } from "./baseService";
 import { FunctionAppService } from "./functionAppService";
 import { KeyVaultManagementClient } from "@azure/arm-keyvault";
-import { KeyPermissions, SecretPermissions, Vault } from "@azure/arm-keyvault/esm/models/index";
+import { Vault, SecretPermissions } from "@azure/arm-keyvault/esm/models";
 
 /**
  * Defines the Azure Key Vault configuration
diff --git a/src/services/baseService.ts b/src/services/baseService.ts
index 1ff9f46..e1a513a 100644
--- a/src/services/baseService.ts
+++ b/src/services/baseService.ts
@@ -36,7 +36,7 @@ export abstract class BaseService {
     Guard.null(serverless);
     this.setDefaultValues();
     this.config = serverless.service as any;
-    this.setConfigFromCli();
+    this.setupConfig();
 
     this.baseUrl = "https://management.azure.com";
     this.serviceName = this.getServiceName();
@@ -256,8 +256,9 @@ export abstract class BaseService {
    * Overwrite values for resourceGroup, prefix, region and stage
    * in config if passed through CLI
    */
-  private setConfigFromCli() {
+  private setupConfig() {
     const { prefix, region, stage, subscriptionId } = this.config.provider;
+
     this.config.provider = {
       ...this.config.provider,
       prefix: this.getOption("prefix") || prefix,
diff --git a/src/shared/bindings.ts b/src/shared/bindings.ts
index 7022b72..d9c9d22 100644
--- a/src/shared/bindings.ts
+++ b/src/shared/bindings.ts
@@ -57,15 +57,12 @@ export class BindingUtils {
     return bindingUserSettingsMetaData;
   }
 
-  public static getHttpOutBinding(bindingUserSettings) {
+  public static getHttpOutBinding() {
     const binding = {};
 
     binding[constants.type] = "http";
     binding[constants.direction] = constants.outDirection;
-    binding[constants.name] = "$return";
-    if (bindingUserSettings[constants.webHookType]) {
-      binding[constants.name] = "res";
-    }
+    binding[constants.name] = "res";
 
     return binding;
   }
diff --git a/src/shared/utils.ts b/src/shared/utils.ts
index 3d091fc..0a26b5a 100644
--- a/src/shared/utils.ts
+++ b/src/shared/utils.ts
@@ -78,7 +78,7 @@ export class Utils {
     }
 
     if (bindingType === constants.httpTrigger) {
-      bindings.push(BindingUtils.getHttpOutBinding(bindingUserSettings));
+      bindings.push(BindingUtils.getHttpOutBinding());
     }
 
     functionsJson.bindings = bindings;


Test patch:
diff --git a/src/index.test.ts b/src/index.test.ts
index 1eb39ab..76e999c 100644
--- a/src/index.test.ts
+++ b/src/index.test.ts
@@ -8,6 +8,13 @@ import { AzureLoginPlugin } from "./plugins/login/azureLoginPlugin";
 import { AzureApimServicePlugin } from "./plugins/apim/azureApimServicePlugin";
 import { AzureApimFunctionPlugin } from "./plugins/apim/azureApimFunctionPlugin";
 import AzureProvider from "./provider/azureProvider";
+import { AzureFuncPlugin } from "./plugins/func/azureFuncPlugin";
+import { AzureOfflinePlugin } from "./plugins/offline/azureOfflinePlugin";
+import { AzureRollbackPlugin } from "./plugins/rollback/azureRollbackPlugin";
+
+jest.genMockFromModule("./services/baseService");
+jest.mock("./services/baseService")
+import { BaseService } from "./services/baseService";
 
 describe("Azure Index", () => {
   it("contains all registered plugins", () => {
@@ -25,5 +32,23 @@ describe("Azure Index", () => {
     expect(sls.pluginManager.addPlugin).toBeCalledWith(AzureDeployPlugin);
     expect(sls.pluginManager.addPlugin).toBeCalledWith(AzureApimServicePlugin);
     expect(sls.pluginManager.addPlugin).toBeCalledWith(AzureApimFunctionPlugin);
+    expect(sls.pluginManager.addPlugin).toBeCalledWith(AzureFuncPlugin);
+    expect(sls.pluginManager.addPlugin).toBeCalledWith(AzureOfflinePlugin);
+    expect(sls.pluginManager.addPlugin).toBeCalledWith(AzureRollbackPlugin);
+  });
+
+  it("does not initialize BaseService in constructor of any plugin", () => {
+    const sls = MockFactory.createTestServerless();
+    const options = MockFactory.createTestServerlessOptions();
+    new AzureIndex(sls, options);
+    sls.setProvider = jest.fn();
+
+    const calls = (sls.pluginManager.addPlugin as any).mock.calls;
+
+    for (const call of calls) {
+      const pluginClass = call[0];
+      new pluginClass(MockFactory.createTestServerless(), MockFactory.createTestServerlessOptions());
+    }
+    expect(BaseService).not.toBeCalled();
   });
 });
diff --git a/src/shared/binding.test.ts b/src/shared/binding.test.ts
index 870385d..6ff6930 100644
--- a/src/shared/binding.test.ts
+++ b/src/shared/binding.test.ts
@@ -21,4 +21,51 @@ describe("Bindings", () => {
     BindingUtils.getBindingsMetaData(sls);
     expect(sls.cli.log).toBeCalledWith("Parsing Azure Functions Bindings.json...");
   });
+
+  it("Http output bindings should default to 'res'", () => {
+    const binding = BindingUtils.getHttpOutBinding();
+
+    expect(binding).toMatchObject({
+      type: "http",
+      direction: "out",
+      name: "res"
+    });
+  });
+
+  it("Gets the http binding with default settings", () => {
+    const serverless = MockFactory.createTestServerless();
+    const parsedBindings = BindingUtils.getBindingsMetaData(serverless);
+    const bindingType = "http";
+
+    const bindingTypes = parsedBindings.bindingTypes;
+    const bindingTypeIndex = bindingTypes.indexOf(bindingType);
+    const bindingSettings = parsedBindings.bindingSettings[bindingTypeIndex];
+
+    const binding = BindingUtils.getBinding(bindingType, bindingSettings, {});
+
+    expect(binding).toMatchObject({
+      type: "http",
+      direction: "out",
+      name: "res",
+    });
+  });
+
+  it("Gets the http binding with custom name", () => {
+    const serverless = MockFactory.createTestServerless();
+    const parsedBindings = BindingUtils.getBindingsMetaData(serverless);
+    const bindingType = "http";
+    const userSettings = { name: "custom" };
+
+    const bindingTypes = parsedBindings.bindingTypes;
+    const bindingTypeIndex = bindingTypes.indexOf(bindingType);
+    const bindingSettings = parsedBindings.bindingSettings[bindingTypeIndex];
+
+    const binding = BindingUtils.getBinding(bindingType, bindingSettings, userSettings);
+
+    expect(binding).toMatchObject({
+      type: "http",
+      direction: "out",
+      name: userSettings.name,
+    });
+  });
 });