You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "Technoboy- (via GitHub)" <gi...@apache.org> on 2023/06/01 08:46:02 UTC
[GitHub] [pulsar] Technoboy- opened a new pull request, #20456: [feat][broker] Support revoking permission for AuthorizationProvider.java
Technoboy- opened a new pull request, #20456:
URL: https://github.com/apache/pulsar/pull/20456
### Motivation
Current AuthorizationProvider does not actually fully support the revocation of ACLs :
- Revocation is only supported for subscription-specific ACLs on a namespace
- When we revoke the permission on Pulsar admin REST API, the handler will always try to remove it from the namespace policies ([https://github.com/apache/pulsar/blob/242758d5770de46e506855ff881472cbc274cedb/pul[…]in/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java](https://github.com/apache/pulsar/blob/242758d5770de46e506855ff881472cbc274cedb/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java#L655)), instead of asking the AuthorizationProvider to do so.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1212911675
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return
+ */
+ default CompletableFuture<Void> revokePermissionAsync(NamespaceName namespace, String role) {
Review Comment:
It’s better to have `default` implementations here, otherwise it will cause compile issue when upgrade.
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1575080211
> @Technoboy- - one important detail that we should also cover is the fact that all of the "get permission" endpoints make the assumption that the permissions are stored in zk. We should probably expand the `AuthorizationProvider` to include get methods.
I think these might be trickier to support. Many backends would not be able to answer that question directly (or at least the broker might not have access to it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1572231288
> I have the same question as jizhai if the default method is return CompletableFuture with Exception. maybe we need cover the exception in the outer admin api handler to avoid earlier version of user-define AuthorizationProvider fail.
I may not get what you mean. Earlier version won't fail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lifepuzzlefun commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1213347991
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return CompletableFuture<Void>
+ */
+ default CompletableFuture<Void> revokePermissionAsync(NamespaceName namespace, String role) {
+ return FutureUtil.failedFuture(new IllegalStateException(
Review Comment:
agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1572350626
> > > I have the same question as jizhai if the default method is return CompletableFuture with Exception. maybe we need cover the exception in the outer admin api handler to avoid earlier version of user-define AuthorizationProvider fail.
> >
> >
> > I may not get what you mean. Earlier version won't fail
>
> Sorry for my misunderstanding comments. I mean that if previous version user-define AuthorizationProvider didn't override the default method. when user call the admin api and execute the default method. will it be failed ? like `org.apache.pulsar.broker.authorization.MockAuthorizationProvider`.
Ah, yes, it will fail in this case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1213339811
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return
+ */
+ default CompletableFuture<Void> revokePermissionAsync(NamespaceName namespace, String role) {
Review Comment:
Agree, also the "revoke" is already broken today when using a custom authz plugin. Even if today it succeeds, the revoke does nothing because it would just try to remove the ACL from zookeeper, where there are not any.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1573211781
> LGTM
>
> @Technoboy- It looks like you will create another PR to make sure all the revoking calls point to the newly added method, right?
yes, right.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1213287482
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java:
##########
@@ -249,6 +249,31 @@ public CompletableFuture<Void> grantPermissionAsync(TopicName topicName, Set<Aut
});
}
+ @Override
+ public CompletableFuture<Void> revokePermissionAsync(TopicName topicName, String role) {
+ return getPoliciesReadOnlyAsync().thenCompose(readonly -> {
+ if (readonly) {
+ if (log.isDebugEnabled()) {
+ log.debug("Policies are read-only. Broker cannot do read-write operations");
+ }
+ throw new IllegalStateException("policies are in readonly mode");
+ }
+ return pulsarResources.getNamespaceResources()
+ .setPoliciesAsync(topicName.getNamespaceObject(), policies -> {
+ policies.auth_policies.getTopicAuthentication()
+ .computeIfAbsent(topicName.toString(), __ -> new HashMap<>())
Review Comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1213288515
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return CompletableFuture<Void>
+ */
+ default CompletableFuture<Void> revokePermissionAsync(NamespaceName namespace, String role) {
+ return FutureUtil.failedFuture(new IllegalStateException(
Review Comment:
We'd better keep the same exception as with other default implementation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lifepuzzlefun commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1213212636
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return CompletableFuture<Void>
+ */
+ default CompletableFuture<Void> revokePermissionAsync(NamespaceName namespace, String role) {
+ return FutureUtil.failedFuture(new IllegalStateException(
Review Comment:
maybe `java.lang.UnsupportedOperationException` is more suitable.
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java:
##########
@@ -249,6 +249,31 @@ public CompletableFuture<Void> grantPermissionAsync(TopicName topicName, Set<Aut
});
}
+ @Override
+ public CompletableFuture<Void> revokePermissionAsync(TopicName topicName, String role) {
+ return getPoliciesReadOnlyAsync().thenCompose(readonly -> {
+ if (readonly) {
+ if (log.isDebugEnabled()) {
+ log.debug("Policies are read-only. Broker cannot do read-write operations");
+ }
+ throw new IllegalStateException("policies are in readonly mode");
+ }
+ return pulsarResources.getNamespaceResources()
+ .setPoliciesAsync(topicName.getNamespaceObject(), policies -> {
+ policies.auth_policies.getTopicAuthentication()
+ .computeIfAbsent(topicName.toString(), __ -> new HashMap<>())
Review Comment:
if topic not in authentication maybe we can just return
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lifepuzzlefun commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1572304621
> > I have the same question as jizhai if the default method is return CompletableFuture with Exception. maybe we need cover the exception in the outer admin api handler to avoid earlier version of user-define AuthorizationProvider fail.
>
> I may not get what you mean. Earlier version won't fail
Sorry for my misunderstanding comments. I mean that if previous version user-define AuthorizationProvider didn't override the default method. when user call the admin api and execute the default method. will it be failed ? like `org.apache.pulsar.broker.authorization.MockAuthorizationProvider`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codecov-commenter commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1573094103
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#20456](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (4bf5972) into [master](https://app.codecov.io/gh/apache/pulsar/commit/5e6e6cebcdbeec32ed49729f658f2d5cd0d98347?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (5e6e6ce) will **decrease** coverage by `0.04%`.
> The diff coverage is `57.89%`.
[](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #20456 +/- ##
============================================
- Coverage 72.95% 72.91% -0.04%
- Complexity 31914 31916 +2
============================================
Files 1867 1867
Lines 138485 138522 +37
Branches 15202 15204 +2
============================================
- Hits 101037 101009 -28
- Misses 29438 29503 +65
Partials 8010 8010
```
| Flag | Coverage Δ | |
|---|---|---|
| inttests | `24.16% <0.00%> (+0.04%)` | :arrow_up: |
| systests | `24.96% <2.63%> (-0.08%)` | :arrow_down: |
| unittests | `72.20% <57.89%> (-0.03%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...ar/broker/authorization/AuthorizationProvider.java](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvYXV0aG9yaXphdGlvbi9BdXRob3JpemF0aW9uUHJvdmlkZXIuamF2YQ==) | `8.51% <0.00%> (-0.80%)` | :arrow_down: |
| [...ker/authorization/PulsarAuthorizationProvider.java](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvYXV0aG9yaXphdGlvbi9QdWxzYXJBdXRob3JpemF0aW9uUHJvdmlkZXIuamF2YQ==) | `71.19% <61.29%> (-1.45%)` | :arrow_down: |
| [...sar/broker/authorization/AuthorizationService.java](https://app.codecov.io/gh/apache/pulsar/pull/20456?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWJyb2tlci1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3B1bHNhci9icm9rZXIvYXV0aG9yaXphdGlvbi9BdXRob3JpemF0aW9uU2VydmljZS5qYXZh) | `57.53% <100.00%> (+0.33%)` | :arrow_up: |
... and [62 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/20456/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on a diff in pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "jiazhai (via GitHub)" <gi...@apache.org>.
jiazhai commented on code in PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#discussion_r1212866736
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return
Review Comment:
How about defining a little of the expected behavior for the return value?
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/AuthorizationProvider.java:
##########
@@ -185,6 +185,18 @@ CompletableFuture<Boolean> allowSinkOpsAsync(NamespaceName namespaceName, String
CompletableFuture<Void> grantPermissionAsync(NamespaceName namespace, Set<AuthAction> actions, String role,
String authDataJson);
+ /**
+ * Revoke authorization-action permission on a namespace to the given client.
+ * @param namespace
+ * @param role
+ * @return
+ */
+ default CompletableFuture<Void> revokePermissionAsync(NamespaceName namespace, String role) {
Review Comment:
Why do we need a default implementation here? How about removing it so that we can keep aligned with other APIs in this file?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1575080694
That's fair, but I think it might make sense to let the interface's implementation decide how to handle that instead of reading irrelevant information from the metadata store. I don't think it has to be fixed along with this PR, but in my opinion, that is the right direction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] merlimat commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1575085068
Sure, it makes sense. Right now it's anyway broken when it returns an empty list
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "michaeljmarshall (via GitHub)" <gi...@apache.org>.
michaeljmarshall commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1574289785
@Technoboy- - one important detail that we should also cover is the fact that all of the "get permission" endpoints make the assumption that the permissions are stored in zk. We should probably expand the `AuthorizationProvider` to include get methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lifepuzzlefun commented on pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "lifepuzzlefun (via GitHub)" <gi...@apache.org>.
lifepuzzlefun commented on PR #20456:
URL: https://github.com/apache/pulsar/pull/20456#issuecomment-1572141598
I have the same question as jizhai if the default method is return CompletableFuture with Exception.
maybe we need cover the exception in the outer admin api handler to avoid earlier version of user-define AuthorizationProvider fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #20456: [improve][broker] Support revoking permission for AuthorizationProvider
Posted by "Technoboy- (via GitHub)" <gi...@apache.org>.
Technoboy- merged PR #20456:
URL: https://github.com/apache/pulsar/pull/20456
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org