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

[core] Introduce privilege system for catalog based on FileSystem #2789

Merged
merged 4 commits into from
Apr 29, 2024

Conversation

tsreaper
Copy link
Contributor

@tsreaper tsreaper commented Jan 25, 2024

Purpose

This PR introduces an identity-based privilege system for catalogs. Catalogs can now be updated into a privileged catalogs, where privileged users can be created and granted privileges to tables, databases or the whole catalog.

Tests

  • PrivilegeManagerTest
  • PrivilegeProcedureITCase

API and Format

Yes. This PR introduces two new system tables: user and privilege.

User table is the table which stores all user information. The schema of user table is:

  • user (string): user name (primary key)
  • sha256 (bytes): sha256 of password

Privilege table is the table storing what privileges each user have. Its schema is:

  • name (string): user or role name (primary key)
  • entity_type (string): user or role (primary key)
  • identifier (string): identifier of object (primary key)
  • privilege (string): name of privilege (primary key), see PrivilegeType

Documentation

Yes. Document is also added.

@JingsongLi
Copy link
Contributor

Please finish documentation in this PR.

@tsreaper tsreaper marked this pull request as draft January 25, 2024 06:04
@tsreaper tsreaper marked this pull request as ready for review January 25, 2024 08:38
import java.util.List;

/** {@link FileStore} with privilege checks. */
public class PrivilegedFileStore<T> implements FileStore<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not wrap FileStore, all things should be finished in Table interface.

@@ -54,32 +61,43 @@
import static org.apache.paimon.utils.Preconditions.checkArgument;

/** Common implementation of {@link Catalog}. */
public abstract class AbstractCatalog implements Catalog {
public abstract class AbstractCatalog implements Catalog, PrivilegedCatalog {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should wrap a catalog just like PrevilegedFileStoreTable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some catalogs may not use FileSystem based Previlege system.
For example, RestCatalog will register users in server not filesystem.

Copy link
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe storage format for privilege system in the PR description?

@JingsongLi JingsongLi changed the title [core] Introduce privilege system for catalog [core] Introduce privilege system for catalog based on FileSystem Feb 23, 2024
@zhangjun0x01
Copy link
Contributor

hi, @tsreaper , will there be plan to integrate with apache ranger in the future? like this: https://doris.apache.org/docs/dev/admin-manual/privilege-ldap/ranger/

@tsreaper tsreaper force-pushed the priv2 branch 3 times, most recently from fcd60bc to 85ee0ec Compare April 18, 2024 02:54
## Enable Privileges

Paimon currently only supports file-based privilege system.
Only catalogs with `'metastore' = 'filesystem'` (the default value) support such privilege system.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we support filesystem privilege for hive catalog too?

Copy link
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@JingsongLi JingsongLi merged commit bc26bb9 into apache:master Apr 29, 2024
10 checks passed
sunxiaojian pushed a commit to sunxiaojian/paimon that referenced this pull request May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants