Storage Adaptor for S3 only supports Amazon S3: Please add some options for the S3 Client

[osseonews]

The S3 adapter appears to assume only AWS S3. But there are may S3 compatible storage providers, like Cloudflare R2 and Digital Ocean Spaces (which we use) that will work with the @aws-sdk/client-s3, but just require the necessary options. The S3 Client set up in the plugin doesn't include the necessary lines for these compatible storage providers. For for example, when used with DigitalOcean Spaces, setupLifecycle() fails because the client does not support configuring a custom endpoint and performs AWS-specific bucket lifecycle operations. Could the adapter support an optional endpoint configuration?

The fix is really simple, in your code, you just need to add 2 new options for the S3 Client:

your Code:

this.s3 = new S3Client({ region: this.options.region, credentials: { accessKeyId: this.options.accessKeyId, secretAccessKey: this.options.secretAccessKey, }, })

Just add 2 options, (see below)

Add the other necessary options for Digital Ocean Spaces:

1. endpoint and
2. forcePathStyle.

An S3 client config for Digital Ocean looks like this basically

private constructor(config: S3Config) {
    

        this.config = {
            endpoint: config.endpoint, //you are mising this!
            accessKeyId: config.accessKeyId,
            secretAccessKey: config.secretAccessKey,
            bucket: config.bucket,
        };

        this.s3 = new S3Client({
            endpoint: this.config.endpoint,
            region: "us-east-1", // required even for S3-compatible services - can store in ENV
            credentials: {
                accessKeyId: this.config.accessKeyId,
                secretAccessKey: this.config.secretAccessKey,
            },
            forcePathStyle: true, // important for many S3-compatible providers
        });
    }

[osseonews]

BTW, I noticed other parts of the code that are just for Amazon S3 and not for S3 compatible storage, like Cloudflare R2, Digital Ocean, Supabase etc. Probably need to rewrite the plugin for a more generalized use case.

[ivictbor]

Well, good issue. We thought about it many times and still in roadmap.

Yes, passing custom endpoint to different s3-compatible API is good idea. But main blocker is that many s3-compatble APIs don't support all features which we need. For this adapter, to work correctly it is very important to support tagging and lifecycle removal.
This allows in plugins, which use adapter to have best UX and keep storage clean and synced. For example upload plugin allows to upload file with pre-signed upload URL, and puts tag which allows to cleanup an object (later within some period) if user will not save the record linking to this object to database. Same for record updates. When user will click save button, cleanup tag will be removed.

• API does not support tagging + lifecycle cleanup we need some deep emulation by storing path-to-tags linking in dedicated storage. This is less reliable, cause any manual innervation in object storage can cause desync, but I think we can handle it

For example: R2 https://developers.cloudflare.com/r2/api/s3/api/#unimplemented-object-level-operations has neither tagging support nor lifecycle rules

MiniIO for example supports tags and manual cleanup with CLI, like:

mc ilm rule add \
  --tags "cleanup=true" \
  --expire-days 7 \
  myminio/mybucket

So DevOps will have more work to setup it, but I think this is good.

For tagging emulation I think we can implement it, but in this mode this adapter will have to consume other KV adapter, e.g. leveldb.

[osseonews]

Yeah, it's a lot of work to maintain all the S3 compatible storage providers. I suggest maybe you just use this library instead for the plugin: https://flydrive.dev/docs/introduction - we use it already for a long time and its maintained by the Adonisjs team, so its really good. Probably will be faster for your team to just use this as a dependency for non-Amazon S3 stuff than code everything again.

[ivictbor]

From what I see FlyDrive also has no own emulation for tags so it will not help unfortunately. They allow to proxy custom options to s3 but again non-s3 will never support it and all our concept will brake. They indeed give high-level service-agnostic abstraction but for very primitive operations as file upload/read (at least from my first vision on their docs, maybe you know more). What we need is to have ability to "attach" some metadata to object to mark it for postponed cleanup and do efficient filter on such objects with expiration attribute to wipe them quickly and keep storage synced and not waste money on storing unneeded items.

Anyway our adapter has very simple contract so you can basically implement anything quickly - even without publishing - this should be just a class which implements several methods. So you can try to do adminforth-storage-adapter-flydrive and try solving mentioned issues in some way.

I think our roadmap will stick to tagging emulation for s3-compatible APIs, as a dedicated adapter, as I mentioned earlier, using additional key-value adapter on top of, we already discussed it with @yaroslav8765 today, it should be quick and acceptable solution which unblocks R2, DO, MiniIO - anything. So we will get only minimal things from storage and store metadata and do cleanup on our side

[osseonews]

