Merge pull request #221750 from rhendric/rhendric/nixos/snapper

2 files changed, 127 insertions, 39 deletions

diff --git a/nixos/doc/manual/release-notes/rl-2305.section.md b/nixos/doc/manual/release-notes/rl-2305.section.md
index c35312097ca45..1e869cf144010 100644
--- a/nixos/doc/manual/release-notes/rl-2305.section.md
+++ b/nixos/doc/manual/release-notes/rl-2305.section.md
@@ -197,6 +197,26 @@ In addition to numerous new and upgraded packages, this release has the followin
 
 - `services.sourcehut.dispatch` and the corresponding package (`sourcehut.dispatchsrht`) have been removed due to [upstream deprecation](https://sourcehut.org/blog/2022-08-01-dispatch-deprecation-plans/).
 
+- The attributes used by `services.snapper.configs.<name>` have changed. Migrate from this:
+
+  ```nix
+  services.snapper.configs.example = {
+    subvolume = "/example";
+    extraConfig = ''
+      ALLOW_USERS="alice"
+    '';
+  };
+  ```
+
+  to this:
+
+  ```nix
+  services.snapper.configs.example = {
+    SUBVOLUME = "/example";
+    ALLOW_USERS = [ "alice" ];
+  };
+  ```
+
 - The [services.snapserver.openFirewall](#opt-services.snapserver.openFirewall) module option default value has been changed from `true` to `false`. You will need to explicitly set this option to `true`, or configure your firewall.
 
 - The [services.tmate-ssh-server.openFirewall](#opt-services.tmate-ssh-server.openFirewall) module option default value has been changed from `true` to `false`. You will need to explicitly set this option to `true`, or configure your firewall.
diff --git a/nixos/modules/services/misc/snapper.nix b/nixos/modules/services/misc/snapper.nix
index 30ed30e7a8fa3..569433c3c71d1 100644
--- a/nixos/modules/services/misc/snapper.nix
+++ b/nixos/modules/services/misc/snapper.nix
@@ -4,6 +4,81 @@ with lib;
 
 let
   cfg = config.services.snapper;
+
+  mkValue = v:
+    if isList v then "\"${concatMapStringsSep " " (escape [ "\\" " " ]) v}\""
+    else if v == true then "yes"
+    else if v == false then "no"
+    else if isString v then "\"${v}\""
+    else builtins.toJSON v;
+
+  mkKeyValue = k: v: "${k}=${mkValue v}";
+
+  # "it's recommended to always specify the filesystem type"  -- man snapper-configs
+  defaultOf = k: if k == "FSTYPE" then null else configOptions.${k}.default or null;
+
+  safeStr = types.strMatching "[^\n\"]*" // {
+    description = "string without line breaks or quotes";
+    descriptionClass = "conjunction";
+  };
+
+  configOptions = {
+    SUBVOLUME = mkOption {
+      type = types.path;
+      description = lib.mdDoc ''
+        Path of the subvolume or mount point.
+        This path is a subvolume and has to contain a subvolume named
+        .snapshots.
+        See also man:snapper(8) section PERMISSIONS.
+      '';
+    };
+
+    FSTYPE = mkOption {
+      type = types.enum [ "btrfs" ];
+      default = "btrfs";
+      description = lib.mdDoc ''
+        Filesystem type. Only btrfs is stable and tested.
+      '';
+    };
+
+    ALLOW_GROUPS = mkOption {
+      type = types.listOf safeStr;
+      default = [];
+      description = lib.mdDoc ''
+        List of groups allowed to operate with the config.
+
+        Also see the PERMISSIONS section in man:snapper(8).
+      '';
+    };
+
+    ALLOW_USERS = mkOption {
+      type = types.listOf safeStr;
+      default = [];
+      example = [ "alice" ];
+      description = lib.mdDoc ''
+        List of users allowed to operate with the config. "root" is always
+        implicitly included.
+
+        Also see the PERMISSIONS section in man:snapper(8).
+      '';
+    };
+
+    TIMELINE_CLEANUP = mkOption {
+      type = types.bool;
+      default = false;
+      description = lib.mdDoc ''
+        Defines whether the timeline cleanup algorithm should be run for the config.
+      '';
+    };
+
+    TIMELINE_CREATE = mkOption {
+      type = types.bool;
+      default = false;
+      description = lib.mdDoc ''
+        Defines whether hourly snapshots should be created.
+      '';
+    };
+  };
 in
 
 {
@@ -52,49 +127,23 @@ in
       example = literalExpression ''
         {
           home = {
-            subvolume = "/home";
-            extraConfig = '''
-              ALLOW_USERS="alice"
-              TIMELINE_CREATE=yes
-              TIMELINE_CLEANUP=yes
-            ''';
+            SUBVOLUME = "/home";
+            ALLOW_USERS = [ "alice" ];
+            TIMELINE_CREATE = true;
+            TIMELINE_CLEANUP = true;
           };
         }
       '';
 
       description = lib.mdDoc ''
-        Subvolume configuration
+        Subvolume configuration. Any option mentioned in man:snapper-configs(5)
+        is valid here, even if NixOS doesn't document it.
       '';
 
       type = types.attrsOf (types.submodule {
-        options = {
-          subvolume = mkOption {
-            type = types.path;
-            description = lib.mdDoc ''
-              Path of the subvolume or mount point.
-              This path is a subvolume and has to contain a subvolume named
-              .snapshots.
-              See also man:snapper(8) section PERMISSIONS.
-            '';
-          };
-
-          fstype = mkOption {
-            type = types.enum [ "btrfs" ];
-            default = "btrfs";
-            description = lib.mdDoc ''
-              Filesystem type. Only btrfs is stable and tested.
-            '';
-          };
+        freeformType = types.attrsOf (types.oneOf [ (types.listOf safeStr) types.bool safeStr types.number ]);
 
-          extraConfig = mkOption {
-            type = types.lines;
-            default = "";
-            description = lib.mdDoc ''
-              Additional configuration next to SUBVOLUME and FSTYPE.
-              See man:snapper-configs(5).
-            '';
-          };
-        };
+        options = configOptions;
       });
     };
   };
@@ -117,11 +166,7 @@ in
 
       }
       // (mapAttrs' (name: subvolume: nameValuePair "snapper/configs/${name}" ({
-        text = ''
-          ${subvolume.extraConfig}
-          FSTYPE="${subvolume.fstype}"
-          SUBVOLUME="${subvolume.subvolume}"
-        '';
+        text = lib.generators.toKeyValue { inherit mkKeyValue; } (filterAttrs (k: v: v != defaultOf k) subvolume);
       })) cfg.configs)
       // (lib.optionalAttrs (cfg.filters != null) {
         "snapper/filters/default.txt".text = cfg.filters;
@@ -181,5 +226,28 @@ in
       unitConfig.ConditionPathExists = "/etc/snapper/configs/root";
     };
 
+    assertions =
+      concatMap
+        (name:
+          let
+            sub = cfg.configs.${name};
+          in
+          [ { assertion = !(sub ? extraConfig);
+              message = ''
+                The option definition `services.snapper.configs.${name}.extraConfig' no longer has any effect; please remove it.
+                The contents of this option should be migrated to attributes on `services.snapper.configs.${name}'.
+              '';
+            }
+          ] ++
+          map
+            (attr: {
+              assertion = !(hasAttr attr sub);
+              message = ''
+                The option definition `services.snapper.configs.${name}.${attr}' has been renamed to `services.snapper.configs.${name}.${toUpper attr}'.
+              '';
+            })
+            [ "fstype" "subvolume" ]
+        )
+        (attrNames cfg.configs);
   });
 }