Merge pull request #12354 from saraycp/cleanup_running_workflows_with…

…_closed_prs

Clean up running workflows with closed PRs

src/api/app/components/workflow_run_row_component.html.haml

@@ -1,7 +1,7 @@
 .col-1.text-left
   %i{ title: status_title, class: status_icon }
 .col.text-left
-  = link_to "#{hook_event.humanize} event", token_workflow_run_path(@workflow_run.token, @workflow_run)
+  = link_to "#{hook_event.humanize} event", token_workflow_run_path(workflow_run.token, workflow_run)
   - if hook_action
     %span.badge.badge-primary= hook_action.humanize
 .col.text-left

src/api/app/components/workflow_run_row_component.rb

@@ -1,55 +1,21 @@
 class WorkflowRunRowComponent < ApplicationComponent
-  SOURCE_NAME_PAYLOAD_MAPPING = {
-    'pull_request' => ['pull_request', 'number'],
-    'Merge Request Hook' => ['object_attributes', 'iid'],
-    'push' => ['head_commit', 'id'],
-    'Push Hook' => ['commits', 0, 'id']
-  }.freeze
-
-  SOURCE_URL_PAYLOAD_MAPPING = {
-    'pull_request' => ['pull_request', 'html_url'],
-    'Merge Request Hook' => ['object_attributes', 'url'],
-    'push' => ['head_commit', 'url'],
-    'Push Hook' => ['commits', 0, 'url']
-  }.freeze
-
-  attr_reader :workflow_run, :status, :hook_event
+  attr_reader :workflow_run, :status, :hook_event, :hook_action, :repository_name, :repository_url, :event_source_name, :event_source_url
 
   def initialize(workflow_run:)
     super
 
     @workflow_run = workflow_run
     @status = workflow_run.status
     @hook_event = workflow_run.hook_event
-  end
-
-  def hook_action
-    return payload['action'] if pull_request_with_allowed_action
-    return payload.dig('object_attributes', 'action') if merge_request_with_allowed_action
-  end
-
-  def repository_name
-    payload.dig('repository', 'full_name') || # For GitHub
-      payload.dig('repository', 'name') # For GitLab
-  end
-
-  def repository_url
-    payload.dig('repository', 'html_url') || # For GitHub
-      payload.dig('repository', 'git_http_url') || payload.dig('repository', 'url') # For GitLab
-  end
-
-  def event_source_name
-    path = SOURCE_NAME_PAYLOAD_MAPPING[@hook_event]
-    payload.dig(*path) if path
-  end
-
-  def event_source_url
-    mapped_source_url = SOURCE_URL_PAYLOAD_MAPPING[@hook_event]
-    payload.dig(*mapped_source_url) if mapped_source_url
+    @hook_action = workflow_run.hook_action
+    @repository_name = workflow_run.repository_name
+    @repository_url = workflow_run.repository_url
+    @event_source_name = workflow_run.event_source_name
+    @event_source_url = workflow_run.event_source_url
   end
 
   def formatted_event_source_name
-    case @hook_event
+    case hook_event
     when 'pull_request', 'Merge Request Hook'
       "##{event_source_name}"
     else
@@ -79,22 +45,4 @@ def status_icon
               end
     classes.join(' ')
   end
-
-  private
-
-  def payload
-    @payload ||= JSON.parse(workflow_run.request_payload)
-  rescue JSON::ParserError
-    { payload: 'unparseable' }
-  end
-
-  def pull_request_with_allowed_action
-    @hook_event == 'pull_request' &&
-      ScmWebhookEventValidator::ALLOWED_PULL_REQUEST_ACTIONS.include?(payload['action'])
-  end
-
-  def merge_request_with_allowed_action
-    @hook_event == 'Merge Request Hook' &&
-      ScmWebhookEventValidator::ALLOWED_MERGE_REQUEST_ACTIONS.include?(payload.dig('object_attributes', 'action'))
-  end
 end