Yeah, I haven't read the FlyDrive code at all, we just use it as it integrates smoothly with Digital Ocean and R2 which we use. but it may not do what you need to do or conflict with your code, which I didn't read too carefully either. I guess the goal is to just have a kind of generic API that can be used with various S3 compatible image providers, and not just AWS. These other providers are really popular. Something like Supabase for files is also big and the other day Neon announced an S3 compatible storage service.

[ivictbor]

When services like R2, MiniIO or other implement s3-compatible API they already implement contracts same to AWS, so existing JS AWS SDK or Python boto is already generic way to be connected to any s3-compatible API. The issue is not compatibility of methods which they implement but that they don't implement full set of APIs and endpoints. Generaly they stop only on very common things like put/get/list object, this is enough for 99% of use-cases. What we do is using rare tagging features, which are free from AWS PoV anyway so very handy and normalized way to attach some info to record...

Anyway @yaroslav8765 already implemented emulation using our KV adapter (for now level-db based is supported) and working on documentation and tests right now. R2 already tested and working perfectly

[yaroslav8765]

Here is docs for the S3 compatible adapter: https://adminforth.dev/docs/tutorial/Adapters/storage-adapters/

It wasn't tested on production projects, but should works fine

[osseonews]

Thanks. I read the docs. Unfortunately, this won't work for us on the Digital Ocean App platform and probably won't work on any of the popular providers like Render, as you are using some sort of memory storage for the keys? docs say: "Since levelDb is used for storing keys ...docker volume on deployed application, so you will not loose this data after re-deploy." but that's not how services like Digital Ocean work. All data will be lost on redeploy. What is this levelDB for anyway? What are you storing exactly? It's not clear from the docs.

[osseonews]

In your types in your code for the cleanupkey you have: "A separate scheduler/worker is expected to process these markers later." This is very confusing and seems to be adding complexity. We don't want to run a worker just to upload a file to an S3 storage. why is this key thing needed and why do we need to clean this up? I don't see this cleanupkey thing in AWS adaptor, or maybe I'm missing where it is called? I just don't understand what the point of this is.

[ivictbor]

on the Digital Ocean App platform and probably won't work on any of the popular providers like Render

Yes, leveldb adapter now makes container stateful and would loose on redeploy if your service does not support persisted volume mount for one sub-path like in many docker based services like ECS.
Then you will need another persisted remote KV adapter. We can do it I think on top of DB resource

Price of loosing this storage is not super-high - actually if you loose the state only thing that happen that it will not be able to cleanup stale uploaded files within last 10 days. So you might potentially reach the state where you pay extra $ for files which were uploaded in create/upload form but where not yet commited.

A separate scheduler/worker is expected to process these markers later
This is very confusing
We don't want to run a worker just

You don't need to do anything from your side, this is internal scheduler called automatically internally, you can restart/redeploy your app as many times as you want - this scheduler will do everything within node process. Also even you your process is not running 24/7 - it will still work fine, but will do cleanup later.

"A separate scheduler/worker is expected to process these markers later"

I think this message confused you a lot, we will remove it or rework it. Thanks

I don't see this cleanupkey thing in AWS adaptor

As I explained above we don't need this internal scheduler in AWS adapter because AWS has lifecycle policy inside, so this scheduler is 1 to 1 emulation of AWS lifecycle behavior.

[osseonews]

Ah, ok, I think I understand what you are doing now. You are afraid that if a user uploads a file, but doesn't hit submit, then they lose that file, so you save the file in some sort of local storage and allow them to upload later? Is that why you have this and AWS handles this type of thing on its own via lifecycle? In that case, I don't really think this is necessary from a user perspective, if we are using a S3 compatible provider. I've never actually run into a situation like this ever with any user. They upload and hit submit. So if making this key/value optional won't break anything in your system (I don't know), then I would make this key/value store optional and not require it with the plugin. It's really just a nice to have feature, not something essential, if I'm understanding it correctly.

[ivictbor]

Ah, ok, I think I understand what you are doing now. You are afraid that if a user uploads a file, but doesn't hit submit, then they lose that file, so you save the file in some sort of local storage and allow them to upload later? Is that why you have this and AWS handles this type of thing on its own via lifecycle? In that case, I don't really think this is necessary from a user perspective, if we are using a S3 compatible provider. I've never actually run into a situation like this ever with any user. They upload and hit submit. So if making this key/value optional won't break anything in your system (I don't know), then I would make this key/value store optional and not require it with the plugin. It's really just a nice to have feature, not something essential, if I'm understanding it correctly.

Here is detailed explanation https://tracklify.com/video/3fbd1207-2a13-4ede-a61a-11b796c27c4d