From c33f09d2549d2228a5ac7ceb7cb099774fbd826e Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 18:49:30 +0100 Subject: [PATCH 01/13] Update triggers API --- lib/api/entities.rb | 4 - lib/api/triggers.rb | 68 ++++++++-- lib/api/v3/entities.rb | 8 ++ lib/api/v3/triggers.rb | 77 +++++++++++- spec/requests/api/v3/triggers_spec.rb | 171 ++++++++++++++++++++++++++ 5 files changed, 311 insertions(+), 17 deletions(-) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index d2d21f5d03a..82d05d85ead 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -592,10 +592,6 @@ module API end end - class TriggerRequest < Grape::Entity - expose :id, :variables - end - class Runner < Grape::Entity expose :id expose :description diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index b7c9c5f2b7f..c324708a16d 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -6,7 +6,7 @@ module API requires :id, type: String, desc: 'The ID of a project' end resource :projects do - desc 'Trigger a GitLab project build' do + desc 'Trigger a GitLab project pipeline' do success Entities::TriggerRequest end params do @@ -14,7 +14,7 @@ module API requires :token, type: String, desc: 'The unique token of trigger' optional :variables, type: Hash, desc: 'The list of variables to be injected into build' end - post ":id/(ref/:ref/)trigger/builds" do + post ":id/(ref/:ref/)trigger/pipeline" do project = find_project(params[:id]) trigger = Ci::Trigger.find_by_token(params[:token].to_s) not_found! unless project && trigger @@ -29,9 +29,9 @@ module API # create request and trigger builds trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) if trigger_request - present trigger_request, with: Entities::TriggerRequest + present trigger_request.pipeline, with: Entities::Pipeline else - errors = 'No builds created' + errors = 'No pipeline create' render_api_error!(errors, 400) end end @@ -55,13 +55,13 @@ module API success Entities::Trigger end params do - requires :token, type: String, desc: 'The unique token of trigger' + requires :trigger_id, type: Integer, desc: 'The trigger ID' end - get ':id/triggers/:token' do + get ':id/triggers/:trigger_id' do authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.find_by(token: params[:token].to_s) + trigger = user_project.triggers.find(params[:trigger_id]) return not_found!('Trigger') unless trigger present trigger, with: Entities::Trigger @@ -70,26 +70,72 @@ module API desc 'Create a trigger' do success Entities::Trigger end + params do + requires :description, type: String, desc: 'The trigger description' + end post ':id/triggers' do authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.create + trigger = user_project.triggers.create( + declared_params(include_missing: false).merge(owner: current_user)) + if trigger.valid? + present trigger, with: Entities::Trigger + else + render_validation_error!(trigger) + end + end + + desc 'Update a trigger' do + success Entities::Trigger + end + params do + requires :trigger_id, type: Integer, desc: 'The trigger ID' + optional :description, type: String, desc: 'The trigger description' + end + delete ':id/triggers/:trigger_id' do + authenticate! + authorize! :admin_build, user_project + + trigger = user_project.triggers.find(params[:trigger_id]) + return not_found!('Trigger') unless trigger + + trigger = trigger.update(declared_params(include_missing: false)) present trigger, with: Entities::Trigger end + desc 'Take ownership of trigger' do + success Entities::Trigger + end + params do + requires :trigger_id, type: Integer, desc: 'The trigger ID' + end + post ':id/triggers/:trigger_id/take' do + authenticate! + authorize! :admin_build, user_project + + trigger = user_project.triggers.find(params[:trigger_id]) + return not_found!('Trigger') unless trigger + + if trigger.update(owner: current_user) + present trigger, with: Entities::Trigger + else + render_validation_error!(trigger) + end + end + desc 'Delete a trigger' do success Entities::Trigger end params do - requires :token, type: String, desc: 'The unique token of trigger' + requires :trigger_id, type: Integer, desc: 'The trigger ID' end - delete ':id/triggers/:token' do + delete ':id/triggers/:trigger_id' do authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.find_by(token: params[:token].to_s) + trigger = user_project.triggers.find(params[:trigger_id]) return not_found!('Trigger') unless trigger trigger.destroy diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 270d99a2348..29a44d4c7e5 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -186,6 +186,14 @@ module API class Environment < ::API::Entities::EnvironmentBasic expose :project, using: Entities::Project end + + class Trigger < Grape::Entity + expose :token, :created_at, :updated_at, :deleted_at, :last_used + end + + class TriggerRequest < Grape::Entity + expose :id, :variables + end end end end diff --git a/lib/api/v3/triggers.rb b/lib/api/v3/triggers.rb index 4051d4bca8d..1dfdb6a5956 100644 --- a/lib/api/v3/triggers.rb +++ b/lib/api/v3/triggers.rb @@ -7,8 +7,81 @@ module API requires :id, type: String, desc: 'The ID of a project' end resource :projects do + desc 'Trigger a GitLab project build' do + success ::API::V3::Entities::TriggerRequest + end + params do + requires :ref, type: String, desc: 'The commit sha or name of a branch or tag' + requires :token, type: String, desc: 'The unique token of trigger' + optional :variables, type: Hash, desc: 'The list of variables to be injected into build' + end + post ":id/(ref/:ref/)trigger/builds" do + project = find_project(params[:id]) + trigger = Ci::Trigger.find_by_token(params[:token].to_s) + not_found! unless project && trigger + unauthorized! unless trigger.project == project + + # validate variables + variables = params[:variables].to_h + unless variables.all? { |key, value| key.is_a?(String) && value.is_a?(String) } + render_api_error!('variables needs to be a map of key-valued strings', 400) + end + + # create request and trigger builds + trigger_request = Ci::CreateTriggerRequestService.new.execute(project, trigger, params[:ref].to_s, variables) + if trigger_request + present trigger_request, with: ::API::V3::Entities::TriggerRequest + else + errors = 'No builds created' + render_api_error!(errors, 400) + end + end + + desc 'Get triggers list' do + success ::API::V3::Entities::Trigger + end + params do + use :pagination + end + get ':id/triggers' do + authenticate! + authorize! :admin_build, user_project + + triggers = user_project.triggers.includes(:trigger_requests) + + present paginate(triggers), with: ::API::V3::Entities::Trigger + end + + desc 'Get specific trigger of a project' do + success ::API::V3::Entities::Trigger + end + params do + requires :token, type: String, desc: 'The unique token of trigger' + end + get ':id/triggers/:token' do + authenticate! + authorize! :admin_build, user_project + + trigger = user_project.triggers.find_by(token: params[:token].to_s) + return not_found!('Trigger') unless trigger + + present trigger, with: ::API::V3::Entities::Trigger + end + + desc 'Create a trigger' do + success ::API::V3::Entities::Trigger + end + post ':id/triggers' do + authenticate! + authorize! :admin_build, user_project + + trigger = user_project.triggers.create + + present trigger, with: ::API::V3::Entities::Trigger + end + desc 'Delete a trigger' do - success ::API::Entities::Trigger + success ::API::V3::Entities::Trigger end params do requires :token, type: String, desc: 'The unique token of trigger' @@ -22,7 +95,7 @@ module API trigger.destroy - present trigger, with: ::API::Entities::Trigger + present trigger, with: ::API::V3::Entities::Trigger end end end diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index 721ce4a361b..4819269d69f 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -11,6 +11,177 @@ describe API::V3::Triggers do let!(:developer) { create(:project_member, :developer, user: user2, project: project) } let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + describe 'POST /projects/:project_id/trigger' do + let!(:project2) { create(:project) } + let(:options) do + { + token: trigger_token + } + end + + before do + stub_ci_pipeline_to_return_yaml_file + end + + context 'Handles errors' do + it 'returns bad request if token is missing' do + post v3_api("/projects/#{project.id}/trigger/builds"), ref: 'master' + expect(response).to have_http_status(400) + end + + it 'returns not found if project is not found' do + post v3_api('/projects/0/trigger/builds'), options.merge(ref: 'master') + expect(response).to have_http_status(404) + end + + it 'returns unauthorized if token is for different project' do + post v3_api("/projects/#{project2.id}/trigger/builds"), options.merge(ref: 'master') + expect(response).to have_http_status(401) + end + end + + context 'Have a commit' do + let(:pipeline) { project.pipelines.last } + + it 'creates builds' do + post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'master') + expect(response).to have_http_status(201) + pipeline.builds.reload + expect(pipeline.builds.pending.size).to eq(2) + expect(pipeline.builds.size).to eq(5) + end + + it 'creates builds on webhook from other gitlab repository and branch' do + expect do + post v3_api("/projects/#{project.id}/ref/master/trigger/builds?token=#{trigger_token}"), { ref: 'refs/heads/other-branch' } + end.to change(project.builds, :count).by(5) + expect(response).to have_http_status(201) + end + + it 'returns bad request with no builds created if there\'s no commit for that ref' do + post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'other-branch') + expect(response).to have_http_status(400) + expect(json_response['message']).to eq('No builds created') + end + + context 'Validates variables' do + let(:variables) do + { 'TRIGGER_KEY' => 'TRIGGER_VALUE' } + end + + it 'validates variables to be a hash' do + post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(variables: 'value', ref: 'master') + expect(response).to have_http_status(400) + expect(json_response['error']).to eq('variables is invalid') + end + + it 'validates variables needs to be a map of key-valued strings' do + post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(variables: { key: %w(1 2) }, ref: 'master') + expect(response).to have_http_status(400) + expect(json_response['message']).to eq('variables needs to be a map of key-valued strings') + end + + it 'creates trigger request with variables' do + post v3_api("/projects/#{project.id}/trigger/builds"), options.merge(variables: variables, ref: 'master') + expect(response).to have_http_status(201) + pipeline.builds.reload + expect(pipeline.builds.first.trigger_request.variables).to eq(variables) + end + end + end + end + + describe 'GET /projects/:id/triggers' do + context 'authenticated user with valid permissions' do + it 'returns list of triggers' do + get v3_api("/projects/#{project.id}/triggers", user) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_a(Array) + expect(json_response[0]).to have_key('token') + end + end + + context 'authenticated user with invalid permissions' do + it 'does not return triggers list' do + get v3_api("/projects/#{project.id}/triggers", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not return triggers list' do + get v3_api("/projects/#{project.id}/triggers") + + expect(response).to have_http_status(401) + end + end + end + + describe 'GET /projects/:id/triggers/:token' do + context 'authenticated user with valid permissions' do + it 'returns trigger details' do + get v3_api("/projects/#{project.id}/triggers/#{trigger.token}", user) + + expect(response).to have_http_status(200) + expect(json_response).to be_a(Hash) + end + + it 'responds with 404 Not Found if requesting non-existing trigger' do + get v3_api("/projects/#{project.id}/triggers/abcdef012345", user) + + expect(response).to have_http_status(404) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not return triggers list' do + get v3_api("/projects/#{project.id}/triggers/#{trigger.token}", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not return triggers list' do + get v3_api("/projects/#{project.id}/triggers/#{trigger.token}") + + expect(response).to have_http_status(401) + end + end + end + + describe 'POST /projects/:id/triggers' do + context 'authenticated user with valid permissions' do + it 'creates trigger' do + expect do + post v3_api("/projects/#{project.id}/triggers", user) + end.to change{project.triggers.count}.by(1) + + expect(response).to have_http_status(201) + expect(json_response).to be_a(Hash) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not create trigger' do + post v3_api("/projects/#{project.id}/triggers", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not create trigger' do + post v3_api("/projects/#{project.id}/triggers") + + expect(response).to have_http_status(401) + end + end + end + describe 'DELETE /projects/:id/triggers/:token' do context 'authenticated user with valid permissions' do it 'deletes trigger' do From 0a75de2909152351483b317024fcab2e9bb08e16 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 15 Feb 2017 19:36:31 +0100 Subject: [PATCH 02/13] Make Pipeline Triggers to be user aware - they can have owner, - they can be edited, - they have description, - you can take ownership of them --- app/models/ci/trigger.rb | 12 +++++++++--- .../20170215164610_add_owner_id_to_triggers.rb | 10 ++++++++++ .../20170215165036_add_description_to_triggers.rb | 9 +++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20170215164610_add_owner_id_to_triggers.rb create mode 100644 db/migrate/20170215165036_add_description_to_triggers.rb diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index 39a1dd86241..f76f06eb9c6 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -5,10 +5,12 @@ module Ci acts_as_paranoid belongs_to :project, foreign_key: :gl_project_id + belongs_to :owner, class_name: "User" + has_many :trigger_requests, dependent: :destroy - validates :token, presence: true - validates :token, uniqueness: true + validates :token, presence: true, uniqueness: true + validates :owner, presence: true before_validation :set_default_values @@ -25,7 +27,11 @@ module Ci end def short_token - token[0...10] + token[0...4] + end + + def can_show_token?(user) + owner.blank? || owner == user end end end diff --git a/db/migrate/20170215164610_add_owner_id_to_triggers.rb b/db/migrate/20170215164610_add_owner_id_to_triggers.rb new file mode 100644 index 00000000000..02c77ee4b5e --- /dev/null +++ b/db/migrate/20170215164610_add_owner_id_to_triggers.rb @@ -0,0 +1,10 @@ +class AddOwnerIdToTriggers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_triggers, :owner_id, :integer + add_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :nullify + end +end diff --git a/db/migrate/20170215165036_add_description_to_triggers.rb b/db/migrate/20170215165036_add_description_to_triggers.rb new file mode 100644 index 00000000000..1dca0e37412 --- /dev/null +++ b/db/migrate/20170215165036_add_description_to_triggers.rb @@ -0,0 +1,9 @@ +class AddDescriptionToTriggers < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :ci_triggers, :description, :string + end +end From 91ce04678ea78f2042026e8584e5d1069853dbc4 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 18:23:42 +0100 Subject: [PATCH 03/13] Make triggers to be user aware --- app/models/user.rb | 1 + .../ci/create_trigger_request_service.rb | 2 +- .../20170215164610_add_owner_id_to_triggers.rb | 2 +- spec/models/ci/trigger_spec.rb | 14 +++++++++++--- spec/models/user_spec.rb | 1 + .../ci/create_trigger_request_service_spec.rb | 18 ++++++++++++++++-- 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index d3bb04060bf..dfba51d3b00 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -95,6 +95,7 @@ class User < ActiveRecord::Base has_many :todos, dependent: :destroy has_many :notification_settings, dependent: :destroy has_many :award_emoji, dependent: :destroy + has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id has_many :assigned_issues, dependent: :nullify, foreign_key: :assignee_id, class_name: "Issue" has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" diff --git a/app/services/ci/create_trigger_request_service.rb b/app/services/ci/create_trigger_request_service.rb index 6af3c1ca5b1..dca5aa9f5d7 100644 --- a/app/services/ci/create_trigger_request_service.rb +++ b/app/services/ci/create_trigger_request_service.rb @@ -3,7 +3,7 @@ module Ci def execute(project, trigger, ref, variables = nil) trigger_request = trigger.trigger_requests.create(variables: variables) - pipeline = Ci::CreatePipelineService.new(project, nil, ref: ref). + pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: ref). execute(ignore_skip_ci: true, trigger_request: trigger_request) if pipeline.persisted? trigger_request diff --git a/db/migrate/20170215164610_add_owner_id_to_triggers.rb b/db/migrate/20170215164610_add_owner_id_to_triggers.rb index 02c77ee4b5e..845c89c703e 100644 --- a/db/migrate/20170215164610_add_owner_id_to_triggers.rb +++ b/db/migrate/20170215164610_add_owner_id_to_triggers.rb @@ -5,6 +5,6 @@ class AddOwnerIdToTriggers < ActiveRecord::Migration def change add_column :ci_triggers, :owner_id, :integer - add_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :nullify + add_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :cascade end end diff --git a/spec/models/ci/trigger_spec.rb b/spec/models/ci/trigger_spec.rb index 3ca9231f58e..074cf1a0bd7 100644 --- a/spec/models/ci/trigger_spec.rb +++ b/spec/models/ci/trigger_spec.rb @@ -1,16 +1,24 @@ require 'spec_helper' describe Ci::Trigger, models: true do - let(:project) { FactoryGirl.create :empty_project } + let(:project) { create :empty_project } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:owner) } + it { is_expected.to have_many(:trigger_requests) } + end describe 'before_validation' do it 'sets an random token if none provided' do - trigger = FactoryGirl.create :ci_trigger_without_token, project: project + trigger = create(:ci_trigger_without_token, project: project) + expect(trigger.token).not_to be_nil end it 'does not set an random token if one provided' do - trigger = FactoryGirl.create :ci_trigger, project: project + trigger = create(:ci_trigger, project: project) + expect(trigger.token).to eq('token') end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e86b4a761d9..b99cde64675 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -32,6 +32,7 @@ describe User, models: true do it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } + it { is_expected.to have_many(:triggers).dependent(:destroy) } it { is_expected.to have_many(:builds).dependent(:nullify) } it { is_expected.to have_many(:pipelines).dependent(:nullify) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } diff --git a/spec/services/ci/create_trigger_request_service_spec.rb b/spec/services/ci/create_trigger_request_service_spec.rb index d8c443d29d5..5e68343784d 100644 --- a/spec/services/ci/create_trigger_request_service_spec.rb +++ b/spec/services/ci/create_trigger_request_service_spec.rb @@ -13,8 +13,22 @@ describe Ci::CreateTriggerRequestService, services: true do context 'valid params' do subject { service.execute(project, trigger, 'master') } - it { expect(subject).to be_kind_of(Ci::TriggerRequest) } - it { expect(subject.builds.first).to be_kind_of(Ci::Build) } + context 'without owner' do + it { expect(subject).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.builds.first).to be_kind_of(Ci::Build) } + end + + context 'with owner' do + let(:owner) { create(:user) } + let(:trigger) { create(:ci_trigger, project: project, owner: owner) } + + it { expect(subject).to be_kind_of(Ci::TriggerRequest) } + it { expect(subject.pipeline).to be_kind_of(Ci::Pipeline) } + it { expect(subject.pipeline.user).to eq(owner) } + it { expect(subject.builds.first).to be_kind_of(Ci::Build) } + it { expect(subject.builds.first.user).to eq(owner) } + end end context 'no commit for ref' do From 4ac1467c5ce969e082d96e7b2a60ae8e1654c881 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 18:28:32 +0100 Subject: [PATCH 04/13] Update db/schema --- ...ers.rb => 20170217151948_add_owner_id_to_triggers.rb} | 0 ....rb => 20170217151949_add_description_to_triggers.rb} | 0 db/schema.rb | 9 ++++++--- 3 files changed, 6 insertions(+), 3 deletions(-) rename db/migrate/{20170215164610_add_owner_id_to_triggers.rb => 20170217151948_add_owner_id_to_triggers.rb} (100%) rename db/migrate/{20170215165036_add_description_to_triggers.rb => 20170217151949_add_description_to_triggers.rb} (100%) diff --git a/db/migrate/20170215164610_add_owner_id_to_triggers.rb b/db/migrate/20170217151948_add_owner_id_to_triggers.rb similarity index 100% rename from db/migrate/20170215164610_add_owner_id_to_triggers.rb rename to db/migrate/20170217151948_add_owner_id_to_triggers.rb diff --git a/db/migrate/20170215165036_add_description_to_triggers.rb b/db/migrate/20170217151949_add_description_to_triggers.rb similarity index 100% rename from db/migrate/20170215165036_add_description_to_triggers.rb rename to db/migrate/20170217151949_add_description_to_triggers.rb diff --git a/db/schema.rb b/db/schema.rb index cd5aa339269..75daae86a4a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170217151947) do +ActiveRecord::Schema.define(version: 20170217151949) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -111,7 +111,7 @@ ActiveRecord::Schema.define(version: 20170217151947) do t.boolean "plantuml_enabled" t.integer "max_pages_size", default: 100, null: false t.integer "terminal_max_session_time", default: 0, null: false - t.string "default_artifacts_expire_in", default: '0', null: false + t.string "default_artifacts_expire_in", default: "0", null: false end create_table "audit_events", force: :cascade do |t| @@ -377,6 +377,8 @@ ActiveRecord::Schema.define(version: 20170217151947) do t.datetime "created_at" t.datetime "updated_at" t.integer "gl_project_id" + t.integer "owner_id" + t.string "description" end add_index "ci_triggers", ["gl_project_id"], name: "index_ci_triggers_on_gl_project_id", using: :btree @@ -581,9 +583,9 @@ ActiveRecord::Schema.define(version: 20170217151947) do end add_index "labels", ["group_id", "project_id", "title"], name: "index_labels_on_group_id_and_project_id_and_title", unique: true, using: :btree - add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree add_index "labels", ["project_id"], name: "index_labels_on_project_id", using: :btree add_index "labels", ["title"], name: "index_labels_on_title", using: :btree + add_index "labels", ["type", "project_id"], name: "index_labels_on_type_and_project_id", using: :btree create_table "lfs_objects", force: :cascade do |t| t.string "oid", null: false @@ -1333,6 +1335,7 @@ ActiveRecord::Schema.define(version: 20170217151947) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "boards", "projects" + add_foreign_key "ci_triggers", "users", column: "owner_id", on_delete: :cascade add_foreign_key "issue_metrics", "issues", on_delete: :cascade add_foreign_key "label_priorities", "labels", on_delete: :cascade add_foreign_key "label_priorities", "projects", on_delete: :cascade From ab972295bc0ec6db90ae451904a867788d66f49d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 18:30:58 +0100 Subject: [PATCH 05/13] Fix trigger model --- app/models/ci/trigger.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index f76f06eb9c6..8aa45b2f02e 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -10,7 +10,6 @@ module Ci has_many :trigger_requests, dependent: :destroy validates :token, presence: true, uniqueness: true - validates :owner, presence: true before_validation :set_default_values From 140b51ce980bc519f3478bf4321dfd4a35d6bd3c Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 20:58:08 +0100 Subject: [PATCH 06/13] Introduce tests for pipeline triggers --- app/helpers/triggers_helper.rb | 4 +- .../introduce-pipeline-triggers.yml | 4 + doc/ci/triggers/README.md | 16 +- lib/api/entities.rb | 4 +- lib/api/triggers.rb | 22 +-- spec/requests/api/triggers_spec.rb | 149 ++++++++++++++---- 6 files changed, 146 insertions(+), 53 deletions(-) create mode 100644 changelogs/unreleased/introduce-pipeline-triggers.yml diff --git a/app/helpers/triggers_helper.rb b/app/helpers/triggers_helper.rb index b0135ea2e95..a48d4475e97 100644 --- a/app/helpers/triggers_helper.rb +++ b/app/helpers/triggers_helper.rb @@ -1,9 +1,9 @@ module TriggersHelper def builds_trigger_url(project_id, ref: nil) if ref.nil? - "#{Settings.gitlab.url}/api/v3/projects/#{project_id}/trigger/builds" + "#{Settings.gitlab.url}/api/v4/projects/#{project_id}/trigger/pipeline" else - "#{Settings.gitlab.url}/api/v3/projects/#{project_id}/ref/#{ref}/trigger/builds" + "#{Settings.gitlab.url}/api/v4/projects/#{project_id}/ref/#{ref}/trigger/pipeline" end end diff --git a/changelogs/unreleased/introduce-pipeline-triggers.yml b/changelogs/unreleased/introduce-pipeline-triggers.yml new file mode 100644 index 00000000000..ce5a230d48f --- /dev/null +++ b/changelogs/unreleased/introduce-pipeline-triggers.yml @@ -0,0 +1,4 @@ +--- +title: Introduce Pipeline Triggers that are user-aware +merge_request: +author: diff --git a/doc/ci/triggers/README.md b/doc/ci/triggers/README.md index 1ad9621c8a0..ccaee33dc92 100644 --- a/doc/ci/triggers/README.md +++ b/doc/ci/triggers/README.md @@ -36,7 +36,7 @@ it will not trigger a job. To trigger a job you need to send a `POST` request to GitLab's API endpoint: ``` -POST /projects/:id/trigger/builds +POST /projects/:id/trigger/pipeline ``` The required parameters are the trigger's `token` and the Git `ref` on which @@ -71,7 +71,7 @@ To trigger a job from webhook of another project you need to add the following webhook url for Push and Tag push events: ``` -https://gitlab.example.com/api/v4/projects/:id/ref/:ref/trigger/builds?token=TOKEN +https://gitlab.example.com/api/v4/projects/:id/ref/:ref/trigger/pipeline?token=TOKEN ``` > **Note**: @@ -105,7 +105,7 @@ Using cURL you can trigger a rebuild with minimal effort, for example: curl --request POST \ --form token=TOKEN \ --form ref=master \ - https://gitlab.example.com/api/v4/projects/9/trigger/builds + https://gitlab.example.com/api/v4/projects/9/trigger/pipeline ``` In this case, the project with ID `9` will get rebuilt on `master` branch. @@ -114,7 +114,7 @@ Alternatively, you can pass the `token` and `ref` arguments in the query string: ```bash curl --request POST \ - "https://gitlab.example.com/api/v4/projects/9/trigger/builds?token=TOKEN&ref=master" + "https://gitlab.example.com/api/v4/projects/9/trigger/pipeline?token=TOKEN&ref=master" ``` ### Triggering a job within `.gitlab-ci.yml` @@ -128,7 +128,7 @@ need to add in project's A `.gitlab-ci.yml`: build_docs: stage: deploy script: - - "curl --request POST --form token=TOKEN --form ref=master https://gitlab.example.com/api/v4/projects/9/trigger/builds" + - "curl --request POST --form token=TOKEN --form ref=master https://gitlab.example.com/api/v4/projects/9/trigger/pipeline" only: - tags ``` @@ -187,7 +187,7 @@ curl --request POST \ --form token=TOKEN \ --form ref=master \ --form "variables[UPLOAD_TO_S3]=true" \ - https://gitlab.example.com/api/v4/projects/9/trigger/builds + https://gitlab.example.com/api/v4/projects/9/trigger/pipeline ``` ### Using webhook to trigger job @@ -195,7 +195,7 @@ curl --request POST \ You can add the following webhook to another project in order to trigger a job: ``` -https://gitlab.example.com/api/v4/projects/9/ref/master/trigger/builds?token=TOKEN&variables[UPLOAD_TO_S3]=true +https://gitlab.example.com/api/v4/projects/9/ref/master/trigger/pipeline?token=TOKEN&variables[UPLOAD_TO_S3]=true ``` ### Using cron to trigger nightly jobs @@ -205,7 +205,7 @@ in conjunction with cron. The example below triggers a job on the `master` branch of project with ID `9` every night at `00:30`: ```bash -30 0 * * * curl --request POST --form token=TOKEN --form ref=master https://gitlab.example.com/api/v4/projects/9/trigger/builds +30 0 * * * curl --request POST --form token=TOKEN --form ref=master https://gitlab.example.com/api/v4/projects/9/trigger/pipeline ``` [ci-229]: https://gitlab.com/gitlab-org/gitlab-ci/merge_requests/229 diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 82d05d85ead..c0c94044ced 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -639,7 +639,9 @@ module API end class Trigger < Grape::Entity - expose :token, :created_at, :updated_at, :deleted_at, :last_used + expose :token, :description + expose :created_at, :updated_at, :deleted_at, :last_used + expose :owner, using: Entities::UserBasic end class Variable < Grape::Entity diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index c324708a16d..157f3cef1fd 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -7,7 +7,7 @@ module API end resource :projects do desc 'Trigger a GitLab project pipeline' do - success Entities::TriggerRequest + success Entities::Pipeline end params do requires :ref, type: String, desc: 'The commit sha or name of a branch or tag' @@ -31,7 +31,7 @@ module API if trigger_request present trigger_request.pipeline, with: Entities::Pipeline else - errors = 'No pipeline create' + errors = 'No pipeline created' render_api_error!(errors, 400) end end @@ -61,7 +61,7 @@ module API authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.find(params[:trigger_id]) + trigger = user_project.triggers.find(params.delete(:trigger_id)) return not_found!('Trigger') unless trigger present trigger, with: Entities::Trigger @@ -94,15 +94,18 @@ module API requires :trigger_id, type: Integer, desc: 'The trigger ID' optional :description, type: String, desc: 'The trigger description' end - delete ':id/triggers/:trigger_id' do + put ':id/triggers/:trigger_id' do authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.find(params[:trigger_id]) + trigger = user_project.triggers.find(params.delete(:trigger_id)) return not_found!('Trigger') unless trigger - trigger = trigger.update(declared_params(include_missing: false)) - present trigger, with: Entities::Trigger + if trigger.update(declared_params(include_missing: false)) + present trigger, with: Entities::Trigger + else + render_validation_error!(trigger) + end end desc 'Take ownership of trigger' do @@ -115,10 +118,11 @@ module API authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.find(params[:trigger_id]) + trigger = user_project.triggers.find(params.delete(:trigger_id)) return not_found!('Trigger') unless trigger if trigger.update(owner: current_user) + status :ok present trigger, with: Entities::Trigger else render_validation_error!(trigger) @@ -135,7 +139,7 @@ module API authenticate! authorize! :admin_build, user_project - trigger = user_project.triggers.find(params[:trigger_id]) + trigger = user_project.triggers.find(params.delete(:trigger_id)) return not_found!('Trigger') unless trigger trigger.destroy diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 153e2791cbe..f2effd71755 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -14,7 +14,7 @@ describe API::Triggers do let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2) } let!(:trigger_request) { create(:ci_trigger_request, trigger: trigger, created_at: '2015-01-01 12:13:14') } - describe 'POST /projects/:project_id/trigger' do + describe 'POST /projects/:project_id/trigger/pipeline' do let!(:project2) { create(:project) } let(:options) do { @@ -28,17 +28,20 @@ describe API::Triggers do context 'Handles errors' do it 'returns bad request if token is missing' do - post api("/projects/#{project.id}/trigger/builds"), ref: 'master' + post api("/projects/#{project.id}/trigger/pipeline"), ref: 'master' + expect(response).to have_http_status(400) end it 'returns not found if project is not found' do - post api('/projects/0/trigger/builds'), options.merge(ref: 'master') + post api('/projects/0/trigger/pipeline'), options.merge(ref: 'master') + expect(response).to have_http_status(404) end it 'returns unauthorized if token is for different project' do - post api("/projects/#{project2.id}/trigger/builds"), options.merge(ref: 'master') + post api("/projects/#{project2.id}/trigger/pipeline"), options.merge(ref: 'master') + expect(response).to have_http_status(401) end end @@ -46,9 +49,11 @@ describe API::Triggers do context 'Have a commit' do let(:pipeline) { project.pipelines.last } - it 'creates builds' do - post api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'master') + it 'creates pipeline' do + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'master') + expect(response).to have_http_status(201) + expect(json_response).to include('id' => pipeline.id) pipeline.builds.reload expect(pipeline.builds.pending.size).to eq(2) expect(pipeline.builds.size).to eq(5) @@ -56,15 +61,17 @@ describe API::Triggers do it 'creates builds on webhook from other gitlab repository and branch' do expect do - post api("/projects/#{project.id}/ref/master/trigger/builds?token=#{trigger_token}"), { ref: 'refs/heads/other-branch' } + post api("/projects/#{project.id}/ref/master/trigger/pipeline?token=#{trigger_token}"), { ref: 'refs/heads/other-branch' } end.to change(project.builds, :count).by(5) + expect(response).to have_http_status(201) end - it 'returns bad request with no builds created if there\'s no commit for that ref' do - post api("/projects/#{project.id}/trigger/builds"), options.merge(ref: 'other-branch') + it 'returns bad request with no pipeline created if there\'s no commit for that ref' do + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'other-branch') + expect(response).to have_http_status(400) - expect(json_response['message']).to eq('No builds created') + expect(json_response['message']).to eq('No pipeline created') end context 'Validates variables' do @@ -73,22 +80,24 @@ describe API::Triggers do end it 'validates variables to be a hash' do - post api("/projects/#{project.id}/trigger/builds"), options.merge(variables: 'value', ref: 'master') + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(variables: 'value', ref: 'master') + expect(response).to have_http_status(400) expect(json_response['error']).to eq('variables is invalid') end it 'validates variables needs to be a map of key-valued strings' do - post api("/projects/#{project.id}/trigger/builds"), options.merge(variables: { key: %w(1 2) }, ref: 'master') + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(variables: { key: %w(1 2) }, ref: 'master') + expect(response).to have_http_status(400) expect(json_response['message']).to eq('variables needs to be a map of key-valued strings') end it 'creates trigger request with variables' do - post api("/projects/#{project.id}/trigger/builds"), options.merge(variables: variables, ref: 'master') + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(variables: variables, ref: 'master') + expect(response).to have_http_status(201) - pipeline.builds.reload - expect(pipeline.builds.first.trigger_request.variables).to eq(variables) + expect(pipeline.builds.reload.first.trigger_request.variables).to eq(variables) end end end @@ -123,17 +132,17 @@ describe API::Triggers do end end - describe 'GET /projects/:id/triggers/:token' do + describe 'GET /projects/:id/triggers/:trigger_id' do context 'authenticated user with valid permissions' do it 'returns trigger details' do - get api("/projects/#{project.id}/triggers/#{trigger.token}", user) + get api("/projects/#{project.id}/triggers/#{trigger.id}", user) expect(response).to have_http_status(200) expect(json_response).to be_a(Hash) end it 'responds with 404 Not Found if requesting non-existing trigger' do - get api("/projects/#{project.id}/triggers/abcdef012345", user) + get api("/projects/#{project.id}/triggers/-5", user) expect(response).to have_http_status(404) end @@ -141,7 +150,7 @@ describe API::Triggers do context 'authenticated user with invalid permissions' do it 'does not return triggers list' do - get api("/projects/#{project.id}/triggers/#{trigger.token}", user2) + get api("/projects/#{project.id}/triggers/#{trigger.id}", user2) expect(response).to have_http_status(403) end @@ -149,7 +158,7 @@ describe API::Triggers do context 'unauthenticated user' do it 'does not return triggers list' do - get api("/projects/#{project.id}/triggers/#{trigger.token}") + get api("/projects/#{project.id}/triggers/#{trigger.id}") expect(response).to have_http_status(401) end @@ -158,19 +167,31 @@ describe API::Triggers do describe 'POST /projects/:id/triggers' do context 'authenticated user with valid permissions' do - it 'creates trigger' do - expect do - post api("/projects/#{project.id}/triggers", user) - end.to change{project.triggers.count}.by(1) + context 'with required parameters' do + it 'creates trigger' do + expect do + post api("/projects/#{project.id}/triggers", user), + description: 'trigger' + end.to change{project.triggers.count}.by(1) - expect(response).to have_http_status(201) - expect(json_response).to be_a(Hash) + expect(response).to have_http_status(201) + expect(json_response).to include('description' => 'trigger') + end + end + + context 'without required parameters' do + it 'creates trigger' do + post api("/projects/#{project.id}/triggers", user) + + expect(response).to have_http_status(:bad_request) + end end end context 'authenticated user with invalid permissions' do it 'does not create trigger' do - post api("/projects/#{project.id}/triggers", user2) + post api("/projects/#{project.id}/triggers", user2), + description: 'trigger' expect(response).to have_http_status(403) end @@ -178,25 +199,87 @@ describe API::Triggers do context 'unauthenticated user' do it 'does not create trigger' do - post api("/projects/#{project.id}/triggers") + post api("/projects/#{project.id}/triggers"), + description: 'trigger' expect(response).to have_http_status(401) end end end - describe 'DELETE /projects/:id/triggers/:token' do + describe 'PUT /projects/:id/triggers/:trigger_id' do + context 'authenticated user with valid permissions' do + let(:new_description) { 'new description' } + + it 'updates description' do + put api("/projects/#{project.id}/triggers/#{trigger.id}", user), + description: new_description + + expect(response).to have_http_status(200) + expect(json_response).to include('description' => new_description) + expect(trigger.reload.description).to eq(new_description) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger.id}", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not update trigger' do + put api("/projects/#{project.id}/triggers/#{trigger.id}") + + expect(response).to have_http_status(401) + end + end + end + + describe 'POST /projects/:id/triggers/:trigger_id/take' do + context 'authenticated user with valid permissions' do + it 'updates owner' do + expect(trigger.owner).to be_nil + + post api("/projects/#{project.id}/triggers/#{trigger.id}/take", user) + + expect(response).to have_http_status(200) + expect(json_response).to include('owner') + expect(trigger.reload.owner).to eq(user) + end + end + + context 'authenticated user with invalid permissions' do + it 'does not update owner' do + post api("/projects/#{project.id}/triggers/#{trigger.id}/take", user2) + + expect(response).to have_http_status(403) + end + end + + context 'unauthenticated user' do + it 'does not update owner' do + post api("/projects/#{project.id}/triggers/#{trigger.id}/take") + + expect(response).to have_http_status(401) + end + end + end + + describe 'DELETE /projects/:id/triggers/:trigger_id' do context 'authenticated user with valid permissions' do it 'deletes trigger' do expect do - delete api("/projects/#{project.id}/triggers/#{trigger.token}", user) + delete api("/projects/#{project.id}/triggers/#{trigger.id}", user) expect(response).to have_http_status(204) end.to change{project.triggers.count}.by(-1) end it 'responds with 404 Not Found if requesting non-existing trigger' do - delete api("/projects/#{project.id}/triggers/abcdef012345", user) + delete api("/projects/#{project.id}/triggers/-5", user) expect(response).to have_http_status(404) end @@ -204,7 +287,7 @@ describe API::Triggers do context 'authenticated user with invalid permissions' do it 'does not delete trigger' do - delete api("/projects/#{project.id}/triggers/#{trigger.token}", user2) + delete api("/projects/#{project.id}/triggers/#{trigger.id}", user2) expect(response).to have_http_status(403) end @@ -212,7 +295,7 @@ describe API::Triggers do context 'unauthenticated user' do it 'does not delete trigger' do - delete api("/projects/#{project.id}/triggers/#{trigger.token}") + delete api("/projects/#{project.id}/triggers/#{trigger.id}") expect(response).to have_http_status(401) end From b565ee4912d742ef01d10e9a6fae64fe79d6b7bf Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 21:18:00 +0100 Subject: [PATCH 07/13] Update documentation and expose ID --- doc/api/README.md | 2 +- doc/api/build_triggers.md | 108 ---------------------- doc/api/pipeline_triggers.md | 170 +++++++++++++++++++++++++++++++++++ lib/api/entities.rb | 1 + 4 files changed, 172 insertions(+), 109 deletions(-) delete mode 100644 doc/api/build_triggers.md create mode 100644 doc/api/pipeline_triggers.md diff --git a/doc/api/README.md b/doc/api/README.md index 3399e2bb5f6..285cd2435ac 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -12,7 +12,6 @@ following locations: - [Branches](branches.md) - [Broadcast Messages](broadcast_messages.md) - [Builds](builds.md) -- [Build Triggers](build_triggers.md) - [Build Variables](build_variables.md) - [Commits](commits.md) - [Deployments](deployments.md) @@ -33,6 +32,7 @@ following locations: - [Notes](notes.md) (comments) - [Notification settings](notification_settings.md) - [Pipelines](pipelines.md) +- [Pipeline Triggers](pipeline_triggers.md) - [Projects](projects.md) including setting Webhooks - [Project Access Requests](access_requests.md) - [Project Members](members.md) diff --git a/doc/api/build_triggers.md b/doc/api/build_triggers.md deleted file mode 100644 index 28befba69d6..00000000000 --- a/doc/api/build_triggers.md +++ /dev/null @@ -1,108 +0,0 @@ -# Build triggers - -You can read more about [triggering builds through the API](../ci/triggers/README.md). - -## List project triggers - -Get a list of project's build triggers. - -``` -GET /projects/:id/triggers -``` - -| Attribute | Type | required | Description | -|-----------|---------|----------|---------------------| -| `id` | integer | yes | The ID of a project | - -``` -curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers" -``` - -```json -[ - { - "created_at": "2015-12-23T16:24:34.716Z", - "deleted_at": null, - "last_used": "2016-01-04T15:41:21.986Z", - "token": "fbdb730c2fbdb095a0862dbd8ab88b", - "updated_at": "2015-12-23T16:24:34.716Z" - }, - { - "created_at": "2015-12-23T16:25:56.760Z", - "deleted_at": null, - "last_used": null, - "token": "7b9148c158980bbd9bcea92c17522d", - "updated_at": "2015-12-23T16:25:56.760Z" - } -] -``` - -## Get trigger details - -Get details of project's build trigger. - -``` -GET /projects/:id/triggers/:token -``` - -| Attribute | Type | required | Description | -|-----------|---------|----------|--------------------------| -| `id` | integer | yes | The ID of a project | -| `token` | string | yes | The `token` of a trigger | - -``` -curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/7b9148c158980bbd9bcea92c17522d" -``` - -```json -{ - "created_at": "2015-12-23T16:25:56.760Z", - "deleted_at": null, - "last_used": null, - "token": "7b9148c158980bbd9bcea92c17522d", - "updated_at": "2015-12-23T16:25:56.760Z" -} -``` - -## Create a project trigger - -Create a build trigger for a project. - -``` -POST /projects/:id/triggers -``` - -| Attribute | Type | required | Description | -|-----------|---------|----------|--------------------------| -| `id` | integer | yes | The ID of a project | - -``` -curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers" -``` - -```json -{ - "created_at": "2016-01-07T09:53:58.235Z", - "deleted_at": null, - "last_used": null, - "token": "6d056f63e50fe6f8c5f8f4aa10edb7", - "updated_at": "2016-01-07T09:53:58.235Z" -} -``` - -## Remove a project trigger - -Remove a project's build trigger. - -``` -DELETE /projects/:id/triggers/:token -``` - -| Attribute | Type | required | Description | -|-----------|---------|----------|--------------------------| -| `id` | integer | yes | The ID of a project | -| `token` | string | yes | The `token` of a trigger | - -``` -curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/7b9148c158980bbd9bcea92c17522d" -``` diff --git a/doc/api/pipeline_triggers.md b/doc/api/pipeline_triggers.md new file mode 100644 index 00000000000..aebc84b9a66 --- /dev/null +++ b/doc/api/pipeline_triggers.md @@ -0,0 +1,170 @@ +# Pipeline triggers + +You can read more about [triggering pipelines through the API](../ci/triggers/README.md). + +## List project triggers + +Get a list of project's build triggers. + +``` +GET /projects/:id/triggers +``` + +| Attribute | Type | required | Description | +|-----------|---------|----------|---------------------| +| `id` | integer | yes | The ID of a project | + +``` +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers" +``` + +```json +[ + { + "id": 10, + "description": "my trigger", + "created_at": "2016-01-07T09:53:58.235Z", + "deleted_at": null, + "last_used": null, + "token": "6d056f63e50fe6f8c5f8f4aa10edb7", + "updated_at": "2016-01-07T09:53:58.235Z", + "owner": null + } +] +``` + +## Get trigger details + +Get details of project's build trigger. + +``` +GET /projects/:id/triggers/:trigger_id +``` + +| Attribute | Type | required | Description | +|-----------|---------|----------|--------------------------| +| `id` | integer | yes | The ID of a project | +| `token` | string | yes | The `token` of a trigger | + +``` +curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/5" +``` + +```json +{ + "id": 10, + "description": "my trigger", + "created_at": "2016-01-07T09:53:58.235Z", + "deleted_at": null, + "last_used": null, + "token": "6d056f63e50fe6f8c5f8f4aa10edb7", + "updated_at": "2016-01-07T09:53:58.235Z", + "owner": null +} +``` + +## Create a project trigger + +Create a trigger for a project. + +``` +POST /projects/:id/triggers +``` + +| Attribute | Type | required | Description | +|---------------|---------|----------|--------------------------| +| `id` | integer | yes | The ID of a project | +| `description` | string | yes | The trigger name | + +``` +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -F description="my description" "https://gitlab.example.com/api/v4/projects/1/triggers" +``` + +```json +{ + "id": 10, + "description": "my trigger", + "created_at": "2016-01-07T09:53:58.235Z", + "deleted_at": null, + "last_used": null, + "token": "6d056f63e50fe6f8c5f8f4aa10edb7", + "updated_at": "2016-01-07T09:53:58.235Z", + "owner": null +} +``` + +## Update a project trigger + +Update a trigger for a project. + +``` +PUT /projects/:id/triggers/:trigger_id +``` + +| Attribute | Type | required | Description | +|---------------|---------|----------|--------------------------| +| `trigger_id` | integer | yes | The trigger id | +| `description` | string | no | The trigger name | + +``` +curl --request PUT -F description="my description" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/10" +``` + +```json +{ + "id": 10, + "description": "my trigger", + "created_at": "2016-01-07T09:53:58.235Z", + "deleted_at": null, + "last_used": null, + "token": "6d056f63e50fe6f8c5f8f4aa10edb7", + "updated_at": "2016-01-07T09:53:58.235Z", + "owner": null +} +``` + +## Take ownership of a project trigger + +Update an owner of a project trigger. + +``` +POST /projects/:id/triggers/:trigger_id/take +``` + +| Attribute | Type | required | Description | +|---------------|---------|----------|--------------------------| +| `trigger_id` | integer | yes | The trigger id | + +``` +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/10/take" +``` + +```json +{ + "id": 10, + "description": "my trigger", + "created_at": "2016-01-07T09:53:58.235Z", + "deleted_at": null, + "last_used": null, + "token": "6d056f63e50fe6f8c5f8f4aa10edb7", + "updated_at": "2016-01-07T09:53:58.235Z", + "owner": null +} +``` + +## Remove a project trigger + +Remove a project's build trigger. + +``` +DELETE /projects/:id/triggers/:trigger_id +``` + +| Attribute | Type | required | Description | +|----------------|---------|----------|--------------------------| +| `id` | integer | yes | The ID of a project | +| `trigger_id` | integer | yes | The trigger id | + +``` +curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/5" +``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c0c94044ced..98ef9d4118e 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -639,6 +639,7 @@ module API end class Trigger < Grape::Entity + expose :id expose :token, :description expose :created_at, :updated_at, :deleted_at, :last_used expose :owner, using: Entities::UserBasic From b626869f8fccfef65a652b0acca7c52a3647aeb3 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 21:23:57 +0100 Subject: [PATCH 08/13] Fix import model attributes --- spec/lib/gitlab/import_export/all_models.yml | 1 + spec/lib/gitlab/import_export/safe_model_attributes.yml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index eef283c2460..f20b6be51e1 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -97,6 +97,7 @@ variables: triggers: - project - trigger_requests +- owner deploy_keys: - user - deploy_keys_projects diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 6534902b52d..3bd1f335a89 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -240,6 +240,8 @@ Ci::Trigger: - created_at - updated_at - gl_project_id +- owner_id +- description DeployKey: - id - user_id From e4a9b8a9e1f4052fde9ecacad73e2f96fdd8d21d Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 21:40:59 +0100 Subject: [PATCH 09/13] Move foreign key to separate migration --- db/migrate/20170217151948_add_owner_id_to_triggers.rb | 1 - db/migrate/20170305203726_add_owner_id_foreign_key.rb | 11 +++++++++++ db/schema.rb | 4 ++-- 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20170305203726_add_owner_id_foreign_key.rb diff --git a/db/migrate/20170217151948_add_owner_id_to_triggers.rb b/db/migrate/20170217151948_add_owner_id_to_triggers.rb index 845c89c703e..16d7cc5bed6 100644 --- a/db/migrate/20170217151948_add_owner_id_to_triggers.rb +++ b/db/migrate/20170217151948_add_owner_id_to_triggers.rb @@ -5,6 +5,5 @@ class AddOwnerIdToTriggers < ActiveRecord::Migration def change add_column :ci_triggers, :owner_id, :integer - add_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :cascade end end diff --git a/db/migrate/20170305203726_add_owner_id_foreign_key.rb b/db/migrate/20170305203726_add_owner_id_foreign_key.rb new file mode 100644 index 00000000000..3eece0e2eb5 --- /dev/null +++ b/db/migrate/20170305203726_add_owner_id_foreign_key.rb @@ -0,0 +1,11 @@ +class AddOwnerIdForeignKey < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def change + add_concurrent_foreign_key :ci_triggers, :users, column: :owner_id, on_delete: :cascade + end +end diff --git a/db/schema.rb b/db/schema.rb index 75daae86a4a..9deed46530e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170217151949) do +ActiveRecord::Schema.define(version: 20170305203726) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1335,7 +1335,7 @@ ActiveRecord::Schema.define(version: 20170217151949) do add_index "web_hooks", ["project_id"], name: "index_web_hooks_on_project_id", using: :btree add_foreign_key "boards", "projects" - add_foreign_key "ci_triggers", "users", column: "owner_id", on_delete: :cascade + add_foreign_key "ci_triggers", "users", column: "owner_id", name: "fk_e8e10d1964", on_delete: :cascade add_foreign_key "issue_metrics", "issues", on_delete: :cascade add_foreign_key "label_priorities", "labels", on_delete: :cascade add_foreign_key "label_priorities", "projects", on_delete: :cascade From 16897d32d2d8fe848c7bbd7ba2acb8ff64bbd0bf Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 5 Mar 2017 23:03:54 +0100 Subject: [PATCH 10/13] Lint doc --- doc/api/pipeline_triggers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/pipeline_triggers.md b/doc/api/pipeline_triggers.md index aebc84b9a66..a26bdfdbfca 100644 --- a/doc/api/pipeline_triggers.md +++ b/doc/api/pipeline_triggers.md @@ -77,7 +77,7 @@ POST /projects/:id/triggers | `description` | string | yes | The trigger name | ``` -curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" -F description="my description" "https://gitlab.example.com/api/v4/projects/1/triggers" +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form description="my description" "https://gitlab.example.com/api/v4/projects/1/triggers" ``` ```json @@ -107,7 +107,7 @@ PUT /projects/:id/triggers/:trigger_id | `description` | string | no | The trigger name | ``` -curl --request PUT -F description="my description" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/10" +curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form description="my description" "https://gitlab.example.com/api/v4/projects/1/triggers/10" ``` ```json From 01f99bd26910bb2b0480c44e752bdc7de107cfb1 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Mar 2017 11:48:33 +0100 Subject: [PATCH 11/13] Update after review --- doc/api/build_triggers.md | 1 + doc/api/v3_to_v4.md | 3 +++ spec/requests/api/triggers_spec.rb | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 doc/api/build_triggers.md diff --git a/doc/api/build_triggers.md b/doc/api/build_triggers.md new file mode 100644 index 00000000000..20d924ab35e --- /dev/null +++ b/doc/api/build_triggers.md @@ -0,0 +1 @@ +This document was moved to [Pipeline Triggers](pipeline_triggers.md). diff --git a/doc/api/v3_to_v4.md b/doc/api/v3_to_v4.md index 39dc6d98e7b..67ee2b69c3f 100644 --- a/doc/api/v3_to_v4.md +++ b/doc/api/v3_to_v4.md @@ -59,3 +59,6 @@ changes are in V4: - Return 202 with JSON body on async removals on V4 API (DELETE `/projects/:id/repository/merged_branches` and DELETE `/projects/:id`) [!9449](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9449) - `projects/:id/milestones?iid[]=x&iid[]=y` array filter has been renamed to `iids` [!9096](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9096) - Return basic info about pipeline in `GET /projects/:id/pipelines` [!8875](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8875) +- Rename Build Triggers to be Pipeline Triggers API [!9713](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/9713) + - `POST /projects/:id/trigger/builds` to `POST /projects/:id/trigger/pipeline` + - Require description when creating a new trigger `POST /projects/:id/triggers` diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index f2effd71755..c4e8c9b09d7 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -180,7 +180,7 @@ describe API::Triggers do end context 'without required parameters' do - it 'creates trigger' do + it 'does not create trigger' do post api("/projects/#{project.id}/triggers", user) expect(response).to have_http_status(:bad_request) From d5f7060400a06d98f9e7107949aca8d89eaba7a8 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Mar 2017 12:00:15 +0100 Subject: [PATCH 12/13] Rename `/take` to `/take_ownership`, expose `owner` in `v3`. --- doc/api/pipeline_triggers.md | 4 ++-- lib/api/triggers.rb | 2 +- lib/api/v3/entities.rb | 1 + spec/requests/api/triggers_spec.rb | 8 ++++---- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/api/pipeline_triggers.md b/doc/api/pipeline_triggers.md index a26bdfdbfca..fdb41a1d615 100644 --- a/doc/api/pipeline_triggers.md +++ b/doc/api/pipeline_triggers.md @@ -128,7 +128,7 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" --form descrip Update an owner of a project trigger. ``` -POST /projects/:id/triggers/:trigger_id/take +POST /projects/:id/triggers/:trigger_id/take_ownership ``` | Attribute | Type | required | Description | @@ -136,7 +136,7 @@ POST /projects/:id/triggers/:trigger_id/take | `trigger_id` | integer | yes | The trigger id | ``` -curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/10/take" +curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/triggers/10/take_ownership" ``` ```json diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb index 157f3cef1fd..119e9024712 100644 --- a/lib/api/triggers.rb +++ b/lib/api/triggers.rb @@ -114,7 +114,7 @@ module API params do requires :trigger_id, type: Integer, desc: 'The trigger ID' end - post ':id/triggers/:trigger_id/take' do + post ':id/triggers/:trigger_id/take_ownership' do authenticate! authorize! :admin_build, user_project diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 29a44d4c7e5..2492481e4f0 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -189,6 +189,7 @@ module API class Trigger < Grape::Entity expose :token, :created_at, :updated_at, :deleted_at, :last_used + expose :owner, using: Entities::UserBasic end class TriggerRequest < Grape::Entity diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index c4e8c9b09d7..424c02932ab 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -238,12 +238,12 @@ describe API::Triggers do end end - describe 'POST /projects/:id/triggers/:trigger_id/take' do + describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do context 'authenticated user with valid permissions' do it 'updates owner' do expect(trigger.owner).to be_nil - post api("/projects/#{project.id}/triggers/#{trigger.id}/take", user) + post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user) expect(response).to have_http_status(200) expect(json_response).to include('owner') @@ -253,7 +253,7 @@ describe API::Triggers do context 'authenticated user with invalid permissions' do it 'does not update owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take", user2) + post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user2) expect(response).to have_http_status(403) end @@ -261,7 +261,7 @@ describe API::Triggers do context 'unauthenticated user' do it 'does not update owner' do - post api("/projects/#{project.id}/triggers/#{trigger.id}/take") + post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership") expect(response).to have_http_status(401) end From 20feaa9eff410cba133e5598f643883e603ba5da Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 6 Mar 2017 12:27:55 +0100 Subject: [PATCH 13/13] Fix UserBasic --- lib/api/v3/entities.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 2492481e4f0..69853d33bec 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -189,7 +189,7 @@ module API class Trigger < Grape::Entity expose :token, :created_at, :updated_at, :deleted_at, :last_used - expose :owner, using: Entities::UserBasic + expose :owner, using: ::API::Entities::UserBasic end class TriggerRequest < Grape::Entity