From 89dba90892c2dfa9ce16c75cd0f816c6ac00c06e Mon Sep 17 00:00:00 2001
From: Sander van der Burg <svanderburg@gmail.com>
Date: Sat, 3 Apr 2021 15:45:12 +0200
Subject: [PATCH] Fix fcron service to properly load the crontab on first
 startup, make it multi-instance aware, add tests

---
 example-deployments/services/processes.nix |  2 +-
 services-agnostic/constructors.nix         |  2 +-
 services-agnostic/fcron/declarative.nix    | 15 ++++----
 services-agnostic/fcron/default.nix        | 18 +++++++++-
 tests/default.nix                          |  4 +++
 tests/fcron/default.nix                    | 24 +++++++++++++
 tests/fcron/processes.nix                  | 40 ++++++++++++++++++++++
 7 files changed, 96 insertions(+), 9 deletions(-)
 create mode 100644 tests/fcron/default.nix
 create mode 100644 tests/fcron/processes.nix

diff --git a/example-deployments/services/processes.nix b/example-deployments/services/processes.nix
index 2e0bc28..1b9d055 100644
--- a/example-deployments/services/processes.nix
+++ b/example-deployments/services/processes.nix
@@ -127,7 +127,7 @@ rec {
     pkg = constructors.declarativeFcron {
       fcrontabPerUser = {
         systab = ''
-          @ 1 echo hello >> /tmp/hello
+          */1 * * * * echo hello >> /tmp/hello
         '';
       };
     };
diff --git a/services-agnostic/constructors.nix b/services-agnostic/constructors.nix
index 291455c..5d00627 100644
--- a/services-agnostic/constructors.nix
+++ b/services-agnostic/constructors.nix
@@ -74,7 +74,7 @@ in
 
   declarativeFcron = import ./fcron/declarative.nix {
     inherit createManagedProcess stateDir spoolDir runtimeDir tmpDir forceDisableUserChange;
-    inherit (pkgs) lib writeTextFile fcron;
+    inherit (pkgs) lib writeTextFile fcron utillinux;
   };
 
   hydra-evaluator = import ./hydra/hydra-evaluator.nix {
diff --git a/services-agnostic/fcron/declarative.nix b/services-agnostic/fcron/declarative.nix
index a1045a9..6ba05b2 100644
--- a/services-agnostic/fcron/declarative.nix
+++ b/services-agnostic/fcron/declarative.nix
@@ -1,4 +1,4 @@
-{createManagedProcess, writeTextFile, lib, fcron, stateDir, runtimeDir, tmpDir, spoolDir, forceDisableUserChange}:
+{createManagedProcess, writeTextFile, lib, fcron, utillinux, stateDir, runtimeDir, tmpDir, spoolDir, forceDisableUserChange}:
 
 { instanceSuffix ? ""
 , instanceName ? "fcron${instanceSuffix}"
@@ -8,14 +8,15 @@
 
 let
   fcronSpoolDir = "${spoolDir}/${instanceName}";
+  fcronEtcDir = "${stateDir}/etc/${instanceName}";
 in
 import ./default.nix {
   inherit createManagedProcess writeTextFile lib fcron stateDir runtimeDir tmpDir spoolDir forceDisableUserChange;
 } {
   inherit instanceSuffix instanceName;
 
-  initialize = ''
-    ${lib.concatMapStrings (user:
+  initialize =
+    lib.concatMapStrings (user:
       let
         fcrontab = builtins.getAttr user fcrontabPerUser;
         fcrontabFile = writeTextFile {
@@ -26,7 +27,9 @@ import ./default.nix {
       ''
         cp ${fcrontabFile} ${fcronSpoolDir}/${user}.orig
       ''
-    ) (builtins.attrNames fcrontabPerUser)}
-    ${initialize}
-  '';
+      + ''
+        ${lib.optionalString (!forceDisableUserChange) "${utillinux}/bin/runuser -u root -g ${instanceName} --"} fcrontab -c ${fcronEtcDir}/fcron.conf -u systab -z
+      ''
+    ) (builtins.attrNames fcrontabPerUser)
+    + initialize;
 }
diff --git a/services-agnostic/fcron/default.nix b/services-agnostic/fcron/default.nix
index 7aa746c..0b1b3ee 100644
--- a/services-agnostic/fcron/default.nix
+++ b/services-agnostic/fcron/default.nix
@@ -20,6 +20,21 @@ let
 
   user = instanceName;
   group = instanceName;
+
+  # We must override the package configuration settings to compile it with different configuration settings,
+  # if we want to use a different group or run fcron as unprivileged user
+  fcronPkg =
+    if forceDisableUserChange then fcron.overrideAttrs (originalAttrs:
+      originalAttrs // {
+        configureFlags = originalAttrs.configureFlags ++ [ "--with-run-non-privileged" "--with-rootname=unprivileged" "--with-rootgroup=users" "--with-username=unprivileged" "--with-groupname=users" ];
+      }
+    )
+    else if user != "fcron" || group != "fcron" then fcron.overrideAttrs (originalAttrs:
+      originalAttrs // {
+        configureFlags = originalAttrs.configureFlags ++ [ "--with-rootgroup=${group}" "--with-username=${user}" "--with-groupname=${group}" ];
+      }
+    )
+    else fcron;
 in
 createManagedProcess {
   inherit instanceName;
@@ -34,7 +49,8 @@ createManagedProcess {
     ${initialize}
   '';
 
-  process = "${fcron}/bin/fcron";
+  path = [ fcronPkg ];
+  process = "${fcronPkg}/bin/fcron";
   args = [ "--configfile" "${fcronEtcDir}/fcron.conf" ];
   foregroundProcessExtraArgs = [ "--foreground" "--nosyslog" ];
   daemonExtraArgs = [ "--background" ];
diff --git a/tests/default.nix b/tests/default.nix
index 90199fc..d97363a 100644
--- a/tests/default.nix
+++ b/tests/default.nix
@@ -27,6 +27,10 @@ in
     inherit pkgs processManagers profiles testService;
   };
 
+  fcron = import ./fcron {
+    inherit pkgs processManagers profiles testService;
+  };
+
   influxdb = import ./influxdb {
     inherit pkgs processManagers profiles testService;
   };
diff --git a/tests/fcron/default.nix b/tests/fcron/default.nix
new file mode 100644
index 0000000..05a3280
--- /dev/null
+++ b/tests/fcron/default.nix
@@ -0,0 +1,24 @@
+{ pkgs, testService, processManagers, profiles }:
+
+testService {
+  exprFile = ./processes.nix;
+
+  readiness = {instanceName, instance, ...}:
+    ''
+    '';
+
+  tests = {instanceName, instance, ...}:
+    ''
+      machine.succeed("sleep 70")
+    ''
+    +
+    (if instanceName == "fcron" then ''
+      machine.succeed("grep 'hello' /tmp/hello")
+    ''
+    else if instanceName == "fcron-secondary" then ''
+      machine.succeed("grep 'bye' /tmp/bye")
+    ''
+    else "");
+
+  inherit processManagers profiles;
+}
diff --git a/tests/fcron/processes.nix b/tests/fcron/processes.nix
new file mode 100644
index 0000000..2b35227
--- /dev/null
+++ b/tests/fcron/processes.nix
@@ -0,0 +1,40 @@
+{ pkgs ? import <nixpkgs> { inherit system; }
+, system ? builtins.currentSystem
+, stateDir ? "/var"
+, runtimeDir ? "${stateDir}/run"
+, logDir ? "${stateDir}/log"
+, spoolDir ? "${stateDir}/spool"
+, cacheDir ? "${stateDir}/cache"
+, libDir ? "${stateDir}/lib"
+, tmpDir ? (if stateDir == "/var" then "/tmp" else "${stateDir}/tmp")
+, forceDisableUserChange ? false
+, processManager
+}:
+
+let
+  constructors = import ../../services-agnostic/constructors.nix {
+    inherit pkgs stateDir runtimeDir logDir tmpDir cacheDir libDir spoolDir forceDisableUserChange processManager;
+  };
+in
+rec {
+  fcron = {
+    pkg = constructors.declarativeFcron {
+      fcrontabPerUser = {
+        systab = ''
+          */1 * * * * echo hello >> /tmp/hello
+        '';
+      };
+    };
+  };
+
+  fcron-secondary = {
+    pkg = constructors.declarativeFcron {
+      instanceSuffix = "-secondary";
+      fcrontabPerUser = {
+        systab = ''
+          */1 * * * * echo bye >> /tmp/bye
+        '';
+      };
+    };
+  };
+}