nixos/switchable-system: improve switch inhibitors (#477800)

3 files changed, 117 insertions, 73 deletions

diff --git a/nixos/modules/services/system/dbus.nix b/nixos/modules/services/system/dbus.nix
index 3751fa5e57fd..1e21fe20d714 100644
--- a/nixos/modules/services/system/dbus.nix
+++ b/nixos/modules/services/system/dbus.nix
@@ -104,6 +104,8 @@ in
 
   config = mkIf cfg.enable (mkMerge [
     {
+      system.switch.inhibitors.dbus-implementation = cfg.implementation;
+
       environment.etc."dbus-1".source = configDir;
 
       environment.pathsToLink = [
diff --git a/nixos/modules/system/activation/switchable-system.nix b/nixos/modules/system/activation/switchable-system.nix
index 0b840c978fb8..f6aeb70d7aa4 100644
--- a/nixos/modules/system/activation/switchable-system.nix
+++ b/nixos/modules/system/activation/switchable-system.nix
@@ -32,12 +32,12 @@
     };
 
     inhibitors = lib.mkOption {
-      type = lib.types.listOf lib.types.pathInStore;
-      default = [ ];
+      type = lib.types.attrsOf lib.types.str;
+      default = { };
       description = ''
-        List of derivations that will prevent switching into a configuration when
+        Attribute set of strings that will prevent switching into a configuration when
         they change.
-        This can be manually overridden on the command line if required.
+        The switch can be manually forced on the command line if required.
       '';
     };
   };
@@ -67,80 +67,86 @@
         ln -s ${config.system.build.inhibitSwitch} $out/switch-inhibitors
       '';
 
-      build.inhibitSwitch = pkgs.writeTextFile {
-        name = "switch-inhibitors";
-        text = lib.concatMapStringsSep "\n" (drv: drv.outPath) config.system.switch.inhibitors;
-      };
+      build.inhibitSwitch = pkgs.writers.writeJSON "switch-inhibitors" config.system.switch.inhibitors;
 
       preSwitchChecks.switchInhibitors =
         let
           realpath = lib.getExe' pkgs.coreutils "realpath";
-          sha256sum = lib.getExe' pkgs.coreutils "sha256sum";
-          diff = lib.getExe' pkgs.diffutils "diff";
+          mktemp = lib.getExe' pkgs.coreutils "mktemp";
+          rm = lib.getExe' pkgs.coreutils "rm";
+          jq = lib.getExe' pkgs.jq "jq";
         in
-        lib.mkIf (lib.length config.system.switch.inhibitors > 0)
-          # bash
-          ''
-            incoming="''${1-}"
-            action="''${2-}"
-
-            if [ "$action" == "boot" ]; then
-              echo "Not checking switch inhibitors (action = $action)"
-              exit
-            fi
-
-            echo -n "Checking switch inhibitors..."
-
-            booted_inhibitors="$(${realpath} /run/booted-system)/switch-inhibitors"
-            booted_inhibitors_sha="$(
-              if [ -f "$booted_inhibitors" ]; then
-                ${sha256sum} - < "$booted_inhibitors"
-              else
-                echo 'none'
-              fi
-            )"
-
-            if [ "$booted_inhibitors_sha" == "none" ]; then
-              echo
-              echo "The previous configuration did not specify switch inhibitors, nothing to check."
-              exit
-            fi
-
-            new_inhibitors="$(${realpath} "$incoming")/switch-inhibitors"
-            new_inhibitors_sha="$(
-              if [ -f "$new_inhibitors" ]; then
-                ${sha256sum} - < "$new_inhibitors"
-              else
-                echo 'none'
-              fi
-            )"
-
-            if [ "$new_inhibitors_sha" == "none" ]; then
-              echo
-              echo "The new configuration does not specify switch inhibitors, nothing to check."
-              exit
-            fi
-
-            if [ "$new_inhibitors_sha" != "$booted_inhibitors_sha" ]; then
-              echo
-              echo "Found diff in switch inhibitors:"
-              echo
-              ${diff} --color "$booted_inhibitors" "$new_inhibitors"
-              echo
-              echo "The new configuration contains changes to packages that were"
-              echo "listed as switch inhibitors."
-              echo "You probably want to run 'nixos-rebuild boot' and reboot your system."
-              echo
-              echo "If you really want to switch into this configuration directly, then"
-              echo "you can set NIXOS_NO_CHECK=1 to ignore pre-switch checks."
-              echo
-              echo "WARNING: doing so might cause the switch to fail or your system to become unstable."
-              echo
-              exit 1
-            else
-              echo " done"
-            fi
-          '';
+        # bash
+        ''
+          incoming="''${1-}"
+          action="''${2-}"
+
+          if [ "$action" == "boot" ]; then
+            echo "Not checking switch inhibitors (action = $action)"
+            exit
+          fi
+
+          echo -n "Checking switch inhibitors..."
+
+          # Create a temporary file that we use in case a generation does not have
+          # the switch-inhibitors file.
+          empty="$(${mktemp} -t switch_inhibit.XXXX)"
+          # shellcheck disable=SC2329
+          clean_up() {
+            ${rm} -f "$empty"
+          }
+          trap clean_up EXIT
+          echo "{}" > "$empty"
+
+          current_inhibitors="$(${realpath} /run/current-system)/switch-inhibitors"
+          if [ ! -f "$current_inhibitors" ]; then
+            current_inhibitors="$empty"
+          fi
+
+          new_inhibitors="$(${realpath} "$incoming")/switch-inhibitors"
+          if [ ! -f "$new_inhibitors" ]; then
+            new_inhibitors="$empty"
+          fi
+
+          diff="$(
+            ${jq} \
+              --raw-output \
+              --null-input \
+              --rawfile current "$current_inhibitors" \
+              --rawfile newgen "$new_inhibitors" \
+            '
+              $current | try fromjson catch {} as $old |
+              $newgen | try fromjson catch {} as $new |
+              $old |
+              to_entries |
+              map(
+                select(.key | in ($new)) |
+                select(.value != $new.[.key]) |
+                [ .key, ":", .value, "->", $new.[.key] ] | join(" ")
+              ) |
+              join("\n")
+            ' \
+          )"
+
+          if [ -n "$diff" ]; then
+            echo
+            echo "There are changes to critical components of the system:"
+            echo
+            echo "$diff"
+            echo
+            echo "Switching into this system is not recommended."
+            echo "You probably want to run 'nixos-rebuild boot' and reboot your system instead."
+            echo
+            echo "If you really want to switch into this configuration directly, then"
+            echo "you can set NIXOS_NO_CHECK=1 to ignore pre-switch checks."
+            echo
+            echo "WARNING: doing so might cause the switch to fail or your system to become unstable."
+            echo
+            exit 1
+          else
+            echo " done"
+          fi
+        '';
     };
 
     security =
