nixos/plausible: Fix shell scripting errors, runtime fixes

[nh2]

Motivation for this change

See https://github.com/NixOS/nixpkgs/pull/124055/files#r668271575

Fixes script continuing on fatal errors instead of aborting.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • CI will show if they pass.
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• 21.11 Release Notes (or backporting 21.05 Relase notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[happysalada]

Thank you for this!
Should we also add assertions that the necessary variables are set in the config? Just to provide a friendlier error message.

Out of curiosity what was your failure case? Did you not set those conf variables or was it something else?

[Ma27]

@ofborg test plausible

[nh2]

Should we also add assertions that the necessary variables are set in the config? Just to provide a friendlier error message.

@happysalada I'm not sure what you mean, assertions on nix variables or shell variables?

For the shell variables, this PR adds set -u, so it'll complain e.g. MYVAR: unbound variable if an undefined shell variable is accessed.

Out of curiosity what was your failure case? Did you not set those conf variables or was it something else?

The problem from #124055 (comment): I didn't put my secrets into the nix store, where they are world-readable. As a result, the DynamicUser couldn't read them, but the script continued without giving good error messages.

[happysalada]

I was thinking of a nix assert. But in your case you don't want to put those variables in the nix store right? So no assertions is fine probably.

Out of curiosity what solution did you end up with ? Something like agenix or sops2nix ?

[nh2]

Out of curiosity what solution did you end up with ?

@happysalada I changed the module for now to not use DynamicUser.

[nh2]

@Ma27 @happysalada I pushed 2 more commits, one that fixes that the patch introduced in https://github.com/NixOS/nixpkgs/pull/124055/files#diff-17ac0fd74e68fbbd03f80f2621d691d3f120a776537bae768bc2206de9559063R31 completely breaks Plausible because it's incompatible with the version of ecto_sql that plausible pins in its lockfile.

[Ma27]

Please wait with merging, I'd like to take a closer look at this today!

[happysalada]

instead of adding a patch,, how about updating plausible to an unstable version ? (trying to use the current master for example)
There are a couple of things that work differently, but I would be happy to details the changes (the ones I followed).

[happysalada]

An additional info regarding this. Upstream provides now a secure way to read secrets, you can now specify a configuration folder and the which file the secret should be read from.
Appart from that, I don't think much has changed. If you are still against an update to an unstable version, no worries.

[Ma27]

<xref xlink:href="..." /> should be sufficient here.

[Ma27]

@nh2 I have to admit that I somehow lost track of what your issues are specifically after reading something in multiple threads/conversations. Are the fixes here plus #130101 sufficient?

Also regarding 1656e8e: can you elaborate what exactly went wrong there? I mean, the createdb.sh somehow succeeded still. I mean, I believe you, but I'd really like to fully understand what went wrong there :D

[nh2]

I have to admit that I somehow lost track of what your issues are specifically after reading something in multiple threads/conversations. Are the fixes here plus #130101 sufficient?

@Ma27 Yes. Concretely, there were 3 issues I encountered in this order:

1. Clickhouse is not available on cache.nixos.org (clickhouse does not get built by Hydra #130101).
2. Our shell scripts not erroring on failure, making it hard to debug why things aren't working (fixed by 8613698 in this PR)
3. migrate.sh failing to set up the clickhouse schema for plausible. Because the nixpkgs patch bumped the ecto_sql dependency to 3.6, which resulted in a call to function lock_for_migrations. But this function does not exist in the clickhouse_ecto version pinned by plausible-1.3.0. Thus, migrate.sh crashes with ClickhouseEcto.lock_for_migrations/3 is undefined or private. (Fixed by 6de23b9 in this PR)
   lock_for_migrations

I mean, the createdb.sh somehow succeeded still.

I made a mistake in the commit message: It is migrate.sh that does the nonexistent function call, not createdb.sh. I've just force-pushed to fix the commit message.

I believe it should be impossible to deploy a working Plausible service on 6de23b983d2659d1a4782db4ea582bdfb52eb5b5^ (the commit before my fix), or on nixos-unstable, with the Plausible version packaged there.

Because that function that's called there, lock_for_migrations, does not exist, so Elixir crashes.

I have verified that using:

cd /nix/store/a9scafm69xld0hnl3vhvzrqb08zbfhk8-plausible-1.3.0/lib/clickhouse_ecto-0.2.8/ebin/
iex
import Elixir.ClickhouseEcto
exports Elixir.ClickhouseEcto

This prints the functions in the module, and lock_for_migrations/3 is not among them in nixos-unstable.

If you run the NixOS tests on current nixos-unstable, it does indeed show the failing function call:

NIX_PATH=nixpkgs=. nix-build --no-link nixos/tests/plausible.nix

Output:

machine # [   29.129497] dmqx3vgkg844pjdvgw0adx41lckzip01-plausible-setup[1086]: ** (UndefinedFunctionError) function ClickhouseEcto.lock_for_migrations/3 is undefined or private

So the conclusion here is that because of the lack of effective set -e the nixpkgs pre-start scripts, they just continued even though migrate.sh failed, and the checks that the NixOS VM test currently does work anyway, even though the Clickhouse DB is not properly migrated.

I'd really like to fully understand what went wrong there :D

Yes, full understanding is usually the best approach. 👍

Hope the above helps clear it up!

[nh2]

Another related topic you may want to be aware of, also involving bash issues:

#130244

I have found the options to set the plausible listen IPs, and will make a PR for it soon.

[Ma27]

I don't think it's reasonable to delay this even more. The current changes solve an actual problem, so I'm going to merge this.
If you prefer to go to an unstable version now rather than waiting for an upstream-release, I'd be fine with that.