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

CORE-3092 support metadata v9 #22669

Conversation

michael-redpanda
Copy link
Contributor

@michael-redpanda michael-redpanda commented Jul 31, 2024

Adds support for metadata API version 8

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.2.x
  • v24.1.x
  • v23.3.x

Release Notes

Improvements

  • Added support for Metadata API v8

@michael-redpanda michael-redpanda requested a review from a team July 31, 2024 14:00
@michael-redpanda michael-redpanda self-assigned this Jul 31, 2024
@michael-redpanda michael-redpanda requested review from BenPope and removed request for a team July 31, 2024 14:00
@michael-redpanda michael-redpanda requested a review from a team as a code owner July 31, 2024 14:00
@michael-redpanda michael-redpanda requested review from jackietung-redpanda and andijcr and removed request for a team July 31, 2024 14:00
tests/docker/ducktape-deps/sarama-examples Outdated Show resolved Hide resolved
tests/go/sarama/produce_test/main.go Outdated Show resolved Hide resolved
src/v/kafka/server/tests/metadata_test.cc Outdated Show resolved Hide resolved
Comment on lines 99 to 169
security::server_first_message send_scram_client_first(
kafka::client::transport& client,
const security::client_first_message& client_first) {
kafka::sasl_authenticate_request client_first_req;
{
auto msg = client_first.message();
client_first_req.data.auth_bytes = bytes(msg.cbegin(), msg.cend());
}
auto client_first_resp = client.dispatch(client_first_req).get0();
BOOST_REQUIRE_EQUAL(
client_first_resp.data.error_code, kafka::error_code::none);
return security::server_first_message(
client_first_resp.data.auth_bytes);
}

security::server_final_message send_scram_client_final(
kafka::client::transport& client,
const security::client_final_message& client_final) {
kafka::sasl_authenticate_request client_last_req;
{
auto msg = client_final.message();
client_last_req.data.auth_bytes = bytes(msg.cbegin(), msg.cend());
}
auto client_last_resp = client.dispatch(client_last_req).get0();

BOOST_REQUIRE_EQUAL(
client_last_resp.data.error_code, kafka::error_code::none);
return security::server_final_message(
std::move(client_last_resp.data.auth_bytes));
}

void do_sasl_handshake(kafka::client::transport& client) {
kafka::sasl_handshake_request req;
req.data.mechanism = security::scram_sha256_authenticator::name;

auto resp = client.dispatch(req).get0();
BOOST_REQUIRE_EQUAL(resp.data.error_code, kafka::error_code::none);
}

void authn_kafka_client(
kafka::client::transport& client,
const ss::sstring& username,
const ss::sstring& password) {
do_sasl_handshake(client);
const auto nonce = random_generators::gen_alphanum_string(130);
const security::client_first_message client_first(username, nonce);
const auto server_first = send_scram_client_first(client, client_first);

BOOST_REQUIRE(
std::string_view(server_first.nonce()).starts_with(nonce));
BOOST_REQUIRE_GE(
server_first.iterations(), security::scram_sha256::min_iterations);
security::client_final_message client_final(
bytes("n,,"), server_first.nonce());
auto salted_password = security::scram_sha256::hi(
bytes(password.cbegin(), password.cend()),
server_first.salt(),
server_first.iterations());
client_final.set_proof(security::scram_sha256::client_proof(
salted_password, client_first, server_first, client_final));

auto server_final = send_scram_client_final(client, client_final);
BOOST_REQUIRE(!server_final.error());

auto server_key = security::scram_sha256::server_key(salted_password);
auto server_sig = security::scram_sha256::server_signature(
server_key, client_first, server_first, client_final);

BOOST_REQUIRE_EQUAL(server_final.signature(), server_sig);
}
};
Copy link
Member

Choose a reason for hiding this comment

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

There's a lot duplication with kafka/client/sasl_client, was it too hard to extract?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know :(. That uses a shared_broker_t rather than the raw transport to handle network I/O.

@vbotbuildovich
Copy link
Collaborator

vbotbuildovich commented Jul 31, 2024

new failures in https://buildkite.com/redpanda/redpanda/builds/52317#0191095c-d1a2-4a43-aa67-cf5158cb09b1:

"rptest.tests.archival_test.ArchivalTest.test_single_partition_leadership_transfer.cloud_storage_type=CloudStorageType.ABS"

new failures in https://buildkite.com/redpanda/redpanda/builds/52317#0191095c-d1a3-4a66-a94d-568835d9a0df:

"rptest.tests.simple_e2e_test.SimpleEndToEndTest.test_consumer_interruption"

new failures in https://buildkite.com/redpanda/redpanda/builds/52317#01910973-7913-4972-87b2-1e2e8b8af7d7:

