Cumulus 748

.eslint-ratchet-high-water-mark

@@ -1 +1 @@
-670
+467

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
     - update the StreamSpecification DynamoDB tables to have StreamViewType: "NEW_AND_OLD_IMAGES"
     - delete granule files in s3
 - **CUMULUS-398** - Fix not able to filter executions bu workflow
+- **CUMULUS-748** - Fix invalid lambda .zip files being validated/uploaded to AWS
 
 ## [v1.6.0] - 2018-06-06
 
@@ -353,4 +354,3 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 [v1.1.0]: https://github.com/cumulus-nasa/cumulus/compare/v1.0.1...v1.1.0
 [v1.0.1]: https://github.com/cumulus-nasa/cumulus/compare/v1.0.0...v1.0.1
 [v1.0.0]: https://github.com/cumulus-nasa/cumulus/compare/pre-v1-release...v1.0.0
-

packages/deployment/lib/lambda.js

@@ -7,6 +7,8 @@ const path = require('path');
 const utils = require('kes').utils;
 const { Lambda } = require('kes');
 
+const yauzl = require('yauzl');
+
 /**
  * A sub-class of the Kes Lambda class that changes
  * how kes handles Lambda function compression and
@@ -27,6 +29,28 @@ class UpdatedLambda extends Lambda {
     super(config);
     this.config = config;
   }
+
+  /**
+   * Makes use of yauzl.open's verification
+   * to validate the file referenced in lambda.local
+   * is a valid zip archive
+   *
+   * @param {Object} lambda - the lambda object
+   *
+   * @returns {Promise} promise resolution indicates success, will reject if
+   *                    zipfile cannot be opened
+   **/
+  validateZipFile(lambda) {
+    return new Promise((resolve, reject) => {
+      yauzl.open(lambda.local, (err, _zipfile) => {
+        if (err) {
+          reject(err);
+        }
+        resolve();
+      });
+    });
+  }
+
   /**
    * Copies the source code of a given lambda function, zips it, calculates
    * the hash of the source code and updates the lambda object with
@@ -35,23 +59,28 @@ class UpdatedLambda extends Lambda {
    * @param {Object} lambda - the lambda object
    * @returns {Promise} returns the updated lambda object
    */
