[GitHub] [pulsar] nodece commented on a diff in pull request #16014: [fix][broker] Fix create client with TLS config

[GitBox <gi...@apache.org>]

nodece commented on code in PR #16014:
URL: https://github.com/apache/pulsar/pull/16014#discussion_r895192277


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java:
##########
@@ -1418,12 +1418,13 @@ public synchronized PulsarClient getClient() throws PulsarServerException {
                 ClientConfigurationData conf =
                         ConfigurationDataUtils.loadData(overrides, initialConf, ClientConfigurationData.class);
 
-                conf.setServiceUrl(this.getConfiguration().isTlsEnabled()
-                                ? this.brokerServiceUrlTls : this.brokerServiceUrl);
-                conf.setTlsAllowInsecureConnection(this.getConfiguration().isTlsAllowInsecureConnection());
-                conf.setTlsTrustCertsFilePath(this.getConfiguration().getTlsCertificateFilePath());
+                boolean tlsEnabled = this.getConfiguration().isBrokerClientTlsEnabled();

Review Comment:
   > Looks like the reason why we can't use the `isTlsEnabled` here is that we can have tls port and non-tls port for a broker? It's better to add a description here to clarify it.
   
   The `isTlsEnabled` has been deprecated and used for the broker, not the client.  The `brokerClient` prefix is used for client/admin in config file.
   
   Currently, the `getAdminClient()` has the same check, link to https://github.com/apache/pulsar/pull/16014/files#diff-8db8be87611ed21e4a80eb15b8749729d23a49a835cb0401c8c72e5225241b21R1461, we should keep config.
   
   > And, please also add the compatibility explanation for the change. Or do we need to consider the compatibility issue? Since users can upgrade to a new version broker but with the old version config file.
   
   OK, we should update this description to be cleaner.
   
   
   



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