-
Notifications
You must be signed in to change notification settings - Fork 120
Connector support pulsar cluster with both JWT and TLS auth #543
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,7 @@ public static ClientConfigurationData newClientConf(String serviceUrl, Propertie | |
clientConf.setAuthParams(properties.getProperty(PulsarOptions.AUTH_PARAMS_KEY)); | ||
clientConf.setAuthPluginClassName( | ||
properties.getProperty(PulsarOptions.AUTH_PLUGIN_CLASSNAME_KEY)); | ||
clientConf.setTlsTrustCertsFilePath(PulsarOptions.TLS_TRUSTCERTS_FILEPATH); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Pulsar client cannot support JWT authentication with TLS transport, which only provides the use of "AuthenticationTls" to set up TLS transport/authentication, this is a conflict with JWT authentication. BTW, the trust certificate is a CA certificate, this is useless. Usually, we use mTLS, which requires a certificate and private key. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously, I submitted apache/pulsar#15634 to improve this, welcome to review if you are interesting. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nodece Yes, Our cluster scene is JWT for authentication and TLS for transport encryption. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if you do not use mTLS, this PR is right. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a test for these changes? |
||
} | ||
return clientConf; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a mistake? Have you tested on your local machine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry,I didn't catch your meaning. I had tested on our pulsar cluster. PS: our cluster is run with both token authentication and tls for transport encryption. If only config one auth-plugin, connector would get error.