diff --git a/src/api/app/controllers/trigger_workflow_controller.rb b/src/api/app/controllers/trigger_workflow_controller.rb index e5869405c85..c5d4b86fdf5 100644 --- a/src/api/app/controllers/trigger_workflow_controller.rb +++ b/src/api/app/controllers/trigger_workflow_controller.rb @@ -1,14 +1,15 @@ class TriggerWorkflowController < TriggerController # We don't need to validate that the body of the request is XML. We receive JSON skip_before_action :validate_xml_request, :set_project_name, :set_package_name, :set_project, :set_package, :set_object_to_authorize, :set_multibuild_flavor - before_action :create_workflow_run before_action :set_scm_event + before_action :abort_trigger_if_ignored_pull_request_action + before_action :create_workflow_run before_action :validate_scm_event def create authorize @token, :trigger? @token.user.run_as do - validation_errors = @token.call(scm: scm, event: event, payload: payload, workflow_run: @workflow_run) + validation_errors = @token.call(workflow_run: @workflow_run, scm_webhook: @scm_webhook) if validation_errors.none? @workflow_run.update(status: 'success', response_body: render_ok) @@ -60,4 +61,10 @@ def create_workflow_run request_headers = request.headers.to_h.keys.map { |k| "#{k}: #{request.headers[k]}" if k.match?(/^HTTP_/) }.compact.join("\n") @workflow_run = @token.workflow_runs.create(request_headers: request_headers, request_payload: request.body.read) end + + def abort_trigger_if_ignored_pull_request_action + @scm_webhook = TriggerControllerService::ScmExtractor.new(scm, event, payload).call + + render_ok if @scm_webhook && @scm_webhook.ignored_pull_request_action? + end end diff --git a/src/api/app/models/scm_webhook.rb b/src/api/app/models/scm_webhook.rb index 085a078e673..9c0ce7389ea 100644 --- a/src/api/app/models/scm_webhook.rb +++ b/src/api/app/models/scm_webhook.rb @@ -6,6 +6,11 @@ class ScmWebhook validates_with ScmWebhookEventValidator + IGNORED_PULL_REQUEST_ACTIONS = ['assigned', 'auto_merge_disabled', 'auto_merge_enabled', 'converted_to_draft', + 'edited', 'labeled', 'locked', 'ready_for_review', 'review_request_removed', + 'review_requested', 'unassigned', 'unlabeled', 'unlocked'].freeze + IGNORED_MERGE_REQUEST_ACTIONS = ['approved', 'unapproved'].freeze + def initialize(attributes = {}) super # To safely navigate the hash and compare keys @@ -44,6 +49,10 @@ def pull_request_event? github_pull_request? || gitlab_merge_request? end + def ignored_pull_request_action? + ignored_github_pull_request_action? || ignored_gitlab_merge_request_action? + end + private def github_push_event? @@ -69,4 +78,12 @@ def github_pull_request? def gitlab_merge_request? @payload[:scm] == 'gitlab' && @payload[:event] == 'Merge Request Hook' end + + def ignored_github_pull_request_action? + github_pull_request? && IGNORED_PULL_REQUEST_ACTIONS.include?(@payload[:action]) + end + + def ignored_gitlab_merge_request_action? + gitlab_merge_request? && IGNORED_MERGE_REQUEST_ACTIONS.include?(@payload[:action]) + end end diff --git a/src/api/app/models/token/workflow.rb b/src/api/app/models/token/workflow.rb index 9c1ce27a8d7..076d5a3bcee 100644 --- a/src/api/app/models/token/workflow.rb +++ b/src/api/app/models/token/workflow.rb @@ -9,10 +9,10 @@ def self.token_name def call(options) set_triggered_at + @scm_webhook = options[:scm_webhook] - raise Token::Errors::MissingPayload, 'A payload is required' if options[:payload].nil? + raise Token::Errors::MissingPayload, 'A payload is required' if @scm_webhook.payload.blank? - @scm_webhook = TriggerControllerService::ScmExtractor.new(options[:scm], options[:event], options[:payload]).call options[:workflow_run].update(response_url: @scm_webhook.payload[:api_endpoint]) yaml_file = Workflows::YAMLDownloader.new(@scm_webhook.payload, token: self).call @workflows = Workflows::YAMLToWorkflowsService.new(yaml_file: yaml_file, scm_webhook: @scm_webhook, token: self).call diff --git a/src/api/spec/controllers/trigger_workflow_controller_spec.rb b/src/api/spec/controllers/trigger_workflow_controller_spec.rb index f9212b394b2..59d049ac29f 100644 --- a/src/api/spec/controllers/trigger_workflow_controller_spec.rb +++ b/src/api/spec/controllers/trigger_workflow_controller_spec.rb @@ -60,10 +60,44 @@ end it { expect(response).to have_http_status(:bad_request) } + it { expect(WorkflowRun.count).to eq(0) } + end - it { expect(WorkflowRun.count).to eq(1) } - it { expect(WorkflowRun.last.status).to eq('fail') } - it { expect(response.body).to include(WorkflowRun.last.response_body) } + context 'scm action is invalid' do + let(:octokit_client) { instance_double(Octokit::Client) } + let(:token_extractor_instance) { instance_double(::TriggerControllerService::TokenExtractor) } + let(:token) { build_stubbed(:workflow_token, user: build_stubbed(:confirmed_user, :in_beta)) } + let(:github_payload) do + { + action: 'assigned', + pull_request: { + head: { + repo: { full_name: 'username/test_repo' } + }, + base: { + ref: 'main', + repo: { full_name: 'rubhanazeem/hello_world' } + } + }, + number: 4, + sender: { url: 'https://api.github.com' } + } + end + + before do + allow(::TriggerControllerService::TokenExtractor).to receive(:new).and_return(token_extractor_instance) + allow(token_extractor_instance).to receive(:call).and_return(token) + allow(Octokit::Client).to receive(:new).and_return(octokit_client) + allow(octokit_client).to receive(:content).and_return({ download_url: 'https://google.com' }) + allow(Down).to receive(:download).and_raise(Down::Error, 'Beep Boop, something is wrong') + request.headers['ACCEPT'] = '*/*' + request.headers['CONTENT_TYPE'] = 'application/json' + request.headers['HTTP_X_GITHUB_EVENT'] = 'pull_request' + post :create, body: github_payload.to_json + end + + it { expect(response).to have_http_status(:ok) } + it { expect(WorkflowRun.count).to eq(0) } end context 'scm payload is invalid' do @@ -84,10 +118,6 @@ end it { expect(response).to have_http_status(:bad_request) } - - it { expect(WorkflowRun.count).to eq(1) } - it { expect(WorkflowRun.last.status).to eq('fail') } - it { expect(response.body).to include(WorkflowRun.last.response_body) } end context 'payload can not be parsed' do @@ -96,10 +126,6 @@ end it { expect(response).to have_http_status(:bad_request) } - - it { expect(WorkflowRun.count).to eq(1) } - it { expect(WorkflowRun.last.status).to eq('fail') } - it { expect(response.body).to include(WorkflowRun.last.response_body) } end end diff --git a/src/api/spec/models/token/workflow_spec.rb b/src/api/spec/models/token/workflow_spec.rb index b8c5276f44d..a7db88077a4 100644 --- a/src/api/spec/models/token/workflow_spec.rb +++ b/src/api/spec/models/token/workflow_spec.rb @@ -8,7 +8,10 @@ context 'without a payload' do it do - expect { workflow_token.call({ workflow_run: workflow_run }) }.to raise_error(Token::Errors::MissingPayload, 'A payload is required').and(change(workflow_token, :triggered_at)) + expect do + workflow_token.call({ workflow_run: workflow_run, + scm_webhook: ScmWebhook.new(payload: {}) }) + end.to raise_error(Token::Errors::MissingPayload, 'A payload is required').and(change(workflow_token, :triggered_at)) end end @@ -70,7 +73,7 @@ allow(yaml_to_workflows_service).to receive(:call).and_return(workflows) end - subject { workflow_token.call(scm: scm, event: event, payload: github_payload, workflow_run: workflow_run) } + subject { workflow_token.call(workflow_run: workflow_run, scm_webhook: scm_extractor.call) } it 'returns no validation errors' do expect(subject).to eq([]) @@ -123,7 +126,7 @@ allow(yaml_to_workflows_service).to receive(:call).and_return(workflows) end - subject { workflow_token.call(scm: scm, event: event, payload: github_payload, workflow_run: workflow_run) } + subject { workflow_token.call(workflow_run: workflow_run, scm_webhook: scm_extractor.call) } it 'returns the validation errors' do expect(subject).to eq(['Event not supported.', 'Workflow steps are not present'])