[proposal] RequestData|ResponseData API migration

[piotrpdev]

Summary

Eliminate Kroxylicious's exposure to Kafka internal package re-organisations by taking full source ownership of the Kafka protocol classes, delivering a stable public API surface before 1.0.

Approach

Kafka's MessageGenerator (~35 files) and the non-generated support classes (protocol.*, record.*, scattered common.* types) are copied into the Kroxylicious repo as source under io.kroxylicious.kafka.*, retaining their original Apache 2.0 licence headers. The generator is run at build time against the upstream IDL specs (bundled in kafka-clients) to produce the *Data classes:

• Source ownership from day one: every upstream change is a deliberate choice - review, absorb, or skip
• Generator enhancements (type-safe request-response pairing, aliases, Javadoc from IDL) are first-class commits, not patches
• japicmp drift detection: kafka-clients is shaded into a throwaway reference artifact with matching package names; japicmp then reports any API divergence between our classes and upstream
• Automated upgrades: whenever dependabot opens a PR bumping the kafka-clients version; a new GitHub Workflow regenerates *Data classes and runs fidelity tests and japicmp on the PR, reporting any incompatibility immediately

Key Benefits

• KAFKA-20128-style re-organisations are handled by updating the copied source - filter developers see no change
• Single breaking change (one import migration) rather than repeated breakage per Kafka release
• Wire compatibility proven continuously by fidelity tests across all 198 specs and every supported protocol version
• No bytecode transformation dependency to eventually unwind - the source is ours to replace incrementally with Netty-native implementations

Migration

Filter developers update all org.apache.kafka.* imports to io.kroxylicious.kafka.*. A sed-based migration script and package mapping guide will be provided alongside the change.

Relates-to: kroxylicious/kroxylicious#3996

[k-wall]

Not sure if I have understood this right. I think the kafka-patches exist to allow us to add features to the Kafka code generator (field aliasing, for example). Have I groked this right?

Patching can get tricky and I'd worry about having it in our foundations.

I prefer the approach of forking the generator once and owning the forked code going forward. We'd rely on the serialisation/deserialisation fidelity tests to catch regressions between ourselves and Kafka.

[piotrpdev]

I think the kafka-patches exist to allow us to add features to the Kafka code generator...

Yes, they would also allow us to bypass the upstream changes we don't like (e.g. we add overloads for functions where the signature has changed).

I prefer the approach of forking the generator...

I worry that if we keep excluding upstream changes that don't affect the (de)serialized output (e.g. fixes/refactorings), eventually there may be an upstream change that both does affect the output and depends on a series of previous non-output-affecting changes, which would make the update significantly harder. I believe the proposed shading + patches approach helps mitigate this (and drift) in the best way.

[k-wall]

So would we release kafka-patches in lock step with kafka releases. If a kafka release is made which doesn't change wire level API (e.g. 4.3.1) would we still release it?

[piotrpdev]

Presumably, the proposed GitHub workflow creates a PR bumping the version, and all the tests pass straight away, so we just merge it and have another workflow automatically create the patch release. If there are conflicts, I suppose we could optionally defer until a minor release if we're busy?

[k-wall]

If Kafka changed its MemoryRecord API, we wouldn't necessarily want to ripple the change into our public API surface.

[piotrpdev]

I imagine most changes would be enhancements and fixes, no? I thought we would want to absorb most of the stuff, and we could add patches for reverting what we don't want.

[k-wall]

When we were talking about forking, I understood that to mean 'just' the generator and the support classes (MemoryRecords, ApiMessage etc). I don't think any of us imagined forking the whole kafka repo.

[piotrpdev]

By "forking" I thought we meant GitHub forking, apologies. Wouldn't forking the whole repo on GitHub allow us to track changes in the files we want more easily?

[k-wall]

Thanks @piotrpdev. The shading idea is certainly an interesting approach, but I don't think we should go that way. Having the project's foundation relying on shading would worry me.

I would:

1. fork the generator and support classes (ApiMessage, MemoryRecords etc) into kroxylicious. Literally copy the sources into the kroxylicious repo. Maintain the original licence on the sources so people understand the heritage.
2. The APIs for the imported classes (ApiMessage, MemoryRecords) and generated Data becomes our public API. At this point, we've severed the link to the Kafka internal classes completely. It is now our code.
3. Implement serialisation/deserialisation tests proving wire level interop with that written by Kafka Client.

If Kafka choose to refactor say MemoryRecords, we probably don't care. We don't necessarily need to take the changes into our version. We might, but we don't have to. The important thing is our MemoryRecords has a stable public API and it is interoperable.

@SamBarker @robobario WDYT?

[piotrpdev]

Literally copy the sources into the kroxylicious repo.

