diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 9bfcd9a9ec65210a8fd514280b2c546a563c53a9..a4342aa5b90436edd0800d4c2dc49079bf1d7692 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -102,9 +102,11 @@ def async_execute break log_rate_limited if rate_limited? break log_recursion_blocked if recursion_blocked? - data[:_gitlab_recursion_detection_request_uuid] = Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid + params = { + recursion_detection_request_uuid: Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid + }.compact - WebHookWorker.perform_async(hook.id, data, hook_name) + WebHookWorker.perform_async(hook.id, data, hook_name, params) end end diff --git a/app/workers/web_hook_worker.rb b/app/workers/web_hook_worker.rb index 952ac94d5e694664dbab95e2f395c9103e2719e1..fdcd22128a35a43ddd431036d031b8c7b0faedfe 100644 --- a/app/workers/web_hook_worker.rb +++ b/app/workers/web_hook_worker.rb @@ -6,14 +6,14 @@ class WebHookWorker include ApplicationWorker feature_category :integrations - loggable_arguments 2 + loggable_arguments 2, 3 data_consistency :delayed sidekiq_options retry: 4, dead: false urgency :low worker_has_external_dependencies! - # Webhook recursion detection properties are passed through the `data` arg. + # Webhook recursion detection properties may be passed through the `data` arg. # This will be migrated to the `params` arg over the next few releases. # See https://gitlab.com/gitlab-org/gitlab/-/issues/347389. def perform(hook_id, data, hook_name, params = {}) @@ -21,12 +21,14 @@ def perform(hook_id, data, hook_name, params = {}) return unless hook data = data.with_indifferent_access + params.symbolize_keys! - # Before executing the hook, reapply any recursion detection UUID that was - # initially present in the request header so the hook can pass this same header - # value in its request. - recursion_detection_uuid = data.delete(:_gitlab_recursion_detection_request_uuid) - Gitlab::WebHooks::RecursionDetection.set_request_uuid(recursion_detection_uuid) + # TODO: Remove in 14.9 https://gitlab.com/gitlab-org/gitlab/-/issues/347389 + params[:recursion_detection_request_uuid] ||= data.delete(:_gitlab_recursion_detection_request_uuid) + + # Before executing the hook, reapply any recursion detection UUID that was initially + # present in the request header so the hook can pass this same header value in its request. + Gitlab::WebHooks::RecursionDetection.set_request_uuid(params[:recursion_detection_request_uuid]) WebHookService.new(hook, data, hook_name, jid).execute end diff --git a/spec/services/feature_flags/update_service_spec.rb b/spec/services/feature_flags/update_service_spec.rb index abe0112b27ef788ff43f9c2ce9dcc1a3a638afd0..f5e94c4af0fa3d7d1ded96965345cc698c9673bc 100644 --- a/spec/services/feature_flags/update_service_spec.rb +++ b/spec/services/feature_flags/update_service_spec.rb @@ -130,7 +130,7 @@ it 'executes hooks' do hook = create(:project_hook, :all_events_enabled, project: project) - expect(WebHookWorker).to receive(:perform_async).with(hook.id, an_instance_of(Hash), 'feature_flag_hooks') + expect(WebHookWorker).to receive(:perform_async).with(hook.id, an_instance_of(Hash), 'feature_flag_hooks', an_instance_of(Hash)) subject end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 14aa4e364a14ae753adef03fd866f075a31da75a..448d00076c1efc0bbfa8533e01222d710e1923d5 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -404,7 +404,7 @@ def run_service describe '#async_execute' do def expect_to_perform_worker(hook) - expect(WebHookWorker).to receive(:perform_async).with(hook.id, data, 'push_hooks') + expect(WebHookWorker).to receive(:perform_async).with(hook.id, data, 'push_hooks', an_instance_of(Hash)) end def expect_to_rate_limit(hook, threshold:, throttled: false) diff --git a/spec/workers/web_hook_worker_spec.rb b/spec/workers/web_hook_worker_spec.rb index bbb8844a447ab8cfd0ae140aa6cfdace2d3425e2..dbdf7a2b978d5d8eff403493c10341b69e83e390 100644 --- a/spec/workers/web_hook_worker_spec.rb +++ b/spec/workers/web_hook_worker_spec.rb @@ -19,7 +19,16 @@ expect { subject.perform(non_existing_record_id, data, hook_name) }.not_to raise_error end - it 'retrieves recursion detection data, reinstates it, and cleans it from payload', :request_store, :aggregate_failures do + it 'retrieves recursion detection data and reinstates it', :request_store, :aggregate_failures do + uuid = SecureRandom.uuid + params = { recursion_detection_request_uuid: uuid } + + expect_next(WebHookService, project_hook, data.with_indifferent_access, hook_name, anything).to receive(:execute) + expect { subject.perform(project_hook.id, data, hook_name, params) } + .to change { Gitlab::WebHooks::RecursionDetection::UUID.instance.request_uuid }.to(uuid) + end + + it 'retrieves recursion detection data, reinstates it, and cleans it from payload when passed through as data', :request_store, :aggregate_failures do uuid = SecureRandom.uuid full_data = data.merge({ _gitlab_recursion_detection_request_uuid: uuid })