From ff540960341a1ad088a46d722c83e291e5da8c2b Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Thu, 26 Sep 2024 21:57:18 +0800 Subject: [PATCH 1/8] Add pip-383 --- pip/pip-383.md | 100 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) create mode 100644 pip/pip-383.md diff --git a/pip/pip-383.md b/pip/pip-383.md new file mode 100644 index 0000000000000..561997c4feb99 --- /dev/null +++ b/pip/pip-383.md @@ -0,0 +1,100 @@ +# PIP-383: Support granting permissions for multiple topics + +## Background + +In AuthorizationProvider, the authorization interface `grantPermissionAsync(TopicName topicName, Set actions, String role, String authDataJson)` currently only supports granting permissions to a single topic. +If multiple topics need to be authorized under a namespace, the client needs to call the authorization interface concurrently. +Since the permissions information are stored in the namespace-level policies, and multiple topics may be on different brokers, concurrent authorization will cause concurrent modification exceptions. +Therefore, supporting granting permissions for multiple topics is very friendly. + + +## Motivation + +Supporting granting/revoking permissions for multiple topics, +add `grantPermissionAsync(List options)` and `revokePermissionAsync(List options)` in AuthorizationProvider. + +## Goals + +### In Scope + +- Add `grantPermissionAsync(List options)` in AuthorizationProvider. +- Add `revokePermissionAsync(List options)` in AuthorizationProvider. + +## High-Level Design + +### Design & Implementation Details + +```java + +public interface AuthorizationProvider extends Closeable { + + CompletableFuture grantPermissionAsync(List options); + + CompletableFuture revokePermissionAsync(List options); +} +``` + +``` +@Data +@Builder +public class GrantPermissionOptions { + + private final String topic; + + private final String role; + + private final Set actions; +} + +@Data +@Builder +public class RevokePermissionOptions { + + private final String topic; + + private final String role; +} +``` + +Support this in the namespace admin API. + +```java +public interface Namespaces { + + CompletableFuture grantPermissionAsync(List options); + + void grantPermission(List options) throws PulsarAdminException; + + CompletableFuture revokePermissionAsync(List options); + + void revokePermission(List options) throws PulsarAdminException; +} +``` + +so user can grant/revoke permissions to multi-topics like : +```java +// grant permission for multi-topics +List grantPermissions = new ArrayList<>(); +grantPermissions.add(GrantPermissionOptions.builder().topic("topic1").role("role1").actions(Set.of(AuthAction.produce)).build()); +grantPermissions.add(GrantPermissionOptions.builder().topic("topic2").role("role2").actions(Set.of(AuthAction.consume)).build()); +admin.namespaces().grantPermission(grantPermissions); +// revoke permission topics +List revokePermissions = new ArrayList<>(); +revokePermissions.add(RevokePermissionOptions.builder().topic("topic1").role("role1")).build()); +revokePermissions.add(RevokePermissionOptions.builder().topic("topic2").role("role2")).build()); +admin.namespaces().revokePermission(revokePermissions); + +``` + +## Backward & Forward Compatibility + + + +## Alternatives + +## General Notes + +## Links + +* Mailing List discussion thread: +* Mailing List voting thread: From 4ccfa521b9e3ecda714d928772b1dad8854fabc8 Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Thu, 26 Sep 2024 21:59:12 +0800 Subject: [PATCH 2/8] update --- pip/pip-383.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index 561997c4feb99..7705dab409f26 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -1,4 +1,4 @@ -# PIP-383: Support granting permissions for multiple topics +# PIP-383: Support granting/revoking permissions for multiple topics ## Background From 2955e6a98e9ce8cac44be3fe0e9b807faa9d265c Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Thu, 26 Sep 2024 22:06:53 +0800 Subject: [PATCH 3/8] add discuss link --- pip/pip-383.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index 7705dab409f26..1b0d7ba27bac8 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -96,5 +96,5 @@ admin.namespaces().revokePermission(revokePermissions); ## Links -* Mailing List discussion thread: +* Mailing List discussion thread: https://lists.apache.org/thread/6n2jdl9bsf1f6xz2orygz3kvxmy11ykh * Mailing List voting thread: From dacea1a73c63d930f59337d60da44a551e26d372 Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Thu, 26 Sep 2024 22:18:26 +0800 Subject: [PATCH 4/8] update --- pip/pip-383.md | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index 1b0d7ba27bac8..42315f218a19a 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -24,6 +24,7 @@ add `grantPermissionAsync(List options)` and `revokePerm ### Design & Implementation Details +Add method in AuthorizationProvider ```java public interface AuthorizationProvider extends Closeable { @@ -56,7 +57,7 @@ public class RevokePermissionOptions { } ``` -Support this in the namespace admin API. +Add namespace admin API. ```java public interface Namespaces { @@ -71,6 +72,37 @@ public interface Namespaces { } ``` +Add namespace rest implementation in broker side. +```java +@POST +@Path("/grantPermissions") +public void grantPermissions(@Suspended final AsyncResponse asyncResponse, + List options) { + internalGrantPermissionsAsync(options) + .thenAccept(__ -> asyncResponse.resume(Response.noContent().build())) + .exceptionally(ex -> { + log.error("[{}] Failed to grant permissions {}", + clientAppId(), options, ex); + resumeAsyncResponseExceptionally(asyncResponse, ex); + return null; + }); +} + +@POST +@Path("/revokePermissions") +public void revokePermissions(@Suspended final AsyncResponse asyncResponse, + List options) { + internalRevokePermissionsAsync(options) + .thenAccept(__ -> asyncResponse.resume(Response.noContent().build())) + .exceptionally(ex -> { + log.error("[{}] Failed to revoke permissions {}", + clientAppId(), options, ex); + resumeAsyncResponseExceptionally(asyncResponse, ex); + return null; + }); +} +``` + so user can grant/revoke permissions to multi-topics like : ```java // grant permission for multi-topics From 62f53075a4ddab8c4ff2602dad8d2b4812fa3d0d Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Fri, 27 Sep 2024 10:43:30 +0800 Subject: [PATCH 5/8] improve description --- pip/pip-383.md | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index 42315f218a19a..ae01be8b89a65 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -2,10 +2,10 @@ ## Background -In AuthorizationProvider, the authorization interface `grantPermissionAsync(TopicName topicName, Set actions, String role, String authDataJson)` currently only supports granting permissions to a single topic. -If multiple topics need to be authorized under a namespace, the client needs to call the authorization interface concurrently. -Since the permissions information are stored in the namespace-level policies, and multiple topics may be on different brokers, concurrent authorization will cause concurrent modification exceptions. -Therefore, supporting granting permissions for multiple topics is very friendly. +In AuthorizationProvider, the authorization interface `grantPermissionAsync(TopicName topicName, Set actions, String role, String authDataJson)` currently only supports granting permissions to a single topic at a time. +When multiple topics need to be authorized under a namespace, the client makes the calls to the authorization interface concurrently. +Since the permissions information is stored in the namespace-level policies, and multiple topics may be on different brokers, concurrent authorization modification will cause concurrent modification exceptions. +Therefore, supporting granting permissions for multiple topics is very beneficial. ## Motivation @@ -105,16 +105,22 @@ public void revokePermissions(@Suspended final AsyncResponse asyncResponse, so user can grant/revoke permissions to multi-topics like : ```java -// grant permission for multi-topics -List grantPermissions = new ArrayList<>(); -grantPermissions.add(GrantPermissionOptions.builder().topic("topic1").role("role1").actions(Set.of(AuthAction.produce)).build()); -grantPermissions.add(GrantPermissionOptions.builder().topic("topic2").role("role2").actions(Set.of(AuthAction.consume)).build()); -admin.namespaces().grantPermission(grantPermissions); -// revoke permission topics -List revokePermissions = new ArrayList<>(); -revokePermissions.add(RevokePermissionOptions.builder().topic("topic1").role("role1")).build()); -revokePermissions.add(RevokePermissionOptions.builder().topic("topic2").role("role2")).build()); -admin.namespaces().revokePermission(revokePermissions); +public class TestAuthorization { + + @Test + public void testGrantPermission() { + // grant permission for multi-topics + List grantPermissions = new ArrayList<>(); + grantPermissions.add(GrantPermissionOptions.builder().topic("topic1").role("role1").actions(Set.of(AuthAction.produce)).build()); + grantPermissions.add(GrantPermissionOptions.builder().topic("topic2").role("role2").actions(Set.of(AuthAction.consume)).build()); + admin.namespaces().grantPermission(grantPermissions); + // revoke permission topics + List revokePermissions = new ArrayList<>(); + revokePermissions.add(RevokePermissionOptions.builder().topic("topic1").role("role1").build()); + revokePermissions.add(RevokePermissionOptions.builder().topic("topic2").role("role2").build()); + admin.namespaces().revokePermission(revokePermissions); + } +} ``` From 8f086493c4af5645893920ea824f363a3a862c39 Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Fri, 27 Sep 2024 13:57:33 +0800 Subject: [PATCH 6/8] improve --- pip/pip-383.md | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index ae01be8b89a65..61c9d1cf90467 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -11,14 +11,14 @@ Therefore, supporting granting permissions for multiple topics is very beneficia ## Motivation Supporting granting/revoking permissions for multiple topics, -add `grantPermissionAsync(List options)` and `revokePermissionAsync(List options)` in AuthorizationProvider. +add `grantPermissionAsync(List options)` and `revokePermissionAsync(List options)` in AuthorizationProvider. ## Goals ### In Scope -- Add `grantPermissionAsync(List options)` in AuthorizationProvider. -- Add `revokePermissionAsync(List options)` in AuthorizationProvider. +- Add `grantPermissionAsync(List options)` in AuthorizationProvider. +- Add `revokePermissionAsync(List options)` in AuthorizationProvider. ## High-Level Design @@ -29,16 +29,22 @@ Add method in AuthorizationProvider public interface AuthorizationProvider extends Closeable { - CompletableFuture grantPermissionAsync(List options); + default CompletableFuture grantPermissionAsync(List options) { + return FutureUtil.failedFuture(new IllegalStateException( + String.format("grantPermissionAsync is not supported by the Authorization"))); + } - CompletableFuture revokePermissionAsync(List options); + default CompletableFuture revokePermissionAsync(List options) { + return FutureUtil.failedFuture(new IllegalStateException( + String.format("revokePermissionAsync is not supported by the Authorization"))); + } } ``` ``` @Data @Builder -public class GrantPermissionOptions { +public class GrantTopicPermissionOptions { private final String topic; @@ -49,7 +55,7 @@ public class GrantPermissionOptions { @Data @Builder -public class RevokePermissionOptions { +public class RevokeTopicPermissionOptions { private final String topic; @@ -62,13 +68,13 @@ Add namespace admin API. ```java public interface Namespaces { - CompletableFuture grantPermissionAsync(List options); + CompletableFuture grantPermissionOnTopicsAsync(List options); - void grantPermission(List options) throws PulsarAdminException; + void grantPermissionOnTopics(List options) throws PulsarAdminException; - CompletableFuture revokePermissionAsync(List options); + CompletableFuture revokePermissionOnTopicsAsync(List options); - void revokePermission(List options) throws PulsarAdminException; + void revokePermissionOnTopics(List options) throws PulsarAdminException; } ``` @@ -76,8 +82,8 @@ Add namespace rest implementation in broker side. ```java @POST @Path("/grantPermissions") -public void grantPermissions(@Suspended final AsyncResponse asyncResponse, - List options) { +public void grantPermissionOnTopics(@Suspended final AsyncResponse asyncResponse, + List options) { internalGrantPermissionsAsync(options) .thenAccept(__ -> asyncResponse.resume(Response.noContent().build())) .exceptionally(ex -> { @@ -90,8 +96,8 @@ public void grantPermissions(@Suspended final AsyncResponse asyncResponse, @POST @Path("/revokePermissions") -public void revokePermissions(@Suspended final AsyncResponse asyncResponse, - List options) { +public void revokePermissionOnTopics(@Suspended final AsyncResponse asyncResponse, + List options) { internalRevokePermissionsAsync(options) .thenAccept(__ -> asyncResponse.resume(Response.noContent().build())) .exceptionally(ex -> { @@ -113,12 +119,12 @@ public class TestAuthorization { List grantPermissions = new ArrayList<>(); grantPermissions.add(GrantPermissionOptions.builder().topic("topic1").role("role1").actions(Set.of(AuthAction.produce)).build()); grantPermissions.add(GrantPermissionOptions.builder().topic("topic2").role("role2").actions(Set.of(AuthAction.consume)).build()); - admin.namespaces().grantPermission(grantPermissions); + admin.namespaces().grantPermissionOnTopics(grantPermissions); // revoke permission topics List revokePermissions = new ArrayList<>(); revokePermissions.add(RevokePermissionOptions.builder().topic("topic1").role("role1").build()); revokePermissions.add(RevokePermissionOptions.builder().topic("topic2").role("role2").build()); - admin.namespaces().revokePermission(revokePermissions); + admin.namespaces().revokePermissionOnTopics(revokePermissions); } } From 6877b7c92c4c10f69bacf8d361c9c060d9fb019c Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Fri, 27 Sep 2024 14:05:02 +0800 Subject: [PATCH 7/8] update --- pip/pip-383.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index 61c9d1cf90467..b6bf7f5ab6123 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -11,7 +11,7 @@ Therefore, supporting granting permissions for multiple topics is very beneficia ## Motivation Supporting granting/revoking permissions for multiple topics, -add `grantPermissionAsync(List options)` and `revokePermissionAsync(List options)` in AuthorizationProvider. +add `grantPermissionAsync(List options)` and `revokePermissionAsync(List options)` in AuthorizationProvider. ## Goals @@ -24,7 +24,7 @@ add `grantPermissionAsync(List options)` and `revo ### Design & Implementation Details -Add method in AuthorizationProvider +Add default method implementation in AuthorizationProvider ```java public interface AuthorizationProvider extends Closeable { From f8c7449676619a02501a98a91abf3a11d128410b Mon Sep 17 00:00:00 2001 From: Jiwe Guo Date: Sun, 29 Sep 2024 19:34:33 +0800 Subject: [PATCH 8/8] add vote thread --- pip/pip-383.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pip/pip-383.md b/pip/pip-383.md index b6bf7f5ab6123..72f4e182ea7ea 100644 --- a/pip/pip-383.md +++ b/pip/pip-383.md @@ -141,4 +141,4 @@ public class TestAuthorization { ## Links * Mailing List discussion thread: https://lists.apache.org/thread/6n2jdl9bsf1f6xz2orygz3kvxmy11ykh -* Mailing List voting thread: +* Mailing List voting thread: https://lists.apache.org/thread/qbyvs75r0d64h6jk8w1swr782l85b77h