Skip to content

Commit

Permalink
Use needles from correct ref of NEEDLES_DIR
Browse files Browse the repository at this point in the history 
If the NEEDLES_DIR (or CASEDIR as a backup) variable is a git repo URL
with a fragment specifying the ref of the repository, we attempt to use
it as the source of needles when viewing needle diffs in test results.

If the NEEDLES_GIT_SHA variable is set, we use that instead as it should
be the most accurate way to determine exactly which commit was used
  • Loading branch information
@sdclarke
sdclarke committed Jun 2, 2023
1 parent cd57805 commit 0953592
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 15 deletions.
1 change: 1 addition & 0 deletions lib/OpenQA/Setup.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ sub read_config ($app) {
update_branch => '',
do_push => 'no',
do_cleanup => 'no',
checkout_needles_sha => 'no',
},
'scheduler' => {
max_job_scheduled_time => 7,
Expand Down
23 changes: 22 additions & 1 deletion lib/OpenQA/Utils.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use OpenQA::Log qw(log_info log_debug log_warning log_error);
use Config::Tiny;
use Time::HiRes qw(tv_interval);
use File::Basename;
use File::Path qw(make_path);
use File::Spec;
use File::Spec::Functions qw(catfile catdir);
use Fcntl;
Expand Down Expand Up @@ -248,7 +249,27 @@ sub is_in_tests {
sub needledir { productdir(@_) . '/needles' }

sub locate_needle {
my ($relative_needle_path, $needles_dir) = @_;
my ($relative_needle_path, $needles_dir, $needles_ref) = @_;

if ($needles_ref) {
my $needles_dir_basename = basename(dirname($needles_dir));
my $temp_needles_dir = "/tmp/$needles_dir_basename/worktrees/$needles_ref/needles";
if (File::Spec->splitdir($relative_needle_path) > 1) {
make_path($temp_needles_dir . '/' . dirname($relative_needle_path));
}
my $temp_json_path = $temp_needles_dir . '/' . $relative_needle_path;
open my $temp_json_fh, '>', $temp_json_path;
my $jsonfile = qx{git -C $needles_dir show $needles_ref:./$relative_needle_path};
print $temp_json_fh $jsonfile;
close $temp_json_fh;
my $png_name = dirname($relative_needle_path) . '/' . basename($relative_needle_path, '.json') . '.png';
my $temp_png_path = $temp_needles_dir . '/' . $png_name;
open my $temp_png_fh, '>', $temp_png_path;
my $pngfile = qx{git -C $needles_dir show $needles_ref:./$png_name};
print $temp_png_fh $pngfile;
close $temp_png_fh;
return $temp_json_path if $? == 0;
}

my $absolute_filename = catdir($needles_dir, $relative_needle_path);
my $needle_exists = -f $absolute_filename;
Expand Down
14 changes: 13 additions & 1 deletion lib/OpenQA/WebAPI/Controller/File.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,23 @@ sub needle ($self) {

# make sure the directory of the file parameter is a real subdir of testcasedir before
# using it to find needle subdirectory, to prevent access outside of the zoo
if ($jsonfile && !is_in_tests($jsonfile)) {
# Also allow the json file to be under /tmp
if ($jsonfile && !is_in_tests($jsonfile) && index($jsonfile, '/tmp') != 0) {
my $prjdir = prjdir();
warn "$jsonfile is not in a subdir of $prjdir/share/tests or $prjdir/tests";
return $self->render(text => 'Forbidden', status => 403);
}
# If the json file in not in the tests we may be using a temporary
# directory for needles from a different git SHA
# Allow only if the jsonfile is under /tmp
if (!is_in_tests($jsonfile) && index($jsonfile, '/tmp') == 0) {
$needledir = dirname($jsonfile);
# In case we're in a subdirectory, keep taking the dirname until we
# have the path of the `needles` directory
while (basename($needledir) ne 'needles') {
$needledir = dirname($needledir);
}
}
# Reject directory traversal breakouts here...
if (index($jsonfile, '..') != -1) {
warn "jsonfile value $jsonfile is invalid, cannot contain ..";
Expand Down
60 changes: 47 additions & 13 deletions lib/OpenQA/WebAPI/Controller/Step.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use Mojo::Util 'decode';
use OpenQA::Utils qw(ensure_timestamp_appended find_bug_number locate_needle needledir testcasedir);
use OpenQA::Jobs::Constants;
use File::Basename;
use File::Path 'make_path';
use File::Which 'which';
use POSIX 'strftime';
use Mojo::JSON 'decode_json';
Expand Down Expand Up @@ -91,6 +92,32 @@ sub view ($self) {
$self->viewimg;
}

sub _create_tmpdir_for_needles_refspec ($self, $job) {
return undef unless $self->app->config->{'scm git'}->{checkout_needles_sha} eq 'yes';
my $needle_dir = $job->needle_dir;
$needle_dir = realpath($needle_dir) // $needle_dir;
my $needles_dir_var = $job->settings->single({key => 'NEEDLES_DIR'});
$needles_dir_var = $job->settings->single({key => 'CASEDIR'}) unless $needles_dir_var;
return undef unless $needles_dir_var;
my $needles_url = Mojo::URL->new($needles_dir_var->value);
return undef unless $needles_url->scheme;
my $needles_ref = $needles_url->fragment;
eval {
my $vars_json = Mojo::File->new($job->result_dir(), 'vars.json')->slurp;
my $vars = decode_json($vars_json);
$needles_ref = $vars->{NEEDLES_GIT_HASH};
};
chomp($needles_ref);
return undef unless $needles_ref;
my $needle_dir_basename = basename(dirname($needle_dir));
my $new_path = "/tmp/$needle_dir_basename/worktrees/$needles_ref/needles";
if (!-d $new_path) {
make_path($new_path);
qx{git -C "$needle_dir" fetch --depth 1 origin "$needles_ref" &>/dev/null};
}
return $needles_ref;
}

# Needle editor
sub edit ($self) {
return $self->reply->not_found unless $self->_init && $self->check_tabmode();
Expand All @@ -101,6 +128,7 @@ sub edit ($self) {
my $distri = $job->DISTRI;
my $dversion = $job->VERSION || '';
my $needle_dir = $job->needle_dir;
my $needle_ref = $self->_create_tmpdir_for_needles_refspec($job);
my $app = $self->app;
my $needles_rs = $app->schema->resultset('Needles');

Expand Down Expand Up @@ -130,7 +158,7 @@ sub edit ($self) {
# Second position: the only needle (with the same matches)
my $needle_info
= $self->_extended_needle_info($needle_dir, $needle_name, \%basic_needle_data, $module_detail->{json},
0, \@error_messages);
0, \@error_messages, $needle_ref);
if ($needle_info) {
$needle_info->{matches} = $screenshot->{matches};
push(@needles, $needle_info);
Expand All @@ -144,10 +172,10 @@ sub edit ($self) {
# $needle contains information from result, in which 'areas' refers to the best matches.
# We also use $area for transforming the match information into a real area
for my $needle (@$module_detail_needles) {
my $needle_info = $self->_extended_needle_info(
$needle_dir, $needle->{name}, \%basic_needle_data,
$needle->{json}, $needle->{error}, \@error_messages
) || next;
my $needle_info
= $self->_extended_needle_info($needle_dir, $needle->{name}, \%basic_needle_data,
$needle->{json}, $needle->{error}, \@error_messages, $needle_ref)
|| next;
my $matches = $needle_info->{matches};
for my $match (@{$needle->{area}}) {
my %area = (
Expand Down Expand Up @@ -188,7 +216,7 @@ sub edit ($self) {
# get needle info to show the needle also in selection
my $needle_info
= $self->_extended_needle_info($needle_dir, $new_needle->name, \%basic_needle_data, $new_needle->path,
undef, \@error_messages)
undef, \@error_messages, $needle_ref)
|| next;
$needle_info->{title} = 'new: ' . $needle_info->{title};
push(@needles, $needle_info);
Expand Down Expand Up @@ -274,9 +302,9 @@ sub _new_screenshot ($self, $tags, $image_name, $matches = undef) {
return \%screenshot;
}

sub _basic_needle_info ($self, $name, $distri, $version, $file_name, $needles_dir) {
sub _basic_needle_info ($self, $name, $distri, $version, $file_name, $needles_dir, $needle_ref) {
$file_name //= "$name.json";
$file_name = locate_needle($file_name, $needles_dir) if !-f $file_name;
$file_name = locate_needle($file_name, $needles_dir, $needle_ref) if !-f $file_name;
return (undef, 'File not found') unless defined $file_name;

my $needle;
Expand All @@ -303,11 +331,14 @@ sub _basic_needle_info ($self, $name, $distri, $version, $file_name, $needles_di
return ($needle, undef);
}

sub _extended_needle_info ($self, $needle_dir, $needle_name, $basic_needle_data, $file_name, $error, $error_messages) {
sub _extended_needle_info ($self, $needle_dir, $needle_name, $basic_needle_data, $file_name, $error, $error_messages,
$needle_ref)
{
my $overall_list_of_tags = $basic_needle_data->{tags};
my $distri = $basic_needle_data->{distri};
my $version = $basic_needle_data->{version};
my ($needle_info, $err) = $self->_basic_needle_info($needle_name, $distri, $version, $file_name, $needle_dir);
my ($needle_info, $err)
= $self->_basic_needle_info($needle_name, $distri, $version, $file_name, $needle_dir, $needle_ref);
unless (defined $needle_info) {
push(@$error_messages, "Could not parse needle $needle_name for $distri $version: $err");
return undef;
Expand Down Expand Up @@ -465,6 +496,7 @@ sub viewimg ($self) {
my $distri = $job->DISTRI;
my $dversion = $job->VERSION || '';
my $needle_dir = $job->needle_dir;
my $needle_ref = $self->_create_tmpdir_for_needles_refspec($job);
my $real_needle_dir = realpath($needle_dir) // $needle_dir;
my $needles_rs = $self->app->schema->resultset('Needles');

Expand All @@ -477,7 +509,7 @@ sub viewimg ($self) {
my $append_needle_info = sub ($tags, $needle_info) {
# add timestamps and URLs from database
$self->populate_hash_with_needle_timestamps_and_urls(
$needles_rs->find_needle($real_needle_dir, "$needle_info->{name}.json"), $needle_info);
$needles_rs->find_needle($job->needle_dir, "$needle_info->{name}.json"), $needle_info);

# handle case when the needle has (for some reason) no tags
if (!$tags) {
Expand All @@ -500,7 +532,8 @@ sub viewimg ($self) {
# load primary needle match
my $primary_match;
if (my $needle = $module_detail->{needle}) {
my ($needleinfo) = $self->_basic_needle_info($needle, $distri, $dversion, $module_detail->{json}, $needle_dir);
my ($needleinfo)
= $self->_basic_needle_info($needle, $distri, $dversion, $module_detail->{json}, $needle_dir, $needle_ref);
if ($needleinfo) {
my $info = {
name => $needle,
Expand All @@ -522,7 +555,8 @@ sub viewimg ($self) {
if ($module_detail->{needles}) {
for my $needle (@{$module_detail->{needles}}) {
my $needlename = $needle->{name};
my ($needleinfo) = $self->_basic_needle_info($needlename, $distri, $dversion, $needle->{json}, $needle_dir);
my ($needleinfo)
= $self->_basic_needle_info($needlename, $distri, $dversion, $needle->{json}, $needle_dir, $needle_ref);
next unless $needleinfo;
my $info = {
name => $needlename,
Expand Down

0 comments on commit 0953592

Please sign in to comment.