update file upload api #132
Conversation
Add extra audit logging messages
Submit a report for approval
Drop attachment enum type
src/lib/s3Uploader.js
Outdated
| let s3Config; | ||
|
|
||
| if (process.env.VCAP_SERVICES) { | ||
| const bucket = process.env.VCAP_SERVICES.s3[0]; |
There was a problem hiding this comment.
VCAP_SERVICES is a json string in the environment. I know in ruby you need to call JSON.parse(ENV["VCAP_SERVICES"]) to get the values out. Does node handle that transparently?
src/lib/s3Uploader.js
Outdated
| s3Config = { | ||
| accessKeyId: bucket.access_key_id, | ||
| endpoint: bucket.uri, | ||
| secretAccessKey: bucket.secret_access_key, |
There was a problem hiding this comment.
I think these paths won't quite be correct. See the example data I DM'd you in slack. There's a credentials object that they're held in inside of what you've assigned to bucket
src/lib/s3Uploader.js
Outdated
| s3ForcePathStyle: true, | ||
| }; | ||
| } | ||
| export const s3 = new S3(s3Config); | ||
|
|
||
| export const verifyVersioning = async (bucket = process.env.bucket, s3Client = s3) => { |
There was a problem hiding this comment.
Setting bucket name should also come from VCAP_SERVICES in deployed environments. Looks like there's two points the bucket name is set, and right now they're being set to different env vars. process.env.bucket here and process.env.S3_BUCKET on line 47
Update deployed env vars
…Head-Start-TTADP into cm-pr-267-add-file-upload-api
src/lib/s3Uploader.js
Outdated
| let bucketName = process.env.S3_BUCKET; | ||
| if (process.env.VCAP_SERVICES) { | ||
| const { credentials, name } = process.env.VCAP_SERVICES.s3[0]; | ||
| bucketName = name; |
There was a problem hiding this comment.
name here is the cloud.gov service name. We want to set bucketName = credentials.bucket
| secretAccessKey: credentials.secret_access_key, | ||
| signatureVersion: 'v4', | ||
| s3ForcePathStyle: true, | ||
| }; |
There was a problem hiding this comment.
I tested all these paths with the sample data you sent and they all work now.
Co-authored-by: Ryan Ahearn <[email protected]>
…Head-Start-TTADP into cm-pr-267-add-file-upload-api
Description of change
Update file upload api in response to comments on HHS#267
Issue(s)