Merge pull request #12076 from rubhanazeem/ignore-pull-merge

Check pull/merge request actions before initializing dependencies

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

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

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

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
 

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'])