Skip to content
This repository has been archived by the owner on Dec 14, 2022. It is now read-only.

[FEATURE]: Configure CryptoKeyReader in FlinkPulsarSource and FlinkPulsarSink #386

Open
objecttrouve opened this issue Aug 8, 2021 · 4 comments
Assignees
Labels
compute/data-processing type/feature Indicates new functionality

Comments

@objecttrouve
Copy link

Is your feature request related to a problem? Please describe.

I want to configure a CryptoKeyReader with both a FlinkPulsarSource and a FlinkPulsarSink. I found that this isn't possible just by making settings in the Properties parameter (see also #11562 in the apache/pulsar project).

Describe the solution you'd like

Possibility to pass an instance of CryptoKeyReader as an argument to the construction of FlinkPulsarSource and FlinkPulsarSink.

Describe alternatives you've considered

Specify a class name in the properties and do a PR in the pulsar project that instantiates it. However, in most cases you'll want to configure the CryptoKeyReader and a no-arg instance would be useless. So it's not a good idea.

Additional context

Also asked about it on Slack.

@objecttrouve objecttrouve added the type/feature Indicates new functionality label Aug 8, 2021
@objecttrouve objecttrouve changed the title [FEATURE]: Configure CryptoKeyReader in FlinkPulsarSource and a FlinkPulsarSink [FEATURE]: Configure CryptoKeyReader in FlinkPulsarSource and FlinkPulsarSink Aug 8, 2021
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Aug 8, 2021
As requested [here](streamnative#386), it's now possible to pass a `CryptoKeyReader` (and encryption keys) to `FlinkPulsarSource` and `FlinkPulsarSink`.

Used builder pattern for easy extensibility without breaking or excessively overloading public c'tors.

Added integration test. (Maybe it can be moved to one of the other tests to avoid overhead.)
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Aug 8, 2021
As requested [here](streamnative#386), it's now possible to pass a `CryptoKeyReader` (and encryption keys) to `FlinkPulsarSource` and `FlinkPulsarSink`.

Used builder pattern for easy extensibility without breaking or excessively overloading public c'tors.

Added integration test. (Maybe it can be moved to one of the other tests to avoid overhead.)
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Aug 8, 2021
As requested [here](streamnative#386), it's now possible to pass a `CryptoKeyReader` (and encryption keys) to `FlinkPulsarSource` and `FlinkPulsarSink`.

Used builder pattern for easy extensibility without breaking or excessively overloading public c'tors.

Added integration test. (Maybe it can be moved to one of the other tests to avoid overhead.)
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Aug 8, 2021
As requested [here](streamnative#386), it's now possible to pass a `CryptoKeyReader` (and encryption keys) to `FlinkPulsarSource` and `FlinkPulsarSink`.

Used builder pattern for easy extensibility without breaking or excessively overloading public c'tors.

Added integration test. (Maybe it can be moved to one of the other tests to avoid overhead.)
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Sep 15, 2021
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Sep 15, 2021
Turned out builder pattern wasn't necessary here. There's one path now for the CryptoKeyReader (plus the encryption keys) and possibly others where it's null (and the keys are empty).
objecttrouve added a commit to objecttrouve/pulsar-flink that referenced this issue Sep 15, 2021
@luigidalmare
Copy link

i have the same issue. is there any plan to configure cryptokeyreader?

@objecttrouve
Copy link
Author

@luigidalmare, there's a PR about to be merged.

@syhily
Copy link
Contributor

syhily commented Nov 4, 2021

@objecttrouve Sorry for the late response. I still want to drop the Builder class, I would do a little code refactor and get this could be merged on release-1.9, release-1.11, release-1.2 and master branch.

@objecttrouve
Copy link
Author

@syhily, please modify as you please. Thanks for picking it up!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compute/data-processing type/feature Indicates new functionality
Projects
None yet
Development

No branches or pull requests

3 participants