mirrors/scylladb
1
0
Fork 0
mirror of https://github.com/scylladb/scylladb.git synced 2026-05-13 03:12:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge '[Backport 2025.1] encryption_at_rest_test: Fix some spurious errors' from Scylladb[bot]

Browse Source 
Fixes #24574

* Ensure we close the embedded load_cache objects on encryption shutdown, otherwise we can, in unit testing, get destruction of these while a timer is still active -> assert
* Add extra exception handling to `network_error_test_helper`, so even if test framework might exception-escape, we properly stop the network proxy to avoid use after free.

- (cherry picked from commit ee98f5d361)

- (cherry picked from commit 8d37e5e24b)

Parent PR: #24633

Closes scylladb/scylladb#24767

* github.com:scylladb/scylladb:
  encryption_at_rest_test: Add exception handler to ensure proxy stop
  encryption: Ensure stopping timers in provider cache objects
This commit is contained in:
Botond Dénes
2026-05-08 06:36:42 +03:00
parent 52605e70b7 ac072fe4f9
commit f16a7aab4f
7 changed files with 75 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
ent/encryption/encryption.cc
View File

@@ -475,6 +475,14 @@ public:
for (auto&& [id, h] : _per_thread_kmip_host_cache[this_shard_id()]) {
co_await h->disconnect();
}
static auto stop_all = [](auto&& cache) -> future<> {
for (auto& [k, host] : cache) {
co_await host->stop();
}
};
co_await stop_all(_per_thread_kms_host_cache[this_shard_id()]);
co_await stop_all(_per_thread_gcp_host_cache[this_shard_id()]);
_per_thread_provider_cache[this_shard_id()].clear();
_per_thread_system_key_cache[this_shard_id()].clear();
_per_thread_kmip_host_cache[this_shard_id()].clear();

10
ent/encryption/gcp_host.cc
View File

@@ -102,6 +102,7 @@ public:
~impl() = default;
future<> init();
future<> stop();
const host_options& options() const {
return _options;
}
@@ -874,6 +875,11 @@ future<> encryption::gcp_host::impl::init() {
_initialized = true;
}
future<> encryption::gcp_host::impl::stop() {
co_await _attr_cache.stop();
co_await _id_cache.stop();
}
std::tuple<std::string, std::string> encryption::gcp_host::impl::parse_key(std::string_view spec) {
auto i = spec.find_last_of('/');
if (i == std::string_view::npos) {
@@ -1036,6 +1042,10 @@ future<> encryption::gcp_host::init() {
return _impl->init();
}
future<> encryption::gcp_host::stop() {
return _impl->stop();
}
const encryption::gcp_host::host_options& encryption::gcp_host::options() const {
return _impl->options();
}

2
ent/encryption/gcp_host.hh
View File

@@ -65,6 +65,8 @@ public:
~gcp_host();
future<> init();
future<> stop();
const host_options& options() const;
struct option_override : public t_credentials_source<std::optional<std::string>> {

4
ent/encryption/kmip_host.cc
View File

@@ -724,9 +724,11 @@ future<> kmip_host::impl::connect() {
}
future<> kmip_host::impl::disconnect() {
return do_for_each(_options.hosts, [this](const sstring& host) {
co_await do_for_each(_options.hosts, [this](const sstring& host) {
return clear_connections(host);
});
co_await _attr_cache.stop();
co_await _id_cache.stop();
}
static unsigned from_str(unsigned (*f)(char*, int, int*), const sstring& s, const sstring& what) {

11
ent/encryption/kms_host.cc
View File

@@ -160,6 +160,8 @@ public:
~impl() = default;
future<> init();
future<> stop();
const host_options& options() const {
return _options;
}
@@ -992,6 +994,11 @@ future<> encryption::kms_host::impl::init() {
_initialized = true;
}
future<> encryption::kms_host::impl::stop() {
co_await _attr_cache.stop();
co_await _id_cache.stop();
}
future<encryption::kms_host::impl::key_and_id_type> encryption::kms_host::impl::create_key(const attr_cache_key& k) {
auto& master_key = k.master_key;
auto& aws_assume_role_arn = k.aws_assume_role_arn;
@@ -1154,6 +1161,10 @@ future<> encryption::kms_host::init() {
return _impl->init();
}
future<> encryption::kms_host::stop() {
return _impl->stop();
}
const encryption::kms_host::host_options& encryption::kms_host::options() const {
return _impl->options();
}

2
ent/encryption/kms_host.hh
View File

@@ -63,6 +63,8 @@ public:
~kms_host();
future<> init();
future<> stop();
const host_options& options() const;
struct option_override {

70
test/boost/encryption_at_rest_test.cc
View File

@@ -1078,41 +1078,49 @@ static future<> test_broken_encrypted_commitlog(const test_provider_args& args,
*/
static future<> network_error_test_helper(const tmpdir& tmp, const std::string& host, std::function<std::tuple<scopts_map, std::string>(const fake_proxy&)> make_opts) {
fake_proxy proxy(host);
std::exception_ptr p;
try {
auto [scopts, yaml] = make_opts(proxy);
auto [scopts, yaml] = make_opts(proxy);
test_provider_args args{
.tmp = tmp,
.extra_yaml = yaml,
.n_tables = 10,
.before_create_table = [&](auto& env) {
// turn off proxy. all key resolution after this should fail
proxy.enable(false);
// wait for key cache expiry.
seastar::sleep(10ms).get();
// ensure commitlog will create a new segment on write -> eventual write failure
env.db().invoke_on_all([](replica::database& db) {
return db.commitlog()->force_new_active_segment();
}).get();
},
.on_insert_exception = [&](auto&&) {
// once we get the exception we have to enable key resolution again,
// otherwise we can't shut down cql test env.
proxy.enable(true);
},
.timeout = timeout_config{
// set really low write timeouts so we get a failure (timeout)
// when we fail to write to commitlog
100ms, 100ms, 100ms, 100ms, 100ms, 100ms, 100ms
},
};
test_provider_args args{
.tmp = tmp,
.extra_yaml = yaml,
.n_tables = 10,
.before_create_table = [&](auto& env) {
// turn off proxy. all key resolution after this should fail
proxy.enable(false);
// wait for key cache expiry.
seastar::sleep(10ms).get();
// ensure commitlog will create a new segment on write -> eventual write failure
env.db().invoke_on_all([](replica::database& db) {
return db.commitlog()->force_new_active_segment();
}).get();
},
.on_insert_exception = [&](auto&&) {
// once we get the exception we have to enable key resolution again,
// otherwise we can't shut down cql test env.
proxy.enable(true);
},
.timeout = timeout_config{
// set really low write timeouts so we get a failure (timeout)
// when we fail to write to commitlog
100ms, 100ms, 100ms, 100ms, 100ms, 100ms, 100ms
},
};
BOOST_REQUIRE_THROW(
co_await test_broken_encrypted_commitlog(args, scopts);
, std::exception
);
BOOST_REQUIRE_THROW(
co_await test_broken_encrypted_commitlog(args, scopts);
, exceptions::mutation_write_timeout_exception
);
} catch (...) {
p = std::current_exception();
}
co_await proxy.stop();
if (p) {
std::rethrow_exception(p);
}
}
SEASTAR_TEST_CASE(test_kms_network_error, *check_run_test_decorator("ENABLE_KMS_TEST")) {