Merge pull request #97023 from Infinisil/module-assertions

Module-builtin assertions, disabling assertions and submodule assertions
This commit is contained in:
Silvan Mosberger 2020-12-18 14:17:52 +01:00 committed by GitHub
commit 7698aa9776
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 442 additions and 96 deletions

View File

@ -46,6 +46,7 @@ let
showFiles
showOption
unknownModule
literalExample
;
in
@ -72,14 +73,20 @@ rec {
check ? true
}:
let
# This internal module declare internal options under the `_module'
# attribute. These options are fragile, as they are used by the
# module system to change the interpretation of modules.
# An internal module that's always added, defining special options which
# change the behavior of the module evaluation itself. This is under a
# `_`-prefixed namespace in order to prevent name clashes with
# user-defined options
internalModule = rec {
_file = ./modules.nix;
# FIXME: Using ./modules.nix directly breaks the doc for some reason
_file = "lib/modules.nix";
key = _file;
# Most of these options are set to be internal only for prefix != [],
# aka it's a submodule evaluation. This way their docs are displayed
# only once as a top-level NixOS option, but will be hidden for all
# submodules, even though they are available there too
options = {
_module.args = mkOption {
# Because things like `mkIf` are entirely useless for
@ -89,7 +96,7 @@ rec {
# a `_module.args.pkgs = import (fetchTarball { ... }) {}` won't
# start a download when `pkgs` wasn't evaluated.
type = types.lazyAttrsOf types.unspecified;
internal = true;
internal = prefix != [];
description = "Arguments passed to each module.";
};
@ -97,13 +104,19 @@ rec {
type = types.bool;
internal = true;
default = check;
description = "Whether to check whether all option definitions have matching declarations.";
description = ''
Whether to check whether all option definitions have matching
declarations.
Note that this has nothing to do with the similarly named
<option>_module.checks</option> option
'';
};
_module.freeformType = mkOption {
# Disallow merging for now, but could be implemented nicely with a `types.optionType`
type = types.nullOr (types.uniq types.attrs);
internal = true;
internal = prefix != [];
default = null;
description = ''
If set, merge all definitions that don't have an associated option
@ -116,6 +129,75 @@ rec {
turned off.
'';
};
_module.checks = mkOption {
description = ''
Evaluation checks to trigger during module evaluation. The
attribute name will be displayed when it is triggered, allowing
users to disable/change these checks if necessary. See
the section on Warnings and Assertions in the manual for more
information.
'';
example = literalExample ''
{
gpgSshAgent = {
enable = config.programs.gnupg.agent.enableSSHSupport && config.programs.ssh.startAgent;
message = "You can't use ssh-agent and GnuPG agent with SSH support enabled at the same time!";
};
grafanaPassword = {
enable = config.services.grafana.database.password != "";
message = "Grafana passwords will be stored as plaintext in the Nix store!";
type = "warning";
};
}
'';
default = {};
internal = prefix != [];
type = types.attrsOf (types.submodule {
options.enable = mkOption {
description = ''
Whether to enable this check. Set this to false to not trigger
any errors or warning messages. This is useful for ignoring a
check in case it doesn't make sense in certain scenarios.
'';
default = true;
type = types.bool;
};
options.check = mkOption {
description = ''
The condition that must succeed in order for this check to be
successful and not trigger a warning or error.
'';
readOnly = true;
type = types.bool;
};
options.type = mkOption {
description = ''
The type of the check. The default
<literal>"error"</literal> type will cause evaluation to fail,
while the <literal>"warning"</literal> type will only show a
warning.
'';
type = types.enum [ "error" "warning" ];
default = "error";
example = "warning";
};
options.message = mkOption {
description = ''
The message to display if this check triggers.
To display option names in the message, add
<literal>options</literal> to the module function arguments
and use <literal>''${options.path.to.option}</literal>.
'';
type = types.str;
example = "Enabling both \${options.services.foo.enable} and \${options.services.bar.enable} is not possible.";
};
});
};
};
config = {
@ -154,6 +236,35 @@ rec {
# paths, meaning recursiveUpdate will never override any value
else recursiveUpdate freeformConfig declaredConfig;
# Triggers all checks defined by _module.checks before returning its argument
triggerChecks = let
handleCheck = errors: name:
let
value = config._module.checks.${name};
show =
# Assertions with a _ prefix aren't meant to be configurable
if lib.hasPrefix "_" name then value.message
else "[${showOption prefix}${optionalString (prefix != []) "/"}${name}] ${value.message}";
in
if value.enable -> value.check then errors
else if value.type == "warning" then lib.warn show errors
else if value.type == "error" then errors ++ [ show ]
else abort "Unknown check type ${value.type}";
errors = lib.foldl' handleCheck [] (lib.attrNames config._module.checks);
errorMessage = ''
Failed checks:
${lib.concatMapStringsSep "\n" (a: "- ${a}") errors}
'';
trigger = if errors == [] then null else throw errorMessage;
in builtins.seq trigger;
finalConfig = triggerChecks (removeAttrs config [ "_module" ]);
checkUnmatched =
if config._module.check && config._module.freeformType == null && merged.unmatchedDefns != [] then
let
@ -173,7 +284,7 @@ rec {
result = builtins.seq checkUnmatched {
inherit options;
config = removeAttrs config [ "_module" ];
config = finalConfig;
inherit (config) _module;
};
in result;
@ -514,6 +625,8 @@ rec {
definitions = map (def: def.value) res.defsFinal;
files = map (def: def.file) res.defsFinal;
inherit (res) isDefined;
# This allows options to be correctly displayed using `${options.path.to.it}`
__toString = _: showOption loc;
};
# Merge definitions of a value of a given type.
@ -773,14 +886,15 @@ rec {
visible = false;
apply = x: throw "The option `${showOption optionName}' can no longer be used since it's been removed. ${replacementInstructions}";
});
config.assertions =
let opt = getAttrFromPath optionName options; in [{
assertion = !opt.isDefined;
config._module.checks =
let opt = getAttrFromPath optionName options; in {
${"removed-" + showOption optionName} = lib.mkIf opt.isDefined {
message = ''
The option definition `${showOption optionName}' in ${showFiles opt.files} no longer has any effect; please remove it.
${replacementInstructions}
'';
}];
};
};
};
/* Return a module that causes a warning to be shown if the
@ -841,14 +955,18 @@ rec {
})) from);
config = {
warnings = filter (x: x != "") (map (f:
let val = getAttrFromPath f config;
opt = getAttrFromPath f options;
in
optionalString
(val != "_mkMergedOptionModule")
"The option `${showOption f}' defined in ${showFiles opt.files} has been changed to `${showOption to}' that has a different type. Please read `${showOption to}' documentation and update your configuration accordingly."
) from);
_module.checks =
let warningMessages = map (f:
let val = getAttrFromPath f config;
opt = getAttrFromPath f options;
in {
${"merged" + showOption f} = lib.mkIf (val != "_mkMergedOptionModule") {
type = "warning";
message = "The option `${showOption f}' defined in ${showFiles opt.files} has been changed to `${showOption to}' that has a different type. Please read `${showOption to}' documentation and update your configuration accordingly.";
};
}
) from;
in mkMerge warningMessages;
} // setAttrByPath to (mkMerge
(optional
(any (f: (getAttrFromPath f config) != "_mkMergedOptionModule") from)
@ -907,8 +1025,10 @@ rec {
});
config = mkMerge [
{
warnings = optional (warn && fromOpt.isDefined)
"The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'.";
_module.checks.${"renamed-" + showOption from} = mkIf (warn && fromOpt.isDefined) {
type = "warning";
message = "The option `${showOption from}' defined in ${showFiles fromOpt.files} has been renamed to `${showOption to}'.";
};
}
(if withPriority
then mkAliasAndWrapDefsWithPriority (setAttrByPath to) fromOpt

View File

@ -192,7 +192,7 @@ rec {
let ss = opt.type.getSubOptions opt.loc;
in if ss != {} then optionAttrSetToDocList' opt.loc ss else [];
in
[ docOption ] ++ optionals docOption.visible subOptions) (collect isOption options);
[ docOption ] ++ optionals (docOption.visible && ! docOption.internal) subOptions) (collect isOption options);
/* This function recursively removes all derivation attributes from

View File

@ -655,7 +655,7 @@ runTests {
modules = [ module ];
}).options;
locs = filter (o: ! o.internal) (optionAttrSetToDocList options);
locs = filter (o: ! o.internal) (optionAttrSetToDocList (removeAttrs options [ "_module" ]));
in map (o: o.loc) locs;
expected = [ [ "foo" ] [ "foo" "<name>" "bar" ] [ "foo" "bar" ] ];
};

View File

@ -27,37 +27,50 @@ reportFailure() {
fail=$((fail + 1))
}
checkConfigOutput() {
checkConfigCodeOutErr() {
local expectedExit=$1
shift;
local outputContains=$1
shift;
if evalConfig "$@" 2>/dev/null | grep --silent "$outputContains" ; then
pass=$((pass + 1))
return 0;
else
echo 2>&1 "error: Expected result matching '$outputContains', while evaluating"
local errorContains=$1
shift;
out=$(mktemp)
err=$(mktemp)
evalConfig "$@" 1>"$out" 2>"$err"
if [[ "$?" -ne "$expectedExit" ]]; then
echo 2>&1 "error: Expected exit code $expectedExit while evaluating"
reportFailure "$@"
return 1
fi
if [[ -n "$outputContains" ]] && ! grep -zP --silent "$outputContains" "$out"; then
echo 2>&1 "error: Expected output matching '$outputContains', while evaluating"
reportFailure "$@"
return 1
fi
if [[ -n "$errorContains" ]] && ! grep -zP --silent "$errorContains" "$err"; then
echo 2>&1 "error: Expected error matching '$errorContains', while evaluating"
reportFailure "$@"
return 1
fi
pass=$((pass + 1))
# Clean up temp files
rm "$out" "$err"
}
checkConfigOutput() {
local outputContains=$1
shift;
checkConfigCodeOutErr 0 "$outputContains" "" "$@"
}
checkConfigError() {
local errorContains=$1
local err=""
shift;
if err==$(evalConfig "$@" 2>&1 >/dev/null); then
echo 2>&1 "error: Expected error code, got exit code 0, while evaluating"
reportFailure "$@"
return 1
else
if echo "$err" | grep -zP --silent "$errorContains" ; then
pass=$((pass + 1))
return 0;
else
echo 2>&1 "error: Expected error matching '$errorContains', while evaluating"
reportFailure "$@"
return 1
fi
fi
checkConfigCodeOutErr 1 "" "$errorContains" "$@"
}
# Check boolean option.
@ -262,6 +275,29 @@ checkConfigOutput true config.value.mkbefore ./types-anything/mk-mods.nix
checkConfigOutput 1 config.value.nested.foo ./types-anything/mk-mods.nix
checkConfigOutput baz config.value.nested.bar.baz ./types-anything/mk-mods.nix
## Module assertions
# Check that assertions are triggered by default for just evaluating config
checkConfigError 'Failed checks:\n- \[test\] Assertion failed' config ./assertions/simple.nix
# Assertion is not triggered when enable is false or condition is true
checkConfigOutput '{ }' config ./assertions/condition-true.nix
checkConfigOutput '{ }' config ./assertions/enable-false.nix
# Warnings should be displayed on standard error
checkConfigCodeOutErr 0 '{ }' 'warning: \[test\] Warning message' config ./assertions/warning.nix
# Check that multiple assertions and warnings can be triggered at once
checkConfigError 'Failed checks:\n- \[test1\] Assertion 1 failed\n- \[test2\] Assertion 2 failed' config ./assertions/multi.nix
checkConfigError 'trace: warning: \[test3\] Warning 3 failed\ntrace: warning: \[test4\] Warning 4 failed' config ./assertions/multi.nix
# Submodules should be able to trigger assertions and display the submodule prefix in their error
checkConfigError 'Failed checks:\n- \[foo/test\] Assertion failed' config.foo ./assertions/submodule.nix
checkConfigError 'Failed checks:\n- \[foo.bar/test\] Assertion failed' config.foo.bar ./assertions/submodule-attrsOf.nix
checkConfigError 'Failed checks:\n- \[foo.bar.baz/test\] Assertion failed' config.foo.bar.baz ./assertions/submodule-attrsOf-attrsOf.nix
# Assertions with an attribute starting with _ shouldn't have their name displayed
checkConfigError 'Failed checks:\n- Assertion failed' config ./assertions/underscore-attributes.nix
cat <<EOF
====== module tests ======
$pass Pass

View File

@ -0,0 +1,8 @@
{
_module.checks.test = {
check = true;
message = "Assertion failed";
};
}

View File

@ -0,0 +1,9 @@
{
_module.checks.test = {
enable = false;
check = false;
message = "Assertion failed";
};
}

View File

@ -0,0 +1,23 @@
{
_module.checks = {
test1 = {
check = false;
message = "Assertion 1 failed";
};
test2 = {
check = false;
message = "Assertion 2 failed";
};
test3 = {
check = false;
message = "Warning 3 failed";
type = "warning";
};
test4 = {
check = false;
message = "Warning 4 failed";
type = "warning";
};
};
}

View File

@ -0,0 +1,6 @@
{
_module.checks.test = {
check = false;
message = "Assertion failed";
};
}

View File

@ -0,0 +1,13 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = { bar.baz = {}; };
type = lib.types.attrsOf (lib.types.attrsOf (lib.types.submodule {
_module.checks.test = {
check = false;
message = "Assertion failed";
};
}));
};
}

View File

@ -0,0 +1,13 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = { bar = {}; };
type = lib.types.attrsOf (lib.types.submodule {
_module.checks.test = {
check = false;
message = "Assertion failed";
};
});
};
}

View File

@ -0,0 +1,13 @@
{ lib, ... }: {
options.foo = lib.mkOption {
default = {};
type = lib.types.submodule {
_module.checks.test = {
check = false;
message = "Assertion failed";
};
};
};
}

View File

@ -0,0 +1,8 @@
{
_module.checks._test = {
check = false;
message = "Assertion failed";
};
}

View File

@ -0,0 +1,9 @@
{
_module.checks.test = {
check = false;
type = "warning";
message = "Warning message";
};
}

View File

@ -3,72 +3,155 @@
xmlns:xi="http://www.w3.org/2001/XInclude"
version="5.0"
xml:id="sec-assertions">
<title>Warnings and Assertions</title>
<title>Evaluation Checks</title>
<para>
When configuration problems are detectable in a module, it is a good idea to
write an assertion or warning. Doing so provides clear feedback to the user
and prevents errors after the build.
write a check for catching it early. Doing so can provide clear feedback to
the user and can prevent errors before the build.
</para>
<para>
Although Nix has the <literal>abort</literal> and
<literal>builtins.trace</literal>
<link xlink:href="https://nixos.org/nix/manual/#ssec-builtins">functions</link>
to perform such tasks, they are not ideally suited for NixOS modules. Instead
of these functions, you can declare your warnings and assertions using the
NixOS module system.
to perform such tasks generally, they are not ideally suited for NixOS
modules. Instead of these functions, you can declare your evaluation checks
using the NixOS module system.
</para>
<section xml:id="sec-assertions-warnings">
<title>Warnings</title>
<section xml:id="sec-assertions-define">
<title>Defining Checks</title>
<para>
This is an example of using <literal>warnings</literal>.
Checks can be defined using the <xref linkend="opt-_module.checks"/> option.
Each check needs an attribute name, under which you can define a trigger
assertion using <xref linkend="opt-_module.checks._name_.check"/> and a
message using <xref linkend="opt-_module.checks._name_.message"/>.
For the message, you can add
<literal>options</literal> to the module arguments and use
<literal>${options.path.to.option}</literal> to print a context-aware string
representation of an option path. Here is an example showing how this can be
done.
</para>
<programlisting>
<![CDATA[
{ config, lib, ... }:
{
config = lib.mkIf config.services.foo.enable {
warnings =
if config.services.foo.bar
then [ ''You have enabled the bar feature of the foo service.
This is known to cause some specific problems in certain situations.
'' ]
else [];
}
{ config, options, ... }: {
_module.checks.gpgSshAgent = {
check = config.programs.gnupg.agent.enableSSHSupport -> !config.programs.ssh.startAgent;
message = "If you have ${options.programs.gnupg.agent.enableSSHSupport} enabled,"
+ " you can't enable ${options.programs.ssh.startAgent} as well!";
};
_module.checks.grafanaPassword = {
check = config.services.grafana.database.password == "";
message = "The grafana password defined with ${options.services.grafana.database.password}"
+ " will be stored as plaintext in the Nix store!";
# This is a non-fatal warning
type = "warning";
};
}
]]>
</programlisting>
</section>
<section xml:id="sec-assertions-assertions">
<title>Assertions</title>
<section xml:id="sec-assertions-ignoring">
<title>Ignoring Checks</title>
<para>
This example, extracted from the
<link xlink:href="https://github.com/NixOS/nixpkgs/blob/release-17.09/nixos/modules/services/logging/syslogd.nix">
<literal>syslogd</literal> module </link> shows how to use
<literal>assertions</literal>. Since there can only be one active syslog
daemon at a time, an assertion is useful to prevent such a broken system
from being built.
Sometimes you can get failing checks that don't apply to your specific case
and you wish to ignore them, or at least make errors non-fatal. You can do so
for all checks defined using <xref linkend="opt-_module.checks"/> by
using the attribute name of the definition, which is conveniently printed
using <literal>[...]</literal> when the check is triggered. For above
example, the evaluation output when the checks are triggered looks as
follows:
</para>
<programlisting>
<![CDATA[
{ config, lib, ... }:
{
config = lib.mkIf config.services.syslogd.enable {
assertions =
[ { assertion = !config.services.rsyslogd.enable;
message = "rsyslogd conflicts with syslogd";
}
];
}
}
]]>
trace: warning: [grafanaPassword] The grafana password defined with
services.grafana.database.password will be stored as plaintext in the Nix store!
error: Failed checks:
- [gpgSshAgent] If you have programs.gnupg.agent.enableSSHSupport
enabled, you can't enable programs.ssh.startAgent as well!
</programlisting>
<para>
The <literal>[grafanaPassword]</literal> and <literal>[gpgSshAgent]</literal>
strings tell you that these were defined under the <literal>grafanaPassword
</literal> and <literal>gpgSshAgent</literal> attributes of
<xref linkend="opt-_module.checks"/> respectively. With this knowledge
you can adjust them to your liking:
</para>
<programlisting>
{
# Change the error into a non-fatal warning
_module.checks.gpgSshAgent.type = "warning";
# We don't care about this warning, disable it
_module.checks.grafanaPassword.enable = false;
}
</programlisting>
</section>
<section xml:id="sec-assertions-submodules">
<title>Checks in Submodules</title>
<para>
Evaluation checks can be defined within submodules in the same way. Here is an example:
</para>
<programlisting>
{ lib, ... }: {
options.myServices = lib.mkOption {
type = lib.types.attrsOf (lib.types.submodule ({ config, options, ... }: {
options.port = lib.mkOption {};
config._module.checks.portConflict = {
check = config.port != 80;
message = "Port ${toString config.port} defined using"
+ " ${options.port} is usually used for HTTP";
type = "warning";
};
}));
};
}
</programlisting>
<para>
When this check is triggered, it shows both the submodule path along with
the check attribute within that submodule, joined by a
<literal>/</literal>. Note also how <literal>${options.port}</literal>
correctly shows the context of the option.
</para>
<programlisting>
trace: warning: [myServices.foo/portConflict] Port 80 defined using
myServices.foo.port is usually used for HTTP
</programlisting>
<para>
Therefore to disable such a check, you can do so by changing the
<xref linkend="opt-_module.checks"/> option within the
<literal>myServices.foo</literal> submodule:
</para>
<programlisting>
{
myServices.foo._module.checks.portConflict.enable = false;
}
</programlisting>
<note>
<para>
Checks defined in submodules under <literal>types.listOf</literal> can't be
ignored, since there's no way to change previously defined list items.
</para>
</note>
</section>
</section>

View File

@ -1,4 +1,4 @@
{ lib, ... }:
{ lib, config, ... }:
with lib;
@ -30,5 +30,18 @@ with lib;
};
};
config._module.checks = lib.listToAttrs (lib.imap1 (n: value:
let
name = "_${toString n}";
isWarning = lib.isString value;
result = {
check = if isWarning then false else value.assertion;
type = if isWarning then "warning" else "error";
message = if isWarning then value else value.message;
};
in nameValuePair name result
) (config.assertions ++ config.warnings));
# impl of assertions is in <nixpkgs/nixos/modules/system/activation/top-level.nix>
}

View File

@ -117,18 +117,10 @@ let
perl = "${pkgs.perl}/bin/perl " + (concatMapStringsSep " " (lib: "-I${lib}/${pkgs.perl.libPrefix}") (with pkgs.perlPackages; [ FileSlurp NetDBus XMLParser XMLTwig ]));
};
# Handle assertions and warnings
failedAssertions = map (x: x.message) (filter (x: !x.assertion) config.assertions);
baseSystemAssertWarn = if failedAssertions != []
then throw "\nFailed assertions:\n${concatStringsSep "\n" (map (x: "- ${x}") failedAssertions)}"
else showWarnings config.warnings baseSystem;
# Replace runtime dependencies
system = fold ({ oldDependency, newDependency }: drv:
pkgs.replaceDependency { inherit oldDependency newDependency drv; }
) baseSystemAssertWarn config.system.replaceRuntimeDependencies;
) baseSystem config.system.replaceRuntimeDependencies;
in