3617ecb · typeable/nixpkgs

git.iko.soy/typeable/nixpkgs

nixos/switch-to-configuration: Fix backslashes in unit names

systemd needs this so special characters (like the ones in wireguard units that appear because they are part of base64) can be escaped using the \x syntax. Root of the issue is that `glob()` handles the backslash internally which is obviously not what we want here. Also add a test case and fix some perlcritic issues in the subroutine.

commit3617ecb67f447f50f79ccd9b1bdaa5e151749ec7 authorJanne Heß <janne@hess.ooo> dateThu 17 Feb 2022, 12:46 (5 years ago) parentb516e55

2 files changed+51−10

compare with master

unifiedsplit

nixos/modules/system/activation/switch-to-configuration.pl+15−10 nixos/tests/switch-test.nix+36

nixos/modules/system/activation/switch-to-configuration.pl+15−10

@@ -173,13 +173,18 @@ sub parseSystemdIni {

173173#

174174# If a directory with the same basename ending in .d exists next to the unit file, it will be

175175# assumed to contain override files which will be parsed as well and handled properly.

176−sub parseUnit {

177− my ($unitPath) = @_;

176+sub parse_unit {

177+ my ($unit_path) = @_;

178178

179179 # Parse the main unit and all overrides

180− my %unitData;

181− parseSystemdIni(\%unitData, $_) for glob("${unitPath}{,.d/*.conf}");

182− return %unitData;

180+ my %unit_data;

181+ # Replace \ with \\ so glob() still works with units that have a \ in them

182+ # Valid characters in unit names are ASCII letters, digits, ":", "-", "_", ".", and "\"

183+ $unit_path =~ s/\\/\\\\/gmsx;

184+ foreach (glob "${unit_path}{,.d/*.conf}") {

185+ parseSystemdIni(\%unit_data, "$_")

186+ }

187+ return %unit_data;

183188}

184189

185190# Checks whether a specified boolean in a systemd unit is true

@@ -310,7 +315,7 @@ sub handleModifiedUnit {

310315 # Revert of the attempt: https://github.com/NixOS/nixpkgs/pull/147609

311316 # More details: https://github.com/NixOS/nixpkgs/issues/74899#issuecomment-981142430

312317 } else {

313− my %unitInfo = $newUnitInfo ? %{$newUnitInfo} : parseUnit($newUnitFile);

318+ my %unitInfo = $newUnitInfo ? %{$newUnitInfo} : parse_unit($newUnitFile);

314319 if (parseSystemdBool(\%unitInfo, "Service", "X-ReloadIfChanged", 0) and not $unitsToRestart->{$unit} and not $unitsToStop->{$unit}) {

315320 $unitsToReload->{$unit} = 1;

316321 recordUnit($reloadListFile, $unit);

@@ -412,12 +417,12 @@ while (my ($unit, $state) = each %{$activePrev}) {

412417

413418 if (-e $prevUnitFile && ($state->{state} eq "active" || $state->{state} eq "activating")) {

414419 if (! -e $newUnitFile || abs_path($newUnitFile) eq "/dev/null") {

415− my %unitInfo = parseUnit($prevUnitFile);

420+ my %unitInfo = parse_unit($prevUnitFile);

416421 $unitsToStop{$unit} = 1 if parseSystemdBool(\%unitInfo, "Unit", "X-StopOnRemoval", 1);

417422 }

418423

419424 elsif ($unit =~ /\.target$/) {

420− my %unitInfo = parseUnit($newUnitFile);

425+ my %unitInfo = parse_unit($newUnitFile);

421426

422427 # Cause all active target units to be restarted below.

423428 # This should start most changed units we stop here as

@@ -451,8 +456,8 @@ while (my ($unit, $state) = each %{$activePrev}) {

451456 }

452457

453458 else {

454− my %old_unit_info = parseUnit($prevUnitFile);

455− my %new_unit_info = parseUnit($newUnitFile);

459+ my %old_unit_info = parse_unit($prevUnitFile);

460+ my %new_unit_info = parse_unit($newUnitFile);

456461 my $diff = compare_units(\%old_unit_info, \%new_unit_info);

457462 if ($diff eq 1) {

458463 handleModifiedUnit($unit, $baseName, $newUnitFile, \%new_unit_info, $activePrev, \%unitsToStop, \%unitsToStart, \%unitsToReload, \%unitsToRestart, \%unitsToSkip);

nixos/tests/switch-test.nix+36

@@ -145,6 +145,23 @@ import ./make-test-python.nix ({ pkgs, ...} : {

145145 systemd.services.test.serviceConfig."X-Test" = "test";

146146 };

147147

148+ unitWithBackslash.configuration = {

149+ systemd.services."escaped\\x2ddash" = {

150+ wantedBy = [ "multi-user.target" ];

151+ serviceConfig = {

152+ Type = "oneshot";

153+ RemainAfterExit = true;

154+ ExecStart = "${pkgs.coreutils}/bin/true";

155+ ExecReload = "${pkgs.coreutils}/bin/true";

156+ };

157+ };

158+ };

159+

160+ unitWithBackslashModified.configuration = {

161+ imports = [ unitWithBackslash.configuration ];

162+ systemd.services."escaped\\x2ddash".serviceConfig.X-Test = "test";

163+ };

164+

148165 restart-and-reload-by-activation-script.configuration = {

149166 systemd.services = rec {

150167 simple-service = {

@@ -433,6 +450,25 @@ import ./make-test-python.nix ({ pkgs, ...} : {

433450 assert_lacks(out, "as well:")

434451 assert_contains(out, "would start the following units: test.service\n")

435452

453+ # Ensure \ works in unit names

454+ out = switch_to_specialisation("${machine}", "unitWithBackslash")

455+ assert_contains(out, "stopping the following units: test.service\n")

456+ assert_lacks(out, "NOT restarting the following changed units:")

457+ assert_lacks(out, "reloading the following units:")

458+ assert_lacks(out, "\nrestarting the following units:")

459+ assert_lacks(out, "\nstarting the following units:")

460+ assert_contains(out, "the following new units were started: escaped\\x2ddash.service\n")

461+ assert_lacks(out, "as well:")

462+

463+ out = switch_to_specialisation("${machine}", "unitWithBackslashModified")

464+ assert_contains(out, "stopping the following units: escaped\\x2ddash.service\n")

465+ assert_lacks(out, "NOT restarting the following changed units:")

466+ assert_lacks(out, "reloading the following units:")

467+ assert_lacks(out, "\nrestarting the following units:")

468+ assert_contains(out, "\nstarting the following units: escaped\\x2ddash.service\n")

469+ assert_lacks(out, "the following new units were started:")

470+ assert_lacks(out, "as well:")

471+

436472 with subtest("failing units"):

437473 # Let the simple service fail

438474 switch_to_specialisation("${machine}", "simpleServiceModified")