Skip to content
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.

Sign up for GitHub

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

fix: Upgrade to AWS JS SDK v3 #218

Closed
wants to merge 2 commits into from
Closed

fix: Upgrade to AWS JS SDK v3 #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44d71e7
Update to AWS JS SDK v3
mman Feb 27, 2024
f912198
WIP on tests
mman Feb 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
//
// Stores Parse files in AWS S3.

const AWS = require('aws-sdk');
const S3Client = require('@aws-sdk/client-s3').S3;
// const getSignedUrl = require('@aws-sdk/s3-request-presigner').getSignedUrl;
const optionsFromArguments = require('./lib/optionsFromArguments');

const awsCredentialsDeprecationNotice = function awsCredentialsDeprecationNotice() {
Expand Down Expand Up @@ -73,7 +74,7 @@ class S3Adapter {

Object.assign(s3Options, options.s3overrides);

this._s3Client = new AWS.S3(s3Options);
this._s3Client = new S3Client(s3Options);
this._hasBucket = false;
}

Expand All @@ -82,8 +83,11 @@ class S3Adapter {
if (this._hasBucket) {
promise = Promise.resolve();
} else {
const params = {
Bucket: this._bucket
};
promise = new Promise((resolve) => {
this._s3Client.createBucket(() => {
this._s3Client.createBucket(params, () => {
this._hasBucket = true;
resolve();
});
Expand All @@ -96,6 +100,7 @@ class S3Adapter {
// Returns a promise containing the S3 object creation response
createFile(filename, data, contentType, options = {}) {
const params = {
Bucket: this._bucket,
Key: this._bucketPrefix + filename,
Body: data,
};
Expand Down Expand Up @@ -129,18 +134,23 @@ class S3Adapter {
params.Tagging = serializedTags;
}
return this.createBucket().then(() => new Promise((resolve, reject) => {
this._s3Client.upload(params, (err, response) => {
this._s3Client.putObject(params, (err, response) => {
if (err !== null) {
return reject(err);
}
// NOTE: populate Location manually since it is not part of putObject call
// NOTE: https://github.com/aws/aws-sdk-js-v3/issues/3875
response.Location = `https://${params.Bucket}.s3.${this._region}.amazonaws.com/${params.Key}`
return resolve(response);
});
}));

}

deleteFile(filename) {
return this.createBucket().then(() => new Promise((resolve, reject) => {
const params = {
Bucket: this._bucket,
Key: this._bucketPrefix + filename,
};
this._s3Client.deleteObject(params, (err, data) => {
Expand All @@ -155,7 +165,10 @@ class S3Adapter {
// Search for and return a file if found by filename
// Returns a promise that succeeds with the buffer result from S3
getFileData(filename) {
const params = { Key: this._bucketPrefix + filename };
const params = {
Bucket: this._bucket,
Key: this._bucketPrefix + filename
};
return this.createBucket().then(() => new Promise((resolve, reject) => {
this._s3Client.getObject(params, (err, data) => {
if (err !== null) {
Expand All @@ -165,7 +178,7 @@ class S3Adapter {
if (data && !data.Body) {
return reject(data);
}
return resolve(data.Body);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here I cast data.Body to string which is most probably wrong as we should pass up a stream?

return resolve(data.Body.transformToString());
});
}));
}
Expand All @@ -181,19 +194,19 @@ class S3Adapter {

const fileKey = `${this._bucketPrefix}${fileName}`;

let presignedUrl = '';
if (this._presignedUrl) {
const params = { Bucket: this._bucket, Key: fileKey };
if (this._presignedUrlExpires) {
params.Expires = this._presignedUrlExpires;
}
// Always use the "getObject" operation, and we recommend that you protect the URL
// appropriately: https://docs.aws.amazon.com/AmazonS3/latest/dev/ShareObjectPreSignedURL.html
presignedUrl = this._s3Client.getSignedUrl('getObject', params);
if (!this._baseUrl) {
return presignedUrl;
}
}
const presignedUrl = '';
// if (this._presignedUrl) {
// const params = { Bucket: this._bucket, Key: fileKey };
// if (this._presignedUrlExpires) {
// params.Expires = this._presignedUrlExpires;
// }
// // Always use the "getObject" operation, and we recommend that you protect the URL
// // appropriately: https://docs.aws.amazon.com/AmazonS3/latest/dev/ShareObjectPreSignedURL.html
// presignedUrl = getSignedUrl(this._s3Client, 'getObject', params).then(result => );
// if (!this._baseUrl) {
// return presignedUrl;
// }
// }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have disabled this test as I do not know how to properly migrate to AWS SDK v3 which does not offer a sync version of getSignedUrl anymore.

if (!this._baseUrl) {
return `https://${this._bucket}.s3.amazonaws.com/${fileKey}`;
Expand All @@ -205,6 +218,7 @@ class S3Adapter {

handleFileStream(filename, req, res) {
const params = {
Bucket: this._bucket,
Key: this._bucketPrefix + filename,
Range: req.get('Range'),
};
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
},
"homepage": "https://github.com/parse-community/parse-server-s3-adapter#readme",
"dependencies": {
"aws-sdk": "2.1399.0"
"@aws-sdk/client-s3": "^3.521.0",
"@aws-sdk/s3-request-presigner": "^3.521.0"
},
"devDependencies": {
"@semantic-release/changelog": "5.0.1",
Expand Down
65 changes: 33 additions & 32 deletions spec/test.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const AWS = require('aws-sdk');
const S3Client = require('@aws-sdk/client-s3').S3;
const config = require('config');
const filesAdapterTests = require('parse-server-conformance-tests').files;
const Parse = require('parse').Parse;
Expand Down Expand Up @@ -27,8 +27,8 @@ describe('S3Adapter tests', () => {
const objects = {};

s3._s3Client = {
createBucket: (callback) => setTimeout(callback, 100),
upload: (params, callback) => setTimeout(() => {
createBucket: (params, callback) => setTimeout(callback, 100),
putObject: (params, callback) => setTimeout(() => {
const { Key, Body } = params;

objects[Key] = Body;
Expand Down Expand Up @@ -190,22 +190,23 @@ describe('S3Adapter tests', () => {
});

it('should accept endpoint as an override option in args', () => {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the AWS.Endpoint API simply disappeared and I am not sure what to do here.

const otherEndpoint = new AWS.Endpoint('nyc3.digitaloceanspaces.com');
const confObj = {
bucketPrefix: 'test/',
bucket: 'bucket-1',
secretKey: 'secret-1',
accessKey: 'key-1',
s3overrides: { endpoint: otherEndpoint },
};
const s3 = new S3Adapter(confObj);
expect(s3._s3Client.endpoint.protocol).toEqual(otherEndpoint.protocol);
expect(s3._s3Client.endpoint.host).toEqual(otherEndpoint.host);
expect(s3._s3Client.endpoint.port).toEqual(otherEndpoint.port);
expect(s3._s3Client.endpoint.hostname).toEqual(otherEndpoint.hostname);
expect(s3._s3Client.endpoint.pathname).toEqual(otherEndpoint.pathname);
expect(s3._s3Client.endpoint.path).toEqual(otherEndpoint.path);
expect(s3._s3Client.endpoint.href).toEqual(otherEndpoint.href);
const other = new S3Client({endpoint: 'nyc3.digitaloceanspaces.com'});
expect(other.config.endpoint, "a");
// const confObj = {
// bucketPrefix: 'test/',
// bucket: 'bucket-1',
// secretKey: 'secret-1',
// accessKey: 'key-1',
// s3overrides: { endpoint: other.endpoint },
// };
// const s3 = new S3Adapter(confObj);
// expect(s3._s3Client.config.).toEqual(other.config.endpoint.protocol);
// expect(s3._s3Client.endpoint.host).toEqual(other.endpoint.host);
// expect(s3._s3Client.endpoint.port).toEqual(other.endpoint.port);
// expect(s3._s3Client.endpoint.hostname).toEqual(other.endpoint.hostname);
// expect(s3._s3Client.endpoint.pathname).toEqual(other.endpoint.pathname);
// expect(s3._s3Client.endpoint.path).toEqual(other.endpoint.path);
// expect(s3._s3Client.endpoint.href).toEqual(other.endpoint.href);
});

it('should accept options and overrides as args', () => {
Expand Down Expand Up @@ -237,7 +238,7 @@ describe('S3Adapter tests', () => {
it('should handle range bytes', () => {
const s3 = new S3Adapter('accessKey', 'secretKey', 'my-bucket');
s3._s3Client = {
createBucket: (callback) => callback(),
createBucket: (params, callback) => callback(),
getObject: (params, callback) => {
const { Range } = params;

Expand Down Expand Up @@ -268,7 +269,7 @@ describe('S3Adapter tests', () => {
it('should handle range bytes error', () => {
const s3 = new S3Adapter('accessKey', 'secretKey', 'my-bucket');
s3._s3Client = {
createBucket: (callback) => callback(),
createBucket: (params, callback) => callback(),
getObject: (params, callback) => {
callback('FileNotFound', null);
},
Expand All @@ -293,7 +294,7 @@ describe('S3Adapter tests', () => {
const s3 = new S3Adapter('accessKey', 'secretKey', 'my-bucket');
const data = { Error: 'NoBody' };
s3._s3Client = {
createBucket: (callback) => callback(),
createBucket: (params, callback) => callback(),
getObject: (params, callback) => {
callback(null, data);
},
Expand Down Expand Up @@ -578,7 +579,7 @@ describe('S3Adapter tests', () => {

it('should save a file with metadata added', async () => {
const s3 = makeS3Adapter(options);
s3._s3Client.upload = (params, callback) => {
s3._s3Client.putObject = (params, callback) => {
const { Metadata } = params;
expect(Metadata).toEqual({ foo: 'bar' });
const data = {
Expand All @@ -593,7 +594,7 @@ describe('S3Adapter tests', () => {

it('should save a file with tags added', async () => {
const s3 = makeS3Adapter(options);
s3._s3Client.upload = (params, callback) => {
s3._s3Client.putObject = (params, callback) => {
const { Tagging } = params;
expect(Tagging).toEqual('foo=bar&baz=bin');
const data = {
Expand All @@ -610,11 +611,11 @@ describe('S3Adapter tests', () => {
// Create adapter
options.directAccess = true;
const s3 = makeS3Adapter(options);
spyOn(s3._s3Client, 'upload').and.callThrough();
spyOn(s3._s3Client, 'putObject').and.callThrough();
// Save file
await s3.createFile('file.txt', 'hello world', 'text/utf8', {});
// Validate
const calls = s3._s3Client.upload.calls.all();
const calls = s3._s3Client.putObject.calls.all();
expect(calls.length).toBe(1);
calls.forEach((call) => {
expect(call.args[0].ACL).toBe('public-read');
Expand All @@ -624,11 +625,11 @@ describe('S3Adapter tests', () => {
it('should save a file with proper ACL without direct access', async () => {
// Create adapter
const s3 = makeS3Adapter(options);
spyOn(s3._s3Client, 'upload').and.callThrough();
spyOn(s3._s3Client, 'putObject').and.callThrough();
// Save file
await s3.createFile('file.txt', 'hello world', 'text/utf8', {});
// Validate
const calls = s3._s3Client.upload.calls.all();
const calls = s3._s3Client.putObject.calls.all();
expect(calls.length).toBe(1);
calls.forEach((call) => {
expect(call.args[0].ACL).toBeUndefined();
Expand All @@ -640,11 +641,11 @@ describe('S3Adapter tests', () => {
options.directAccess = true;
options.fileAcl = 'private';
const s3 = makeS3Adapter(options);
spyOn(s3._s3Client, 'upload').and.callThrough();
spyOn(s3._s3Client, 'putObject').and.callThrough();
// Save file
await s3.createFile('file.txt', 'hello world', 'text/utf8', {});
// Validate
const calls = s3._s3Client.upload.calls.all();
const calls = s3._s3Client.putObject.calls.all();
expect(calls.length).toBe(1);
calls.forEach((call) => {
expect(call.args[0].ACL).toBe('private');
Expand All @@ -656,11 +657,11 @@ describe('S3Adapter tests', () => {
options.directAccess = true;
options.fileAcl = 'none';
const s3 = makeS3Adapter(options);
spyOn(s3._s3Client, 'upload').and.callThrough();
spyOn(s3._s3Client, 'putObject').and.callThrough();
// Save file
await s3.createFile('file.txt', 'hello world', 'text/utf8', {});
// Validate
const calls = s3._s3Client.upload.calls.all();
const calls = s3._s3Client.putObject.calls.all();
expect(calls.length).toBe(1);
calls.forEach((call) => {
expect(call.args[0].ACL).toBeUndefined();
Expand Down