nixos/confinement: Allow to include the full unit

From @edolstra at [1]:

  BTW we probably should take the closure of the whole unit rather than
  just the exec commands, to handle things like Environment variables.

With this commit, there is now a "fullUnit" option, which can be enabled
to include the full closure of the service unit into the chroot.

However, I did not enable this by default, because I do disagree here
and *especially* things like environment variables or environment files
shouldn't be in the closure of the chroot.

For example if you have something like:

  { pkgs, ... }:

  {
    systemd.services.foobar = {
      serviceConfig.EnvironmentFile = ${pkgs.writeText "secrets" ''
        user=admin
        password=abcdefg
      '';
    };
  }

We really do not want the *file* to end up in the chroot, but rather
just the environment variables to be exported.

Another thing is that this makes it less predictable what actually will
end up in the chroot, because we have a "globalEnvironment" option that
will get merged in as well, so users adding stuff to that option will
also make it available in confined units.

I also added a big fat warning about that in the description of the
fullUnit option.

[1]: https://github.com/NixOS/nixpkgs/pull/57519#issuecomment-472855704

Signed-off-by: aszlig <aszlig@nix.build>
This commit is contained in:
aszlig 2019-03-14 19:48:20 +01:00
parent 46f7dd436f
commit 9e9af4f9c0
No known key found for this signature in database
GPG key ID: 684089CE67EBB691
2 changed files with 37 additions and 3 deletions

View file

@ -21,6 +21,22 @@ in {
'';
};
options.confinement.fullUnit = lib.mkOption {
type = types.bool;
default = false;
description = ''
Whether to include the full closure of the systemd unit file into the
chroot, instead of just the dependencies for the executables.
<warning><para>While it may be tempting to just enable this option to
make things work quickly, please be aware that this might add paths
to the closure of the chroot that you didn't anticipate. It's better
to use <option>confinement.packages</option> to <emphasis
role="strong">explicitly</emphasis> add additional store paths to the
chroot.</para></warning>
'';
};
options.confinement.packages = lib.mkOption {
type = types.listOf (types.either types.str types.package);
default = [];
@ -32,7 +48,9 @@ in {
${lib.concatMapStringsSep ", " mkScOption [
"ExecReload" "ExecStartPost" "ExecStartPre" "ExecStop"
"ExecStopPost"
]} and ${mkScOption "ExecStart"} options.
]} and ${mkScOption "ExecStart"} options. If you want to have all the
dependencies of this systemd unit, you can use
<option>confinement.fullUnit</option>.
<note><para><emphasis role="strong">Only</emphasis> the latter
(${mkScOption "ExecStart"}) will be used if
@ -87,7 +105,7 @@ in {
config = let
rootName = "${mkPathSafeName name}-chroot";
inherit (config.confinement) binSh;
inherit (config.confinement) binSh fullUnit;
in lib.mkIf config.confinement.enable {
serviceConfig = {
RootDirectory = pkgs.runCommand rootName {} "mkdir \"$out\"";
@ -111,7 +129,10 @@ in {
execPkgs = lib.concatMap (opt: let
isSet = config.serviceConfig ? ${opt};
in lib.optional isSet config.serviceConfig.${opt}) execOpts;
in execPkgs ++ lib.optional (binSh != null) binSh;
unitAttrs = toplevelConfig.systemd.units."${name}.service";
allPkgs = lib.singleton (builtins.toJSON unitAttrs);
unitPkgs = if fullUnit then allPkgs else execPkgs;
in unitPkgs ++ lib.optional (binSh != null) binSh;
};
}));
};

View file

@ -132,6 +132,19 @@ import ./make-test.nix {
);
'';
}
{ description = "check if only Exec* dependencies are included";
config.environment.FOOBAR = pkgs.writeText "foobar" "eek\n";
testScript = ''
$machine->succeed('test "$(chroot-exec \'cat "$FOOBAR"\')" != eek');
'';
}
{ description = "check if all unit dependencies are included";
config.environment.FOOBAR = pkgs.writeText "foobar" "eek\n";
config.confinement.fullUnit = true;
testScript = ''
$machine->succeed('test "$(chroot-exec \'cat "$FOOBAR"\')" = eek');
'';
}
];
options.__testSteps = lib.mkOption {