Just some food for thought, one of my comment responses:

I worry that if we keep excluding upstream changes that don't affect the (de)serialized output (e.g. fixes/refactorings), eventually there may be an upstream change that both does affect the output and depends on a series of previous non-output-affecting changes, which would make the update significantly harder. I believe the proposed shading + patches approach helps mitigate this (and drift) in the best way.

@tombentley any thoughts 👀 ?

[SamBarker]

Thanks for the thorough proposal @piotrpdev, and for the prototype work backing it up. The problem statement is spot on — we need to own this namespace before 1.0, and the verification harness you've designed is valuable regardless of which approach we take.

Reading through the discussion with @k-wall, I find myself coming from a similar space to Keith — my instinct is toward owning the source rather than owning a bytecode transformation. But I also see genuine value in the automatic upstream tracking that shading provides, and I don't think either of the two models I'm about to describe is exactly what's proposed in the document or what Keith suggested. I want to set them out side by side so we can have a clear conversation about the trade-offs and figure out which direction fits us best.

On patches

You'll notice neither model below includes the patch-based workflow from the proposal. Maintaining patches against Kafka's generator is a cunning solution to avoid forking it, but I don't think it pulls its weight. If we can detect drift with tooling (japicmp + fidelity tests), we don't need to stay mechanically coupled to upstream's source tree. When we need to change the generator, we fork it and own it — cleanly, not through a patch queue.

The destination

I think we all agree that Kroxylicious needs to own the namespace and own the source. Personally, I'd also like us to eventually be in a position to replace key classes (like MemoryRecords and RecordBatch) with implementations that properly integrate with Netty's ByteBuf — but that's my longer-term aspiration. Either way, the question is about the journey — what are the intermediate states, and what does each step cost?

Both models below reach full ownership. They differ in what order they get there and what you're carrying at each stage.

Model A: Shade first, own incrementally

1. Now: Shade kafka-clients in-tree (mono-repo module, not a separate repo) under io.kroxylicious.kafka.*. One shade rule, no source to maintain. Automatic absorption of upstream changes on each Kafka bump.
2. When we need generator changes: Fork the generator into the mono-repo — the same thing Model B does in step 1, just deferred. The ~50 non-generated classes can remain shaded or be copied in too. The question is whether deferring this fork actually saves us anything, or whether we've just delayed the same work.
3. When we're ready: Replace the remaining shaded classes with Kroxylicious-native implementations (Netty ByteBuf integration, etc.). The shaded jar shrinks and eventually disappears.

What it gives us: The cheapest possible step 1 — namespace ownership with almost no source to maintain. Automatic upstream tracking until we choose to take over each class.

What it costs: We carry a bytecode transformation as our foundation until step 3. And step 2 is the same work as Model B's step 1 — deferred, not avoided.

What it gets right: Non-wire behavioural changes (bug fixes, validation improvements, etc.) are absorbed automatically. We're not trying to fork Kafka — we want to be additive to upstream. We own the namespace and the wire contract, and we layer our own enhancements on top (e.g. field aliasing to maintain API compatibility when upstream renames things).

Model B: Own the source from the start

1. Now: Fork Kafka's MessageGenerator into the mono-repo. Run it against the upstream IDL specs to produce *Data source under our namespace. Copy in the non-generated classes (protocol.*, record.*, scattered common.* types) as source.
2. When we need generator changes: They're just commits — type-safe pairing, Javadoc, aliases. No patches, no layering.
3. When we're ready: Replace buffer/record classes with Netty-native implementations. Same as Model A's step 3, but there's no shading to unwind — we already own the source.

What it gives us: Full source ownership from day one. Every upstream change is a deliberate choice — review, cherry-pick, skip. Generator enhancements and buffer replacements are the same kind of work (modify source we own). Uniform handling of all change types.

What it costs: More manual effort to absorb upstream changes. Risk of drift if we neglect upstream tracking — though tooling mitigates this (see below).

Drift detection for either model

japicmp-maven-plugin compares by fully-qualified class name, so we can't point it directly at our jar and kafka-clients — the package names won't match. But we can shade kafka-clients with the same org.apache.kafka → io.kroxylicious.kafka relocation into a throwaway reference artifact, then run japicmp against that. Both jars have matching package names and japicmp tells us exactly which APIs have diverged from upstream.

Combined with the wire-level fidelity tests from the proposal, this gives us two complementary signals: japicmp catches API-level divergence, fidelity tests catch wire-format divergence. We already use japicmp in the main build, so the plugin infrastructure is there — it's just a matter of configuring a comparison module. As long as we have wire compatibility, we're insulated from the need to absorb every API change as it happens.

The sequencing question