src/api/app/models/workflow_run.rb

@@ -1,4 +1,18 @@
 class WorkflowRun < ApplicationRecord
+  SOURCE_NAME_PAYLOAD_MAPPING = {
+    'pull_request' => ['pull_request', 'number'],
+    'Merge Request Hook' => ['object_attributes', 'iid'],
+    'push' => ['head_commit', 'id'],
+    'Push Hook' => ['commits', 0, 'id']
+  }.freeze
+
+  SOURCE_URL_PAYLOAD_MAPPING = {
+    'pull_request' => ['pull_request', 'html_url'],
+    'Merge Request Hook' => ['object_attributes', 'url'],
+    'push' => ['head_commit', 'url'],
+    'Push Hook' => ['commits', 0, 'url']
+  }.freeze
+
   validates :response_url, length: { maximum: 255 }
   validates :request_headers, :status, presence: true
 
@@ -17,11 +31,42 @@ def update_to_fail(message)
     update(response_body: message, status: 'fail')
   end
 
+  def payload
+    JSON.parse(request_payload)
+  rescue JSON::ParserError
+    { payload: 'unparseable' }
+  end
+
   def hook_event
     parsed_request_headers['HTTP_X_GITHUB_EVENT'] ||
       parsed_request_headers['HTTP_X_GITLAB_EVENT']
   end
 
+  def hook_action
+    return payload['action'] if pull_request_with_allowed_action
+    return payload.dig('object_attributes', 'action') if merge_request_with_allowed_action
+  end
+
+  def repository_name
+    payload.dig('repository', 'full_name') || # For GitHub
+      payload.dig('repository', 'name') # For GitLab
+  end
+
+  def repository_url
+    payload.dig('repository', 'html_url') || # For GitHub
+      payload.dig('repository', 'git_http_url') || payload.dig('repository', 'url') # For GitLab
+  end
+
+  def event_source_name
+    path = SOURCE_NAME_PAYLOAD_MAPPING[hook_event]
+    payload.dig(*path) if path
+  end
+
+  def event_source_url
+    mapped_source_url = SOURCE_URL_PAYLOAD_MAPPING[hook_event]
+    payload.dig(*mapped_source_url) if mapped_source_url
+  end
+
   private
 
   def parsed_request_headers
@@ -30,6 +75,16 @@ def parsed_request_headers
       headers[k] = v.strip
     end
   end
+
+  def pull_request_with_allowed_action
+    hook_event == 'pull_request' &&
+      ScmWebhookEventValidator::ALLOWED_PULL_REQUEST_ACTIONS.include?(payload['action'])
+  end
+
+  def merge_request_with_allowed_action
+    hook_event == 'Merge Request Hook' &&
+      ScmWebhookEventValidator::ALLOWED_MERGE_REQUEST_ACTIONS.include?(payload.dig('object_attributes', 'action'))
+  end
 end
 
 # == Schema Information

src/api/lib/tasks/workflows.rake

@@ -0,0 +1,33 @@
+namespace :workflows do
+  desc 'Remove projects that were not closed as expected and set workflow run status to running'
+  task cleanup_non_closed_projects: :environment do
+    workflow_runs = WorkflowRun.where(status: 'running')
+                               .select do |workflow_run|
+                                 workflow_run.hook_event.in?(['pull_request', 'Merge Request Hook']) &&
+                                   workflow_run.hook_action.in?(['closed', 'close', 'merge'])
+                               end
+
+    puts "There are #{workflow_runs.count} workflow runs affected"
+
+    workflow_runs.each do |workflow_run|
+      projects = Project.where('name LIKE ?', "%#{target_project_name_postfix(workflow_run)}")
+
+      # If there is more than one project, we don't know which of them is the one related to the current
+      # workflow run (as we only can get the postfix, we don't have the full project name).
+      next if projects.count > 1
+
+      # If there is no project to remove (previously removed), the workflow run should change the status anyway.
+      User.get_default_admin.run_as { projects.first.destroy } if projects.count == 1
+      workflow_run.update(status: 'success')
+    rescue StandardError => e
+      Airbrake.notify("Failed to remove project created by the workflow: #{e}")
+      next
+    end
+  end
+end
+
+# If the name of the project created by the workflow is "home:Iggy:iggy:hello_world:PR-68", its postfix
+# is "iggy:hello_world:PR-68". This is the only information we can extract from the workflow_run.
+def target_project_name_postfix(workflow_run)
+  ":#{workflow_run.repository_name.tr('/', ':')}:PR-#{workflow_run.event_source_name}" if workflow_run.repository_name && workflow_run.event_source_name
+end