"rptest.tests.compatibility.sarama_test.SaramaTest.test_sarama_interceptors"

new failures in https://buildkite.com/redpanda/redpanda/builds/52326#01910a78-c894-43f5-a037-9b7f5a1d3db7:

"rptest.tests.simple_e2e_test.SimpleEndToEndTest.test_consumer_interruption"

new failures in https://buildkite.com/redpanda/redpanda/builds/52326#01912307-28fb-41da-95c8-8f8180506541:

"rptest.tests.simple_e2e_test.SimpleEndToEndTest.test_consumer_interruption"

new failures in https://buildkite.com/redpanda/redpanda/builds/52491#01912459-05fa-4121-a8fa-d6979efeb931:

"rptest.tests.simple_e2e_test.SimpleEndToEndTest.test_consumer_interruption"

@michael-redpanda michael-redpanda force-pushed the CORE-3092-Support-metadata-v9 branch from f6e5555 to b5df3f7 Compare July 31, 2024 19:11
@michael-redpanda
Copy link
Contributor Author

Force push b5df3f7:

  • Updated sarama verion
  • s/get0/get/
  • Fixed up intercepters example test due to change in behavior of sarama

@michael-redpanda michael-redpanda requested a review from BenPope July 31, 2024 19:11
BenPope
BenPope previously approved these changes Aug 1, 2024
andijcr
andijcr previously approved these changes Aug 1, 2024
@BenPope
Copy link
Member

BenPope commented Aug 5, 2024

@michael-redpanda michael-redpanda dismissed stale reviews from andijcr and BenPope via 6631b7f August 5, 2024 19:18
@michael-redpanda michael-redpanda force-pushed the CORE-3092-Support-metadata-v9 branch from b5df3f7 to 6631b7f Compare August 5, 2024 19:18
If `include_topic_authorized_operations` is not set to true in the
request, the `topic_authorized_operations` field in the topic metadata
response must be the expected default value of -2147483648.

Signed-off-by: Michael Boquard <michael@redpanda.com>
We do not need to audit authz checks in situations when returning back
a bitfield representing the operations the authenticated user is
permitted to perform.

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda michael-redpanda force-pushed the CORE-3092-Support-metadata-v9 branch from 6631b7f to 96c9f0c Compare August 5, 2024 19:18
@michael-redpanda
Copy link
Contributor Author

michael-redpanda commented Aug 5, 2024

Force push 6631b7f:

  • Added a comment to not update metadata to v11 or beyond without also including support for DescribeCluster v0

@michael-redpanda
Copy link
Contributor Author

michael-redpanda commented Aug 5, 2024

Force push 96c9f0c:

  • Rebased dev, maybe that'll fix the weird CI failures

andijcr
andijcr previously approved these changes Aug 6, 2024
BenPope
BenPope previously approved these changes Aug 6, 2024
@michael-redpanda
Copy link
Contributor Author

/ci-repeat 1
release
skip-unit
dt-log-level=trace
skip-redpanda-build
tests/rptest/tests/raft_availability_test.py::RaftAvailabilityTest.test_two_nodes_down

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda michael-redpanda dismissed stale reviews from BenPope and andijcr via f18bd51 August 22, 2024 20:23
@michael-redpanda michael-redpanda force-pushed the CORE-3092-Support-metadata-v9 branch 2 times, most recently from f18bd51 to 2783b87 Compare August 22, 2024 20:33
@michael-redpanda
Copy link
Contributor Author

michael-redpanda commented Aug 22, 2024

Force push:

  • f18bd51
    • Switched metadata from v9 -> v8
    • There is some sort of issue with the java kafka library where it acts differently with consumer groups when metadata v9 is in use causing tests to fail. The implementation of metadata v9 is fine (there's really no difference between v8->v9), but there's something else happening which needs to be better understood before we go to v9
  • 2783b87
    • Backed out sarama changes - they require v9

BenPope
BenPope previously approved these changes Aug 23, 2024
Copy link
Member

@BenPope BenPope left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
Copy link
Contributor Author

Force push b68d8d4:

  • Fixed kafka fixture tests

@michael-redpanda michael-redpanda merged commit 1cba765 into redpanda-data:dev Aug 23, 2024
18 checks passed
@vbotbuildovich
Copy link
Collaborator

/backport v24.2.x

Comment on lines +27 to +31
// IMPORTANT: Do not bump support to v11 (or beyond) unless DescribeCluster v0
// has been implemented. v11 drops support for the authorized operations list
// and moves those lists to DCv0
// Keep this at v8. Moving to v9 appears to cause issues with the Kafka Java
// Client
Copy link
Member

Choose a reason for hiding this comment

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

🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants