b12f/nixpkgs
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

logrotate: add configuration check at build time

Browse source 
Now the service no longer starts immediately,
check if the config we generated makes sense as soon as possible.

The check isn't perfect because logrotate --debug wants to check
users required, there are two problems:
 - /etc/passwd and /etc/group are sandboxed and we don't have
visibility of system users
 - the check phase runs as nixbld which cannot su to other users
and logrotate fails on this

Until these two problems can be addressed, users-related checks
are filtered out, it's still much better than no check.
The check can be disabled with services.logrotate.checkConfig
if required
(bird also has a preCheck param, to prepare the environment
before check, but we can add it if it becomes necessary)

Since this makes for very verbose builds, we only show errors:
There is no way to control log level, but logrotate hardcodes
'error:' at common log level, so we can use grep, taking care
to keep error codes

Some manual tests:
───────┬──────────────────────────────────────────
       │ File: valid-config.conf
───────┼──────────────────────────────────────────
   1   │ missingok
───────┴──────────────────────────────────────────
logrotate --debug ok
grep ok

───────┬──────────────────────────────────────────
       │ File: postrotate-no-end.conf
───────┼──────────────────────────────────────────
   1   │ missingok
   2   │ /file {
   3   │    postrotate
   4   │      test
   5   │ }
───────┴──────────────────────────────────────────
error: postrotate-no-end.conf:prerotate, postrotate or preremove without endscript

───────┬──────────────────────────────────────────
       │ File: missing-file.conf
───────┼──────────────────────────────────────────
   1   │ "test" { daily }
───────┴──────────────────────────────────────────
error: stat of test failed: No such file or directory

───────┬──────────────────────────────────────────
       │ File: unknown-option.conf
───────┼──────────────────────────────────────────
   1   │ some syntax error
───────┴──────────────────────────────────────────
logrotate --debug ok
error: unknown-option.conf:1 unknown option 'some' -- ignoring line

───────┬──────────────────────────────────────────
       │ File: unknown-user.conf
───────┼──────────────────────────────────────────
   1   │ su notauser notagroup
───────┴──────────────────────────────────────────
error: unknown-user.conf:1 unknown user 'notauser'

In particular note that logrotate would not error on unknown option
(it just ignores the line) but this change makes the check fail.
This commit is contained in:
Dominique Martinet 2022-03-02 22:12:45 +09:00
parent e92c05349c
commit 45ef5c1741
2 changed files with 64 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
nixos/modules/services/logging/logrotate.nix
View file

@ -152,11 +152,45 @@ let
{ header = { global = true; priority = 100; }; }
]
)));
configFile = pkgs.writeText "logrotate.conf" (
concatStringsSep "\n" (
configFile = pkgs.writeTextFile {
name = "logrotate.conf";
text = concatStringsSep "\n" (
map mkConf settings
)
);
);
checkPhase = optionalString cfg.checkConfig ''
# logrotate --debug also checks that users specified in config
# file exist, but we only have sandboxed users here so brown these
# out. according to man page that means su, create and createolddir.
# files required to exist also won't be present, so missingok is forced.
user=$(${pkgs.coreutils}/bin/id -un)
group=$(${pkgs.coreutils}/bin/id -gn)
sed -e "s/\bsu\s.*/su $user $group/" \
-e "s/\b\(create\s\+[0-9]*\s*\|createolddir\s\+[0-9]*\s\+\).*/\1$user $group/" \
-e "1imissingok" -e "s/\bnomissingok\b//" \
$out > /tmp/logrotate.conf
# Since this makes for very verbose builds only show real error.
# There is no way to control log level, but logrotate hardcodes
# 'error:' at common log level, so we can use grep, taking care
# to keep error codes
set -o pipefail
if ! ${pkgs.logrotate}/sbin/logrotate --debug /tmp/logrotate.conf 2>&1 \
| ( ! grep "error:" ) > /tmp/logrotate-error; then
echo "Logrotate configuration check failed."
echo "The failing configuration (after adjustments to pass tests in sandbox) was:"
printf "%s\n" "-------"
cat /tmp/logrotate.conf
printf "%s\n" "-------"
echo "The error reported by logrotate was as follow:"
printf "%s\n" "-------"
cat /tmp/logrotate-error
printf "%s\n" "-------"
echo "You can disable this check with services.logrotate.checkConfig = false,"
echo "but if you think it should work please report this failure along with"
echo "the config file being tested!"
false
fi
'';
};
mailOption =
if foldr (n: a: a || n ? mail) false (attrValues cfg.settings)
@ -256,6 +290,24 @@ in
'';
};
checkConfig = mkOption {
type = types.bool;
default = true;
description = ''
Whether the config should be checked at build time.
Some options are not checkable at build time because of the build sandbox:
for example, the test does not know about existing files and system users are
not known.
These limitations mean we must adjust the file for tests (missingok is forced
and users are replaced by dummy users).
Conversely there are still things that might make this check fail incorrectly
(e.g. a file path where we don't have access to intermediate directories):
in this case you can disable the failing check with this option.
'';
};
# deprecated legacy compat settings
paths = mkOption {
type = with types; attrsOf (submodule pathOpts);

8
nixos/tests/logrotate.nix
View file

@ -40,6 +40,14 @@ import ./make-test-python.nix ({ pkgs, ... }: rec {
postrotate = {
postrotate = "touch /dev/null";
};
# check checkConfig works as expected: there is nothing to check here
# except that the file build passes
checkConf = {
su = "root utmp";
createolddir = "0750 root utmp";
create = "root utmp";
"create " = "0750 root utmp";
};
# multiple paths should be aggregated
multipath = {
files = [ "file1" "file2" ];