Re: [libvirt] [PATCH v2 3/3] tests: Introduce check-file-access.pl

On Tue, May 10, 2016 at 17:24:14 +0200, Michal Privoznik wrote: > This script will check output generated by virtestmock against a > white list. All non matching records found are printed out. So > far, the white list is rather sparse at the moment. > This test should be ran only after all other tests finished, and > should cleanup the temporary file before their execution. Because > I'm unable to reflect these requirements in Makefile.am > correctly, I've introduced new target 'check-access' under which > this test is available. > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > Makefile.am | 3 ++ > tests/Makefile.am | 13 +++++ > tests/check-file-access.pl | 106 ++++++++++++++++++++++++++++++++++++++++ > tests/file_access_whitelist.txt | 23 +++++++++ > 4 files changed, 145 insertions(+) > create mode 100755 tests/check-file-access.pl > create mode 100644 tests/file_access_whitelist.txt > > diff --git a/Makefile.am b/Makefile.am > index c52a4cb..da07e6c 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -67,6 +67,9 @@ rpm: clean > > check-local: all tests > > +check-access: > + @($(MAKE) $(AM_MAKEFLAGS) -C tests check-access) > + > cov: clean-cov > $(MKDIR_P) $(top_builddir)/coverage > $(LCOV) -c -o $(top_builddir)/coverage/libvirt.info.tmp \ > diff --git a/tests/Makefile.am b/tests/Makefile.am > index da68f2e..7cd641d 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -451,6 +451,19 @@ test_libraries += virusbmock.la \ > $(NULL) > endif WITH_LINUX > > +if WITH_LINUX > +check-access: file-access-clean > + $(MAKE) $(AM_MAKEFLAGS) check > + $(PERL) check-file-access.pl I added '| sort -u' while trying this. There's a lot of multiplicated lines.

> + > +file-access-clean: > + > test_file_access.txt > +endif WITH_LINUX > + > +EXTRA_DIST += \ > + check-file-access.pl \ > + file_access_whitelist.txt > + > if WITH_TESTS > noinst_PROGRAMS = $(test_programs) $(test_helpers) > noinst_LTLIBRARIES = $(test_libraries) > diff --git a/tests/check-file-access.pl b/tests/check-file-access.pl > new file mode 100755 > index 0000000..6e59201 > --- /dev/null > +++ b/tests/check-file-access.pl > @@ -0,0 +1,106 @@ > +#!/usr/bin/perl -w > +# > +# Copyright (C) 2016 Red Hat, Inc. > +# > +# This library is free software; you can redistribute it and/or > +# modify it under the terms of the GNU Lesser General Public > +# License as published by the Free Software Foundation; either > +# version 2.1 of the License, or (at your option) any later version. > +# > +# This library is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +# Lesser General Public License for more details. > +# > +# You should have received a copy of the GNU Lesser General Public > +# License along with this library. If not, see > +# <http://www.gnu.org/licenses/>;. > +# > +# This script is supposed to check test_file_access.txt file and > +# warn about file accesses outside our working tree. > +# > +# > + > +use strict; > +use warnings; > + > +my $access_file = "test_file_access.txt"; > +my $whitelist_file = "file_access_whitelist.txt"; > + > +my @files; > +my @whitelist; > + > +open FILE, "<", $access_file or die "Unable to open $access_file: $!"; > +while (<FILE>) { > + chomp; > + if (/^\s*#.*$/) { > + # comment Will this file ever contain comments?

> + } elsif (/^(\S*):\s*(\S*)(\s*:\s*(.*))?$/) { > + my %rec; > + ${rec}{path} = $1; > + ${rec}{progname} = $2; > + if (defined $4) { > + ${rec}{testname} = $4; > + } > + push (@files, \%rec); > + } else { > + die "Malformed line $_"; > + } > +} > +close FILE; > + > +open FILE, "<", $whitelist_file or die "Unable to open $whitelist_file: $!"; > +while (<FILE>) { > + chomp; > + if (/^\s*#.*$/) { > + # comment > + } elsif (/^(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/) { > + my %rec; > + ${rec}{path} = $1; > + if (defined $3) { > + ${rec}{progname} = $3; > + } > + if (defined $5) { > + ${rec}{testname} = $5; > + } > + push (@whitelist, \%rec); > + } else { > + die "Malformed line $_"; > + } > +} > +close FILE; > + > +# Now we should check if %traces is included in $whitelist. For > +# now checking just keys is sufficient > +my $error = 0; > +for my $file (@files) { > + my $match = 0; > + > + for my $rule (@whitelist) { > + if (not %${file}{path} =~ m/^$rule->{path}$/) { > + next; > + } > + > + if (defined %${rule}{progname} and > + not %${file}{progname} =~ m/^$rule->{progname}$/) { > + next; > + } > + > + if (defined %${rule}{testname} and > + defined %${file}{testname} and > + not %${file}{testname} =~ m/^$rule->{testname}$/) { > + next; > + } > + > + $match = 1; > + } > + > + if (not $match) { > + $error = 1; > + print "$file->{path}: $file->{progname}"; > + print ": $file->{testname}" if defined %${file}{testname}; > + print "\n"; > + } > +} > + > +$error; perl complains that the above line is useless. Also the script doesn't return failure if it finds files out of the build path. > diff --git a/tests/file_access_whitelist.txt b/tests/file_access_whitelist.txt > new file mode 100644 > index 0000000..850b285 > --- /dev/null > +++ b/tests/file_access_whitelist.txt > @@ -0,0 +1,23 @@ > +# This is a whitelist that allows accesses to files not in our > +# build directory nor source directory. The records are in the > +# following format: > +# > +# $path: $progname: $testname > +# > +# All these three are evaluated as perl RE. So to allow /dev/sda > +# and /dev/sdb, you can just '/dev/sd[a-b]', or to allow > +# /proc/$pid/status you can '/proc/\d+/status' and so on. > +# Moreover, $progname and $testname can be empty, in which which > +# case $path is allowed for all tests. > + > +/bin/cat: sysinfotest > +/bin/dirname: sysinfotest: x86 sysinfo > +/bin/sleep: commandtest > +/bin/true: commandtest > +/dev/null > +/dev/urandom > +/etc/hosts > +/proc/\d+/status My test run showed that there is a looot of stuff to add unfortunately. Looks good to me, but my perl knowledge is rather poor.