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

Thu May 12 12:33:09 UTC 2016

On 12.05.2016 09:09, Peter Krempa wrote:
> 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 at redhat.com>
>> ---
>>  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.

Okay.

> 
>> +
>> +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?

I guess not. I can remove it.

> 
>> +    } 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.
> 

Correct. There's still plenty of rules to add. That's why the perl is
not returning any error. Not just yet.

Michal