Split config.yaml into global+persistent files; add -c/--config-file
config.yaml now holds only the global section (log-level, unshare-*, with-veth, with-ipv6); a new persistent.yaml holds volumes/networks. load_config_file()/write_config_file() are replaced by load_global_config()/load_persistent_config()/write_global_config()/ write_persistent_config(), each touching only their own file. -c/--config-file <path> lets one invocation use an alternate file for the global section only -- persistent.yaml is always the one fixed path, regardless of -c, so an experiment can never affect real volumes/networks (a -c file's own volumes/networks, if any, are simply never read either). A -c path that doesn't exist is a hard error, unlike the default path's existing missing-file leniency. migrate_legacy_config_if_needed() moves volumes/networks out of an old-format config.yaml into persistent.yaml on first run after upgrading, always against the fixed default paths regardless of -c. A name collision aborts the migration for that run (touching neither file) rather than risking data loss. Required reordering main() to parse CLI args before loading config (so -c's value is known first) -- ParsedArgs::log_level_flag_given tracks whether --log-level was already given so the config file's own log-level doesn't clobber it despite the reversed call order.
This commit is contained in:
@@ -23,6 +23,7 @@
|
||||
// actually accessed.
|
||||
|
||||
#include <algorithm>
|
||||
#include <fstream>
|
||||
#include <string>
|
||||
#include <vector>
|
||||
|
||||
@@ -47,9 +48,9 @@ TEST_CASE("config file -> NamespaceConfig -> bwrap argv: disabled namespaces are
|
||||
AppConfig written;
|
||||
written.unshare_net = false;
|
||||
written.unshare_uts = false;
|
||||
REQUIRE(write_config_file(config_path, written));
|
||||
REQUIRE(write_global_config(config_path, written));
|
||||
|
||||
auto loaded = load_config_file(config_path);
|
||||
auto loaded = load_global_config(config_path);
|
||||
REQUIRE(loaded.has_value());
|
||||
CHECK(loaded->unshare_net == std::optional<bool>(false));
|
||||
CHECK(loaded->unshare_uts == std::optional<bool>(false));
|
||||
@@ -79,10 +80,10 @@ TEST_CASE("config file -> NamespaceConfig -> bwrap argv: default (unset) config
|
||||
ScratchXdgDirs scratch;
|
||||
auto config_path = scratch.path() / "config.yaml";
|
||||
|
||||
// Nothing set -- write_config_file()/load_config_file() round-trip an
|
||||
// otherwise-empty AppConfig, so every unshare-* key comes back unset.
|
||||
REQUIRE(write_config_file(config_path, AppConfig{}));
|
||||
auto loaded = load_config_file(config_path);
|
||||
// Nothing set -- write_global_config()/load_global_config() round-trip
|
||||
// an otherwise-empty AppConfig, so every unshare-* key comes back unset.
|
||||
REQUIRE(write_global_config(config_path, AppConfig{}));
|
||||
auto loaded = load_global_config(config_path);
|
||||
REQUIRE(loaded.has_value());
|
||||
CHECK_FALSE(loaded->unshare_net.has_value());
|
||||
|
||||
@@ -110,9 +111,9 @@ TEST_CASE("config file -> AppConfig: global.with-veth/with-ipv6 round-trip", "[i
|
||||
AppConfig written;
|
||||
written.with_veth = false;
|
||||
written.with_ipv6 = false;
|
||||
REQUIRE(write_config_file(config_path, written));
|
||||
REQUIRE(write_global_config(config_path, written));
|
||||
|
||||
auto loaded = load_config_file(config_path);
|
||||
auto loaded = load_global_config(config_path);
|
||||
REQUIRE(loaded.has_value());
|
||||
CHECK(loaded->with_veth == std::optional<bool>(false));
|
||||
CHECK(loaded->with_ipv6 == std::optional<bool>(false));
|
||||
@@ -120,8 +121,123 @@ TEST_CASE("config file -> AppConfig: global.with-veth/with-ipv6 round-trip", "[i
|
||||
// Same "unset means enabled" convention as the six unshare-* keys --
|
||||
// create_network_command()'s own resolution (commands.cpp) is
|
||||
// `args.network_with_*_flag.value_or(config.with_*.value_or(true))`.
|
||||
auto loaded_empty = load_config_file(scratch.path() / "nonexistent.yaml");
|
||||
auto loaded_empty = load_global_config(scratch.path() / "nonexistent.yaml");
|
||||
REQUIRE(loaded_empty.has_value());
|
||||
CHECK_FALSE(loaded_empty->with_veth.has_value());
|
||||
CHECK_FALSE(loaded_empty->with_ipv6.has_value());
|
||||
}
|
||||
|
||||
TEST_CASE("persistent file -> AppConfig: volumes/networks round-trip, global section ignored",
|
||||
"[integration]") {
|
||||
ScratchXdgDirs scratch;
|
||||
auto persistent_path = scratch.path() / "persistent.yaml";
|
||||
|
||||
AppConfig written;
|
||||
written.log_level = "debug"; // global-only field -- must never reach persistent.yaml
|
||||
written.volumes.push_back({"myvol", "/home/user/myvol"});
|
||||
written.networks.push_back({"mynet", NetworkKind::extern_, "10.168.0.0/24", true, "fdf0::/64", true});
|
||||
REQUIRE(write_persistent_config(persistent_path, written));
|
||||
|
||||
auto loaded = load_persistent_config(persistent_path);
|
||||
REQUIRE(loaded.has_value());
|
||||
REQUIRE(loaded->volumes.size() == 1);
|
||||
CHECK(loaded->volumes[0].name == "myvol");
|
||||
CHECK(loaded->volumes[0].directory == "/home/user/myvol");
|
||||
REQUIRE(loaded->networks.size() == 1);
|
||||
CHECK(loaded->networks[0].name == "mynet");
|
||||
CHECK(loaded->networks[0].subnet == "10.168.0.0/24");
|
||||
// write_persistent_config() never writes a "global" mapping at all, so
|
||||
// there's nothing for load_persistent_config() to (deliberately) ignore
|
||||
// here -- confirmed via load_global_config() against the same file
|
||||
// instead, below.
|
||||
CHECK_FALSE(loaded->log_level.has_value());
|
||||
|
||||
auto global_view = load_global_config(persistent_path);
|
||||
REQUIRE(global_view.has_value());
|
||||
CHECK_FALSE(global_view->log_level.has_value());
|
||||
}
|
||||
|
||||
TEST_CASE("migrate_legacy_config_if_needed: moves volumes/networks out of an old-format config.yaml",
|
||||
"[integration]") {
|
||||
ScratchXdgDirs scratch;
|
||||
|
||||
// Hand-write an old-format single-file config.yaml -- global + volumes +
|
||||
// networks all combined, exactly the pre-split shape -- directly at the
|
||||
// real default config_file_path() (ScratchXdgDirs already points
|
||||
// XDG_CONFIG_HOME here for this test's lifetime, so this doesn't touch
|
||||
// the real developer's own config).
|
||||
auto config_path = config_file_path();
|
||||
std::filesystem::create_directories(config_path.parent_path());
|
||||
{
|
||||
std::ofstream legacy(config_path);
|
||||
legacy << "global:\n"
|
||||
" log-level: debug\n"
|
||||
"volumes:\n"
|
||||
" myvol: /home/user/myvol\n"
|
||||
"networks:\n"
|
||||
" mynet:\n"
|
||||
" kind: extern\n"
|
||||
" subnet: 10.168.0.0/24\n"
|
||||
" ipv6: true\n"
|
||||
" subnet6: fdf0::/64\n"
|
||||
" veth: true\n";
|
||||
}
|
||||
|
||||
REQUIRE(migrate_legacy_config_if_needed());
|
||||
|
||||
auto persistent = load_persistent_config(persistent_file_path());
|
||||
REQUIRE(persistent.has_value());
|
||||
REQUIRE(persistent->volumes.size() == 1);
|
||||
CHECK(persistent->volumes[0].name == "myvol");
|
||||
REQUIRE(persistent->networks.size() == 1);
|
||||
CHECK(persistent->networks[0].name == "mynet");
|
||||
|
||||
// config.yaml itself is rewritten global-only -- the legacy
|
||||
// volumes/networks are gone from it, but the global section survives.
|
||||
auto remaining_global = load_global_config(config_path);
|
||||
REQUIRE(remaining_global.has_value());
|
||||
CHECK(remaining_global->log_level == std::optional<std::string>("debug"));
|
||||
auto remaining_persistent_view = load_persistent_config(config_path);
|
||||
REQUIRE(remaining_persistent_view.has_value());
|
||||
CHECK(remaining_persistent_view->volumes.empty());
|
||||
CHECK(remaining_persistent_view->networks.empty());
|
||||
|
||||
// A second run is a clean no-op -- nothing left to migrate.
|
||||
REQUIRE(migrate_legacy_config_if_needed());
|
||||
auto persistent_again = load_persistent_config(persistent_file_path());
|
||||
REQUIRE(persistent_again.has_value());
|
||||
CHECK(persistent_again->volumes.size() == 1);
|
||||
CHECK(persistent_again->networks.size() == 1);
|
||||
}
|
||||
|
||||
TEST_CASE("migrate_legacy_config_if_needed: a name collision aborts the migration, touching neither file",
|
||||
"[integration]") {
|
||||
ScratchXdgDirs scratch;
|
||||
|
||||
AppConfig existing_persistent;
|
||||
existing_persistent.volumes.push_back({"myvol", "/already/here"});
|
||||
REQUIRE(write_persistent_config(persistent_file_path(), existing_persistent));
|
||||
|
||||
AppConfig legacy;
|
||||
legacy.volumes.push_back({"myvol", "/legacy/path"});
|
||||
REQUIRE(write_global_config(config_file_path(), AppConfig{})); // an existing global section
|
||||
// write_persistent_config() targets persistent_file_path(), not
|
||||
// config_file_path() -- to plant "legacy" volumes/networks directly in
|
||||
// config.yaml the way an old-format file would have them, write there
|
||||
// explicitly instead.
|
||||
REQUIRE(write_persistent_config(config_file_path(), legacy));
|
||||
|
||||
REQUIRE(migrate_legacy_config_if_needed()); // false only on a genuine I/O error -- a collision just warns
|
||||
|
||||
// Neither file was touched: the legacy copy is still in config.yaml,
|
||||
// and persistent.yaml's own pre-existing entry is unchanged.
|
||||
auto still_legacy = load_persistent_config(config_file_path());
|
||||
REQUIRE(still_legacy.has_value());
|
||||
REQUIRE(still_legacy->volumes.size() == 1);
|
||||
CHECK(still_legacy->volumes[0].directory == "/legacy/path");
|
||||
|
||||
auto still_persistent = load_persistent_config(persistent_file_path());
|
||||
REQUIRE(still_persistent.has_value());
|
||||
REQUIRE(still_persistent->volumes.size() == 1);
|
||||
CHECK(still_persistent->volumes[0].directory == "/already/here");
|
||||
}
|
||||
|
||||
@@ -223,3 +223,29 @@ TEST_CASE("parse_args: --kill requires a numeric pid", "[unit]") {
|
||||
REQUIRE(bad.exit_code.has_value());
|
||||
CHECK(*bad.exit_code == 1);
|
||||
}
|
||||
|
||||
TEST_CASE("parse_args: -c/--config-file populates config_file_flag, unset by default", "[unit]") {
|
||||
auto without = run_parse({"--list-volumes"});
|
||||
REQUIRE_FALSE(without.exit_code.has_value());
|
||||
CHECK_FALSE(without.args.config_file_flag.has_value());
|
||||
|
||||
auto with_short = run_parse({"-c", "/tmp/alt.yaml", "--list-volumes"});
|
||||
REQUIRE_FALSE(with_short.exit_code.has_value());
|
||||
REQUIRE(with_short.args.config_file_flag.has_value());
|
||||
CHECK(*with_short.args.config_file_flag == "/tmp/alt.yaml");
|
||||
|
||||
auto with_long = run_parse({"--config-file", "/tmp/alt2.yaml", "-w"});
|
||||
REQUIRE_FALSE(with_long.exit_code.has_value());
|
||||
REQUIRE(with_long.args.config_file_flag.has_value());
|
||||
CHECK(*with_long.args.config_file_flag == "/tmp/alt2.yaml");
|
||||
}
|
||||
|
||||
TEST_CASE("parse_args: --log-level sets log_level_flag_given", "[unit]") {
|
||||
auto without = run_parse({"--list-volumes"});
|
||||
REQUIRE_FALSE(without.exit_code.has_value());
|
||||
CHECK_FALSE(without.args.log_level_flag_given);
|
||||
|
||||
auto with_level = run_parse({"--log-level", "debug", "--list-volumes"});
|
||||
REQUIRE_FALSE(with_level.exit_code.has_value());
|
||||
CHECK(with_level.args.log_level_flag_given);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user