Repository: serverless/serverless-azure-functions (ts)

Issue:
.serverless folder is not cleaned up before deployment
`.serverless` hosts packaged function artifacts. 

Currently the artifacts from previous deployment are not cleared with new one.
This creates two issues:
- Stale, not longer needed files pile up in `.serverless`
- In corner case scenarios files from previous deployments are used when they should not be. (e.g. if we suddenly try to deploy with  `package:individually` setting which [is not supported](https://github.com/serverless/serverless-azure-functions/issues/254), then plugin ignores fact that expected artifacts were not created, as it sees ones created in previous deployment, and uses them)


Pull request description:
fix: Clean up .serverless folder before packaging new artifact
Cleans up `.serverless` folder unless the package plugin is being executed from a `deploy` command in which the `--package` option was included (don't want to remove a package the user is trying to deploy).

Resolves #255 

Patch:
diff --git a/package-lock.json b/package-lock.json
index 7afd997..7465556 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -5019,6 +5019,17 @@
         "flatted": "^2.0.0",
         "rimraf": "2.6.3",
         "write": "1.0.3"
+      },
+      "dependencies": {
+        "rimraf": {
+          "version": "2.6.3",
+          "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.6.3.tgz",
+          "integrity": "sha512-mwqeW5XsA2qAejG46gYdENaxXjx9onRNCfn7L0duuP4hCuTIi/QO7PDK07KJfp1d+izWPrzEJDcSqBa0OZQriA==",
+          "dev": true,
+          "requires": {
+            "glob": "^7.1.3"
+          }
+        }
       }
     },
     "flatted": {
@@ -5086,17 +5097,6 @@
       "resolved": "https://registry.npmjs.org/fs-constants/-/fs-constants-1.0.0.tgz",
       "integrity": "sha512-y6OAwoSIf7FyjMIv94u+b5rdheZEjzR63GTyZJm5qh4Bi+2YgwLCcI/fPFZkL5PSixOt6ZNKm+w+Hfp/Bciwow=="
     },
-    "fs-extra": {
-      "version": "4.0.3",
-      "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-4.0.3.tgz",
-      "integrity": "sha512-q6rbdDd1o2mAnQreO7YADIxf/Whx4AHBiRf6d+/cVT8h44ss+lHgxf1FemcqDnQt9X3ct4McHr+JMGlYSsK7Cg==",
-      "optional": true,
-      "requires": {
-        "graceful-fs": "^4.1.2",
-        "jsonfile": "^4.0.0",
-        "universalify": "^0.1.0"
-      }
-    },
     "fs.realpath": {
       "version": "1.0.0",
       "resolved": "https://registry.npmjs.org/fs.realpath/-/fs.realpath-1.0.0.tgz",
@@ -5119,7 +5119,8 @@
         },
         "ansi-regex": {
           "version": "2.1.1",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "aproba": {
           "version": "1.2.0",
@@ -5154,7 +5155,8 @@
         },
         "code-point-at": {
           "version": "1.1.0",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "concat-map": {
           "version": "0.0.1",
@@ -5162,7 +5164,8 @@
         },
         "console-control-strings": {
           "version": "1.1.0",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "core-util-is": {
           "version": "1.0.2",
@@ -5265,7 +5268,8 @@
         },
         "inherits": {
           "version": "2.0.3",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "ini": {
           "version": "1.3.5",
@@ -5275,6 +5279,7 @@
         "is-fullwidth-code-point": {
           "version": "1.0.0",
           "bundled": true,
+          "optional": true,
           "requires": {
             "number-is-nan": "^1.0.0"
           }
@@ -5298,6 +5303,7 @@
         "minipass": {
           "version": "2.3.5",
           "bundled": true,
+          "optional": true,
           "requires": {
             "safe-buffer": "^5.1.2",
             "yallist": "^3.0.0"
@@ -5314,6 +5320,7 @@
         "mkdirp": {
           "version": "0.5.1",
           "bundled": true,
+          "optional": true,
           "requires": {
             "minimist": "0.0.8"
           }
@@ -5396,6 +5403,7 @@
         "once": {
           "version": "1.4.0",
           "bundled": true,
+          "optional": true,
           "requires": {
             "wrappy": "1"
           }
@@ -5471,7 +5479,8 @@
         },
         "safe-buffer": {
           "version": "5.1.2",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "safer-buffer": {
           "version": "2.1.2",
@@ -5501,6 +5510,7 @@
         "string-width": {
           "version": "1.0.2",
           "bundled": true,
+          "optional": true,
           "requires": {
             "code-point-at": "^1.0.0",
             "is-fullwidth-code-point": "^1.0.0",
@@ -5518,6 +5528,7 @@
         "strip-ansi": {
           "version": "3.0.1",
           "bundled": true,
+          "optional": true,
           "requires": {
             "ansi-regex": "^2.0.0"
           }
@@ -5556,11 +5567,13 @@
         },
         "wrappy": {
           "version": "1.0.2",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         },
         "yallist": {
           "version": "3.0.3",
-          "bundled": true
+          "bundled": true,
+          "optional": true
         }
       }
     },
@@ -9501,9 +9514,9 @@
       }
     },
     "rimraf": {
-      "version": "2.6.3",
-      "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.6.3.tgz",
-      "integrity": "sha512-mwqeW5XsA2qAejG46gYdENaxXjx9onRNCfn7L0duuP4hCuTIi/QO7PDK07KJfp1d+izWPrzEJDcSqBa0OZQriA==",
+      "version": "2.7.1",
+      "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.7.1.tgz",
+      "integrity": "sha512-uWjbaKIK3T1OSVptzX7Nl6PvQ3qAGtKEtVRjRuazjfL3Bx5eI409VZSqgND+4UNnmzLVdPj9FqFJNPqBZFve4w==",
       "requires": {
         "glob": "^7.1.3"
       }
@@ -9875,6 +9888,17 @@
         "ts-node": "^3.2.0"
       },
       "dependencies": {
+        "fs-extra": {
+          "version": "4.0.3",
+          "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-4.0.3.tgz",
+          "integrity": "sha512-q6rbdDd1o2mAnQreO7YADIxf/Whx4AHBiRf6d+/cVT8h44ss+lHgxf1FemcqDnQt9X3ct4McHr+JMGlYSsK7Cg==",
+          "optional": true,
+          "requires": {
+            "graceful-fs": "^4.1.2",
+            "jsonfile": "^4.0.0",
+            "universalify": "^0.1.0"
+          }
+        },
         "semver": {
           "version": "5.7.1",
           "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.1.tgz",
diff --git a/package.json b/package.json
index 474d8ea..49f40d1 100644
--- a/package.json
+++ b/package.json
@@ -55,7 +55,7 @@
     "md5": "^2.2.1",
     "open": "^6.3.0",
     "request": "^2.81.0",
-    "rimraf": "^2.6.3",
+    "rimraf": "^2.7.1",
     "semver": "^6.3.0",
     "xml": "^1.0.1"
   },
diff --git a/src/models/serverless.ts b/src/models/serverless.ts
index 6aba1a0..517e7f1 100644
--- a/src/models/serverless.ts
+++ b/src/models/serverless.ts
@@ -53,12 +53,71 @@ export interface ServerlessAzureProvider {
   runtime: string;
 }
 
+/**
+ * Expanding type of incomplete `Serverless` type.
+ */
+export interface ServerlessObject {
+  /** Input provided by user */
+  processedInput: {
+    /** Commands issued after `serverless` */
+    commands: ServerlessCliCommand[];
+    /** Flags provided in CLI */
+    options: any;
+  };
+  /** Provider specific configuration */
+  service: ServerlessAzureConfig;
+  /** Provider agnostic configuration */
+  config: ServerlessConfig;
+}
+
+/**
+ * Commands that can be issued in CLI
+ */
+export enum ServerlessCliCommand {
+  /** Package service */
+  PACKAGE = "package",
+  /** Deploy service */
+  DEPLOY = "deploy",
+  /** Invoke HTTP endpoint */
+  INVOKE = "invoke",
+  /** Roll back service */
+  ROLLBACK = "rollback",
+  /** Run service offline */
+  OFFLINE = "offline",
+  /** Start pre-built offline service - subcommand for offline*/
+  START = "start",
+  /** Build an offline service - subcommand for offline */
+  BUILD = "build",
+  /** Cleanup files from offline build - subcommand for offline */
+  CLEANUP = "cleanup",
+  /** List deployments - subcommand for deploy */
+  LIST = "list",
+  /** Command to add or remove functions */
+  FUNC = "func",
+  /** Add a function - subcommand for func */
+  ADD = "add",
+  /** Remove a function - subcommand for func */
+  REMOVE = "remove"
+}
+
+/** Vendor agnostic Serverless configuration */
+export interface ServerlessConfig {
+  /** Path to root Serverless service */
+  servicePath: string;
+  /** Path to packaged artifact of service */
+  packagePath: string;
+}
+
 export interface ServerlessAzureConfig {
   service: string;
   provider: ServerlessAzureProvider;
   plugins: string[];
   functions: any;
-  package: any;
+  package: {
+    individually: boolean;
+    artifactDirectoryName: string;
+    artifact: string;
+  };
 }
 
 export interface ServerlessAzureFunctionConfig {
diff --git a/src/plugins/azureBasePlugin.ts b/src/plugins/azureBasePlugin.ts
index d366814..f68b9fe 100644
--- a/src/plugins/azureBasePlugin.ts
+++ b/src/plugins/azureBasePlugin.ts
@@ -1,13 +1,15 @@
-import { Guard } from "../shared/guard";
 import Serverless from "serverless";
+import { ServerlessAzureConfig, ServerlessCliCommand,
+  ServerlessCommandMap, ServerlessHookMap, ServerlessObject } from "../models/serverless";
+import { Guard } from "../shared/guard";
 import { Utils } from "../shared/utils";
-import { ServerlessCommandMap, ServerlessHookMap, ServerlessAzureConfig } from "../models/serverless";
 
 export abstract class AzureBasePlugin<TOptions=Serverless.Options> {
 
   public hooks: ServerlessHookMap
   protected config: ServerlessAzureConfig;
   protected commands: ServerlessCommandMap;
+  protected processedCommands: ServerlessCliCommand[];
 
   public constructor(
     protected serverless: Serverless,
@@ -15,6 +17,7 @@ export abstract class AzureBasePlugin<TOptions=Serverless.Options> {
   ) {
     Guard.null(serverless);
     this.config = serverless.service as any;
+    this.processedCommands = (serverless as any as ServerlessObject).processedInput.commands;
   }
 
   protected log(message: string) {
diff --git a/src/plugins/package/azurePackagePlugin.ts b/src/plugins/package/azurePackagePlugin.ts
index 77b8a64..2c30567 100644
--- a/src/plugins/package/azurePackagePlugin.ts
+++ b/src/plugins/package/azurePackagePlugin.ts
@@ -3,6 +3,7 @@ import Serverless from "serverless";
 import AzureProvider from "../../provider/azureProvider";
 import { PackageService } from "../../services/packageService";
 import { AzureBasePlugin } from "../azureBasePlugin";
+import { ServerlessCliCommand } from "../../models/serverless";
 
 export class AzurePackagePlugin extends AzureBasePlugin {
   private bindingsCreated: boolean = false;
@@ -21,14 +22,15 @@ export class AzurePackagePlugin extends AzureBasePlugin {
   }
 
   private async setupProviderConfiguration(): Promise<void> {
-    if (this.getOption("package")) {
-      this.log("No need to create bindings. Using pre-existing package");
-      return Promise.resolve();
+    if (this.processedCommands[0] === ServerlessCliCommand.DEPLOY && this.getOption("package")) {
+      this.log("Deploying pre-built package. No need to create bindings");
+      return;
     }
     if (this.config.package && this.config.package.individually) {
       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();
     this.bindingsCreated = true;
 
diff --git a/src/services/packageService.ts b/src/services/packageService.ts
index e81ca47..4e340cc 100644
--- a/src/services/packageService.ts
+++ b/src/services/packageService.ts
@@ -1,5 +1,6 @@
 import fs from "fs";
 import path from "path";
+import rimraf from "rimraf";
 import Serverless from "serverless";
 import { FunctionMetadata, Utils } from "../shared/utils";
 import { BaseService } from "./baseService";
@@ -12,6 +13,14 @@ export class PackageService extends BaseService {
     super(serverless, options, false);
   }
 
+  public cleanUpServerlessDir() {
+    const serverlessDir = path.join(this.serverless.config.servicePath, ".serverless");
+    if (fs.existsSync(serverlessDir)) {
+      this.log("Removing .serverless directory")
+      rimraf.sync(serverlessDir);
+    }
+  }
+
   /**
    * Creates the function.json binding files required for the serverless service
    */


Test patch:
diff --git a/src/plugins/apim/azureApimServicePlugin.test.ts b/src/plugins/apim/azureApimServicePlugin.test.ts
index aaff0fb..b748395 100644
--- a/src/plugins/apim/azureApimServicePlugin.test.ts
+++ b/src/plugins/apim/azureApimServicePlugin.test.ts
@@ -12,7 +12,7 @@ describe("APIM Service Plugin", () => {
   });
 
   it("can be instantiated", () => {
-    const serverless = new Serverless();
+    const serverless = MockFactory.createTestServerless();
     const options: Serverless.Options = {
       stage: "",
       region: "",
diff --git a/src/plugins/package/azurePackagePlugin.test.ts b/src/plugins/package/azurePackagePlugin.test.ts
index e789d44..45e3137 100644
--- a/src/plugins/package/azurePackagePlugin.test.ts
+++ b/src/plugins/package/azurePackagePlugin.test.ts
@@ -20,11 +20,13 @@ describe("Azure Package Plugin", () => {
   it("sets creates function bindings before package:setupProviderConfiguration life cycle event", async () => {
     await invokeHook(plugin, "before:package:setupProviderConfiguration");
     expect(PackageService.prototype.createBindings).toBeCalled();
+    expect(PackageService.prototype.cleanUpServerlessDir).toBeCalled();
   });
 
   it("prepares the package for webpack before webpack:package:packageModules life cycle event", async () => {
     await invokeHook(plugin, "before:webpack:package:packageModules");
     expect(PackageService.prototype.createBindings).toBeCalled();
+    expect(PackageService.prototype.cleanUpServerlessDir).toBeCalled();
     expect(PackageService.prototype.prepareWebpack).toBeCalled();
   });
 
@@ -34,6 +36,7 @@ describe("Azure Package Plugin", () => {
 
     expect(PackageService.prototype.createBindings).toBeCalledTimes(1);
     expect(PackageService.prototype.prepareWebpack).toBeCalledTimes(1);
+    expect(PackageService.prototype.cleanUpServerlessDir).toBeCalledTimes(1);
   });
 
   it("cleans up package after package:finalize", async () => {
@@ -63,19 +66,22 @@ describe("Azure Package Plugin", () => {
     it("does not call create bindings if package specified in options", async () => {
       await invokeHook(plugin, "before:package:setupProviderConfiguration");
       expect(PackageService.prototype.createBindings).not.toBeCalled();
-      expect(sls.cli.log).lastCalledWith("No need to create bindings. Using pre-existing package");
+      expect(sls.cli.log).lastCalledWith("Deploying pre-built package. No need to create bindings");
     });
 
     it("does not call webpack if package specified in options", async () => {
       await invokeHook(plugin, "before:webpack:package:packageModules");
       expect(PackageService.prototype.createBindings).not.toBeCalled();
       expect(PackageService.prototype.prepareWebpack).not.toBeCalled();
+      expect(PackageService.prototype.cleanUpServerlessDir).not.toBeCalled();
+
       expect(sls.cli.log).lastCalledWith("No need to perform webpack. Using pre-existing package");
     });
 
     it("does not call finalize if package specified in options", async () => {
       await invokeHook(plugin, "after:package:finalize");
       expect(PackageService.prototype.cleanUp).not.toBeCalled();
+      expect(PackageService.prototype.cleanUpServerlessDir).not.toBeCalled();
       expect(sls.cli.log).lastCalledWith("No need to clean up generated folders & files. Using pre-existing package");
     });
   })
diff --git a/src/services/armService.test.ts b/src/services/armService.test.ts
index f09a2bd..d70cb46 100644
--- a/src/services/armService.test.ts
+++ b/src/services/armService.test.ts
@@ -70,7 +70,7 @@ describe("Arm Service", () => {
     });
 
     it("Creates a custom ARM template from well-known type", async () => {
-      sls.service.provider.runtime = "nodejs6.9.x";      
+      sls.service.provider.runtime = "nodejs6.9.x";
       const deployment = await service.createDeploymentFromType("premium");
 
       expect(deployment).not.toBeNull();
@@ -95,36 +95,37 @@ describe("Arm Service", () => {
     });
 
     it("throws error when invalid nodejs version in defined", async () => {
-      sls.service.provider.runtime = "nodejs10.6.1"; 
+      sls.service.provider.runtime = "nodejs10.6.1";
       await expect(service.createDeploymentFromType("premium")).rejects.toThrowError("Could not find runtime version matching nodejs10.6.1");
     });
 
     it("throws error when incomplete nodejs version in defined", async () => {
-      sls.service.provider.runtime = "nodejs8"; 
+      sls.service.provider.runtime = "nodejs8";
       await expect(service.createDeploymentFromType("premium")).rejects.toThrowError("Could not find runtime version matching nodejs8");
     });
 
     it("throws error when unsupported nodejs version in defined", async () => {
-      sls.service.provider.runtime = "nodejs5.x"; 
+      sls.service.provider.runtime = "nodejs5.x";
       await expect(service.createDeploymentFromType("premium")).rejects.toThrowError("Could not find runtime version matching nodejs5.x");
     });
 
     it("Does not throw an error when valid nodejs version is defined", async () => {
-      sls.service.provider.runtime = "nodejs10.x"; 
+      sls.service.provider.runtime = "nodejs10.x";
       await expect(service.createDeploymentFromType("premium")).resolves.not.toThrow();
     });
 
     it("Does not throw an error when nodejs version with major and minor is defined", async () => {
-      sls.service.provider.runtime = "nodejs6.9.x"; 
+      sls.service.provider.runtime = "nodejs6.9.x";
       await expect(service.createDeploymentFromType("premium")).resolves.not.toThrow();
     });
 
     it("Does not throw an error when specific nodejs version is defined", async () => {
-      sls.service.provider.runtime = "nodejs10.6.0"; 
+      sls.service.provider.runtime = "nodejs10.6.0";
       await expect(service.createDeploymentFromType("premium")).resolves.not.toThrow();
     });
 
-    it("throws an error when no nodejs version in defined", async () => {
+    it("throws an error when no nodejs version is defined", async () => {
+      sls.service.provider.runtime = undefined;
       await expect(service.createDeploymentFromType("premium")).rejects.toThrowError("Runtime version not specified in serverless.yml");
     });
 
@@ -178,7 +179,7 @@ describe("Arm Service", () => {
     });
 
     it("Consumption template includes correct resources", async () => {
-      sls.service.provider.runtime = "nodejs10.x";      
+      sls.service.provider.runtime = "nodejs10.x";
       const deployment = await service.createDeploymentFromType(ArmTemplateType.Consumption);
 
       expect(deployment.template.resources.find((resource) => resource.type === "Microsoft.Web/hostingEnvironments")).toBeUndefined();
diff --git a/src/services/packageService.test.ts b/src/services/packageService.test.ts
index 9050927..8fac111 100644
--- a/src/services/packageService.test.ts
+++ b/src/services/packageService.test.ts
@@ -6,6 +6,9 @@ import { PackageService } from "./packageService";
 import { MockFactory } from "../test/mockFactory";
 import { FunctionMetadata } from "../shared/utils";
 
+jest.mock("rimraf");
+import rimraf from "rimraf";
+
 describe("Package Service", () => {
   let sls: Serverless;
   let packageService: PackageService;
@@ -25,6 +28,17 @@ describe("Package Service", () => {
     mockFs.restore();
   });
 
+  it("cleans up previous .serverless folder", async () => {
+    mockFs({
+      ".serverless": {
+        "artifact.zip": "contents"
+      }
+    });
+
+    packageService.cleanUpServerlessDir();
+    expect(rimraf.sync).toBeCalledWith(path.join(process.cwd(), ".serverless"));
+  });
+
   it("cleans up function.json and function folders", async () => {
     const fsConfig = {};
     const functionNames = sls.service.getAllFunctions();
diff --git a/src/test/mockFactory.ts b/src/test/mockFactory.ts
index 63a3b69..d6c1a68 100644
--- a/src/test/mockFactory.ts
+++ b/src/test/mockFactory.ts
@@ -15,7 +15,7 @@ import { ApiCorsPolicy, ApiManagementConfig } from "../models/apiManagement";
 import { ArmDeployment, ArmResourceTemplate, ArmTemplateProvisioningState } from "../models/armTemplates";
 import { ServicePrincipalEnvVariables } from "../models/azureProvider";
 import { Logger } from "../models/generic";
-import { ServerlessAzureConfig, ServerlessAzureProvider, ServerlessAzureFunctionConfig } from "../models/serverless";
+import { ServerlessAzureConfig, ServerlessAzureProvider, ServerlessAzureFunctionConfig, ServerlessCliCommand } from "../models/serverless";
 
 function getAttribute(object: any, prop: string, defaultValue: any): any {
   if (object && object[prop]) {
@@ -34,6 +34,10 @@ export class MockFactory {
     sls.service = getAttribute(config, "service", MockFactory.createTestService());
     sls.config.servicePath = "";
     sls.setProvider = jest.fn();
+    sls["processedInput"] = {
+      commands: [ ServerlessCliCommand.DEPLOY ],
+      options: {}
+    };
     return sls;
   }
 
@@ -360,6 +364,7 @@ export class MockFactory {
       deploymentName: "myDeploymentName",
       region: "eastus2",
       stage: "dev",
+      runtime: "nodejs10.x",
     }
   }