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/11/11 03:39:23 UTC
[GitHub] [pulsar] rdhabalia opened a new pull request #12743: [pulsar-broker] Handle lookup redirect for V1-topics with different cluster
rdhabalia opened a new pull request #12743:
URL: https://github.com/apache/pulsar/pull/12743
### Motivation
Right now, if pulsar broker is not redirecting lookup requests if V1-topic belongs to the different region instead it gives Authorization Exception.
```
23:48:06.480 [pulsar-client-io-1-1] WARN org.apache.pulsar.client.impl.BinaryProtoLookupService - [persistent://p1/us-went1/ns/t1] failed to get Partitioned metadata :
Proxy Client is not authorized to Get Partition Metadata yrid:
org.apache.pulsar.client.api.PulsarClientException$AuthorizationException: Proxy Client is not authorized to Get Partition Metadata
```
### Modification
Broker should be able to redirect lookup to appropriate cluster so, user doesn't have to change broker URL and able to connect to right region after following redirect.
--
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] codelipenghui merged pull request #12743: [pulsar-broker] Handle lookup redirect for V1-topics with different cluster
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #12743:
URL: https://github.com/apache/pulsar/pull/12743
--
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 a change in pull request #12743: [pulsar-broker] Handle lookup redirect for V1-topics with different cluster
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #12743:
URL: https://github.com/apache/pulsar/pull/12743#discussion_r747263854
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
##########
@@ -370,20 +370,24 @@ public void initialize(ServiceConfiguration conf, PulsarResources pulsarResource
}
private CompletableFuture<Boolean> checkAuthorization(TopicName topicName, String role, AuthAction action) {
- return checkPermission(topicName, role, action)
- .thenApply(isPermission -> isPermission && checkCluster(topicName));
+ return checkPermission(topicName, role, action).
+ thenApply(isPermission -> isPermission).
+ thenCompose(permission -> permission ? checkCluster(topicName) :
+ CompletableFuture.completedFuture(false));
}
- private boolean checkCluster(TopicName topicName) {
+ private CompletableFuture<Boolean> checkCluster(TopicName topicName) {
if (topicName.isGlobal() || conf.getClusterName().equals(topicName.getCluster())) {
- return true;
- } else {
- if (log.isDebugEnabled()) {
- log.debug("Topic [{}] does not belong to local cluster [{}]", topicName.toString(),
- conf.getClusterName());
- }
- return false;
+ return CompletableFuture.completedFuture(true);
}
+ if (log.isDebugEnabled()) {
+ log.debug("Topic [{}] does not belong to local cluster [{}]", topicName.toString(), conf.getClusterName());
+ }
+ return pulsarResources.getClusterResources().listAsync()
+ .thenApply(clusters -> {
+ log.info("clusters {}, cluster {}",clusters, topicName.getCluster());
Review comment:
Will this be printed out maty times?
--
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] github-actions[bot] commented on pull request #12743: [pulsar-broker] Handle lookup redirect for V1-topics with different cluster
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12743:
URL: https://github.com/apache/pulsar/pull/12743#issuecomment-965969171
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)
--
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] rdhabalia commented on a change in pull request #12743: [pulsar-broker] Handle lookup redirect for V1-topics with different cluster
Posted by GitBox <gi...@apache.org>.
rdhabalia commented on a change in pull request #12743:
URL: https://github.com/apache/pulsar/pull/12743#discussion_r747308076
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authorization/PulsarAuthorizationProvider.java
##########
@@ -370,20 +370,24 @@ public void initialize(ServiceConfiguration conf, PulsarResources pulsarResource
}
private CompletableFuture<Boolean> checkAuthorization(TopicName topicName, String role, AuthAction action) {
- return checkPermission(topicName, role, action)
- .thenApply(isPermission -> isPermission && checkCluster(topicName));
+ return checkPermission(topicName, role, action).
+ thenApply(isPermission -> isPermission).
+ thenCompose(permission -> permission ? checkCluster(topicName) :
+ CompletableFuture.completedFuture(false));
}
- private boolean checkCluster(TopicName topicName) {
+ private CompletableFuture<Boolean> checkCluster(TopicName topicName) {
if (topicName.isGlobal() || conf.getClusterName().equals(topicName.getCluster())) {
- return true;
- } else {
- if (log.isDebugEnabled()) {
- log.debug("Topic [{}] does not belong to local cluster [{}]", topicName.toString(),
- conf.getClusterName());
- }
- return false;
+ return CompletableFuture.completedFuture(true);
}
+ if (log.isDebugEnabled()) {
+ log.debug("Topic [{}] does not belong to local cluster [{}]", topicName.toString(), conf.getClusterName());
+ }
+ return pulsarResources.getClusterResources().listAsync()
+ .thenApply(clusters -> {
+ log.info("clusters {}, cluster {}",clusters, topicName.getCluster());
Review comment:
oh.. that was for testing. let me remove 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] Anonymitaet commented on pull request #12743: [pulsar-broker] Handle lookup redirect for V1-topics with different cluster
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on pull request #12743:
URL: https://github.com/apache/pulsar/pull/12743#issuecomment-966137631
@rdhabalia seems this is a bug fix and do not affect docs?
--
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