Merge pull request #1991 from jagerman/lokinet-vpn-cleanups

Lokinet vpn cleanups

daemon/lokinet-vpn.cpp

@@ -1,5 +1,6 @@
 #include <oxenmq/oxenmq.h>
 #include <nlohmann/json.hpp>
+#include <fmt/core.h>
 #include <cxxopts.hpp>
 #include <future>
 #include <vector>
@@ -16,11 +17,11 @@
 #include <sys/wait.h>
 #endif
 
-/// do a oxenmq request on an lmq instance blocking style
+/// do a oxenmq request on an omq instance blocking style
 /// returns a json object parsed from the result
 std::optional<nlohmann::json>
-LMQ_Request(
+OMQ_Request(
-    oxenmq::OxenMQ& lmq,
+    oxenmq::OxenMQ& omq,
     const oxenmq::ConnectionID& id,
     std::string_view method,
     std::optional<nlohmann::json> args = std::nullopt)
@@ -37,11 +38,11 @@ LMQ_Request(
   };
   if (args.has_value())
   {
-    lmq.request(id, method, handleRequest, args->dump());
+    omq.request(id, method, handleRequest, args->dump());
   }
   else
   {
-    lmq.request(id, method, handleRequest);
+    omq.request(id, method, handleRequest);
   }
   auto ftr = result_promise.get_future();
   const auto str = ftr.get();
@@ -50,6 +51,50 @@ LMQ_Request(
   return std::nullopt;
 }
 
+namespace
+{
+  template <typename T>
+  constexpr bool is_optional = false;
+  template <typename T>
+  constexpr bool is_optional<std::optional<T>> = true;
+
+  // Extracts a value from a cxxopts result and assigns it into `value` if present.  The value can
+  // either be a plain value or a std::optional.  If not present, `value` is not touched.
+  template <typename T>
+  void
+  extract_option(const cxxopts::ParseResult& r, const std::string& name, T& value)
+  {
+    if (r.count(name))
+    {
+      if constexpr (is_optional<T>)
+        value = r[name].as<typename T::value_type>();
+      else
+        value = r[name].as<T>();
+    }
+  }
+
+  // Takes a code, prints a message, and returns the code.  Intended use is:
+  //     return exit_error(1, "blah: {}", 42);
+  // from within main().
+  template <typename... T>
+  [[nodiscard]] int
+  exit_error(int code, const std::string& format, T&&... args)
+  {
+    fmt::print(format, std::forward<T>(args)...);
+    fmt::print("\n");
+    return code;
+  }
+
+  // Same as above, but with code omitted (uses exit code 1)
+  template <typename... T>
+  [[nodiscard]] int
+  exit_error(const std::string& format, T&&... args)
+  {
+    return exit_error(1, format, std::forward<T>(args)...);
+  }
+
+}  // namespace
+
 int
 main(int argc, char* argv[])
 {
@@ -74,8 +119,8 @@ main(int argc, char* argv[])
   oxenmq::address rpcURL("tcp://127.0.0.1:1190");
   std::string exitAddress;
   std::string endpoint = "default";
-  std::optional<std::string> token;
+  std::string token;
-  std::string range = "::/0";
+  std::optional<std::string> range;
   oxenmq::LogLevel logLevel = oxenmq::LogLevel::warn;
   bool goUp = false;
   bool goDown = false;
@@ -95,69 +140,48 @@ main(int argc, char* argv[])
     {
       logLevel = oxenmq::LogLevel::debug;
     }
-    if (result.count("rpc") > 0)
-    {
-      rpcURL = oxenmq::address(result["rpc"].as<std::string>());
-    }
-    if (result.count("exit") > 0)
-    {
-      exitAddress = result["exit"].as<std::string>();
-    }
     goUp = result.count("up") > 0;
     goDown = result.count("down") > 0;
     printStatus = result.count("status") > 0;
     killDaemon = result.count("kill") > 0;
 
-    if (result.count("endpoint") > 0)
+    extract_option(result, "rpc", rpcURL);
-    {
+    extract_option(result, "exit", exitAddress);
-      endpoint = result["endpoint"].as<std::string>();
+    extract_option(result, "endpoint", endpoint);
-    }
+    extract_option(result, "token", token);
-    if (result.count("token") > 0)
+    extract_option(result, "auth", token);
-    {
+    extract_option(result, "range", range);
-      token = result["token"].as<std::string>();
-    }
-    if (result.count("auth") > 0)
-    {
-      token = result["auth"].as<std::string>();
-    }
-    if (result.count("range") > 0)
-    {
-      range = result["range"].as<std::string>();
-    }
   }
   catch (const cxxopts::option_not_exists_exception& ex)
   {
-    std::cerr << ex.what();
+    return exit_error(2, "{}\n{}", ex.what(), opts.help());
-    std::cout << opts.help() << std::endl;
-    return 1;
   }
   catch (std::exception& ex)
   {
-    std::cout << ex.what() << std::endl;
+    return exit_error(2, "{}", ex.what());
-    return 1;
-  }
-  if ((not goUp) and (not goDown) and (not printStatus) and (not killDaemon))
-  {
-    std::cout << opts.help() << std::endl;
-    return 1;
   }
+
+  int num_commands = goUp + goDown + printStatus + killDaemon;
+
+  if (num_commands == 0)
+    return exit_error(3, "One of --up/--down/--status/--kill must be specified");
+  if (num_commands != 1)
+    return exit_error(3, "Only one of --up/--down/--status/--kill may be specified");
+
   if (goUp and exitAddress.empty())
-  {
+    return exit_error("no exit address provided");
-    std::cout << "no exit address provided" << std::endl;
-    return 1;
-  }
 
-  oxenmq::OxenMQ lmq{
+  oxenmq::OxenMQ omq{
       [](oxenmq::LogLevel lvl, const char* file, int line, std::string msg) {
         std::cout << lvl << " [" << file << ":" << line << "] " << msg << std::endl;
       },
       logLevel};
 
-  lmq.start();
+  omq.start();
 
   std::promise<bool> connectPromise;
 
-  const auto connID = lmq.connect_remote(
+  const auto connID = omq.connect_remote(
       rpcURL,
       [&connectPromise](auto) { connectPromise.set_value(true); },
       [&connectPromise](auto, std::string_view msg) {
@@ -173,23 +197,16 @@ main(int argc, char* argv[])
 
   if (killDaemon)
   {
-    const auto maybe = LMQ_Request(lmq, connID, "llarp.halt");
+    if (not OMQ_Request(omq, connID, "llarp.halt"))
-    if (not maybe.has_value())
+      return exit_error("call to llarp.halt failed");
-    {
-      std::cout << "call to llarp.admin.die failed" << std::endl;
-      return 1;
-    }
     return 0;
   }
 
   if (printStatus)
   {
-    const auto maybe_status = LMQ_Request(lmq, connID, "llarp.status");
+    const auto maybe_status = OMQ_Request(omq, connID, "llarp.status");
-    if (not maybe_status.has_value())
+    if (not maybe_status)
-    {
+      return exit_error("call to llarp.status failed");
-      std::cout << "call to llarp.status failed" << std::endl;
-      return 1;
-    }
 
     try
     {
@@ -209,43 +226,33 @@ main(int argc, char* argv[])
     }
     catch (std::exception& ex)
     {
-      std::cout << "failed to parse result: " << ex.what() << std::endl;
+      return exit_error("failed to parse result: {}", ex.what());
-      return 1;
     }
     return 0;
   }
   if (goUp)
   {
-    std::optional<nlohmann::json> maybe_result;
+    nlohmann::json opts{{"exit", exitAddress}, {"token", token}};
-    if (token.has_value())
+    if (range)
-    {
+      opts["range"] = *range;
-      maybe_result = LMQ_Request(
-          lmq,
-          connID,
-          "llarp.exit",
-          nlohmann::json{{"exit", exitAddress}, {"range", range}, {"token", *token}});
-    }
-    else
-    {
-      maybe_result = LMQ_Request(
-          lmq, connID, "llarp.exit", nlohmann::json{{"exit", exitAddress}, {"range", range}});
-    }
 
-    if (not maybe_result.has_value())
+    auto maybe_result = OMQ_Request(omq, connID, "llarp.exit", std::move(opts));
-    {
+
-      std::cout << "could not add exit" << std::endl;
+    if (not maybe_result)
-      return 1;
+      return exit_error("could not add exit");
-    }
 
-    if (maybe_result->contains("error") and maybe_result->at("error").is_string())
+    if (auto err_it = maybe_result->find("error"); err_it != maybe_result->end())
     {
-      std::cout << maybe_result->at("error").get<std::string>() << std::endl;
+      return exit_error("{}", err_it->get<std::string_view>());
-      return 1;
     }
   }
   if (goDown)
   {
-    LMQ_Request(lmq, connID, "llarp.exit", nlohmann::json{{"range", range}, {"unmap", true}});
+    nlohmann::json opts{{"unmap", true}};
+    if (range)
+      opts["range"] = *range;
+    if (not OMQ_Request(omq, connID, "llarp.exit", std::move(opts)))
+      return exit_error("failed to unmap exit");
   }
 
   return 0;

llarp/nodedb.cpp

@@ -20,6 +20,8 @@ static const std::string RC_FILE_EXT = ".signed";
 
 namespace llarp
 {
+  static auto logcat = log::Cat("nodedb");
+
   NodeDB::Entry::Entry(RouterContact value) : rc(std::move(value)), insertedAt(llarp::time_now_ms())
   {}
 
@@ -159,7 +161,7 @@ namespace llarp
 
     if (not purge.empty())
     {
-      LogWarn("removing {} invalid RC from disk", purge.size());
+      log::warning(logcat, "removing {} invalid RCs from disk", purge.size());
 
       for (const auto& fpath : purge)
         fs::remove(fpath);

llarp/rpc/rpc_server.cpp

@@ -477,7 +477,7 @@ namespace llarp::rpc
                   map = false;
                 }
                 const auto range_itr = obj.find("range");
-                if (range_itr == obj.end())
+                if (range_itr == obj.end() or range_itr->is_null())
                 {
                   // platforms without ipv6 support will shit themselves
                   // here if we give them an exit mapping that is ipv6
@@ -500,9 +500,9 @@ namespace llarp::rpc
                   reply(CreateJSONError("ipv6 ranges not supported on this platform"));
                   return;
                 }
-                std::optional<std::string> token;
+                std::string token;
                 const auto token_itr = obj.find("token");
-                if (token_itr != obj.end())
+                if (token_itr != obj.end() and not token_itr->is_null())
                 {
                   token = token_itr->get<std::string>();
                 }
@@ -526,10 +526,10 @@ namespace llarp::rpc
                       ep->MapExitRange(range, addr);
 
                       bool shouldSendAuth = false;
-                      if (token.has_value())
+                      if (not token.empty())
                       {
                         shouldSendAuth = true;
-                        ep->SetAuthInfoForEndpoint(*exit, service::AuthInfo{*token});
+                        ep->SetAuthInfoForEndpoint(*exit, service::AuthInfo{token});
                       }
                       auto onGoodResult = [r, reply](std::string reason) {
                         if (r->HasClientExit())