src/api/spec/lib/tasks/workflows_spec.rb

@@ -0,0 +1,76 @@
+require 'rails_helper'
+
+# rubocop:disable RSpec/DescribeClass
+RSpec.describe 'workflows' do
+  # rubocop:enable RSpec/DescribeClass
+  include_context 'rake'
+
+  let!(:admin_user) { create(:admin_user, login: 'Admin') }
+  let(:gitlab_request_headers) do
+    <<~END_OF_HEADERS
+      HTTP_X_GITLAB_EVENT: Merge Request Hook
+    END_OF_HEADERS
+  end
+  let(:github_request_payload_opened) do
+    <<~END_OF_REQUEST
+      {
+        "action": "opened",
+        "pull_request": {
+          "number": 1
+        },
+        "repository": {
+          "full_name": "iggy/hello_world"
+        }
+      }
+    END_OF_REQUEST
+  end
+  let(:github_request_payload_closed) do
+    <<~END_OF_REQUEST
+      {
+        "action": "closed",
+        "pull_request": {
+          "number": 2
+        },
+        "repository": {
+          "full_name": "iggy/hello_world"
+        }
+      }
+    END_OF_REQUEST
+  end
+  let(:gitlab_request_payload_merge) do
+    <<~END_OF_REQUEST
+      {
+        "object_kind": "merge_request",
+        "event_type": "merge_request",
+        "object_attributes": {
+          "iid": 3,
+          "action": "merge"
+        },
+        "repository": {
+          "name": "iggy/test"
+        }
+      }
+    END_OF_REQUEST
+  end
+
+  let!(:project_iggy_hello_world_pr1) { create(:project, name: 'home:Iggy:iggy:hello_world:PR-1') }
+  let!(:project_iggy_hello_world_pr2) { create(:project, name: 'home:Iggy:iggy:hello_world:PR-2') }
+  let!(:project_iggy_test_pr3) { create(:project, name: 'home:Iggy:iggy:test:PR-3') }
+
+  let!(:workflow_run_running_pr_opened) { create(:running_workflow_run, request_payload: github_request_payload_opened) }
+  let!(:workflow_run_succeeded_pr_opened) { create(:succeeded_workflow_run, request_payload: github_request_payload_opened) }
+  let!(:workflow_run_failed_pr_opened) { create(:failed_workflow_run, request_payload: github_request_payload_opened) }
+  let!(:workflow_run_running_pr_closed) { create(:running_workflow_run, request_payload: github_request_payload_closed) }
+  let!(:another_workflow_run_running_pr_closed) { create(:running_workflow_run, request_payload: github_request_payload_closed) }
+  let!(:workflow_run_running_pr_merge) { create(:running_workflow_run, request_headers: gitlab_request_headers, request_payload: gitlab_request_payload_merge) }
+
+  describe 'cleanup_non_closed_projects' do
+    let(:task) { 'workflows:cleanup_non_closed_projects' }
+
+    it { expect { rake_task.invoke }.to change(WorkflowRun.where(status: 'running'), :count).from(4).to(1) }
+
+    # The workflow runs defined above will create two target projects that should be deleted
+    # because the corresponding PR or MR are closed/merged.
+    it { expect { rake_task.invoke }.to change(Project, :count).from(3).to(1) }
+  end
+end