Skip to content

Conversation

@loserwang1024
Copy link
Contributor

@loserwang1024 loserwang1024 commented Mar 12, 2025

Purpose

Linked issue: close #485

This pr includes three things:

  • ZkNodeChangeNotificationWatcher : watch zk node change notifications(also used for acl change notifications later).
  • ServerMetadataCache: add upsertTableBucketMetadata. For leader node, can lookup table and database name from tableId.
  • ACL operation:
    • Client side interface: com.alibaba.fluss.client.admin.Admin
    • Server side: maily com.alibaba.fluss.server.authorizer.ZooKeeperBasedAuthorizer

Test

  • com.alibaba.fluss.client.security.acl.FlussAuthorizationITCase: ITCase to test each rpc request's authotization.
  • com.alibaba.fluss.server.authorizer.ZooKeeperBasedAuthorizerTest: UT cases.
  • com.alibaba.fluss.server.authorizer.ZkNodeChangeNotificationWatcherTest: test zk node change notifications.

@loserwang1024 loserwang1024 requested a review from wuchong March 12, 2025 07:29
@loserwang1024
Copy link
Contributor Author

@wuchong , CC

@loserwang1024 loserwang1024 force-pushed the fluss-acl-api branch 4 times, most recently from 7767d13 to ca714de Compare March 13, 2025 07:38
@loserwang1024 loserwang1024 changed the title Add Acl Operation to Fluss Admin Api Introduce fluss acl. Apr 14, 2025
@loserwang1024 loserwang1024 changed the title Introduce fluss acl. Fluss support ACL authotization Apr 14, 2025
Copy link
Member

@wuchong wuchong left a comment

Choose a reason for hiding this comment

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

  1. Let's add FlinkAuthorizationITCase for general read/write/create/drop operations on the tables.
  2. We can remove IDEMPOTENT_WRITE operation type in this PR, to just not check permission for it, and supports authorization for it in the next PR #756
  3. I created #757 to update the implementation of ServerMetadataCache instead of relying on notifyLeader RPC

if (authorizer != null) {
authorizer.authorize(
currentSession(),
OperationType.CREATE,
Copy link
Member

Choose a reason for hiding this comment

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

For partitions, I think we should use WRITE permission under the table. That means if a user has the write permission of a table, then he can create partitions. Otherwise, he has to apply CREATE/DROP permission which is very hard to use. Partition is just an internal data distribution.

Add IT case for this in FlinkAuthorizationITCase

public CompletableFuture<FetchLogResponse> fetchLog(FetchLogRequest request) {
CompletableFuture<FetchLogResponse> response = new CompletableFuture<>();
Map<TableBucket, FetchData> fetchLogData = getFetchLogData(request);
for (TableBucket tableBucket : fetchLogData.keySet()) {
Copy link
Member

Choose a reason for hiding this comment

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

deduplicate table id?

@loserwang1024 loserwang1024 force-pushed the fluss-acl-api branch 5 times, most recently from 9151836 to af35430 Compare April 22, 2025 07:09
Copy link
Member

@wuchong wuchong left a comment

Choose a reason for hiding this comment

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

Looks good to me in general. I left some minor comments and pushed a commit the address them. Please take a look at that changes.

@loserwang1024
Copy link
Contributor Author

I left some minor comments and pushed a commit the address them. Please take a look at that changes.

LGTM

@wuchong wuchong merged commit 5573ae3 into apache:main Apr 25, 2025
3 checks passed
polyzos pushed a commit to polyzos/fluss that referenced this pull request Apr 27, 2025
@loserwang1024 loserwang1024 deleted the fluss-acl-api branch June 17, 2025 03:43
ZmmBigdata pushed a commit to ZmmBigdata/fluss that referenced this pull request Jun 20, 2025
polyzos pushed a commit to polyzos/fluss that referenced this pull request Aug 30, 2025
polyzos pushed a commit to Alibaba-HZY/fluss that referenced this pull request Aug 31, 2025
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.

Fluss support ACL authotization

2 participants