Merge 'vector_search: fix TLS server name with IP' from Karol Nowacki

SNI works only with DNS hostnames. Adding an IP address causes warnings
on the server side.
This change adds SNI only if it is not an IP address.

This change has no unit tests, as this behavior is not critical,
since it causes a warning on the server side.
The critical part, that the server name is verified, is already covered.

This PR also adds warning logs to improve future troubleshooting of connections to the vector-store nodes.

Fixes: VECTOR-528

Backports to 2025.04 and 2026.01 are required, as these branches are also affected.

Closes scylladb/scylladb#28637

* github.com:scylladb/scylladb:
  vector_search: fix TLS server name with IP
  vector_search: add warn log for failed ann requests
This commit is contained in:
Piotr Dulikowski
2026-03-09 15:03:22 +01:00
3 changed files with 43 additions and 5 deletions

View File

@@ -1102,7 +1102,7 @@ SEASTAR_TEST_CASE(vector_store_client_https_wrong_hostname) {
}));
}
SEASTAR_TEST_CASE(vector_store_client_https_different_ca_cert_verification_error) {
SEASTAR_TEST_CASE(vector_store_client_https_wrong_cacert_verification_error) {
auto broken_cert = co_await seastar::make_tmp_file();
certificates certs;
auto server = co_await make_vs_mock_server(co_await make_server_credentials(certs));
@@ -1129,6 +1129,33 @@ SEASTAR_TEST_CASE(vector_store_client_https_different_ca_cert_verification_error
}));
}
SEASTAR_TEST_CASE(vector_store_client_https_wrong_cacert_verification_error_host_is_ip) {
auto broken_cert = co_await seastar::make_tmp_file();
certificates certs;
auto server = co_await make_vs_mock_server(co_await make_server_credentials(certs));
auto cfg = make_config();
cfg.db_config->vector_store_primary_uri.set(format("https://{}:{}", server->host(), server->port()));
cfg.db_config->vector_store_encryption_options.set({{"truststore", broken_cert.get_path().string()}});
co_await do_with_cql_env(
[&](cql_test_env& env) -> future<> {
auto as = abort_source_timeout();
auto schema = co_await create_test_table(env, "ks", "idx");
auto& vs = env.local_qp().vector_store_client();
configure(vs).with_dns({{server->host(), std::vector<std::string>{server->host()}}});
vs.start_background_tasks();
auto keys = co_await vs.ann("ks", "idx", schema, std::vector<float>{0.1, 0.2, 0.3}, 2, rjson::empty_object(), as.reset());
BOOST_REQUIRE(!keys);
BOOST_CHECK(std::holds_alternative<vector_store_client::service_unavailable>(keys.error()));
},
cfg)
.finally(seastar::coroutine::lambda([&] -> future<> {
co_await server->stop();
co_await remove(broken_cert);
}));
}
SEASTAR_TEST_CASE(vector_store_client_high_availability_unreachable) {
auto server = co_await make_vs_mock_server();
auto unreachable = co_await make_unreachable_socket();

View File

@@ -21,6 +21,7 @@
#include <chrono>
#include <fmt/format.h>
#include <netinet/tcp.h>
#include <seastar/net/inet_address.hh>
using namespace seastar;
using namespace std::chrono_literals;
@@ -28,6 +29,10 @@ using namespace std::chrono_literals;
namespace vector_search {
namespace {
bool is_ip_address(const sstring& host) {
return net::inet_address::parse_numerical(host).has_value();
}
class client_connection_factory : public http::experimental::connection_factory {
client::endpoint_type _endpoint;
shared_ptr<tls::certificate_credentials> _creds;
@@ -55,7 +60,11 @@ private:
future<connected_socket> connect() {
auto addr = socket_address(_endpoint.ip, _endpoint.port);
if (_creds) {
auto socket = co_await tls::connect(_creds, addr, tls::tls_options{.server_name = _endpoint.host});
tls::tls_options opts;
if (!is_ip_address(_endpoint.host)) {
opts.server_name = _endpoint.host;
}
auto socket = co_await tls::connect(_creds, addr, std::move(opts));
// tls::connect() only performs the TCP handshake — the TLS handshake is deferred until the first I/O operation.
// Force the TLS handshake to happen here so that the connection timeout applies to it.
co_await tls::check_session_is_resumed(socket);
@@ -124,7 +133,7 @@ seastar::future<client::request_result> client::request(
co_return std::unexpected{aborted_error{}};
}
if (is_server_problem(err)) {
handle_server_unavailable();
handle_server_unavailable(err);
}
co_return std::unexpected{co_await map_err(err)};
}
@@ -165,8 +174,9 @@ seastar::future<> client::close() {
co_await _http_client.close();
}
void client::handle_server_unavailable() {
void client::handle_server_unavailable(std::exception_ptr err) {
if (!is_checking_status_in_progress()) {
_logger.warn("Request to vector store {} {}:{} failed: {}", _endpoint.host, _endpoint.ip, _endpoint.port, err);
_checking_status_future = run_checking_status();
}
}

View File

@@ -12,6 +12,7 @@
#include "utils/log.hh"
#include "utils/updateable_value.hh"
#include <chrono>
#include <exception>
#include <seastar/core/future.hh>
#include <seastar/core/sstring.hh>
#include <seastar/core/abort_source.hh>
@@ -60,7 +61,7 @@ private:
seastar::future<response> request_impl(seastar::httpd::operation_type method, seastar::sstring path, std::optional<seastar::sstring> content,
std::optional<seastar::http::reply::status_type>&& expected, seastar::abort_source& as);
seastar::future<bool> check_status();
void handle_server_unavailable();
void handle_server_unavailable(std::exception_ptr err);
seastar::future<> run_checking_status();
bool is_checking_status_in_progress() const;
std::chrono::milliseconds backoff_retry_max() const;