From 32ee119e29ad2484e9961ac6e44eb23e4687038d Mon Sep 17 00:00:00 2001 From: tuuli Date: Tue, 16 Oct 2018 12:07:00 +0200 Subject: [PATCH] Cleanup --- .env.test | 3 +- .gitignore | 1 + .../__mocks__/akSubjectAccessData.js | 1 - .../resources/akSubjectAccessData.js | 1 - lib/dynamodb/operationsLogger.js | 3 +- lib/util/__mocks__/sendSAREmail.js | 5 --- lib/util/__mocks__/shipToS3.js | 3 -- lib/util/__mocks__/zipCSVFiles.js | 4 -- lib/util/processSubjectAccessRequest.js | 14 +++---- lib/util/shipToS3.js | 38 +++++++++---------- lib/util/zipCSVFiles.js | 13 ++++--- members-service/akSubjectAccessData.js | 11 ++---- members-service/champaignSubjectAccessData.js | 12 ++---- .../triggerSubjectAccessRequest.js | 37 +++++++++--------- .../triggerSubjectAccessRequest.test.js | 3 ++ members-service/updateMember.js | 4 +- serverless.yml | 4 ++ settings/production.yml | 6 +++ 18 files changed, 73 insertions(+), 90 deletions(-) delete mode 100644 lib/util/__mocks__/sendSAREmail.js delete mode 100644 lib/util/__mocks__/shipToS3.js delete mode 100644 lib/util/__mocks__/zipCSVFiles.js diff --git a/.env.test b/.env.test index f27b28a..d18c45d 100644 --- a/.env.test +++ b/.env.test @@ -14,4 +14,5 @@ export AK_MYSQL_HOST="iamahost", export AK_MYSQL_USER="user", export AK_MYSQL_PWD="pwd", export AK_MYSQL_DB="mysli" -export MEMBER_SERVICES_EMAIL=info@example.com +export MEMBER_SERVICES_EMAIL=tuuli@sumofus.org +export LOCAL_TMP=true diff --git a/.gitignore b/.gitignore index d474aa8..922e1f0 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,7 @@ coverage # dotenv environment variables file .env +.env.development secrets.sh .secrets diff --git a/lib/clients/actionkit/resources/__mocks__/akSubjectAccessData.js b/lib/clients/actionkit/resources/__mocks__/akSubjectAccessData.js index d36afdd..c10d3d3 100644 --- a/lib/clients/actionkit/resources/__mocks__/akSubjectAccessData.js +++ b/lib/clients/actionkit/resources/__mocks__/akSubjectAccessData.js @@ -3,6 +3,5 @@ const AKMockData = { }; export function AKSubjectAccessData(email) { - console.log('Im in the ak subject access data mock function'); return Promise.resolve(AKMockData); } diff --git a/lib/clients/actionkit/resources/akSubjectAccessData.js b/lib/clients/actionkit/resources/akSubjectAccessData.js index d52f8c0..a3ce746 100644 --- a/lib/clients/actionkit/resources/akSubjectAccessData.js +++ b/lib/clients/actionkit/resources/akSubjectAccessData.js @@ -5,7 +5,6 @@ import { subjectAccessQueryParser } from './subjectAccessQueries/subjectAccessQu import { json2csv } from 'json-2-csv'; export function AKSubjectAccessData(email) { - console.log('UH OH, ACTUALLY CONNECTING TO MYSQL'); AKMysqlClient.connect(); return ( diff --git a/lib/dynamodb/operationsLogger.js b/lib/dynamodb/operationsLogger.js index 4244667..fc8db42 100644 --- a/lib/dynamodb/operationsLogger.js +++ b/lib/dynamodb/operationsLogger.js @@ -69,7 +69,7 @@ export class OperationsLogger { const names = { '#s': 'status', ...reduce(keys, namesReducer, {}) }; const values = reduce(keys, valuesReducer, {}); - return { + const updateObject = { TableName: this.tableName, Key: { id: record.id, @@ -79,6 +79,7 @@ export class OperationsLogger { ExpressionAttributeNames: names, ExpressionAttributeValues: values, }; + return updateObject; } dynamodbPutParams(logData: LogData) { diff --git a/lib/util/__mocks__/sendSAREmail.js b/lib/util/__mocks__/sendSAREmail.js deleted file mode 100644 index 62995a0..0000000 --- a/lib/util/__mocks__/sendSAREmail.js +++ /dev/null @@ -1,5 +0,0 @@ -export function sendEmail(url, recipient, email, processor) { - return Promise.resolve({ - MessageId: 'EXAMPLE78603177f-7a5433e7-8edb-42ae-af10-f0181f34d6ee-000000', - }); -} diff --git a/lib/util/__mocks__/shipToS3.js b/lib/util/__mocks__/shipToS3.js deleted file mode 100644 index 07f7765..0000000 --- a/lib/util/__mocks__/shipToS3.js +++ /dev/null @@ -1,3 +0,0 @@ -export function shipToS3(file, bucket) { - return Promise.resolve('www.bogus-url.com'); -} diff --git a/lib/util/__mocks__/zipCSVFiles.js b/lib/util/__mocks__/zipCSVFiles.js deleted file mode 100644 index 81c209e..0000000 --- a/lib/util/__mocks__/zipCSVFiles.js +++ /dev/null @@ -1,4 +0,0 @@ -export function zipCSVFiles(dir, filename) { - console.log('Mock zip csv files'); - return Promise.resolve('/path/to/zip/file.zip'); -} diff --git a/lib/util/processSubjectAccessRequest.js b/lib/util/processSubjectAccessRequest.js index 761d202..a3e3713 100644 --- a/lib/util/processSubjectAccessRequest.js +++ b/lib/util/processSubjectAccessRequest.js @@ -40,14 +40,9 @@ export function SARconstructor( sendEmail = sendEmail ) { return function(data, processor, email) { - console.log( - 'PROCESS SUBJECT ACCESS REQUEST (FO REAL NO MOCK)...', - data, - processor - ); return new Promise(function(resolve, reject) { - const tmpDir = `${__dirname}/tmp`; - console.log('TMPDIR:', tmpDir); + // Lambda only allows you to write to /tmp. On your local environment you probably don't want to write to /tmp. + const tmpDir = process.env.LOCAL_TMP ? `${__dirname}/tmp` : '/tmp'; fs.ensureDirSync(`${tmpDir}/csv`); _forOwn(data, function(val, key) { @@ -74,7 +69,7 @@ export function SARconstructor( }) .then(function(_) { // Makes sense to do cleanup in case the lambda environment gets reused for multiple invocations: - return fs.remove(tmpDir); + return fs.emptyDir(tmpDir); }) .then(function(_) { resolve( @@ -82,7 +77,8 @@ export function SARconstructor( email }` ); - }); + }) + .catch(reject); }); }; } diff --git a/lib/util/shipToS3.js b/lib/util/shipToS3.js index 3e748ba..be92bcf 100644 --- a/lib/util/shipToS3.js +++ b/lib/util/shipToS3.js @@ -6,26 +6,22 @@ import fs from 'fs'; // Sends a file to a specified bucket on S3. Returns a promise that resolves with a temporary signed URL to the object // that is valid for 10 minutes. export function shipToS3(file, bucket) { - const s3 = new AWS.S3(); - - const signedUrlExpireSeconds = 60 * 10; // 10 minutes - const key = path.parse(file).base; - const readStream = fs.createReadStream(file); - - const params = { - Bucket: bucket, // 'subjectAccessRequests' - Key: key, // e.g. tuuli@sumofus.org-champaign.zip - Body: readStream, - }; - - return s3 - .upload(params) - .promise() - .then(function(_) { - return s3.getSignedUrl('getObject', { - Bucket: bucket, - Key: key, - Expires: signedUrlExpireSeconds, + return new Promise(function(resolve, reject) { + const readStream = fs.createReadStream(file); + const key = path.parse(file).base; + resolve({ Body: readStream, Key: key, Bucket: bucket }); + }).then(function(params) { + const signedUrlExpireSeconds = 60 * 10; // 10 minutes + const s3 = new AWS.S3(); + return s3 + .upload(params) + .promise() + .then(function(_) { + return s3.getSignedUrl('getObject', { + Bucket: params.Bucket, + Key: params.Key, + Expires: signedUrlExpireSeconds, + }); }); - }); + }); } diff --git a/lib/util/zipCSVFiles.js b/lib/util/zipCSVFiles.js index 41c571a..10fbf02 100644 --- a/lib/util/zipCSVFiles.js +++ b/lib/util/zipCSVFiles.js @@ -7,9 +7,14 @@ import { moveSync } from 'fs-extra'; // Takes a directory and a name of a zip file, zips the directory into filename.zip, moves the resulting zipfile into // the directory, and returns a promise that resolves to the complete path of the resulting zipfile. export function zipCSVFiles(dir, filename) { - console.log('In the real zip csv function :('); return new Promise(function(resolve, reject) { - var output = fs.createWriteStream(filename); + // PROBLEM: On Lambda, I can only create files in /tmp. Archiver can only deal with a single level path passed to + // the directory argument when zipping a directory. I have to create a zipfile in /tmp and then I have to zip the + // contents of /tmp, which leads to an archive that includes both the /csv subdirectory, and the zipfile itself. + // I resolved this earlier by creating the zip file elsewhere and moving it to /tmp, but that throws an error on + // Lambda since you can only write to /tmp. This current solution includes the .zip in the archive. + const zipPath = `${dir}/${filename}`; + var output = fs.createWriteStream(zipPath); var archive = archiver('zip', { zlib: { level: 9 }, }); @@ -31,9 +36,7 @@ export function zipCSVFiles(dir, filename) { archive.finalize(); output.on('close', function() { - const destPath = `${dir}/${filename}`; - moveSync(filename, destPath, { overwrite: true }); - resolve(destPath); + resolve(zipPath); }); }); } diff --git a/members-service/akSubjectAccessData.js b/members-service/akSubjectAccessData.js index 0ca2621..8109a70 100644 --- a/members-service/akSubjectAccessData.js +++ b/members-service/akSubjectAccessData.js @@ -44,12 +44,6 @@ export const handlerFunc = ( return logger .updateStatus(record, { actionkit: 'SUCCESS' }) .then(dynamodbSuccess => { - console.log( - 'SUCCESSFUL SUBJECT ACCESS REQUEST EVENT - call callback with: ', - success, - ' callback: ', - callback - ); return callback(null, success); }) .catch(dynamodbError => { @@ -60,11 +54,12 @@ export const handlerFunc = ( return logger .updateStatus(record, { actionkit: 'FAILURE' }) .then(dynamodbSuccess => { - return callback(err); + return callback(null, err); }) .catch(dynamodbError => { // Wow, nothing is going right today. The request failed AND DynamoDB didn't update the record. - return callback(dynamodbError); + // return a success response because we don't want a week of retries. + return callback(null, dynamodbError); }); }); }; diff --git a/members-service/champaignSubjectAccessData.js b/members-service/champaignSubjectAccessData.js index 3aa115a..6ba6805 100644 --- a/members-service/champaignSubjectAccessData.js +++ b/members-service/champaignSubjectAccessData.js @@ -47,12 +47,6 @@ export const handlerFunc = ( return logger .updateStatus(record, { champaign: 'SUCCESS' }) .then(dynamodbSuccess => { - console.log( - 'SUCCESSFUL SUBJECT ACCESS REQUEST EVENT - call callback with: ', - success, - ' callback: ', - callback - ); return callback(null, success); }) .catch(dynamodbError => { @@ -60,15 +54,15 @@ export const handlerFunc = ( }); }) .catch(err => { - console.log('FAILURE!', err); return logger .updateStatus(record, { champaign: 'FAILURE' }) .then(dynamodbSuccess => { - return callback(err); + return callback(null, err); }) .catch(dynamodbError => { // Wow, nothing is going right today. The request failed AND DynamoDB didn't update the record. - return callback(dynamodbError); + // return a success response because we don't want a week of retries. + return callback(null, dynamodbError); }); }); }; diff --git a/members-service/triggerSubjectAccessRequest.js b/members-service/triggerSubjectAccessRequest.js index 80809e1..4079893 100644 --- a/members-service/triggerSubjectAccessRequest.js +++ b/members-service/triggerSubjectAccessRequest.js @@ -3,7 +3,7 @@ import { OperationsLogger } from '../lib/dynamodb/operationsLogger'; import { DocumentClient } from 'aws-sdk/clients/dynamodb'; import { validateRequest } from '../lib/request-validator'; import { SUBJECT_ACCESS_REQUEST_SCHEMA } from './request-schemas'; -import { response, badRequest, ok } from '../lib/lambda-utils/responses'; +import { response, badRequest } from '../lib/lambda-utils/responses'; import log from '../lib/logger'; @@ -20,25 +20,22 @@ export const handlerFunc = (event: any, context: any, callback: any) => { ...payload, }; - return validateRequest(SUBJECT_ACCESS_REQUEST_SCHEMA, parameters).then( - params => { - logger - .log({ - event: 'MEMBER:SUBJECT_ACCESS_REQUEST', - data: { - email: parameters.email, - }, - status: { actionkit: 'PENDING', champaign: 'PENDING' }, - }) - .then( - result => callback(null, response(result)), - error => callback(null, response(error)) - ); - }, - errors => { - callback(null, badRequest({ cors: true, body: errors })); - } - ); + return validateRequest(SUBJECT_ACCESS_REQUEST_SCHEMA, parameters) + .then(params => { + return logger.log({ + event: 'MEMBER:SUBJECT_ACCESS_REQUEST', + data: { + email: parameters.email, + }, + status: { actionkit: 'PENDING', champaign: 'PENDING' }, + }); + }) + .then(res => { + return callback(null, response({ cors: true, body: res })); + }) + .catch(err => { + return callback(null, badRequest({ cors: true, body: err })); + }); }; export const handler = log(handlerFunc); diff --git a/members-service/triggerSubjectAccessRequest.test.js b/members-service/triggerSubjectAccessRequest.test.js index baee716..ffd2430 100644 --- a/members-service/triggerSubjectAccessRequest.test.js +++ b/members-service/triggerSubjectAccessRequest.test.js @@ -2,6 +2,9 @@ import { handlerFunc as handler } from './triggerSubjectAccessRequest'; import { DocumentClient } from 'aws-sdk/clients/dynamodb'; import { OperationsLogger } from '../lib/dynamodb/operationsLogger'; +jest + .spyOn(DocumentClient.prototype, 'put') + .mockImplementation(opts => ({ promise: () => Promise.resolve(opts) })); jest.spyOn(OperationsLogger.prototype, 'log'); describe('triggerSubjectAccessRequest handler', function() { diff --git a/members-service/updateMember.js b/members-service/updateMember.js index 36bfc9f..b0dd68f 100644 --- a/members-service/updateMember.js +++ b/members-service/updateMember.js @@ -39,8 +39,8 @@ export const handlerFunc = (event: any, context: any, callback: any) => { return result; }) .then( - result => callback(null, response(result)), - error => callback(null, response(error)) + result => callback(null, response({ cors: true, body: result })), + error => callback(null, response({ cors: true, body: error })) ); }, errors => callback(null, badRequest({ cors: true, body: errors })) diff --git a/serverless.yml b/serverless.yml index 75f3ebc..a01cc73 100644 --- a/serverless.yml +++ b/serverless.yml @@ -22,6 +22,10 @@ provider: - dynamodb:ListStreams Resource: "arn:aws:dynamodb:us-east-1:*:table/${self:provider.environment.DB_LOG_TABLE}" Effect: Allow + - Action: + - s3:PutObject + Resource: "arn:aws:s3:::champaign/*" + Effect: Allow resources: Resources: diff --git a/settings/production.yml b/settings/production.yml index ada5e30..ccaa765 100644 --- a/settings/production.yml +++ b/settings/production.yml @@ -14,3 +14,9 @@ environment: UNSUBSCRIBE_PAGE_NAME: ${ssm:/api-services/production/UNSUBSCRIBE_PAGE_NAME} BRAINTREE_MERCHANT_CURRENCIES: ${ssm:/api-services/production/BRAINTREE_MERCHANT_CURRENCIES} COGNITO_POOL_ARN: ${ssm:/api-services/production/COGNITO_POOL_ARN} + AK_MYSQL_HOST: ${ssm:/api-services/production/AK_MYSQL_HOST} + AK_MYSQL_USER: ${ssm:/api-services/production/AK_MYSQL_USER} + AK_MYSQL_PWD: ${ssm:/api-services/production/AK_MYSQL_PWD} + AK_MYSQL_DB: ${ssm:/api-services/production/AK_MYSQL_DB} + MEMBER_SERVICES_EMAIL: ${ssm:/api-services/production/MEMBER_SERVICES_EMAIL} +