The real question isn't which model is "right" — it's which sequence of intermediate states we're comfortable carrying. Model A gives the cheapest step 1 but means step 2 is a transition (from shading to source ownership for the generated classes). Model B front-loads more work but every subsequent step is the same kind of work: modify source we own.

I'm also not sure how much step 1 of Model A actually saves over Model B. The non-generated classes (~50 or so) are relatively stable — copying them in as source with japicmp tracking drift might be nearly as cheap as shading them, without the bytecode-transformation dependency.

What do people think? I'm genuinely open — I see the appeal of both paths and I'd like to hear where others land.

[SamBarker]

We've been sitting without any progress for a few weeks, so I want to give a clearer steer rather than leaving the question open.

My previous comment deliberately laid out Model A and Model B as neutral alternatives to start a debate. However, since there's been no further input from anyone, I'm going to take a position.

Model A's real advantage is speed: less upfront work, faster to absorb a new Kafka release. That's a genuine benefit and I wanted to give space for others to argue for it. But in the absence of that argument, I think Model B is the stronger foundation. It gives us uniform handling of all change types from day one, avoids carrying a bytecode transformation we'll eventually unwind anyway, and generator enhancements are enhancements we can now make rather than patches layered on top.

My recommendation: Model B — own the source from the start.

Concretely, that means:

• Fork the MessageGenerator into the mono-repo
• Copy in the non-generated support classes as source under our namespace
• Use japicmp + wire-level fidelity tests for drift detection
• Generator enhancements (type-safe pairing, field aliases, Javadoc) are enhancements we can now make

I realise this is a significant change to the proposal's direction. Would you be open to reworking it along these lines, @piotrpdev? Happy to talk through it on a call if that would help — reshaping a proposal after review is normal and this has been a really valuable exploration of the problem space.

[piotrpdev]

@SamBarker done in 79a1796 (this PR) 👍

[tombentley]

@piotrpdev can you update the PR description to describe the current approach?

[tombentley]

A few nits, but over all I'm happy with this.

[tombentley]

"as a new module": let's decide what it's called.

[piotrpdev]

