Skip to content

Conversation

@robobario
Copy link

Type of change

  • Enhancement / new feature

Description

Test ApiVersions v0 upstream response is forwarded

  • Makes the test KafkaClient less smart, it must be told the expected response apiVersion, rather than implementing a fallback if it cannot understand the response. This means that our test that expects to successfully decode
    a v0 ApiVersions response would fail if it received a message encoded with another apiVersion.
  • Fixed the MockServer constructor, it wasn't wiring up the response apiVersion correctly.
  • Fixed a problem where an error at decode time in the test client resulted in a timeout. If we've obtained a correlation, then we can complete the response future exceptionally. This makes the test fail faster if the message can't be decoded at the expected version.
  • Adds coverage to the production KafkaResponseDecoder that sonar is upset about

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • PR raised from a fork of this repository and made from a branch rather than main.
  • Write tests
  • Update documentation
  • Make sure all unit/integration tests pass
  • Make sure all Sonarcloud warnings are addressed or are justifiably ignored.
  • If applicable to the change, trigger the system test suite. Make sure tests pass.
  • If applicable to the change, trigger the performance test suite. Ensure that any degradations to performance numbers are understood and justified.
  • Ensure the PR references relevant issue(s) so they are closed on merging.
  • For user facing changes, update CHANGELOG.md (remember to include changes affecting the API of the test artefacts too).

NOTE: You must be a member of @kroxylicious/developers to trigger the system test and performance test suites. If you are not part of this group, comment on the PR requesting a trigger, tagging @kroxylicious/developers.

* Makes the test KafkaClient less smart, it must be told the expected response
  apiVersion, rather than implementing a fallback if it cannot understand the
  response. This means that our test that expects to successfully decode
  a v0 ApiVersions response would fail if it received a message encoded
  with another apiVersion.
* Fixed the MockServer constructor, it wasn't wiring up the response
  apiVersion correctly.
* Fixed a problem where an error at decode time in the test client
  resulted in a timeout. If we've obtained a correlation, then we can
  complete the response future exceptionally. This makes the test fail
  faster if the message can't be decoded at the expected version.

Signed-off-by: Robert Young <[email protected]>
@robobario robobario changed the title Consolidate api versions handling it Test v0 ApiVersions forwarding with IT Jul 23, 2025
We want to use KafkaClient to declare what response apiVersion we expect
the proxy to respond with. The kafka-client code we depend on to read
the messages will sometimes be backwards compatible, for example if we
try to read a v1 or v2 ApiVersions response message, then it will
successfully read the v0 portion of the message, leaving some additional
fields like throttleTimeMs unread.

By KIP-511 the contract is we will send a v0 response, not a v0
compatible response, so I think it's good for the test client to require
that all bytes are read at the response apiVersion we declare we expect.

Signed-off-by: Robert Young <[email protected]>
Copy link
Owner

@SamBarker SamBarker left a comment

Choose a reason for hiding this comment

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

Thanks @robobario

@SamBarker
Copy link
Owner

Merging I'll address the test failure on the main PR

@SamBarker SamBarker merged commit 31232bb into SamBarker:considlate-apiVersions-handling Jul 23, 2025
2 of 3 checks passed
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.

2 participants