diff --git a/nixos/tests/switch-test.nix b/nixos/tests/switch-test.nix
index 3ff6dcbc4e02..07e80cb86abd 100644
--- a/nixos/tests/switch-test.nix
+++ b/nixos/tests/switch-test.nix
@@ -646,6 +646,23 @@ in
                 EOF
               '';
             };
+
+          no_inhibitors.configuration.system.switch.inhibitors = lib.mkForce { };
+
+          inhibitors.configuration.system.switch.inhibitors = lib.mkForce {
+            foo = "bar";
+            quz = "bor";
+          };
+
+          inhibitors_changed.configuration.system.switch.inhibitors = lib.mkForce {
+            foo = "baz";
+            quz = "boz";
+          };
+
+          inhibitors_new.configuration.system.switch.inhibitors = lib.mkForce {
+            foo = "bar";
+            qux = "baz";
+          };
         };
       };
 
@@ -658,6 +675,7 @@ in
         echo "this should succeed (config: $config, action: $action)"
         [ "$action" == "check" ] || [ "$action" == "test" ]
       '';
+      boot.loader.grub.enable = false;
       specialisation.failingCheck.configuration.system.preSwitchChecks.failEveryTime = ''
         echo this will fail
         false
@@ -745,6 +763,24 @@ in
           out = switch_to_specialisation("${otherSystem}", "failingCheck", action="check", fail=True)
           assert_contains(out, "this will fail")
 
+      with subtest("switch inhibitors"):
+          # Start without any inhibitors
+          switch_to_specialisation("${machine}", "no_inhibitors", action="switch")
+          # Check that we can switch into a generation with inhibitors from one that doesn't have any
+          switch_to_specialisation("${machine}", "inhibitors", action="switch")
+          # Check that we cannot switch into a generation that has a different value for an existing inhibitor
+          out = switch_to_specialisation("${machine}", "inhibitors_changed", action="switch", fail=True)
+          assert_contains(out, "There are changes to critical components of the system")
+          assert_contains(out, "foo")
+          assert_contains(out, "bar")
+          assert_contains(out, "baz")
+          # Confirm that we can set that same generation as the new boot default
+          switch_to_specialisation("${machine}", "inhibitors_changed", action="boot")
+          # Check that we can switch into a new generation with new inhibitors, but same values for existing ones
+          switch_to_specialisation("${machine}", "inhibitors_new", action="switch")
+          # Check that we can switch back into a generation without inhibitors
+          switch_to_specialisation("${machine}", "no_inhibitors", action="switch")
+
       with subtest("actions"):
           # boot action
           out = switch_to_specialisation("${machine}", "simpleService", action="boot")