ejabberd icon indicating copy to clipboard operation
ejabberd copied to clipboard

Implement XEP-0363 (HTTP File Upload) for S3-Compatible Storage

Open RomanHargrave opened this issue 3 years ago • 9 comments

This is a functionally complete implementation of the current revision of XEP-0363 (so, only advertising urn:xmpp:http:upload:0) using S3-compatible storage instead of local storage. This way of implementing XEP-0363 drastically simplifies clustered deployments, as it does not require any local state management or persistence, nor any shared storage between nodes. I have tested this with Wasabi.

Currently incomplete are:

  • Module config documentation in docs repo

See #3625 and #3624

RomanHargrave avatar Sep 04 '22 09:09 RomanHargrave

Hi @RomanHargrave, many thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

p1bot avatar Sep 04 '22 09:09 p1bot

You did it @RomanHargrave!

Thank you for signing the ProcessOne Contribution License Agreement.

We will have a look at your contribution!

p1bot avatar Sep 04 '22 09:09 p1bot

Hi! It seems Xref detected a small typo in your code:

ejabbered_router:route_error/2 is undefined function (Xref)

badlop avatar Sep 08 '22 14:09 badlop

Yes, I noticed that last night while reviewing workflow failures. I'll see about it this evening and about getting the request out of draft mode at that point.

RomanHargrave avatar Sep 08 '22 16:09 RomanHargrave

It looks like i need to clean up my type specs for dialyzer, and then we can figure out how compatible we want to make this. OTP 24.3 would be ideal since it's fully compatible, but some shims could get things to 21.3 if needed. Anything before 21.3 is going to need a complete port of uri_string.

RomanHargrave avatar Sep 08 '22 20:09 RomanHargrave

Coverage Status

Coverage decreased (-0.06%) to 33.375% when pulling 1907c26735b2e14493b5d2a6168cce348e866ed4 on RomanHargrave:roman/feat-http-upload-s3 into 3312eaa51dcd48797a068eb3f1786bd133b93d61 on processone:master.

coveralls avatar Sep 09 '22 05:09 coveralls

~~OTP 22 is a pretty hard minimum here (so far) because it adds cryptography APIs upon which this mod depends.~~

RomanHargrave avatar Sep 09 '22 05:09 RomanHargrave

By the way, this module serves a very specific purpose, tied to Amazon S3... maybe this module is better hosted in ejabberd-contrib.

Benefits of providing the module in ejabberd-contrib.git instead of ejabberd.git:

  • the module can be installed easily in any recent ejabberd version, even ones from previous months/years
  • you can apply improvements anytime and the changes are available immediately for anybody to upgrade its module, no need to wait for the next ejabberd release
  • as it isn't included in ejabberd upstream, it doesn't need to satisfy all the requirements expected for code included in ejabberd

Drawbacks of providing the module in ejabberd-contrib:

  • the module must be installed, as it isn't included in ejabberd base (ejabberdctl modules_update_specs && ejabberdctl module_install mod_s3_upload)

BTW; I recently added a new page in ejabberd's WebAdmin to easily view, install and update modules from ejabberd-contrib. It's in ejabberd's WebAdmin -> Nodes -> your node -> Contrib Modules.

I've also added static code testing in ejabberd-contrib (code compilation, xref, and dialyzer). In specific cases like this module, this can be relaxed to test the module only against newer erlang, and explain it in the module README.

In conclusion, once you are happy with your new module, and you consider it's ready to publish/merge, I'll propose you to submit it as an independent module in ejabberd-contrib. From what I know, it requires no change in the source code, and of course I'll help with the dir structure, README or anything else.

badlop avatar Sep 09 '22 09:09 badlop

By the way, this module serves a very specific purpose, tied to Amazon S3

Actually, it will work with many different storage services, since they all implement Amazon's API. I am currently using it with Wasabi, but it can work with Amazon, Linode, Backblaze, and many other managed and self-hosted offerings - probably even Google. For instance, I could run MinIO in my cluster and serve the files myself using this module.

I'd like to get this into ejabberd proper as I think it would be useful, and it looks like a few other people would like it - plus, I would really prefer that my containers start with everything that they need and don't download and run new code after starting. I'll see about making some of the changes to use misc:crypto_hmac and what can be done about other module dependencies. Would you like to aim for 19.3 or something a little newer?

RomanHargrave avatar Sep 09 '22 20:09 RomanHargrave

@RomanHargrave: Can you add it in ejabberd-contrib?

Hope before ejabbberd 22.10...

Neustradamus avatar Oct 25 '22 06:10 Neustradamus

Hope before ejabbberd 22.10...

I would really prefer that my containers start with everything that they need and don't download and run new code after starting.

Since ejabberd 22.10 (concretely in https://github.com/processone/ejabberd/commit/d0bc83147a02805fcd0c6262472bac1bccaa5b63 ) the ejabberd-modules source code is included in the ejabberd container images that can be downloaded from https://github.com/processone/ejabberd/pkgs/container/ejabberd

As the master image is updated for every relevant commit in the ejabberd repository, it usually contains recent ejabberd-modules source code.

The only thing to do when using those new images is to compile and install the contributed module in that container the first time it is started, using CTL_ON_CREATE.

Of course, if that module requires specific configuration, that configuration should be set in a specific configuration file for that module, and that one replaces the default one (using volumes)


For example, this docker-compose.yml registers an account and installs a module the first time the container is started. In the same path of that file I copied ejabberd.yml with some customizations, and mod_statsdx.yml with a configuration different that the default one:

version: '3.7'

services:
  main:
    container_name: ejabberd
    image: ghcr.io/processone/ejabberd:latest
    environment:
      - CTL_ON_CREATE=register admin localhost asd ;
                      module_install mod_statsdx
      - CTL_ON_START=stats registeredusers ;
                     check_password admin localhost asd ;
                     status
    volumes:
      - ejabberd.yml:/opt/ejabberd/conf/ejabberd.yml:ro
      - mod_statsdx.yml:/opt/ejabberd/.ejabberd-modules/sources/ejabberd-contrib/mod_statsdx/conf/mod_statsdx.yml:ro
    ports:
      - "5222:5222"
      - "5269:5269"
      - "5280:5280"
      - "5443:5443"

mod_statsdx.yml

modules:
  mod_statsdx:
    hooks: true

When the container is created, after ejabberd starts, it installs the module with my desired configuration. No need to download anything at all. Of course, I can update the contributed modules source code with modules_update_specs anyatime manually with ejabberdctl, or adding it in CTL_ON_CREATE or CTL_ON_START.

badlop avatar Oct 31 '22 13:10 badlop

@RomanHargrave: Good job, thanks, your PR has been merged in ejabberd-contrib!

Linked to:

  • https://github.com/processone/ejabberd-contrib/pull/314
  • https://github.com/processone/ejabberd-contrib/issues/305
  • https://github.com/processone/ejabberd/issues/3625
  • https://github.com/processone/ejabberd/issues/3624

Neustradamus avatar Dec 29 '22 22:12 Neustradamus

I would really prefer that my containers start with everything that they need and don't download and run new code after starting.

The ejabberd container includes ejabberd-contrib, and since this recent commit it is possible to compile and install them automatically in the very first ejabberd start, simply configure in ejabberd.yml:

install_contrib_modules:
  - mod_s3_upload

modules:
  mod_s3_upload:
    ...

badlop avatar Jun 16 '23 08:06 badlop