|
|
8c41f40 |
From fee60fdcc32ceadfba92736b518dd223de3f18b6 Mon Sep 17 00:00:00 2001
|
|
|
574ef65 |
From: Adam Williamson <awilliam@redhat.com>
|
|
|
574ef65 |
Date: Mon, 16 Nov 2020 15:09:28 -0800
|
|
|
8c41f40 |
Subject: [PATCH 1/4] Handle placeholders after parsing _URL settings
|
|
|
574ef65 |
|
|
|
574ef65 |
b3bc8ebf caused some problems by moving the parsing of `_URL`
|
|
|
574ef65 |
settings to after the call to `JobSettings::generate_settings`,
|
|
|
574ef65 |
which is where handling of placeholders (%FOO%), usually from the
|
|
|
574ef65 |
templates, is done. This broke the case where e.g. a template
|
|
|
574ef65 |
uses `%ISO%` in a value, and the job is posted with `ISO_URL` but
|
|
|
574ef65 |
no explicit `ISO` set. In that case the derived `ISO` setting
|
|
|
574ef65 |
should be used in the placeholder expansion, but this was broken.
|
|
|
574ef65 |
|
|
|
574ef65 |
This fixes the problems by splitting up the "parse _URL settings"
|
|
|
574ef65 |
and "create download list" functions of `create_downloads_list`,
|
|
|
574ef65 |
and moving the parsing part into `generate_settings`, just before
|
|
|
574ef65 |
placeholder expansion happens. This is after we pull in settings
|
|
|
574ef65 |
from the templates, so the bug b3bc8ebf was trying to fix (POO
|
|
|
574ef65 |
62159) should still be fixed. Download list creation should still
|
|
|
574ef65 |
happen the same as currently, so POO #70687 should still be fixed.
|
|
|
574ef65 |
|
|
|
574ef65 |
Signed-off-by: Adam Williamson <awilliam@redhat.com>
|
|
|
574ef65 |
---
|
|
|
8c41f40 |
lib/OpenQA/JobSettings.pm | 39 ++++++++++++++++++++++++
|
|
|
8c41f40 |
lib/OpenQA/Utils.pm | 63 +++++++++++++++++----------------------
|
|
|
8c41f40 |
t/api/02-iso-download.t | 13 ++++++++
|
|
|
8c41f40 |
3 files changed, 79 insertions(+), 36 deletions(-)
|
|
|
574ef65 |
|
|
|
574ef65 |
diff --git a/lib/OpenQA/JobSettings.pm b/lib/OpenQA/JobSettings.pm
|
|
|
574ef65 |
index 0d6c11811..b0e364c8c 100644
|
|
|
574ef65 |
--- a/lib/OpenQA/JobSettings.pm
|
|
|
574ef65 |
+++ b/lib/OpenQA/JobSettings.pm
|
|
|
574ef65 |
@@ -17,6 +17,12 @@ package OpenQA::JobSettings;
|
|
|
574ef65 |
use strict;
|
|
|
574ef65 |
use warnings;
|
|
|
574ef65 |
|
|
|
574ef65 |
+use File::Basename;
|
|
|
574ef65 |
+use Mojo::URL;
|
|
|
574ef65 |
+use Mojo::Util 'url_unescape';
|
|
|
574ef65 |
+use OpenQA::Log 'log_debug';
|
|
|
574ef65 |
+use OpenQA::Utils 'get_url_short';
|
|
|
574ef65 |
+
|
|
|
574ef65 |
sub generate_settings {
|
|
|
574ef65 |
my ($params) = @_;
|
|
|
574ef65 |
my $settings = $params->{settings};
|
|
|
574ef65 |
@@ -53,6 +59,7 @@ sub generate_settings {
|
|
|
574ef65 |
$settings->{JOB_DESCRIPTION} = $job_template->description if length $job_template->description;
|
|
|
574ef65 |
}
|
|
|
574ef65 |
|
|
|
574ef65 |
+ parse_url_settings($settings);
|
|
|
574ef65 |
handle_plus_in_settings($settings);
|
|
|
574ef65 |
return expand_placeholders($settings);
|
|
|
574ef65 |
}
|
|
|
574ef65 |
@@ -104,4 +111,36 @@ sub handle_plus_in_settings {
|
|
|
574ef65 |
}
|
|
|
574ef65 |
}
|
|
|
574ef65 |
|
|
|
574ef65 |
+# Given a hashref of settings, parse any whose names end in _URL
|
|
|
574ef65 |
+# to the short name, then if there is not already a setting with
|
|
|
574ef65 |
+# the short name, set it to the filename from the URL (with the
|
|
|
574ef65 |
+# compression extension removed in the case of _DECOMPRESS_URL).
|
|
|
574ef65 |
+# This has to happen *before* generate_jobs
|
|
|
574ef65 |
+sub parse_url_settings {
|
|
|
574ef65 |
+ my ($settings) = @_;
|
|
|
574ef65 |
+ for my $setting (keys %$settings) {
|
|
|
574ef65 |
+ my ($short, $do_extract) = get_url_short($setting);
|
|
|
574ef65 |
+ next unless ($short);
|
|
|
574ef65 |
+ next if ($settings->{$short});
|
|
|
574ef65 |
+ # As this comes in from an API call, URL will be URI-encoded
|
|
|
574ef65 |
+ # This obviously creates a vuln if untrusted users can POST
|
|
|
574ef65 |
+ $settings->{$setting} = url_unescape($settings->{$setting});
|
|
|
574ef65 |
+ my $url = $settings->{$setting};
|
|
|
574ef65 |
+ my $filename;
|
|
|
574ef65 |
+ $filename = Mojo::URL->new($url)->path->parts->[-1];
|
|
|
574ef65 |
+ if ($do_extract) {
|
|
|
574ef65 |
+ # if user wants to extract downloaded file, final filename
|
|
|
574ef65 |
+ # will have last extension removed
|
|
|
574ef65 |
+ $filename = fileparse($filename, qr/\.[^.]*/);
|
|
|
574ef65 |
+ }
|
|
|
574ef65 |
+ $settings->{$short} = $filename;
|
|
|
574ef65 |
+ if (!$settings->{$short}) {
|
|
|
574ef65 |
+ log_debug("Unable to get filename from $url. Ignoring $setting");
|
|
|
574ef65 |
+ delete $settings->{$short} unless $settings->{$short};
|
|
|
574ef65 |
+ next;
|
|
|
574ef65 |
+ }
|
|
|
574ef65 |
+ }
|
|
|
574ef65 |
+ return undef;
|
|
|
574ef65 |
+}
|
|
|
574ef65 |
+
|
|
|
574ef65 |
1;
|
|
|
574ef65 |
diff --git a/lib/OpenQA/Utils.pm b/lib/OpenQA/Utils.pm
|
|
|
8c41f40 |
index 4db65eaeb..6dc3d0880 100644
|
|
|
574ef65 |
--- a/lib/OpenQA/Utils.pm
|
|
|
574ef65 |
+++ b/lib/OpenQA/Utils.pm
|
|
|
574ef65 |
@@ -21,7 +21,6 @@ use Carp;
|
|
|
574ef65 |
use Cwd 'abs_path';
|
|
|
574ef65 |
use IPC::Run();
|
|
|
574ef65 |
use Mojo::URL;
|
|
|
574ef65 |
-use Mojo::Util 'url_unescape';
|
|
|
574ef65 |
use Regexp::Common 'URI';
|
|
|
574ef65 |
use Try::Tiny;
|
|
|
574ef65 |
use Mojo::File 'path';
|
|
|
574ef65 |
@@ -62,6 +61,7 @@ our @EXPORT = qw(
|
|
|
574ef65 |
asset_type_from_setting
|
|
|
574ef65 |
check_download_url
|
|
|
574ef65 |
check_download_passlist
|
|
|
574ef65 |
+ get_url_short
|
|
|
574ef65 |
create_downloads_list
|
|
|
574ef65 |
human_readable_size
|
|
|
574ef65 |
locate_asset
|
|
|
8c41f40 |
@@ -499,45 +499,36 @@ sub check_download_passlist {
|
|
|
574ef65 |
return ();
|
|
|
574ef65 |
}
|
|
|
574ef65 |
|
|
|
574ef65 |
+sub get_url_short {
|
|
|
574ef65 |
+ # Given a setting name, if it ends with _URL or _DECOMPRESS_URL
|
|
|
574ef65 |
+ # return the name with that string stripped, and a flag indicating
|
|
|
574ef65 |
+ # whether decompression will be needed. If it doesn't, returns
|
|
|
574ef65 |
+ # empty string and 0.
|
|
|
574ef65 |
+ my ($arg) = @_;
|
|
|
574ef65 |
+ return ('', 0) unless ($arg =~ /_URL$/);
|
|
|
574ef65 |
+ my $short;
|
|
|
574ef65 |
+ my $do_extract = 0;
|
|
|
574ef65 |
+ if ($arg =~ /_DECOMPRESS_URL$/) {
|
|
|
574ef65 |
+ $short = substr($arg, 0, -15);
|
|
|
574ef65 |
+ $do_extract = 1;
|
|
|
574ef65 |
+ }
|
|
|
574ef65 |
+ else {
|
|
|
574ef65 |
+ $short = substr($arg, 0, -4);
|
|
|
574ef65 |
+ }
|
|
|
574ef65 |
+ return ($short, $do_extract);
|
|
|
574ef65 |
+}
|
|
|
574ef65 |
+
|
|
|
574ef65 |
sub create_downloads_list {
|
|
|
574ef65 |
my ($args) = @_;
|
|
|
574ef65 |
my %downloads = ();
|
|
|
574ef65 |
for my $arg (keys %$args) {
|
|
|
574ef65 |
- next unless ($arg =~ /_URL$/);
|
|
|
574ef65 |
- # As this comes in from an API call, URL will be URI-encoded
|
|
|
574ef65 |
- # This obviously creates a vuln if untrusted users can POST
|
|
|
574ef65 |
- $args->{$arg} = url_unescape($args->{$arg});
|
|
|
574ef65 |
- my $url = $args->{$arg};
|
|
|
574ef65 |
- my $do_extract = 0;
|
|
|
574ef65 |
- my $short;
|
|
|
8c41f40 |
- my $filename;
|
|
|
574ef65 |
- # if $args{FOO_URL} or $args{FOO_DECOMPRESS_URL} is set but $args{FOO}
|
|
|
574ef65 |
- # is not, we will set $args{FOO} (the filename of the downloaded asset)
|
|
|
574ef65 |
- # to the URL filename. This has to happen *before*
|
|
|
574ef65 |
- # generate_jobs so the jobs have FOO set
|
|
|
574ef65 |
- if ($arg =~ /_DECOMPRESS_URL$/) {
|
|
|
574ef65 |
- $do_extract = 1;
|
|
|
574ef65 |
- $short = substr($arg, 0, -15); # remove whole _DECOMPRESS_URL substring
|
|
|
574ef65 |
- }
|
|
|
574ef65 |
- else {
|
|
|
574ef65 |
- $short = substr($arg, 0, -4); # remove _URL substring
|
|
|
574ef65 |
- }
|
|
|
8c41f40 |
- if (!$args->{$short}) {
|
|
|
574ef65 |
- $filename = Mojo::URL->new($url)->path->parts->[-1];
|
|
|
574ef65 |
- if ($do_extract) {
|
|
|
574ef65 |
- # if user wants to extract downloaded file, final filename
|
|
|
574ef65 |
- # will have last extension removed
|
|
|
574ef65 |
- $filename = fileparse($filename, qr/\.[^.]*/);
|
|
|
574ef65 |
- }
|
|
|
574ef65 |
- $args->{$short} = $filename;
|
|
|
574ef65 |
- if (!$args->{$short}) {
|
|
|
574ef65 |
- log_debug("Unable to get filename from $url. Ignoring $arg");
|
|
|
574ef65 |
- delete $args->{$short} unless $args->{$short};
|
|
|
574ef65 |
- next;
|
|
|
574ef65 |
- }
|
|
|
8c41f40 |
- }
|
|
|
8c41f40 |
- else {
|
|
|
8c41f40 |
- $filename = $args->{$short};
|
|
|
8c41f40 |
+ my $url = $args->{$arg};
|
|
|
8c41f40 |
+ my ($short, $do_extract) = get_url_short($arg);
|
|
|
8c41f40 |
+ next unless ($short);
|
|
|
8c41f40 |
+ my $filename = $args->{$short};
|
|
|
8c41f40 |
+ unless ($filename) {
|
|
|
574ef65 |
+ log_debug("No target filename set for $url. Ignoring $arg");
|
|
|
574ef65 |
+ next;
|
|
|
574ef65 |
}
|
|
|
8c41f40 |
# We're only going to allow downloading of asset types. We also
|
|
|
8c41f40 |
# need this to determine the download location later
|
|
|
574ef65 |
diff --git a/t/api/02-iso-download.t b/t/api/02-iso-download.t
|
|
|
574ef65 |
index fcb3744f0..53b67ebf5 100644
|
|
|
574ef65 |
--- a/t/api/02-iso-download.t
|
|
|
574ef65 |
+++ b/t/api/02-iso-download.t
|
|
|
574ef65 |
@@ -298,4 +298,17 @@ subtest 'download task only blocks the related job when test suites have differe
|
|
|
574ef65 |
is scalar(@{$gru_dep_tasks->{$gru_task_ids[0]}}), 3, 'one download task was created and it blocked 3 jobs';
|
|
|
574ef65 |
};
|
|
|
574ef65 |
|
|
|
574ef65 |
+subtest 'placeholder expansions work with _URL-derived settings' => sub {
|
|
|
574ef65 |
+ $test_suites->find({name => 'kde'})->settings->create({key => 'FOOBAR', value => '%ISO%'});
|
|
|
574ef65 |
+ my $new_params = {%params, ISO_URL => 'http://localhost/openSUSE-13.1-DVD-i586-Build0091-Media.iso', TEST => 'kde'};
|
|
|
574ef65 |
+ $rsp = schedule_iso($new_params, 200);
|
|
|
574ef65 |
+ is $rsp->json->{count}, 1, 'one job was scheduled';
|
|
|
574ef65 |
+ my $expanderjob = get_job($rsp->json->{ids}->[0]);
|
|
|
574ef65 |
+ is(
|
|
|
574ef65 |
+ $expanderjob->{settings}->{FOOBAR},
|
|
|
574ef65 |
+ 'openSUSE-13.1-DVD-i586-Build0091-Media.iso',
|
|
|
574ef65 |
+ '%ISO% in template is expanded by posted ISO_URL'
|
|
|
574ef65 |
+ );
|
|
|
574ef65 |
+};
|
|
|
574ef65 |
+
|
|
|
574ef65 |
done_testing();
|
|
|
574ef65 |
--
|
|
|
574ef65 |
2.29.2
|
|
|
574ef65 |
|