You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/12/18 16:08:20 UTC

[GitHub] [pulsar] yuruguo opened a new pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

yuruguo opened a new pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393


   This PR is similar to [13157](https://github.com/apache/pulsar/pull/13157)
   ### Motivation
   Currently, there are some namespace operations that only `superUser` can access,. In fact, `tenantAdmin` of this namespace should also be able to access.
   The namespace operations involved includes:
   - set-bookie-affinity-group
   - get-bookie-affinity-group
   - unload-namespace-bundle
   - split-namespace-bundle
   
   So far, all namespace operation permissions belonging to `superUser` have been reduced to `tenantAdmin` in [NamespacesBase](https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java).
   
   ### Documentation  
   - [x] `no-need-doc` 
   


-- 
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] yuruguo commented on pull request #13393: [Broker] Modify return result of NamespacesBase#internalGetBookieAffinityGroup

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999611990


   /pulsarbot run-failure-checks
   
   > @yuruguo can you please close this PR and open a new one ? otherwise comments are mixed and it is hard to understand the PR in the future for people who will try to understand this work
   
   Okay :)


-- 
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] eolivelli commented on pull request #13393: [Broker] Modify return result of NamespacesBase#internalGetBookieAffinityGroup

Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999597865


   @yuruguo 
   can you please close this PR and open a new one ?
   otherwise comments are mixed and it is hard to understand the PR in the future for people who will try to understand this work
   
   


-- 
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] yuruguo removed a comment on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997598315


   /pulsarbot run-failure-checks


-- 
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] yuruguo removed a comment on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997624654


   /pulsarbot run-failure-checks


-- 
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] yuruguo removed a comment on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997224343


   @eolivelli @michaeljmarshall @codelipenghui @315157973 An namespace authorization-related PR, please take a look when you have a moment 


-- 
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] massakam commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
massakam commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999407725


   Should we grant these privileges to tenant administrators? At least in our company, we don't want tenant administrators to perform these operations on their own...


-- 
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] yuruguo removed a comment on pull request #13393: [Broker] Modify return result of NamespacesBase#internalGetBookieAffinityGroup

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999508099


   /pulsarbot run-failure-checks


-- 
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] yuruguo commented on a change in pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#discussion_r772068978



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -999,12 +999,9 @@ protected BookieAffinityGroupData internalGetBookieAffinityGroup() {
             final BookieAffinityGroupData bookkeeperAffinityGroup = getLocalPolicies().getLocalPolicies(namespaceName)
                     .orElseThrow(() -> new RestException(Status.NOT_FOUND,
                             "Namespace local-policies does not exist")).bookieAffinityGroup;
-            if (bookkeeperAffinityGroup == null) {
-                throw new RestException(Status.NOT_FOUND, "bookie-affinity group does not exist");

Review comment:
       It is similar to this [PR](https://github.com/apache/pulsar/pull/13237#issue-1077409702)




-- 
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] yuruguo commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997598315


   /pulsarbot run-failure-checks


-- 
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] yuruguo edited a comment on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo edited a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997224343


   @eolivelli @michaeljmarshall @codelipenghui @315157973 An namespace authorization-related PR, please take a look when you have a moment 


-- 
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] massakam commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
massakam commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999422522


   > so you are saying that you do not want tenant admin to admin tenant level operations ?
   
   Yes, I think these operations should only be possible by the administrators of the entire Pulsar instance, i.e. the superuser.


-- 
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] yuruguo commented on pull request #13393: [Broker] Modify return result of NamespacesBase#internalGetBookieAffinityGroup

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999462350


   @massakam @eolivelli Thanks for your reply and checking, I have reverted the modification about `authz` part, and only keep [`fix return value in method internalGetBookieAffinityGroup`](https://github.com/apache/pulsar/pull/13393#discussion_r772068978)  part, in addition, I have also changed the title
   
   PTAL again, thx!


-- 
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] yuruguo closed pull request #13393: [Broker] Modify return result of NamespacesBase#internalGetBookieAffinityGroup

Posted by GitBox <gi...@apache.org>.
yuruguo closed pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393


   


-- 
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] yuruguo commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997715775


   /pulsarbot run-failure-checks


-- 
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] yuruguo commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997591583


   /pulsarbot run-failure-checks


-- 
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] yuruguo commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997624654


   /pulsarbot run-failure-checks


-- 
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] yuruguo commented on pull request #13393: [Broker] Modify return result of NamespacesBase#internalGetBookieAffinityGroup

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-999508099


   /pulsarbot run-failure-checks


-- 
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] yuruguo commented on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997224343


   @eolivelli @michaeljmarshall @codelipenghui @315157973 PTAL, thx!


-- 
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] yuruguo removed a comment on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997715775


   /pulsarbot run-failure-checks


-- 
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] yuruguo commented on a change in pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo commented on a change in pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#discussion_r772068978



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java
##########
@@ -999,12 +999,9 @@ protected BookieAffinityGroupData internalGetBookieAffinityGroup() {
             final BookieAffinityGroupData bookkeeperAffinityGroup = getLocalPolicies().getLocalPolicies(namespaceName)
                     .orElseThrow(() -> new RestException(Status.NOT_FOUND,
                             "Namespace local-policies does not exist")).bookieAffinityGroup;
-            if (bookkeeperAffinityGroup == null) {
-                throw new RestException(Status.NOT_FOUND, "bookie-affinity group does not exist");

Review comment:
       It is similar to this [PR](https://github.com/apache/pulsar/pull/13237#issue-1077409702), we should return `null` instead of `RestException`.




-- 
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] yuruguo removed a comment on pull request #13393: [Authorization] Reduce authz of ns operation from superUser to tenantAdmin

Posted by GitBox <gi...@apache.org>.
yuruguo removed a comment on pull request #13393:
URL: https://github.com/apache/pulsar/pull/13393#issuecomment-997591583


   /pulsarbot run-failure-checks


-- 
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