Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Base64-encoded output of key and/or payload data #206

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jjlin
Copy link

@jjlin jjlin commented Nov 8, 2019

Also includes some refactoring of Avro-related code and minor doc fixes.

@jjlin
Copy link
Author

jjlin commented Nov 8, 2019

Here's a first cut at base64 support using a derivative of Jouni Malinen's base64 implementation. I started from PR #164, but it's diverged almost completely since then.

A few notes:

  • The way I'm looking at it, for the consumer, there can be an input transform ("deserialization") when reading off the Kafka topic. That corresponds to the existing -s option. There can also be an output transform (serialization) as we write to stdout/file. That corresponds to the -S option I added in this PR. Ideally these two options should be independent (e.g., you might want to unpack values read off the Kafka topic, but then write it out base64-encoded to avoid ambiguities when piped to some other tool). In this PR, -s and -S can't be used concurrently.

  • I added a consumer example (kafkacat -b mybroker -t mytopic -S base64 -K,) to the README. I think this is a pretty useful output format for piping to another tool, which can then easily read one complete key/value pair per line. There's an obvious producer analog for reading base64-encoded key/value pairs from stdin/file, splitting on the delimiter, decoding them, and writing to Kafka. Unfortunately, as in PR Add basic support for converting Avro records to JSON for output #151, the producer is not set up for handling this yet.

  • I've never really used Avro, so I don't have the background or environment to test the changes I made there. Hopefully someone can help with that.

@wojtek-oledzki
Copy link

👍 for this feature

@jjlin jjlin requested a review from edenhill January 19, 2020 00:52
@wojtek-oledzki
Copy link

Hi @jjlin - what's the current status of you work? I'm planning to build kafkacat using your branch to move some binary data. Is there anything I need to be aware of?

It would be great if the -S base64 will endup in master at some point :)

@jjlin
Copy link
Author

jjlin commented Jan 20, 2020

@wojtek-oledzki AFAIK, the base64-encoding functionality works fine, and I'm using it myself.

@wojtek-oledzki
Copy link

@jjlin how do you import data back to Kafka when exported with -S base64?

@jjlin
Copy link
Author

jjlin commented Jan 21, 2020

@wojtek-oledzki To clarify what I said about "the producer is not set up for handling this yet", the kafkacat producer doesn't support transformations currently, so base64-decoding is not supported yet either.

@fryckbos
Copy link

I also needed the producer mode and created a PR for that: jjlin#1

@jjlin
Copy link
Author

jjlin commented Mar 24, 2020

@edenhill Following up since it's been a few months. What would it take to get this merged? Is there anything you want changed in the overall approach?

@modax
Copy link

modax commented Apr 3, 2020

It would be great to have this PR integrated asap. I see that PR also adds base64 to -J which is great. RIght now -J is broken with binary data since it assumes that data is in UTF-8

@EugenDueck
Copy link

@edenhill May we get your thoughts on this PR?

kafkacat is a great tool, however its current limitation to non-binary data prevents us from using it.

Of the kafkacat PRs for base64 output that I have seen, this is the one I like most, because it can also encode the keys.

Also includes some refactoring of Avro-related code and minor doc fixes.
@jsenko
Copy link

jsenko commented Nov 3, 2023

hi @edenhill could you please review/respond to this PR? This feature would be very useful. If the project is dead (as asked in #424) at least make it official so people can readjust. I'm willing to make a small donation to the project if that is helpful, I get that OSS maintenance requires time and effort.

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.

6 participants