Connector support pulsar cluster with both JWT and TLS auth #543
Conversation
|
@asagjj:Thanks for your contribution. For this PR, do we need to update docs? |
github-actions
bot
added
doc-info-missing
|
@asagjj:Thanks for providing doc info! |
|
@asagjj Thanks for your contribution. Could you fix some failed workflow check. |
|
@syhily Hi,please take a review. |
| @@ -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.
This could be a mistake? Have you tested on your local machine?
There was a problem hiding this comment.
This could be a mistake? Have you tested on your local machine?
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.
| @@ -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.
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.
Previously, I submitted apache/pulsar#15634 to improve this, welcome to review if you are interesting.
There was a problem hiding this comment.
@nodece Yes, Our cluster scene is JWT for authentication and TLS for transport encryption.
And, tls requires a ca.cert.pem file.
There was a problem hiding this comment.
Yes, if you do not use mTLS, this PR is right.
There was a problem hiding this comment.
Could you add a test for these changes?
(If this PR fixes a github issue, please add
Fixes #<xyz>.)Motivation
when pulsar cluster start up with JWT auth and TLS for transport encryption. Connector cannot configure both authentication methods at the same time。
Modifications
I have provided a simple patch to fix this, add a config 'tls-trustcert-filepath' like 'auth-plugin'.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Documentation
related config key as below:
Catalog : catalog-tls-trustcerts-filepath
DDL: properties.tls-trustcerts-filepath
Check the box below.
Need to update docs?
doc-required[DOC] Catalog config and DDL properties in README doc is out of date #544