Skip to content

Commit

Permalink
Validate the test name when creating new jobs
Browse files Browse the repository at this point in the history
* Use the same regex we already use when validating the YAML schema for job
  templates
* See https://progress.opensuse.org/issues/177267
  • Loading branch information
Martchus committed Feb 20, 2025
1 parent 8d6811c commit 98d3a34
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
4 changes: 4 additions & 0 deletions lib/OpenQA/Jobs/Constants.pm
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ use Mojo::Base -base, -signatures;

use Exporter 'import';

# define regex for validating test names in accordance with `JobScenarios-01.yaml` and `JobTemplates-01.yaml`
use constant TEST_NAME_REGEX => qr/^[A-Za-z\s0-9_*.+-]+$/;

# job states
use constant {
# initial job state; the job is supposed to be assigned to a worker by the scheduler
Expand Down Expand Up @@ -103,6 +106,7 @@ use constant DEFAULT_JOB_PRIORITY => 50;
use constant TAG_ID_COLUMN => "concat(VERSION, '-', BUILD)";

our @EXPORT = qw(
TEST_NAME_REGEX
ASSIGNED
CANCELLED
COMPLETE_RESULTS
Expand Down
5 changes: 3 additions & 2 deletions lib/OpenQA/Schema/ResultSet/Jobs.pm
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ sub create_from_settings {
die 'The ' . join(',', @invalid_keys) . " cannot include / in value\n" if @invalid_keys;

# validate special settings
my %special_settings = (_PRIORITY => delete $settings{_PRIORITY});
my %special_settings = (TEST => delete $settings{TEST}, _PRIORITY => delete $settings{_PRIORITY});
my $validator = Mojolicious::Validator->new;
my $v = Mojolicious::Validator::Validation->new(validator => $validator, input => \%special_settings);
my $test = $v->required('TEST')->like(TEST_NAME_REGEX)->param;
my $prio = $v->optional('_PRIORITY')->num->param;
die 'The following settings are invalid: ' . join(', ', @{$v->failed}) . "\n" if $v->has_error;

Expand Down Expand Up @@ -176,9 +177,9 @@ sub create_from_settings {
my $value = $settings{$key};
$settings{$key} = decode_utf8 encode_json $value if (ref $value eq 'ARRAY' || ref $value eq 'HASH');
}
$new_job_args{TEST} = $settings{TEST};

# move important keys from the settings directly to the job
$new_job_args{TEST} = $test;
for my $key (OpenQA::Schema::Result::Jobs::MAIN_SETTINGS) {
if (my $value = delete $settings{$key}) { $new_job_args{$key} = $value }
}
Expand Down
10 changes: 10 additions & 0 deletions t/api/04-jobs.t
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,16 @@ subtest 'get job status' => sub {
->json_is('/error_status' => 404, 'Status code correct')->json_is('/error' => 'Job does not exist');
};

subtest 'validation of test name' => sub {
my @disallowed = ('spam@eggs', 'spam:eggs');
my @allowed = ('spam.eggs', 'spam+eggs');
$t->post_ok('/api/v1/jobs', form => {TEST => $_})->status_is(400, "test name $_ disallowed") for @disallowed;
$t->json_is('/error' => 'The following settings are invalid: TEST', 'error for invalid test name returned');
$t->post_ok('/api/v1/jobs', form => {TEST => $_})->status_is(200, "test name $_ allowed") for @allowed;
is $jobs->search({TEST => {-in => \@disallowed}})->count, 0, 'no jobs with disallowed names created';
is $jobs->search({TEST => {-in => \@allowed}})->count, @allowed, 'all jobs with allowed names created';
};

subtest 'cancel job' => sub {
$t->post_ok('/api/v1/jobs/99963/cancel')->status_is(200);
is_deeply(
Expand Down

0 comments on commit 98d3a34

Please sign in to comment.