linagora / james-project

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

[Multitenant] PGSQL multitenancy #5271

Open Arsnael opened 2 months ago

Arsnael commented 2 months ago

After the refactoring of the base API (BlobStore and BlobStoreDAO), you need to refactor PostgresBlobStoreDAO to implement the new methods taking multitenancy per bucket into account.

DoD: Integration tests still green

Arsnael commented 2 months ago

Question: Does postgres implem actually does it already? I dont remember it doing it, and trying to look into the code I dont see any of it either. It looks like to me everything goes into the same default bucket name.

Or do we imply it works if rls is enabled? But even then it's more of a differentiation between domains and not really bucket name, correct?

quantranhong1999 commented 2 months ago

Or do we imply it works if rls is enabled?

Good point.

Arsnael commented 2 months ago

Except I just double checked and rls is disabled for that table

quantranhong1999 commented 2 months ago

Except I just double checked and rls is disabled for that table

Not sure why we did that. Maybe we can try to see if we can enable RLS for that table?

vttranlina commented 2 months ago

Not sure why we did that.

Currently, The blobStoreDAO API doen't have domain parameter input.

I remember we prefer rls for only mailbox module.

Maybe we can try to see if we can enable RLS for that table?

It can be able when input parameter is Bucket(Bucketname, Optional)

chibenwa commented 2 months ago

Except I just double checked and rls is disabled for that table Not sure why we did that. Maybe we can try to see if we can enable RLS for that table?

Exactly the "domain" information was not caried over.

I remember isolation was achieved by the use of distincts bockets...

Arsnael commented 1 month ago

I remember isolation was achieved by the use of distincts bockets...

Would like you to point out where then cause nobody in the team seems able to find that piece of code honestly ^^'

chibenwa commented 1 month ago

Ok after a quick code review I confirm this is unimplemented...

Arsnael commented 1 month ago

Configuration needed to enable/disable multitenancy per bucket or not for pgsql?

Arsnael commented 1 month ago

How about just enabling RLS with this table to achieve isolation per domain (aka tenant)?

chibenwa commented 1 month ago

How about just enabling RLS with this table to achieve isolation per domain (aka tenant)?

And what do we do for places that might store in the blobstore without a clearly identified domain IE the mailQueue?

Arsnael commented 1 month ago

Ok fair point