nixos/borgbackup: fix env vars not being passed verbatim in job wrappers

[erikarvstedt]

Copy of commit msg

Like in the borg-job systemd service, env var values are now passed verbatim to the borg process.
Previously, the var values were evaluated as bash double quoted strings in the Nix build environment for the job wrapper .

Test this with:

read -d '' container <<'EOF' || :
{
  containers.borg-test = {
    config = {
      documentation.enable = false;
      services.borgbackup.jobs.main = {
        startAt = [];
        paths = [ "/" ];
        repo = "myrepo";
        encryption.mode = "none";
        environment.BORG_REMOTE_PATH = "$HOME/bin/borg";
      };
    };
  };
}
EOF
extra-container shell --nixpkgs-path /home/main/d/nix-dev/nixpkgs -E "$container" --run c bash -c 'cat $(which borg-job-main)' | grep BORG_REMOTE_PATH

Output before this PR:

export BORG_REMOTE_PATH='/homeless-shelter/bin/borg'

Output after this PR:

export BORG_REMOTE_PATH='$HOME/bin/borg'

[erikarvstedt]

@ofborg test borgbackup

[pennae]

this will definitely need a changelog entry, just in case anyone does rely on expansion for envs. escaping these does look like a good idea in general (can't think of anything that would need env vars to be expanded like this).

[erikarvstedt]

I've added a changelog entry.

Example: For cheap VPS nodes that only allow access to a $HOME directory, one would install borg in $HOME and set BORG_REMOTE_PATH = "$HOME/path/to/bin/borg".

[pennae]

Example: For cheap VPS nodes that only allow access to a $HOME directory, one would install borg in $HOME and set BORG_REMOTE_PATH = "$HOME/path/to/bin/borg".

but the builder does not have access to the correct $HOME expansion for the target system, and even if it did borg itself does not expand its variables so it'd either see something completely wrong (as you noted) or not fully expanded. or am i missing something?

[erikarvstedt]

BORG_REMOTE_PATH is evaluated via SSH in the remote context, so $HOME expands correctly.

[pennae]

right, that makes sense then.

[pennae]

while testing this we noticed that systemd specifiers in the environment remain unescaped in the generated unit, giving another source of differences between the unit and the job wrapper. that'll also need to be solved somehow, but it looks like that would be a multi-year effort since it'd have to touch unit file generation 😕

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/695

[erikarvstedt]

Rebased to master.

This rebase can be reviewed with:

git fetch https://github.com/NixOS/nixpkgs 77382a5a242d72c4f4e60590aeadd6bb55fa5bcf
git range-diff FETCH_HEAD...HEAD

[SuperSandro2000]

We want to move this to the 23.05 release notes