Merge "Sanitize hostnames resolving on start" from Pavel E

"
On start scylla resolves several hostnames into addresses. Different
places use different hostname selection logic, e.g. the API address
can be the listen one if the dedicated option not set. Failure to
resolve a hostname is reported with an exception that (sometimes)
contains the hostname, but it doesn't look very convenient -- better
to know the config option name. Also resolving of different hostnames
has different decoration around, e.g. prometheus carries a main-local
lambda just to nicely wrap the try/catch block.

This set unifies this zoo and makes main() shorter and less hairy:

1. All failures to resolve a hostname are reported with an
   exception containing the relevant config option

2. The || operator for named_value's is introduced to make
   the option selection look as short as

     resolve(cfg->some_address() || cfg->another_address())

3. All sanity checks are explicit and happen early in main

4. No dangling local variables carrying the cfg->...() value

5. Use resolved IP when logging a "... is listening on ..."
   message after a service start

tests: unit(dev)
"

* 'br-ip-resolve-on-start' of https://github.com/xemul/scylla:
  main: Move fb-utilities initialization up the main
  code: Use utils::resolve instead of inet_address::lookup
  main: Remove unused variable
  main: Sanitize resolving of listen address
  main: Sanitize resolving of broadcast address
  main: Sanitize resolving of broadcast RPC address
  main: Sanitize resolving of API address
  main: Sanitize resolving of prometheus address
  utils: Introduce || operator for named_values
  db.config: Verbose address resolver helper
  main: Remove api-port and prometheus-port variables
  alternator: Resolve address with the help of inet_address
  redis, thrift: Remove unused captures

alternator/controller.cc

@@ -85,12 +85,7 @@ future<> controller::start_server() {
         rmw_operation::set_default_write_isolation(_config.alternator_write_isolation());
         executor::set_default_timeout(std::chrono::milliseconds(_config.alternator_timeout_in_ms()));
 
-        net::inet_address addr;
-        try {
-            addr = net::dns::get_host_by_name(_config.alternator_address(), family).get0().addr_list.front();
-        } catch (...) {
-            std::throw_with_nested(std::runtime_error(fmt::format("Unable to resolve alternator_address {}", _config.alternator_address())));
-        }
+        net::inet_address addr = utils::resolve(_config.alternator_address, family).get0();
 
         auto get_cdc_metadata = [] (cdc::generation_service& svc) { return std::ref(svc.get_cdc_metadata()); };
 

db/config.cc

@@ -1082,4 +1082,19 @@ future<> configure_tls_creds_builder(seastar::tls::credentials_builder& creds, d
     }
 }
 
+future<gms::inet_address> resolve(const config_file::named_value<sstring>& address, gms::inet_address::opt_family family, gms::inet_address::opt_family preferred) {
+    std::exception_ptr ex;
+    try {
+        co_return co_await gms::inet_address::lookup(address(), family, preferred);
+    } catch (...) {
+        try {
+            std::throw_with_nested(std::runtime_error(fmt::format("Couldn't resolve {}", address.name())));
+        } catch (...) {
+            ex = std::current_exception();
+        }
+    }
+
+    co_return coroutine::exception(std::move(ex));
+}
+
 }

db/config.hh

@@ -420,5 +420,5 @@ inline bool is_true(sstring val) {
 }
 
 future<> configure_tls_creds_builder(seastar::tls::credentials_builder& creds, db::config::string_map options);
-
+future<gms::inet_address> resolve(const config_file::named_value<sstring>&, gms::inet_address::opt_family family = {}, gms::inet_address::opt_family preferred = {});
 }

db/system_keyspace.cc

