From f661980904f718440cc700b7d1f38ea8bf0e5480 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 3 May 2017 04:36:13 +0200 Subject: Added repository_update hook --- app/models/hooks/system_hook.rb | 2 ++ app/models/hooks/web_hook.rb | 1 + app/workers/post_receive.rb | 25 ++++++++++++++-- ...15_add_repository_update_events_to_web_hooks.rb | 15 ++++++++++ db/schema.rb | 1 + lib/gitlab/data_builder/repository.rb | 35 ++++++++++++++++++++++ lib/gitlab/git_post_receive.rb | 10 +++++++ .../gitlab/import_export/safe_model_attributes.yml | 1 + spec/models/hooks/system_hook_spec.rb | 8 +++++ spec/workers/post_receive_spec.rb | 23 +++++++++++++- 10 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb create mode 100644 lib/gitlab/data_builder/repository.rb diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index 777bad1e724..f97debb7113 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -1,4 +1,6 @@ class SystemHook < WebHook + scope :repository_update_hooks, -> { where(repository_update_events: true) } + def async_execute(data, hook_name) Sidekiq::Client.enqueue(SystemHookWorker, id, data, hook_name) end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 22a177ed367..7cf03aabd6f 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -10,6 +10,7 @@ class WebHook < ActiveRecord::Base default_value_for :tag_push_events, false default_value_for :build_events, false default_value_for :pipeline_events, false + default_value_for :repository_update_events, false default_value_for :enable_ssl_verification, true scope :push_hooks, -> { where(push_events: true) } diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index 127d8dfbb61..c29571d3c62 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -20,13 +20,32 @@ class PostReceive # Nothing defined here yet. else process_project_changes(post_received) + process_repository_update(post_received) end end - def process_project_changes(post_received) - post_received.changes.each do |change| - oldrev, newrev, ref = change.strip.split(' ') + def process_repository_update(post_received) + changes = [] + refs = Set.new + + post_received.changes_refs do |oldrev, newrev, ref| + @user ||= post_received.identify(newrev) + unless @user + log("Triggered hook for non-existing user \"#{post_received.identifier}\"") + return false + end + + changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref) + refs << ref + end + + hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, @user, changes, refs.to_a) + SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks) + end + + def process_project_changes(post_received) + post_received.changes_refs do |oldrev, newrev, ref| @user ||= post_received.identify(newrev) unless @user diff --git a/db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb b/db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb new file mode 100644 index 00000000000..0faea87a962 --- /dev/null +++ b/db/migrate/20170503023315_add_repository_update_events_to_web_hooks.rb @@ -0,0 +1,15 @@ +class AddRepositoryUpdateEventsToWebHooks < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_column_with_default :web_hooks, :repository_update_events, :boolean, default: false, allow_null: false + end + + def down + remove_column :web_hooks, :repository_update_events + end +end diff --git a/db/schema.rb b/db/schema.rb index 60077ffd812..65eaccf766a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1404,6 +1404,7 @@ ActiveRecord::Schema.define(version: 20170508190732) do t.string "token" t.boolean "pipeline_events", default: false, null: false t.boolean "confidential_issues_events", default: false, null: false + t.boolean "repository_update_events", default: false, null: false end add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree diff --git a/lib/gitlab/data_builder/repository.rb b/lib/gitlab/data_builder/repository.rb new file mode 100644 index 00000000000..b42dc052949 --- /dev/null +++ b/lib/gitlab/data_builder/repository.rb @@ -0,0 +1,35 @@ +module Gitlab + module DataBuilder + module Repository + extend self + + # Produce a hash of post-receive data + def update(project, user, changes, refs) + { + event_name: 'repository_update', + + user_id: user.id, + user_name: user.name, + user_email: user.email, + user_avatar: user.avatar_url, + + project_id: project.id, + project: project.hook_attrs, + + changes: changes, + + refs: refs + } + end + + # Produce a hash of partial data for a single change + def single_change(oldrev, newrev, ref) + { + before: oldrev, + after: newrev, + ref: ref + } + end + end + end +end diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index 0e14253ab4e..742118b76a8 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -13,6 +13,16 @@ module Gitlab super(identifier, project, revision) end + def changes_refs + return enum_for(:changes_refs) unless block_given? + + changes.each do |change| + oldrev, newrev, ref = change.strip.split(' ') + + yield oldrev, newrev, ref + end + end + private def deserialize_changes(changes) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index f63fb7aeec6..2b95f76e045 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -317,6 +317,7 @@ ProjectHook: - token - group_id - confidential_issues_events +- repository_update_events ProtectedBranch: - id - project_id diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index 8acec805584..a9e3ac3b679 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -105,4 +105,12 @@ describe SystemHook, models: true do ).once end end + + describe '.repository_update_hooks' do + it 'returns hooks for repository update events only' do + hook = create(:system_hook, repository_update_events: true) + create(:system_hook, repository_update_events: false) + expect(SystemHook.repository_update_hooks).to eq([hook]) + end + end end diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 0260416dbe2..3289c2df1fb 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -9,7 +9,7 @@ describe PostReceive do let(:key) { create(:key, user: project.owner) } let(:key_id) { key.shell_id } - context "as a resque worker" do + context "as a sidekiq worker" do it "reponds to #perform" do expect(described_class.new).to respond_to(:perform) end @@ -93,6 +93,27 @@ describe PostReceive do end end + describe '#process_repository_update' do + let(:changes) {'123456 789012 refs/heads/tést'} + let(:fake_hook_data) do + { event_name: 'repository_update' } + end + + before do + allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner) + allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data) + # silence hooks so we can isolate + allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true) + allow(subject).to receive(:process_project_changes).and_return(true) + end + + it 'calls SystemHooksService' do + expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true) + + subject.perform(pwd(project), key_id, base64_changes) + end + end + context "webhook" do it "fetches the correct project" do expect(Project).to receive(:find_by).with(id: project.id.to_s) -- cgit v1.2.1 From 2e97db051ca52d464382df0c84562fbc5e67b956 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 5 May 2017 14:11:28 +0200 Subject: Make the new repository_update_events configurable in System Hooks UI --- app/controllers/admin/hooks_controller.rb | 1 + app/models/hooks/system_hook.rb | 3 +++ app/views/admin/hooks/_form.html.haml | 9 +++++++- app/views/admin/hooks/index.html.haml | 2 +- spec/controllers/admin/hooks_controller_spec.rb | 28 +++++++++++++++++++++++++ spec/models/hooks/system_hook_spec.rb | 13 ++++++++++++ 6 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 spec/controllers/admin/hooks_controller_spec.rb diff --git a/app/controllers/admin/hooks_controller.rb b/app/controllers/admin/hooks_controller.rb index a119934febc..ccfe553c89e 100644 --- a/app/controllers/admin/hooks_controller.rb +++ b/app/controllers/admin/hooks_controller.rb @@ -60,6 +60,7 @@ class Admin::HooksController < Admin::ApplicationController :enable_ssl_verification, :push_events, :tag_push_events, + :repository_update_events, :token, :url ) diff --git a/app/models/hooks/system_hook.rb b/app/models/hooks/system_hook.rb index f97debb7113..c645805c6da 100644 --- a/app/models/hooks/system_hook.rb +++ b/app/models/hooks/system_hook.rb @@ -1,6 +1,9 @@ class SystemHook < WebHook scope :repository_update_hooks, -> { where(repository_update_events: true) } + default_value_for :push_events, false + default_value_for :repository_update_events, true + def async_execute(data, hook_name) Sidekiq::Client.enqueue(SystemHookWorker, id, data, hook_name) end diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml index 6217d5fb135..7f7377b1da1 100644 --- a/app/views/admin/hooks/_form.html.haml +++ b/app/views/admin/hooks/_form.html.haml @@ -18,12 +18,19 @@ or adding ssh key. But you can also enable extra triggers like Push events. .prepend-top-default + = form.check_box :repository_update_events, class: 'pull-left' + .prepend-left-20 + = form.label :repository_update_events, class: 'list-label' do + %strong Repository update events + %p.light + This url will be triggered when repository is updated + %div = form.check_box :push_events, class: 'pull-left' .prepend-left-20 = form.label :push_events, class: 'list-label' do %strong Push events %p.light - This url will be triggered by a push to the repository + This url will be triggered for each branch updated to the repository %div = form.check_box :tag_push_events, class: 'pull-left' .prepend-left-20 diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index 71117758921..43df8b62367 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -27,7 +27,7 @@ = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm' .monospace= hook.url %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| + - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events repository_update_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray= trigger.titleize %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'} diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb new file mode 100644 index 00000000000..c0e7246b2ec --- /dev/null +++ b/spec/controllers/admin/hooks_controller_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +describe Admin::HooksController do + let(:admin) { create(:admin) } + + before do + sign_in(admin) + end + + describe 'POST #create' do + it 'sets all parameters' do + hook_params = { + enable_ssl_verification: true, + push_events: true, + tag_push_events: true, + repository_update_events: true, + token: "TEST TOKEN", + url: "http://example.com", + } + + post :create, hook: hook_params + + expect(response).to have_http_status(302) + expect(SystemHook.all.size).to eq(1) + expect(SystemHook.first).to have_attributes(hook_params) + end + end +end diff --git a/spec/models/hooks/system_hook_spec.rb b/spec/models/hooks/system_hook_spec.rb index a9e3ac3b679..4340170888d 100644 --- a/spec/models/hooks/system_hook_spec.rb +++ b/spec/models/hooks/system_hook_spec.rb @@ -1,6 +1,19 @@ require "spec_helper" describe SystemHook, models: true do + context 'default attributes' do + let(:system_hook) { build(:system_hook) } + + it 'sets defined default parameters' do + attrs = { + push_events: false, + repository_update_events: true, + enable_ssl_verification: true + } + expect(system_hook).to have_attributes(attrs) + end + end + describe "execute" do let(:system_hook) { create(:system_hook) } let(:user) { create(:user) } -- cgit v1.2.1 From cfb4404ef6464ed4780bc7619268287716ae5e73 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Fri, 5 May 2017 18:00:32 +0200 Subject: Update SystemHooks API to expose and handle new repository_update_events --- lib/api/entities.rb | 2 +- spec/requests/api/system_hooks_spec.rb | 3 ++- spec/requests/api/v3/system_hooks_spec.rb | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 00d494f02f5..3fc2b453eb6 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -53,7 +53,7 @@ module API end class Hook < Grape::Entity - expose :id, :url, :created_at, :push_events, :tag_push_events + expose :id, :url, :created_at, :push_events, :tag_push_events, :repository_update_events expose :enable_ssl_verification end diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index c7b84173570..2eb191d6049 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -32,8 +32,9 @@ describe API::SystemHooks do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.first['url']).to eq(hook.url) - expect(json_response.first['push_events']).to be true + expect(json_response.first['push_events']).to be false expect(json_response.first['tag_push_events']).to be false + expect(json_response.first['repository_update_events']).to be true end end end diff --git a/spec/requests/api/v3/system_hooks_spec.rb b/spec/requests/api/v3/system_hooks_spec.rb index 72c7d14b8ba..ae427541abb 100644 --- a/spec/requests/api/v3/system_hooks_spec.rb +++ b/spec/requests/api/v3/system_hooks_spec.rb @@ -31,8 +31,9 @@ describe API::V3::SystemHooks do expect(response).to have_http_status(200) expect(json_response).to be_an Array expect(json_response.first['url']).to eq(hook.url) - expect(json_response.first['push_events']).to be true + expect(json_response.first['push_events']).to be false expect(json_response.first['tag_push_events']).to be false + expect(json_response.first['repository_update_events']).to be true end end end -- cgit v1.2.1 From 7045f1af45ffc8180ca6a66a467b30dd38c21c0f Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 6 May 2017 02:25:09 +0200 Subject: Changelog --- changelogs/unreleased/26325-system-hooks.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/26325-system-hooks.yml diff --git a/changelogs/unreleased/26325-system-hooks.yml b/changelogs/unreleased/26325-system-hooks.yml new file mode 100644 index 00000000000..62b8adaeccd --- /dev/null +++ b/changelogs/unreleased/26325-system-hooks.yml @@ -0,0 +1,4 @@ +--- +title: 'Backported new SystemHook event: `repository_update`' +merge_request: 11140 +author: -- cgit v1.2.1 From 551c000d6175d037cb896115d920dfd9f0c79e20 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 6 May 2017 02:48:03 +0200 Subject: Documentation for repository_update_events --- doc/system_hooks/system_hooks.md | 50 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/doc/system_hooks/system_hooks.md b/doc/system_hooks/system_hooks.md index ad5ffc84473..583ec5522fd 100644 --- a/doc/system_hooks/system_hooks.md +++ b/doc/system_hooks/system_hooks.md @@ -266,7 +266,8 @@ X-Gitlab-Event: System Hook ## Push events -Triggered when you push to the repository except when pushing tags. +Triggered when you push to the repository, except when pushing tags. +It generates one event per modified branch. **Request header**: @@ -332,6 +333,7 @@ X-Gitlab-Event: System Hook ## Tag events Triggered when you create (or delete) tags to the repository. +It generates one event per modified tag. **Request header**: @@ -381,3 +383,49 @@ X-Gitlab-Event: System Hook "total_commits_count": 0 } ``` +## Repository Update events + +Triggered only once when you push to the repository (including tags). + +**Request header**: + +``` +X-Gitlab-Event: System Hook +``` + +**Request body:** + +```json +{ + "event_name": "repository_update", + "user_id": 1, + "user_name": "John Smith", + "user_email": "admin@example.com", + "user_avatar": "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80", + "project_id": 1, + "project": { + "name":"Example", + "description":"", + "web_url":"http://example.com/jsmith/example", + "avatar_url":null, + "git_ssh_url":"git@example.com:jsmith/example.git", + "git_http_url":"http://example.com/jsmith/example.git", + "namespace":"Jsmith", + "visibility_level":0, + "path_with_namespace":"jsmith/example", + "default_branch":"master", + "homepage":"http://example.com/jsmith/example", + "url":"git@example.com:jsmith/example.git", + "ssh_url":"git@example.com:jsmith/example.git", + "http_url":"http://example.com/jsmith/example.git", + }, + "changes": [ + { + "before":"8205ea8d81ce0c6b90fbe8280d118cc9fdad6130", + "after":"4045ea7a3df38697b3730a20fb73c8bed8a3e69e", + "ref":"refs/heads/master" + } + ], + "refs":["refs/heads/master"] +} +``` -- cgit v1.2.1 From dffeb1a48db1a3becc8e11885fd28c0b5030eed7 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 10 May 2017 02:53:16 +0100 Subject: Minor cosmetic fixes in hooks admin screen --- app/views/admin/hooks/_form.html.haml | 6 +++--- app/views/admin/hooks/index.html.haml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/admin/hooks/_form.html.haml b/app/views/admin/hooks/_form.html.haml index 7f7377b1da1..645005c6deb 100644 --- a/app/views/admin/hooks/_form.html.haml +++ b/app/views/admin/hooks/_form.html.haml @@ -23,21 +23,21 @@ = form.label :repository_update_events, class: 'list-label' do %strong Repository update events %p.light - This url will be triggered when repository is updated + This URL will be triggered when repository is updated %div = form.check_box :push_events, class: 'pull-left' .prepend-left-20 = form.label :push_events, class: 'list-label' do %strong Push events %p.light - This url will be triggered for each branch updated to the repository + This URL will be triggered for each branch updated to the repository %div = form.check_box :tag_push_events, class: 'pull-left' .prepend-left-20 = form.label :tag_push_events, class: 'list-label' do %strong Tag push events %p.light - This url will be triggered when a new tag is pushed to the repository + This URL will be triggered when a new tag is pushed to the repository .form-group = form.label :enable_ssl_verification, 'SSL verification', class: 'control-label checkbox' .col-sm-10 diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index 43df8b62367..3338b677bf5 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -27,7 +27,7 @@ = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm' .monospace= hook.url %div - - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events repository_update_events).each do |trigger| + - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray= trigger.titleize %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'} -- cgit v1.2.1 From 44129aceaf6d26ae52834548072a91f6fc62f74b Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Sat, 13 May 2017 02:18:57 +0100 Subject: Codestyle --- spec/controllers/admin/hooks_controller_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/controllers/admin/hooks_controller_spec.rb b/spec/controllers/admin/hooks_controller_spec.rb index c0e7246b2ec..1d1070e90f4 100644 --- a/spec/controllers/admin/hooks_controller_spec.rb +++ b/spec/controllers/admin/hooks_controller_spec.rb @@ -15,7 +15,7 @@ describe Admin::HooksController do tag_push_events: true, repository_update_events: true, token: "TEST TOKEN", - url: "http://example.com", + url: "http://example.com" } post :create, hook: hook_params -- cgit v1.2.1