michaeljmarshall commented on code in PR #17411:
URL: https://github.com/apache/pulsar/pull/17411#discussion_r975579399
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java:
##########
@@ -1080,12 +1082,25 @@ protected void handleSubscribe(final CommandSubscribe
subscribe) {
}
Optional<Map<String, String>> subscriptionProperties =
SubscriptionOption.getPropertiesMap(
subscribe.getSubscriptionPropertiesList());
+
+ boolean createTopicIfDoesNotExist = isAuthorizedToCreateTopic
&& forceTopicCreation
+ &&
service.isAllowAutoTopicCreation(topicName.toString());
Review Comment:
By making each of these required, it breaks the current contract, which is
that permission to produce to or consume from a topic is sufficient to auto
create a topic when auto topic creation is enabled.
I think it would make more sense to add this feature as an alternative to
auto topic creation since we're explicitly giving a producer or consumer
permission to create a topic. I think changing the PR to align with my
suggestion is as simple as using `or` instead of `and` in this part of the code
and any other relevant parts:
```suggestion
boolean createTopicIfDoesNotExist = forceTopicCreation ||
(isAuthorizedToCreateTopic
&&
service.isAllowAutoTopicCreation(topicName.toString()));
```
Note that the `forceTopicCreation` is from the consumer, and I think we
should honor that input even when the consumer has permission to create the
topic.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
