Merge branch 'config-arg': make the services honour --config

Fixes packaged .deb installs silently running on built-in defaults because the
systemd units pass --config but the services only ever read argv[1].

CMakeLists.txt

@@ -20,6 +20,7 @@ add_library(smartbotic_common STATIC
     lib/common/error.cpp
     lib/common/string_utils.cpp
     lib/common/config_defaults.cpp
+    lib/common/config_arg.cpp
 )
 target_include_directories(smartbotic_common PUBLIC
     ${CMAKE_CURRENT_SOURCE_DIR}/lib

lib/common/config_arg.cpp

@@ -0,0 +1,61 @@
+#include "config_arg.hpp"
+
+namespace smartbotic::common {
+
+namespace {
+
+// Returns the value part of "--config=/etc/x.json" style arguments, or an empty
+// string when the argument does not carry one.
+std::string valueAfterEquals(const std::string& argument, const std::string& flag) {
+    const std::string prefix = flag + "=";
+    if (argument.rfind(prefix, 0) == 0) {
+        return argument.substr(prefix.size());
+    }
+    return {};
+}
+
+}  // namespace
+
+ConfigArg parseConfigArg(int argc, char* argv[], const std::filesystem::path& default_path) {
+    ConfigArg result;
+    result.path = default_path;
+    result.explicitly_given = false;
+
+    for (int i = 1; i < argc; ++i) {
+        const std::string argument = argv[i];
+
+        if (argument == "--config" || argument == "-c") {
+            // The value is the next argument. Without one this is a malformed
+            // command line, so leave the default in place rather than treating
+            // the flag itself as a path.
+            if (i + 1 < argc) {
+                result.path = argv[i + 1];
+                result.explicitly_given = true;
+                ++i;
+            }
+            continue;
+        }
+
+        const std::string equals_value =
+            !valueAfterEquals(argument, "--config").empty()
+                ? valueAfterEquals(argument, "--config")
+                : valueAfterEquals(argument, "-c");
+        if (!equals_value.empty()) {
+            result.path = equals_value;
+            result.explicitly_given = true;
+            continue;
+        }
+
+        // A bare first argument is the original calling convention and stays
+        // supported. Anything beginning with '-' is some other flag and is not
+        // a path.
+        if (i == 1 && !argument.empty() && argument[0] != '-') {
+            result.path = argument;
+            result.explicitly_given = true;
+        }
+    }
+
+    return result;
+}
+
+}  // namespace smartbotic::common

lib/common/config_arg.hpp

@@ -0,0 +1,42 @@
+#pragma once
+
+#include <filesystem>
+#include <string>
+
+namespace smartbotic::common {
+
+struct ConfigArg {
+    std::filesystem::path path;
+
+    /**
+     * True when the caller named a config file on the command line, false when
+     * the built-in default path is being used.
+     *
+     * The distinction decides what a missing file means. Falling back to
+     * defaults is reasonable when nobody asked for a particular file; it is
+     * never reasonable when someone did, because the settings they meant to
+     * apply - database address, project namespace, ports - would be silently
+     * replaced by other values.
+     */
+    bool explicitly_given = false;
+};
+
+/**
+ * Read the config file location from a service's command line.
+ *
+ * Accepts, in order of preference:
+ *   --config <path>     --config=<path>
+ *   -c <path>           -c=<path>
+ *   <path>              (bare first argument, the original form)
+ *
+ * The flag forms exist because the packaged systemd units have always passed
+ * "--config /etc/.../webserver.json", while the services only ever read
+ * argv[1] as a path. That mismatch made every .deb install run on built-in
+ * defaults with nothing but a warning to say so.
+ *
+ * Unknown arguments are ignored rather than rejected, so this stays additive
+ * for anything else a deployment may already be passing.
+ */
+ConfigArg parseConfigArg(int argc, char* argv[], const std::filesystem::path& default_path);
+
+}  // namespace smartbotic::common

src/runner/main.cpp

@@ -3,6 +3,7 @@
 #include "runner_service.hpp"
 #include "logging/logger.hpp"
 #include "config/config_loader.hpp"
+#include "common/config_arg.hpp"
 
 using namespace smartbotic;
 
@@ -23,15 +24,18 @@ int main(int argc, char* argv[]) {
     LOG_INFO("SmartBotic Runner starting...");
 
     // Load configuration
-    std::filesystem::path config_path = "./config/runner.json";
-    if (argc > 1) {
-        config_path = argv[1];
-    }
+    const auto config_arg = common::parseConfigArg(argc, argv, "./config/runner.json");
+    const std::filesystem::path config_path = config_arg.path;
 
     runner::RunnerServiceConfig config;
     if (std::filesystem::exists(config_path)) {
         config = runner::RunnerService::loadConfig(config_path);
         LOG_INFO("Loaded configuration from {}", config_path.string());
+    } else if (config_arg.explicitly_given) {
+        // See the webserver's main for why this is fatal rather than a warning.
+        LOG_ERROR("Config file not found at {}. It was given on the command line, "
+                  "so refusing to start on built-in defaults", config_path.string());
+        return 1;
     } else {
         LOG_WARN("Config file not found at {}, using defaults", config_path.string());
     }

src/webserver/main.cpp

@@ -3,6 +3,7 @@
 #include "webserver_service.hpp"
 #include "logging/logger.hpp"
 #include "config/config_loader.hpp"
+#include "common/config_arg.hpp"
 
 using namespace smartbotic;
 
@@ -23,15 +24,21 @@ int main(int argc, char* argv[]) {
     LOG_INFO("SmartBotic WebServer starting...");
 
     // Load configuration
-    std::filesystem::path config_path = "./config/webserver.json";
-    if (argc > 1) {
-        config_path = argv[1];
-    }
+    const auto config_arg = common::parseConfigArg(argc, argv, "./config/webserver.json");
+    const std::filesystem::path config_path = config_arg.path;
 
     webserver::WebServerServiceConfig config;
     if (std::filesystem::exists(config_path)) {
         config = webserver::WebServerService::loadConfig(config_path);
         LOG_INFO("Loaded configuration from {}", config_path.string());
+    } else if (config_arg.explicitly_given) {
+        // Someone named this file, so starting on defaults would run the
+        // service against a different database, project namespace and port
+        // than they asked for - and the only sign would be a warning nobody
+        // reads. Refuse to start instead.
+        LOG_ERROR("Config file not found at {}. It was given on the command line, "
+                  "so refusing to start on built-in defaults", config_path.string());
+        return 1;
     } else {
         LOG_WARN("Config file not found at {}, using defaults", config_path.string());
     }