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

[Multitenant] PGSQL multitenancy #5271

Open
Arsnael opened this issue Sep 18, 2024 · 12 comments
Open

[Multitenant] PGSQL multitenancy #5271

Arsnael opened this issue Sep 18, 2024 · 12 comments

Comments

@Arsnael
Copy link
Member

Arsnael commented Sep 18, 2024

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 Arsnael changed the title [S3] PGSQL multitenancy [Multitenant] PGSQL multitenancy Sep 18, 2024
@Arsnael Arsnael changed the title [Multitenant] PGSQL multitenancy [Multitenant] PGSQL multitenancy: bucket Sep 18, 2024
@Arsnael Arsnael changed the title [Multitenant] PGSQL multitenancy: bucket [Multitenant] PGSQL multitenancy Sep 18, 2024
@Arsnael
Copy link
Member Author

Arsnael commented Sep 18, 2024

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
Copy link
Member

Or do we imply it works if rls is enabled?

Good point.

@Arsnael
Copy link
Member Author

Arsnael commented Sep 20, 2024

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

@quantranhong1999
Copy link
Member

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
Copy link
Member

vttranlina commented Sep 20, 2024

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
Copy link
Member

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
Copy link
Member Author

Arsnael commented Sep 24, 2024

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
Copy link
Member

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

@Arsnael
Copy link
Member Author

Arsnael commented Sep 26, 2024

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

@Arsnael
Copy link
Member Author

Arsnael commented Sep 26, 2024

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

@chibenwa
Copy link
Member

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
Copy link
Member Author

Arsnael commented Sep 26, 2024

Ok fair point

@Arsnael Arsnael removed the grooming label Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants