Commit d454e100 authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'fix/regressions-in-ci-v4-api' into 'master'

Fix regressions introduced while adding CI endpoints in API v4

See merge request !9952
parents a040e3ba 37b36b46
...@@ -539,6 +539,16 @@ module Ci ...@@ -539,6 +539,16 @@ module Ci
Gitlab::Ci::Build::Credentials::Factory.new(self).create! Gitlab::Ci::Build::Credentials::Factory.new(self).create!
end end
def dependencies
depended_jobs = depends_on_builds
return depended_jobs unless options[:dependencies].present?
depended_jobs.select do |job|
options[:dependencies].include?(job.name)
end
end
private private
def update_artifacts_size def update_artifacts_size
......
...@@ -768,7 +768,7 @@ module API ...@@ -768,7 +768,7 @@ module API
end end
class Dependency < Grape::Entity class Dependency < Grape::Entity
expose :id, :name expose :id, :name, :token
expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? } expose :artifacts_file, using: ArtifactFile, if: ->(job, _) { job.artifacts? }
end end
...@@ -796,7 +796,7 @@ module API ...@@ -796,7 +796,7 @@ module API
expose :artifacts, using: Artifacts expose :artifacts, using: Artifacts
expose :cache, using: Cache expose :cache, using: Cache
expose :credentials, using: Credentials expose :credentials, using: Credentials
expose :depends_on_builds, as: :dependencies, using: Dependency expose :dependencies, using: Dependency
end end
end end
end end
......
...@@ -41,14 +41,6 @@ module API ...@@ -41,14 +41,6 @@ module API
(Time.now - current_runner.contacted_at) >= contacted_at_max_age (Time.now - current_runner.contacted_at) >= contacted_at_max_age
end end
def job_not_found!
if headers['User-Agent'].to_s =~ /gitlab(-ci-multi)?-runner \d+\.\d+\.\d+(~beta\.\d+\.g[0-9a-f]+)? /
no_content!
else
not_found!
end
end
def validate_job!(job) def validate_job!(job)
not_found! unless job not_found! unless job
......
...@@ -47,11 +47,25 @@ module API ...@@ -47,11 +47,25 @@ module API
authenticate_runner! authenticate_runner!
Ci::Runner.find_by_token(params[:token]).destroy Ci::Runner.find_by_token(params[:token]).destroy
end end
desc 'Validates authentication credentials' do
http_codes [[200, 'Credentials are valid'], [403, 'Forbidden']]
end
params do
requires :token, type: String, desc: %q(Runner's authentication token)
end
post '/verify' do
authenticate_runner!
status 200
end
end end
resource :jobs do resource :jobs do
desc 'Request a job' do desc 'Request a job' do
success Entities::JobRequest::Response success Entities::JobRequest::Response
http_codes [[201, 'Job was scheduled'],
[204, 'No job for Runner'],
[403, 'Forbidden']]
end end
params do params do
requires :token, type: String, desc: %q(Runner's authentication token) requires :token, type: String, desc: %q(Runner's authentication token)
...@@ -60,13 +74,13 @@ module API ...@@ -60,13 +74,13 @@ module API
end end
post '/request' do post '/request' do
authenticate_runner! authenticate_runner!
not_found! unless current_runner.active? no_content! unless current_runner.active?
update_runner_info update_runner_info
if current_runner.is_runner_queue_value_latest?(params[:last_update]) if current_runner.is_runner_queue_value_latest?(params[:last_update])
header 'X-GitLab-Last-Update', params[:last_update] header 'X-GitLab-Last-Update', params[:last_update]
Gitlab::Metrics.add_event(:build_not_found_cached) Gitlab::Metrics.add_event(:build_not_found_cached)
return job_not_found! return no_content!
end end
new_update = current_runner.ensure_runner_queue_value new_update = current_runner.ensure_runner_queue_value
...@@ -80,7 +94,7 @@ module API ...@@ -80,7 +94,7 @@ module API
else else
Gitlab::Metrics.add_event(:build_not_found) Gitlab::Metrics.add_event(:build_not_found)
header 'X-GitLab-Last-Update', new_update header 'X-GitLab-Last-Update', new_update
job_not_found! no_content!
end end
else else
# We received build that is invalid due to concurrency conflict # We received build that is invalid due to concurrency conflict
......
...@@ -152,6 +152,34 @@ describe API::Runner do ...@@ -152,6 +152,34 @@ describe API::Runner do
end end
end end
end end
describe 'POST /api/v4/runners/verify' do
let(:runner) { create(:ci_runner) }
context 'when no token is provided' do
it 'returns 400 error' do
post api('/runners/verify')
expect(response).to have_http_status :bad_request
end
end
context 'when invalid token is provided' do
it 'returns 403 error' do
post api('/runners/verify'), token: 'invalid-token'
expect(response).to have_http_status 403
end
end
context 'when valid token is provided' do
it 'verifies Runner credentials' do
post api('/runners/verify'), token: runner.token
expect(response).to have_http_status 200
end
end
end
end end
describe '/api/v4/jobs' do describe '/api/v4/jobs' do
...@@ -220,18 +248,6 @@ describe API::Runner do ...@@ -220,18 +248,6 @@ describe API::Runner do
it { expect(response).to have_http_status(204) } it { expect(response).to have_http_status(204) }
end end
end end
context "when runner doesn't send version in User-Agent" do
let(:user_agent) { 'Go-http-client/1.1' }
it { expect(response).to have_http_status(404) }
end
context "when runner doesn't have a User-Agent" do
let(:user_agent) { nil }
it { expect(response).to have_http_status(404) }
end
end end
context 'when no token is provided' do context 'when no token is provided' do
...@@ -254,10 +270,10 @@ describe API::Runner do ...@@ -254,10 +270,10 @@ describe API::Runner do
context 'when Runner is not active' do context 'when Runner is not active' do
let(:runner) { create(:ci_runner, :inactive) } let(:runner) { create(:ci_runner, :inactive) }
it 'returns 404 error' do it 'returns 204 error' do
request_job request_job
expect(response).to have_http_status 404 expect(response).to have_http_status 204
end end
end end
...@@ -401,9 +417,39 @@ describe API::Runner do ...@@ -401,9 +417,39 @@ describe API::Runner do
end end
context 'when project and pipeline have multiple jobs' do context 'when project and pipeline have multiple jobs' do
let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) }
let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) }
let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) }
before { job.success } before do
job.success
job2.success
end
it 'returns dependent jobs' do
request_job
expect(response).to have_http_status(201)
expect(json_response['id']).to eq(test_job.id)
expect(json_response['dependencies'].count).to eq(2)
expect(json_response['dependencies']).to include({ 'id' => job.id, 'name' => job.name, 'token' => job.token },
{ 'id' => job2.id, 'name' => job2.name, 'token' => job2.token })
end
end
context 'when explicit dependencies are defined' do
let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) }
let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) }
let!(:test_job) do
create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy',
stage: 'deploy', stage_idx: 1,
options: { dependencies: [job2.name] })
end
before do
job.success
job2.success
end
it 'returns dependent jobs' do it 'returns dependent jobs' do
request_job request_job
...@@ -411,7 +457,7 @@ describe API::Runner do ...@@ -411,7 +457,7 @@ describe API::Runner do
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(json_response['id']).to eq(test_job.id) expect(json_response['id']).to eq(test_job.id)
expect(json_response['dependencies'].count).to eq(1) expect(json_response['dependencies'].count).to eq(1)
expect(json_response['dependencies'][0]).to include('id' => job.id, 'name' => 'spinach') expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => job2.token)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment