From 257db1dd4a2dc4569ba5bfeb9d89f40cd891410c Mon Sep 17 00:00:00 2001
From: squalus <squalus@squalus.net>
Date: Mon, 1 Aug 2022 09:44:29 -0700
Subject: [PATCH] nixos: systemd-coredump: improve disabled state

The systemd-coredump module required systemd to be built with
withCoredump=true, even if the module was disabled.

- allow systemd to be missing systemd-coredump if the module is disabled
- switch to mkDefault for the sysctl config to allow user overrides when
  the module is disabled
- add nixos tests for both the enabled and disabled cases
---
 .../modules/system/boot/systemd/coredump.nix  | 51 +++++++++++--------
 nixos/tests/all-tests.nix                     |  1 +
 nixos/tests/systemd-coredump.nix              | 44 ++++++++++++++++
 3 files changed, 74 insertions(+), 22 deletions(-)
 create mode 100644 nixos/tests/systemd-coredump.nix

diff --git a/nixos/modules/system/boot/systemd/coredump.nix b/nixos/modules/system/boot/systemd/coredump.nix
index b6ee2cff1f9a..d1dff01d3fa0 100644
--- a/nixos/modules/system/boot/systemd/coredump.nix
+++ b/nixos/modules/system/boot/systemd/coredump.nix
@@ -28,30 +28,37 @@ in {
     };
   };
 
-  config = {
-    systemd.additionalUpstreamSystemUnits = [
-      "systemd-coredump.socket"
-      "systemd-coredump@.service"
-    ];
+  config = mkMerge [
 
-    environment.etc = {
-      "systemd/coredump.conf".text =
-      ''
-        [Coredump]
-        ${cfg.extraConfig}
-      '';
+    (mkIf cfg.enable {
+      systemd.additionalUpstreamSystemUnits = [
+        "systemd-coredump.socket"
+        "systemd-coredump@.service"
+      ];
 
-      # install provided sysctl snippets
-      "sysctl.d/50-coredump.conf".source = "${systemd}/example/sysctl.d/50-coredump.conf";
-      "sysctl.d/50-default.conf".source = "${systemd}/example/sysctl.d/50-default.conf";
-    };
+      environment.etc = {
+        "systemd/coredump.conf".text =
+        ''
+          [Coredump]
+          ${cfg.extraConfig}
+        '';
 
-    users.users.systemd-coredump = {
-      uid = config.ids.uids.systemd-coredump;
-      group = "systemd-coredump";
-    };
-    users.groups.systemd-coredump = {};
+        # install provided sysctl snippets
+        "sysctl.d/50-coredump.conf".source = "${systemd}/example/sysctl.d/50-coredump.conf";
+        "sysctl.d/50-default.conf".source = "${systemd}/example/sysctl.d/50-default.conf";
+      };
+
+      users.users.systemd-coredump = {
+        uid = config.ids.uids.systemd-coredump;
+        group = "systemd-coredump";
+      };
+      users.groups.systemd-coredump = {};
+    })
+
+    (mkIf (!cfg.enable) {
+     boot.kernel.sysctl."kernel.core_pattern" = mkDefault "core";
+    })
+
+  ];
 
-    boot.kernel.sysctl."kernel.core_pattern" = mkIf (!cfg.enable) "core";
-  };
 }
diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix
index ff54a327424e..06210095cfce 100644
--- a/nixos/tests/all-tests.nix
+++ b/nixos/tests/all-tests.nix
@@ -537,6 +537,7 @@ in {
   systemd-binfmt = handleTestOn ["x86_64-linux"] ./systemd-binfmt.nix {};
   systemd-boot = handleTest ./systemd-boot.nix {};
   systemd-confinement = handleTest ./systemd-confinement.nix {};
+  systemd-coredump = handleTest ./systemd-coredump.nix {};
   systemd-cryptenroll = handleTest ./systemd-cryptenroll.nix {};
   systemd-escaping = handleTest ./systemd-escaping.nix {};
   systemd-initrd-btrfs-raid = handleTest ./systemd-initrd-btrfs-raid.nix {};
diff --git a/nixos/tests/systemd-coredump.nix b/nixos/tests/systemd-coredump.nix
new file mode 100644
index 000000000000..62137820878b
--- /dev/null
+++ b/nixos/tests/systemd-coredump.nix
@@ -0,0 +1,44 @@
+import ./make-test-python.nix ({ pkgs, ... }:
+
+let
+
+  crasher = pkgs.writeCBin "crasher" "int main;";
+
+  commonConfig = {
+    systemd.services.crasher.serviceConfig = {
+      ExecStart = "${crasher}/bin/crasher";
+      StateDirectory = "crasher";
+      WorkingDirectory = "%S/crasher";
+      Restart = "no";
+    };
+  };
+
+in
+
+{
+  name = "systemd-coredump";
+  meta = with pkgs.lib.maintainers; {
+    maintainers = [ squalus ];
+  };
+
+  nodes.machine1 = { pkgs, lib, ... }: commonConfig;
+  nodes.machine2 = { pkgs, lib, ... }: lib.recursiveUpdate commonConfig {
+    systemd.coredump.enable = false;
+    systemd.package = pkgs.systemd.override {
+      withCoredump = false;
+    };
+  };
+
+  testScript = ''
+    with subtest("systemd-coredump enabled"):
+      machine1.wait_for_unit("multi-user.target")
+      machine1.wait_for_unit("systemd-coredump.socket")
+      machine1.systemctl("start crasher");
+      machine1.wait_until_succeeds("coredumpctl list | grep crasher", timeout=10)
+      machine1.fail("stat /var/lib/crasher/core")
+
+    with subtest("systemd-coredump disabled"):
+      machine2.systemctl("start crasher");
+      machine2.wait_until_succeeds("stat /var/lib/crasher/core", timeout=10)
+  '';
+})