Merge pull request #9214 from Bugagazavr/hook-events

Added X-GitLab-Event header for web hooks

CHANGELOG

@@ -23,6 +23,7 @@ v 7.11.0 (unreleased)
   - Fix bug causing `@whatever` inside an inline code snippet (backtick-style) to be picked up as a user mention.
   - When use change branches link at MR form - save source branch selection instead of target one
   - Improve handling of large diffs
+  - Added GitLab Event header for project hooks
   -
   - Show Atom feed buttons everywhere where applicable.
   - Add project activity atom feed.

app/controllers/admin/hooks_controller.rb

@@ -33,7 +33,7 @@ class Admin::HooksController < Admin::ApplicationController
       owner_name: "Someone",
       owner_email: "example@gitlabhq.com"
     }
-    @hook.execute(data)
+    @hook.execute(data, 'system_hooks')
 
     redirect_to :back
   end

app/models/hooks/service_hook.rb

@@ -17,4 +17,8 @@
 
 class ServiceHook < WebHook
   belongs_to :service
+
+  def execute(data)
+    super(data, 'service_hook')
+  end
 end

app/models/hooks/web_hook.rb

@@ -30,12 +30,15 @@ class WebHook < ActiveRecord::Base
   validates :url, presence: true,
                   format: { with: /\A#{URI.regexp(%w(http https))}\z/, message: "should be a valid url" }
 
-  def execute(data)
+  def execute(data, hook_name)
     parsed_url = URI.parse(url)
     if parsed_url.userinfo.blank?
       WebHook.post(url,
                    body: data.to_json,
-                   headers: { "Content-Type" => "application/json" },
+                   headers: {
+                     "Content-Type" => "application/json",
+                     "X-Gitlab-Event" => hook_name.singularize.titleize
+                   },
                    verify: false)
     else
       post_url = url.gsub("#{parsed_url.userinfo}@", "")
@@ -45,7 +48,10 @@ class WebHook < ActiveRecord::Base
       }
       WebHook.post(post_url,
                    body: data.to_json,
-                   headers: { "Content-Type" => "application/json" },
+                   headers: {
+                     "Content-Type" => "application/json",
+                     "X-Gitlab-Event" => hook_name.singularize.titleize
+                   },
                    verify: false,
                    basic_auth: auth)
     end
@@ -54,7 +60,7 @@ class WebHook < ActiveRecord::Base
     false
   end
 
-  def async_execute(data)
-    Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data)
+  def async_execute(data, hook_name)
+    Sidekiq::Client.enqueue(ProjectWebHookWorker, id, data, hook_name)
   end
 end

app/models/project.rb

@@ -483,7 +483,7 @@ class Project < ActiveRecord::Base
 
   def execute_hooks(data, hooks_scope = :push_hooks)
     hooks.send(hooks_scope).each do |hook|
-      hook.async_execute(data)
+      hook.async_execute(data, hooks_scope.to_s)
     end
   end
 

app/services/system_hooks_service.rb

@@ -7,12 +7,12 @@ class SystemHooksService
 
   def execute_hooks(data)
     SystemHook.all.each do |sh|
-      async_execute_hook sh, data
+      async_execute_hook(sh, data, 'system_hooks')
     end
   end
 
-  def async_execute_hook(hook, data)
-    Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data)
+  def async_execute_hook(hook, data, hook_name)
+    Sidekiq::Client.enqueue(SystemHookWorker, hook.id, data, hook_name)
   end
 
   def build_event_data(model, event)

app/services/test_hook_service.rb

 class TestHookService
   def execute(hook, current_user)
     data = Gitlab::PushDataBuilder.build_sample(hook.project, current_user)
-    hook.execute(data)
+    hook.execute(data, 'push_hooks')
   end
 end

app/workers/project_web_hook_worker.rb

@@ -3,8 +3,8 @@ class ProjectWebHookWorker
 
   sidekiq_options queue: :project_web_hook
 
-  def perform(hook_id, data)
+  def perform(hook_id, data, hook_name)
     data = data.with_indifferent_access
-    WebHook.find(hook_id).execute(data)
+    WebHook.find(hook_id).execute(data, hook_name)
   end
 end

app/workers/system_hook_worker.rb

@@ -3,7 +3,7 @@ class SystemHookWorker
 
   sidekiq_options queue: :system_hook
 
-  def perform(hook_id, data)
-    SystemHook.find(hook_id).execute data
+  def perform(hook_id, data, hook_name)
+    SystemHook.find(hook_id).execute(data, hook_name)
   end
 end

doc/system_hooks/system_hooks.md

@@ -6,6 +6,12 @@ System hooks can be used, e.g. for logging or changing information in a LDAP ser
 
 ## Hooks request example
 
+**Request header**:
+
+```
+X-Gitlab-Event: System Hook
+```
+
 **Project created:**
 
 ```json

doc/web_hooks/web_hooks.md

@@ -12,6 +12,12 @@ If you send a web hook to an SSL endpoint [the certificate will not be verified]
 
 Triggered when you push to the repository except when pushing tags.
 
+**Request header**:
+
+```
+X-Gitlab-Event: Push Hook
+```
+
 **Request body:**
 
 ```json
@@ -63,6 +69,13 @@ Triggered when you push to the repository except when pushing tags.
 
 Triggered when you create (or delete) tags to the repository.
 
+**Request header**:
+
+```
+X-Gitlab-Event: Tag Push Hook
+```
+
+
 **Request body:**
 
 ```json
@@ -92,6 +105,12 @@ Triggered when you create (or delete) tags to the repository.
 
 Triggered when a new issue is created or an existing issue was updated/closed/reopened.
 
+**Request header**:
+
+```
+X-Gitlab-Event: Issue Hook
+```
+
 **Request body:**
 
 ```json
@@ -126,6 +145,12 @@ Triggered when a new issue is created or an existing issue was updated/closed/re
 
 Triggered when a new merge request is created or an existing merge request was updated/merged/closed.
 
+**Request header**:
+
+```
+X-Gitlab-Event: Merge Request Hook
+```
+
 **Request body:**
 
 ```json

lib/api/system_hooks.rb

@@ -47,7 +47,7 @@ module API
           owner_name: "Someone",
           owner_email: "example@gitlabhq.com"
         }
-        @hook.execute(data)
+        @hook.execute(data, 'system_hooks')
         data
       end
 

spec/models/hooks/service_hook_spec.rb

@@ -21,4 +21,37 @@ describe ServiceHook do
   describe "Associations" do
     it { is_expected.to belong_to :service }
   end
+
+  describe "execute" do
+    before(:each) do
+      @service_hook = create(:service_hook)
+      @data = { project_id: 1, data: {}}
+
+      WebMock.stub_request(:post, @service_hook.url)
+    end
+
+    it "POSTs to the web hook URL" do
+      @service_hook.execute(@data)
+      expect(WebMock).to have_requested(:post, @service_hook.url).with(
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook'}
+      ).once
+    end
+
+    it "POSTs the data as JSON" do
+      json = @data.to_json
+
+      @service_hook.execute(@data)
+      expect(WebMock).to have_requested(:post, @service_hook.url).with(
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Service Hook'}
+      ).once
+    end
+
+    it "catches exceptions" do
+      expect(WebHook).to receive(:post).and_raise("Some HTTP Post error")
+
+      expect {
+        @service_hook.execute(@data)
+      }.to raise_error
+    end
+  end
 end

spec/models/hooks/system_hook_spec.rb

@@ -26,32 +26,47 @@ describe SystemHook do
 
     it "project_create hook" do
       Projects::CreateService.new(create(:user), name: 'empty').execute
-      expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /project_create/).once
+      expect(WebMock).to have_requested(:post, @system_hook.url).with(
+        body: /project_create/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
+      ).once
     end
 
     it "project_destroy hook" do
       user = create(:user)
       project = create(:empty_project, namespace: user.namespace)
       Projects::DestroyService.new(project, user, {}).execute
-      expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /project_destroy/).once
+      expect(WebMock).to have_requested(:post, @system_hook.url).with(
+        body: /project_destroy/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
+      ).once
     end
 
     it "user_create hook" do
       create(:user)
-      expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_create/).once
+      expect(WebMock).to have_requested(:post, @system_hook.url).with(
+        body: /user_create/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
+      ).once
     end
 
     it "user_destroy hook" do
       user = create(:user)
       user.destroy
-      expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_destroy/).once
+      expect(WebMock).to have_requested(:post, @system_hook.url).with(
+        body: /user_destroy/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
+      ).once
     end
 
     it "project_create hook" do
       user = create(:user)
       project = create(:project)
       project.team << [user, :master]
-      expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_add_to_team/).once
+      expect(WebMock).to have_requested(:post, @system_hook.url).with(
+        body: /user_add_to_team/, 
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
+      ).once
     end
 
     it "project_destroy hook" do
@@ -59,13 +74,17 @@ describe SystemHook do
       project = create(:project)
       project.team << [user, :master]
       project.project_members.destroy_all
-      expect(WebMock).to have_requested(:post, @system_hook.url).with(body: /user_remove_from_team/).once
+      expect(WebMock).to have_requested(:post, @system_hook.url).with(
+        body: /user_remove_from_team/, 
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
+      ).once
     end
 
     it 'group create hook' do
       create(:group)
       expect(WebMock).to have_requested(:post, @system_hook.url).with(
-        body: /group_create/
+        body: /group_create/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
       ).once
     end
 
@@ -73,7 +92,8 @@ describe SystemHook do
       group = create(:group)
       group.destroy
       expect(WebMock).to have_requested(:post, @system_hook.url).with(
-        body: /group_destroy/
+        body: /group_destroy/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
       ).once
     end
 
@@ -82,7 +102,8 @@ describe SystemHook do
       user = create(:user)
       group.add_user(user, Gitlab::Access::MASTER)
       expect(WebMock).to have_requested(:post, @system_hook.url).with(
-        body: /user_add_to_group/
+        body: /user_add_to_group/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
       ).once
     end
 
@@ -92,7 +113,8 @@ describe SystemHook do
       group.add_user(user, Gitlab::Access::MASTER)
       group.group_members.destroy_all
       expect(WebMock).to have_requested(:post, @system_hook.url).with(
-        body: /user_remove_from_group/
+        body: /user_remove_from_group/,
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'System Hook'}
       ).once
     end
 

spec/models/hooks/web_hook_spec.rb

@@ -52,22 +52,26 @@ describe ProjectHook do
     end
 
     it "POSTs to the web hook URL" do
-      @project_hook.execute(@data)
-      expect(WebMock).to have_requested(:post, @project_hook.url).once
+      @project_hook.execute(@data, 'push_hooks')
+      expect(WebMock).to have_requested(:post, @project_hook.url).with(
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook'}
+      ).once
     end
 
     it "POSTs the data as JSON" do
       json = @data.to_json
 
-      @project_hook.execute(@data)
-      expect(WebMock).to have_requested(:post, @project_hook.url).with(body: json).once
+      @project_hook.execute(@data, 'push_hooks')
+      expect(WebMock).to have_requested(:post, @project_hook.url).with(
+        headers: {'Content-Type'=>'application/json', 'X-Gitlab-Event'=>'Push Hook'}
+      ).once
     end
 
     it "catches exceptions" do
       expect(WebHook).to receive(:post).and_raise("Some HTTP Post error")
 
       expect {
-        @project_hook.execute(@data)
+        @project_hook.execute(@data, 'push_hooks')
       }.to raise_error
     end
   end