Decided on kroxylicious-kafka-message-generator for the generator and kroxylicious-kafka-common for the rest (we copy Kafka's package structure), open to suggestions though 😃.

40a6db8 (this PR)

[tombentley]

Personally I think this is just about worth doing. On the one hand it's a bit cute and doesn't offer a lot of extra type safety. But it also doesn't seem particularly hard or risky.

[k-wall]

Let's keep focus on what we have to do. This proposal is big enough already. If we decide we want to propose it separately, after this proposal is delivered, but before the release, fine.

[SamBarker]

Agree with @k-wall — this is a good idea but let's keep it out of scope for this proposal. The goal here is to take ownership of the protocol surface and unblock Kafka version upgrades. Type-safe pairing can be a follow-up once the foundation is in place.

[tombentley]

Type-safe pairing can be a follow-up once the foundation is in place.

Not without breaking compatibility again. We if we want to do this separately we'd need to land a PR for it in the same release.

[tombentley]

I'm currently working on kroxylicious/kroxylicious#4391. The code which I have currently uses some of Apache Kafka's internal classes in org.apache.kafka.common.security.***.internals. I'm trying to figure out whether that means my work has a dependency on this proposal as it stands.

• One option would be for the authentication related classes I care about to be included in the scope of 116. I suppose that means they would end up in our API module, even though they're not really implementation details (rather than API) and are specifically about SASL.
• Another option would be for me to just fork the classes I care about into my own module. But we already have the OAUTHBEAR validation filter, which also has a dependency on some of the same classes. It would be nice to work towards solving the kafka clients dependency in that filter as well as the one I'm intended to propose and implement.
• A 3rd option: we create a new module, which will be a shared dependency for both OAUTHBEAR validation filter, and my work on 4391, but without doing that in the scope of this proposal or adding those classes to the API module.

Thoughts?

[piotrpdev]

The code which I have currently uses some of Apache Kafka's internal classes in org.apache.kafka.common.security.***.internals
-- @tombentley

Hmm, is that not a similar case to OAuthBearerValidatorCallbackHandler in kroxylicious/kroxylicious#3996 (comment), where we decided to keep the dependency on kafka-clients?

[k-wall]

So kroxylicious-api is going to depend on kroxylicious-kafka-common. Curious - did you consider running the generator in kroxylicious-api instead? I'm wondering if we really need the kroxylicious-kafka-common module?

[piotrpdev]

I decided against it because kroxylicious-api looks to entirely consist of interfaces and very simple/short classes, so I thought putting implementation-heavy code in there would be a bad idea. Let me know if this is an incorrect assumption though.

[k-wall]

After this change [kroxylicious-krpc-plugin] depends on the new artifact instead (or we could codegen an ApiKeys from the RPC definitions themselves). Its role in generating filter interfaces, invokers, and decoders is otherwise unchanged.

Either way it sounds a bit ugly.

I think eliminating the use of ApiKeys class from KrpcGenerator mechanism itself will simplify the problem. From a quick look, I think its only use-case is mapping id to name. The Kafka RPC definitions include the ids and names, so I think it should be an easy refactor.

{
  "apiKey": 1,
  "type": "request",
  "listeners": ["broker", "controller"],
  "name": "FetchRequest"
}

[SamBarker]

Good idea, but I'd keep it out of scope for this proposal unless it simplifies the package move itself. If eliminating the ApiKeys dependency from KrpcGenerator makes the migration easier — fewer things to relocate, fewer cross-module dependencies to untangle — then it's worth doing as part of this work. If it's just a nice cleanup, it can come after.

[k-wall]

I question whether we really need this. The important thing is wire level compatibility.

[SamBarker]

I think wire compatibility is necessary but not sufficient on its own. These are Kafka's internal classes — Kafka doesn't consider them public API and won't reliably document changes to them. The 4.3.0 record.* move that motivated this proposal is a good example: even reading the JIRA (https://issues.apache.org/jira/browse/KAFKA-20128) you wouldn't pick it as a class relocation — you'd have to read the actual PR (apache/kafka#21412) to connect the dots. No reasonable level of release note monitoring would have caught it proactively.

japicmp is the only automated signal for structural drift that isn't wire-visible — a renamed method, a changed signature, a removed type. Without it, the alternative is manually diffing our copies against upstream, which won't happen consistently.

That said, it's a transitional safety net. As each copied class gets replaced with a Kroxylicious-native implementation, we stop running japicmp against it — we've deliberately diverged at that point. But for classes that are still essentially Kafka's code, the drift signal remains valuable. The scope shrinks progressively, class by class.

[k-wall]

Could we add a high level implementation plan sketching out the path to being done? We will want to chunk this work up, so let's think about it up-front.

[SamBarker]

Building on the review comments from @tombentley (module naming) and @k-wall (module structure, whether a separate module is needed, where tests live), I think the proposal would benefit from spelling out the module structure more concretely. The current text says "copied into Kroxylicious repo as a new module" and "invoked during the build" but does not cover the relationships between modules, how the generator is invoked, or where the generated source and tests land.

One way this could look:

• kroxylicious-kafka-protocol — the runtime artifact that filter authors and Kroxylicious internals depend on. Generated *Data classes, copied protocol infrastructure (ApiMessage, Readable, Writable, schema model), record classes, common.* types. The unifying principle is purpose: everything in this module exists to represent or operate on the Kafka wire protocol. How the code got there — generated, copied from upstream, or eventually written from scratch — is an implementation detail, so the name should not encode provenance.
• kroxylicious-kafka-protocol-generator — a library module (not a plugin) containing the copied MessageGenerator source. Analogous to the generation logic inside kroxylicious-krpc-plugin, but producing Kafka's protocol message types rather than Kroxylicious filter interfaces/invokers/decoders.
• kroxylicious-krpc-plugin — gains a new goal that depends on the generator library and invokes it during the build to produce *Data source into kroxylicious-kafka-protocol. This keeps all code generation orchestrated through a single Maven plugin rather than introducing a second one.
• Fidelity tests in kroxylicious-kafka-protocol with kafka-clients as a test-scoped dependency.

I think making this structure explicit in the proposal would address several of the open questions — @k-wall's points about whether a separate module is needed and where tests live, and @tombentley's question about the module name.

@piotrpdev what do you think? If you see a better structure or have concerns about any of this, happy to discuss.

[SamBarker]

I think this is out of scope for this proposal — 116 is about owning the types that appear in the Kroxylicious API surface. Filters are a separate domain and are free to depend on kafka-clients directly for internal classes they need.

For your specific case, option 3 (shared module) actually makes sense to me. It's not just your SASL termination work — we have SASL initiation alongside the existing OAUTHBEARER usage as well, so there's a natural set of consumers for those security classes. That said, I'd keep it as a separate effort rather than adding it to 116's scope.

[piotrpdev]

@SamBarker Thanks, I mentioned/named the modules more explictly in 40a6db8 (this PR) and 207c48b (this PR) 👍.

Also, I mentioned using exec-maven-plugin during the generate-sources phase, to mirror Kafka's approach instead of adding a goal to kroxylicious-krpc-plugin. I don't know if mixing the FreeMaker and generator approaches by running them under the same module would be a good idea.
02102a0 (this PR)

[piotrpdev]

Could we add a high level implementation plan sketching out the path to being done? We will want to chunk this work up, so let's think about it up-front.

Done in b88eda3 (this PR) 👍