From 8da5e9f7395182e25acfb770432359e6c4e77022 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Wed, 21 Oct 2026 16:50:37 +0800 Subject: [PATCH 01/12] [fix](load) Validate Routine Load Kafka client properties ### What problem does this PR solve? Routine Load forwards custom Kafka properties to librdkafka without limiting the configuration surface. Restrict the exposed options before FE metadata requests and BE client configuration. - Share explicit property allowlists through Thrift constants, with FE validation for CREATE/ALTER and persisted jobs, and BE validation before any custom `Conf::set()` call. - Reject command/module/provider options and unknown properties. Certificate paths must reference Doris-managed `FILE:` objects. - Disable librdkafka's internal Kerberos ticket acquisition and renewal. Only a zero refresh interval is accepted; FE also sends it explicitly for older BEs during rolling upgrades. - Publish converted properties only after successful file resolution, so a failed conversion cannot leave a partial cache. Preserve supported consumer, SASL, TLS, OAuth and AWS MSK IAM options. ### Release note Routine Load Kafka options are now restricted to an explicit allowlist. Unknown and executable options are rejected, and certificate paths must use files registered with `CREATE FILE`. Kerberos jobs must migrate from per-job keytabs/kinit commands to operator-managed ticket caches on eligible BEs. Existing jobs with rejected properties must be recreated with supported options. See `docs/routine-load-kafka-properties.md` for migration details. ### Check List (For Author) - Test: - [ ] Regression test: added broker-independent CREATE rejection cases; execution blocked locally by missing JDK 17. - [ ] Unit Test: added FE CREATE/ALTER, persisted-job and rolling-upgrade cases, plus BE property validation, managed-file and default-Kerberos cases. FE/BE test entry points were attempted but stopped at the JDK 17 environment check. - [x] Manual/static validation: FE Maven Checkstyle (zero violations), clang-format 16 verification, BE build-hygiene gates, and `git diff --check` passed. Checked all listed librdkafka options against pinned 2.11.0 source. Checked the affected Thrift definition on branch-3.1/4.0/4.1/4.2; no field IDs or wire formats change. - Full compilation and clang-tidy remain unverified: this checkout lacks third-party build dependencies and `compile_commands.json`. - Behavior changed: Yes; property acceptance and Kerberos ticket management are restricted as described above. - Does this need documentation: Yes; migration and supported-option policy are included in `docs/routine-load-kafka-properties.md`. No new network request path is introduced. Existing Kafka/OAuth endpoints remain configured by users holding the Routine Load LOAD privilege. --- be/src/load/routine_load/data_consumer.cpp | 14 +- be/src/load/routine_load/kafka_properties.cpp | 69 ++++++++ be/src/load/routine_load/kafka_properties.h | 34 ++++ .../routine_load/kafka_properties_test.cpp | 105 ++++++++++++ docs/routine-load-kafka-properties.md | 59 +++++++ .../kafka/KafkaDataSourceProperties.java | 24 +++ .../kafka/KafkaRoutineLoadJob.java | 28 ++-- .../KafkaPropertyValidationTest.java | 153 ++++++++++++++++++ gensrc/thrift/BackendService.thrift | 102 ++++++++++++ .../test_kafka_property_validation.groovy | 57 +++++++ 10 files changed, 631 insertions(+), 14 deletions(-) create mode 100644 be/src/load/routine_load/kafka_properties.cpp create mode 100644 be/src/load/routine_load/kafka_properties.h create mode 100644 be/test/load/routine_load/kafka_properties_test.cpp create mode 100644 docs/routine-load-kafka-properties.md create mode 100644 fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java create mode 100644 regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy diff --git a/be/src/load/routine_load/data_consumer.cpp b/be/src/load/routine_load/data_consumer.cpp index b305ba6c9aae38..7bee3077b07618 100644 --- a/be/src/load/routine_load/data_consumer.cpp +++ b/be/src/load/routine_load/data_consumer.cpp @@ -18,6 +18,7 @@ #include "load/routine_load/data_consumer.h" #include +#include #include #include #include @@ -47,6 +48,7 @@ #include "common/metrics/doris_metrics.h" #include "common/status.h" #include "load/routine_load/consumer_helpers.h" +#include "load/routine_load/kafka_properties.h" #include "load/routine_load/kinesis_conf.h" #include "runtime/aws_msk_iam_auth.h" #include "runtime/exec_env.h" @@ -77,6 +79,10 @@ Status KafkaDataConsumer::init(std::shared_ptr ctx) { // conf has to be deleted finally Defer delete_conf {[conf]() { delete conf; }}; + // Metadata probes and scheduled tasks share this path, including jobs from + // older FEs. Validate before any user-controlled Conf::set() can have effects. + RETURN_IF_ERROR(configure_kafka_security(*conf, ctx->kafka_info->properties)); + std::string errstr; auto set_conf = [&conf, &errstr](const std::string& conf_key, const std::string& conf_val) { RdKafka::Conf::ConfResult res = conf->set(conf_key, conf_val, errstr); @@ -118,10 +124,10 @@ Status KafkaDataConsumer::init(std::shared_ptr ctx) { for (auto& item : ctx->kafka_info->properties) { _custom_properties.emplace(item.first, item.second); - // AWS properties (aws.*) are Doris-specific for MSK IAM authentication - // and should not be passed to librdkafka - if (starts_with(item.first, "aws.")) { - LOG(INFO) << "Skipping AWS property for librdkafka: " << item.first; + // Doris handles AWS authentication itself. Older FEs can also send offset + // bookkeeping properties; neither kind should reach librdkafka. + if (g_BackendService_constants.KAFKA_DORIS_PROPERTIES.contains(item.first)) { + LOG(INFO) << "Skipping Doris property for librdkafka: " << item.first; continue; } diff --git a/be/src/load/routine_load/kafka_properties.cpp b/be/src/load/routine_load/kafka_properties.cpp new file mode 100644 index 00000000000000..85899e05be923d --- /dev/null +++ b/be/src/load/routine_load/kafka_properties.cpp @@ -0,0 +1,69 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "load/routine_load/kafka_properties.h" + +#include +#include + +#include "common/status.h" +#include "util/string_util.h" + +namespace doris { + +Status configure_kafka_security(RdKafka::Conf& conf, + const std::map& properties) { + for (const auto& [key, value] : properties) { + // A new FE also sends zero explicitly to protect rolling upgrades to old BEs. + // Never allow a user to re-enable the default kinit command with a nonzero interval. + if (key == "sasl.kerberos.min.time.before.relogin" && value != "0") { + return Status::InvalidArgument( + "PAUSE: Kafka property '{}' is not allowed in Routine Load with a nonzero " + "value; " + "Kerberos tickets must be managed externally", + key); + } + // Certificate paths supplied by SQL users must be Doris-managed files. + if (g_BackendService_constants.KAFKA_FILE_PROPERTIES.contains(key)) { + if (!starts_with(value, "FILE:") || value.size() == 5) { + return Status::InvalidArgument( + "PAUSE: Kafka property '{}' must reference a file registered with CREATE " + "FILE", + key); + } + } else if (!g_BackendService_constants.KAFKA_ALLOWED_PROPERTIES.contains(key) && + !g_BackendService_constants.KAFKA_DORIS_PROPERTIES.contains(key)) { + // For example, plugin.library.paths can load code during Conf::set(), + // before a consumer is even created. Unknown properties fail closed. + return Status::InvalidArgument( + "PAUSE: Kafka property '{}' is not allowed in Routine Load", key); + } + } + + // Rejecting kinit.cmd alone is insufficient: the default command interpolates + // the principal/keytab. Setting the interval to zero disables both the initial + // command and the refresh timer. Operators must maintain the ticket cache. + // This is mandatory, so do not use init's best-effort set_conf wrapper here. + std::string error; + if (conf.set("sasl.kerberos.min.time.before.relogin", "0", error) != RdKafka::Conf::CONF_OK) { + return Status::InternalError( + "PAUSE: failed to disable Kafka Kerberos command execution: {}", error); + } + return Status::OK(); +} + +} // namespace doris diff --git a/be/src/load/routine_load/kafka_properties.h b/be/src/load/routine_load/kafka_properties.h new file mode 100644 index 00000000000000..94f2adc9e71697 --- /dev/null +++ b/be/src/load/routine_load/kafka_properties.h @@ -0,0 +1,34 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#pragma once + +#include +#include + +namespace RdKafka { +class Conf; +} + +namespace doris { +class Status; + +// Validate the entire map before applying any user properties, then disable +// librdkafka's shell-based Kerberos ticket acquisition and renewal. +Status configure_kafka_security(RdKafka::Conf& conf, + const std::map& properties); +} // namespace doris diff --git a/be/test/load/routine_load/kafka_properties_test.cpp b/be/test/load/routine_load/kafka_properties_test.cpp new file mode 100644 index 00000000000000..296979f506dfae --- /dev/null +++ b/be/test/load/routine_load/kafka_properties_test.cpp @@ -0,0 +1,105 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "load/routine_load/kafka_properties.h" + +#include +#include +#include + +#include + +#include "common/status.h" +#include "load/routine_load/data_consumer.h" + +namespace doris { + +TEST(KafkaPropertiesTest, RejectExecutableAndUnknownProperties) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + for (const auto* key : + {"sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", + "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", + "ssl.engine.id", "ssl.providers", "unknown.future.property", "sasl.jaas.config"}) { + auto status = configure_kafka_security(*conf, {{key, "sensitive-value"}}); + EXPECT_FALSE(status.ok()) << key; + EXPECT_NE(status.to_string().find("is not allowed in Routine Load"), std::string::npos); + EXPECT_EQ(status.to_string().find("sensitive-value"), std::string::npos); + } +} + +TEST(KafkaPropertiesTest, RejectKerberosRefreshReenable) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + for (const auto* value : {"1", "60000", "00", "FILE:123:checksum"}) { + EXPECT_FALSE( + configure_kafka_security(*conf, {{"sasl.kerberos.min.time.before.relogin", value}}) + .ok()); + } +} + +TEST(KafkaPropertiesTest, RequireManagedCertificateFiles) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + for (const auto& key : g_BackendService_constants.KAFKA_FILE_PROPERTIES) { + for (const auto* value : {"/etc/ssl/private/key.pem", "relative.pem", "", "FILE:"}) { + EXPECT_FALSE(configure_kafka_security(*conf, {{key, value}}).ok()) << key; + } + EXPECT_TRUE(configure_kafka_security(*conf, {{key, "FILE:123:checksum"}}).ok()) << key; + } +} + +TEST(KafkaPropertiesTest, DisableDefaultKerberosCommand) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + ASSERT_TRUE(configure_kafka_security(*conf, {}).ok()); + std::string value; + ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.min.time.before.relogin", value)); + EXPECT_EQ("0", value); + // Principal values can contain shell metacharacters. They remain data because + // neither the initial kinit command nor its renewal timer can run. + ASSERT_TRUE(configure_kafka_security(*conf, {{"sasl.kerberos.principal", "user;unused"}, + {"security.protocol", "SASL_PLAINTEXT"}, + {"sasl.mechanisms", "GSSAPI"}}) + .ok()); + ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.min.time.before.relogin", value)); + EXPECT_EQ("0", value); +} + +TEST(KafkaPropertiesTest, AcceptConsumerAndAwsProperties) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + EXPECT_TRUE(configure_kafka_security(*conf, {{"group.id", "test-group"}, + {"sasl.kerberos.min.time.before.relogin", "0"}, + {"isolation.level", "read_committed"}, + {"fetch.max.bytes", "1048576"}, + {"security.protocol", "SASL_SSL"}, + {"sasl.mechanism", "OAUTHBEARER"}, + {"aws.region", "us-east-1"}}) + .ok()); +} + +TEST(KafkaPropertiesTest, ConsumerRejectsBeforeResolvingFilesOrCreatingClient) { + auto ctx = std::make_shared(nullptr); + TKafkaLoadInfo info; + info.__set_properties( + {{"https.ca.location", "FILE:123:checksum"}, {"plugin.library.paths", "unused.so"}}); + ctx->kafka_info = std::make_unique(info); + KafkaDataConsumer consumer(ctx); + // No ExecEnv or Kafka broker: rejection must precede FILE resolution and + // client creation even when an allowed property is present in the same map. + auto status = consumer.init(ctx); + EXPECT_FALSE(status.ok()); + EXPECT_NE(status.to_string().find("plugin.library.paths"), std::string::npos); +} + +} // namespace doris diff --git a/docs/routine-load-kafka-properties.md b/docs/routine-load-kafka-properties.md new file mode 100644 index 00000000000000..649846b9115170 --- /dev/null +++ b/docs/routine-load-kafka-properties.md @@ -0,0 +1,59 @@ +# Routine Load Kafka property validation + +Kafka `property.*` options are checked against an explicit allowlist in both FE +and BE. Unknown options are rejected instead of being forwarded to librdkafka. +The shared source of truth is `KAFKA_ALLOWED_PROPERTIES`, +`KAFKA_FILE_PROPERTIES`, and `KAFKA_DORIS_PROPERTIES` in +[`gensrc/thrift/BackendService.thrift`](../gensrc/thrift/BackendService.thrift). +New librdkafka options require review before they can be exposed through SQL. + +Consumer identity, fetch tuning, offset isolation, TLS, SASL PLAIN/SCRAM, +OAuth bearer authentication, and Doris AWS MSK IAM options remain supported +through the listed properties. Both `property.sasl.mechanism` and +`property.sasl.mechanisms` are accepted. Existing `aws.*` and `property.aws.*` +spellings are supported for the listed AWS options. Java-only properties such +as `sasl.jaas.config` are rejected; use librdkafka's SASL properties instead. + +## Certificates + +Certificate/key/CA/CRL/keystore path options must use `FILE:` referencing a +file registered with `CREATE FILE` in the job's database and the `kafka` +catalog. Raw absolute or relative paths on a BE are rejected. FE resolves the +registered file to `FILE::`; BE downloads and resolves that managed file +before passing its local path to librdkafka. Inline PEM properties remain +available where listed in the allowlist. + +Executable module settings, including `plugin.library.paths`, `ssl.engine.id`, +`ssl.engine.location`, and `ssl.providers`, are rejected even when their values +use `FILE:`. + +## Kerberos + +Routine Load no longer runs librdkafka's internal `kinit` command. The +`sasl.kerberos.kinit.cmd` and `sasl.kerberos.keytab` properties are rejected. +`sasl.kerberos.min.time.before.relogin` accepts only the literal value `0` and +is also forced to `0` by Doris. This disables both initial ticket acquisition +and periodic renewal by librdkafka, including its default command. + +For GSSAPI authentication, operators must provision and renew a Kerberos ticket +cache accessible to the BE process on every BE eligible to run the job. The +principal and service-name properties remain available. Jobs relying on +per-job keytabs or shell commands must migrate to operator-managed tickets +before upgrading. + +## Upgrade behavior + +- FE validates both CREATE and ALTER statements before forwarding properties. +- Persisted job properties are validated again before file conversion and + metadata requests. Jobs containing rejected properties fail with an error + identifying the property; remove the incompatible configuration and recreate + the job. ALTER merges properties and cannot remove an existing rejected key. +- FE explicitly sends the zero Kerberos refresh interval to protect requests + sent to older BEs during a rolling upgrade. +- Upgraded BEs validate the complete property map before applying any custom + setting, covering metadata probes and tasks from older FEs. They disable + internal Kerberos commands even when the FE did not send the interval. +- Both sides should be upgraded. An old FE sending work to an old BE retains + its previous behavior until one of those components is upgraded. + +No Thrift field IDs, enum values, or persisted data formats change. diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java index 9cab113563da61..3b7d14364e115c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java @@ -23,6 +23,7 @@ import org.apache.doris.common.util.TimeUtils; import org.apache.doris.load.routineload.AbstractDataSourceProperties; import org.apache.doris.load.routineload.LoadDataSourceType; +import org.apache.doris.thrift.BackendServiceConstants; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; @@ -253,10 +254,33 @@ private void analyzeCustomProperties() throws AnalysisException { } } + validateCustomProperties(customKafkaProperties); + // Validate AWS MSK IAM authentication configuration if present validateAwsMskIamConfig(); } + // Also validate persisted jobs before converting FILE references or making BE metadata requests. + public static void validateCustomProperties(Map properties) throws AnalysisException { + for (Map.Entry property : properties.entrySet()) { + String key = property.getKey(); + // Only zero is safe: it prevents the default kinit command from executing. + if (key.equals("sasl.kerberos.min.time.before.relogin") && !property.getValue().equals("0")) { + throw new AnalysisException("Kafka property '" + key + "' is not allowed in Routine Load" + + " with a nonzero value; Kerberos tickets must be managed externally"); + } + if (BackendServiceConstants.KAFKA_FILE_PROPERTIES.contains(key)) { + if (!property.getValue().startsWith("FILE:") || property.getValue().length() == 5) { + throw new AnalysisException("Kafka property '" + key + + "' must reference a file registered with CREATE FILE using FILE:"); + } + } else if (!BackendServiceConstants.KAFKA_ALLOWED_PROPERTIES.contains(key) + && !BackendServiceConstants.KAFKA_DORIS_PROPERTIES.contains(key)) { + throw new AnalysisException("Kafka property '" + key + "' is not allowed in Routine Load"); + } + } + } + private void validateAwsMskIamConfig() throws AnalysisException { String securityProtocol = customKafkaProperties.get(KafkaConfiguration.SECURITY_PROTOCOL); String saslMechanism = customKafkaProperties.get(KafkaConfiguration.SASL_MECHANISM); diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java index 0570b356edf133..1f9aeb321d0181 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java @@ -27,6 +27,7 @@ import org.apache.doris.catalog.RandomDistributionInfo; import org.apache.doris.cloud.proto.Cloud; import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.DdlException; import org.apache.doris.common.InternalErrorCode; @@ -208,30 +209,37 @@ public void prepare() throws UserException { } private void convertCustomProperties(boolean rebuild) throws DdlException { - if (customProperties.isEmpty()) { - return; + // Replay can restore properties accepted by an older FE. Reject them before + // metadata requests, including during a rolling upgrade with older BEs. + try { + KafkaDataSourceProperties.validateCustomProperties(customProperties); + } catch (AnalysisException e) { + throw new DdlException(e.getMessage(), e); } - if (!rebuild && !convertedCustomProperties.isEmpty()) { return; } - if (rebuild) { - convertedCustomProperties.clear(); - } - - SmallFileMgr smallFileMgr = Env.getCurrentEnv().getSmallFileMgr(); + // Publish only a complete conversion. A failed FILE lookup must not leave + // a partial cache that skips validation/conversion on the next metadata request. + Map convertedProperties = Maps.newHashMap(); for (Map.Entry entry : customProperties.entrySet()) { if (entry.getValue().startsWith("FILE:")) { // convert FILE:file_name -> FILE:file_id:md5 String file = entry.getValue().substring(entry.getValue().indexOf(":") + 1); + SmallFileMgr smallFileMgr = Env.getCurrentEnv().getSmallFileMgr(); SmallFile smallFile = smallFileMgr.getSmallFile(dbId, KAFKA_FILE_CATALOG, file, true); - convertedCustomProperties.put(entry.getKey(), "FILE:" + smallFile.id + ":" + smallFile.md5); + convertedProperties.put(entry.getKey(), "FILE:" + smallFile.id + ":" + smallFile.md5); } else { - convertedCustomProperties.put(entry.getKey(), entry.getValue()); + convertedProperties.put(entry.getKey(), entry.getValue()); } } + // Older BEs still use librdkafka's default shell command. Send the safe + // setting explicitly so new FE -> old BE also disables ticket execution. + convertedProperties.put("sasl.kerberos.min.time.before.relogin", "0"); + convertedCustomProperties = convertedProperties; + // This is mainly for compatibility. In the previous version, we directly obtained the value of the // KAFKA_DEFAULT_OFFSETS attribute. In the new version, we support date time as the value of // KAFKA_DEFAULT_OFFSETS, and this attribute will be converted into a timestamp during the analyzing phase, diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java new file mode 100644 index 00000000000000..f5c9541c2cef0f --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -0,0 +1,153 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.load.routineload; + +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.jmockit.Deencapsulation; +import org.apache.doris.load.routineload.kafka.KafkaDataSourceProperties; +import org.apache.doris.load.routineload.kafka.KafkaRoutineLoadJob; +import org.apache.doris.thrift.BackendServiceConstants; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class KafkaPropertyValidationTest { + private KafkaDataSourceProperties properties(String key, String value, boolean alter) { + Map source = new HashMap<>(); + if (!alter) { + source.put("kafka_broker_list", "127.0.0.1:9092"); + source.put("kafka_topic", "test_topic"); + } + source.put("property." + key, value); + KafkaDataSourceProperties properties = new KafkaDataSourceProperties(source); + properties.setAlter(alter); + properties.setTimezone("UTC"); + return properties; + } + + @Test + public void testCreateAndAlterRejectExecutableAndUnknownProperties() { + for (boolean alter : new boolean[] {false, true}) { + for (String key : List.of("sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", + "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", + "ssl.engine.id", "ssl.providers", "unknown.future.property", "sasl.jaas.config", + "security.protocol\0suffix")) { + AnalysisException error = Assertions.assertThrows(AnalysisException.class, + () -> properties(key, "sensitive-value", alter).analyze()); + Assertions.assertTrue(error.getMessage().contains("is not allowed in Routine Load"), key); + Assertions.assertFalse(error.getMessage().contains("sensitive-value")); + } + } + } + + @Test + public void testRejectKerberosRefreshReenable() { + for (boolean alter : new boolean[] {false, true}) { + for (String value : List.of("1", "60000", "00", "0\0suffix", "FILE:interval")) { + Assertions.assertThrows(AnalysisException.class, + () -> properties("sasl.kerberos.min.time.before.relogin", value, alter).analyze()); + } + } + } + + @Test + public void testCertificateFilesRequireManagedReferences() throws Exception { + for (boolean alter : new boolean[] {false, true}) { + for (String key : BackendServiceConstants.KAFKA_FILE_PROPERTIES) { + for (String value : List.of("/etc/ssl/private/key.pem", "relative.pem", "", "FILE:")) { + AnalysisException error = Assertions.assertThrows(AnalysisException.class, + () -> properties(key, value, alter).analyze()); + Assertions.assertTrue(error.getMessage().contains("CREATE FILE")); + } + KafkaDataSourceProperties properties = properties(key, "FILE:client.pem", alter); + properties.analyze(); + Assertions.assertEquals("FILE:client.pem", properties.getCustomKafkaProperties().get(key)); + } + } + } + + @Test + public void testNormalConsumerPropertiesAndDefaultOffsets() throws Exception { + Map accepted = Map.of( + "group.id", "routine-load-group", + "client.id", "routine-load-client", + "security.protocol", "SASL_SSL", + "sasl.mechanisms", "SCRAM-SHA-256", + "sasl.username", "user", + "sasl.password", "secret", + "isolation.level", "read_committed", + "fetch.max.bytes", "1048576", + "kafka_default_offsets", "OFFSET_BEGINNING"); + for (boolean alter : new boolean[] {false, true}) { + for (Map.Entry property : accepted.entrySet()) { + properties(property.getKey(), property.getValue(), alter).analyze(); + } + } + } + + @Test + public void testAwsPropertiesWithAndWithoutPrefix() throws Exception { + for (String prefix : List.of("", "property.")) { + Map source = new HashMap<>(); + source.put("kafka_broker_list", "127.0.0.1:9098"); + source.put("kafka_topic", "test_topic"); + source.put("property.security.protocol", "SASL_SSL"); + source.put("property.sasl.mechanism", "OAUTHBEARER"); + source.put(prefix + "aws.region", "us-east-1"); + source.put(prefix + "aws.role_arn", "arn:aws:iam::123456789012:role/test"); + KafkaDataSourceProperties properties = new KafkaDataSourceProperties(source); + properties.setTimezone("UTC"); + properties.analyze(); + Assertions.assertEquals("us-east-1", properties.getCustomKafkaProperties().get("aws.region")); + } + } + + @Test + public void testDisableKerberosCommandForOlderBackends() throws Exception { + properties("sasl.kerberos.min.time.before.relogin", "0", false).analyze(); + for (Map custom : List.of(Map.of(), + Map.of("group.id", "test", "sasl.kerberos.principal", "user;unused"), + Map.of("kafka_default_offsets", "OFFSET_BEGINNING", + "kafka_origin_default_offsets", "OFFSET_BEGINNING"))) { + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 1L, + "127.0.0.1:9092", "test_topic", UserIdentity.ADMIN); + Deencapsulation.setField(job, "customProperties", custom); + Deencapsulation.invoke(job, "convertCustomProperties", true); + Assertions.assertEquals("0", job.getConvertedCustomProperties() + .get("sasl.kerberos.min.time.before.relogin")); + } + } + + @Test + public void testPersistedPropertiesRejectedBeforeMetadataRequests() { + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 1L, + "127.0.0.1:9092", "test_topic", UserIdentity.ADMIN); + Deencapsulation.setField(job, "customProperties", Map.of("plugin.library.paths", "unused.so")); + // Even a previously populated conversion cache must not bypass validation. + Deencapsulation.setField(job, "convertedCustomProperties", Map.of("group.id", "cached")); + DdlException error = Assertions.assertThrows(DdlException.class, + () -> Deencapsulation.invoke(job, "convertCustomProperties", false)); + Assertions.assertTrue(error.getMessage().contains("is not allowed in Routine Load")); + } +} diff --git a/gensrc/thrift/BackendService.thrift b/gensrc/thrift/BackendService.thrift index 19d80620702f5c..98f258b9222242 100644 --- a/gensrc/thrift/BackendService.thrift +++ b/gensrc/thrift/BackendService.thrift @@ -47,6 +47,108 @@ struct TTabletStatResult { 2: optional list tablet_stat_list } +// Custom Routine Load properties cross the SQL-user to BE-process trust boundary. +// Keep this allowlist shared by FE analysis and BE consumer initialization. New +// librdkafka options must be reviewed before being exposed to SQL users. Never +// allow shell commands, loadable modules/providers, or arbitrary local paths. +const set KAFKA_ALLOWED_PROPERTIES = [ + "allow.auto.create.topics", + "api.version.fallback.ms", + "api.version.request", + "api.version.request.timeout.ms", + "auto.offset.reset", + "broker.address.family", + "broker.address.ttl", + "broker.version.fallback", + "check.crcs", + "client.dns.lookup", + "client.id", + "connections.max.idle.ms", + "enable.partition.eof", + "enable.sasl.oauthbearer.unsecure.jwt", + "enable.ssl.certificate.verification", + "fetch.error.backoff.ms", + "fetch.max.bytes", + "fetch.message.max.bytes", + "fetch.min.bytes", + "fetch.wait.max.ms", + "group.id", + "group.instance.id", + "heartbeat.interval.ms", + "isolation.level", + "max.partition.fetch.bytes", + "max.poll.interval.ms", + "metadata.max.age.ms", + "partition.assignment.strategy", + "queued.max.messages.kbytes", + "queued.min.messages", + "receive.message.max.bytes", + "reconnect.backoff.max.ms", + "reconnect.backoff.ms", + "retry.backoff.max.ms", + "retry.backoff.ms", + // Only the value "0" is accepted; ticket management is external. + "sasl.kerberos.min.time.before.relogin", + "sasl.kerberos.principal", + "sasl.kerberos.service.name", + "sasl.mechanism", + "sasl.mechanisms", + "sasl.oauthbearer.client.id", + "sasl.oauthbearer.client.secret", + "sasl.oauthbearer.config", + "sasl.oauthbearer.extensions", + "sasl.oauthbearer.method", + "sasl.oauthbearer.scope", + "sasl.oauthbearer.token.endpoint.url", + "sasl.password", + "sasl.username", + "security.protocol", + "session.timeout.ms", + "socket.connection.setup.timeout.ms", + "socket.keepalive.enable", + "socket.max.fails", + "socket.nagle.disable", + "socket.receive.buffer.bytes", + "socket.send.buffer.bytes", + "socket.timeout.ms", + "ssl.ca.pem", + "ssl.certificate.pem", + "ssl.cipher.suites", + "ssl.curves.list", + "ssl.endpoint.identification.algorithm", + "ssl.key.password", + "ssl.key.pem", + "ssl.keystore.password", + "ssl.sigalgs.list", + "topic.metadata.propagation.max.ms", + "topic.metadata.refresh.fast.interval.ms", + "topic.metadata.refresh.interval.ms", + "topic.metadata.refresh.sparse", +] + +// These data files must be registered through CREATE FILE, never raw BE paths. +const set KAFKA_FILE_PROPERTIES = [ + "https.ca.location", + "ssl.ca.location", + "ssl.certificate.location", + "ssl.crl.location", + "ssl.key.location", + "ssl.keystore.location", +] + +// Doris consumes these itself; they must not be forwarded to librdkafka. +const set KAFKA_DORIS_PROPERTIES = [ + "aws.access_key", + "aws.credentials_provider", + "aws.external_id", + "aws.profile_name", + "aws.region", + "aws.role_arn", + "aws.secret_key", + "kafka_default_offsets", + "kafka_origin_default_offsets", +] + struct TKafkaLoadInfo { 1: required string brokers; 2: required string topic; diff --git a/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy b/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy new file mode 100644 index 00000000000000..f06fbf385db68e --- /dev/null +++ b/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy @@ -0,0 +1,57 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_kafka_property_validation", "p0") { + sql "DROP TABLE IF EXISTS test_kafka_property_validation" + sql """ + CREATE TABLE test_kafka_property_validation (id INT) + DUPLICATE KEY(id) + DISTRIBUTED BY HASH(id) BUCKETS 1 + PROPERTIES ("replication_num" = "1") + """ + + // All failures must happen during analysis, without contacting a Kafka broker. + def rejected = ["sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", + "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", + "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property"] + rejected.each { key -> + test { + sql """ + CREATE ROUTINE LOAD test_kafka_property_validation_job ON test_kafka_property_validation + FROM KAFKA ( + "kafka_broker_list" = "127.0.0.1:1", + "kafka_topic" = "unused", + "property.${key}" = "unused" + ) + """ + exception "Kafka property '${key}' is not allowed in Routine Load" + } + } + ["ssl.ca.location", "ssl.certificate.location", "ssl.key.location"].each { key -> + test { + sql """ + CREATE ROUTINE LOAD test_kafka_property_validation_job ON test_kafka_property_validation + FROM KAFKA ( + "kafka_broker_list" = "127.0.0.1:1", + "kafka_topic" = "unused", + "property.${key}" = "/etc/ssl/private/unused.pem" + ) + """ + exception "must reference a file registered with CREATE FILE" + } + } +} From b00894a6825df5ac97fb6cd3e27aa58185d245f6 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Wed, 21 Oct 2026 17:30:17 +0800 Subject: [PATCH 02/12] [fix](load) Preserve managed Kerberos login without shell commands ### What problem does this PR solve? Problem Summary: Keep Routine Load per-job keytab login and renewal while rejecting executable Kafka configuration. Resolve managed keytabs, use the native ticket manager, and bind each Cyrus SASL connection to its consumer's explicit cache through the pinned librdkafka patch. Adapt refresh timing to ticket lifetime and retry transient refresh failures. ### Release note Managed FILE:keytab login and automatic renewal are supported on upgraded BEs. Custom kinit commands remain forbidden. Rebuild librdkafka with the updated Doris patch; older BEs cannot perform the new native login. ### Check List (For Author) - Test: Checkstyle, clang-format 16, build hygiene, diff checks and dependency patch dry-run passed. FE/BE/regression execution blocked by missing JDK 17; full compilation, clang-tidy and live Kerberos/Kafka tests remain unverified. - Behavior changed: Yes; managed keytabs use native login and per-consumer caches. - Does this need documentation: Yes; docs/routine-load-kafka-properties.md. --- .../common/kerberos/kerberos_ticket_cache.cpp | 32 +++-- .../common/kerberos/kerberos_ticket_cache.h | 6 +- be/src/load/routine_load/data_consumer.cpp | 15 ++- be/src/load/routine_load/data_consumer.h | 7 ++ be/src/load/routine_load/kafka_properties.cpp | 53 ++++++++- be/src/load/routine_load/kafka_properties.h | 11 ++ .../kerberos/kerberos_ticket_cache_test.cpp | 26 +++- .../routine_load/kafka_properties_test.cpp | 111 +++++++++++++++++- docs/routine-load-kafka-properties.md | 56 +++++++-- .../kafka/KafkaDataSourceProperties.java | 2 +- .../KafkaPropertyValidationTest.java | 4 +- gensrc/thrift/BackendService.thrift | 1 + .../test_kafka_property_validation.groovy | 4 +- thirdparty/patches/librdkafka-2.16.0.patch | 103 ++++++++++++++++ 14 files changed, 396 insertions(+), 35 deletions(-) diff --git a/be/src/common/kerberos/kerberos_ticket_cache.cpp b/be/src/common/kerberos/kerberos_ticket_cache.cpp index eb87ff368fff37..49301c3fe80a58 100644 --- a/be/src/common/kerberos/kerberos_ticket_cache.cpp +++ b/be/src/common/kerberos/kerberos_ticket_cache.cpp @@ -17,6 +17,7 @@ #include "common/kerberos/kerberos_ticket_cache.h" +#include #include #include #include @@ -200,23 +201,38 @@ Status KerberosTicketCache::refresh_tickets() { } } +std::chrono::milliseconds KerberosTicketCache::get_refresh_interval() { + std::lock_guard lock(_mutex); + std::chrono::milliseconds interval = + std::chrono::seconds(std::max(1, _config.get_refresh_interval_second())); + if (_ticket_lifetime_sec > 0) { + interval = std::min(interval, std::chrono::milliseconds(_ticket_lifetime_sec * 500)); + } + return interval; +} + void KerberosTicketCache::start_periodic_refresh() { _should_stop_refresh = false; _refresh_thread = std::make_unique([this]() { - auto refresh_interval = std::chrono::milliseconds( - static_cast(_config.get_refresh_interval_second() * 1000)); - auto sleep_duration = _refresh_thread_sleep_time; - std::chrono::milliseconds accumulated_time(0); + auto refresh_interval = get_refresh_interval(); + auto next_refresh = std::chrono::steady_clock::now() + refresh_interval; while (!_should_stop_refresh) { + // Short-lived tickets must not wait for the default five-second poll. + auto sleep_duration = std::min(_refresh_thread_sleep_time, refresh_interval); std::this_thread::sleep_for(sleep_duration); - accumulated_time += sleep_duration; - if (accumulated_time >= refresh_interval) { - accumulated_time = std::chrono::milliseconds(0); // Reset accumulated time + if (_should_stop_refresh) { + break; + } + // Use elapsed clock time: a delayed worker must not treat a long + // scheduler stall as only one polling interval. + if (std::chrono::steady_clock::now() >= next_refresh) { Status st = refresh_tickets(); if (!st.ok()) { - // ignore and continue + // Keep the refresh due: retry on the next poll after a KDC outage. LOG(WARNING) << st.to_string(); } else { + refresh_interval = get_refresh_interval(); + next_refresh = std::chrono::steady_clock::now() + refresh_interval; LOG(INFO) << "refresh kerberos ticket cache: " << _ticket_cache_path << ", lifetime sec: " << _ticket_lifetime_sec; } diff --git a/be/src/common/kerberos/kerberos_ticket_cache.h b/be/src/common/kerberos/kerberos_ticket_cache.h index 9ca595bd5fa4a3..84e25826d38e4c 100644 --- a/be/src/common/kerberos/kerberos_ticket_cache.h +++ b/be/src/common/kerberos/kerberos_ticket_cache.h @@ -106,6 +106,10 @@ class KerberosTicketCache : public std::enable_shared_from_this _refresh_thread; diff --git a/be/src/load/routine_load/data_consumer.cpp b/be/src/load/routine_load/data_consumer.cpp index 7bee3077b07618..e3845a9cefff0e 100644 --- a/be/src/load/routine_load/data_consumer.cpp +++ b/be/src/load/routine_load/data_consumer.cpp @@ -121,6 +121,7 @@ Status KafkaDataConsumer::init(std::shared_ptr ctx) { RETURN_IF_ERROR(set_conf("debug", config::kafka_debug)); } + std::string kerberos_keytab_path; for (auto& item : ctx->kafka_info->properties) { _custom_properties.emplace(item.first, item.second); @@ -146,12 +147,24 @@ Status KafkaDataConsumer::init(std::shared_ptr ctx) { return Status::InternalError("PAUSE: failed to get file for config: {}, error: {}", item.first, st.to_string()); } - RETURN_IF_ERROR(set_conf(item.first, file_path)); + if (item.first == "sasl.kerberos.keytab") { + // The keytab is consumed by libkrb5, never by a shell command. + kerberos_keytab_path = std::move(file_path); + } else { + RETURN_IF_ERROR(set_conf(item.first, file_path)); + } } else { RETURN_IF_ERROR(set_conf(item.first, item.second)); } } + if (!kerberos_keytab_path.empty()) { + RETURN_IF_ERROR(configure_kafka_kerberos( + *conf, ctx->kafka_info->properties.at("sasl.kerberos.principal"), + kerberos_keytab_path, *ctx->exec_env()->kerberos_ticket_mgr(), + &_kerberos_ticket_cache)); + } + // if not specified group id, generate a random one. // ATTN: In the new version, we have set a group.id on the FE side for jobs that have not set a groupid, // but in order to ensure compatibility, we still do a check here. diff --git a/be/src/load/routine_load/data_consumer.h b/be/src/load/routine_load/data_consumer.h index 0d88cf29553a30..a35497161812b1 100644 --- a/be/src/load/routine_load/data_consumer.h +++ b/be/src/load/routine_load/data_consumer.h @@ -44,6 +44,10 @@ namespace doris { +namespace kerberos { +class KerberosTicketCache; +} + template class BlockingQueue; @@ -176,6 +180,9 @@ class KafkaDataConsumer : public DataConsumer { KafkaEventCb _k_event_cb; RdKafka::KafkaConsumer* _k_consumer = nullptr; + // Held until after _k_consumer closes its connections in the destructor. + std::shared_ptr _kerberos_ticket_cache; + // AWS MSK IAM authentication callback (must outlive _k_consumer) std::unique_ptr _aws_msk_oauth_callback; }; diff --git a/be/src/load/routine_load/kafka_properties.cpp b/be/src/load/routine_load/kafka_properties.cpp index 85899e05be923d..6a14f7a5ff9d9b 100644 --- a/be/src/load/routine_load/kafka_properties.cpp +++ b/be/src/load/routine_load/kafka_properties.cpp @@ -20,6 +20,9 @@ #include #include +#include + +#include "common/kerberos/kerberos_ticket_mgr.h" #include "common/status.h" #include "util/string_util.h" @@ -34,10 +37,10 @@ Status configure_kafka_security(RdKafka::Conf& conf, return Status::InvalidArgument( "PAUSE: Kafka property '{}' is not allowed in Routine Load with a nonzero " "value; " - "Kerberos tickets must be managed externally", + "Doris manages keytab login and renewal without shell commands", key); } - // Certificate paths supplied by SQL users must be Doris-managed files. + // Certificate and keytab paths supplied by SQL users must be Doris-managed files. if (g_BackendService_constants.KAFKA_FILE_PROPERTIES.contains(key)) { if (!starts_with(value, "FILE:") || value.size() == 5) { return Status::InvalidArgument( @@ -54,9 +57,29 @@ Status configure_kafka_security(RdKafka::Conf& conf, } } + // A keytab without a principal or GSSAPI would otherwise silently use the + // process identity or be ignored by a different SASL mechanism. + if (properties.contains("sasl.kerberos.keytab")) { + auto principal = properties.find("sasl.kerberos.principal"); + auto protocol = properties.find("security.protocol"); + if (principal == properties.end() || principal->second.empty() || + principal->second.find('\0') != std::string::npos || protocol == properties.end() || + (protocol->second != "SASL_SSL" && protocol->second != "SASL_PLAINTEXT")) { + return Status::InvalidArgument( + "PAUSE: Kafka keytab requires an explicit Kerberos principal and SASL " + "protocol"); + } + for (const auto* mechanism : {"sasl.mechanism", "sasl.mechanisms"}) { + auto it = properties.find(mechanism); + if (it != properties.end() && it->second != "GSSAPI") { + return Status::InvalidArgument("PAUSE: Kafka keytab requires GSSAPI"); + } + } + } + // Rejecting kinit.cmd alone is insufficient: the default command interpolates // the principal/keytab. Setting the interval to zero disables both the initial - // command and the refresh timer. Operators must maintain the ticket cache. + // command and the refresh timer. Doris handles managed keytabs through libkrb5. // This is mandatory, so do not use init's best-effort set_conf wrapper here. std::string error; if (conf.set("sasl.kerberos.min.time.before.relogin", "0", error) != RdKafka::Conf::CONF_OK) { @@ -66,4 +89,28 @@ Status configure_kafka_security(RdKafka::Conf& conf, return Status::OK(); } +Status configure_kafka_kerberos(RdKafka::Conf& conf, const std::string& principal, + const std::string& keytab_path, + kerberos::KerberosTicketMgr& manager, + std::shared_ptr* ticket_cache) { + kerberos::KerberosConfig config; + config.set_principal_and_keytab(principal, keytab_path); + std::shared_ptr cache; + RETURN_IF_ERROR(manager.get_or_set_ticket_cache(config, &cache)); + // This internal property is deliberately absent from the SQL allowlist. + // The patched Cyrus provider acquires credentials from this cache for each + // connection, so two consumers never change each other's default identity. + std::string error; + if (conf.set("sasl.kerberos.principal", principal, error) != RdKafka::Conf::CONF_OK) { + return Status::InternalError("PAUSE: failed to set Kafka Kerberos principal: {}", error); + } + if (conf.set("sasl.kerberos.ticket.cache", "FILE:" + cache->get_ticket_cache_path(), error) != + RdKafka::Conf::CONF_OK) { + return Status::InternalError("PAUSE: failed to bind Kafka Kerberos ticket cache: {}", + error); + } + *ticket_cache = std::move(cache); + return Status::OK(); +} + } // namespace doris diff --git a/be/src/load/routine_load/kafka_properties.h b/be/src/load/routine_load/kafka_properties.h index 94f2adc9e71697..abd84e54acd6bc 100644 --- a/be/src/load/routine_load/kafka_properties.h +++ b/be/src/load/routine_load/kafka_properties.h @@ -18,6 +18,7 @@ #pragma once #include +#include #include namespace RdKafka { @@ -26,9 +27,19 @@ class Conf; namespace doris { class Status; +namespace kerberos { +class KerberosTicketCache; +class KerberosTicketMgr; +} // namespace kerberos // Validate the entire map before applying any user properties, then disable // librdkafka's shell-based Kerberos ticket acquisition and renewal. Status configure_kafka_security(RdKafka::Conf& conf, const std::map& properties); +// keytab_path must already have been resolved by SmallFileMgr. Keep the returned +// cache alive until the Kafka consumer (and all of its connections) is destroyed. +Status configure_kafka_kerberos(RdKafka::Conf& conf, const std::string& principal, + const std::string& keytab_path, + kerberos::KerberosTicketMgr& manager, + std::shared_ptr* ticket_cache); } // namespace doris diff --git a/be/test/common/kerberos/kerberos_ticket_cache_test.cpp b/be/test/common/kerberos/kerberos_ticket_cache_test.cpp index cbd6294fde4e52..1f8e2e363b7941 100644 --- a/be/test/common/kerberos/kerberos_ticket_cache_test.cpp +++ b/be/test/common/kerberos/kerberos_ticket_cache_test.cpp @@ -106,14 +106,21 @@ class KerberosTicketCacheTest : public testing::Test { std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - void ExpectForLogin(const std::string& cache_path) { + void ExpectForLogin(const std::string& cache_path, int lifetime_seconds = 3600) { // Setup expectations for login EXPECT_CALL(*_mock_krb5_ptr, kt_resolve(_, _, _)).WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, cc_resolve(_, _, _)).WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, get_init_creds_opt_alloc(_, _)) .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, get_init_creds_keytab(_, _, _, _, _, _, _)) - .WillOnce(testing::Return(Status::OK())); + .WillOnce( + testing::Invoke([lifetime_seconds](krb5_context, krb5_creds* creds, + krb5_principal, krb5_keytab, krb5_deltat, + const char*, krb5_get_init_creds_opt*) { + creds->times.starttime = 100; + creds->times.endtime = 100 + lifetime_seconds; + return Status::OK(); + })); EXPECT_CALL(*_mock_krb5_ptr, cc_initialize(_, _, _)) .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, cc_store_cred(_, _, _)) @@ -208,6 +215,21 @@ TEST_F(KerberosTicketCacheTest, RefreshTickets) { ASSERT_GT(new_write_time, initial_write_time); } +TEST_F(KerberosTicketCacheTest, RefreshIntervalTracksTicketLifetime) { + SetupBasicInitExpectations(); + ASSERT_TRUE(_cache->initialize().ok()); + EXPECT_EQ(std::chrono::seconds(2), _cache->get_refresh_interval()); + ExpectForLogin(_cache->get_ticket_cache_path(), 2); + ASSERT_TRUE(_cache->login().ok()); + EXPECT_EQ(std::chrono::seconds(1), _cache->get_refresh_interval()); + ExpectForLogin(_cache->get_ticket_cache_path(), 1); + ASSERT_TRUE(_cache->login().ok()); + EXPECT_EQ(std::chrono::milliseconds(500), _cache->get_refresh_interval()); + ExpectForLogin(_cache->get_ticket_cache_path(), 3600); + ASSERT_TRUE(_cache->login().ok()); + EXPECT_EQ(std::chrono::seconds(2), _cache->get_refresh_interval()); +} + TEST_F(KerberosTicketCacheTest, PeriodicRefresh) { SetupBasicInitExpectations(); ASSERT_TRUE(_cache->initialize().ok()); diff --git a/be/test/load/routine_load/kafka_properties_test.cpp b/be/test/load/routine_load/kafka_properties_test.cpp index 296979f506dfae..31a4adba085ee3 100644 --- a/be/test/load/routine_load/kafka_properties_test.cpp +++ b/be/test/load/routine_load/kafka_properties_test.cpp @@ -18,11 +18,14 @@ #include "load/routine_load/kafka_properties.h" #include +#include #include #include +#include #include +#include "common/kerberos/kerberos_ticket_mgr.h" #include "common/status.h" #include "load/routine_load/data_consumer.h" @@ -31,7 +34,7 @@ namespace doris { TEST(KafkaPropertiesTest, RejectExecutableAndUnknownProperties) { std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); for (const auto* key : - {"sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", + {"sasl.kerberos.kinit.cmd", "sasl.kerberos.ticket.cache", "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property", "sasl.jaas.config"}) { auto status = configure_kafka_security(*conf, {{key, "sensitive-value"}}); @@ -50,13 +53,17 @@ TEST(KafkaPropertiesTest, RejectKerberosRefreshReenable) { } } -TEST(KafkaPropertiesTest, RequireManagedCertificateFiles) { +TEST(KafkaPropertiesTest, RequireManagedFiles) { std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); for (const auto& key : g_BackendService_constants.KAFKA_FILE_PROPERTIES) { for (const auto* value : {"/etc/ssl/private/key.pem", "relative.pem", "", "FILE:"}) { EXPECT_FALSE(configure_kafka_security(*conf, {{key, value}}).ok()) << key; } - EXPECT_TRUE(configure_kafka_security(*conf, {{key, "FILE:123:checksum"}}).ok()) << key; + EXPECT_TRUE(configure_kafka_security(*conf, {{key, "FILE:123:checksum"}, + {"sasl.kerberos.principal", "user@REALM"}, + {"security.protocol", "SASL_SSL"}}) + .ok()) + << key; } } @@ -102,4 +109,102 @@ TEST(KafkaPropertiesTest, ConsumerRejectsBeforeResolvingFilesOrCreatingClient) { EXPECT_NE(status.to_string().find("plugin.library.paths"), std::string::npos); } +TEST(KafkaPropertiesTest, KeytabRequiresExplicitIdentityAndGssapi) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + std::map properties = {{"sasl.kerberos.keytab", "FILE:123:checksum"}}; + EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); + properties["sasl.kerberos.principal"] = "user@REALM"; + EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); + properties["security.protocol"] = "SASL_SSL"; + EXPECT_TRUE(configure_kafka_security(*conf, properties).ok()); + for (const auto* mechanism : {"sasl.mechanism", "sasl.mechanisms"}) { + properties[mechanism] = "PLAIN"; + EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); + properties[mechanism] = "GSSAPI"; + EXPECT_TRUE(configure_kafka_security(*conf, properties).ok()); + } + properties["sasl.kerberos.principal"] = std::string("user\0suffix", 11); + EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); +} + +namespace { +class KafkaMockTicketCache : public kerberos::KerberosTicketCache { +public: + using KerberosTicketCache::KerberosTicketCache; + MOCK_METHOD(Status, initialize, (), (override)); + MOCK_METHOD(Status, login, (), (override)); + MOCK_METHOD(Status, write_ticket_cache, (), (override)); + MOCK_METHOD(void, start_periodic_refresh, (), (override)); +}; + +class KafkaMockTicketMgr : public kerberos::KerberosTicketMgr { +public: + KafkaMockTicketMgr() + : KerberosTicketMgr((std::filesystem::temp_directory_path() / + ("kafka-kerberos-" + UniqueId::gen_uid().to_string())) + .string()) {} + bool fail_login = false; + +protected: + std::shared_ptr _make_new_ticket_cache( + const kerberos::KerberosConfig& config) override { + auto cache = + std::make_shared>(config, _root_path); + cache->set_ticket_cache_path(_root_path + "/" + config.get_hash_code()); + testing::InSequence sequence; + EXPECT_CALL(*cache, initialize()).WillOnce(testing::Return(Status::OK())); + if (fail_login) { + EXPECT_CALL(*cache, login()) + .WillOnce(testing::Return(Status::InternalError("KDC unavailable"))); + } else { + EXPECT_CALL(*cache, login()).WillOnce(testing::Return(Status::OK())); + EXPECT_CALL(*cache, write_ticket_cache()).WillOnce(testing::Return(Status::OK())); + EXPECT_CALL(*cache, start_periodic_refresh()).Times(1); + } + return cache; + } +}; +} // namespace + +TEST(KafkaPropertiesTest, ManagedLoginStartsRefreshAndBindsDistinctCaches) { + KafkaMockTicketMgr manager; + std::shared_ptr first, second, shared; + std::unique_ptr a(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + std::unique_ptr b(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + ASSERT_TRUE(configure_kafka_security(*a, {}).ok()); + ASSERT_TRUE(configure_kafka_security(*b, {}).ok()); + ASSERT_TRUE( + configure_kafka_kerberos(*a, "alice@REALM", "/managed/1.keytab", manager, &first).ok()); + ASSERT_TRUE( + configure_kafka_kerberos(*b, "bob@REALM", "/managed/2.keytab", manager, &second).ok()); + ASSERT_NE(first, second); + std::string a_path, b_path, principal, interval; + ASSERT_EQ(RdKafka::Conf::CONF_OK, a->get("sasl.kerberos.ticket.cache", a_path)); + ASSERT_EQ(RdKafka::Conf::CONF_OK, b->get("sasl.kerberos.ticket.cache", b_path)); + EXPECT_EQ("FILE:" + first->get_ticket_cache_path(), a_path); + EXPECT_EQ("FILE:" + second->get_ticket_cache_path(), b_path); + EXPECT_NE(a_path, b_path); + ASSERT_EQ(RdKafka::Conf::CONF_OK, a->get("sasl.kerberos.principal", principal)); + EXPECT_EQ("alice@REALM", principal); + ASSERT_EQ(RdKafka::Conf::CONF_OK, a->get("sasl.kerberos.min.time.before.relogin", interval)); + EXPECT_EQ("0", interval); + // Reusing an identity must retain the same cache and its renewal worker. + ASSERT_TRUE(configure_kafka_kerberos(*a, "alice@REALM", "/managed/1.keytab", manager, &shared) + .ok()); + EXPECT_EQ(first, shared); +} + +TEST(KafkaPropertiesTest, FailedManagedLoginDoesNotBindOrPublishCache) { + KafkaMockTicketMgr manager; + manager.fail_login = true; + std::shared_ptr cache; + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + auto status = + configure_kafka_kerberos(*conf, "user@REALM", "/managed/1.keytab", manager, &cache); + EXPECT_FALSE(status.ok()); + EXPECT_NE(status.to_string().find("KDC unavailable"), std::string::npos); + EXPECT_EQ(nullptr, cache); + EXPECT_EQ(nullptr, manager.get_ticket_cache("user@REALM", "/managed/1.keytab")); +} + } // namespace doris diff --git a/docs/routine-load-kafka-properties.md b/docs/routine-load-kafka-properties.md index 649846b9115170..c3a61334025b97 100644 --- a/docs/routine-load-kafka-properties.md +++ b/docs/routine-load-kafka-properties.md @@ -29,17 +29,43 @@ use `FILE:`. ## Kerberos -Routine Load no longer runs librdkafka's internal `kinit` command. The -`sasl.kerberos.kinit.cmd` and `sasl.kerberos.keytab` properties are rejected. -`sasl.kerberos.min.time.before.relogin` accepts only the literal value `0` and -is also forced to `0` by Doris. This disables both initial ticket acquisition -and periodic renewal by librdkafka, including its default command. - -For GSSAPI authentication, operators must provision and renew a Kerberos ticket -cache accessible to the BE process on every BE eligible to run the job. The -principal and service-name properties remain available. Jobs relying on -per-job keytabs or shell commands must migrate to operator-managed tickets -before upgrading. +Routine Load supports automatic Kerberos login and ticket refresh with a +per-job principal and a Doris-managed keytab: + +```sql +"property.security.protocol" = "SASL_SSL", +"property.sasl.mechanism" = "GSSAPI", +"property.sasl.kerberos.service.name" = "kafka", +"property.sasl.kerberos.principal" = "loader@EXAMPLE.COM", +"property.sasl.kerberos.keytab" = "FILE:loader.keytab" +``` + +Register `loader.keytab` with `CREATE FILE` in the job's database and `kafka` +catalog first. An explicit, nonempty principal and a SASL protocol are required +when a keytab is supplied. Raw BE keytab paths are rejected. The BE still needs +operator-configured Kerberos realm/KDC settings and network access to the KDC. + +After resolving the managed file, Doris's native Kerberos ticket manager calls +libkrb5 to acquire tickets. It maintains separate caches for different +principal/keytab pairs and refreshes tickets at half their lifetime or the +configured refresh interval, whichever is shorter. A refresh failure is logged +and retried on the next poll. Consumers retain the cache while they are alive. + +Doris's librdkafka patch binds each GSSAPI connection to credentials acquired +from that consumer's cache using `gss_acquire_cred_from` and `SASL_GSS_CREDS`. +Connections acquire fresh credential handles on reconnect, picking up renewed +tickets. No process-wide `KRB5CCNAME` switching is used. Cache acquisition or +binding failure fails authentication instead of selecting another job's identity. +The internal `sasl.kerberos.ticket.cache` property cannot be set through SQL. + +`sasl.kerberos.kinit.cmd` remains forbidden. The librdkafka +`sasl.kerberos.min.time.before.relogin` option accepts only the literal `0` and +is forced to `0` by Doris: it disables librdkafka's shell command, **not** Doris's +native renewal worker. No SQL property is interpolated into a shell command. +Users who explicitly configured this interval must remove it or set it to `0`. + +Without a keytab, GSSAPI can continue using an operator-managed default ticket +cache; in that mode the operator remains responsible for ticket renewal. ## Upgrade behavior @@ -49,11 +75,17 @@ before upgrading. identifying the property; remove the incompatible configuration and recreate the job. ALTER merges properties and cannot remove an existing rejected key. - FE explicitly sends the zero Kerberos refresh interval to protect requests - sent to older BEs during a rolling upgrade. + sent to older BEs during a rolling upgrade. Native automatic login requires + upgraded BEs; an older BE cannot consume the new per-job ticket cache. Upgrade + eligible BEs before relying on managed keytab login. - Upgraded BEs validate the complete property map before applying any custom setting, covering metadata probes and tasks from older FEs. They disable internal Kerberos commands even when the FE did not send the interval. - Both sides should be upgraded. An old FE sending work to an old BE retains its previous behavior until one of those components is upgraded. +Rebuild the `librdkafka-2.16.0` third-party dependency with the updated Doris patch +before building BE. An unpatched library fails the mandatory cache-binding +configuration; it does not silently fall back to the process cache. + No Thrift field IDs, enum values, or persisted data formats change. diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java index 3b7d14364e115c..e2bd3be72c2c15 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java @@ -267,7 +267,7 @@ public static void validateCustomProperties(Map properties) thro // Only zero is safe: it prevents the default kinit command from executing. if (key.equals("sasl.kerberos.min.time.before.relogin") && !property.getValue().equals("0")) { throw new AnalysisException("Kafka property '" + key + "' is not allowed in Routine Load" - + " with a nonzero value; Kerberos tickets must be managed externally"); + + " with a nonzero value; Doris manages keytab login and renewal without shell commands"); } if (BackendServiceConstants.KAFKA_FILE_PROPERTIES.contains(key)) { if (!property.getValue().startsWith("FILE:") || property.getValue().length() == 5) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index f5c9541c2cef0f..3d745463df447f 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -49,7 +49,7 @@ private KafkaDataSourceProperties properties(String key, String value, boolean a @Test public void testCreateAndAlterRejectExecutableAndUnknownProperties() { for (boolean alter : new boolean[] {false, true}) { - for (String key : List.of("sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", + for (String key : List.of("sasl.kerberos.kinit.cmd", "sasl.kerberos.ticket.cache", "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property", "sasl.jaas.config", "security.protocol\0suffix")) { @@ -72,7 +72,7 @@ public void testRejectKerberosRefreshReenable() { } @Test - public void testCertificateFilesRequireManagedReferences() throws Exception { + public void testCertificateAndKeytabFilesRequireManagedReferences() throws Exception { for (boolean alter : new boolean[] {false, true}) { for (String key : BackendServiceConstants.KAFKA_FILE_PROPERTIES) { for (String value : List.of("/etc/ssl/private/key.pem", "relative.pem", "", "FILE:")) { diff --git a/gensrc/thrift/BackendService.thrift b/gensrc/thrift/BackendService.thrift index 98f258b9222242..41afe8079905db 100644 --- a/gensrc/thrift/BackendService.thrift +++ b/gensrc/thrift/BackendService.thrift @@ -129,6 +129,7 @@ const set KAFKA_ALLOWED_PROPERTIES = [ // These data files must be registered through CREATE FILE, never raw BE paths. const set KAFKA_FILE_PROPERTIES = [ "https.ca.location", + "sasl.kerberos.keytab", "ssl.ca.location", "ssl.certificate.location", "ssl.crl.location", diff --git a/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy b/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy index f06fbf385db68e..c105e5e516f383 100644 --- a/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy +++ b/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy @@ -25,7 +25,7 @@ suite("test_kafka_property_validation", "p0") { """ // All failures must happen during analysis, without contacting a Kafka broker. - def rejected = ["sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", + def rejected = ["sasl.kerberos.kinit.cmd", "sasl.kerberos.ticket.cache", "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property"] rejected.each { key -> @@ -41,7 +41,7 @@ suite("test_kafka_property_validation", "p0") { exception "Kafka property '${key}' is not allowed in Routine Load" } } - ["ssl.ca.location", "ssl.certificate.location", "ssl.key.location"].each { key -> + ["ssl.ca.location", "ssl.certificate.location", "ssl.key.location", "sasl.kerberos.keytab"].each { key -> test { sql """ CREATE ROUTINE LOAD test_kafka_property_validation_job ON test_kafka_property_validation diff --git a/thirdparty/patches/librdkafka-2.16.0.patch b/thirdparty/patches/librdkafka-2.16.0.patch index cf33d2d9b82b4b..2b4fe682761df1 100644 --- a/thirdparty/patches/librdkafka-2.16.0.patch +++ b/thirdparty/patches/librdkafka-2.16.0.patch @@ -47,3 +47,106 @@ rd_assert(TAILQ_EMPTY(&rkb->rkb_monitors)); rd_assert(TAILQ_EMPTY(&rkb->rkb_outbufs.rkbq_bufs)); rd_assert(TAILQ_EMPTY(&rkb->rkb_waitresps.rkbq_bufs)); +--- src/rdkafka_conf.h ++++ src/rdkafka_conf.h +@@ -321,6 +321,7 @@ + char *service_name; + char *kinit_cmd; + char *keytab; ++ char *ticket_cache; /* Doris-managed per-client cache */ + int relogin_min_time; + /** Protects .username and .password access after client + * instance has been created (see sasl_set_credentials()). */ +--- src/rdkafka_conf.c ++++ src/rdkafka_conf.c +@@ -1035,6 +1035,11 @@ + "-k %{sasl.kerberos.principal} || " + "kinit -t \"%{sasl.kerberos.keytab}\" -k %{sasl.kerberos.principal}", + _UNSUPPORTED_WIN32_GSSAPI}, ++ {_RK_GLOBAL, "sasl.kerberos.ticket.cache", _RK_C_STR, ++ _RK(sasl.ticket_cache), ++ "Doris-managed credential cache for this client's GSSAPI connections. " ++ "Requires the Cyrus SASL provider; does not change the process default cache.", ++ _UNSUPPORTED_WIN32_GSSAPI}, + {_RK_GLOBAL, "sasl.kerberos.keytab", _RK_C_STR, _RK(sasl.keytab), + "Path to Kerberos keytab file. " + "This configuration property is only used as a variable in " +--- src/rdkafka_sasl_cyrus.c ++++ src/rdkafka_sasl_cyrus.c +@@ -45,6 +45,9 @@ + #endif + + #include ++#include ++#include ++#include + + /** + * @brief Process-global lock to avoid simultaneous invocation of +@@ -67,6 +70,7 @@ + */ + typedef struct rd_kafka_sasl_cyrus_state_s { + sasl_conn_t *conn; ++ gss_cred_id_t gss_creds; /* Owned until after sasl_dispose(). */ + sasl_callback_t callbacks[16]; + } rd_kafka_sasl_cyrus_state_t; + +@@ -489,6 +493,10 @@ + sasl_dispose(&state->conn); + mtx_unlock(&rktrans->rktrans_rkb->rkb_rk->rk_conf.sasl.lock); + } ++ if (state->gss_creds != GSS_C_NO_CREDENTIAL) { ++ OM_uint32 minor; ++ gss_release_cred(&minor, &state->gss_creds); ++ } + rd_free(state); + rktrans->rktrans_sasl.state = NULL; + } +@@ -555,6 +563,47 @@ + return -1; + } + ++ if (rk->rk_conf.sasl.ticket_cache && ++ !strcmp(rk->rk_conf.sasl.mechanisms, "GSSAPI")) { ++ OM_uint32 major, minor, release_minor; ++ gss_name_t name = GSS_C_NO_NAME; ++ gss_buffer_desc principal = { ++ strlen(rk->rk_conf.sasl.principal), ++ rk->rk_conf.sasl.principal}; ++ gss_key_value_element_desc element = { ++ "ccache", rk->rk_conf.sasl.ticket_cache}; ++ gss_key_value_set_desc store = {1, &element}; ++ gss_OID_set_desc mechs = {1, (gss_OID)gss_mech_krb5}; ++ ++ /* Acquire anew on each connection, picking up tickets renewed ++ * by Doris. Never fall back to the process default identity. */ ++ major = gss_import_name(&minor, &principal, ++ GSS_KRB5_NT_PRINCIPAL_NAME, &name); ++ if (GSS_ERROR(major)) { ++ rd_snprintf(errstr, errstr_size, ++ "Cannot import Kerberos principal (%u/%u)", ++ (unsigned)major, (unsigned)minor); ++ return -1; ++ } ++ major = gss_acquire_cred_from( ++ &minor, name, 0, &mechs, GSS_C_INITIATE, &store, ++ &state->gss_creds, NULL, NULL); ++ gss_release_name(&release_minor, &name); ++ if (GSS_ERROR(major)) { ++ rd_snprintf(errstr, errstr_size, ++ "Cannot acquire managed Kerberos credentials (%u/%u)", ++ (unsigned)major, (unsigned)minor); ++ return -1; ++ } ++ r = sasl_setprop(state->conn, SASL_GSS_CREDS, state->gss_creds); ++ if (r != SASL_OK) { ++ rd_snprintf(errstr, errstr_size, ++ "Cannot bind managed Kerberos credentials: %s", ++ sasl_errstring(r, NULL, NULL)); ++ return -1; ++ } ++ } ++ + if (rk->rk_conf.debug & RD_KAFKA_DBG_SECURITY) { + const char *avail_mechs; + sasl_listmech(state->conn, NULL, NULL, " ", NULL, &avail_mechs, From 498a8a2b4401dabb2e5648159d7172142a7f150b Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Thu, 22 Oct 2026 10:57:37 +0800 Subject: [PATCH 03/12] [fix](load) Use the default Kerberos cache without modifying librdkafka ### What problem does this PR solve? Problem Summary: Remove the per-consumer credential binding patch from librdkafka. Keep native managed-keytab login and renewal functional by writing the operator-selected default Kerberos cache used by the unmodified client. Retain the existing shared-cache identity semantics. Serialize default-cache writers and do not delete the default cache when a refresh worker is released. Keep private-cache clients in a separate ticket-manager namespace. ### Release note This fix no longer modifies librdkafka or requires a special dependency rebuild. Managed keytabs retain automatic login and refresh without shell commands. The pre-existing multi-identity default-cache limitation remains documented. ### Check List (For Author) - Test: clang-format 16, build hygiene and git diff checks passed. Added default-cache login/refresh/lifetime and manager-key tests. BE UT entry point attempted but blocked by the Java environment (JDK 17 unavailable). Full compilation, clang-tidy and live Kerberos/Kafka tests remain unverified. - Behavior changed: Yes; native Kafka login writes the process default cache. - Does this need documentation: Yes; docs/routine-load-kafka-properties.md. --- be/src/common/kerberos/kerberos_config.cpp | 7 +- be/src/common/kerberos/kerberos_config.h | 13 ++- .../common/kerberos/kerberos_ticket_cache.cpp | 27 ++++- .../common/kerberos/kerberos_ticket_mgr.cpp | 4 +- be/src/common/kerberos/kerberos_ticket_mgr.h | 3 +- be/src/common/kerberos/krb5_interface.h | 1 + .../common/kerberos/krb5_interface_impl.cpp | 4 + be/src/common/kerberos/krb5_interface_impl.h | 1 + be/src/load/routine_load/kafka_properties.cpp | 12 +- be/src/load/routine_load/kafka_properties.h | 3 +- .../common/kerberos/kerberos_config_test.cpp | 12 ++ .../kerberos/kerberos_ticket_cache_test.cpp | 49 ++++++++- .../routine_load/kafka_properties_test.cpp | 45 ++++---- docs/routine-load-kafka-properties.md | 34 +++--- thirdparty/patches/librdkafka-2.16.0.patch | 103 ------------------ 15 files changed, 154 insertions(+), 164 deletions(-) diff --git a/be/src/common/kerberos/kerberos_config.cpp b/be/src/common/kerberos/kerberos_config.cpp index 24827ec6c8b125..c39338154497a1 100644 --- a/be/src/common/kerberos/kerberos_config.cpp +++ b/be/src/common/kerberos/kerberos_config.cpp @@ -27,8 +27,11 @@ namespace doris::kerberos { KerberosConfig::KerberosConfig() : _refresh_interval_second(3600), _min_time_before_refresh_second(600) {} -std::string KerberosConfig::get_hash_code(const std::string& principal, const std::string& keytab) { - return _get_hash_code(principal, keytab); +std::string KerberosConfig::get_hash_code(const std::string& principal, const std::string& keytab, + bool use_default_cache) { + // A Kafka default-cache request must not reuse a private cache for the same + // principal/keytab; the unmodified Kafka client cannot discover that file. + return (use_default_cache ? "default:" : "") + _get_hash_code(principal, keytab); } std::string KerberosConfig::_get_hash_code(const std::string& principal, diff --git a/be/src/common/kerberos/kerberos_config.h b/be/src/common/kerberos/kerberos_config.h index af6bf84746328d..954ec89d917ad7 100644 --- a/be/src/common/kerberos/kerberos_config.h +++ b/be/src/common/kerberos/kerberos_config.h @@ -53,15 +53,24 @@ class KerberosConfig { // Get the minimum time before refresh in seconds int32_t get_min_time_before_refresh_second() const { return _min_time_before_refresh_second; } - std::string get_hash_code() const { return _get_hash_code(_principal, _keytab_path); } + // Kafka uses the same default cache as the unmodified SASL client. Other + // users retain the existing manager-owned cache files. + void set_use_default_cache(bool value) { _use_default_cache = value; } + bool use_default_cache() const { return _use_default_cache; } + + std::string get_hash_code() const { + return get_hash_code(_principal, _keytab_path, _use_default_cache); + } // Use principal and keytab to generate a hash code. - static std::string get_hash_code(const std::string& principal, const std::string& keytab); + static std::string get_hash_code(const std::string& principal, const std::string& keytab, + bool use_default_cache = false); private: static std::string _get_hash_code(const std::string& principal, const std::string& keytab); private: + bool _use_default_cache = false; // Kerberos principal name (e.g., "user@REALM.COM") std::string _principal; // Path to the Kerberos keytab file diff --git a/be/src/common/kerberos/kerberos_ticket_cache.cpp b/be/src/common/kerberos/kerberos_ticket_cache.cpp index 49301c3fe80a58..a6bf376b403c81 100644 --- a/be/src/common/kerberos/kerberos_ticket_cache.cpp +++ b/be/src/common/kerberos/kerberos_ticket_cache.cpp @@ -37,7 +37,9 @@ KerberosTicketCache::KerberosTicketCache(const KerberosConfig& config, const std KerberosTicketCache::~KerberosTicketCache() { stop_periodic_refresh(); _cleanup_context(); - if (std::filesystem::exists(_ticket_cache_path)) { + // The default cache belongs to the process/deployment and may still be in + // use by Kafka or another client after this refresh worker is released. + if (!_config.use_default_cache() && std::filesystem::exists(_ticket_cache_path)) { std::filesystem::remove(_ticket_cache_path); } LOG(INFO) << "destroy kerberos ticket cache " << _ticket_cache_path @@ -46,8 +48,8 @@ KerberosTicketCache::~KerberosTicketCache() { Status KerberosTicketCache::initialize() { std::lock_guard lock(_mutex); - RETURN_IF_ERROR(_init_ticket_cache_path()); - Status st = _initialize_context(); + RETURN_IF_ERROR(_initialize_context()); + Status st = _init_ticket_cache_path(); LOG(INFO) << "initialized kerberos ticket cache " << _ticket_cache_path << " with principal: " << _config.get_principal() << " and keytab: " << _config.get_keytab_path() << ": " << st.to_string(); @@ -55,6 +57,16 @@ Status KerberosTicketCache::initialize() { } Status KerberosTicketCache::_init_ticket_cache_path() { + if (_config.use_default_cache()) { + // Honor operator-selected KRB5CCNAME/krb5.conf defaults without changing + // the process environment. Preserve the cache type prefix (FILE:, etc.). + const char* name = _krb5_interface->cc_default_name(_context); + if (name == nullptr) { + return Status::InternalError("Failed to resolve default Kerberos credential cache"); + } + _ticket_cache_path = name; + return Status::OK(); + } std::string cache_file_md5 = "doris_krb_" + _config.get_hash_code(); // The path should be with prefix "_ccache_root_dir" @@ -92,6 +104,15 @@ Status KerberosTicketCache::_init_ticket_cache_path() { Status KerberosTicketCache::login() { std::lock_guard lock(_mutex); + // Like librdkafka's old kinit lock, serialize writers to the shared default + // cache. This prevents interleaved initialize/store operations; it does not + // provide identity isolation when different jobs use the same default cache. + static std::mutex default_cache_login_mutex; + std::unique_lock default_cache_lock(default_cache_login_mutex, std::defer_lock); + if (_config.use_default_cache()) { + default_cache_lock.lock(); + } + krb5_keytab keytab = nullptr; krb5_ccache temp_ccache = nullptr; try { diff --git a/be/src/common/kerberos/kerberos_ticket_mgr.cpp b/be/src/common/kerberos/kerberos_ticket_mgr.cpp index 720167ba05f5dd..40b39efb1d02d1 100644 --- a/be/src/common/kerberos/kerberos_ticket_mgr.cpp +++ b/be/src/common/kerberos/kerberos_ticket_mgr.cpp @@ -127,8 +127,8 @@ Status KerberosTicketMgr::get_or_set_ticket_cache( } std::shared_ptr KerberosTicketMgr::get_ticket_cache( - const std::string& principal, const std::string& keytab_path) { - std::string key = KerberosConfig::get_hash_code(principal, keytab_path); + const std::string& principal, const std::string& keytab_path, bool use_default_cache) { + std::string key = KerberosConfig::get_hash_code(principal, keytab_path, use_default_cache); std::lock_guard lock(_mutex); auto it = _ticket_caches.find(key); diff --git a/be/src/common/kerberos/kerberos_ticket_mgr.h b/be/src/common/kerberos/kerberos_ticket_mgr.h index 07a73d6181b98d..e4f2eb3b8501d6 100644 --- a/be/src/common/kerberos/kerberos_ticket_mgr.h +++ b/be/src/common/kerberos/kerberos_ticket_mgr.h @@ -60,7 +60,8 @@ class KerberosTicketMgr { // Get the ticket cache object. This is used by HdfsHandler to hold a reference std::shared_ptr get_ticket_cache(const std::string& principal, - const std::string& keytab_path); + const std::string& keytab_path, + bool use_default_cache = false); // Get detailed information about all active Kerberos ticket caches std::vector get_krb_ticket_cache_info(); diff --git a/be/src/common/kerberos/krb5_interface.h b/be/src/common/kerberos/krb5_interface.h index 550fea7e62a9a7..26269e67483128 100644 --- a/be/src/common/kerberos/krb5_interface.h +++ b/be/src/common/kerberos/krb5_interface.h @@ -35,6 +35,7 @@ class Krb5Interface { virtual Status parse_name(krb5_context context, const char* name, krb5_principal* principal) = 0; virtual Status kt_resolve(krb5_context context, const char* name, krb5_keytab* keytab) = 0; + virtual const char* cc_default_name(krb5_context context) = 0; virtual Status cc_resolve(krb5_context context, const char* name, krb5_ccache* ccache) = 0; virtual Status get_init_creds_opt_alloc(krb5_context context, krb5_get_init_creds_opt** opt) = 0; diff --git a/be/src/common/kerberos/krb5_interface_impl.cpp b/be/src/common/kerberos/krb5_interface_impl.cpp index 6ae8d11b6c788d..d4c335fac44a5f 100644 --- a/be/src/common/kerberos/krb5_interface_impl.cpp +++ b/be/src/common/kerberos/krb5_interface_impl.cpp @@ -38,6 +38,10 @@ Status Krb5InterfaceImpl::kt_resolve(krb5_context context, const char* name, krb return _check_error(code, context, "Failed to resolve keytab"); } +const char* Krb5InterfaceImpl::cc_default_name(krb5_context context) { + return krb5_cc_default_name(context); +} + Status Krb5InterfaceImpl::cc_resolve(krb5_context context, const char* name, krb5_ccache* ccache) { krb5_error_code code = krb5_cc_resolve(context, name, ccache); return _check_error(code, context, "Failed to resolve credential cache"); diff --git a/be/src/common/kerberos/krb5_interface_impl.h b/be/src/common/kerberos/krb5_interface_impl.h index 916dcaf59dfc8c..7519e71344f58c 100644 --- a/be/src/common/kerberos/krb5_interface_impl.h +++ b/be/src/common/kerberos/krb5_interface_impl.h @@ -26,6 +26,7 @@ class Krb5InterfaceImpl : public Krb5Interface { Status init_context(krb5_context* context) override; Status parse_name(krb5_context context, const char* name, krb5_principal* principal) override; Status kt_resolve(krb5_context context, const char* name, krb5_keytab* keytab) override; + const char* cc_default_name(krb5_context context) override; Status cc_resolve(krb5_context context, const char* name, krb5_ccache* ccache) override; Status get_init_creds_opt_alloc(krb5_context context, krb5_get_init_creds_opt** opt) override; Status get_init_creds_keytab(krb5_context context, krb5_creds* creds, krb5_principal client, diff --git a/be/src/load/routine_load/kafka_properties.cpp b/be/src/load/routine_load/kafka_properties.cpp index 6a14f7a5ff9d9b..af8029a51c71f9 100644 --- a/be/src/load/routine_load/kafka_properties.cpp +++ b/be/src/load/routine_load/kafka_properties.cpp @@ -95,20 +95,16 @@ Status configure_kafka_kerberos(RdKafka::Conf& conf, const std::string& principa std::shared_ptr* ticket_cache) { kerberos::KerberosConfig config; config.set_principal_and_keytab(principal, keytab_path); + config.set_use_default_cache(true); std::shared_ptr cache; RETURN_IF_ERROR(manager.get_or_set_ticket_cache(config, &cache)); - // This internal property is deliberately absent from the SQL allowlist. - // The patched Cyrus provider acquires credentials from this cache for each - // connection, so two consumers never change each other's default identity. + // Unmodified librdkafka discovers the process default Kerberos cache. + // Native login and renewal therefore write that cache, just as kinit did. + // This retains the existing shared-cache behavior, not per-job isolation. std::string error; if (conf.set("sasl.kerberos.principal", principal, error) != RdKafka::Conf::CONF_OK) { return Status::InternalError("PAUSE: failed to set Kafka Kerberos principal: {}", error); } - if (conf.set("sasl.kerberos.ticket.cache", "FILE:" + cache->get_ticket_cache_path(), error) != - RdKafka::Conf::CONF_OK) { - return Status::InternalError("PAUSE: failed to bind Kafka Kerberos ticket cache: {}", - error); - } *ticket_cache = std::move(cache); return Status::OK(); } diff --git a/be/src/load/routine_load/kafka_properties.h b/be/src/load/routine_load/kafka_properties.h index abd84e54acd6bc..4dc59b7f62548f 100644 --- a/be/src/load/routine_load/kafka_properties.h +++ b/be/src/load/routine_load/kafka_properties.h @@ -37,7 +37,8 @@ class KerberosTicketMgr; Status configure_kafka_security(RdKafka::Conf& conf, const std::map& properties); // keytab_path must already have been resolved by SmallFileMgr. Keep the returned -// cache alive until the Kafka consumer (and all of its connections) is destroyed. +// refresh worker alive until the Kafka consumer is destroyed. Tickets are written +// to the process default cache, preserving the existing shared-cache semantics. Status configure_kafka_kerberos(RdKafka::Conf& conf, const std::string& principal, const std::string& keytab_path, kerberos::KerberosTicketMgr& manager, diff --git a/be/test/common/kerberos/kerberos_config_test.cpp b/be/test/common/kerberos/kerberos_config_test.cpp index c27be4bfd312fc..9aa66b8572c3ca 100644 --- a/be/test/common/kerberos/kerberos_config_test.cpp +++ b/be/test/common/kerberos/kerberos_config_test.cpp @@ -57,4 +57,16 @@ TEST_F(KerberosConfigTest, SetterAndGetter) { EXPECT_EQ(config.get_min_time_before_refresh_second(), 800); } +TEST_F(KerberosConfigTest, DefaultCacheModeHasSeparateManagerKey) { + KerberosConfig config; + config.set_principal_and_keytab("user@REALM", "/managed/keytab"); + EXPECT_FALSE(config.use_default_cache()); + const auto private_key = config.get_hash_code(); + EXPECT_EQ(private_key, KerberosConfig::get_hash_code("user@REALM", "/managed/keytab")); + config.set_use_default_cache(true); + EXPECT_NE(private_key, config.get_hash_code()); + EXPECT_EQ(config.get_hash_code(), + KerberosConfig::get_hash_code("user@REALM", "/managed/keytab", true)); +} + } // namespace doris::kerberos diff --git a/be/test/common/kerberos/kerberos_ticket_cache_test.cpp b/be/test/common/kerberos/kerberos_ticket_cache_test.cpp index 1f8e2e363b7941..3b8cc5dcc448f1 100644 --- a/be/test/common/kerberos/kerberos_ticket_cache_test.cpp +++ b/be/test/common/kerberos/kerberos_ticket_cache_test.cpp @@ -36,6 +36,7 @@ class MockKrb5Interface : public Krb5Interface { MOCK_METHOD(Status, init_context, (krb5_context*), (override)); MOCK_METHOD(Status, parse_name, (krb5_context, const char*, krb5_principal*), (override)); MOCK_METHOD(Status, kt_resolve, (krb5_context, const char*, krb5_keytab*), (override)); + MOCK_METHOD(const char*, cc_default_name, (krb5_context), (override)); MOCK_METHOD(Status, cc_resolve, (krb5_context, const char*, krb5_ccache*), (override)); MOCK_METHOD(Status, get_init_creds_opt_alloc, (krb5_context, krb5_get_init_creds_opt**), (override)); @@ -91,6 +92,15 @@ class KerberosTicketCacheTest : public testing::Test { } } + void ResetForDefaultCache() { + _cache.reset(); + _config.set_use_default_cache(true); + auto mock = std::make_unique>(); + _mock_krb5_ptr = mock.get(); + _cache = + std::make_unique(_config, _test_dir.string(), std::move(mock)); + } + // Helper function to set up basic expectations for initialization void SetupBasicInitExpectations() { EXPECT_CALL(*_mock_krb5_ptr, init_context(_)).WillOnce(testing::Return(Status::OK())); @@ -109,7 +119,8 @@ class KerberosTicketCacheTest : public testing::Test { void ExpectForLogin(const std::string& cache_path, int lifetime_seconds = 3600) { // Setup expectations for login EXPECT_CALL(*_mock_krb5_ptr, kt_resolve(_, _, _)).WillOnce(testing::Return(Status::OK())); - EXPECT_CALL(*_mock_krb5_ptr, cc_resolve(_, _, _)).WillOnce(testing::Return(Status::OK())); + EXPECT_CALL(*_mock_krb5_ptr, cc_resolve(_, testing::StrEq(cache_path), _)) + .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, get_init_creds_opt_alloc(_, _)) .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, get_init_creds_keytab(_, _, _, _, _, _, _)) @@ -126,7 +137,9 @@ class KerberosTicketCacheTest : public testing::Test { EXPECT_CALL(*_mock_krb5_ptr, cc_store_cred(_, _, _)) .WillOnce(testing::DoAll( testing::Invoke([this, cache_path](krb5_context, krb5_ccache, krb5_creds*) { - SimulateTicketCacheFileUpdate(cache_path); + SimulateTicketCacheFileUpdate(cache_path.starts_with("FILE:") + ? cache_path.substr(5) + : cache_path); return Status::OK(); }))); @@ -215,6 +228,38 @@ TEST_F(KerberosTicketCacheTest, RefreshTickets) { ASSERT_GT(new_write_time, initial_write_time); } +TEST_F(KerberosTicketCacheTest, DefaultCacheIsUsedForLoginAndRefreshAndNotDeleted) { + const auto file = (_test_dir / "operator-cache").string(); + for (const auto* prefix : {"", "FILE:"}) { + ResetForDefaultCache(); + std::ofstream(file) << "existing tickets"; + const std::string name = prefix + file; + SetupBasicInitExpectations(); + EXPECT_CALL(*_mock_krb5_ptr, cc_default_name(_)).WillOnce(testing::Return(name.c_str())); + ASSERT_TRUE(_cache->initialize().ok()); + EXPECT_EQ(name, _cache->get_ticket_cache_path()); + // Initialization must not remove an operator-owned default cache. + std::ifstream input(file); + std::string contents; + std::getline(input, contents); + EXPECT_EQ("existing tickets", contents); + input.close(); + ExpectForLogin(name); + ASSERT_TRUE(_cache->login().ok()); + ExpectForLogin(name); + ASSERT_TRUE(_cache->refresh_tickets().ok()); + _cache.reset(); + EXPECT_TRUE(std::filesystem::exists(file)); + } +} + +TEST_F(KerberosTicketCacheTest, DefaultCacheResolutionFailureIsReported) { + ResetForDefaultCache(); + SetupBasicInitExpectations(); + EXPECT_CALL(*_mock_krb5_ptr, cc_default_name(_)).WillOnce(testing::Return(nullptr)); + EXPECT_FALSE(_cache->initialize().ok()); +} + TEST_F(KerberosTicketCacheTest, RefreshIntervalTracksTicketLifetime) { SetupBasicInitExpectations(); ASSERT_TRUE(_cache->initialize().ok()); diff --git a/be/test/load/routine_load/kafka_properties_test.cpp b/be/test/load/routine_load/kafka_properties_test.cpp index 31a4adba085ee3..95193c062487bb 100644 --- a/be/test/load/routine_load/kafka_properties_test.cpp +++ b/be/test/load/routine_load/kafka_properties_test.cpp @@ -166,35 +166,30 @@ class KafkaMockTicketMgr : public kerberos::KerberosTicketMgr { }; } // namespace -TEST(KafkaPropertiesTest, ManagedLoginStartsRefreshAndBindsDistinctCaches) { +TEST(KafkaPropertiesTest, ManagedLoginStartsRefreshInDefaultCacheMode) { KafkaMockTicketMgr manager; - std::shared_ptr first, second, shared; - std::unique_ptr a(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); - std::unique_ptr b(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); - ASSERT_TRUE(configure_kafka_security(*a, {}).ok()); - ASSERT_TRUE(configure_kafka_security(*b, {}).ok()); - ASSERT_TRUE( - configure_kafka_kerberos(*a, "alice@REALM", "/managed/1.keytab", manager, &first).ok()); - ASSERT_TRUE( - configure_kafka_kerberos(*b, "bob@REALM", "/managed/2.keytab", manager, &second).ok()); - ASSERT_NE(first, second); - std::string a_path, b_path, principal, interval; - ASSERT_EQ(RdKafka::Conf::CONF_OK, a->get("sasl.kerberos.ticket.cache", a_path)); - ASSERT_EQ(RdKafka::Conf::CONF_OK, b->get("sasl.kerberos.ticket.cache", b_path)); - EXPECT_EQ("FILE:" + first->get_ticket_cache_path(), a_path); - EXPECT_EQ("FILE:" + second->get_ticket_cache_path(), b_path); - EXPECT_NE(a_path, b_path); - ASSERT_EQ(RdKafka::Conf::CONF_OK, a->get("sasl.kerberos.principal", principal)); + std::shared_ptr cache, shared; + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + ASSERT_TRUE(configure_kafka_security(*conf, {}).ok()); + ASSERT_TRUE(configure_kafka_kerberos(*conf, "alice@REALM", "/managed/1.keytab", manager, &cache) + .ok()); + ASSERT_TRUE(cache->get_config().use_default_cache()); + EXPECT_EQ(cache, manager.get_ticket_cache("alice@REALM", "/managed/1.keytab", true)); + EXPECT_EQ(nullptr, manager.get_ticket_cache("alice@REALM", "/managed/1.keytab")); + std::string principal, interval; + ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.principal", principal)); EXPECT_EQ("alice@REALM", principal); - ASSERT_EQ(RdKafka::Conf::CONF_OK, a->get("sasl.kerberos.min.time.before.relogin", interval)); + ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.min.time.before.relogin", interval)); EXPECT_EQ("0", interval); - // Reusing an identity must retain the same cache and its renewal worker. - ASSERT_TRUE(configure_kafka_kerberos(*a, "alice@REALM", "/managed/1.keytab", manager, &shared) - .ok()); - EXPECT_EQ(first, shared); + // Only upstream librdkafka properties are needed; the same identity reuses + // its native refresh worker without creating a private Kafka cache binding. + ASSERT_TRUE( + configure_kafka_kerberos(*conf, "alice@REALM", "/managed/1.keytab", manager, &shared) + .ok()); + EXPECT_EQ(cache, shared); } -TEST(KafkaPropertiesTest, FailedManagedLoginDoesNotBindOrPublishCache) { +TEST(KafkaPropertiesTest, FailedManagedLoginDoesNotPublishCache) { KafkaMockTicketMgr manager; manager.fail_login = true; std::shared_ptr cache; @@ -204,7 +199,7 @@ TEST(KafkaPropertiesTest, FailedManagedLoginDoesNotBindOrPublishCache) { EXPECT_FALSE(status.ok()); EXPECT_NE(status.to_string().find("KDC unavailable"), std::string::npos); EXPECT_EQ(nullptr, cache); - EXPECT_EQ(nullptr, manager.get_ticket_cache("user@REALM", "/managed/1.keytab")); + EXPECT_EQ(nullptr, manager.get_ticket_cache("user@REALM", "/managed/1.keytab", true)); } } // namespace doris diff --git a/docs/routine-load-kafka-properties.md b/docs/routine-load-kafka-properties.md index c3a61334025b97..5356fe69cae137 100644 --- a/docs/routine-load-kafka-properties.md +++ b/docs/routine-load-kafka-properties.md @@ -46,17 +46,22 @@ when a keytab is supplied. Raw BE keytab paths are rejected. The BE still needs operator-configured Kerberos realm/KDC settings and network access to the KDC. After resolving the managed file, Doris's native Kerberos ticket manager calls -libkrb5 to acquire tickets. It maintains separate caches for different -principal/keytab pairs and refreshes tickets at half their lifetime or the -configured refresh interval, whichever is shorter. A refresh failure is logged -and retried on the next poll. Consumers retain the cache while they are alive. - -Doris's librdkafka patch binds each GSSAPI connection to credentials acquired -from that consumer's cache using `gss_acquire_cred_from` and `SASL_GSS_CREDS`. -Connections acquire fresh credential handles on reconnect, picking up renewed -tickets. No process-wide `KRB5CCNAME` switching is used. Cache acquisition or -binding failure fails authentication instead of selecting another job's identity. -The internal `sasl.kerberos.ticket.cache` property cannot be set through SQL. +libkrb5 to acquire tickets and writes the process default credential cache, +selected by the operator's `KRB5CCNAME` or Kerberos configuration. The unmodified +librdkafka/SASL client discovers credentials through its existing default-cache +behavior. Doris does not change `KRB5CCNAME` while jobs are running. + +Doris refreshes tickets at half their lifetime or the configured refresh +interval, whichever is shorter. A refresh failure is logged and retried on the +next poll. Consumers retain their native refresh worker while they are alive. +Default-cache writers are serialized, and releasing a worker does not delete the +default cache. Other users of the ticket manager retain its private-cache mode. + +This preserves the existing shared-cache limitation: jobs using different +identities on one BE can overwrite the same default cache during login or +refresh. This change does not provide per-job credential isolation. Use a common +Kafka identity for jobs sharing a default cache; multi-identity isolation is +outside the scope of this fix. `sasl.kerberos.kinit.cmd` remains forbidden. The librdkafka `sasl.kerberos.min.time.before.relogin` option accepts only the literal `0` and @@ -76,7 +81,7 @@ cache; in that mode the operator remains responsible for ticket renewal. the job. ALTER merges properties and cannot remove an existing rejected key. - FE explicitly sends the zero Kerberos refresh interval to protect requests sent to older BEs during a rolling upgrade. Native automatic login requires - upgraded BEs; an older BE cannot consume the new per-job ticket cache. Upgrade + upgraded BEs; an older BE does not run the native refresh worker. Upgrade eligible BEs before relying on managed keytab login. - Upgraded BEs validate the complete property map before applying any custom setting, covering metadata probes and tasks from older FEs. They disable @@ -84,8 +89,7 @@ cache; in that mode the operator remains responsible for ticket renewal. - Both sides should be upgraded. An old FE sending work to an old BE retains its previous behavior until one of those components is upgraded. -Rebuild the `librdkafka-2.16.0` third-party dependency with the updated Doris patch -before building BE. An unpatched library fails the mandatory cache-binding -configuration; it does not silently fall back to the process cache. +This fix adds no librdkafka patch or custom librdkafka property and requires no +third-party dependency rebuild beyond the normal Doris build requirements. No Thrift field IDs, enum values, or persisted data formats change. diff --git a/thirdparty/patches/librdkafka-2.16.0.patch b/thirdparty/patches/librdkafka-2.16.0.patch index 2b4fe682761df1..cf33d2d9b82b4b 100644 --- a/thirdparty/patches/librdkafka-2.16.0.patch +++ b/thirdparty/patches/librdkafka-2.16.0.patch @@ -47,106 +47,3 @@ rd_assert(TAILQ_EMPTY(&rkb->rkb_monitors)); rd_assert(TAILQ_EMPTY(&rkb->rkb_outbufs.rkbq_bufs)); rd_assert(TAILQ_EMPTY(&rkb->rkb_waitresps.rkbq_bufs)); ---- src/rdkafka_conf.h -+++ src/rdkafka_conf.h -@@ -321,6 +321,7 @@ - char *service_name; - char *kinit_cmd; - char *keytab; -+ char *ticket_cache; /* Doris-managed per-client cache */ - int relogin_min_time; - /** Protects .username and .password access after client - * instance has been created (see sasl_set_credentials()). */ ---- src/rdkafka_conf.c -+++ src/rdkafka_conf.c -@@ -1035,6 +1035,11 @@ - "-k %{sasl.kerberos.principal} || " - "kinit -t \"%{sasl.kerberos.keytab}\" -k %{sasl.kerberos.principal}", - _UNSUPPORTED_WIN32_GSSAPI}, -+ {_RK_GLOBAL, "sasl.kerberos.ticket.cache", _RK_C_STR, -+ _RK(sasl.ticket_cache), -+ "Doris-managed credential cache for this client's GSSAPI connections. " -+ "Requires the Cyrus SASL provider; does not change the process default cache.", -+ _UNSUPPORTED_WIN32_GSSAPI}, - {_RK_GLOBAL, "sasl.kerberos.keytab", _RK_C_STR, _RK(sasl.keytab), - "Path to Kerberos keytab file. " - "This configuration property is only used as a variable in " ---- src/rdkafka_sasl_cyrus.c -+++ src/rdkafka_sasl_cyrus.c -@@ -45,6 +45,9 @@ - #endif - - #include -+#include -+#include -+#include - - /** - * @brief Process-global lock to avoid simultaneous invocation of -@@ -67,6 +70,7 @@ - */ - typedef struct rd_kafka_sasl_cyrus_state_s { - sasl_conn_t *conn; -+ gss_cred_id_t gss_creds; /* Owned until after sasl_dispose(). */ - sasl_callback_t callbacks[16]; - } rd_kafka_sasl_cyrus_state_t; - -@@ -489,6 +493,10 @@ - sasl_dispose(&state->conn); - mtx_unlock(&rktrans->rktrans_rkb->rkb_rk->rk_conf.sasl.lock); - } -+ if (state->gss_creds != GSS_C_NO_CREDENTIAL) { -+ OM_uint32 minor; -+ gss_release_cred(&minor, &state->gss_creds); -+ } - rd_free(state); - rktrans->rktrans_sasl.state = NULL; - } -@@ -555,6 +563,47 @@ - return -1; - } - -+ if (rk->rk_conf.sasl.ticket_cache && -+ !strcmp(rk->rk_conf.sasl.mechanisms, "GSSAPI")) { -+ OM_uint32 major, minor, release_minor; -+ gss_name_t name = GSS_C_NO_NAME; -+ gss_buffer_desc principal = { -+ strlen(rk->rk_conf.sasl.principal), -+ rk->rk_conf.sasl.principal}; -+ gss_key_value_element_desc element = { -+ "ccache", rk->rk_conf.sasl.ticket_cache}; -+ gss_key_value_set_desc store = {1, &element}; -+ gss_OID_set_desc mechs = {1, (gss_OID)gss_mech_krb5}; -+ -+ /* Acquire anew on each connection, picking up tickets renewed -+ * by Doris. Never fall back to the process default identity. */ -+ major = gss_import_name(&minor, &principal, -+ GSS_KRB5_NT_PRINCIPAL_NAME, &name); -+ if (GSS_ERROR(major)) { -+ rd_snprintf(errstr, errstr_size, -+ "Cannot import Kerberos principal (%u/%u)", -+ (unsigned)major, (unsigned)minor); -+ return -1; -+ } -+ major = gss_acquire_cred_from( -+ &minor, name, 0, &mechs, GSS_C_INITIATE, &store, -+ &state->gss_creds, NULL, NULL); -+ gss_release_name(&release_minor, &name); -+ if (GSS_ERROR(major)) { -+ rd_snprintf(errstr, errstr_size, -+ "Cannot acquire managed Kerberos credentials (%u/%u)", -+ (unsigned)major, (unsigned)minor); -+ return -1; -+ } -+ r = sasl_setprop(state->conn, SASL_GSS_CREDS, state->gss_creds); -+ if (r != SASL_OK) { -+ rd_snprintf(errstr, errstr_size, -+ "Cannot bind managed Kerberos credentials: %s", -+ sasl_errstring(r, NULL, NULL)); -+ return -1; -+ } -+ } -+ - if (rk->rk_conf.debug & RD_KAFKA_DBG_SECURITY) { - const char *avail_mechs; - sasl_listmech(state->conn, NULL, NULL, " ", NULL, &avail_mechs, From 0b6f54d9399ce0abe48f49d5348c6b3d45cce180 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Thu, 22 Oct 2026 11:00:24 +0800 Subject: [PATCH 04/12] [doc](load) Remove standalone Kafka property validation document ### What problem does this PR solve? Problem Summary: Remove the standalone Routine Load Kafka property document from this PR as requested. Keep configuration and upgrade notes in the PR description. ### Release note None ### Check List (For Author) - Test: No runtime test needed; documentation deletion only. git diff --check passed. - Behavior changed: No. - Does this need documentation: No; the requested standalone document is removed. --- docs/routine-load-kafka-properties.md | 95 --------------------------- 1 file changed, 95 deletions(-) delete mode 100644 docs/routine-load-kafka-properties.md diff --git a/docs/routine-load-kafka-properties.md b/docs/routine-load-kafka-properties.md deleted file mode 100644 index 5356fe69cae137..00000000000000 --- a/docs/routine-load-kafka-properties.md +++ /dev/null @@ -1,95 +0,0 @@ -# Routine Load Kafka property validation - -Kafka `property.*` options are checked against an explicit allowlist in both FE -and BE. Unknown options are rejected instead of being forwarded to librdkafka. -The shared source of truth is `KAFKA_ALLOWED_PROPERTIES`, -`KAFKA_FILE_PROPERTIES`, and `KAFKA_DORIS_PROPERTIES` in -[`gensrc/thrift/BackendService.thrift`](../gensrc/thrift/BackendService.thrift). -New librdkafka options require review before they can be exposed through SQL. - -Consumer identity, fetch tuning, offset isolation, TLS, SASL PLAIN/SCRAM, -OAuth bearer authentication, and Doris AWS MSK IAM options remain supported -through the listed properties. Both `property.sasl.mechanism` and -`property.sasl.mechanisms` are accepted. Existing `aws.*` and `property.aws.*` -spellings are supported for the listed AWS options. Java-only properties such -as `sasl.jaas.config` are rejected; use librdkafka's SASL properties instead. - -## Certificates - -Certificate/key/CA/CRL/keystore path options must use `FILE:` referencing a -file registered with `CREATE FILE` in the job's database and the `kafka` -catalog. Raw absolute or relative paths on a BE are rejected. FE resolves the -registered file to `FILE::`; BE downloads and resolves that managed file -before passing its local path to librdkafka. Inline PEM properties remain -available where listed in the allowlist. - -Executable module settings, including `plugin.library.paths`, `ssl.engine.id`, -`ssl.engine.location`, and `ssl.providers`, are rejected even when their values -use `FILE:`. - -## Kerberos - -Routine Load supports automatic Kerberos login and ticket refresh with a -per-job principal and a Doris-managed keytab: - -```sql -"property.security.protocol" = "SASL_SSL", -"property.sasl.mechanism" = "GSSAPI", -"property.sasl.kerberos.service.name" = "kafka", -"property.sasl.kerberos.principal" = "loader@EXAMPLE.COM", -"property.sasl.kerberos.keytab" = "FILE:loader.keytab" -``` - -Register `loader.keytab` with `CREATE FILE` in the job's database and `kafka` -catalog first. An explicit, nonempty principal and a SASL protocol are required -when a keytab is supplied. Raw BE keytab paths are rejected. The BE still needs -operator-configured Kerberos realm/KDC settings and network access to the KDC. - -After resolving the managed file, Doris's native Kerberos ticket manager calls -libkrb5 to acquire tickets and writes the process default credential cache, -selected by the operator's `KRB5CCNAME` or Kerberos configuration. The unmodified -librdkafka/SASL client discovers credentials through its existing default-cache -behavior. Doris does not change `KRB5CCNAME` while jobs are running. - -Doris refreshes tickets at half their lifetime or the configured refresh -interval, whichever is shorter. A refresh failure is logged and retried on the -next poll. Consumers retain their native refresh worker while they are alive. -Default-cache writers are serialized, and releasing a worker does not delete the -default cache. Other users of the ticket manager retain its private-cache mode. - -This preserves the existing shared-cache limitation: jobs using different -identities on one BE can overwrite the same default cache during login or -refresh. This change does not provide per-job credential isolation. Use a common -Kafka identity for jobs sharing a default cache; multi-identity isolation is -outside the scope of this fix. - -`sasl.kerberos.kinit.cmd` remains forbidden. The librdkafka -`sasl.kerberos.min.time.before.relogin` option accepts only the literal `0` and -is forced to `0` by Doris: it disables librdkafka's shell command, **not** Doris's -native renewal worker. No SQL property is interpolated into a shell command. -Users who explicitly configured this interval must remove it or set it to `0`. - -Without a keytab, GSSAPI can continue using an operator-managed default ticket -cache; in that mode the operator remains responsible for ticket renewal. - -## Upgrade behavior - -- FE validates both CREATE and ALTER statements before forwarding properties. -- Persisted job properties are validated again before file conversion and - metadata requests. Jobs containing rejected properties fail with an error - identifying the property; remove the incompatible configuration and recreate - the job. ALTER merges properties and cannot remove an existing rejected key. -- FE explicitly sends the zero Kerberos refresh interval to protect requests - sent to older BEs during a rolling upgrade. Native automatic login requires - upgraded BEs; an older BE does not run the native refresh worker. Upgrade - eligible BEs before relying on managed keytab login. -- Upgraded BEs validate the complete property map before applying any custom - setting, covering metadata probes and tasks from older FEs. They disable - internal Kerberos commands even when the FE did not send the interval. -- Both sides should be upgraded. An old FE sending work to an old BE retains - its previous behavior until one of those components is upgraded. - -This fix adds no librdkafka patch or custom librdkafka property and requires no -third-party dependency rebuild beyond the normal Doris build requirements. - -No Thrift field IDs, enum values, or persisted data formats change. From 50fc95ab2515c7b83364cca4b1c3e46bb31f7bb9 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Thu, 22 Oct 2026 17:15:33 +0800 Subject: [PATCH 05/12] [fix](load) Limit Kafka property PR to security hardening ### What problem does this PR solve? Problem Summary: Keep the PR focused on FE/BE Kafka property allowlists and mandatory suppression of librdkafka shell commands. Remove native Kerberos login, automatic refresh, and ticket-cache adaptations and their tests. Keep the previously requested librdkafka and documentation removals intact. ### Release note Job-level keytabs and custom kinit commands are rejected. Kerberos jobs require operator-maintained ticket caches on eligible BEs; this security fix disables shell-based automatic login and renewal without adding a replacement worker. ### Check List (For Author) - Test: FE Checkstyle, clang-format 16, BE build hygiene and diff checks passed. FE/BE/regression entry points attempted; runtime tests blocked by missing JDK 17. Full compilation and live Kafka/Kerberos validation remain unverified. - Behavior changed: Yes; native auto-authentication is outside this PR's scope. - Does this need documentation: Upgrade requirements are in the PR description. --- be/src/common/kerberos/kerberos_config.cpp | 7 +- be/src/common/kerberos/kerberos_config.h | 13 +-- .../common/kerberos/kerberos_ticket_cache.cpp | 59 ++-------- .../common/kerberos/kerberos_ticket_cache.h | 6 +- .../common/kerberos/kerberos_ticket_mgr.cpp | 4 +- be/src/common/kerberos/kerberos_ticket_mgr.h | 3 +- be/src/common/kerberos/krb5_interface.h | 1 - .../common/kerberos/krb5_interface_impl.cpp | 4 - be/src/common/kerberos/krb5_interface_impl.h | 1 - be/src/load/routine_load/data_consumer.cpp | 15 +-- be/src/load/routine_load/data_consumer.h | 7 -- be/src/load/routine_load/kafka_properties.cpp | 49 +------- be/src/load/routine_load/kafka_properties.h | 12 -- .../common/kerberos/kerberos_config_test.cpp | 12 -- .../kerberos/kerberos_ticket_cache_test.cpp | 75 +------------ .../routine_load/kafka_properties_test.cpp | 106 +----------------- .../kafka/KafkaDataSourceProperties.java | 2 +- .../KafkaPropertyValidationTest.java | 4 +- gensrc/thrift/BackendService.thrift | 1 - .../test_kafka_property_validation.groovy | 4 +- 20 files changed, 35 insertions(+), 350 deletions(-) diff --git a/be/src/common/kerberos/kerberos_config.cpp b/be/src/common/kerberos/kerberos_config.cpp index c39338154497a1..24827ec6c8b125 100644 --- a/be/src/common/kerberos/kerberos_config.cpp +++ b/be/src/common/kerberos/kerberos_config.cpp @@ -27,11 +27,8 @@ namespace doris::kerberos { KerberosConfig::KerberosConfig() : _refresh_interval_second(3600), _min_time_before_refresh_second(600) {} -std::string KerberosConfig::get_hash_code(const std::string& principal, const std::string& keytab, - bool use_default_cache) { - // A Kafka default-cache request must not reuse a private cache for the same - // principal/keytab; the unmodified Kafka client cannot discover that file. - return (use_default_cache ? "default:" : "") + _get_hash_code(principal, keytab); +std::string KerberosConfig::get_hash_code(const std::string& principal, const std::string& keytab) { + return _get_hash_code(principal, keytab); } std::string KerberosConfig::_get_hash_code(const std::string& principal, diff --git a/be/src/common/kerberos/kerberos_config.h b/be/src/common/kerberos/kerberos_config.h index 954ec89d917ad7..af6bf84746328d 100644 --- a/be/src/common/kerberos/kerberos_config.h +++ b/be/src/common/kerberos/kerberos_config.h @@ -53,24 +53,15 @@ class KerberosConfig { // Get the minimum time before refresh in seconds int32_t get_min_time_before_refresh_second() const { return _min_time_before_refresh_second; } - // Kafka uses the same default cache as the unmodified SASL client. Other - // users retain the existing manager-owned cache files. - void set_use_default_cache(bool value) { _use_default_cache = value; } - bool use_default_cache() const { return _use_default_cache; } - - std::string get_hash_code() const { - return get_hash_code(_principal, _keytab_path, _use_default_cache); - } + std::string get_hash_code() const { return _get_hash_code(_principal, _keytab_path); } // Use principal and keytab to generate a hash code. - static std::string get_hash_code(const std::string& principal, const std::string& keytab, - bool use_default_cache = false); + static std::string get_hash_code(const std::string& principal, const std::string& keytab); private: static std::string _get_hash_code(const std::string& principal, const std::string& keytab); private: - bool _use_default_cache = false; // Kerberos principal name (e.g., "user@REALM.COM") std::string _principal; // Path to the Kerberos keytab file diff --git a/be/src/common/kerberos/kerberos_ticket_cache.cpp b/be/src/common/kerberos/kerberos_ticket_cache.cpp index a6bf376b403c81..eb87ff368fff37 100644 --- a/be/src/common/kerberos/kerberos_ticket_cache.cpp +++ b/be/src/common/kerberos/kerberos_ticket_cache.cpp @@ -17,7 +17,6 @@ #include "common/kerberos/kerberos_ticket_cache.h" -#include #include #include #include @@ -37,9 +36,7 @@ KerberosTicketCache::KerberosTicketCache(const KerberosConfig& config, const std KerberosTicketCache::~KerberosTicketCache() { stop_periodic_refresh(); _cleanup_context(); - // The default cache belongs to the process/deployment and may still be in - // use by Kafka or another client after this refresh worker is released. - if (!_config.use_default_cache() && std::filesystem::exists(_ticket_cache_path)) { + if (std::filesystem::exists(_ticket_cache_path)) { std::filesystem::remove(_ticket_cache_path); } LOG(INFO) << "destroy kerberos ticket cache " << _ticket_cache_path @@ -48,8 +45,8 @@ KerberosTicketCache::~KerberosTicketCache() { Status KerberosTicketCache::initialize() { std::lock_guard lock(_mutex); - RETURN_IF_ERROR(_initialize_context()); - Status st = _init_ticket_cache_path(); + RETURN_IF_ERROR(_init_ticket_cache_path()); + Status st = _initialize_context(); LOG(INFO) << "initialized kerberos ticket cache " << _ticket_cache_path << " with principal: " << _config.get_principal() << " and keytab: " << _config.get_keytab_path() << ": " << st.to_string(); @@ -57,16 +54,6 @@ Status KerberosTicketCache::initialize() { } Status KerberosTicketCache::_init_ticket_cache_path() { - if (_config.use_default_cache()) { - // Honor operator-selected KRB5CCNAME/krb5.conf defaults without changing - // the process environment. Preserve the cache type prefix (FILE:, etc.). - const char* name = _krb5_interface->cc_default_name(_context); - if (name == nullptr) { - return Status::InternalError("Failed to resolve default Kerberos credential cache"); - } - _ticket_cache_path = name; - return Status::OK(); - } std::string cache_file_md5 = "doris_krb_" + _config.get_hash_code(); // The path should be with prefix "_ccache_root_dir" @@ -104,15 +91,6 @@ Status KerberosTicketCache::_init_ticket_cache_path() { Status KerberosTicketCache::login() { std::lock_guard lock(_mutex); - // Like librdkafka's old kinit lock, serialize writers to the shared default - // cache. This prevents interleaved initialize/store operations; it does not - // provide identity isolation when different jobs use the same default cache. - static std::mutex default_cache_login_mutex; - std::unique_lock default_cache_lock(default_cache_login_mutex, std::defer_lock); - if (_config.use_default_cache()) { - default_cache_lock.lock(); - } - krb5_keytab keytab = nullptr; krb5_ccache temp_ccache = nullptr; try { @@ -222,38 +200,23 @@ Status KerberosTicketCache::refresh_tickets() { } } -std::chrono::milliseconds KerberosTicketCache::get_refresh_interval() { - std::lock_guard lock(_mutex); - std::chrono::milliseconds interval = - std::chrono::seconds(std::max(1, _config.get_refresh_interval_second())); - if (_ticket_lifetime_sec > 0) { - interval = std::min(interval, std::chrono::milliseconds(_ticket_lifetime_sec * 500)); - } - return interval; -} - void KerberosTicketCache::start_periodic_refresh() { _should_stop_refresh = false; _refresh_thread = std::make_unique([this]() { - auto refresh_interval = get_refresh_interval(); - auto next_refresh = std::chrono::steady_clock::now() + refresh_interval; + auto refresh_interval = std::chrono::milliseconds( + static_cast(_config.get_refresh_interval_second() * 1000)); + auto sleep_duration = _refresh_thread_sleep_time; + std::chrono::milliseconds accumulated_time(0); while (!_should_stop_refresh) { - // Short-lived tickets must not wait for the default five-second poll. - auto sleep_duration = std::min(_refresh_thread_sleep_time, refresh_interval); std::this_thread::sleep_for(sleep_duration); - if (_should_stop_refresh) { - break; - } - // Use elapsed clock time: a delayed worker must not treat a long - // scheduler stall as only one polling interval. - if (std::chrono::steady_clock::now() >= next_refresh) { + accumulated_time += sleep_duration; + if (accumulated_time >= refresh_interval) { + accumulated_time = std::chrono::milliseconds(0); // Reset accumulated time Status st = refresh_tickets(); if (!st.ok()) { - // Keep the refresh due: retry on the next poll after a KDC outage. + // ignore and continue LOG(WARNING) << st.to_string(); } else { - refresh_interval = get_refresh_interval(); - next_refresh = std::chrono::steady_clock::now() + refresh_interval; LOG(INFO) << "refresh kerberos ticket cache: " << _ticket_cache_path << ", lifetime sec: " << _ticket_lifetime_sec; } diff --git a/be/src/common/kerberos/kerberos_ticket_cache.h b/be/src/common/kerberos/kerberos_ticket_cache.h index 84e25826d38e4c..9ca595bd5fa4a3 100644 --- a/be/src/common/kerberos/kerberos_ticket_cache.h +++ b/be/src/common/kerberos/kerberos_ticket_cache.h @@ -106,10 +106,6 @@ class KerberosTicketCache : public std::enable_shared_from_this _refresh_thread; diff --git a/be/src/common/kerberos/kerberos_ticket_mgr.cpp b/be/src/common/kerberos/kerberos_ticket_mgr.cpp index 40b39efb1d02d1..720167ba05f5dd 100644 --- a/be/src/common/kerberos/kerberos_ticket_mgr.cpp +++ b/be/src/common/kerberos/kerberos_ticket_mgr.cpp @@ -127,8 +127,8 @@ Status KerberosTicketMgr::get_or_set_ticket_cache( } std::shared_ptr KerberosTicketMgr::get_ticket_cache( - const std::string& principal, const std::string& keytab_path, bool use_default_cache) { - std::string key = KerberosConfig::get_hash_code(principal, keytab_path, use_default_cache); + const std::string& principal, const std::string& keytab_path) { + std::string key = KerberosConfig::get_hash_code(principal, keytab_path); std::lock_guard lock(_mutex); auto it = _ticket_caches.find(key); diff --git a/be/src/common/kerberos/kerberos_ticket_mgr.h b/be/src/common/kerberos/kerberos_ticket_mgr.h index e4f2eb3b8501d6..07a73d6181b98d 100644 --- a/be/src/common/kerberos/kerberos_ticket_mgr.h +++ b/be/src/common/kerberos/kerberos_ticket_mgr.h @@ -60,8 +60,7 @@ class KerberosTicketMgr { // Get the ticket cache object. This is used by HdfsHandler to hold a reference std::shared_ptr get_ticket_cache(const std::string& principal, - const std::string& keytab_path, - bool use_default_cache = false); + const std::string& keytab_path); // Get detailed information about all active Kerberos ticket caches std::vector get_krb_ticket_cache_info(); diff --git a/be/src/common/kerberos/krb5_interface.h b/be/src/common/kerberos/krb5_interface.h index 26269e67483128..550fea7e62a9a7 100644 --- a/be/src/common/kerberos/krb5_interface.h +++ b/be/src/common/kerberos/krb5_interface.h @@ -35,7 +35,6 @@ class Krb5Interface { virtual Status parse_name(krb5_context context, const char* name, krb5_principal* principal) = 0; virtual Status kt_resolve(krb5_context context, const char* name, krb5_keytab* keytab) = 0; - virtual const char* cc_default_name(krb5_context context) = 0; virtual Status cc_resolve(krb5_context context, const char* name, krb5_ccache* ccache) = 0; virtual Status get_init_creds_opt_alloc(krb5_context context, krb5_get_init_creds_opt** opt) = 0; diff --git a/be/src/common/kerberos/krb5_interface_impl.cpp b/be/src/common/kerberos/krb5_interface_impl.cpp index d4c335fac44a5f..6ae8d11b6c788d 100644 --- a/be/src/common/kerberos/krb5_interface_impl.cpp +++ b/be/src/common/kerberos/krb5_interface_impl.cpp @@ -38,10 +38,6 @@ Status Krb5InterfaceImpl::kt_resolve(krb5_context context, const char* name, krb return _check_error(code, context, "Failed to resolve keytab"); } -const char* Krb5InterfaceImpl::cc_default_name(krb5_context context) { - return krb5_cc_default_name(context); -} - Status Krb5InterfaceImpl::cc_resolve(krb5_context context, const char* name, krb5_ccache* ccache) { krb5_error_code code = krb5_cc_resolve(context, name, ccache); return _check_error(code, context, "Failed to resolve credential cache"); diff --git a/be/src/common/kerberos/krb5_interface_impl.h b/be/src/common/kerberos/krb5_interface_impl.h index 7519e71344f58c..916dcaf59dfc8c 100644 --- a/be/src/common/kerberos/krb5_interface_impl.h +++ b/be/src/common/kerberos/krb5_interface_impl.h @@ -26,7 +26,6 @@ class Krb5InterfaceImpl : public Krb5Interface { Status init_context(krb5_context* context) override; Status parse_name(krb5_context context, const char* name, krb5_principal* principal) override; Status kt_resolve(krb5_context context, const char* name, krb5_keytab* keytab) override; - const char* cc_default_name(krb5_context context) override; Status cc_resolve(krb5_context context, const char* name, krb5_ccache* ccache) override; Status get_init_creds_opt_alloc(krb5_context context, krb5_get_init_creds_opt** opt) override; Status get_init_creds_keytab(krb5_context context, krb5_creds* creds, krb5_principal client, diff --git a/be/src/load/routine_load/data_consumer.cpp b/be/src/load/routine_load/data_consumer.cpp index e3845a9cefff0e..7bee3077b07618 100644 --- a/be/src/load/routine_load/data_consumer.cpp +++ b/be/src/load/routine_load/data_consumer.cpp @@ -121,7 +121,6 @@ Status KafkaDataConsumer::init(std::shared_ptr ctx) { RETURN_IF_ERROR(set_conf("debug", config::kafka_debug)); } - std::string kerberos_keytab_path; for (auto& item : ctx->kafka_info->properties) { _custom_properties.emplace(item.first, item.second); @@ -147,24 +146,12 @@ Status KafkaDataConsumer::init(std::shared_ptr ctx) { return Status::InternalError("PAUSE: failed to get file for config: {}, error: {}", item.first, st.to_string()); } - if (item.first == "sasl.kerberos.keytab") { - // The keytab is consumed by libkrb5, never by a shell command. - kerberos_keytab_path = std::move(file_path); - } else { - RETURN_IF_ERROR(set_conf(item.first, file_path)); - } + RETURN_IF_ERROR(set_conf(item.first, file_path)); } else { RETURN_IF_ERROR(set_conf(item.first, item.second)); } } - if (!kerberos_keytab_path.empty()) { - RETURN_IF_ERROR(configure_kafka_kerberos( - *conf, ctx->kafka_info->properties.at("sasl.kerberos.principal"), - kerberos_keytab_path, *ctx->exec_env()->kerberos_ticket_mgr(), - &_kerberos_ticket_cache)); - } - // if not specified group id, generate a random one. // ATTN: In the new version, we have set a group.id on the FE side for jobs that have not set a groupid, // but in order to ensure compatibility, we still do a check here. diff --git a/be/src/load/routine_load/data_consumer.h b/be/src/load/routine_load/data_consumer.h index a35497161812b1..0d88cf29553a30 100644 --- a/be/src/load/routine_load/data_consumer.h +++ b/be/src/load/routine_load/data_consumer.h @@ -44,10 +44,6 @@ namespace doris { -namespace kerberos { -class KerberosTicketCache; -} - template class BlockingQueue; @@ -180,9 +176,6 @@ class KafkaDataConsumer : public DataConsumer { KafkaEventCb _k_event_cb; RdKafka::KafkaConsumer* _k_consumer = nullptr; - // Held until after _k_consumer closes its connections in the destructor. - std::shared_ptr _kerberos_ticket_cache; - // AWS MSK IAM authentication callback (must outlive _k_consumer) std::unique_ptr _aws_msk_oauth_callback; }; diff --git a/be/src/load/routine_load/kafka_properties.cpp b/be/src/load/routine_load/kafka_properties.cpp index af8029a51c71f9..85899e05be923d 100644 --- a/be/src/load/routine_load/kafka_properties.cpp +++ b/be/src/load/routine_load/kafka_properties.cpp @@ -20,9 +20,6 @@ #include #include -#include - -#include "common/kerberos/kerberos_ticket_mgr.h" #include "common/status.h" #include "util/string_util.h" @@ -37,10 +34,10 @@ Status configure_kafka_security(RdKafka::Conf& conf, return Status::InvalidArgument( "PAUSE: Kafka property '{}' is not allowed in Routine Load with a nonzero " "value; " - "Doris manages keytab login and renewal without shell commands", + "Kerberos tickets must be managed externally", key); } - // Certificate and keytab paths supplied by SQL users must be Doris-managed files. + // Certificate paths supplied by SQL users must be Doris-managed files. if (g_BackendService_constants.KAFKA_FILE_PROPERTIES.contains(key)) { if (!starts_with(value, "FILE:") || value.size() == 5) { return Status::InvalidArgument( @@ -57,29 +54,9 @@ Status configure_kafka_security(RdKafka::Conf& conf, } } - // A keytab without a principal or GSSAPI would otherwise silently use the - // process identity or be ignored by a different SASL mechanism. - if (properties.contains("sasl.kerberos.keytab")) { - auto principal = properties.find("sasl.kerberos.principal"); - auto protocol = properties.find("security.protocol"); - if (principal == properties.end() || principal->second.empty() || - principal->second.find('\0') != std::string::npos || protocol == properties.end() || - (protocol->second != "SASL_SSL" && protocol->second != "SASL_PLAINTEXT")) { - return Status::InvalidArgument( - "PAUSE: Kafka keytab requires an explicit Kerberos principal and SASL " - "protocol"); - } - for (const auto* mechanism : {"sasl.mechanism", "sasl.mechanisms"}) { - auto it = properties.find(mechanism); - if (it != properties.end() && it->second != "GSSAPI") { - return Status::InvalidArgument("PAUSE: Kafka keytab requires GSSAPI"); - } - } - } - // Rejecting kinit.cmd alone is insufficient: the default command interpolates // the principal/keytab. Setting the interval to zero disables both the initial - // command and the refresh timer. Doris handles managed keytabs through libkrb5. + // command and the refresh timer. Operators must maintain the ticket cache. // This is mandatory, so do not use init's best-effort set_conf wrapper here. std::string error; if (conf.set("sasl.kerberos.min.time.before.relogin", "0", error) != RdKafka::Conf::CONF_OK) { @@ -89,24 +66,4 @@ Status configure_kafka_security(RdKafka::Conf& conf, return Status::OK(); } -Status configure_kafka_kerberos(RdKafka::Conf& conf, const std::string& principal, - const std::string& keytab_path, - kerberos::KerberosTicketMgr& manager, - std::shared_ptr* ticket_cache) { - kerberos::KerberosConfig config; - config.set_principal_and_keytab(principal, keytab_path); - config.set_use_default_cache(true); - std::shared_ptr cache; - RETURN_IF_ERROR(manager.get_or_set_ticket_cache(config, &cache)); - // Unmodified librdkafka discovers the process default Kerberos cache. - // Native login and renewal therefore write that cache, just as kinit did. - // This retains the existing shared-cache behavior, not per-job isolation. - std::string error; - if (conf.set("sasl.kerberos.principal", principal, error) != RdKafka::Conf::CONF_OK) { - return Status::InternalError("PAUSE: failed to set Kafka Kerberos principal: {}", error); - } - *ticket_cache = std::move(cache); - return Status::OK(); -} - } // namespace doris diff --git a/be/src/load/routine_load/kafka_properties.h b/be/src/load/routine_load/kafka_properties.h index 4dc59b7f62548f..94f2adc9e71697 100644 --- a/be/src/load/routine_load/kafka_properties.h +++ b/be/src/load/routine_load/kafka_properties.h @@ -18,7 +18,6 @@ #pragma once #include -#include #include namespace RdKafka { @@ -27,20 +26,9 @@ class Conf; namespace doris { class Status; -namespace kerberos { -class KerberosTicketCache; -class KerberosTicketMgr; -} // namespace kerberos // Validate the entire map before applying any user properties, then disable // librdkafka's shell-based Kerberos ticket acquisition and renewal. Status configure_kafka_security(RdKafka::Conf& conf, const std::map& properties); -// keytab_path must already have been resolved by SmallFileMgr. Keep the returned -// refresh worker alive until the Kafka consumer is destroyed. Tickets are written -// to the process default cache, preserving the existing shared-cache semantics. -Status configure_kafka_kerberos(RdKafka::Conf& conf, const std::string& principal, - const std::string& keytab_path, - kerberos::KerberosTicketMgr& manager, - std::shared_ptr* ticket_cache); } // namespace doris diff --git a/be/test/common/kerberos/kerberos_config_test.cpp b/be/test/common/kerberos/kerberos_config_test.cpp index 9aa66b8572c3ca..c27be4bfd312fc 100644 --- a/be/test/common/kerberos/kerberos_config_test.cpp +++ b/be/test/common/kerberos/kerberos_config_test.cpp @@ -57,16 +57,4 @@ TEST_F(KerberosConfigTest, SetterAndGetter) { EXPECT_EQ(config.get_min_time_before_refresh_second(), 800); } -TEST_F(KerberosConfigTest, DefaultCacheModeHasSeparateManagerKey) { - KerberosConfig config; - config.set_principal_and_keytab("user@REALM", "/managed/keytab"); - EXPECT_FALSE(config.use_default_cache()); - const auto private_key = config.get_hash_code(); - EXPECT_EQ(private_key, KerberosConfig::get_hash_code("user@REALM", "/managed/keytab")); - config.set_use_default_cache(true); - EXPECT_NE(private_key, config.get_hash_code()); - EXPECT_EQ(config.get_hash_code(), - KerberosConfig::get_hash_code("user@REALM", "/managed/keytab", true)); -} - } // namespace doris::kerberos diff --git a/be/test/common/kerberos/kerberos_ticket_cache_test.cpp b/be/test/common/kerberos/kerberos_ticket_cache_test.cpp index 3b8cc5dcc448f1..cbd6294fde4e52 100644 --- a/be/test/common/kerberos/kerberos_ticket_cache_test.cpp +++ b/be/test/common/kerberos/kerberos_ticket_cache_test.cpp @@ -36,7 +36,6 @@ class MockKrb5Interface : public Krb5Interface { MOCK_METHOD(Status, init_context, (krb5_context*), (override)); MOCK_METHOD(Status, parse_name, (krb5_context, const char*, krb5_principal*), (override)); MOCK_METHOD(Status, kt_resolve, (krb5_context, const char*, krb5_keytab*), (override)); - MOCK_METHOD(const char*, cc_default_name, (krb5_context), (override)); MOCK_METHOD(Status, cc_resolve, (krb5_context, const char*, krb5_ccache*), (override)); MOCK_METHOD(Status, get_init_creds_opt_alloc, (krb5_context, krb5_get_init_creds_opt**), (override)); @@ -92,15 +91,6 @@ class KerberosTicketCacheTest : public testing::Test { } } - void ResetForDefaultCache() { - _cache.reset(); - _config.set_use_default_cache(true); - auto mock = std::make_unique>(); - _mock_krb5_ptr = mock.get(); - _cache = - std::make_unique(_config, _test_dir.string(), std::move(mock)); - } - // Helper function to set up basic expectations for initialization void SetupBasicInitExpectations() { EXPECT_CALL(*_mock_krb5_ptr, init_context(_)).WillOnce(testing::Return(Status::OK())); @@ -116,30 +106,20 @@ class KerberosTicketCacheTest : public testing::Test { std::this_thread::sleep_for(std::chrono::milliseconds(10)); } - void ExpectForLogin(const std::string& cache_path, int lifetime_seconds = 3600) { + void ExpectForLogin(const std::string& cache_path) { // Setup expectations for login EXPECT_CALL(*_mock_krb5_ptr, kt_resolve(_, _, _)).WillOnce(testing::Return(Status::OK())); - EXPECT_CALL(*_mock_krb5_ptr, cc_resolve(_, testing::StrEq(cache_path), _)) - .WillOnce(testing::Return(Status::OK())); + EXPECT_CALL(*_mock_krb5_ptr, cc_resolve(_, _, _)).WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, get_init_creds_opt_alloc(_, _)) .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, get_init_creds_keytab(_, _, _, _, _, _, _)) - .WillOnce( - testing::Invoke([lifetime_seconds](krb5_context, krb5_creds* creds, - krb5_principal, krb5_keytab, krb5_deltat, - const char*, krb5_get_init_creds_opt*) { - creds->times.starttime = 100; - creds->times.endtime = 100 + lifetime_seconds; - return Status::OK(); - })); + .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, cc_initialize(_, _, _)) .WillOnce(testing::Return(Status::OK())); EXPECT_CALL(*_mock_krb5_ptr, cc_store_cred(_, _, _)) .WillOnce(testing::DoAll( testing::Invoke([this, cache_path](krb5_context, krb5_ccache, krb5_creds*) { - SimulateTicketCacheFileUpdate(cache_path.starts_with("FILE:") - ? cache_path.substr(5) - : cache_path); + SimulateTicketCacheFileUpdate(cache_path); return Status::OK(); }))); @@ -228,53 +208,6 @@ TEST_F(KerberosTicketCacheTest, RefreshTickets) { ASSERT_GT(new_write_time, initial_write_time); } -TEST_F(KerberosTicketCacheTest, DefaultCacheIsUsedForLoginAndRefreshAndNotDeleted) { - const auto file = (_test_dir / "operator-cache").string(); - for (const auto* prefix : {"", "FILE:"}) { - ResetForDefaultCache(); - std::ofstream(file) << "existing tickets"; - const std::string name = prefix + file; - SetupBasicInitExpectations(); - EXPECT_CALL(*_mock_krb5_ptr, cc_default_name(_)).WillOnce(testing::Return(name.c_str())); - ASSERT_TRUE(_cache->initialize().ok()); - EXPECT_EQ(name, _cache->get_ticket_cache_path()); - // Initialization must not remove an operator-owned default cache. - std::ifstream input(file); - std::string contents; - std::getline(input, contents); - EXPECT_EQ("existing tickets", contents); - input.close(); - ExpectForLogin(name); - ASSERT_TRUE(_cache->login().ok()); - ExpectForLogin(name); - ASSERT_TRUE(_cache->refresh_tickets().ok()); - _cache.reset(); - EXPECT_TRUE(std::filesystem::exists(file)); - } -} - -TEST_F(KerberosTicketCacheTest, DefaultCacheResolutionFailureIsReported) { - ResetForDefaultCache(); - SetupBasicInitExpectations(); - EXPECT_CALL(*_mock_krb5_ptr, cc_default_name(_)).WillOnce(testing::Return(nullptr)); - EXPECT_FALSE(_cache->initialize().ok()); -} - -TEST_F(KerberosTicketCacheTest, RefreshIntervalTracksTicketLifetime) { - SetupBasicInitExpectations(); - ASSERT_TRUE(_cache->initialize().ok()); - EXPECT_EQ(std::chrono::seconds(2), _cache->get_refresh_interval()); - ExpectForLogin(_cache->get_ticket_cache_path(), 2); - ASSERT_TRUE(_cache->login().ok()); - EXPECT_EQ(std::chrono::seconds(1), _cache->get_refresh_interval()); - ExpectForLogin(_cache->get_ticket_cache_path(), 1); - ASSERT_TRUE(_cache->login().ok()); - EXPECT_EQ(std::chrono::milliseconds(500), _cache->get_refresh_interval()); - ExpectForLogin(_cache->get_ticket_cache_path(), 3600); - ASSERT_TRUE(_cache->login().ok()); - EXPECT_EQ(std::chrono::seconds(2), _cache->get_refresh_interval()); -} - TEST_F(KerberosTicketCacheTest, PeriodicRefresh) { SetupBasicInitExpectations(); ASSERT_TRUE(_cache->initialize().ok()); diff --git a/be/test/load/routine_load/kafka_properties_test.cpp b/be/test/load/routine_load/kafka_properties_test.cpp index 95193c062487bb..296979f506dfae 100644 --- a/be/test/load/routine_load/kafka_properties_test.cpp +++ b/be/test/load/routine_load/kafka_properties_test.cpp @@ -18,14 +18,11 @@ #include "load/routine_load/kafka_properties.h" #include -#include #include #include -#include #include -#include "common/kerberos/kerberos_ticket_mgr.h" #include "common/status.h" #include "load/routine_load/data_consumer.h" @@ -34,7 +31,7 @@ namespace doris { TEST(KafkaPropertiesTest, RejectExecutableAndUnknownProperties) { std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); for (const auto* key : - {"sasl.kerberos.kinit.cmd", "sasl.kerberos.ticket.cache", + {"sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property", "sasl.jaas.config"}) { auto status = configure_kafka_security(*conf, {{key, "sensitive-value"}}); @@ -53,17 +50,13 @@ TEST(KafkaPropertiesTest, RejectKerberosRefreshReenable) { } } -TEST(KafkaPropertiesTest, RequireManagedFiles) { +TEST(KafkaPropertiesTest, RequireManagedCertificateFiles) { std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); for (const auto& key : g_BackendService_constants.KAFKA_FILE_PROPERTIES) { for (const auto* value : {"/etc/ssl/private/key.pem", "relative.pem", "", "FILE:"}) { EXPECT_FALSE(configure_kafka_security(*conf, {{key, value}}).ok()) << key; } - EXPECT_TRUE(configure_kafka_security(*conf, {{key, "FILE:123:checksum"}, - {"sasl.kerberos.principal", "user@REALM"}, - {"security.protocol", "SASL_SSL"}}) - .ok()) - << key; + EXPECT_TRUE(configure_kafka_security(*conf, {{key, "FILE:123:checksum"}}).ok()) << key; } } @@ -109,97 +102,4 @@ TEST(KafkaPropertiesTest, ConsumerRejectsBeforeResolvingFilesOrCreatingClient) { EXPECT_NE(status.to_string().find("plugin.library.paths"), std::string::npos); } -TEST(KafkaPropertiesTest, KeytabRequiresExplicitIdentityAndGssapi) { - std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); - std::map properties = {{"sasl.kerberos.keytab", "FILE:123:checksum"}}; - EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); - properties["sasl.kerberos.principal"] = "user@REALM"; - EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); - properties["security.protocol"] = "SASL_SSL"; - EXPECT_TRUE(configure_kafka_security(*conf, properties).ok()); - for (const auto* mechanism : {"sasl.mechanism", "sasl.mechanisms"}) { - properties[mechanism] = "PLAIN"; - EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); - properties[mechanism] = "GSSAPI"; - EXPECT_TRUE(configure_kafka_security(*conf, properties).ok()); - } - properties["sasl.kerberos.principal"] = std::string("user\0suffix", 11); - EXPECT_FALSE(configure_kafka_security(*conf, properties).ok()); -} - -namespace { -class KafkaMockTicketCache : public kerberos::KerberosTicketCache { -public: - using KerberosTicketCache::KerberosTicketCache; - MOCK_METHOD(Status, initialize, (), (override)); - MOCK_METHOD(Status, login, (), (override)); - MOCK_METHOD(Status, write_ticket_cache, (), (override)); - MOCK_METHOD(void, start_periodic_refresh, (), (override)); -}; - -class KafkaMockTicketMgr : public kerberos::KerberosTicketMgr { -public: - KafkaMockTicketMgr() - : KerberosTicketMgr((std::filesystem::temp_directory_path() / - ("kafka-kerberos-" + UniqueId::gen_uid().to_string())) - .string()) {} - bool fail_login = false; - -protected: - std::shared_ptr _make_new_ticket_cache( - const kerberos::KerberosConfig& config) override { - auto cache = - std::make_shared>(config, _root_path); - cache->set_ticket_cache_path(_root_path + "/" + config.get_hash_code()); - testing::InSequence sequence; - EXPECT_CALL(*cache, initialize()).WillOnce(testing::Return(Status::OK())); - if (fail_login) { - EXPECT_CALL(*cache, login()) - .WillOnce(testing::Return(Status::InternalError("KDC unavailable"))); - } else { - EXPECT_CALL(*cache, login()).WillOnce(testing::Return(Status::OK())); - EXPECT_CALL(*cache, write_ticket_cache()).WillOnce(testing::Return(Status::OK())); - EXPECT_CALL(*cache, start_periodic_refresh()).Times(1); - } - return cache; - } -}; -} // namespace - -TEST(KafkaPropertiesTest, ManagedLoginStartsRefreshInDefaultCacheMode) { - KafkaMockTicketMgr manager; - std::shared_ptr cache, shared; - std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); - ASSERT_TRUE(configure_kafka_security(*conf, {}).ok()); - ASSERT_TRUE(configure_kafka_kerberos(*conf, "alice@REALM", "/managed/1.keytab", manager, &cache) - .ok()); - ASSERT_TRUE(cache->get_config().use_default_cache()); - EXPECT_EQ(cache, manager.get_ticket_cache("alice@REALM", "/managed/1.keytab", true)); - EXPECT_EQ(nullptr, manager.get_ticket_cache("alice@REALM", "/managed/1.keytab")); - std::string principal, interval; - ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.principal", principal)); - EXPECT_EQ("alice@REALM", principal); - ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.min.time.before.relogin", interval)); - EXPECT_EQ("0", interval); - // Only upstream librdkafka properties are needed; the same identity reuses - // its native refresh worker without creating a private Kafka cache binding. - ASSERT_TRUE( - configure_kafka_kerberos(*conf, "alice@REALM", "/managed/1.keytab", manager, &shared) - .ok()); - EXPECT_EQ(cache, shared); -} - -TEST(KafkaPropertiesTest, FailedManagedLoginDoesNotPublishCache) { - KafkaMockTicketMgr manager; - manager.fail_login = true; - std::shared_ptr cache; - std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); - auto status = - configure_kafka_kerberos(*conf, "user@REALM", "/managed/1.keytab", manager, &cache); - EXPECT_FALSE(status.ok()); - EXPECT_NE(status.to_string().find("KDC unavailable"), std::string::npos); - EXPECT_EQ(nullptr, cache); - EXPECT_EQ(nullptr, manager.get_ticket_cache("user@REALM", "/managed/1.keytab", true)); -} - } // namespace doris diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java index e2bd3be72c2c15..3b7d14364e115c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaDataSourceProperties.java @@ -267,7 +267,7 @@ public static void validateCustomProperties(Map properties) thro // Only zero is safe: it prevents the default kinit command from executing. if (key.equals("sasl.kerberos.min.time.before.relogin") && !property.getValue().equals("0")) { throw new AnalysisException("Kafka property '" + key + "' is not allowed in Routine Load" - + " with a nonzero value; Doris manages keytab login and renewal without shell commands"); + + " with a nonzero value; Kerberos tickets must be managed externally"); } if (BackendServiceConstants.KAFKA_FILE_PROPERTIES.contains(key)) { if (!property.getValue().startsWith("FILE:") || property.getValue().length() == 5) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index 3d745463df447f..f5c9541c2cef0f 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -49,7 +49,7 @@ private KafkaDataSourceProperties properties(String key, String value, boolean a @Test public void testCreateAndAlterRejectExecutableAndUnknownProperties() { for (boolean alter : new boolean[] {false, true}) { - for (String key : List.of("sasl.kerberos.kinit.cmd", "sasl.kerberos.ticket.cache", + for (String key : List.of("sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property", "sasl.jaas.config", "security.protocol\0suffix")) { @@ -72,7 +72,7 @@ public void testRejectKerberosRefreshReenable() { } @Test - public void testCertificateAndKeytabFilesRequireManagedReferences() throws Exception { + public void testCertificateFilesRequireManagedReferences() throws Exception { for (boolean alter : new boolean[] {false, true}) { for (String key : BackendServiceConstants.KAFKA_FILE_PROPERTIES) { for (String value : List.of("/etc/ssl/private/key.pem", "relative.pem", "", "FILE:")) { diff --git a/gensrc/thrift/BackendService.thrift b/gensrc/thrift/BackendService.thrift index 41afe8079905db..98f258b9222242 100644 --- a/gensrc/thrift/BackendService.thrift +++ b/gensrc/thrift/BackendService.thrift @@ -129,7 +129,6 @@ const set KAFKA_ALLOWED_PROPERTIES = [ // These data files must be registered through CREATE FILE, never raw BE paths. const set KAFKA_FILE_PROPERTIES = [ "https.ca.location", - "sasl.kerberos.keytab", "ssl.ca.location", "ssl.certificate.location", "ssl.crl.location", diff --git a/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy b/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy index c105e5e516f383..f06fbf385db68e 100644 --- a/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy +++ b/regression-test/suites/load_p0/routine_load/test_kafka_property_validation.groovy @@ -25,7 +25,7 @@ suite("test_kafka_property_validation", "p0") { """ // All failures must happen during analysis, without contacting a Kafka broker. - def rejected = ["sasl.kerberos.kinit.cmd", "sasl.kerberos.ticket.cache", + def rejected = ["sasl.kerberos.kinit.cmd", "sasl.kerberos.keytab", "sasl.kerberos.min.time.before.relogin", "plugin.library.paths", "ssl.engine.location", "ssl.engine.id", "ssl.providers", "unknown.future.property"] rejected.each { key -> @@ -41,7 +41,7 @@ suite("test_kafka_property_validation", "p0") { exception "Kafka property '${key}' is not allowed in Routine Load" } } - ["ssl.ca.location", "ssl.certificate.location", "ssl.key.location", "sasl.kerberos.keytab"].each { key -> + ["ssl.ca.location", "ssl.certificate.location", "ssl.key.location"].each { key -> test { sql """ CREATE ROUTINE LOAD test_kafka_property_validation_job ON test_kafka_property_validation From b7ef8bc06329014a6fd7f8f3b8128502b75e1278 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Thu, 22 Oct 2026 17:43:13 +0800 Subject: [PATCH 06/12] [fix](load) Preserve migrated Kafka jobs during recovery ### What problem does this PR solve? Related PR: #69391 Problem Summary: Routine Load recovery reparses the original CREATE SQL to rebuild load expressions. An older job can retain a raw certificate path in that SQL even after ALTER has migrated its effective properties to CREATE FILE references. Reapplying the new data source admission checks to the stale CREATE SQL terminally cancels this otherwise valid job on FE restart. Use a recovery validation entry point that retains the existing database, table, load, job-property and merge checks without revalidating the original data source properties. CREATE admission and validation of the effective Kafka properties before runtime use remain enforced. ### Release note Kafka Routine Load jobs migrated to managed certificate files remain recoverable when their original CREATE SQL contains a raw certificate path. ### Check List (For Author) - Test: Added Gson round-trip coverage for migrated and unmigrated certificate properties, restored column descriptors, managed-file conversion, runtime rejection and unchanged CREATE admission. Checkstyle and git diff checks passed. run-fe-ut.sh was attempted but could not start tests because JDK 17 is unavailable; runtime behavior remains unverified locally. - Behavior changed: Yes; stale CREATE data source properties no longer cancel a persisted job during expression reconstruction. - Does this need documentation: No; restores intended recovery behavior. --- .../load/routineload/RoutineLoadJob.java | 2 +- .../commands/info/CreateRoutineLoadInfo.java | 16 +++- .../KafkaPropertyValidationTest.java | 77 +++++++++++++++++++ 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java index 4cd4434a73e6c3..d617ad5d1546f2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/RoutineLoadJob.java @@ -2036,7 +2036,7 @@ public void gsonPostProcess() throws IOException { // fall through; let validate() surface the real error } } - createRoutineLoadInfo.validate(ctx); + createRoutineLoadInfo.validateForRecovery(ctx); setRoutineLoadDesc(createRoutineLoadInfo.getRoutineLoadDesc()); } finally { ctx.cleanup(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateRoutineLoadInfo.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateRoutineLoadInfo.java index e8a75c0299b4d6..053ce9ae989400 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateRoutineLoadInfo.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateRoutineLoadInfo.java @@ -378,10 +378,22 @@ public String getWorkloadGroupName() { return workloadGroupName; } + /** + * Rebuild load expressions without validating stale data source properties in the original CREATE SQL. + * The job's current effective data source properties are validated before runtime use. + */ + public void validateForRecovery(ConnectContext ctx) throws UserException { + validate(ctx, false); + } + /** * analyze create table info */ public void validate(ConnectContext ctx) throws UserException { + validate(ctx, true); + } + + private void validate(ConnectContext ctx, boolean validateDataSource) throws UserException { // check dbName and tableName checkDBTable(ctx); // check name @@ -398,7 +410,9 @@ public void validate(ConnectContext ctx) throws UserException { // check routine load job properties include desired concurrent number etc. checkJobProperties(); // check data source properties - checkDataSourceProperties(); + if (validateDataSource) { + checkDataSourceProperties(); + } // analyze merge type if (routineLoadDesc != null) { if (mergeType != LoadTask.MergeType.MERGE && routineLoadDesc.getDeleteCondition() != null) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index f5c9541c2cef0f..a3b9e0a18bb35c 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -18,19 +18,33 @@ package org.apache.doris.load.routineload; import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.Table; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.DdlException; import org.apache.doris.common.jmockit.Deencapsulation; +import org.apache.doris.common.util.SmallFileMgr; +import org.apache.doris.common.util.SmallFileMgr.SmallFile; +import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.load.routineload.kafka.KafkaDataSourceProperties; import org.apache.doris.load.routineload.kafka.KafkaRoutineLoadJob; +import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.trees.plans.commands.load.CreateRoutineLoadCommand; +import org.apache.doris.persist.gson.GsonUtils; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.OriginStatement; import org.apache.doris.thrift.BackendServiceConstants; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; public class KafkaPropertyValidationTest { private KafkaDataSourceProperties properties(String key, String value, boolean alter) { @@ -150,4 +164,67 @@ public void testPersistedPropertiesRejectedBeforeMetadataRequests() { () -> Deencapsulation.invoke(job, "convertCustomProperties", false)); Assertions.assertTrue(error.getMessage().contains("is not allowed in Routine Load")); } + + @Test + public void testRecoverMigratedCertificateProperties() throws Exception { + Env env = Mockito.mock(Env.class); + InternalCatalog catalog = Mockito.mock(InternalCatalog.class); + Database db = Mockito.mock(Database.class); + Table table = Mockito.mock(Table.class); + SmallFileMgr smallFileMgr = Mockito.mock(SmallFileMgr.class); + String originalSql = "CREATE ROUTINE LOAD test_db.test_job ON test_table COLUMNS(id) " + + "FROM KAFKA (\"kafka_broker_list\" = \"127.0.0.1:9092\", " + + "\"kafka_topic\" = \"test_topic\", \"property.ssl.ca.location\" = \"/etc/ssl/ca.pem\")"; + + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + envStatic.when(Env::getCurrentInternalCatalog).thenReturn(catalog); + Mockito.when(env.getInternalCatalog()).thenReturn(catalog); + Mockito.when(env.getSmallFileMgr()).thenReturn(smallFileMgr); + Mockito.doReturn(Optional.of(db)).when(catalog).getDb(1L); + Mockito.doReturn(db).when(catalog).getDbOrAnalysisException("test_db"); + Mockito.when(db.getName()).thenReturn("test_db"); + Mockito.doReturn(Optional.of(table)).when(db).getTable(2L); + Mockito.doReturn(table).when(db).getTableOrAnalysisException("test_table"); + Mockito.when(table.getName()).thenReturn("test_table"); + SmallFile ca = new SmallFile(1L, "kafka", "ca.pem", 3L, "", 0L, "checksum", true); + Mockito.when(smallFileMgr.getSmallFile(1L, "kafka", "ca.pem", true)).thenReturn(ca); + + // ALTER changes the effective map, but the persisted CREATE SQL still contains the old path. + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "test_topic", UserIdentity.ADMIN); + job.setOrigStmt(new OriginStatement(originalSql, 0)); + Deencapsulation.setField(job, "state", RoutineLoadJob.JobState.PAUSED); + Deencapsulation.setField(job, "customProperties", Map.of("ssl.ca.location", "FILE:ca.pem")); + KafkaRoutineLoadJob restored = (KafkaRoutineLoadJob) GsonUtils.GSON.fromJson( + GsonUtils.GSON.toJson(job), RoutineLoadJob.class); + + Assertions.assertEquals(RoutineLoadJob.JobState.PAUSED, restored.getState()); + Assertions.assertEquals(-1L, restored.getEndTimestamp()); + Assertions.assertEquals("id", restored.getColumnExprDescs().descs.get(0).getColumnName()); + restored.prepare(); + Assertions.assertEquals("FILE:3:checksum", restored.getConvertedCustomProperties().get("ssl.ca.location")); + + // Recovery also preserves an unmigrated job, while runtime validation still rejects its current path. + Deencapsulation.setField(job, "customProperties", Map.of("ssl.ca.location", "/etc/ssl/ca.pem")); + KafkaRoutineLoadJob unmigrated = (KafkaRoutineLoadJob) GsonUtils.GSON.fromJson( + GsonUtils.GSON.toJson(job), RoutineLoadJob.class); + Assertions.assertEquals(RoutineLoadJob.JobState.PAUSED, unmigrated.getState()); + DdlException error = Assertions.assertThrows(DdlException.class, unmigrated::prepare); + Assertions.assertTrue(error.getMessage().contains("CREATE FILE")); + + // The recovery path must not weaken CREATE admission for the same SQL. + ConnectContext ctx = new ConnectContext(); + try { + ctx.setThreadLocalInfo(); + CreateRoutineLoadCommand command = (CreateRoutineLoadCommand) new NereidsParser() + .parseSingle(originalSql); + AnalysisException createError = Assertions.assertThrows(AnalysisException.class, + () -> command.getCreateRoutineLoadInfo().validate(ctx)); + Assertions.assertTrue(createError.getMessage().contains("CREATE FILE")); + } finally { + ctx.cleanup(); + } + } + } } From 28d9ed96872467bb78cc2a1a2c1263e820458dcf Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Fri, 23 Oct 2026 12:36:11 +0800 Subject: [PATCH 07/12] [fix](regression) Use allowed Kafka properties in Routine Load display test ### What problem does this PR solve? Issue Number: N/A Related PR: #69391 Problem Summary: The Routine Load display regression passed sasl.jaas.config, which the Kafka property admission lists reject before creating a job. Its unconditional finally cleanup then stopped a nonexistent job and hid the original creation error. Remove the rejected property and its assertions, retain password masking checks for SHOW and information_schema, and create the job before entering try/finally. Existing FE unit coverage continues to check legacy JAAS masking. ### Release note None ### Check List (For Author) - Test: git diff --check passed. Verified that every custom Kafka property in the changed block is allowed by the current Thrift admission lists. Attempted ./run-regression-test.sh --run -d load_p0/routine_load -s test_show_routine_load; it exited before running tests because JDK 17 is unavailable locally. - Behavior changed: No; regression test only. - Does this need documentation: No. --- .../test_show_routine_load.groovy | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/regression-test/suites/load_p0/routine_load/test_show_routine_load.groovy b/regression-test/suites/load_p0/routine_load/test_show_routine_load.groovy index fc330bb7b1dc18..574dc006cbea7d 100644 --- a/regression-test/suites/load_p0/routine_load/test_show_routine_load.groovy +++ b/regression-test/suites/load_p0/routine_load/test_show_routine_load.groovy @@ -205,31 +205,26 @@ suite("test_show_routine_load","p0") { } // test show routine load and information_schema mask sensitive custom properties + def kafkaPassword = "doris_routine_load_password_should_be_hidden" + sql """ + CREATE ROUTINE LOAD testShowSensitiveProperties ON ${tableName} + COLUMNS TERMINATED BY "," + FROM KAFKA + ( + "kafka_broker_list" = "${externalEnvIp}:${kafka_port}", + "kafka_topic" = "${kafkaCsvTpoics[0]}", + "property.kafka_default_offsets" = "OFFSET_BEGINNING", + "property.security.protocol" = "SASL_PLAINTEXT", + "property.sasl.mechanism" = "PLAIN", + "property.sasl.username" = "doris", + "property.sasl.password" = "${kafkaPassword}" + ); + """ try { - def kafkaPassword = "doris_routine_load_password_should_be_hidden" - def kafkaJaasSecret = "doris_routine_load_jaas_secret_should_be_hidden" - sql """ - CREATE ROUTINE LOAD testShowSensitiveProperties ON ${tableName} - COLUMNS TERMINATED BY "," - FROM KAFKA - ( - "kafka_broker_list" = "${externalEnvIp}:${kafka_port}", - "kafka_topic" = "${kafkaCsvTpoics[0]}", - "property.kafka_default_offsets" = "OFFSET_BEGINNING", - "property.security.protocol" = "SASL_PLAINTEXT", - "property.sasl.mechanism" = "PLAIN", - "property.sasl.username" = "doris", - "property.sasl.password" = "${kafkaPassword}", - "property.sasl.jaas.config" = "password=${kafkaJaasSecret}" - ); - """ - def showResult = sql "show routine load for testShowSensitiveProperties" def showCustomProperties = parseJson(showResult[0][13]) assertEquals("******", showCustomProperties["sasl.password"].toString()) - assertEquals("******", showCustomProperties["sasl.jaas.config"].toString()) assertFalse(showResult[0][13].toString().contains(kafkaPassword)) - assertFalse(showResult[0][13].toString().contains(kafkaJaasSecret)) def systemTableResult = sql """ SELECT CUSTOM_PROPERTIES @@ -238,9 +233,7 @@ suite("test_show_routine_load","p0") { """ def systemTableCustomProperties = parseJson(systemTableResult[0][0]) assertEquals("******", systemTableCustomProperties["sasl.password"].toString()) - assertEquals("******", systemTableCustomProperties["sasl.jaas.config"].toString()) assertFalse(systemTableResult[0][0].toString().contains(kafkaPassword)) - assertFalse(systemTableResult[0][0].toString().contains(kafkaJaasSecret)) } finally { sql "stop routine load for testShowSensitiveProperties" } From 970880e1a34ed8b469129197dbbbd2ecce9cd5e4 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Fri, 23 Oct 2026 14:19:36 +0800 Subject: [PATCH 08/12] [test](load) Use managed file references in SSL configuration test ### What problem does this PR solve? Related PR: #69391 Problem Summary: The Kafka property policy requires managed file references, but testSslConfigWithoutSasl still uses raw certificate paths and fails during analysis. Replace its three certificate paths with FILE: references. ### Release note None ### Check List (For Author) - Test: git diff --check passed. FE unit test not run because the worktree lacks thirdparty/installed/bin/protoc. Checkstyle attempted but blocked by JVM initialization failure. - Behavior changed: No - Does this need documentation: No --- .../doris/load/routineload/KafkaAwsMskIamAuthTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaAwsMskIamAuthTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaAwsMskIamAuthTest.java index 18764b97328574..f10b04a0202eb2 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaAwsMskIamAuthTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaAwsMskIamAuthTest.java @@ -255,9 +255,9 @@ public void testPlaintextConfigWithoutSasl() throws UserException { public void testSslConfigWithoutSasl() throws UserException { // Test SSL configuration without SASL dataSourceProperties.put("property.security.protocol", "SSL"); - dataSourceProperties.put("property.ssl.ca.location", "/path/to/ca-cert"); - dataSourceProperties.put("property.ssl.certificate.location", "/path/to/client-cert"); - dataSourceProperties.put("property.ssl.key.location", "/path/to/client-key"); + dataSourceProperties.put("property.ssl.ca.location", "FILE:ca-cert"); + dataSourceProperties.put("property.ssl.certificate.location", "FILE:client-cert"); + dataSourceProperties.put("property.ssl.key.location", "FILE:client-key"); KafkaDataSourceProperties props = new KafkaDataSourceProperties(dataSourceProperties); props.setTimezone("UTC"); From db800308b50d30ffdcd0446b9a5be9a025ce36f7 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Fri, 23 Oct 2026 17:12:56 +0800 Subject: [PATCH 09/12] [test](load) Complete catalog mocks for Kafka recovery test ### What problem does this PR solve? Related PR: #69391 Problem Summary: testRecoverMigratedCertificateProperties mocks the internal catalog but omits the CatalogMgr used by ConnectContext.setDatabase during RoutineLoadJob deserialization. The resulting null pointer cancels the job before the recovery assertions can run. Mock the catalog manager, catalog name, and database lookup by name so recovery uses consistent test metadata. ### Release note None ### Check List (For Author) - Test: git diff --check passed. KafkaPropertyValidationTest.java passed Checkstyle 9.3 with the repository configuration. FE UT not run because thirdparty/installed/bin/protoc is missing; Maven reactor validation also cannot resolve uncached dependencies offline. - Behavior changed: No - Does this need documentation: No --- .../load/routineload/KafkaPropertyValidationTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index a3b9e0a18bb35c..35626245cd67b9 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -26,6 +26,7 @@ import org.apache.doris.common.jmockit.Deencapsulation; import org.apache.doris.common.util.SmallFileMgr; import org.apache.doris.common.util.SmallFileMgr.SmallFile; +import org.apache.doris.datasource.CatalogMgr; import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.load.routineload.kafka.KafkaDataSourceProperties; import org.apache.doris.load.routineload.kafka.KafkaRoutineLoadJob; @@ -168,6 +169,7 @@ public void testPersistedPropertiesRejectedBeforeMetadataRequests() { @Test public void testRecoverMigratedCertificateProperties() throws Exception { Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); InternalCatalog catalog = Mockito.mock(InternalCatalog.class); Database db = Mockito.mock(Database.class); Table table = Mockito.mock(Table.class); @@ -179,10 +181,15 @@ public void testRecoverMigratedCertificateProperties() throws Exception { try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { envStatic.when(Env::getCurrentEnv).thenReturn(env); envStatic.when(Env::getCurrentInternalCatalog).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(catalogMgr.getCatalog(InternalCatalog.INTERNAL_CATALOG_NAME)).thenReturn(catalog); Mockito.when(env.getInternalCatalog()).thenReturn(catalog); Mockito.when(env.getSmallFileMgr()).thenReturn(smallFileMgr); + Mockito.when(catalog.getName()).thenReturn(InternalCatalog.INTERNAL_CATALOG_NAME); Mockito.doReturn(Optional.of(db)).when(catalog).getDb(1L); + Mockito.doReturn(Optional.of(db)).when(catalog).getDb("test_db"); Mockito.doReturn(db).when(catalog).getDbOrAnalysisException("test_db"); + Mockito.when(db.getId()).thenReturn(1L); Mockito.when(db.getName()).thenReturn("test_db"); Mockito.doReturn(Optional.of(table)).when(db).getTable(2L); Mockito.doReturn(table).when(db).getTableOrAnalysisException("test_table"); From 41c559fa7c2e3671b2ac1e9b9de1e3ebcd8469a5 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Fri, 23 Oct 2026 17:37:31 +0800 Subject: [PATCH 10/12] [fix](load) Separate Kafka ALTER replay from runtime preparation ### What problem does this PR solve? Related PR: #69391 Historical ALTER replay ran the current Kafka property policy and file conversion before applying topic, broker, offset and job-setting changes. Rejecting an option accepted by an older FE could therefore leave a partially restored committed transition. Keep live ALTER preparation separate from applying persisted properties. Replay restores the Kafka fields without resolving files, validating current Kafka policy, checking runtime partitions or repeating cloud progress RPCs. Invalidate the converted cache and enforce current policy before preparation and metadata requests, including ALTER offset lookups. Prepare live custom properties in a temporary map before publishing them. ### Release note Preserve complete historical Routine Load ALTER state during recovery. Jobs with unsupported Kafka properties remain recoverable and are paused with an error when scheduled under the current policy. ### Check List (For Author) - Test: Add FE tests for historical ALTER and subsequent repair in cloud/non-cloud modes, deferred file resolution, scheduler pause reporting, metadata admission and live ALTER publication. Targeted FE unit tests attempted through run-fe-ut.sh but blocked before execution by unavailable JDK 17. Checkstyle reports zero violations; git diff --check passed. Full compilation remains unverified; thirdparty/installed/bin/protoc is absent. - Behavior changed: Yes; journal replay applies committed Kafka state without runtime preparation or repeated cloud progress reset RPCs. - Does this need documentation: No; the existing PR upgrade requirements remain applicable. --- .../kafka/KafkaRoutineLoadJob.java | 75 ++++++-- .../KafkaPropertyValidationTest.java | 176 ++++++++++++++++++ 2 files changed, 237 insertions(+), 14 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java index 1f9aeb321d0181..105b997d9c0045 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java @@ -209,21 +209,28 @@ public void prepare() throws UserException { } private void convertCustomProperties(boolean rebuild) throws DdlException { + validateCustomProperties(customProperties); + if (!rebuild && !convertedCustomProperties.isEmpty()) { + return; + } + setConvertedCustomProperties(resolveCustomProperties(customProperties)); + } + + private void validateCustomProperties(Map properties) throws DdlException { // Replay can restore properties accepted by an older FE. Reject them before // metadata requests, including during a rolling upgrade with older BEs. try { - KafkaDataSourceProperties.validateCustomProperties(customProperties); + KafkaDataSourceProperties.validateCustomProperties(properties); } catch (AnalysisException e) { throw new DdlException(e.getMessage(), e); } - if (!rebuild && !convertedCustomProperties.isEmpty()) { - return; - } + } + private Map resolveCustomProperties(Map properties) throws DdlException { // Publish only a complete conversion. A failed FILE lookup must not leave // a partial cache that skips validation/conversion on the next metadata request. Map convertedProperties = Maps.newHashMap(); - for (Map.Entry entry : customProperties.entrySet()) { + for (Map.Entry entry : properties.entrySet()) { if (entry.getValue().startsWith("FILE:")) { // convert FILE:file_name -> FILE:file_id:md5 String file = entry.getValue().substring(entry.getValue().indexOf(":") + 1); @@ -238,6 +245,10 @@ private void convertCustomProperties(boolean rebuild) throws DdlException { // Older BEs still use librdkafka's default shell command. Send the safe // setting explicitly so new FE -> old BE also disables ticket execution. convertedProperties.put("sasl.kerberos.min.time.before.relogin", "0"); + return convertedProperties; + } + + private void setConvertedCustomProperties(Map convertedProperties) { convertedCustomProperties = convertedProperties; // This is mainly for compatibility. In the previous version, we directly obtained the value of the @@ -798,7 +809,9 @@ public void modifyProperties(AlterRoutineLoadCommand command) throws UserExcepti throw new DdlException("Only supports modification of PAUSED jobs"); } - modifyPropertiesInternal(jobProperties, dataSourceProperties); + Map convertedProperties = prepareModifyProperties(dataSourceProperties); + applyProperties(jobProperties, dataSourceProperties); + setConvertedCustomProperties(convertedProperties); AlterRoutineLoadJobOperationLog log = new AlterRoutineLoadJobOperationLog(this.id, jobProperties, dataSourceProperties); @@ -813,21 +826,37 @@ private void convertOffset(KafkaDataSourceProperties dataSourceProperties) throw if (partitionOffsets.isEmpty()) { return; } + Map properties; + String brokerListSnapshot; + String topicSnapshot; + writeLock(); + try { + // Recovery leaves the conversion cache empty. Validate the effective + // properties before an ALTER offset lookup can contact an older BE. + convertCustomProperties(false); + properties = Maps.newHashMap(convertedCustomProperties); + brokerListSnapshot = brokerList; + topicSnapshot = topic; + } finally { + writeUnlock(); + } String computeGroupName = getComputeGroupName(); List> newOffsets; if (dataSourceProperties.isOffsetsForTimes()) { newOffsets = KafkaUtil.getOffsetsForTimes( - brokerList, topic, convertedCustomProperties, partitionOffsets, computeGroupName); + brokerListSnapshot, topicSnapshot, properties, partitionOffsets, computeGroupName); } else { newOffsets = KafkaUtil.getRealOffsets( - brokerList, topic, convertedCustomProperties, partitionOffsets, computeGroupName); + brokerListSnapshot, topicSnapshot, properties, partitionOffsets, computeGroupName); } dataSourceProperties.setKafkaPartitionOffsets(newOffsets); } - private void modifyPropertiesInternal(Map jobProperties, - KafkaDataSourceProperties dataSourceProperties) + // Only a live ALTER validates Kafka properties, resolves files and resets cloud progress. + // Journal replay must apply the already committed transition without these effects. + private Map prepareModifyProperties(KafkaDataSourceProperties dataSourceProperties) throws UserException { + Map convertedProperties = convertedCustomProperties; if (null != dataSourceProperties) { List> kafkaPartitionOffsets = Lists.newArrayList(); Map customKafkaProperties = Maps.newHashMap(); @@ -837,10 +866,13 @@ private void modifyPropertiesInternal(Map jobProperties, customKafkaProperties = dataSourceProperties.getCustomKafkaProperties(); } - // convertCustomProperties and check partitions before reset progress to make modify operation atomic + // Prepare the merged properties without changing durable job state. For + // example, a missing CREATE FILE must not partially apply the ALTER. if (!customKafkaProperties.isEmpty()) { - this.customProperties.putAll(customKafkaProperties); - convertCustomProperties(true); + Map properties = Maps.newHashMap(customProperties); + properties.putAll(customKafkaProperties); + validateCustomProperties(properties); + convertedProperties = resolveCustomProperties(properties); } if (!kafkaPartitionOffsets.isEmpty()) { @@ -866,7 +898,22 @@ private void modifyPropertiesInternal(Map jobProperties, } resetCloudProgress(builder); } + } + return convertedProperties; + } + private void applyProperties(Map jobProperties, + KafkaDataSourceProperties dataSourceProperties) + throws UserException { + if (null != dataSourceProperties) { + List> kafkaPartitionOffsets = Lists.newArrayList(); + if (MapUtils.isNotEmpty(dataSourceProperties.getOriginalDataSourceProperties())) { + kafkaPartitionOffsets = dataSourceProperties.getKafkaPartitionOffsets(); + this.customProperties.putAll(dataSourceProperties.getCustomKafkaProperties()); + } + // A historical ALTER may contain a property rejected by today's policy. + // Restore all fields (including topic/offsets), then validate before use. + convertedCustomProperties = Maps.newHashMap(); // It is necessary to reset the Kafka progress cache if topic change, // and should reset cache before modifying partition offset. if (!Strings.isNullOrEmpty(dataSourceProperties.getTopic())) { @@ -927,7 +974,7 @@ private void resetCloudProgress(Cloud.ResetRLProgressRequest.Builder builder) th @Override public void replayModifyProperties(AlterRoutineLoadJobOperationLog log) { try { - modifyPropertiesInternal(log.getJobProperties(), (KafkaDataSourceProperties) log.getDataSourceProperties()); + applyProperties(log.getJobProperties(), (KafkaDataSourceProperties) log.getDataSourceProperties()); } catch (UserException e) { // should not happen LOG.error("failed to replay modify kafka routine load job: {}", id, e); diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index 35626245cd67b9..2a327c013bbde4 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -21,17 +21,26 @@ import org.apache.doris.catalog.Database; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.Table; +import org.apache.doris.cloud.rpc.MetaServiceProxy; import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; import org.apache.doris.common.DdlException; +import org.apache.doris.common.Pair; import org.apache.doris.common.jmockit.Deencapsulation; +import org.apache.doris.common.util.KafkaUtil; import org.apache.doris.common.util.SmallFileMgr; import org.apache.doris.common.util.SmallFileMgr.SmallFile; import org.apache.doris.datasource.CatalogMgr; import org.apache.doris.datasource.InternalCatalog; import org.apache.doris.load.routineload.kafka.KafkaDataSourceProperties; +import org.apache.doris.load.routineload.kafka.KafkaProgress; import org.apache.doris.load.routineload.kafka.KafkaRoutineLoadJob; import org.apache.doris.nereids.parser.NereidsParser; +import org.apache.doris.nereids.trees.plans.commands.load.AlterRoutineLoadCommand; import org.apache.doris.nereids.trees.plans.commands.load.CreateRoutineLoadCommand; +import org.apache.doris.persist.AlterRoutineLoadJobOperationLog; +import org.apache.doris.persist.EditLog; +import org.apache.doris.persist.RoutineLoadOperation; import org.apache.doris.persist.gson.GsonUtils; import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.OriginStatement; @@ -166,6 +175,173 @@ public void testPersistedPropertiesRejectedBeforeMetadataRequests() { Assertions.assertTrue(error.getMessage().contains("is not allowed in Routine Load")); } + // Build an already analyzed, persisted ALTER from an older FE without applying + // today's CREATE/ALTER admission rules to the historical input. + private AlterRoutineLoadJobOperationLog historicalAlter(Map custom, + String topic, String brokers, List> offsets) { + Map source = new HashMap<>(); + custom.forEach((key, value) -> source.put("property." + key, value)); + source.put("kafka_topic", topic); + source.put("kafka_broker_list", brokers); + KafkaDataSourceProperties properties = new KafkaDataSourceProperties(source); + properties.setAlter(true); + Deencapsulation.setField(properties, "dataSourceType", "KAFKA"); + Deencapsulation.setField(properties, "customKafkaProperties", custom); + Deencapsulation.setField(properties, "topic", topic); + Deencapsulation.setField(properties, "brokerList", brokers); + properties.setKafkaPartitionOffsets(offsets); + AlterRoutineLoadJobOperationLog log = new AlterRoutineLoadJobOperationLog(1L, + Map.of("max_batch_rows", "2000000"), properties); + return GsonUtils.GSON.fromJson(GsonUtils.GSON.toJson(log), AlterRoutineLoadJobOperationLog.class); + } + + @Test + public void testReplayHistoricalAlterRestoresCompleteState() throws Exception { + for (boolean cloud : new boolean[] {false, true}) { + try (MockedStatic config = Mockito.mockStatic(Config.class); + MockedStatic env = Mockito.mockStatic(Env.class); + MockedStatic metaService = Mockito.mockStatic(MetaServiceProxy.class); + MockedStatic kafka = Mockito.mockStatic(KafkaUtil.class)) { + config.when(Config::isCloudMode).thenReturn(cloud); + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "topic_a", UserIdentity.ADMIN); + Deencapsulation.setField(job, "progress", new KafkaProgress(Map.of(0, 5L, 1, 6L))); + job.prepare(); + + AlterRoutineLoadJobOperationLog log = historicalAlter( + Map.of("sasl.kerberos.min.time.before.relogin", "60000"), + "topic_b", "127.0.0.2:9092", List.of(Pair.of(0, 100L))); + job.replayModifyProperties(log); + + Assertions.assertEquals("topic_b", job.getTopic()); + Assertions.assertEquals("127.0.0.2:9092", job.getBrokerList()); + Assertions.assertEquals(Map.of(0, 100L), + ((KafkaProgress) job.getProgress()).getOffsetByPartition()); + Assertions.assertEquals(2000000L, job.getMaxBatchRows()); + Assertions.assertEquals("60000", job.getCustomProperties() + .get("property.sasl.kerberos.min.time.before.relogin")); + Assertions.assertTrue(job.getConvertedCustomProperties().isEmpty()); + + DdlException error = Assertions.assertThrows(DdlException.class, job::prepare); + Assertions.assertTrue(error.getMessage().contains("Kerberos tickets must be managed externally")); + Assertions.assertThrows(DdlException.class, + () -> Deencapsulation.invoke(job, "getAllKafkaPartitions")); + Assertions.assertThrows(DdlException.class, + () -> Deencapsulation.invoke(job, "convertOffset", log.getDataSourceProperties())); + + // A later committed repair must retain topic B and its offsets. + job.replayModifyProperties(historicalAlter( + Map.of("sasl.kerberos.min.time.before.relogin", "0"), "", "", List.of())); + job.prepare(); + Assertions.assertEquals("topic_b", job.getTopic()); + Assertions.assertEquals("127.0.0.2:9092", job.getBrokerList()); + Assertions.assertEquals(Map.of(0, 100L), + ((KafkaProgress) job.getProgress()).getOffsetByPartition()); + Assertions.assertEquals("0", job.getConvertedCustomProperties() + .get("sasl.kerberos.min.time.before.relogin")); + env.verifyNoInteractions(); + metaService.verifyNoInteractions(); + kafka.verifyNoInteractions(); + } + } + } + + @Test + public void testReplayDefersManagedFileResolution() throws Exception { + Env env = Mockito.mock(Env.class); + SmallFileMgr smallFileMgr = Mockito.mock(SmallFileMgr.class); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + Mockito.when(env.getSmallFileMgr()).thenReturn(smallFileMgr); + Mockito.when(smallFileMgr.getSmallFile(1L, "kafka", "missing.pem", true)) + .thenThrow(new DdlException("missing managed file")); + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "topic_a", UserIdentity.ADMIN); + + // Partition 3 need not exist in the pre-replay progress. These offsets + // were checked when the ALTER was committed, not during recovery. + job.replayModifyProperties(historicalAlter(Map.of("ssl.ca.location", "FILE:missing.pem"), + "topic_b", "127.0.0.2:9092", List.of(Pair.of(3, 100L)))); + Assertions.assertEquals("topic_b", job.getTopic()); + Assertions.assertEquals(Map.of(3, 100L), + ((KafkaProgress) job.getProgress()).getOffsetByPartition()); + Assertions.assertEquals(2000000L, job.getMaxBatchRows()); + envStatic.verifyNoInteractions(); + Mockito.verifyNoInteractions(smallFileMgr); + + DdlException error = Assertions.assertThrows(DdlException.class, job::prepare); + Assertions.assertTrue(error.getMessage().contains("missing managed file")); + Assertions.assertTrue(job.getConvertedCustomProperties().isEmpty()); + } + } + + @Test + public void testSchedulerPausesReplayedJobWithRejectedProperties() throws Exception { + Env env = Mockito.mock(Env.class); + EditLog editLog = Mockito.mock(EditLog.class); + RoutineLoadManager manager = Mockito.mock(RoutineLoadManager.class); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class); + MockedStatic kafka = Mockito.mockStatic(KafkaUtil.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + Mockito.when(env.getEditLog()).thenReturn(editLog); + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "topic_a", UserIdentity.ADMIN); + job.replayModifyProperties(historicalAlter(Map.of("plugin.library.paths", "unused.so"), + "topic_b", "127.0.0.2:9092", List.of())); + Mockito.when(manager.getRoutineLoadJobByState(Mockito.anySet())).thenReturn(List.of(job)); + + Deencapsulation.invoke(new RoutineLoadScheduler(manager), "process"); + + Assertions.assertEquals(RoutineLoadJob.JobState.PAUSED, job.getState()); + Assertions.assertTrue(job.getPauseReason().getMsg().contains("plugin.library.paths")); + Assertions.assertEquals("topic_b", job.getTopic()); + kafka.verifyNoInteractions(); + Mockito.verify(editLog).logOpRoutineLoadJob(Mockito.any(RoutineLoadOperation.class)); + } + } + + @Test + public void testLiveAlterPublishesOnlyPreparedProperties() throws Exception { + Env env = Mockito.mock(Env.class); + SmallFileMgr smallFileMgr = Mockito.mock(SmallFileMgr.class); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + Mockito.when(env.getSmallFileMgr()).thenReturn(smallFileMgr); + Mockito.when(smallFileMgr.getSmallFile(1L, "kafka", "missing.pem", true)) + .thenThrow(new DdlException("missing managed file")); + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "topic_a", UserIdentity.ADMIN); + Deencapsulation.setField(job, "state", RoutineLoadJob.JobState.PAUSED); + job.prepare(); + Map converted = new HashMap<>(job.getConvertedCustomProperties()); + AlterRoutineLoadJobOperationLog log = historicalAlter(Map.of("ssl.ca.location", "FILE:missing.pem"), + "topic_b", "127.0.0.2:9092", List.of()); + AlterRoutineLoadCommand command = Mockito.mock(AlterRoutineLoadCommand.class); + Mockito.when(command.getAnalyzedJobProperties()).thenReturn(log.getJobProperties()); + Mockito.when(command.getDataSourceProperties()).thenReturn(log.getDataSourceProperties()); + + Assertions.assertThrows(DdlException.class, () -> job.modifyProperties(command)); + Assertions.assertEquals("topic_a", job.getTopic()); + Assertions.assertEquals("127.0.0.1:9092", job.getBrokerList()); + Assertions.assertTrue(job.getCustomProperties().isEmpty()); + Assertions.assertEquals(converted, job.getConvertedCustomProperties()); + Mockito.verify(env, Mockito.never()).getEditLog(); + + KafkaDataSourceProperties safe = properties("sasl.kerberos.min.time.before.relogin", "0", true); + safe.analyze(); + Mockito.when(command.getDataSourceProperties()).thenReturn(safe); + EditLog editLog = Mockito.mock(EditLog.class); + Mockito.when(env.getEditLog()).thenReturn(editLog); + job.modifyProperties(command); + Assertions.assertEquals(Map.of("property.sasl.kerberos.min.time.before.relogin", "0"), + job.getCustomProperties()); + Assertions.assertEquals("0", job.getConvertedCustomProperties() + .get("sasl.kerberos.min.time.before.relogin")); + Assertions.assertEquals(2000000L, job.getMaxBatchRows()); + Mockito.verify(editLog).logAlterRoutineLoadJob(Mockito.any(AlterRoutineLoadJobOperationLog.class)); + } + } + @Test public void testRecoverMigratedCertificateProperties() throws Exception { Env env = Mockito.mock(Env.class); From d7c0bf5a82fd14cf59a199c709194048c53eb7b5 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Sat, 24 Oct 2026 17:37:02 +0800 Subject: [PATCH 11/12] [fix](load) Preserve Kafka ALTER defaults and validate merged properties ### What problem does this PR solve? Problem Summary: Reusing a datetime default-offset conversion cache during a job-only ALTER could replace the date with its millisecond value, causing new partitions to start at OFFSET_END. Remove both default-offset bookkeeping keys when extracting the original datetime. Offset lookups now validate and resolve the merged ALTER properties, allowing a historical certificate path to be repaired in the same ALTER without publishing state before lookup succeeds. Correct the FE validation test imports and its expected default-offset entry. Add tests for datetime defaults across repeated job-only ALTERs, certificate repairs with numeric or timestamp offsets, and metadata-failure atomicity. ### Release note Preserve datetime defaults for newly discovered Routine Load Kafka partitions and allow certificate repairs together with partition-offset changes. ### Check List (For Author) - Test: FE Checkstyle passed with zero violations; git diff --check passed. An isolated Java reproduction using the actual default-extraction method confirms that reusing the cache preserves the datetime. The standard run-fe-ut.sh entry point was attempted but stopped because JDK 17 is missing; the added FE unit tests have not run locally. - Behavior changed: Yes; repair ALTERs use merged properties for offset lookup, and job-only ALTERs retain datetime default offsets. - Does this need documentation: No; fixes behavior within the existing interface. --- .../kafka/KafkaRoutineLoadJob.java | 15 ++- .../KafkaPropertyValidationTest.java | 116 +++++++++++++++++- 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java index 105b997d9c0045..a5d2dea9782e24 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java +++ b/fe/fe-core/src/main/java/org/apache/doris/load/routineload/kafka/KafkaRoutineLoadJob.java @@ -259,6 +259,9 @@ private void setConvertedCustomProperties(Map convertedPropertie if (convertedCustomProperties.containsKey(KafkaConfiguration.KAFKA_ORIGIN_DEFAULT_OFFSETS.getName())) { kafkaDefaultOffSet = convertedCustomProperties .remove(KafkaConfiguration.KAFKA_ORIGIN_DEFAULT_OFFSETS.getName()); + // Both entries describe the same default. Do not leave the millis value + // behind to overwrite the datetime when an ALTER reuses this cache. + convertedCustomProperties.remove(KafkaConfiguration.KAFKA_DEFAULT_OFFSETS.getName()); return; } if (convertedCustomProperties.containsKey(KafkaConfiguration.KAFKA_DEFAULT_OFFSETS.getName())) { @@ -831,10 +834,14 @@ private void convertOffset(KafkaDataSourceProperties dataSourceProperties) throw String topicSnapshot; writeLock(); try { - // Recovery leaves the conversion cache empty. Validate the effective - // properties before an ALTER offset lookup can contact an older BE. - convertCustomProperties(false); - properties = Maps.newHashMap(convertedCustomProperties); + // An ALTER can repair properties accepted by an older FE. Validate and + // resolve the merged map before lookup without publishing it to the job. + Map mergedProperties = Maps.newHashMap(customProperties); + mergedProperties.putAll(dataSourceProperties.getCustomKafkaProperties()); + validateCustomProperties(mergedProperties); + properties = resolveCustomProperties(mergedProperties); + properties.remove(KafkaConfiguration.KAFKA_ORIGIN_DEFAULT_OFFSETS.getName()); + properties.remove(KafkaConfiguration.KAFKA_DEFAULT_OFFSETS.getName()); brokerListSnapshot = brokerList; topicSnapshot = topic; } finally { diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index 2a327c013bbde4..88abda8940f412 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -25,18 +25,20 @@ import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.DdlException; +import org.apache.doris.common.LoadException; import org.apache.doris.common.Pair; import org.apache.doris.common.jmockit.Deencapsulation; -import org.apache.doris.common.util.KafkaUtil; import org.apache.doris.common.util.SmallFileMgr; import org.apache.doris.common.util.SmallFileMgr.SmallFile; +import org.apache.doris.common.util.TimeUtils; import org.apache.doris.datasource.CatalogMgr; import org.apache.doris.datasource.InternalCatalog; +import org.apache.doris.datasource.kafka.KafkaUtil; import org.apache.doris.load.routineload.kafka.KafkaDataSourceProperties; import org.apache.doris.load.routineload.kafka.KafkaProgress; import org.apache.doris.load.routineload.kafka.KafkaRoutineLoadJob; import org.apache.doris.nereids.parser.NereidsParser; -import org.apache.doris.nereids.trees.plans.commands.load.AlterRoutineLoadCommand; +import org.apache.doris.nereids.trees.plans.commands.AlterRoutineLoadCommand; import org.apache.doris.nereids.trees.plans.commands.load.CreateRoutineLoadCommand; import org.apache.doris.persist.AlterRoutineLoadJobOperationLog; import org.apache.doris.persist.EditLog; @@ -333,7 +335,8 @@ public void testLiveAlterPublishesOnlyPreparedProperties() throws Exception { EditLog editLog = Mockito.mock(EditLog.class); Mockito.when(env.getEditLog()).thenReturn(editLog); job.modifyProperties(command); - Assertions.assertEquals(Map.of("property.sasl.kerberos.min.time.before.relogin", "0"), + Assertions.assertEquals(Map.of("property.sasl.kerberos.min.time.before.relogin", "0", + "property.kafka_default_offsets", KafkaProgress.OFFSET_END), job.getCustomProperties()); Assertions.assertEquals("0", job.getConvertedCustomProperties() .get("sasl.kerberos.min.time.before.relogin")); @@ -342,6 +345,113 @@ public void testLiveAlterPublishesOnlyPreparedProperties() throws Exception { } } + @Test + public void testJobOnlyAlterPreservesDatetimeDefaultForNewPartitions() throws Exception { + Env env = Mockito.mock(Env.class); + EditLog editLog = Mockito.mock(EditLog.class); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class); + MockedStatic kafka = Mockito.mockStatic(KafkaUtil.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + Mockito.when(env.getEditLog()).thenReturn(editLog); + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "topic_a", UserIdentity.ADMIN); + String datetime = "2026-10-01 00:00:00"; + KafkaDataSourceProperties source = properties("kafka_default_offsets", datetime, false); + source.analyze(); + Deencapsulation.setField(job, "customProperties", source.getCustomKafkaProperties()); + Deencapsulation.setField(job, "state", RoutineLoadJob.JobState.PAUSED); + job.prepare(); + + AlterRoutineLoadCommand command = Mockito.mock(AlterRoutineLoadCommand.class); + Mockito.when(command.getAnalyzedJobProperties()).thenReturn(Map.of("max_batch_rows", "2000000")); + job.modifyProperties(command); + job.modifyProperties(command); + Assertions.assertEquals(datetime, Deencapsulation.getField(job, "kafkaDefaultOffSet")); + Assertions.assertFalse(job.getConvertedCustomProperties().containsKey("kafka_default_offsets")); + Assertions.assertFalse(job.getConvertedCustomProperties().containsKey("kafka_origin_default_offsets")); + + // New partitions are initialized before prepare() rebuilds the cache on resume. + long timestamp = TimeUtils.timeStringToLong(datetime, TimeUtils.getOrSystemTimeZone(job.getTimezone())); + List> requested = List.of(Pair.of(1, timestamp)); + List> expected = List.of(Pair.of(1, 42L)); + kafka.when(() -> KafkaUtil.getOffsetsForTimes("127.0.0.1:9092", "topic_a", + job.getConvertedCustomProperties(), requested, null)).thenReturn(expected); + List> offsets = Deencapsulation.invoke(job, + "getNewPartitionOffsetsFromDefaultOffset", List.of(1)); + Assertions.assertEquals(expected, offsets); + kafka.verify(() -> KafkaUtil.getOffsetsForTimes("127.0.0.1:9092", "topic_a", + job.getConvertedCustomProperties(), requested, null)); + kafka.verifyNoMoreInteractions(); + } + } + + @Test + public void testRepairCertificateAndOffsetsInSameAlter() throws Exception { + for (boolean offsetsForTimes : new boolean[] {false, true}) { + Env env = Mockito.mock(Env.class); + SmallFileMgr smallFileMgr = Mockito.mock(SmallFileMgr.class); + EditLog editLog = Mockito.mock(EditLog.class); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class); + MockedStatic kafka = Mockito.mockStatic(KafkaUtil.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + Mockito.when(env.getSmallFileMgr()).thenReturn(smallFileMgr); + Mockito.when(env.getEditLog()).thenReturn(editLog); + SmallFile ca = new SmallFile(1L, "kafka", "ca.pem", 3L, "", 0L, "checksum", true); + Mockito.when(smallFileMgr.getSmallFile(1L, "kafka", "ca.pem", true)).thenReturn(ca); + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "topic_a", UserIdentity.ADMIN); + Deencapsulation.setField(job, "customProperties", + new HashMap<>(Map.of("ssl.ca.location", "/etc/ca.pem"))); + Deencapsulation.setField(job, "progress", new KafkaProgress(Map.of(0, 5L))); + Deencapsulation.setField(job, "state", RoutineLoadJob.JobState.PAUSED); + + Map input = Map.of("property.ssl.ca.location", "FILE:ca.pem", + "kafka_partitions", "0", "kafka_offsets", offsetsForTimes ? "2026-10-01 00:00:00" : "100"); + KafkaDataSourceProperties source = new KafkaDataSourceProperties(input); + source.setAlter(true); + source.setTimezone("UTC"); + source.analyze(); + Map converted = Map.of("ssl.ca.location", "FILE:3:checksum", + "sasl.kerberos.min.time.before.relogin", "0"); + List> requested = source.getKafkaPartitionOffsets(); + List> expected = List.of(Pair.of(0, 100L)); + if (offsetsForTimes) { + kafka.when(() -> KafkaUtil.getOffsetsForTimes("127.0.0.1:9092", "topic_a", + converted, requested, null)).thenThrow(new LoadException("metadata unavailable")) + .thenReturn(expected); + } else { + kafka.when(() -> KafkaUtil.getRealOffsets("127.0.0.1:9092", "topic_a", + converted, requested, null)).thenThrow(new LoadException("metadata unavailable")) + .thenCallRealMethod(); + } + AlterRoutineLoadCommand command = Mockito.mock(AlterRoutineLoadCommand.class); + Mockito.when(command.getAnalyzedJobProperties()).thenReturn(Map.of()); + Mockito.when(command.getDataSourceProperties()).thenReturn(source); + + Assertions.assertThrows(LoadException.class, () -> job.modifyProperties(command)); + Assertions.assertEquals("/etc/ca.pem", job.getCustomProperties().get("property.ssl.ca.location")); + Assertions.assertTrue(job.getConvertedCustomProperties().isEmpty()); + Assertions.assertEquals(Map.of(0, 5L), ((KafkaProgress) job.getProgress()).getOffsetByPartition()); + Mockito.verifyNoInteractions(editLog); + + job.modifyProperties(command); + + Assertions.assertEquals("FILE:ca.pem", job.getCustomProperties().get("property.ssl.ca.location")); + Assertions.assertEquals(converted, job.getConvertedCustomProperties()); + Assertions.assertEquals(Map.of(0, 100L), ((KafkaProgress) job.getProgress()).getOffsetByPartition()); + if (offsetsForTimes) { + kafka.verify(() -> KafkaUtil.getOffsetsForTimes("127.0.0.1:9092", "topic_a", + converted, requested, null), Mockito.times(2)); + } else { + kafka.verify(() -> KafkaUtil.getRealOffsets("127.0.0.1:9092", "topic_a", + converted, requested, null), Mockito.times(2)); + } + kafka.verifyNoMoreInteractions(); + Mockito.verify(editLog).logAlterRoutineLoadJob(Mockito.any(AlterRoutineLoadJobOperationLog.class)); + } + } + } + @Test public void testRecoverMigratedCertificateProperties() throws Exception { Env env = Mockito.mock(Env.class); From 55d7da1b4a52437d20427b5f69bace33d5d13551 Mon Sep 17 00:00:00 2001 From: laihui <1353307710@qq.com> Date: Sat, 24 Oct 2026 20:37:54 +0800 Subject: [PATCH 12/12] [fix](load) Preserve Routine Load OIDC credential aliases ### What problem does this PR solve? Problem Summary: The Kafka property allowlist omitted librdkafka's supported sasl.oauthbearer.client.credentials.client.id and client.secret aliases. Existing OIDC jobs using these names failed runtime validation, and ALTER could not remove the rejected persisted keys. Admit both names in the shared FE/BE allowlist, retaining their existing librdkafka semantics. Add FE CREATE/ALTER and restored-job coverage, including a live ALTER adding a short credential name alongside historical aliases and secret masking. Add BE admission coverage while retaining mandatory Kerberos shell suppression. ### Release note Routine Load continues to accept the supported long OIDC client ID and secret configuration aliases after upgrade. ### Check List (For Author) - Test: FE Checkstyle passed with zero violations; the repository formatter wrapper using clang-format 16 and git diff --check passed. Standard FE and BE unit-test entry points were attempted but stopped because JDK 17 is missing, so the added tests have not run locally. The Thrift context was compared with branch-3.1/4.0/4.1/4.2; only constant-set members change. - Behavior changed: Yes; supported OIDC credential aliases pass FE/BE validation. - Does this need documentation: No; compatibility fix for existing property names. --- .../routine_load/kafka_properties_test.cpp | 16 ++++ .../KafkaPropertyValidationTest.java | 89 +++++++++++++++++++ gensrc/thrift/BackendService.thrift | 2 + 3 files changed, 107 insertions(+) diff --git a/be/test/load/routine_load/kafka_properties_test.cpp b/be/test/load/routine_load/kafka_properties_test.cpp index 296979f506dfae..15f314cd976e41 100644 --- a/be/test/load/routine_load/kafka_properties_test.cpp +++ b/be/test/load/routine_load/kafka_properties_test.cpp @@ -88,6 +88,22 @@ TEST(KafkaPropertiesTest, AcceptConsumerAndAwsProperties) { .ok()); } +TEST(KafkaPropertiesTest, AcceptOidcCredentialAliases) { + std::unique_ptr conf(RdKafka::Conf::create(RdKafka::Conf::CONF_GLOBAL)); + ASSERT_TRUE(configure_kafka_security( + *conf, + {{"security.protocol", "SASL_SSL"}, + {"sasl.mechanism", "OAUTHBEARER"}, + {"sasl.oauthbearer.method", "oidc"}, + {"sasl.oauthbearer.client.credentials.client.id", "test-client"}, + {"sasl.oauthbearer.client.credentials.client.secret", "test-secret"}, + {"sasl.oauthbearer.token.endpoint.url", "https://issuer.example/token"}}) + .ok()); + std::string value; + ASSERT_EQ(RdKafka::Conf::CONF_OK, conf->get("sasl.kerberos.min.time.before.relogin", value)); + EXPECT_EQ("0", value); +} + TEST(KafkaPropertiesTest, ConsumerRejectsBeforeResolvingFilesOrCreatingClient) { auto ctx = std::make_shared(nullptr); TKafkaLoadInfo info; diff --git a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java index 88abda8940f412..a25b38a4cb75e6 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/load/routineload/KafkaPropertyValidationTest.java @@ -132,6 +132,95 @@ public void testNormalConsumerPropertiesAndDefaultOffsets() throws Exception { } } + @Test + public void testOidcCredentialAliasesCreateAndAlter() throws Exception { + for (boolean alter : new boolean[] {false, true}) { + Map source = new HashMap<>(); + source.put("kafka_broker_list", "127.0.0.1:9092"); + source.put("kafka_topic", "test_topic"); + oidcAliasProperties().forEach((key, value) -> source.put("property." + key, value)); + KafkaDataSourceProperties properties = new KafkaDataSourceProperties(source); + properties.setAlter(alter); + properties.setTimezone("UTC"); + properties.analyze(); + oidcAliasProperties().forEach((key, value) -> + Assertions.assertEquals(value, properties.getCustomKafkaProperties().get(key))); + } + } + + private Map oidcAliasProperties() { + return Map.of("security.protocol", "SASL_SSL", + "sasl.mechanism", "OAUTHBEARER", + "sasl.oauthbearer.method", "oidc", + "sasl.oauthbearer.client.credentials.client.id", "test-client", + "sasl.oauthbearer.client.credentials.client.secret", "test-secret", + "sasl.oauthbearer.token.endpoint.url", "https://issuer.example/token"); + } + + @Test + public void testRecoverAndAlterJobWithOidcCredentialAliases() throws Exception { + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + InternalCatalog catalog = Mockito.mock(InternalCatalog.class); + Database db = Mockito.mock(Database.class); + Table table = Mockito.mock(Table.class); + EditLog editLog = Mockito.mock(EditLog.class); + String originalSql = "CREATE ROUTINE LOAD test_db.test_job ON test_table COLUMNS(id) " + + "FROM KAFKA (\"kafka_broker_list\" = \"127.0.0.1:9092\", \"kafka_topic\" = \"test_topic\", " + + "\"property.security.protocol\" = \"SASL_SSL\", \"property.sasl.mechanism\" = \"OAUTHBEARER\", " + + "\"property.sasl.oauthbearer.method\" = \"oidc\", " + + "\"property.sasl.oauthbearer.client.credentials.client.id\" = \"test-client\", " + + "\"property.sasl.oauthbearer.client.credentials.client.secret\" = \"test-secret\", " + + "\"property.sasl.oauthbearer.token.endpoint.url\" = \"https://issuer.example/token\")"; + + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + envStatic.when(Env::getCurrentInternalCatalog).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(catalogMgr.getCatalog(InternalCatalog.INTERNAL_CATALOG_NAME)).thenReturn(catalog); + Mockito.when(env.getInternalCatalog()).thenReturn(catalog); + Mockito.when(env.getEditLog()).thenReturn(editLog); + Mockito.when(catalog.getName()).thenReturn(InternalCatalog.INTERNAL_CATALOG_NAME); + Mockito.doReturn(Optional.of(db)).when(catalog).getDb(1L); + Mockito.doReturn(Optional.of(db)).when(catalog).getDb("test_db"); + Mockito.doReturn(db).when(catalog).getDbOrAnalysisException("test_db"); + Mockito.when(db.getId()).thenReturn(1L); + Mockito.when(db.getName()).thenReturn("test_db"); + Mockito.doReturn(Optional.of(table)).when(db).getTable(2L); + Mockito.doReturn(table).when(db).getTableOrAnalysisException("test_table"); + Mockito.when(table.getName()).thenReturn("test_table"); + + KafkaRoutineLoadJob job = new KafkaRoutineLoadJob(1L, "test_job", 1L, 2L, + "127.0.0.1:9092", "test_topic", UserIdentity.ADMIN); + job.setOrigStmt(new OriginStatement(originalSql, 0)); + Deencapsulation.setField(job, "state", RoutineLoadJob.JobState.PAUSED); + Deencapsulation.setField(job, "customProperties", new HashMap<>(oidcAliasProperties())); + KafkaRoutineLoadJob restored = (KafkaRoutineLoadJob) GsonUtils.GSON.fromJson( + GsonUtils.GSON.toJson(job), RoutineLoadJob.class); + Assertions.assertEquals(RoutineLoadJob.JobState.PAUSED, restored.getState()); + Assertions.assertEquals(-1L, restored.getEndTimestamp()); + restored.prepare(); + oidcAliasProperties().forEach((key, value) -> + Assertions.assertEquals(value, restored.getConvertedCustomProperties().get(key))); + + // Adding short names must not make the retained historical aliases fail validation. + KafkaDataSourceProperties update = properties("sasl.oauthbearer.client.secret", "test-secret", true); + update.analyze(); + AlterRoutineLoadCommand command = Mockito.mock(AlterRoutineLoadCommand.class); + Mockito.when(command.getAnalyzedJobProperties()).thenReturn(Map.of()); + Mockito.when(command.getDataSourceProperties()).thenReturn(update); + restored.modifyProperties(command); + Assertions.assertEquals("test-secret", restored.getConvertedCustomProperties() + .get("sasl.oauthbearer.client.credentials.client.secret")); + Assertions.assertEquals("test-secret", restored.getConvertedCustomProperties() + .get("sasl.oauthbearer.client.secret")); + Assertions.assertEquals("******", restored.getCustomProperties() + .get("property.sasl.oauthbearer.client.credentials.client.secret")); + Assertions.assertFalse(restored.customPropertiesJsonToString().contains("test-secret")); + Mockito.verify(editLog).logAlterRoutineLoadJob(Mockito.any(AlterRoutineLoadJobOperationLog.class)); + } + } + @Test public void testAwsPropertiesWithAndWithoutPrefix() throws Exception { for (String prefix : List.of("", "property.")) { diff --git a/gensrc/thrift/BackendService.thrift b/gensrc/thrift/BackendService.thrift index 98f258b9222242..e63450e6085e75 100644 --- a/gensrc/thrift/BackendService.thrift +++ b/gensrc/thrift/BackendService.thrift @@ -93,6 +93,8 @@ const set KAFKA_ALLOWED_PROPERTIES = [ "sasl.kerberos.service.name", "sasl.mechanism", "sasl.mechanisms", + "sasl.oauthbearer.client.credentials.client.id", + "sasl.oauthbearer.client.credentials.client.secret", "sasl.oauthbearer.client.id", "sasl.oauthbearer.client.secret", "sasl.oauthbearer.config",