From 46464911756ced488d10404769e2bb45434765ac Mon Sep 17 00:00:00 2001
From: Alexandru Scvortov <code@scvalex.net>
Date: Mon, 18 Apr 2022 09:44:46 +0100
Subject: [PATCH] nixos/nbd: fix nbd-server config section ordering

Closes #169103
---
 nixos/modules/services/networking/nbd.nix | 43 +++++++++++++++--------
 nixos/tests/nbd.nix                       | 16 +++++++++
 2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/nixos/modules/services/networking/nbd.nix b/nixos/modules/services/networking/nbd.nix
index 87f8c41a8e5c..df3358f51876 100644
--- a/nixos/modules/services/networking/nbd.nix
+++ b/nixos/modules/services/networking/nbd.nix
@@ -4,28 +4,34 @@ with lib;
 
 let
   cfg = config.services.nbd;
-  configFormat = pkgs.formats.ini { };
   iniFields = with types; attrsOf (oneOf [ bool int float str ]);
-  serverConfig = configFormat.generate "nbd-server-config"
-    ({
-      generic =
-        (cfg.server.extraOptions // {
-          user = "root";
-          group = "root";
-          port = cfg.server.listenPort;
-        } // (optionalAttrs (cfg.server.listenAddress != null) {
-          listenaddr = cfg.server.listenAddress;
-        }));
-    }
-    // (mapAttrs
+  # The `[generic]` section must come before all the others in the
+  # config file.  This means we can't just dump an attrset to INI
+  # because that sorts the sections by name.  Instead, we serialize it
+  # on its own first.
+  genericSection = {
+    generic = (cfg.server.extraOptions // {
+      user = "root";
+      group = "root";
+      port = cfg.server.listenPort;
+    } // (optionalAttrs (cfg.server.listenAddress != null) {
+      listenaddr = cfg.server.listenAddress;
+    }));
+  };
+  exportSections =
+    mapAttrs
       (_: { path, allowAddresses, extraOptions }:
         extraOptions // {
           exportname = path;
         } // (optionalAttrs (allowAddresses != null) {
           authfile = pkgs.writeText "authfile" (concatStringsSep "\n" allowAddresses);
         }))
-      cfg.server.exports)
-    );
+      cfg.server.exports;
+  serverConfig =
+    pkgs.writeText "nbd-server-config" ''
+      ${lib.generators.toINI {} genericSection}
+      ${lib.generators.toINI {} exportSections}
+    '';
   splitLists =
     partition
       (path: hasPrefix "/dev/" path)
@@ -103,6 +109,13 @@ in
   };
 
   config = mkIf cfg.server.enable {
+    assertions = [
+      {
+        assertion = !(cfg.server.exports ? "generic");
+        message = "services.nbd.server exports must not be named 'generic'";
+      }
+    ];
+
     boot.kernelModules = [ "nbd" ];
 
     systemd.services.nbd-server = {
diff --git a/nixos/tests/nbd.nix b/nixos/tests/nbd.nix
index 16255e68e8a1..b4aaf29ee4e5 100644
--- a/nixos/tests/nbd.nix
+++ b/nixos/tests/nbd.nix
@@ -28,6 +28,11 @@ import ./make-test-python.nix ({ pkgs, ... }:
         ## It's also a loopback device to test exporting /dev/...
         systemd.services.create-priv-file =
           mkCreateSmallFileService { path = "/vault-priv.disk"; loop = true; };
+        ## `aaa.disk` is just here because "[aaa]" sorts before
+        ## "[generic]" lexicographically, and nbd-server breaks if
+        ## "[generic]" isn't the first section.
+        systemd.services.create-aaa-file =
+          mkCreateSmallFileService { path = "/aaa.disk"; };
 
         # Needed only for nbd-client used in the tests.
         environment.systemPackages = [ pkgs.nbd ];
@@ -39,6 +44,9 @@ import ./make-test-python.nix ({ pkgs, ... }:
         services.nbd.server = {
           enable = true;
           exports = {
+            aaa = {
+              path = "/aaa.disk";
+            };
             vault-pub = {
               path = "/vault-pub.disk";
             };
@@ -83,5 +91,13 @@ import ./make-test-python.nix ({ pkgs, ... }:
       if foundString != testString:
          raise Exception(f"Read the wrong string from nbd disk. Expected: '{testString}'. Found: '{foundString}'")
       server.succeed("nbd-client -d /dev/nbd0")
+
+      # Server: Successfully connect to the aaa disk
+      server.succeed("nbd-client localhost ${toString listenPort} /dev/nbd0 -name aaa -persist")
+      server.succeed(f"echo '{testString}' | dd of=/dev/nbd0 conv=notrunc")
+      foundString = server.succeed(f"dd status=none if=/aaa.disk count={len(testString)}")[:len(testString)]
+      if foundString != testString:
+         raise Exception(f"Read the wrong string from nbd disk. Expected: '{testString}'. Found: '{foundString}'")
+      server.succeed("nbd-client -d /dev/nbd0")
     '';
   })