transport, redis: Do not assume fixed encryption options

On start main() brushes up the client_encryption_options option so that any user of it sees it in some "clean" state and can avoid using get_or_default() to parse. This patch removes this assumption (and the cleaning code itself). Next patch will make use of it and relax the duplicated parsing complexity back. Signed-off-by: Pavel Emelyanov <[email protected]>
StarostaGit · Aug 20, 2021 · 35209e7 · 35209e7
1 parent 2f5941c
commit 35209e7
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 21 deletions.
diff --git a/main.cc b/main.cc
@@ -687,21 +687,6 @@ int main(int ac, char** av) {
                 utils::fb_utilities::set_broadcast_rpc_address(gms::inet_address::lookup(rpc_address, family, preferred).get0());
             }
 
-            // The start_native_transport method is invoked by API as well, and uses the config object
-            // (through db) directly. Lets fixup default valued right here instead then, so it in turn can be
-            // kept simple
-            // TODO: make intrinsic part of config defaults instead
-            auto ceo = cfg->client_encryption_options();
-            if (utils::is_true(utils::get_or_default(ceo, "enabled", "false"))) {
-                ceo["enabled"] = "true";
-                ceo["certificate"] = utils::get_or_default(ceo, "certificate", db::config::get_conf_sub("scylla.crt").string());
-                ceo["keyfile"] = utils::get_or_default(ceo, "keyfile", db::config::get_conf_sub("scylla.key").string());
-                ceo["require_client_auth"] = utils::is_true(utils::get_or_default(ceo, "require_client_auth", "false")) ? "true" : "false";
-            } else {
-                ceo["enabled"] = "false";
-            }
-            cfg->client_encryption_options(std::move(ceo), cfg->client_encryption_options.source());
-
             using namespace locator;
             // Re-apply strict-dma after we've read the config file, this time
             // to all reactors

diff --git a/redis/service.cc b/redis/service.cc
@@ -72,7 +72,7 @@ future<> redis_service::listen(seastar::sharded<auth::service>& auth_service, db
             }
 
             // main should have made sure values are clean and neatish
-            if (ceo.at("enabled") == "true") {
+            if (utils::is_true(utils::get_or_default(ceo, "enabled", "false"))) {
                 auto cred = std::make_shared<seastar::tls::credentials_builder>();
 
                 cred->set_dh_level(seastar::tls::dh_params::level::MEDIUM);
@@ -81,11 +81,13 @@ future<> redis_service::listen(seastar::sharded<auth::service>& auth_service, db
                 if (ceo.contains("priority_string")) {
                     cred->set_priority_string(ceo.at("priority_string"));
                 }
-                if (ceo.contains("require_client_auth") && ceo.at("require_client_auth") == "true") {
+                if (utils::is_true(utils::get_or_default(ceo, "require_client_auth", "false"))) {
                     cred->set_client_auth(seastar::tls::client_auth::REQUIRE);
                 }
 
-                f = cred->set_x509_key_file(ceo.at("certificate"), ceo.at("keyfile"), seastar::tls::x509_crt_format::PEM);
+                auto cert = utils::get_or_default(ceo, "certificate", db::config::get_conf_sub("scylla.crt").string());
+                auto key = utils::get_or_default(ceo, "keyfile", db::config::get_conf_sub("scylla.key").string());
+                f = cred->set_x509_key_file(cert, key, seastar::tls::x509_crt_format::PEM);
 
                 if (ceo.contains("truststore")) {
                     f = f.then([cred, f = ceo.at("truststore")] { return cred->set_x509_trust_file(f, seastar::tls::x509_crt_format::PEM); });

diff --git a/transport/controller.cc b/transport/controller.cc
@@ -113,7 +113,7 @@ future<> controller::do_start_server() {
         }
 
         // main should have made sure values are clean and neatish
-        if (ceo.at("enabled") == "true") {
+        if (utils::is_true(utils::get_or_default(ceo, "enabled", "false"))) {
             auto cred = std::make_shared<seastar::tls::credentials_builder>();
 
             cred->set_dh_level(seastar::tls::dh_params::level::MEDIUM);
@@ -122,11 +122,13 @@ future<> controller::do_start_server() {
             if (ceo.contains("priority_string")) {
                 cred->set_priority_string(ceo.at("priority_string"));
             }
-            if (ceo.contains("require_client_auth") && ceo.at("require_client_auth") == "true") {
+            if (utils::is_true(utils::get_or_default(ceo, "require_client_auth", "false"))) {
                 cred->set_client_auth(seastar::tls::client_auth::REQUIRE);
             }
 
-            cred->set_x509_key_file(ceo.at("certificate"), ceo.at("keyfile"), seastar::tls::x509_crt_format::PEM).get();
+            auto cert = utils::get_or_default(ceo, "certificate", db::config::get_conf_sub("scylla.crt").string());
+            auto key = utils::get_or_default(ceo, "keyfile", db::config::get_conf_sub("scylla.key").string());
+            cred->set_x509_key_file(cert, key, seastar::tls::x509_crt_format::PEM).get();
 
             if (ceo.contains("truststore")) {
                 cred->set_x509_trust_file(ceo.at("truststore"), seastar::tls::x509_crt_format::PEM).get();