Skip to content

Commit

Permalink
Fixes #21704 - Prevent user from rerunning a job with missing template
Browse files Browse the repository at this point in the history
  • Loading branch information
adamruzicka authored and iNecas committed Aug 6, 2018
1 parent 0441caf commit 7a94a2d
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 7 deletions.
11 changes: 8 additions & 3 deletions app/controllers/api/v2/job_invocations_controller.rb
Expand Up @@ -111,9 +111,14 @@ def cancel
param :failed_only, :bool
def rerun
composer = JobInvocationComposer.from_job_invocation(@job_invocation, params)
composer.trigger!
@job_invocation = composer.job_invocation
process_response @job_invocation
if composer.rerun_possible?
composer.trigger!
@job_invocation = composer.job_invocation
process_response @job_invocation
else
render :json => { :error => _('Could not rerun job %{id} because its template could not be found') % { :id => composer.reruns } },
:status => 404
end
end

private
Expand Down
11 changes: 10 additions & 1 deletion app/models/job_invocation_composer.rb
Expand Up @@ -186,7 +186,8 @@ def params
:concurrency_control => concurrency_control_params,
:execution_timeout_interval => job_invocation.execution_timeout_interval,
:remote_execution_feature_id => job_invocation.remote_execution_feature_id,
:template_invocations => template_invocations_params }.with_indifferent_access
:template_invocations => template_invocations_params,
:reruns => job_invocation.id }.with_indifferent_access
end

private
Expand Down Expand Up @@ -297,13 +298,15 @@ def job_template
end

attr_accessor :params, :job_invocation, :host_ids, :search_query
attr_reader :reruns
delegate :job_category, :remote_execution_feature_id, :pattern_template_invocations, :template_invocations, :targeting, :triggering, :to => :job_invocation

def initialize(params, set_defaults = false)
@params = params
@set_defaults = set_defaults
@job_invocation = JobInvocation.new
@job_invocation.task_group = JobInvocationTaskGroup.new
@reruns = params[:reruns]
compose

@host_ids = validate_host_ids(params[:host_ids])
Expand Down Expand Up @@ -342,6 +345,8 @@ def compose
job_invocation.key_passphrase = params[:key_passphrase]
job_invocation.sudo_password = params[:sudo_password]

job_invocation.job_category = nil unless rerun_possible?

self
end
# rubocop:enable Metrics/AbcSize
Expand Down Expand Up @@ -470,6 +475,10 @@ def job_template_ids
job_invocation.pattern_template_invocations.map(&:template_id)
end

def rerun_possible?
!(reruns && job_invocation.pattern_template_invocations.empty?)
end

private

# builds input values for a given templates id based on params
Expand Down
12 changes: 10 additions & 2 deletions app/views/job_invocations/_form.html.erb
Expand Up @@ -6,7 +6,15 @@

<%= form_for @composer.job_invocation, :html => {'data-refresh-url' => refresh_job_invocations_path, :id => 'job_invocation_form'} do |f| %>
<%= selectable_f f, :job_category, @composer.available_job_categories, {}, :label => _('Job category') %>
<% unless @composer.rerun_possible? %>
<%= alert :text => _('Could not rerun job %{id} because its template could not be found') % { :id => @composer.reruns },
:class => 'alert alert-block alert-danger has-error',
:close => false %>
<% end %>
<%= selectable_f f, :job_category, @composer.available_job_categories,
{ :selected => @composer.job_invocation.job_category, :include_blank => @composer.job_invocation.job_category.nil? },
:label => _('Job category') %>
<%= f.hidden_field(:remote_execution_feature_id, :value => @composer.remote_execution_feature_id) %>
<% selected_templates_per_provider = {} %>
Expand Down Expand Up @@ -116,5 +124,5 @@
<%= render :partial => 'preview_hosts_modal' %>
<%= submit_or_cancel f, false, :cancel_path => job_invocations_path %>
<%= submit_or_cancel f, false, :cancel_path => job_invocations_path, :disabled => !@composer.rerun_possible? %>
<% end %>
2 changes: 1 addition & 1 deletion test/factories/foreman_remote_execution_factories.rb
@@ -1,7 +1,7 @@
FactoryBot.define do
factory :job_template do
sequence(:name) { |n| "Job template #{n}" }
sequence(:job_category) { |n| "job name #{n}" }
sequence(:job_category) { |n| "Job name #{n}" }
template 'id'
provider_type 'SSH'
organizations { [Organization.find_by(name: 'Organization 1')] } if SETTINGS[:organizations_enabled]
Expand Down
14 changes: 14 additions & 0 deletions test/functional/api/v2/job_invocations_controller_test.rb
Expand Up @@ -6,6 +6,11 @@ class JobInvocationsControllerTest < ActionController::TestCase
setup do
@invocation = FactoryBot.create(:job_invocation, :with_template, :with_task)
@template = FactoryBot.create(:job_template, :with_input)

# Without this the template in template_invocations and in pattern_template_invocations
# would belong to different job_categories, causing trouble when trying to rerun
@invocation.job_category = @invocation.pattern_template_invocations.first.template.job_category
@invocation.save!
end

test 'should get index' do
Expand Down Expand Up @@ -178,6 +183,8 @@ class JobInvocationsControllerTest < ActionController::TestCase

test 'should rerun failed only' do
@invocation = FactoryBot.create(:job_invocation, :with_template, :with_failed_task)
@invocation.job_category = @invocation.pattern_template_invocations.first.template.job_category
@invocation.save!
JobInvocation.any_instance.expects(:generate_description)
JobInvocationComposer.any_instance
.expects(:validate_job_category)
Expand All @@ -191,6 +198,13 @@ class JobInvocationsControllerTest < ActionController::TestCase
targeting.user_id.must_equal users(:admin).id
targeting.search_query.must_equal "name ^ (#{hostnames.join(',')})"
end

test 'should return 404 if template is not found' do
@invocation.job_category = 'Missing category'
@invocation.save!
post :rerun, params: { :id => @invocation.id }
assert_response 404
end
end
end
end
18 changes: 18 additions & 0 deletions test/unit/job_invocation_composer_test.rb
Expand Up @@ -156,6 +156,24 @@ class JobInvocationComposerTest < ActiveSupport::TestCase
end
end

describe '#rerun_possible?' do
it 'is true when not rerunning' do
composer.must_be :rerun_possible?
end

it 'is true when rerunning with pattern tempalte invocations' do
composer.expects(:reruns).returns(1)
composer.job_invocation.expects(:pattern_template_invocations).returns([1])
composer.must_be :rerun_possible?
end

it 'is false when rerunning without pattern template invocations' do
composer.expects(:reruns).returns(1)
composer.job_invocation.expects(:pattern_template_invocations).returns([])
composer.wont_be :rerun_possible?
end
end

describe '#selected_job_templates' do
it 'returns no template if none was selected through params' do
composer.selected_job_templates.must_be_empty
Expand Down

0 comments on commit 7a94a2d

Please sign in to comment.