Skip to content

Conversation

@eolivelli
Copy link
Contributor

Motivation:
Currently with the Debezium and with the Kafka Connect Adapter you cannot configure authentication in order to connect to a Pulsar cluster that requires "Token" authentication

Changes:
With this patch we are adding support in the Debezium and in the Kafka Connect Adaptor for connecting to a Pulsar instance that supports Token authentication.

Documentation:
Documentation will be provided as a follow up patch.

@eolivelli eolivelli requested a review from jiazhai May 5, 2021 09:49
Copy link
Member

@sijie sijie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two issues here:

  1. we never add logic just for one authentication method. It should always be using authPlugin and authParams to support all authentication methods.

  2. I don't think this is the right direction for this change. This is also a duplicate of #8668

See more details in #8668

@eolivelli
Copy link
Contributor Author

eolivelli commented May 6, 2021

@sijie @rdhabalia @freeznet @jerrypeng @jiazhai
I agree with the comments in #8668 and we probably should provide PulsarClient together with PulsarAdmin.

This patch was a quick and dirty fix that we had in some local fork in order to unblock usage of these connectors in environments where you use Token Auth.

I wanted to shared this patch more to start a discussion and to see the feedback from the community.

if we want to go down the way of providing PulsarClient I am to work on that solution.

IIRC there were some discussions about Classpath pollution introduced by adding support for PulsarAdmin,
personally I believe that it is better to add PulsarClient and PulsarAdmin to the Context, so I am happy to go that way. It will make life easier for implementors of Pulsar IO adapters.

@eolivelli
Copy link
Contributor Author

I have started a discussion on dev@ about having a way to have the PulsarClient object

@eolivelli eolivelli marked this pull request as draft May 7, 2021 15:09
@sijie
Copy link
Member

sijie commented May 8, 2021

This patch was a quick and dirty fix that we had in some local fork in order to unblock usage of these connectors in environments where you use Token Auth.

Let's avoid adding any dirty hack in the open-source project. If your team wants to maintain your own hack, please maintain that in your own branch.

IIRC there were some discussions about Classpath pollution introduced by adding support for PulsarAdmin,

It is a problem for Pulsar Admin. But it is not a problem for PulsarClient, because we already expose Producer and Message in the context object.

@eolivelli
Copy link
Contributor Author

I am closing this PR, I switched it to draft only to keep it visible.
Sorry I wasn't clear.
I agree that this would be only a partial solution, I meant this PR only to be another starting point for the discussion about needing a full PulsarClient here

@eolivelli eolivelli closed this May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants