Skip to content

Commit 2be356f

Browse files
committed
kafka: auto-create internal topics with their intended configs
The auto create topics path would always create topics with default cluster configurations. This is very incorrect for certain special topics such as `_schemas`, `__consumer_offsets`, or our internal `_redpanda.audit_log` topic. Use the newly available config accessors for these specific topics, and use them when autocreating these topics to ensure we are constructing these topics with the correct properties.
1 parent 932c5bc commit 2be356f

2 files changed

Lines changed: 119 additions & 13 deletions

File tree

src/v/kafka/server/handlers/metadata.cc

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include "kafka/server/handlers/metadata.h"
1111

12+
#include "cluster/members_table.h"
1213
#include "cluster/metadata_cache.h"
1314
#include "cluster/topics_frontend.h"
1415
#include "cluster/types.h"
@@ -20,17 +21,20 @@
2021
#include "kafka/protocol/types.h"
2122
#include "kafka/server/errors.h"
2223
#include "kafka/server/fwd.h"
24+
#include "kafka/server/group_initializer.h"
2325
#include "kafka/server/handlers/describe_cluster.h"
2426
#include "kafka/server/handlers/details/leader_epoch.h"
2527
#include "kafka/server/handlers/details/security.h"
2628
#include "kafka/server/handlers/topics/topic_utils.h"
29+
#include "kafka/server/handlers/topics/types.h"
2730
#include "kafka/server/response.h"
2831
#include "model/errc.h"
2932
#include "model/metadata.h"
3033
#include "model/namespace.h"
3134
#include "model/timeout_clock.h"
3235
#include "random/generators.h"
3336
#include "security/acl.h"
37+
#include "security/audit/audit_log_topic.h"
3438

3539
#include <seastar/core/coroutine.hh>
3640
#include <seastar/core/future-util.hh>
@@ -206,6 +210,49 @@ metadata_response::topic make_topic_response_from_topic_metadata(
206210
}
207211

208212
namespace {
213+
/// Internal topics requested by name are created with their owning
214+
/// subsystem's configuration rather than cluster defaults, mirroring Apache
215+
/// Kafka's special-casing of internal topics during metadata-driven topic
216+
/// auto-creation.
217+
cluster::topic_configuration
218+
autocreate_topic_configuration(request_context& ctx, model::topic topic) {
219+
if (topic == model::kafka_consumer_offsets_topic) {
220+
return consumer_offsets_topic_configuration(
221+
model::kafka_consumer_offsets_nt,
222+
cluster::internal_topic_replication(
223+
ctx.metadata_cache().node_count()));
224+
}
225+
if (topic == model::schema_registry_internal_tp.topic) {
226+
return schema_registry_topic_configuration(
227+
cluster::internal_topic_replication(
228+
ctx.metadata_cache().node_count()));
229+
}
230+
if (topic == model::kafka_audit_logging_topic) {
231+
auto replication_factor
232+
= config::shard_local_cfg().audit_log_replication_factor().value_or(
233+
cluster::internal_topic_replication(
234+
ctx.metadata_cache().node_count()));
235+
cluster::topic_configuration cfg{
236+
model::kafka_namespace,
237+
std::move(topic),
238+
config::shard_local_cfg().audit_log_num_partitions(),
239+
replication_factor};
240+
cfg.properties = security::audit::audit_log_topic_properties();
241+
return cfg;
242+
}
243+
// default topic configuration
244+
cluster::topic_configuration cfg{
245+
model::kafka_namespace,
246+
std::move(topic),
247+
config::shard_local_cfg().default_topic_partitions(),
248+
config::shard_local_cfg().default_topic_replication()};
249+
// Need to respect the default_redpanda_storage_mode when autocreating a
250+
// topic.
251+
cfg.properties.storage_mode
252+
= config::shard_local_cfg().default_redpanda_storage_mode();
253+
return cfg;
254+
}
255+
209256
ss::future<metadata_response::topic> create_topic(
210257
request_context& ctx,
211258
model::topic topic,
@@ -221,20 +268,10 @@ ss::future<metadata_response::topic> create_topic(
221268
t.error_code = error_code::broker_not_available;
222269
co_return t;
223270
}
224-
// default topic configuration
225-
cluster::topic_configuration cfg{
226-
model::kafka_namespace,
227-
topic,
228-
config::shard_local_cfg().default_topic_partitions(),
229-
config::shard_local_cfg().default_topic_replication()};
230-
// Need to respect the default_redpanda_storage_mode when autocreating a
231-
// topic.
232-
cfg.properties.storage_mode
233-
= config::shard_local_cfg().default_redpanda_storage_mode();
234271
auto tout = config::shard_local_cfg().internal_rpc_request_timeout_ms();
235272
try {
236273
auto res = co_await ctx.topics_frontend().autocreate_topics(
237-
{std::move(cfg)}, tout);
274+
{autocreate_topic_configuration(ctx, topic)}, tout);
238275
vassert(res.size() == 1, "expected single result");
239276
// error, neither success nor topic exists
240277
if (!(res[0].ec == cluster::errc::success
@@ -251,10 +288,11 @@ ss::future<metadata_response::topic> create_topic(
251288
ctx.controller_api(),
252289
tout + model::timeout_clock::now());
253290

254-
auto tp_md = ctx.metadata_cache().get_topic_metadata(res[0].tp_ns);
291+
auto tp_md = ctx.metadata_cache().get_topic_metadata(
292+
model::topic_namespace_view(model::kafka_namespace, topic));
255293
if (!tp_md) {
256294
metadata_response::topic t;
257-
t.name = std::move(res[0].tp_ns.tp);
295+
t.name = std::move(topic);
258296
t.error_code = error_code::invalid_topic_exception;
259297
co_return t;
260298
}

src/v/kafka/server/tests/metadata_test.cc

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "kafka/server/handlers/details/security.h"
2121
#include "kafka/server/handlers/metadata.h"
2222
#include "model/fundamental.h"
23+
#include "model/namespace.h"
2324
#include "model/timeout_clock.h"
2425
#include "redpanda/tests/fixture.h"
2526
#include "security/acl.h"
@@ -550,6 +551,73 @@ FIXTURE_TEST(metadata_autocreate, metadata_fixture) {
550551
}
551552
}
552553

554+
FIXTURE_TEST(metadata_autocreate_internal_topics, metadata_fixture) {
555+
// Internal topics auto-created in response to a metadata request must
556+
// get their owning subsystem's configuration, not cluster defaults.
557+
auto undo = set_auto_create_topics(true);
558+
wait_for_controller_leadership().get();
559+
560+
auto client = make_kafka_client().get();
561+
client.connect().get();
562+
auto close = ss::defer([&client] { client.stop().get(); });
563+
564+
auto req = kafka::metadata_request{.data{
565+
.topics = {{
566+
{.name{model::kafka_consumer_offsets_topic}},
567+
{.name{model::schema_registry_internal_tp.topic}},
568+
{.name{model::kafka_audit_logging_topic}},
569+
}},
570+
.allow_auto_topic_creation = true,
571+
.include_cluster_authorized_operations = false,
572+
.include_topic_authorized_operations = false}};
573+
auto resp = client.dispatch(std::move(req), kafka::api_version{9}).get();
574+
575+
// Only check topic-level errors: partitions may transiently report
576+
// leader_not_available until leadership metadata catches up with the
577+
// freshly created topics.
578+
BOOST_REQUIRE_EQUAL(resp.data.topics.size(), 3);
579+
for (const auto& topic : resp.data.topics) {
580+
BOOST_REQUIRE_EQUAL(topic.error_code, kafka::error_code::none);
581+
}
582+
583+
const auto& topics_state = app.controller->get_topics_state().local();
584+
585+
auto co_cfg = topics_state.get_topic_cfg(model::kafka_consumer_offsets_nt);
586+
BOOST_REQUIRE(co_cfg.has_value());
587+
BOOST_CHECK_EQUAL(
588+
co_cfg->partition_count,
589+
config::shard_local_cfg().group_topic_partitions());
590+
BOOST_CHECK(
591+
co_cfg->properties.cleanup_policy_bitflags
592+
== model::cleanup_policy_bitflags::compaction);
593+
594+
auto schemas_cfg = topics_state.get_topic_cfg(
595+
{model::kafka_namespace, model::schema_registry_internal_tp.topic});
596+
BOOST_REQUIRE(schemas_cfg.has_value());
597+
BOOST_CHECK_EQUAL(schemas_cfg->partition_count, 1);
598+
BOOST_CHECK(
599+
schemas_cfg->properties.cleanup_policy_bitflags
600+
== model::cleanup_policy_bitflags::compaction);
601+
BOOST_CHECK(
602+
schemas_cfg->properties.compression == model::compression::none);
603+
BOOST_CHECK(schemas_cfg->properties.retention_duration.is_disabled());
604+
BOOST_CHECK(schemas_cfg->properties.retention_bytes.is_disabled());
605+
606+
auto audit_cfg = topics_state.get_topic_cfg(model::kafka_audit_logging_nt);
607+
BOOST_REQUIRE(audit_cfg.has_value());
608+
BOOST_CHECK_EQUAL(
609+
audit_cfg->partition_count,
610+
config::shard_local_cfg().audit_log_num_partitions());
611+
BOOST_CHECK(
612+
audit_cfg->properties.cleanup_policy_bitflags
613+
== model::cleanup_policy_bitflags::deletion);
614+
BOOST_REQUIRE(
615+
audit_cfg->properties.retention_duration.has_optional_value());
616+
BOOST_CHECK(
617+
audit_cfg->properties.retention_duration.value()
618+
== std::chrono::milliseconds(604800000));
619+
}
620+
553621
FIXTURE_TEST(metadata_v12_unauthorized, metadata_fixture) {
554622
using kafka::api_version;
555623
constexpr auto min_version = api_version{12};

0 commit comments

Comments
 (0)