feat: partition assets into multiple tenants#217
Merged
Conversation
483acbb to
2725726
Compare
b8ad837 to
d09d193
Compare
Pull Request Test Coverage Report for Build 4533387823Warning: This coverage report may be inaccurate.We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
💛 - Coveralls |
bbc2403 to
f7eed68
Compare
pyadav
reviewed
Mar 9, 2023
| } | ||
| } | ||
| if len(namespace.State) > 0 { | ||
| existingNamespace.State = namespace.State |
Member
There was a problem hiding this comment.
We should not allow changing state till automation is not available for migrating one state to another.
Member
Author
There was a problem hiding this comment.
Agree so I have not added namespace update call in cli but I feel that instead of leaving this behind in the API, we can still have it to make it complete. What do you think?
pyadav
reviewed
Mar 10, 2023
1be5c19 to
d29887d
Compare
pyadav
reviewed
Mar 12, 2023
b413726 to
df13bbb
Compare
d71169f to
951cd53
Compare
Signed-off-by: Kush Sharma <thekushsharma@gmail.com>
RLS requires user used for application database connection should not be table owner and a superuser else all RLS are bypassed by default. That means a user that is migrating the application and a user that is used for serving the app should both be different. Signed-off-by: Kush Sharma <thekushsharma@gmail.com>
951cd53 to
c7e0b27
Compare
Signed-off-by: Kush Sharma <thekushsharma@gmail.com>
5100a2b to
5e8cf80
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a breaking change, all existing applications using compass will have to modify their contract to include a namespace. The elasticsearch index strategy has been changed, all the data will need to be re-indexed again. The user flow would be as follows:
postgresand depending on if this tenant issharedordedicatedanindexand analiaswill be created inelasticsearch.assetwill need to pass the namespace id.Get/Listall registered namespaces. One thing to note is, as part of the migration step, compass creates adefaultnamespace to avoid the hassle of bootstrapping compass use-case.Changes are made based on design discussion at #208. API changes are available in raystack/proton#246
To enforce multi-tenant restrictions at the database level, I have used Row Level Security. RLS requires Postgres users used for application database connection not to be a
table owneror asuperuserelse all RLS are bypassed by default. That means a Postgresuserthat is migrating the application and a user that is used to serve the app should both be different.To create a postgres user
A middware for grpc looks for
x-namespace-idheader to extract tenant id if not found falls back todefaultnamespace. Same could be passed in a jwt token of Auth bearer.