-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove ftp functionality #2064
Remove ftp functionality #2064
Conversation
https://eaflood.atlassian.net/browse/IWTF-4278 Remove ftp functionality as package we use in fulfilment and pocl jobs (ssh2-sftp-client) has critical vulnerability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that's the kind of line count diff I like to see, hahaha.
One suggested change to do with the DD process.
packages/pocl-job/src/io/s3.js
Outdated
const dynamicsRecord = await salesApi.getTransactionFile(filename) | ||
if (!dynamicsRecord || !DYNAMICS_IMPORT_STAGE.isAlreadyProcessed(dynamicsRecord.status.description)) { | ||
await storeS3Metadata(file.ETag, filesize(file.Size), filename, file.Key, moment(new Date(file.LastModified))) | ||
} else { | ||
console.log(`${file.Key} is already processed, skipping`) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we want to delete this and the storeS3Metadata
function – I think this can still be used for other file uploads as well and we don't want to blow up the direct debit process. However if we do keep it then it might make sense to move that function out of a file called ftp-to-s3.js anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but let's wait until Iris is back and can re-review than the s3 changes have been made to her satisfaction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One little thing to change
packages/pocl-job/src/config.js
Outdated
'3des-cbc', | ||
'cast128-cbc' | ||
] | ||
|
||
class Config { | ||
_db | ||
_ftp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is needed anymore either, is it?
Quality Gate passedIssues Measures |
https://eaflood.atlassian.net/browse/IWTF-4278
Remove ftp functionality as package we use in fulfilment and pocl jobs (ssh2-sftp-client) has critical vulnerability