nixos/borgbackup: fix extraArgs shell expansion (#332319)

nixos/doc/manual/release-notes/rl-2505.section.md

@@ -154,6 +154,12 @@
   ```
   This changed follows a deprecation period of one year started in NixOS 24.05 (see [PR #283818](https://github.com/NixOS/nixpkgs/pull/283818)).
 
+- The values of `services.borgbackup.jobs.*.extraArgs` and other `extra*Args` options are now represented as Bash arrays. If these arguments were modified using `services.borgbackup.jobs.*.preHook`, they will need to be adjusted to append to these arrays, i.e.
+  ```diff
+  -extraCreateArgs="$extraCreateArgs --exclude /some/path"
+  +extraCreateArgs+=("--exclude" "/some/path")
+  ```
+
 - `nodePackages.ganache` has been removed, as the package has been deprecated by upstream.
 
 - `virtualisation.azure.agent` option provided by `azure-agent.nix` is replaced by `services.waagent`, and will be removed in a future release.

nixos/modules/services/backup/borgbackup.nix

@@ -20,8 +20,19 @@ let
     lib.concatStringsSep " "
       (lib.mapAttrsToList (x: y: "--keep-${x}=${toString y}") cfg.prune.keep);
 
+  mkExtraArgs = cfg:
+    # Create BASH arrays of extra args
+    lib.concatLines
+      (lib.mapAttrsToList (name: values: ''
+        ${name}=(${values})
+      '')
+  { inherit (cfg) extraArgs extraInitArgs extraCreateArgs extraPruneArgs extraCompactArgs; });
+
   mkBackupScript = name: cfg: pkgs.writeShellScript "${name}-script" (''
     set -e
+
+    ${mkExtraArgs cfg}
+
     on_exit()
     {
       exitStatus=$?
@@ -46,35 +57,35 @@ let
     ${cfg.preHook}
   '' + lib.optionalString cfg.doInit ''
     # Run borg init if the repo doesn't exist yet
-    if ! borgWrapper list $extraArgs > /dev/null; then
+    if ! borgWrapper list "''${extraArgs[@]}" > /dev/null; then
-      borgWrapper init $extraArgs \
+      borgWrapper init "''${extraArgs[@]}" \
         --encryption ${cfg.encryption.mode} \
-        $extraInitArgs
+        "''${extraInitArgs[@]}"
       ${cfg.postInit}
     fi
   '' + ''
     (
       set -o pipefail
       ${lib.optionalString (cfg.dumpCommand != null) ''${lib.escapeShellArg cfg.dumpCommand} | \''}
-      borgWrapper create $extraArgs \
+      borgWrapper create "''${extraArgs[@]}" \
         --compression ${cfg.compression} \
         --exclude-from ${mkExcludeFile cfg} \
         --patterns-from ${mkPatternsFile cfg} \
-        $extraCreateArgs \
+        "''${extraCreateArgs[@]}" \
         "::$archiveName$archiveSuffix" \
         ${if cfg.paths == null then "-" else lib.escapeShellArgs cfg.paths}
     )
   '' + lib.optionalString cfg.appendFailedSuffix ''
-    borgWrapper rename $extraArgs \
+    borgWrapper rename "''${extraArgs[@]}" \
       "::$archiveName$archiveSuffix" "$archiveName"
   '' + ''
     ${cfg.postCreate}
   '' + lib.optionalString (cfg.prune.keep != { }) ''
-    borgWrapper prune $extraArgs \
+    borgWrapper prune "''${extraArgs[@]}" \
       ${mkKeepArgs cfg} \
       ${lib.optionalString (cfg.prune.prefix != null) "--glob-archives ${lib.escapeShellArg "${cfg.prune.prefix}*"}"} \
-      $extraPruneArgs
+      "''${extraPruneArgs[@]}"
-    borgWrapper compact $extraArgs $extraCompactArgs
+    borgWrapper compact "''${extraArgs[@]}" "''${extraCompactArgs[@]}"
     ${cfg.postPrune}
   '');
 
@@ -120,7 +131,6 @@ let
       };
       environment = {
         BORG_REPO = cfg.repo;
-        inherit (cfg) extraArgs extraInitArgs extraCreateArgs extraPruneArgs extraCompactArgs;
       } // (mkPassEnv cfg) // cfg.environment;
     };
 
@@ -236,7 +246,7 @@ let
   };
 
 in {
-  meta.maintainers = with lib.maintainers; [ dotlambda ];
+  meta.maintainers = with lib.maintainers; [ dotlambda Scrumplex ];
   meta.doc = ./borgbackup.md;
 
   ###### interface
@@ -581,7 +591,7 @@ in {
             default = "";
             example = ''
               # To add excluded paths at runtime
-              extraCreateArgs="$extraCreateArgs --exclude /some/path"
+              extraCreateArgs+=("--exclude" "/some/path")
             '';
           };
 

nixos/tests/borgbackup.nix

@@ -3,6 +3,8 @@ import ./make-test-python.nix ({ pkgs, ... }:
 let
   passphrase = "supersecret";
   dataDir = "/ran:dom/data";
+  subDir = "not_anything_here";
+  excludedSubDirFile = "not_this_file_either";
   excludeFile = "not_this_file";
   keepFile = "important_file";
   keepFileData = "important_data";
@@ -69,6 +71,7 @@ in {
             yearly = 5;
           };
           exclude = [ "*/${excludeFile}" ];
+          extraCreateArgs = [ "--exclude-caches" "--exclude-if-present" ".dont backup" ];
           postHook = "echo post";
           startAt = [ ]; # Do not run automatically
         };
@@ -166,8 +169,10 @@ in {
     )
     client.succeed("chmod 0600 /root/id_ed25519.appendOnly")
 
-    client.succeed("mkdir -p ${dataDir}")
+    client.succeed("mkdir -p ${dataDir}/${subDir}")
     client.succeed("touch ${dataDir}/${excludeFile}")
+    client.succeed("touch '${dataDir}/${subDir}/.dont backup'")
+    client.succeed("touch ${dataDir}/${subDir}/${excludedSubDirFile}")
     client.succeed("echo '${keepFileData}' > ${dataDir}/${keepFile}")
 
     with subtest("local"):
@@ -180,6 +185,10 @@ in {
         client.fail(
             "{} list '${localRepo}::${archiveName}' | grep -qF '${excludeFile}'".format(borg)
         )
+        # Make sure excludedSubDirFile has been excluded
+        client.fail(
+            "{} list '${localRepo}::${archiveName}' | grep -qF '${subDir}/${excludedSubDirFile}".format(borg)
+        )
         # Make sure keepFile has the correct content
         client.succeed("{} extract '${localRepo}::${archiveName}'".format(borg))
         assert "${keepFileData}" in client.succeed("cat ${dataDir}/${keepFile}")