address roberth comments

SEE
https://git.fediversity.eu/taeer/simple-nixos-fediverse/compare/main...roberth:review

README.md

@@ -2,7 +2,7 @@
 
 This repo is, for now, an attempt to familiarize myself with NixOS options for Fediverse applications, and build up a configuration layer that will set most of the relevant options for you (in a semi-opinionated way) given some high-level configuration. The goal is something in the same vein as [nixos-mailserver](https://gitlab.com/simple-nixos-mailserver/nixos-mailserver) but for fediversity.
 
-Eventually, this will be tailored to high-throughput multi-machine setups. For now, it's just a small configuration to run in VMs.
+Eventually, this will be tailored to high-throughput multi-machine setups. For now, it's just a small set of configurations to run in VMs.
 
 ## Running the VMs
 

common.nix

@@ -1,4 +1,5 @@
 { pkgs, ... }: {
+  # customize nixos-rebuild build-vm to be a bit more convenient
   virtualisation.vmVariant = {
     # let us log in
     users.mutableUsers = false;

garage.nix

@@ -7,12 +7,54 @@ let
   };
 in
 # TODO: expand to a multi-machine setup
-{ config, lib, pkgs, ... }: {
+{ config, lib, pkgs, ... }: 
-  # add in options to ensure creation of buckets and keys
-  options =
 let
-    inherit (lib) types mkOption mkEnableOption;
+  inherit (lib) types mkOption mkEnableOption optionalString concatStringsSep;
+  inherit (lib.strings) escapeShellArg;
+  cfg = config.services.garage;
+  concatMapAttrs = scriptFn: attrset: concatStringsSep "\n" (lib.mapAttrsToList scriptFn attrset);
+  ensureBucketScriptFn = bucket: { website, aliases, corsRules }:
+    let
+      bucketArg = escapeShellArg bucket;
+      corsRulesJSON = escapeShellArg (builtins.toJSON {
+        CORSRules = [{
+          AllowedHeaders = corsRules.allowedHeaders;
+          AllowedMethods = corsRules.allowedMethods;
+          AllowedOrigins = corsRules.allowedOrigins;
+        }];
+      });
+    in ''
+      # garage bucket info tells us if the bucket already exists
+      garage bucket info ${bucketArg} || garage bucket create ${bucketArg}
+
+      # TODO: should this --deny the website if `website` is false?
+      ${optionalString website ''
+        garage bucket website --allow ${bucketArg}
+      ''}
+
+      ${concatStringsSep "\n" (map (alias: ''
+        garage bucket alias ${bucketArg} ${escapeShellArg alias}
+      '') aliases)}
+
+      ${optionalString corsRules.enable ''
+        garage bucket allow --read --write --owner ${bucketArg} --key tmp
+        aws --endpoint http://s3.garage.localhost:3900 s3api put-bucket-cors --bucket ${bucketArg} --cors-configuration ${corsRulesJSON}
+        garage bucket deny --read --write --owner ${bucketArg} --key tmp
+      ''}
+    '';
+  ensureBucketsScript = concatMapAttrs ensureBucketScriptFn cfg.ensureBuckets;
+  ensureAccessScriptFn = key: bucket: { read, write, owner }: ''
+    garage bucket allow ${optionalString read "--read"} ${optionalString write "--write"} ${optionalString owner "--owner"} \
+      ${escapeShellArg bucket} --key ${escapeShellArg key}
+  '';
+  ensureKeyScriptFn = key: {id, secret, ensureAccess}: ''
+    garage key import --yes -n ${escapeShellArg key} ${escapeShellArg id} ${escapeShellArg secret}
+    ${concatMapAttrs (ensureAccessScriptFn key) ensureAccess}
+  '';
+  ensureKeysScript = concatMapAttrs ensureKeyScriptFn cfg.ensureKeys;
 in {
+  # add in options to ensure creation of buckets and keys
+  options = {
     services.garage = {
       ensureBuckets = mkOption {
         type = types.attrsOf (types.submodule {
@@ -55,6 +97,7 @@ in
               type = types.str;
             };
             # TODO: assert at least one of these is true
+            # NOTE: this currently needs to be done at the top level module
             ensureAccess = mkOption {
               type = types.attrsOf (types.submodule {
                 options = {
@@ -122,7 +165,7 @@ in
     systemd.services.ensure-garage = {
       after = [ "garage.service" ];
       wantedBy = [ "garage.service" ];
-      path = [ config.services.garage.package pkgs.perl pkgs.awscli ];
+      path = [ cfg.package pkgs.perl pkgs.awscli ];
       script = ''
         set -xeuo pipefail
         # give garage time to start up
@@ -144,50 +187,11 @@ in
         export AWS_ACCESS_KEY_ID=${snakeoil_key.id};
         export AWS_SECRET_ACCESS_KEY=${snakeoil_key.secret};
 
-        ${
+        ${ensureBucketsScript}
-          lib.concatStringsSep "\n" (lib.mapAttrsToList (bucket: { website, aliases, corsRules }: ''
+        ${ensureKeysScript}
-            # garage bucket info tells us if the bucket already exists
-            garage bucket info ${bucket} || garage bucket create ${bucket}
 
-            # TODO: should this --deny the website if `website` is false?
+        # garage doesn't like deleting keys that once existed
-            ${lib.optionalString website ''
+        # garage key delete ${snakeoil_key.id} --yes
-              garage bucket website --allow ${bucket}
-            ''}
-
-            ${lib.concatStringsSep "\n" (map (alias: ''
-              garage bucket alias ${bucket} ${alias}
-            '') aliases)}
-
-            ${lib.optionalString corsRules.enable ''
-              # TODO: can i turn this whole thing into one builtins.toJSON?
-              export CORS=${lib.concatStrings [
-                "'"
-                ''{"CORSRules":[{''
-                ''"AllowedHeaders":${builtins.toJSON corsRules.allowedHeaders},''
-                ''"AllowedMethods":${builtins.toJSON corsRules.allowedMethods},''
-                ''"AllowedOrigins":${builtins.toJSON corsRules.allowedOrigins}''
-                ''}]}''
-                "'"
-              ]}
-
-              garage bucket allow --read --write --owner ${bucket} --key tmp
-              aws --endpoint http://s3.garage.localhost:3900 s3api put-bucket-cors --bucket ${bucket} --cors-configuration $CORS
-              garage bucket deny --read --write --owner ${bucket} --key tmp
-            ''}
-          '') config.services.garage.ensureBuckets)
-        }
-        ${
-          lib.concatStringsSep "\n" (lib.mapAttrsToList (key: {id, secret, ensureAccess}: ''
-            garage key import --yes -n ${key} ${id} ${secret}
-            ${
-              lib.concatStringsSep "\n" (lib.mapAttrsToList (bucket: { read, write, owner }: ''
-                garage bucket allow ${lib.optionalString read "--read"} ${lib.optionalString write "--write"} ${lib.optionalString owner "--owner"} ${bucket} --key ${key}
-              '') ensureAccess)
-            }
-          '') config.services.garage.ensureKeys)
-        }
-
-        garage key delete ${snakeoil_key.id} --yes
       '';
     };
   };

mastodon.nix

@@ -99,11 +99,7 @@ in
 
         # from the documentation: recommended is the amount of your CPU cores minus one.
         # but it also must be a positive integer
-        streamingProcesses = let
+        streamingProcesses = lib.max 1 (config.virtualisation.cores - 1);
-          ncores = config.virtualisation.cores;
-          max = x: y: if x > y then x else y;
-        in
-          max 1 (ncores - 1);
       };
 
       security.acme = {
@@ -159,20 +155,33 @@ in
         # ensureDatabases = [ "mastodon_development_test" "mastodon_test" ];
       };
 
-      # run rails db:seed so that mastodon sets up the databases for us
+      # Currently, nixos seems to be able to create a single database per
+      # postgres user. This works for the production version of mastodon, which
+      # is what's packaged in nixpkgs. For development, we need two databases,
+      # mastodon_development and mastodon_test. This used to be possible with
+      # ensurePermissions, but that's broken and has been removed. Here I copy
+      # the mastodon-init-db script from upstream nixpkgs, but add the single
+      # line `rails db:setup`, which asks mastodon to create the postgres
+      # databases for us.
+      # FIXME: the commented out lines were breaking things, but presumably they're necessary for something.
+      # TODO: see if we can fix the upstream ensurePermissions stuff. See above for what that config would look like.
       systemd.services.mastodon-init-db.script = lib.mkForce ''
-          if [ `psql -c \
+        result="$(psql -t --csv -c \
             "select count(*) from pg_class c \
             join pg_namespace s on s.oid = c.relnamespace \
             where s.nspname not in ('pg_catalog', 'pg_toast', 'information_schema') \
-                  and s.nspname not like 'pg_temp%';" | sed -n 3p` -eq 0 ]; then
+            and s.nspname not like 'pg_temp%';")" || error_code=$?
+        if [ "''${error_code:-0}" -ne 0 ]; then
+          echo "Failure checking if database is seeded. psql gave exit code $error_code"
+          exit "$error_code"
+        fi
+        if [ "$result" -eq 0 ]; then
           echo "Seeding database"
           rails db:setup
           # SAFETY_ASSURED=1 rails db:schema:load
           rails db:seed
         else
-            echo "Migrating database (this might be a noop)"
+          # echo "Migrating database (this might be a noop)"
-            # TODO: this breaks for some reason
           # rails db:migrate
         fi
       '';