From 020ea32e767b9ad033f9fedcaa902865a01fa944 Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Tue, 2 Aug 2016 18:06:31 +0800
Subject: [PATCH] Implement pipeline hooks, extracted from !5525
Closes #20115
---
CHANGELOG | 1 +
app/controllers/concerns/service_params.rb | 15 ++---
app/controllers/projects/hooks_controller.rb | 1 +
app/models/ci/pipeline.rb | 10 ++-
app/models/hooks/project_hook.rb | 1 +
app/models/hooks/web_hook.rb | 1 +
app/models/service.rb | 5 ++
app/services/ci/create_pipeline_service.rb | 1 +
.../projects/hooks/_project_hook.html.haml | 2 +-
app/views/shared/web_hooks/_form.html.haml | 7 ++
...081025_add_pipeline_events_to_web_hooks.rb | 16 +++++
...8103734_add_pipeline_events_to_services.rb | 16 +++++
lib/api/entities.rb | 6 +-
lib/api/project_hooks.rb | 2 +
.../data_builder/pipeline_data_builder.rb | 66 +++++++++++++++++++
.../pipeline_data_builder_spec.rb | 32 +++++++++
spec/models/build_spec.rb | 6 +-
spec/models/ci/pipeline_spec.rb | 33 +++++++++-
spec/requests/api/project_hooks_spec.rb | 7 +-
19 files changed, 210 insertions(+), 18 deletions(-)
create mode 100644 db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
create mode 100644 db/migrate/20160728103734_add_pipeline_events_to_services.rb
create mode 100644 lib/gitlab/data_builder/pipeline_data_builder.rb
create mode 100644 spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
diff --git a/CHANGELOG b/CHANGELOG
index c099c63ce86..3199e66a0d9 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -28,6 +28,7 @@ v 8.11.0 (unreleased)
- The overhead of instrumented method calls has been reduced
- Remove `search_id` of labels dropdown filter to fix 'Missleading URI for labels in Merge Requests and Issues view'. !5368 (Scott Le)
- Load project invited groups and members eagerly in `ProjectTeam#fetch_members`
+ - Add pipeline events hook
- Bump gitlab_git to speedup DiffCollection iterations
- Make branches sortable without push permission !5462 (winniehell)
- Check for Ci::Build artifacts at database level on pipeline partial
diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb
index 471d15af913..58877c5ad5d 100644
--- a/app/controllers/concerns/service_params.rb
+++ b/app/controllers/concerns/service_params.rb
@@ -7,11 +7,12 @@ module ServiceParams
:build_key, :server, :teamcity_url, :drone_url, :build_type,
:description, :issues_url, :new_issue_url, :restrict_to_branch, :channel,
:colorize_messages, :channels,
- :push_events, :issues_events, :merge_requests_events, :tag_push_events,
- :note_events, :build_events, :wiki_page_events,
- :notify_only_broken_builds, :add_pusher,
- :send_from_committer_email, :disable_diffs, :external_wiki_url,
- :notify, :color,
+ # See app/helpers/services_helper.rb
+ # for why we need issues_events and merge_requests_events.
+ :issues_events, :merge_requests_events,
+ :notify_only_broken_builds, :notify_only_broken_pipelines,
+ :add_pusher, :send_from_committer_email, :disable_diffs,
+ :external_wiki_url, :notify, :color,
:server_host, :server_port, :default_irc_uri, :enable_ssl_verification,
:jira_issue_transition_id]
@@ -19,9 +20,7 @@ module ServiceParams
FILTER_BLANK_PARAMS = [:password]
def service_params
- dynamic_params = []
- dynamic_params.concat(@service.event_channel_names)
-
+ dynamic_params = @service.event_channel_names + @service.event_names
service_params = params.permit(:id, service: ALLOWED_PARAMS + dynamic_params)
if service_params[:service].is_a?(Hash)
diff --git a/app/controllers/projects/hooks_controller.rb b/app/controllers/projects/hooks_controller.rb
index a60027ff477..b5624046387 100644
--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@@ -56,6 +56,7 @@ class Projects::HooksController < Projects::ApplicationController
def hook_params
params.require(:hook).permit(
:build_events,
+ :pipeline_events,
:enable_ssl_verification,
:issues_events,
:merge_requests_events,
diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index bce6a992af6..4e6ccf48c68 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -237,7 +237,15 @@ module Ci
self.started_at = statuses.started_at
self.finished_at = statuses.finished_at
self.duration = statuses.latest.duration
- save
+ saved = save
+ execute_hooks if saved && !skip_ci?
+ saved
+ end
+
+ def execute_hooks
+ pipeline_data = Gitlab::DataBuilder::PipelineDataBuilder.build(self)
+ project.execute_hooks(pipeline_data, :pipeline_hooks)
+ project.execute_services(pipeline_data.dup, :pipeline_hooks)
end
def keep_around_commits
diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb
index ba42a8eeb70..836a75b0608 100644
--- a/app/models/hooks/project_hook.rb
+++ b/app/models/hooks/project_hook.rb
@@ -5,5 +5,6 @@ class ProjectHook < WebHook
scope :note_hooks, -> { where(note_events: true) }
scope :merge_request_hooks, -> { where(merge_requests_events: true) }
scope :build_hooks, -> { where(build_events: true) }
+ scope :pipeline_hooks, -> { where(pipeline_events: true) }
scope :wiki_page_hooks, -> { where(wiki_page_events: true) }
end
diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb
index 8b87b6c3d64..f365dee3141 100644
--- a/app/models/hooks/web_hook.rb
+++ b/app/models/hooks/web_hook.rb
@@ -8,6 +8,7 @@ class WebHook < ActiveRecord::Base
default_value_for :merge_requests_events, false
default_value_for :tag_push_events, false
default_value_for :build_events, false
+ default_value_for :pipeline_events, false
default_value_for :enable_ssl_verification, true
scope :push_hooks, -> { where(push_events: true) }
diff --git a/app/models/service.rb b/app/models/service.rb
index 40cd9b861f0..e4cd44f542a 100644
--- a/app/models/service.rb
+++ b/app/models/service.rb
@@ -36,6 +36,7 @@ class Service < ActiveRecord::Base
scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) }
scope :note_hooks, -> { where(note_events: true, active: true) }
scope :build_hooks, -> { where(build_events: true, active: true) }
+ scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) }
scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) }
scope :external_issue_trackers, -> { issue_trackers.active.without_defaults }
@@ -86,6 +87,10 @@ class Service < ActiveRecord::Base
[]
end
+ def event_names
+ supported_events.map { |event| "#{event}_events" }
+ end
+
def event_field(event)
nil
end
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index be91bf0db85..7a8b0683acb 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -27,6 +27,7 @@ module Ci
end
pipeline.save!
+ pipeline.touch
unless pipeline.create_builds(current_user)
pipeline.errors.add(:base, 'No builds for this pipeline.')
diff --git a/app/views/projects/hooks/_project_hook.html.haml b/app/views/projects/hooks/_project_hook.html.haml
index 8151187d499..3fcf1692e09 100644
--- a/app/views/projects/hooks/_project_hook.html.haml
+++ b/app/views/projects/hooks/_project_hook.html.haml
@@ -3,7 +3,7 @@
.col-md-8.col-lg-7
%strong.light-header= hook.url
%div
- - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events wiki_page_events).each do |trigger|
+ - %w(push_events tag_push_events issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger|
- if hook.send(trigger)
%span.label.label-gray.deploy-project-label= trigger.titleize
.col-md-4.col-lg-5.text-right-lg.prepend-top-5
diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml
index 2585ed9360b..106161d6515 100644
--- a/app/views/shared/web_hooks/_form.html.haml
+++ b/app/views/shared/web_hooks/_form.html.haml
@@ -65,6 +65,13 @@
%strong Build events
%p.light
This url will be triggered when the build status changes
+ %li
+ = f.check_box :pipeline_events, class: 'pull-left'
+ .prepend-left-20
+ = f.label :pipeline_events, class: 'list-label' do
+ %strong Pipeline events
+ %p.light
+ This url will be triggered when the pipeline status changes
%li
= f.check_box :wiki_page_events, class: 'pull-left'
.prepend-left-20
diff --git a/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
new file mode 100644
index 00000000000..b800e6d7283
--- /dev/null
+++ b/db/migrate/20160728081025_add_pipeline_events_to_web_hooks.rb
@@ -0,0 +1,16 @@
+class AddPipelineEventsToWebHooks < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ add_column_with_default(:web_hooks, :pipeline_events, :boolean,
+ default: false, allow_null: false)
+ end
+
+ def down
+ remove_column(:web_hooks, :pipeline_events)
+ end
+end
diff --git a/db/migrate/20160728103734_add_pipeline_events_to_services.rb b/db/migrate/20160728103734_add_pipeline_events_to_services.rb
new file mode 100644
index 00000000000..bcd24fe1566
--- /dev/null
+++ b/db/migrate/20160728103734_add_pipeline_events_to_services.rb
@@ -0,0 +1,16 @@
+class AddPipelineEventsToServices < ActiveRecord::Migration
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ add_column_with_default(:services, :pipeline_events, :boolean,
+ default: false, allow_null: false)
+ end
+
+ def down
+ remove_column(:services, :pipeline_events)
+ end
+end
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 3e21b7a0b8a..b6f6b11d97b 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -48,7 +48,8 @@ module API
class ProjectHook < Hook
expose :project_id, :push_events
- expose :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events
+ expose :issues_events, :merge_requests_events, :tag_push_events
+ expose :note_events, :build_events, :pipeline_events
expose :enable_ssl_verification
end
@@ -342,7 +343,8 @@ module API
class ProjectService < Grape::Entity
expose :id, :title, :created_at, :updated_at, :active
- expose :push_events, :issues_events, :merge_requests_events, :tag_push_events, :note_events, :build_events
+ expose :push_events, :issues_events, :merge_requests_events
+ expose :tag_push_events, :note_events, :build_events, :pipeline_events
# Expose serialized properties
expose :properties do |service, options|
field_names = service.fields.
diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb
index 6bb70bc8bc3..3f63cd678e8 100644
--- a/lib/api/project_hooks.rb
+++ b/lib/api/project_hooks.rb
@@ -45,6 +45,7 @@ module API
:tag_push_events,
:note_events,
:build_events,
+ :pipeline_events,
:enable_ssl_verification
]
@hook = user_project.hooks.new(attrs)
@@ -78,6 +79,7 @@ module API
:tag_push_events,
:note_events,
:build_events,
+ :pipeline_events,
:enable_ssl_verification
]
diff --git a/lib/gitlab/data_builder/pipeline_data_builder.rb b/lib/gitlab/data_builder/pipeline_data_builder.rb
new file mode 100644
index 00000000000..13417ba09eb
--- /dev/null
+++ b/lib/gitlab/data_builder/pipeline_data_builder.rb
@@ -0,0 +1,66 @@
+module Gitlab
+ module DataBuilder
+ module PipelineDataBuilder
+ module_function
+
+ def build(pipeline)
+ {
+ object_kind: 'pipeline',
+ object_attributes: hook_attrs(pipeline),
+ user: pipeline.user.try(:hook_attrs),
+ project: pipeline.project.hook_attrs(backward: false),
+ commit: pipeline.commit.try(:hook_attrs),
+ builds: pipeline.builds.map(&method(:build_hook_attrs))
+ }
+ end
+
+ def hook_attrs(pipeline)
+ first_pending_build = pipeline.builds.first_pending
+ config_processor = pipeline.config_processor
+
+ {
+ id: pipeline.id,
+ ref: pipeline.ref,
+ tag: pipeline.tag,
+ sha: pipeline.sha,
+ before_sha: pipeline.before_sha,
+ status: pipeline.status,
+ stage: first_pending_build.try(:stage),
+ stages: config_processor.try(:stages),
+ created_at: pipeline.created_at,
+ finished_at: pipeline.finished_at,
+ duration: pipeline.duration
+ }
+ end
+
+ def build_hook_attrs(build)
+ {
+ id: build.id,
+ stage: build.stage,
+ name: build.name,
+ status: build.status,
+ created_at: build.created_at,
+ started_at: build.started_at,
+ finished_at: build.finished_at,
+ when: build.when,
+ manual: build.manual?,
+ user: build.user.try(:hook_attrs),
+ runner: build.runner && runner_hook_attrs(build.runner),
+ artifacts_file: {
+ filename: build.artifacts_file.filename,
+ size: build.artifacts_size
+ }
+ }
+ end
+
+ def runner_hook_attrs(runner)
+ {
+ id: runner.id,
+ description: runner.description,
+ active: runner.active?,
+ is_shared: runner.is_shared?
+ }
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
new file mode 100644
index 00000000000..24d39b318c0
--- /dev/null
+++ b/spec/lib/gitlab/data_builder/pipeline_data_builder_spec.rb
@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe Gitlab::DataBuilder::PipelineDataBuilder do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let(:pipeline) do
+ create(:ci_pipeline,
+ project: project, status: 'success',
+ sha: project.commit.sha, ref: project.default_branch)
+ end
+ let!(:build) { create(:ci_build, pipeline: pipeline) }
+
+ describe '.build' do
+ let(:data) { Gitlab::DataBuilder::PipelineDataBuilder.build(pipeline) }
+ let(:attributes) { data[:object_attributes] }
+ let(:build_data) { data[:builds].first }
+ let(:project_data) { data[:project] }
+
+ it { expect(attributes).to be_a(Hash) }
+ it { expect(attributes[:ref]).to eq(pipeline.ref) }
+ it { expect(attributes[:sha]).to eq(pipeline.sha) }
+ it { expect(attributes[:tag]).to eq(pipeline.tag) }
+ it { expect(attributes[:id]).to eq(pipeline.id) }
+ it { expect(attributes[:status]).to eq(pipeline.status) }
+
+ it { expect(build_data).to be_a(Hash) }
+ it { expect(build_data[:id]).to eq(build.id) }
+ it { expect(build_data[:status]).to eq(build.status) }
+
+ it { expect(project_data).to eq(project.hook_attrs(backward: false)) }
+ end
+end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index dc88697199b..47c489e6af1 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -275,7 +275,8 @@ describe Ci::Build, models: true do
context 'when yaml_variables are undefined' do
before do
- build.yaml_variables = nil
+ build.update(yaml_variables: nil)
+ build.reload # reload pipeline so that it resets config_processor
end
context 'use from gitlab-ci.yml' do
@@ -854,7 +855,8 @@ describe Ci::Build, models: true do
context 'if is undefined' do
before do
- build.when = nil
+ build.update(when: nil)
+ build.reload # reload pipeline so that it resets config_processor
end
context 'use from gitlab-ci.yml' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 0d4c86955ce..aa05fc78f94 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -513,7 +513,7 @@ describe Ci::Pipeline, models: true do
create :ci_build, :success, pipeline: pipeline, name: 'rspec'
create :ci_build, :allowed_to_fail, :failed, pipeline: pipeline, name: 'rubocop'
end
-
+
it 'returns true' do
is_expected.to be_truthy
end
@@ -524,7 +524,7 @@ describe Ci::Pipeline, models: true do
create :ci_build, :success, pipeline: pipeline, name: 'rspec'
create :ci_build, :allowed_to_fail, :success, pipeline: pipeline, name: 'rubocop'
end
-
+
it 'returns false' do
is_expected.to be_falsey
end
@@ -542,4 +542,33 @@ describe Ci::Pipeline, models: true do
end
end
end
+
+ describe '#execute_hooks' do
+ let!(:hook) do
+ create(:project_hook, project: project, pipeline_events: enabled)
+ end
+ let(:enabled) { raise NotImplementedError }
+
+ before do
+ WebMock.stub_request(:post, hook.url)
+ pipeline.touch
+ ProjectWebHookWorker.drain
+ end
+
+ context 'with pipeline hooks enabled' do
+ let(:enabled) { true }
+
+ it 'executes pipeline_hook after touched' do
+ expect(WebMock).to have_requested(:post, hook.url).once
+ end
+ end
+
+ context 'with pipeline hooks disabled' do
+ let(:enabled) { false }
+
+ it 'did not execute pipeline_hook after touched' do
+ expect(WebMock).not_to have_requested(:post, hook.url)
+ end
+ end
+ end
end
diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb
index fd1fffa6223..504deed81f9 100644
--- a/spec/requests/api/project_hooks_spec.rb
+++ b/spec/requests/api/project_hooks_spec.rb
@@ -8,8 +8,9 @@ describe API::API, 'ProjectHooks', api: true do
let!(:hook) do
create(:project_hook,
project: project, url: "http://example.com",
- push_events: true, merge_requests_events: true, tag_push_events: true,
- issues_events: true, note_events: true, build_events: true,
+ push_events: true, merge_requests_events: true,
+ tag_push_events: true, issues_events: true, note_events: true,
+ build_events: true, pipeline_events: true,
enable_ssl_verification: true)
end
@@ -33,6 +34,7 @@ describe API::API, 'ProjectHooks', api: true do
expect(json_response.first['tag_push_events']).to eq(true)
expect(json_response.first['note_events']).to eq(true)
expect(json_response.first['build_events']).to eq(true)
+ expect(json_response.first['pipeline_events']).to eq(true)
expect(json_response.first['enable_ssl_verification']).to eq(true)
end
end
@@ -91,6 +93,7 @@ describe API::API, 'ProjectHooks', api: true do
expect(json_response['tag_push_events']).to eq(false)
expect(json_response['note_events']).to eq(false)
expect(json_response['build_events']).to eq(false)
+ expect(json_response['pipeline_events']).to eq(false)
expect(json_response['enable_ssl_verification']).to eq(true)
end
--
GitLab