Skip to content

Commit

Permalink
Ignore errors early, to avoid creating extra WorkflowRuns
Browse files Browse the repository at this point in the history
  • Loading branch information
hellcp-work committed Oct 24, 2023
1 parent 81eca94 commit 869293b
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 4 deletions.
4 changes: 4 additions & 0 deletions src/api/app/controllers/concerns/rescue_handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ module RescueHandler
render_error status: 403, errorcode: 'invalid_token', message: exception.message
end

rescue_from Trigger::Errors::MissingExtractor do |exception|
render_error status: 400, errorcode: 'bad_request', message: exception.message
end

rescue_from Project::WritePermissionError do |exception|
render_error status: 403, errorcode: 'modify_project_no_permission', message: exception.message
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ def hook_event
@github_event || @gitlab_event || @gitea_event
end

def ignored_event?
case scm_vendor
when 'github', 'gitea'
SCMWebhookEventValidator::ALLOWED_GITHUB_AND_GITEA_EVENTS.exclude?(hook_event)
when 'gitlab'
SCMWebhookEventValidator::ALLOWED_GITLAB_EVENTS.exclude?(hook_event)

Check warning on line 31 in src/api/app/controllers/concerns/scm_webhook_headers_data_extractor.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/controllers/concerns/scm_webhook_headers_data_extractor.rb#L31

Added line #L31 was not covered by tests
end
end

def extract_generic_event_type
# We only have filters for push, tag_push, and pull_request
if hook_event == 'Push Hook' || payload.fetch('ref', '').match('refs/heads')
Expand Down
4 changes: 4 additions & 0 deletions src/api/app/controllers/trigger/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ class InvalidToken < APIError
'No valid token found'
end

class MissingExtractor < APIError
setup 'bad_request', 400, 'Extractor could not be created.'
end

class BadSCMPayload < APIError
setup 'bad_request',
400,
Expand Down
20 changes: 17 additions & 3 deletions src/api/app/controllers/trigger_workflow_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ class TriggerWorkflowController < TriggerController
before_action :set_scm_event
before_action :set_scm_extractor
before_action :extract_scm_webhook
before_action :verify_event_and_action
before_action :create_workflow_run
before_action :validate_scm_event

Expand Down Expand Up @@ -40,10 +41,9 @@ def set_scm_extractor

def extract_scm_webhook
@scm_webhook = @scm_extractor.call
return @scm_webhook if @scm_webhook && @scm_extractor.valid?

# There are plenty of different pull/merge request and push events which we don't support.
# Those should not cause an error, we simply ignore them.
render_ok if @scm_webhook && (@scm_webhook.ignored_pull_request_action? || @scm_webhook.ignored_push_event?)
raise Trigger::Errors::MissingExtractor, @scm_extractor.error_message
end

def create_workflow_run
Expand All @@ -63,6 +63,20 @@ def create_workflow_run
generic_event_type: extract_generic_event_type)
end

def verify_event_and_action
# There are plenty of different pull/merge request and push events which we don't support.
# Those should not cause an error, we simply ignore them.
return unless @scm_webhook.ignored_pull_request_action? || @scm_webhook.ignored_push_event? || ignored_event?

action = @scm_webhook.payload[:action]

info_msg = "Events '#{@scm_webhook.payload[:event]}' "
info_msg += "and actions '#{action}' " if action.present?
info_msg += 'are unsupported'

render_ok(data: { info: info_msg })
end

def validate_scm_event
return if @scm_extractor.valid?

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@
end

it { expect(response).to have_http_status(:bad_request) }
it { expect(WorkflowRun.count).to eq(1) }
it { expect(WorkflowRun.count).to eq(0) }

it 'returns an error message in the response body' do
expect(response.body).to eql("<status code=\"bad_request\">\n <summary>This SCM event is not supported</summary>\n</status>\n")
Expand Down

0 comments on commit 869293b

Please sign in to comment.