Skip to content

Commit

Permalink
fix ut
Browse files Browse the repository at this point in the history
  • Loading branch information
morningman committed Jan 27, 2025
1 parent 3ac2379 commit 972fb7e
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 37 deletions.
2 changes: 1 addition & 1 deletion be/src/common/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ DEFINE_Int32(rocksdb_max_write_buffer_number, "5");

DEFINE_mBool(allow_zero_date, "false");
DEFINE_Bool(allow_invalid_decimalv2_literal, "false");
DEFINE_mString(kerberos_ccache_path, "");
DEFINE_mString(kerberos_ccache_path, "/tmp/");
DEFINE_mString(kerberos_krb5_conf_path, "/etc/krb5.conf");
DEFINE_mInt32(kerberos_refresh_interval_second, "3600");

Expand Down
19 changes: 9 additions & 10 deletions be/src/common/kerberos/kerberos_ticket_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,11 @@

namespace doris::kerberos {

// Initialize static member
std::string KerberosTicketCache::_ccache_root_dir = config::kerberos_ccache_path;

KerberosTicketCache::KerberosTicketCache(const KerberosConfig& config,
KerberosTicketCache::KerberosTicketCache(const KerberosConfig& config, const std::string& root_path,
std::unique_ptr<Krb5Interface> krb5_interface)
: _config(config), _krb5_interface(std::move(krb5_interface)) {}
: _config(config),
_ccache_root_dir(root_path),
_krb5_interface(std::move(krb5_interface)) {}

KerberosTicketCache::~KerberosTicketCache() {
stop_periodic_refresh();
Expand All @@ -49,7 +48,8 @@ Status KerberosTicketCache::initialize() {
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() << ": " << st.to_string();
<< " with principal: " << _config.get_principal()
<< " and keytab: " << _config.get_keytab_path() << ": " << st.to_string();
return st;
}

Expand All @@ -60,13 +60,15 @@ Status KerberosTicketCache::_init_ticket_cache_path() {
Md5Digest digest;
digest.update(combined.c_str(), combined.length());
digest.digest();
std::string cache_file_md5 = digest.hex();
std::string cache_file_md5 = "doris_krb_" + digest.hex();

// The path should be with prefix "_ccache_root_dir"
std::filesystem::path full_path = std::filesystem::path(_ccache_root_dir) / cache_file_md5;
full_path = std::filesystem::weakly_canonical(full_path);
std::filesystem::path parent_path = full_path.parent_path();

LOG(INFO) << "try creating kerberos ticket path: " << full_path.string()
<< " for principal: " << _config.get_principal();
try {
if (!std::filesystem::exists(parent_path)) {
// Create the parent dir if not exists
Expand Down Expand Up @@ -254,15 +256,12 @@ Status KerberosTicketCache::_initialize_context() {
void KerberosTicketCache::_cleanup_context() {
if (_principal) {
_krb5_interface->free_principal(_context, _principal);
std::cout << "11" << std::endl;
}
if (_ccache) {
_krb5_interface->cc_close(_context, _ccache);
std::cout << "22" << std::endl;
}
if (_context) {
_krb5_interface->free_context(_context);
std::cout << "33" << std::endl;
}
}

Expand Down
10 changes: 3 additions & 7 deletions be/src/common/kerberos/kerberos_ticket_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class KerberosTicketCache {
public:
// Constructor that takes a Kerberos configuration and an optional KRB5 interface implementation
explicit KerberosTicketCache(
const KerberosConfig& config,
const KerberosConfig& config, const std::string& root_path,
std::unique_ptr<Krb5Interface> krb5_interface = Krb5InterfaceFactory::create());

virtual ~KerberosTicketCache();
Expand Down Expand Up @@ -99,9 +99,6 @@ class KerberosTicketCache {
// For testing purposes
void set_ticket_cache_path(const std::string& mock_path) { _ticket_cache_path = mock_path; }

// For testing purposes
static void set_ccache_root_dir(const std::string& dir) { _ccache_root_dir = dir; }

private:
// Initialize the ticket cache file path using principal and keytab information
Status _init_ticket_cache_path();
Expand All @@ -115,6 +112,8 @@ class KerberosTicketCache {
private:
// Kerberos configuration containing principal, keytab, and refresh settings
KerberosConfig _config;
// For testing purposes
std::string _ccache_root_dir;
// Path to the ticket cache file
std::string _ticket_cache_path;
// Kerberos context handle
Expand All @@ -135,9 +134,6 @@ class KerberosTicketCache {

// Interface for KRB5 operations
std::unique_ptr<Krb5Interface> _krb5_interface;

// For testing purposes
static std::string _ccache_root_dir;
};

} // namespace doris::kerberos
5 changes: 3 additions & 2 deletions be/src/common/kerberos/kerberos_ticket_mgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@

namespace doris::kerberos {

KerberosTicketMgr::KerberosTicketMgr(int32_t cache_expire_seconds) {
KerberosTicketMgr::KerberosTicketMgr(int32_t cache_expire_seconds, const std::string& root_path) {
_root_path = root_path;
_expiration_time_seconds = std::chrono::seconds(cache_expire_seconds);
_start_cleanup_thread();
}
Expand Down Expand Up @@ -80,7 +81,7 @@ Status KerberosTicketMgr::get_or_set_ticket_cache(const KerberosConfig& config,

std::unique_ptr<KerberosTicketCache> KerberosTicketMgr::_make_new_ticket_cache(
const KerberosConfig& config) {
return std::make_unique<KerberosTicketCache>(config);
return std::make_unique<KerberosTicketCache>(config, _root_path);
}

Status KerberosTicketMgr::get_cache_file_path(const std::string& principal,
Expand Down
5 changes: 4 additions & 1 deletion be/src/common/kerberos/kerberos_ticket_mgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct KerberosTicketEntry {
class KerberosTicketMgr {
public:
// Constructor that takes the expiration time for unused ticket caches
explicit KerberosTicketMgr(int32_t cache_expire_seconds);
explicit KerberosTicketMgr(int32_t cache_expire_seconds, const std::string& root_path);

// Get or create a ticket cache for the given Kerberos configuration
// Logic: Checks if cache exists, if not creates new one, initializes it,
Expand Down Expand Up @@ -82,6 +82,9 @@ class KerberosTicketMgr {
virtual std::unique_ptr<KerberosTicketCache> _make_new_ticket_cache(
const KerberosConfig& config);

protected:
// The root dir of ticket caches
std::string _root_path;
// Map storing ticket caches, keyed by MD5 hash of principal and keytab path
std::unordered_map<std::string, KerberosTicketEntry> _ticket_caches;
// Mutex for thread-safe access to the ticket cache map
Expand Down
9 changes: 5 additions & 4 deletions be/src/io/hdfs_builder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

namespace doris {

#ifdef USE_HADOOP_HDFS && USE_DORIS_HADOOP_HDFS
#ifdef USE_DORIS_HADOOP_HDFS
void err_log_message(const char* fmt, ...) {
va_list args;
va_start(args, fmt);
Expand Down Expand Up @@ -91,13 +91,13 @@ void va_err_log_message(const char* fmt, va_list ap) {

struct hdfsLogger logger = {.errLogMessage = err_log_message,
.vaErrLogMessage = va_err_log_message};
#endif // #ifdef USE_HADOOP_HDFS && USE_DORIS_HADOOP_HDFS
#endif // #ifdef USE_DORIS_HADOOP_HDFS

Status HDFSCommonBuilder::init_hdfs_builder() {
#ifdef USE_HADOOP_HDFS && USE_DORIS_HADOOP_HDFS
#ifdef USE_DORIS_HADOOP_HDFS
static std::once_flag flag;
std::call_once(flag, []() { hdfsSetLogger(&logger); });
#endif // #ifdef USE_HADOOP_HDFS && USE_DORIS_HADOOP_HDFS
#endif // #ifdef USE_DORIS_HADOOP_HDFS

hdfs_builder = hdfsNewBuilder();
if (hdfs_builder == nullptr) {
Expand Down Expand Up @@ -233,6 +233,7 @@ Status create_hdfs_builder(const THdfsParams& hdfsParams, const std::string& fs_
hdfsBuilderSetUserName(builder->get(), builder->hadoop_user.c_str());
}
}
return Status::OK();
}

Status create_hdfs_builder(const std::map<std::string, std::string>& properties,
Expand Down
2 changes: 1 addition & 1 deletion be/src/runtime/exec_env_init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ Status ExecEnv::_init(const std::vector<StorePath>& store_paths,
_dns_cache = new DNSCache();
_write_cooldown_meta_executors = std::make_unique<WriteCooldownMetaExecutors>();
_spill_stream_mgr = new vectorized::SpillStreamManager(std::move(spill_store_map));
_kerberos_ticket_mgr = new kerberos::KerberosTicketMgr(86400);
_kerberos_ticket_mgr = new kerberos::KerberosTicketMgr(86400, config::kerberos_ccache_path);
_backend_client_cache->init_metrics("backend");
_frontend_client_cache->init_metrics("frontend");
_broker_client_cache->init_metrics("broker");
Expand Down
3 changes: 1 addition & 2 deletions be/test/common/kerberos/kerberos_ticket_cache_auth_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ class KerberosTicketCacheAuthTest : public testing::Test {
std::filesystem::remove_all(_temp_dir);
}
std::filesystem::create_directories(_temp_dir);
KerberosTicketCache::set_ccache_root_dir(_temp_dir.string());
}

void TearDown() override {
Expand Down Expand Up @@ -100,7 +99,7 @@ TEST_F(KerberosTicketCacheAuthTest, TestRealAuthentication) {
std::string cache_path;
{
// Create ticket cache instance
KerberosTicketCache ticket_cache(_config);
KerberosTicketCache ticket_cache(_config, _temp_dir.string());

// Initialize the ticket cache
Status status = ticket_cache.initialize();
Expand Down
4 changes: 2 additions & 2 deletions be/test/common/kerberos/kerberos_ticket_cache_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@ class KerberosTicketCacheTest : public testing::Test {
// Create a temporary directory for testing
_test_dir = std::filesystem::temp_directory_path() / "kerberos_test";
std::filesystem::create_directories(_test_dir);
KerberosTicketCache::set_ccache_root_dir(_test_dir.string());

_config.set_principal_and_keytab("test_principal", "/path/to/keytab");
_config.set_krb5_conf_path("/etc/krb5.conf");
_config.set_refresh_interval(2);
_config.set_min_time_before_refresh(600);

_cache = std::make_unique<KerberosTicketCache>(_config, std::move(_mock_krb5));
_cache = std::make_unique<KerberosTicketCache>(_config, _test_dir.string(),
std::move(_mock_krb5));
_cache->set_refresh_thread_sleep_time(std::chrono::milliseconds(1));
}

Expand Down
17 changes: 10 additions & 7 deletions be/test/common/kerberos/kerberos_ticket_mgr_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ namespace doris::kerberos {
// Mock KerberosTicketCache class
class MockKerberosTicketCache : public KerberosTicketCache {
public:
explicit MockKerberosTicketCache(const KerberosConfig& config) : KerberosTicketCache(config) {}
explicit MockKerberosTicketCache(const KerberosConfig& config, const std::string& root_path)
: KerberosTicketCache(config, root_path) {}

MOCK_METHOD(Status, initialize, (), (override));
MOCK_METHOD(Status, login, (), (override));
Expand All @@ -42,27 +43,31 @@ class MockKerberosTicketCache : public KerberosTicketCache {

class MockKerberosTicketMgr : public KerberosTicketMgr {
public:
MockKerberosTicketMgr(int32_t cache_expire_seconds) : KerberosTicketMgr(cache_expire_seconds) {}
MockKerberosTicketMgr(int32_t cache_expire_seconds, const std::string& root_path)
: KerberosTicketMgr(cache_expire_seconds, root_path) {}

// by calling this, will first set _mock_cache_path in MockKerberosTicketMgr,
// and _mock_cache_path will pass to KerberosTicketCache to set cache's ticket_cache_path.
void set_mock_cache_path(const std::string& cache_path) { _mock_cache_path = cache_path; }
void set_should_succeed(bool val) { _should_succeed = val; }

protected:
std::unique_ptr<KerberosTicketCache> _make_new_ticket_cache(
const KerberosConfig& config) override {
auto mock_cache = std::make_unique<testing::StrictMock<MockKerberosTicketCache>>(config);
auto mock_cache =
std::make_unique<testing::StrictMock<MockKerberosTicketCache>>(config, _root_path);

if (_should_succeed) {
EXPECT_CALL(*mock_cache, initialize()).WillOnce(testing::Return(Status::OK()));
EXPECT_CALL(*mock_cache, login()).WillOnce(testing::Return(Status::OK()));
EXPECT_CALL(*mock_cache, write_ticket_cache()).WillOnce(testing::Return(Status::OK()));
EXPECT_CALL(*mock_cache, start_periodic_refresh()).Times(1);
mock_cache->set_ticket_cache_path(_mock_cache_path);
} else {
EXPECT_CALL(*mock_cache, initialize())
.WillOnce(testing::Return(Status::InternalError("Init failed")));
}

mock_cache->set_ticket_cache_path(_mock_cache_path);
return mock_cache;
}

Expand All @@ -77,9 +82,7 @@ class KerberosTicketMgrTest : public testing::Test {
// Create a temporary directory for testing
_test_dir = std::filesystem::temp_directory_path() / "kerberos_mgr_test";
std::filesystem::create_directories(_test_dir);
KerberosTicketCache::set_ccache_root_dir(_test_dir.string());

_mgr = std::make_unique<MockKerberosTicketMgr>(1);
_mgr = std::make_unique<MockKerberosTicketMgr>(1, _test_dir.string());
}

void TearDown() override {
Expand Down

0 comments on commit 972fb7e

Please sign in to comment.