diff options
author | Kamil Trzcinski <ayufan@ayufan.eu> | 2017-03-05 20:58:08 +0100 |
---|---|---|
committer | Kamil Trzcinski <ayufan@ayufan.eu> | 2017-03-05 20:58:08 +0100 |
commit | 140b51ce980bc519f3478bf4321dfd4a35d6bd3c (patch) | |
tree | 79b212a3e5e05b9e87acdd2ef04c3beaf5236d78 | |
parent | ab972295bc0ec6db90ae451904a867788d66f49d (diff) | |
download | gitlab-ce-140b51ce980bc519f3478bf4321dfd4a35d6bd3c.tar.gz |
Introduce tests for pipeline triggers
-rw-r--r-- | app/helpers/triggers_helper.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/introduce-pipeline-triggers.yml | 4 | ||||
-rw-r--r-- | doc/ci/triggers/README.md | 16 | ||||
-rw-r--r-- | lib/api/entities.rb | 4 | ||||
-rw-r--r-- | lib/api/triggers.rb | 22 | ||||
-rw-r--r-- | spec/requests/api/triggers_spec.rb | 147 |
6 files changed, 145 insertions, 52 deletions
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 + 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 include('description' => 'trigger') + end + end + + context 'without required parameters' do + it 'creates trigger' do post 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) + 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 '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/:token' do + 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 |