@@ -1240,7 +1240,7 @@ schema_ptr system_keyspace::legacy::aggregates() {
 }
 
 future<> system_keyspace::setup_version(distributed<gms::feature_service>& feat, sharded<netw::messaging_service>& ms, const db::config& cfg) {
-    return gms::inet_address::lookup(cfg.rpc_address()).then([&feat, &ms, &cfg](gms::inet_address a) {
+    return utils::resolve(cfg.rpc_address).then([&feat, &ms, &cfg](gms::inet_address a) {
         sstring req = fmt::format("INSERT INTO system.{} (key, release_version, cql_version, thrift_version, native_protocol_version, data_center, rack, partitioner, rpc_address, broadcast_address, listen_address, supported_features) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"
                         , db::system_keyspace::LOCAL);
         auto& snitch = locator::i_endpoint_snitch::get_local_snitch_ptr();

main.cc

@@ -607,73 +607,50 @@ int main(int ac, char** av) {
                 }).get();
             });
 
-            uint16_t api_port = cfg->api_port();
-            ctx.api_dir = cfg->api_ui_dir();
-            ctx.api_doc = cfg->api_doc_dir();
+            if (cfg->broadcast_address().empty() && cfg->listen_address().empty()) {
+                startlog.error("Bad configuration: neither listen_address nor broadcast_address are defined\n");
+                throw bad_configuration_error();
+            }
+
+            if (cfg->broadcast_rpc_address().empty() && cfg->rpc_address() == "0.0.0.0") {
+                startlog.error("If rpc_address is set to a wildcard address {}, then you must set broadcast_rpc_address", cfg->rpc_address());
+                throw bad_configuration_error();
+            }
+
             auto preferred = cfg->listen_interface_prefer_ipv6() ? std::make_optional(net::inet_address::family::INET6) : std::nullopt;
             auto family = cfg->enable_ipv6_dns_lookup() || preferred ? std::nullopt : std::make_optional(net::inet_address::family::INET);
-            sstring listen_address = cfg->listen_address();
-            sstring rpc_address = cfg->rpc_address();
-            sstring api_address = cfg->api_address() != "" ? cfg->api_address() : rpc_address;
-            sstring broadcast_address = cfg->broadcast_address();
-            sstring broadcast_rpc_address = cfg->broadcast_rpc_address();
+
+            auto broadcast_addr = utils::resolve(cfg->broadcast_address || cfg->listen_address, family, preferred).get0();
+            utils::fb_utilities::set_broadcast_address(broadcast_addr);
+            auto broadcast_rpc_addr = utils::resolve(cfg->broadcast_rpc_address || cfg->rpc_address, family, preferred).get0();
+            utils::fb_utilities::set_broadcast_rpc_address(broadcast_rpc_addr);
+
+            ctx.api_dir = cfg->api_ui_dir();
+            ctx.api_doc = cfg->api_doc_dir();
             const auto hinted_handoff_enabled = cfg->hinted_handoff_enabled();
-            auto prom_addr = [&] {
-                try {
-                    return gms::inet_address::lookup(cfg->prometheus_address(), family, preferred).get0();
-                } catch (...) {
-                    std::throw_with_nested(std::runtime_error(fmt::format("Unable to resolve prometheus_address {}", cfg->prometheus_address())));
-                }
-            }();
+
             supervisor::notify("starting prometheus API server");
-            uint16_t pport = cfg->prometheus_port();
             std::any stop_prometheus;
-            if (pport) {
+            if (cfg->prometheus_port()) {
                 prometheus_server.start("prometheus").get();
-                stop_prometheus = defer_verbose_shutdown("prometheus API server", [&prometheus_server, pport] {
+                stop_prometheus = defer_verbose_shutdown("prometheus API server", [&prometheus_server] {
                     prometheus_server.stop().get();
                 });
 
+                auto ip = utils::resolve(cfg->prometheus_address, family, preferred).get0();
+
                 //FIXME discarded future
                 prometheus::config pctx;
                 pctx.metric_help = "Scylla server statistics";
                 pctx.prefix = cfg->prometheus_prefix();
                 (void)prometheus::start(prometheus_server, pctx);
                 with_scheduling_group(maintenance_scheduling_group, [&] {
-                  return prometheus_server.listen(socket_address{prom_addr, pport}).handle_exception([pport, &cfg] (auto ep) {
-                    startlog.error("Could not start Prometheus API server on {}:{}: {}", cfg->prometheus_address(), pport, ep);
+                  return prometheus_server.listen(socket_address{ip, cfg->prometheus_port()}).handle_exception([&ip, &cfg] (auto ep) {
+                    startlog.error("Could not start Prometheus API server on {}:{}: {}", ip, cfg->prometheus_port(), ep);
                     return make_exception_future<>(ep);
                   });
                 }).get();
             }
-            if (!broadcast_address.empty()) {
-                try {
-                    utils::fb_utilities::set_broadcast_address(gms::inet_address::lookup(broadcast_address, family, preferred).get0());
-                } catch (...) {
-                    startlog.error("Bad configuration: invalid 'broadcast_address': {}: {}", broadcast_address, std::current_exception());
-                    throw bad_configuration_error();
-                }
-            } else if (!listen_address.empty()) {
-                try {
-                    utils::fb_utilities::set_broadcast_address(gms::inet_address::lookup(listen_address, family, preferred).get0());
-                } catch (...) {
-                    startlog.error("Bad configuration: invalid 'listen_address': {}: {}", listen_address, std::current_exception());
-                    throw bad_configuration_error();
-                }
-            } else {
-                startlog.error("Bad configuration: neither listen_address nor broadcast_address are defined\n");
-                throw bad_configuration_error();
-            }
-
-            if (!broadcast_rpc_address.empty()) {
-                utils::fb_utilities::set_broadcast_rpc_address(gms::inet_address::lookup(broadcast_rpc_address, family, preferred).get0());
-            } else {
-                if (rpc_address == "0.0.0.0") {
-                    startlog.error("If rpc_address is set to a wildcard address {}, then you must set broadcast_rpc_address to a value other than {}", rpc_address, rpc_address);
-                    throw bad_configuration_error();
-                }
-                utils::fb_utilities::set_broadcast_rpc_address(gms::inet_address::lookup(rpc_address, family, preferred).get0());
-            }
 
             using namespace locator;
             // Re-apply strict-dma after we've read the config file, this time
@@ -725,24 +702,18 @@ int main(int ac, char** av) {
             i_endpoint_snitch::create_snitch(cfg->endpoint_snitch()).get();
             // #293 - do not stop anything
             // engine().at_exit([] { return i_endpoint_snitch::stop_snitch(); });
-            supervisor::notify("determining DNS name");
-            auto ip = [&] {
-                try {
-                    return gms::inet_address::lookup(api_address, family, preferred).get0();
-                } catch (...) {
-                    std::throw_with_nested(std::runtime_error(fmt::format("Unable to resolve api_address {}", api_address)));
-                }
-            }();
+
+            auto api_addr = utils::resolve(cfg->api_address || cfg->rpc_address, family, preferred).get0();
             supervisor::notify("starting API server");
             ctx.http_server.start("API").get();
             auto stop_http_server = defer_verbose_shutdown("API server", [&ctx] {
                 ctx.http_server.stop().get();
             });
             api::set_server_init(ctx).get();
             with_scheduling_group(maintenance_scheduling_group, [&] {
-                return ctx.http_server.listen(socket_address{ip, api_port});
+                return ctx.http_server.listen(socket_address{api_addr, cfg->api_port()});
             }).get();
-            startlog.info("Scylla API server listening on {}:{} ...", api_address, api_port);
+            startlog.info("Scylla API server listening on {}:{} ...", api_addr, cfg->api_port());
 
             api::set_server_config(ctx, *cfg).get();
 
@@ -759,7 +730,7 @@ int main(int ac, char** av) {
 
             netw::messaging_service::config mscfg;
 
-            mscfg.ip = gms::inet_address::lookup(listen_address, family).get0();
+            mscfg.ip = utils::resolve(cfg->listen_address, family).get0();
             mscfg.port = cfg->storage_port();
             mscfg.ssl_port = cfg->ssl_storage_port();
             mscfg.listen_on_broadcast_address = cfg->listen_on_broadcast_address();

redis/service.cc

@@ -53,7 +53,6 @@ future<> redis_service::listen(seastar::sharded<auth::service>& auth_service, db
     auto server = make_shared<seastar::sharded<redis_transport::redis_server>>();
     _server = server;
 
-    auto addr = cfg.rpc_address();
     auto preferred = cfg.rpc_interface_prefer_ipv6() ? std::make_optional(net::inet_address::family::INET6) : std::nullopt;
     auto family = cfg.enable_ipv6_dns_lookup() || preferred ? std::nullopt : std::make_optional(net::inet_address::family::INET);
     auto ceo = cfg.client_encryption_options();
@@ -64,8 +63,8 @@ future<> redis_service::listen(seastar::sharded<auth::service>& auth_service, db
     redis_cfg._write_consistency_level = make_consistency_level(cfg.redis_write_consistency_level());
     redis_cfg._max_request_size = memory::stats().total_memory() / 10;
     redis_cfg._total_redis_db_count = cfg.redis_database_count();
-    return gms::inet_address::lookup(addr, family, preferred).then([this, server, addr, &cfg, keepalive, ceo = std::move(ceo), redis_cfg, &auth_service] (seastar::net::inet_address ip) {
-        return server->start(std::ref(_query_processor), std::ref(auth_service), redis_cfg).then([this, server, &cfg, addr, ip, ceo, keepalive]() {
+    return utils::resolve(cfg.rpc_address, family, preferred).then([this, server, &cfg, keepalive, ceo = std::move(ceo), redis_cfg, &auth_service] (seastar::net::inet_address ip) {
+        return server->start(std::ref(_query_processor), std::ref(auth_service), redis_cfg).then([this, server, &cfg, ip, ceo, keepalive]() {
             auto f = make_ready_future();
             struct listen_cfg {
                 socket_address addr;

thrift/controller.cc

@@ -74,25 +74,23 @@ future<> thrift_controller::do_start_server() {
     _addr.reset();
 
     auto& cfg = _db.local().get_config();
-    auto port = cfg.rpc_port();
-    auto addr = cfg.rpc_address();
     auto preferred = cfg.rpc_interface_prefer_ipv6() ? std::make_optional(net::inet_address::family::INET6) : std::nullopt;
     auto family = cfg.enable_ipv6_dns_lookup() || preferred ? std::nullopt : std::make_optional(net::inet_address::family::INET);
     auto keepalive = cfg.rpc_keepalive();
     thrift_server_config tsc;
     tsc.timeout_config = make_timeout_config(cfg);
     tsc.max_request_size = cfg.thrift_max_message_length_in_mb() * (uint64_t(1) << 20);
-    return gms::inet_address::lookup(addr, family, preferred).then([this, tserver, addr, port, keepalive, tsc] (gms::inet_address ip) {
+    return utils::resolve(cfg.rpc_address, family, preferred).then([this, tserver, port = cfg.rpc_port(), keepalive, tsc] (gms::inet_address ip) {
         _addr.emplace(ip, port);
-        return tserver->start(std::ref(_db), std::ref(_qp), std::ref(_ss), std::ref(_auth_service), std::ref(_mem_limiter), tsc).then([tserver, port, addr, ip, keepalive] {
+        return tserver->start(std::ref(_db), std::ref(_qp), std::ref(_ss), std::ref(_auth_service), std::ref(_mem_limiter), tsc).then([tserver, port, ip, keepalive] {
             // #293 - do not stop anything
             //engine().at_exit([tserver] {
             //    return tserver->stop();
             //});
             return tserver->invoke_on_all(&thrift_server::listen, socket_address{ip, port}, keepalive);
+        }).then([ip, port] {
+            clogger.info("Thrift server listening on {}:{} ...", ip, port);
         });
-    }).then([addr, port] {
-        clogger.info("Thrift server listening on {}:{} ...", addr, port);
     });
 }
 

transport/controller.cc

@@ -82,7 +82,6 @@ future<> controller::do_start_server() {
         auto cserver = std::make_unique<sharded<cql_server>>();
 
         auto& cfg = _config;
-        auto addr = cfg.rpc_address();
         auto preferred = cfg.rpc_interface_prefer_ipv6() ? std::make_optional(net::inet_address::family::INET6) : std::nullopt;
         auto family = cfg.enable_ipv6_dns_lookup() || preferred ? std::nullopt : std::make_optional(net::inet_address::family::INET);
         auto ceo = cfg.client_encryption_options();
@@ -104,7 +103,7 @@ future<> controller::do_start_server() {
         smp_service_group_config cql_server_smp_service_group_config;
         cql_server_smp_service_group_config.max_nonlocal_requests = 5000;
         cql_server_config.bounce_request_smp_service_group = create_smp_service_group(cql_server_smp_service_group_config).get0();
-        const seastar::net::inet_address ip = gms::inet_address::lookup(addr, family, preferred).get0();
+        const seastar::net::inet_address ip = utils::resolve(cfg.rpc_address, family, preferred).get0();
 
         struct listen_cfg {
             socket_address addr;

utils/config_file.hh

@@ -288,6 +288,14 @@ private:
         _cfgs;
 };
 
+template <typename T>
+requires requires (const config_file::named_value<T>& nv) {
+    { nv().empty() } -> std::same_as<bool>;
+}
+const config_file::named_value<T>& operator||(const config_file::named_value<T>& a, const config_file::named_value<T>& b) {
+    return !a().empty() ? a : b;
+}
+
 extern template struct config_file::named_value<seastar::log_level>;
 
 }