From 62a91659e38828309dc00b32cec55f52d42de0d9 Mon Sep 17 00:00:00 2001 From: Gareth Sylvester-Bradley <31761158+garethsb@users.noreply.github.com> Date: Tue, 20 Feb 2024 15:35:02 +0000 Subject: [PATCH] Discovery mode revisited (#374) * Remove commented out code * Replace discovery_mode with sneakily stashing Host header in the web::uri --- Development/nmos-cpp-node/config.json | 3 - Development/nmos/authorization_operation.cpp | 2 +- Development/nmos/authorization_state.h | 2 +- Development/nmos/client_utils.cpp | 20 +++++ Development/nmos/client_utils.h | 6 ++ Development/nmos/mdns.cpp | 84 +++++--------------- Development/nmos/node_behaviour.cpp | 14 ++-- Development/nmos/node_system_behaviour.cpp | 2 +- Development/nmos/ocsp_behaviour.cpp | 2 +- Development/nmos/settings.h | 3 - 10 files changed, 60 insertions(+), 78 deletions(-) diff --git a/Development/nmos-cpp-node/config.json b/Development/nmos-cpp-node/config.json index c3367a9d4..95b3362ec 100644 --- a/Development/nmos-cpp-node/config.json +++ b/Development/nmos-cpp-node/config.json @@ -235,9 +235,6 @@ // proxy_port [registry, node]: forward proxy port //"proxy_port": 8080, - // discovery_mode [node]: whether the discovered host name (1) or resolved addresses (2) are used to construct request URLs for Registration APIs or System APIs - //"discovery_mode": 1, - // href_mode [registry, node]: whether the host name (1), addresses (2) or both (3) are used to construct response headers, and host and URL fields in the data model //"href_mode": 1, diff --git a/Development/nmos/authorization_operation.cpp b/Development/nmos/authorization_operation.cpp index aab25170c..338c73d77 100644 --- a/Development/nmos/authorization_operation.cpp +++ b/Development/nmos/authorization_operation.cpp @@ -1200,7 +1200,7 @@ namespace nmos const auto service = top_authorization_service(model.settings); const auto auth_uri = service.second; - client.reset(new web::http::client::http_client(auth_uri, make_authorization_http_client_config(model.settings, load_ca_certificates, gate))); + client = nmos::details::make_http_client(auth_uri, make_authorization_http_client_config(model.settings, load_ca_certificates, gate)); auto token = cancellation_source.get_token(); diff --git a/Development/nmos/authorization_state.h b/Development/nmos/authorization_state.h index 2bd57db4d..74f704ad4 100644 --- a/Development/nmos/authorization_state.h +++ b/Development/nmos/authorization_state.h @@ -28,7 +28,7 @@ namespace nmos , immediate(true) , received(false) {} - pubkeys_shared_state(web::http::client::http_client client, nmos::api_version version, web::uri issuer)//, bool one_shot = false) + pubkeys_shared_state(web::http::client::http_client client, nmos::api_version version, web::uri issuer) : engine(seeder) , client(std::unique_ptr(new web::http::client::http_client(client))) , version(std::move(version)) diff --git a/Development/nmos/client_utils.cpp b/Development/nmos/client_utils.cpp index 393c094c9..bb3282bbd 100644 --- a/Development/nmos/client_utils.cpp +++ b/Development/nmos/client_utils.cpp @@ -256,6 +256,26 @@ namespace nmos return config; } + namespace details + { + // make a client for the specified base_uri and config, with Host header sneakily stashed in user info + std::unique_ptr make_http_client(const web::uri& base_uri, const web::http::client::http_client_config& client_config) + { + // unstash the Host header + // cf. nmos::details::resolve_service + const auto& host_name = base_uri.user_info(); + std::unique_ptr client(new web::http::client::http_client(web::uri_builder(base_uri).set_user_info({}).to_uri(), client_config)); + if (!host_name.empty()) + { + client->add_handler([host_name](web::http::http_request request, std::shared_ptr next_stage) -> pplx::task + { + request.headers().add(web::http::header_names::host, host_name); + return next_stage->propagate(request); + }); + } + return client; + } + } // make a request with logging pplx::task api_request(web::http::client::http_client client, web::http::http_request request, slog::base_gate& gate, const pplx::cancellation_token& token) diff --git a/Development/nmos/client_utils.h b/Development/nmos/client_utils.h index 284846c8a..d702f4bd3 100644 --- a/Development/nmos/client_utils.h +++ b/Development/nmos/client_utils.h @@ -31,6 +31,12 @@ namespace nmos web::websockets::client::websocket_client_config make_websocket_client_config(const nmos::settings& settings, load_ca_certificates_handler load_ca_certificates, nmos::experimental::get_authorization_bearer_token_handler get_authorization_bearer_token, slog::base_gate& gate); web::websockets::client::websocket_client_config make_websocket_client_config(const nmos::settings& settings, load_ca_certificates_handler load_ca_certificates, slog::base_gate& gate); + namespace details + { + // make a client for the specified base_uri and config, with Host header sneakily stashed in user info + std::unique_ptr make_http_client(const web::uri& base_uri_with_host_name_in_user_info, const web::http::client::http_client_config& client_config); + } + // make an API request with logging pplx::task api_request(web::http::client::http_client client, web::http::http_request request, slog::base_gate& gate, const pplx::cancellation_token& token = pplx::cancellation_token::none()); pplx::task api_request(web::http::client::http_client client, const web::http::method& mtd, slog::base_gate& gate, const pplx::cancellation_token& token = pplx::cancellation_token::none()); diff --git a/Development/nmos/mdns.cpp b/Development/nmos/mdns.cpp index ab4b25177..91e4549c3 100644 --- a/Development/nmos/mdns.cpp +++ b/Development/nmos/mdns.cpp @@ -495,44 +495,11 @@ namespace nmos update_service(advertiser, service, domain, settings, std::move(add_records)); } - enum discovery_mode - { - discovery_mode_default = 0, - discovery_mode_name = 1, - discovery_mode_addresses = 2 - }; - namespace details { typedef std::vector resolved_services; - std::vector get_resolved_hosts(const mdns::resolve_result& resolved, const nmos::service_protocol& resolved_proto, discovery_mode mode) - { - std::vector results; - - // by default, use the host name if secure communications are in use - if (mode == discovery_mode_name || (mode == discovery_mode_default && is_service_protocol_secure(resolved_proto))) - { - auto host_name = utility::s2us(resolved.host_name); - // remove a trailing '.' to turn an FQDN into a DNS name, for SSL certificate matching - // hmm, this might be more appropriately done by tweaking the Host header in the client request? - if (!host_name.empty() && U('.') == host_name.back()) host_name.pop_back(); - - results.push_back(host_name); - } - - if (mode == discovery_mode_addresses || (mode == discovery_mode_default && !is_service_protocol_secure(resolved_proto))) - { - for (const auto& ip_address : resolved.ip_addresses) - { - results.push_back(utility::s2us(ip_address)); - } - } - - return results; - } - - pplx::task resolve_service(std::shared_ptr results, mdns::service_discovery& discovery, discovery_mode discovery_mode, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, const std::chrono::steady_clock::time_point& timeout, const pplx::cancellation_token& token) + pplx::task resolve_service(std::shared_ptr results, mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, const std::chrono::steady_clock::time_point& timeout, const pplx::cancellation_token& token) { return discovery.browse([=, &discovery](const mdns::browse_result& resolving) { @@ -588,12 +555,17 @@ namespace nmos .set_path(U("/x-nmos/") + utility::s2us(details::service_api(service))); } - auto resolved_hosts = get_resolved_hosts(resolved, resolved_proto, discovery_mode); + auto host_name = utility::s2us(resolved.host_name); + // remove a trailing '.' to turn an FQDN into a DNS name, for SSL certificate matching + if (!host_name.empty() && U('.') == host_name.back()) host_name.pop_back(); - for (const auto& host : resolved_hosts) + for (const auto& ip_address : resolved.ip_addresses) { + // sneakily stash the Host header in user info + // cf. nmos::details::make_http_client results->push_back({ { *resolved_ver, resolved_pri }, resolved_uri - .set_host(host) + .set_user_info(host_name) + .set_host(utility::s2us(ip_address)) .to_uri() }); } @@ -618,7 +590,9 @@ namespace nmos } } - pplx::task> resolve_service_(mdns::service_discovery& discovery, discovery_mode mode, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) + // helper function for resolving instances of the specified service (API) + // with the highest version, highest priority instances at the front, and optionally services with the same priority ordered randomly + pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) { const auto absolute_timeout = std::chrono::steady_clock::now() + timeout; @@ -646,8 +620,8 @@ namespace nmos }; const std::vector> both_tasks{ - details::resolve_service(both_results[0], discovery, mode, nmos::service_types::register_, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, linked_token), - details::resolve_service(both_results[1], discovery, mode, service, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, linked_token) + details::resolve_service(both_results[0], discovery, nmos::service_types::register_, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, linked_token), + details::resolve_service(both_results[1], discovery, service, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, linked_token) }; // when either task is completed, cancel and wait for the other to be completed @@ -675,12 +649,12 @@ namespace nmos } else { - resolve_task = details::resolve_service(results, discovery, mode, nmos::service_types::register_, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, token); + resolve_task = details::resolve_service(results, discovery, nmos::service_types::register_, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, token); } } else { - resolve_task = details::resolve_service(results, discovery, mode, service, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, token); + resolve_task = details::resolve_service(results, discovery, service, browse_domain, api_ver, priorities, api_proto, api_auth, absolute_timeout, token); } return resolve_task.then([results, randomize](bool) @@ -722,9 +696,11 @@ namespace nmos }); } - pplx::task> resolve_service(mdns::service_discovery& discovery, discovery_mode mode, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) + // helper function for resolving instances of the specified service (API) + // with the highest version, highest priority instances at the front, and optionally services with the same priority ordered randomly + pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) { - return resolve_service_(discovery, mode, service, browse_domain, api_ver, priorities, api_proto, api_auth, randomize, timeout, token).then([](std::list resolved_services) + return resolve_service_(discovery, service, browse_domain, api_ver, priorities, api_proto, api_auth, randomize, timeout, token).then([](std::list resolved_services) { // add the version to each uri return boost::copy_range>(resolved_services | boost::adaptors::transformed([](const resolved_service& s) @@ -734,18 +710,10 @@ namespace nmos }); } - // helper function for resolving instances of the specified service (API) - // with the highest version, highest priority instances at the front, and (by default) services with the same priority ordered randomly - pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) - { - return resolve_service(discovery, discovery_mode_default, service, browse_domain, api_ver, priorities, api_proto, api_auth, randomize, timeout, token); - } - // helper function for resolving instances of the specified service (API) based on the specified settings // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly pplx::task> resolve_service(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { - const auto mode = discovery_mode(nmos::experimental::fields::discovery_mode(settings)); const auto browse_domain = utility::us2s(nmos::get_domain(settings)); const auto versions = details::service_versions(service, settings); const auto priorities = details::service_priorities(service, settings); @@ -756,21 +724,13 @@ namespace nmos // when no cancellation token is specified const auto timeout = token.is_cancelable() ? nmos::fields::discovery_backoff_max(settings) : 1; - return resolve_service(discovery, mode, service, browse_domain, versions, priorities, protocols, authorization, true, std::chrono::seconds(timeout), token); - } - - // helper function for resolving instances of the specified service (API) - // with the highest version, highest priority instances at the front, and (by default) services with the same priority ordered randomly - pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const std::string& browse_domain, const std::set& api_ver, const std::pair& priorities, const std::set& api_proto, const std::set& api_auth, bool randomize, const std::chrono::steady_clock::duration& timeout, const pplx::cancellation_token& token) - { - return resolve_service_(discovery, discovery_mode_default, service, browse_domain, api_ver, priorities, api_proto, api_auth, randomize, timeout, token); + return resolve_service(discovery, service, browse_domain, versions, priorities, protocols, authorization, true, std::chrono::duration_cast(std::chrono::seconds(timeout)), token); } // helper function for resolving instances of the specified service (API) based on the specified settings // with the highest version, highest priority instances at the front, and services with the same priority ordered randomly pplx::task> resolve_service_(mdns::service_discovery& discovery, const nmos::service_type& service, const nmos::settings& settings, const pplx::cancellation_token& token) { - const auto mode = discovery_mode(nmos::experimental::fields::discovery_mode(settings)); const auto browse_domain = utility::us2s(nmos::get_domain(settings)); const auto versions = details::service_versions(service, settings); const auto priorities = details::service_priorities(service, settings); @@ -781,7 +741,7 @@ namespace nmos // when no cancellation token is specified const auto timeout = token.is_cancelable() ? nmos::fields::discovery_backoff_max(settings) : 1; - return resolve_service_(discovery, mode, service, browse_domain, versions, priorities, protocols, authorization, true, std::chrono::seconds(timeout), token); + return resolve_service_(discovery, service, browse_domain, versions, priorities, protocols, authorization, true, std::chrono::duration_cast(std::chrono::seconds(timeout)), token); } } } diff --git a/Development/nmos/node_behaviour.cpp b/Development/nmos/node_behaviour.cpp index 6b0c7a26b..85ba40e0d 100644 --- a/Development/nmos/node_behaviour.cpp +++ b/Development/nmos/node_behaviour.cpp @@ -760,7 +760,8 @@ namespace nmos grain.updated = strictly_increasing_update(resources); }); - registration_client.reset(new web::http::client::http_client(base_uri, make_registration_client_config(model.settings, load_ca_certificates, get_authorization_bearer_token ? get_authorization_bearer_token() : web::http::oauth2::experimental::oauth2_token{}, gate))); + const auto bearer_token = get_authorization_bearer_token ? get_authorization_bearer_token() : web::http::oauth2::experimental::oauth2_token{}; + registration_client = nmos::details::make_http_client(base_uri, make_registration_client_config(model.settings, load_ca_certificates, bearer_token, gate)); } events = web::json::value::array(); @@ -901,8 +902,8 @@ namespace nmos if (registry_version != grain->version) break; const auto bearer_token = get_authorization_bearer_token ? get_authorization_bearer_token() : web::http::oauth2::experimental::oauth2_token{}; - registration_client.reset(new web::http::client::http_client(base_uri, make_registration_client_config(model.settings, load_ca_certificates, bearer_token, gate))); - heartbeat_client.reset(new web::http::client::http_client(base_uri, make_heartbeat_client_config(model.settings, load_ca_certificates, bearer_token, gate))); + registration_client = nmos::details::make_http_client(base_uri, make_registration_client_config(model.settings, load_ca_certificates, bearer_token, gate)); + heartbeat_client = nmos::details::make_http_client(base_uri, make_heartbeat_client_config(model.settings, load_ca_certificates, bearer_token, gate)); // "The first interaction with a new Registration API [after a server side or connectivity issue] // should be a heartbeat to confirm whether whether the Node is still present in the registry" @@ -946,6 +947,7 @@ namespace nmos { heartbeat_time = std::chrono::steady_clock::now(); + // renew heartbeat_client if bearer token has changed if (get_authorization_bearer_token) { const auto& bearer_token = get_authorization_bearer_token(); @@ -954,7 +956,7 @@ namespace nmos slog::log(gate, SLOG_FLF) << "Update heartbeat client with new authorization token"; heartbeat_bearer_token = bearer_token; - heartbeat_client.reset(new web::http::client::http_client(base_uri, make_heartbeat_client_config(model.settings, load_ca_certificates, bearer_token, gate))); + heartbeat_client = nmos::details::make_http_client(base_uri, make_heartbeat_client_config(model.settings, load_ca_certificates, bearer_token, gate)); } } @@ -1003,7 +1005,7 @@ namespace nmos auto token = cancellation_source.get_token(); - // renew regsitration_client if bearer token has changed + // renew registration_client if bearer token has changed if (get_authorization_bearer_token) { const auto& bearer_token = get_authorization_bearer_token(); @@ -1012,7 +1014,7 @@ namespace nmos slog::log(gate, SLOG_FLF) << "Update registration client with new authorization token"; registration_bearer_token = bearer_token; - registration_client.reset(new web::http::client::http_client(registration_client->base_uri(), make_registration_client_config(model.settings, load_ca_certificates, bearer_token, gate))); + registration_client = nmos::details::make_http_client(registration_client->base_uri(), make_registration_client_config(model.settings, load_ca_certificates, bearer_token, gate)); } } diff --git a/Development/nmos/node_system_behaviour.cpp b/Development/nmos/node_system_behaviour.cpp index 2e215824b..bc29d0da5 100644 --- a/Development/nmos/node_system_behaviour.cpp +++ b/Development/nmos/node_system_behaviour.cpp @@ -396,7 +396,7 @@ namespace nmos if (!state.client) { const auto base_uri = top_system_service(model.settings); - state.client.reset(new web::http::client::http_client(base_uri, make_system_client_config(model.settings, load_ca_certificates, gate))); + state.client = nmos::details::make_http_client(base_uri, make_system_client_config(model.settings, load_ca_certificates, gate)); } auto token = cancellation_source.get_token(); diff --git a/Development/nmos/ocsp_behaviour.cpp b/Development/nmos/ocsp_behaviour.cpp index f16140be6..a835d5937 100644 --- a/Development/nmos/ocsp_behaviour.cpp +++ b/Development/nmos/ocsp_behaviour.cpp @@ -347,7 +347,7 @@ namespace nmos { const auto ocsp_uri = ocsp_uris.front(); const auto secure = web::is_secure_uri_scheme(ocsp_uri.scheme()); - state.client.reset(new web::http::client::http_client(ocsp_uri, make_ocsp_client_config(secure, model.settings, state.load_ca_certificates, gate))); + state.client = nmos::details::make_http_client(ocsp_uri, make_ocsp_client_config(secure, model.settings, state.load_ca_certificates, gate)); } auto token = cancellation_source.get_token(); diff --git a/Development/nmos/settings.h b/Development/nmos/settings.h index b046a0293..300ee7295 100644 --- a/Development/nmos/settings.h +++ b/Development/nmos/settings.h @@ -294,9 +294,6 @@ namespace nmos // proxy_port [registry, node]: forward proxy port const web::json::field_as_integer_or proxy_port{ U("proxy_port"), 8080 }; - // discovery_mode [node]: whether the discovered host name (1) or resolved addresses (2) are used to construct request URLs for Registration APIs or System APIs - const web::json::field_as_integer_or discovery_mode{ U("discovery_mode"), 0 }; // when omitted, a default heuristic is used - // href_mode [registry, node]: whether the host name (1), addresses (2) or both (3) are used to construct response headers, and host and URL fields in the data model const web::json::field_as_integer_or href_mode{ U("href_mode"), 0 }; // when omitted, a default heuristic is used