[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]