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.

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

Add support for saving files to AWS S3 #113

Merged
merged 2 commits into from
Feb 1, 2016
Merged

Add support for saving files to AWS S3 #113

merged 2 commits into from
Feb 1, 2016

Conversation

skinp
Copy link
Contributor

@skinp skinp commented Feb 1, 2016

Using Mongo's GridStore for file storage is fine for basic usage, but it won't scale much. Heavy production users should use a proper file/object storage system. Let's at least officially add support for S3 storage for Parse files.

To use this:

var api = new ParseServer({
  ...
  filesAdapter: new S3Adapter(
    "<AWS_ACCESS_KEY>",
    "<AWS_SECRET_ACCESS_KEY>",
    {bucket: "<BUCKET_NAME>", bucketPrefix: "files/", directAccess: true}
  ),
  ...
});

Once this is accepted, I'll add docs to the wiki on how to use this properly and explain all the available options.

// For a given config object, filename, and data, store a file in S3
// Returns a promise containing the S3 object creation response
S3Adapter.prototype.create = function(config, filename, data) {
var self = this;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm curious if this is the best/most idiomatic way of doing this. I couldn't reference this object within the Promise function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, the AWS node library can load it's config from a JS object. I'm debating whether the S3Adapter should just take such an object as parameter or something. That would potentially allow more configuration of the S3 object, although would probably make it less errr simple. Again, what's more idiomatic?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to access this, as ExportAdapter uses it heavily...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok discussed offline, apparently ES6 arrow function (=>) is the way to go

@skinp skinp mentioned this pull request Feb 1, 2016
@lucianmat
Copy link

👍

@skinp
Copy link
Contributor Author

skinp commented Feb 1, 2016

hmmm I clearly messed up something in that rebase. Let me try to fix this

@skinp
Copy link
Contributor Author

skinp commented Feb 1, 2016

Ok, I fixed it. Should be good to merge when accepted.

gfosco added a commit that referenced this pull request Feb 1, 2016
Add support for saving files to AWS S3
@gfosco gfosco merged commit 3191793 into parse-community:master Feb 1, 2016
@edvincandon
Copy link

@skinp looking forward to your wiki/docs on your AWS implementation, can't seem to get it working..

@skinp
Copy link
Contributor Author

skinp commented Feb 1, 2016

@edvincandon the S3Adapter object is not exposed directly just yet. If you're trying to use my example from earlier, you need one more require line to get access to S3Adapter object:

var S3Adapter = require('parse-server/S3Adapter');

Docs to come, and this object should be exposed in some next versions.

@skinp skinp deleted the s3-files branch February 1, 2016 22:35
@nitrag
Copy link

nitrag commented Feb 1, 2016

@skinp This is great! Looking forward to a guide!

@edvincandon
Copy link

Hey @skinp - Stil trying to figure out what's wrong,
FilesAdapter is catching an error and throwing me this on the client side.
{"code":130,"error":"Could not store file."}

Server-side logs empty for this issue.

I'm fairly new to S3 and the AWS SDK, but I saw they had their own error handling codes and messages - would it be possible to pass those to Parse.Error instead of just throwing this in files.js :

...
}).catch((error) => {
next(new Parse.Error(Parse.Error.FILE_SAVE_ERROR,
'Could not store file.'));
});

Would be a nice implementation for debugging purposes, no?
Thanks for your time!

EDIT - Error was on my side with my bucket configuration.

@skinp
Copy link
Contributor Author

skinp commented Feb 2, 2016

@edvincandon glad you got it figured out. Yeah I was printing that error when developing this module but the errors coming from the AWS SDK ended up being too verbose. Maybe we could print them to the server log instead. I'm going to first write docs for this. Let's see if having clear docs helps reduce the amount of errors/confusion people get with this Adapter and we'll see after if we need better errors/logging.

@leobortolotti
Copy link

@edvincandon I'm having the same error here
[Error]: Could not store file. (Code: 130, Version: 1.12.0)
Could you tell us what was the problem you were having in the bucket configuration?
Thanks

@edvincandon
Copy link

@leobortolotti
I had to edit the bucket policies on S3 for it to work. Use the bucket policy generator on AWS (found under Properties > Permissions once you select your target bucket on AWS or http://awspolicygen.s3.amazonaws.com/policygen.html )

Next select S3 Bucket Policy as your desired type of policy
Select Bucket Policy
Enter * in the Principal field,
SelectGetObject as your desired Action
Enter your Amazon Resource Name -> Your resource name for a S3 bucket : arn:aws:s3:::<YOUR_BUCKET_NAME>/*

Generate the policy and add it to your bucket!

screen shot 2016-02-03 at 19 37 15

Hope it helps! Also make sure your bucket is CORS enabled, your CORS configuration should look something like this:

<CORSConfiguration>
<CORSRule>
<AllowedOrigin>*</AllowedOrigin>
<AllowedMethod>GET</AllowedMethod>
<MaxAgeSeconds>3000</MaxAgeSeconds>
<AllowedHeader>Authorization</AllowedHeader>
</CORSRule>
</CORSConfiguration>

@leobortolotti
Copy link

@edvincandon Thank you very much!!!
I did everything you said, but still getting the error, seems that this problem was one but not the only that I have to solve!! hahaha

@edvincandon
Copy link

Are you using the latest parse-server release? If not, try updating..
Also, you're probably inserting that Parse.file in some object, try deleting the _schema for that particular object (worked for me..)

@leobortolotti
Copy link

I did this but I'm still getting this error! But thanks again!
I think that I will wait for the Docs Guide...

@leobortolotti
Copy link

Ohhhh, I got it!!! Seems like my Access Key and Secret Key was not working/inactive, I don't know why!!
I generated new ones and now it is working!! Thanks man!!!

@davidruisinger
Copy link

As far as I understand the code this does not support deleting the file via REST API, right?
Any plans on implementing the delete functionality as well?

In my setup I'm deleting files using Parse Cloud:

  Parse.Cloud.httpRequest({
    method: 'DELETE',
    url: imageUrl,
    headers: {
      "X-Parse-Application-Id": "******"
    },
    success: function(httpResponse) {
      console.log('Delete succeeded  ' + httpResponse.text);
    },
    error: function(httpResponse) {
      console.log('error');
    }
  });

which returns an Access Denied error.

@gfosco
Copy link
Contributor

gfosco commented Feb 11, 2016

@flavordaaave Delete will be implemented in #354 and will require the master key.

@facebook-github-bot
Copy link

@skinp updated the pull request.

@davidruisinger
Copy link

@gfosco still getting an Access Denied message with the latest update (2.0.8):

  Parse.Cloud.httpRequest({
    method: 'DELETE',
    url: imageUrl,
    headers: {
      "X-Parse-Application-Id": "******",
      "X-Parse-Master-Key": "******"
    },
    success: function(httpResponse) {
      console.log('Delete succeeded  ' + httpResponse.text);
    },
    error: function(httpResponse) {
      console.log('error');
    }
  });

@facebook-github-bot
Copy link

@skinp updated the pull request.

bernhardharrer pushed a commit to bernhardharrer/parse-server that referenced this pull request Feb 22, 2017
Updating to use latest parse-server and parse sdk for Live Query launch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants