-
Notifications
You must be signed in to change notification settings - Fork 392
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
[#2236] feat(core): Add the support of Group entity #2735
Conversation
|
||
/** The interface of a Group. The Group is the entity which contains users. */ | ||
@Evolving | ||
public interface Group extends Auditable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does Gravitino know which user belongs to which group?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get the groups by using user name in the AuthorizationFilter. We can use user name to retrieve the groups from the external user system like LDAP, RANGER, IAM.
I will add GroupMappingServiceProvider
for accomplish this feature.
interface GroupMappingServiceProvider {
List<String> getGroups(String user);
}
* manage group, just sets up the relationship between the metalake and the group. Metalake is like | ||
* a concept of the organization. `AddGroup` means that a group enter an organization. | ||
*/ | ||
public class GroupManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can merge the User and GroupManager together as "UserGroupManager", WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
What changes were proposed in this pull request?
Add the support of Group entity
Why are the changes needed?
Fix: #2236
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Add a new UT.