-  zipLambda(lambda) {
+  async zipLambda(lambda) {
     let msg = `Zipping ${lambda.local}`;
     // skip if the file with the same hash is zipped
+    // and is a valid zip file
     if (fs.existsSync(lambda.local)) {
-      return Promise.resolve(lambda);
+      try {
+        await this.validateZipFile(lambda);
+        return Promise.resolve(lambda);
+      }
+      catch (e) {
+        console.log(`${lambda.local} appears to be an invalid zip file, and will be re-built`);
+      }
     }
     const fileList = [lambda.source];
-
     if (lambda.useMessageAdapter) {
       const kesFolder = path.join(this.config.kesFolder, 'build', 'adapter');
       fileList.push(kesFolder);
       msg += ' and injecting message adapter';
     }
 
     console.log(`${msg} for ${lambda.name}`);
-
-    return utils.zip(lambda.local, fileList).then(() => lambda);
+    return utils.zip(lambda.local, fileList).then(() => lambda).catch((e) => console.log(`Error zipping ${e}`));
   }
 
   /**

packages/deployment/test/test_lambda.js

@@ -25,7 +25,7 @@ test.beforeEach(async (t) => {
     bucket: 'testbucket',
     stack: 'teststack'
   };
-
+  t.context.fixturedir = 'test/fixtures';
   t.context.lambda = {
     handler: 'index.handler',
     name: 'lambda-example',
@@ -62,7 +62,7 @@ test.serial('zipLambda: works for lambda not using message adapter', async (t) =
   t.context.lambda.useMessageAdapter = false;
   const lambdaLocalOrigin = t.context.lambda.local;
   const lambdaRemoteOrigin = t.context.lambda.remote;
-  const l = new Lambda(t.context.config)
+  const l = new Lambda(t.context.config);
   await l.zipLambda(l.buildS3Path(t.context.lambda));
   t.truthy(fs.statSync(t.context.lambda.local));
   t.is(t.context.lambda.local, lambdaLocalOrigin);
@@ -73,7 +73,7 @@ test.serial('zipLambda: works for lambda using message adapter', async (t) => {
   t.context.lambda.useMessageAdapter = true;
   const lambdaLocalOrigin = t.context.lambda.local;
   const lambdaRemoteOrigin = t.context.lambda.remote;
-  const l = new Lambda(t.context.config)
+  const l = new Lambda(t.context.config);
   await l.zipLambda(l.buildS3Path(t.context.lambda));
   t.truthy(fs.statSync(t.context.lambda.local));
   t.is(
@@ -87,7 +87,42 @@ test.serial('zipLambda: works for lambda using message adapter', async (t) => {
 });
 
 test.serial(
-  `zipLambda: for lambda using message adapter, no new file is generated 
+  `zipLambda: given an invalid zip file generated from a previous run,
+  a new valid lambda file is generated`,
+  async (t) => {
+    t.context.lambda.useMessageAdapter = true;
+    const lambdaLocalOrigin = t.context.lambda.local;
+    const lambdaRemoteOrigin = t.context.lambda.remote;
+
+    // put an empty lambda zip file there as the result of the previous run
+    const existingLambdaLocal = path.join(
+      path.dirname(t.context.lambda.local),
+      `${Lambda.messageAdapterZipFileHash}-${path.basename(t.context.lambda.local)}`
+    );
+
+    fs.writeFileSync(existingLambdaLocal, 'hello');
+    t.is(fs.statSync(existingLambdaLocal).size, 5);
+
+    const l = new Lambda(t.context.config);
+    await l.zipLambda(l.buildS3Path(t.context.lambda));
+    t.truthy(fs.statSync(t.context.lambda.local));
+    t.true(fs.statSync(t.context.lambda.local).size > 5);
+    t.is(t.context.lambda.local, existingLambdaLocal);
+
+    t.is(
+      path.basename(t.context.lambda.local),
+      `${Lambda.messageAdapterZipFileHash}-${path.basename(lambdaLocalOrigin)}`
+    );
+    t.is(
+      path.basename(t.context.lambda.remote),
+      `${Lambda.messageAdapterZipFileHash}-${path.basename(lambdaRemoteOrigin)}`
+    );
+  }
+);
+
+
+test.serial(
+  `zipLambda: for lambda using message adapter, no new file is generated
   if the task and message adapter are not updated`,
   async (t) => {
     t.context.lambda.useMessageAdapter = true;
@@ -103,20 +138,20 @@ test.serial(
       `${Lambda.messageAdapterZipFileHash}-${path.basename(t.context.lambda.remote)}`
     );
 
-    fs.writeFileSync(existingLambdaLocal, 'hello');
-    t.is(fs.statSync(existingLambdaLocal).size, 5);
+    fs.copySync(`${t.context.fixturedir}/zipfile-fixture.zip`, existingLambdaLocal);
+    t.is(fs.statSync(existingLambdaLocal).size, 180);
 
-    const l = new Lambda(t.context.config)
+    const l = new Lambda(t.context.config);
     await l.zipLambda(l.buildS3Path(t.context.lambda));
     t.truthy(fs.statSync(t.context.lambda.local));
-    t.is(fs.statSync(t.context.lambda.local).size, 5);
+    t.is(fs.statSync(t.context.lambda.local).size, 180);
     t.is(t.context.lambda.local, existingLambdaLocal);
     t.is(t.context.lambda.remote, existingLambdaRemote);
   }
 );
 
 test.serial(
-  `zipLambda: for lambda using message adapter, a new file is created 
+  `zipLambda: for lambda using message adapter, a new file is created
   if the message adapter is updated`,
   async (t) => {
     t.context.lambda.useMessageAdapter = true;
@@ -136,7 +171,7 @@ test.serial(
     const adapterHashOrigin = Lambda.messageAdapterZipFileHash;
     Lambda.messageAdapterZipFileHash = `${adapterHashOrigin}123`;
 
-    const l = new Lambda(t.context.config)
+    const l = new Lambda(t.context.config);
     await l.zipLambda(l.buildS3Path(t.context.lambda));
     t.truthy(fs.statSync(t.context.lambda.local));
     t.true(fs.statSync(t.context.lambda.local).size > 5);