linagora / james-project

Mirror of Apache James Project
Apache License 2.0
72 stars 62 forks source link

[Multitenant] S3 multitenancy: configuration #5272

Open Arsnael opened 2 months ago

Arsnael commented 2 months ago

Following multitenancy blobstore api refactoring, the first step of multitenancy for S3 would be having a proper configuration, on which every implementation of S3 for multitenancy could rely on.

multi-tenancy.mode=none|bucket|ssec|prefix

DoD:

chibenwa commented 2 months ago

As I said to Florent, tickets purely technical and not delivering features tend to leave half implemented things that do not deliver value. A more reliable way to cut topics - if possible - is to functionally divide tasks so that they all make intrinsecly sense - if possible.

Just the conf tickets fall into the scope of a technical-task split and are suspicious in essence...

While we could get along with this this time, we could spend more time on next groomings to find smarter tasks splits.

Arsnael commented 2 months ago

It does not deliver a feature indeed... Was more in the spirit though to allow quick parallel work with following tickets with avoiding too much complex rebases (the difference with Florent is that he is working alone one ticket after an other, while here a all team could be working on some topics in parallel?)

But I'm listening if you have anything better.

chibenwa commented 2 months ago

But I'm listening if you have anything better.

We shall refrain to use "you".

Sorry I do not have the time to take on the exercise, I'm fine with ticket split provided by the team, but was just making a remark for improvment. I think this is a point we can easily improve on.

Arsnael commented 2 months ago

@quantranhong1999 Proposition: How about conf:

multi-tenancy.enabled=true|false
s3.multi-tenancy.mode=bucket|ssec|prefix

For a general conf:

chibenwa commented 2 months ago

How about

s3.multi-tenancy.mode=ssec,prefix

Coma separated list of multi-tenancy mechanism to apply, none if empty.

Arsnael commented 2 months ago

Why do we want a comma separated list now? Not sure we want multiple modes enabled at the same time?

chibenwa commented 2 months ago

ssec is not compatible with deduplicating blob store because w would end up using different keys to encrypt the same object

However this is not the case when doing sse-c + prefix

That's why I want a coma separated list.