diff options
Diffstat (limited to 'spec/requests/api/ci')
| -rw-r--r-- | spec/requests/api/ci/job_artifacts_spec.rb | 6 | ||||
| -rw-r--r-- | spec/requests/api/ci/jobs_spec.rb | 52 | ||||
| -rw-r--r-- | spec/requests/api/ci/pipeline_schedules_spec.rb | 4 | ||||
| -rw-r--r-- | spec/requests/api/ci/pipelines_spec.rb | 197 | ||||
| -rw-r--r-- | spec/requests/api/ci/runner/jobs_artifacts_spec.rb | 63 | ||||
| -rw-r--r-- | spec/requests/api/ci/runner/jobs_put_spec.rb | 6 | ||||
| -rw-r--r-- | spec/requests/api/ci/runner/jobs_request_post_spec.rb | 115 | ||||
| -rw-r--r-- | spec/requests/api/ci/runner/runners_delete_spec.rb | 92 | ||||
| -rw-r--r-- | spec/requests/api/ci/runner/runners_post_spec.rb | 81 | ||||
| -rw-r--r-- | spec/requests/api/ci/runner/runners_verify_post_spec.rb | 69 | ||||
| -rw-r--r-- | spec/requests/api/ci/runners_reset_registration_token_spec.rb | 15 | ||||
| -rw-r--r-- | spec/requests/api/ci/runners_spec.rb | 166 | ||||
| -rw-r--r-- | spec/requests/api/ci/secure_files_spec.rb | 2 | ||||
| -rw-r--r-- | spec/requests/api/ci/variables_spec.rb | 2 |
14 files changed, 517 insertions, 353 deletions
diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index ee390773f29..7cea744cdb9 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -190,7 +190,7 @@ RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do end context 'when project is public with artifacts that are non public' do - let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } + let(:job) { create(:ci_build, :artifacts, :with_private_artifacts_config, pipeline: pipeline) } it 'rejects access to artifacts' do project.update_column(:visibility_level, @@ -439,7 +439,7 @@ RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do context 'when public project guest and artifacts are non public' do let(:api_user) { guest } - let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline) } + let(:job) { create(:ci_build, :artifacts, :with_private_artifacts_config, pipeline: pipeline) } before do project.update_column(:visibility_level, @@ -644,7 +644,7 @@ RSpec.describe API::Ci::JobArtifacts, feature_category: :build_artifacts do end context 'when project is public with non public artifacts' do - let(:job) { create(:ci_build, :artifacts, :non_public_artifacts, pipeline: pipeline, user: api_user) } + let(:job) { create(:ci_build, :artifacts, :with_private_artifacts_config, pipeline: pipeline, user: api_user) } let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } let(:public_builds) { true } diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 8b3ec59b785..ed0cec46a42 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -198,22 +198,22 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do let_it_be(:agent_authorizations_without_env) do [ - create(:agent_group_authorization, agent: create(:cluster_agent, project: other_project), group: group), - create(:agent_project_authorization, agent: create(:cluster_agent, project: project), project: project), - Clusters::Agents::ImplicitAuthorization.new(agent: create(:cluster_agent, project: project)) + create(:agent_ci_access_group_authorization, agent: create(:cluster_agent, project: other_project), group: group), + create(:agent_ci_access_project_authorization, agent: create(:cluster_agent, project: project), project: project), + Clusters::Agents::Authorizations::CiAccess::ImplicitAuthorization.new(agent: create(:cluster_agent, project: project)) ] end let_it_be(:agent_authorizations_with_review_and_production_env) do [ create( - :agent_group_authorization, + :agent_ci_access_group_authorization, agent: create(:cluster_agent, project: other_project), group: group, environments: ['production', 'review/*'] ), create( - :agent_project_authorization, + :agent_ci_access_project_authorization, agent: create(:cluster_agent, project: project), project: project, environments: ['production', 'review/*'] @@ -224,13 +224,13 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do let_it_be(:agent_authorizations_with_staging_env) do [ create( - :agent_group_authorization, + :agent_ci_access_group_authorization, agent: create(:cluster_agent, project: other_project), group: group, environments: ['staging'] ), create( - :agent_project_authorization, + :agent_ci_access_project_authorization, agent: create(:cluster_agent, project: project), project: project, environments: ['staging'] @@ -546,40 +546,18 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do describe 'GET /projects/:id/jobs rate limited' do let(:query) { {} } - context 'with the ci_enforce_rate_limits_jobs_api feature flag on' do - before do - stub_feature_flags(ci_enforce_rate_limits_jobs_api: true) - - allow_next_instance_of(Gitlab::ApplicationRateLimiter::BaseStrategy) do |strategy| - threshold = Gitlab::ApplicationRateLimiter.rate_limits[:jobs_index][:threshold] - allow(strategy).to receive(:increment).and_return(threshold + 1) - end - - get api("/projects/#{project.id}/jobs", api_user), params: query + before do + allow_next_instance_of(Gitlab::ApplicationRateLimiter::BaseStrategy) do |strategy| + threshold = Gitlab::ApplicationRateLimiter.rate_limits[:jobs_index][:threshold] + allow(strategy).to receive(:increment).and_return(threshold + 1) end - it 'enforces rate limits for the endpoint' do - expect(response).to have_gitlab_http_status :too_many_requests - expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') - end + get api("/projects/#{project.id}/jobs", api_user), params: query end - context 'with the ci_enforce_rate_limits_jobs_api feature flag off' do - before do - stub_feature_flags(ci_enforce_rate_limits_jobs_api: false) - - allow_next_instance_of(Gitlab::ApplicationRateLimiter::BaseStrategy) do |strategy| - threshold = Gitlab::ApplicationRateLimiter.rate_limits[:jobs_index][:threshold] - allow(strategy).to receive(:increment).and_return(threshold + 1) - end - - get api("/projects/#{project.id}/jobs", api_user), params: query - end - - it 'makes a successful request' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_limited_pagination_headers - end + it 'enforces rate limits for the endpoint' do + expect(response).to have_gitlab_http_status :too_many_requests + expect(json_response['message']['error']).to eq('This endpoint has been requested too many times. Try again later.') end end diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index 2a2c5f65aee..d760e4ddf28 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -473,12 +473,12 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra end context 'as the existing owner of the schedule' do - it 'rejects the request and leaves the schedule unchanged' do + it 'accepts the request and leaves the schedule unchanged' do expect do post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/take_ownership", developer) end.not_to change { pipeline_schedule.reload.owner } - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(:success) end end end diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 6d69da85449..869b0ec9dca 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -14,7 +14,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let_it_be(:pipeline) do create(:ci_empty_pipeline, project: project, sha: project.commit.id, - ref: project.default_branch, user: user) + ref: project.default_branch, user: user, name: 'Build pipeline') end before do @@ -25,7 +25,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do it_behaves_like 'pipelines visibility table' context 'authorized user' do - it 'returns project pipelines' do + it 'returns project pipelines', :aggregate_failures do get api("/projects/#{project.id}/pipelines", user) expect(response).to have_gitlab_http_status(:ok) @@ -41,8 +41,44 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do it 'includes pipeline source' do get api("/projects/#{project.id}/pipelines", user) - expect(json_response.first.keys).to contain_exactly(*%w[id iid project_id sha ref status web_url created_at updated_at source]) + expect(json_response.first.keys).to contain_exactly(*%w[id iid project_id sha ref status web_url created_at updated_at source name]) end + + context 'when pipeline_name_in_api feature flag is off' do + before do + stub_feature_flags(pipeline_name_in_api: false) + end + + it 'does not include pipeline name in response and ignores name parameter' do + get api("/projects/#{project.id}/pipelines", user), params: { name: 'Chatops pipeline' } + + expect(json_response.length).to eq(1) + expect(json_response.first.keys).not_to include('name') + end + end + end + + it 'avoids N+1 queries' do + # Call to trigger any one time queries + get api("/projects/#{project.id}/pipelines", user), params: {} + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines", user), params: {} + end + + 3.times do + create( + :ci_empty_pipeline, + project: project, + sha: project.commit.id, + ref: project.default_branch, + user: user, + name: 'Build pipeline') + end + + expect do + get api("/projects/#{project.id}/pipelines", user), params: {} + end.not_to exceed_all_query_limit(control) end context 'when parameter is passed' do @@ -52,7 +88,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do create(:ci_pipeline, project: project, status: target) end - it 'returns matched pipelines' do + it 'returns matched pipelines', :aggregate_failures do get api("/projects/#{project.id}/pipelines", user), params: { scope: target } expect(response).to have_gitlab_http_status(:ok) @@ -303,11 +339,24 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end end end + + context 'when name is provided' do + let_it_be(:pipeline2) { create(:ci_empty_pipeline, project: project, user: user, name: 'Chatops pipeline') } + + it 'filters by name' do + get api("/projects/#{project.id}/pipelines", user), params: { name: 'Build pipeline' } + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.length).to eq(1) + expect(json_response.first['name']).to eq('Build pipeline') + end + end end end context 'unauthorized user' do - it 'does not return project pipelines' do + it 'does not return project pipelines', :aggregate_failures do get api("/projects/#{project.id}/pipelines", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -335,13 +384,13 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'authorized user' do - it 'returns pipeline jobs' do + it 'returns pipeline jobs', :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array end - it 'returns correct values' do + it 'returns correct values', :aggregate_failures do expect(json_response).not_to be_empty expect(json_response.first['commit']['id']).to eq project.commit.id expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) @@ -354,7 +403,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } end - it 'returns pipeline data' do + it 'returns pipeline data', :aggregate_failures do json_job = json_response.first expect(json_job['pipeline']).not_to be_empty @@ -368,7 +417,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'filter jobs with one scope element' do let(:query) { { 'scope' => 'pending' } } - it do + it :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array @@ -382,7 +431,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when filtering to only running jobs' do let(:query) { { 'scope' => 'running' } } - it do + it :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array @@ -402,7 +451,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'filter jobs with array of scope elements' do let(:query) { { scope: %w(pending running) } } - it do + it :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(json_response).to be_an Array end @@ -442,7 +491,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let_it_be(:successor) { create(:ci_build, :success, name: 'build', pipeline: pipeline) } - it 'does not return retried jobs by default' do + it 'does not return retried jobs by default', :aggregate_failures do expect(json_response).to be_an Array expect(json_response.length).to eq(1) end @@ -450,7 +499,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when include_retried is false' do let(:query) { { include_retried: false } } - it 'does not return retried jobs' do + it 'does not return retried jobs', :aggregate_failures do expect(json_response).to be_an Array expect(json_response.length).to eq(1) end @@ -459,7 +508,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when include_retried is true' do let(:query) { { include_retried: true } } - it 'returns retried jobs' do + it 'returns retried jobs', :aggregate_failures do expect(json_response).to be_an Array expect(json_response.length).to eq(2) expect(json_response[0]['name']).to eq(json_response[1]['name']) @@ -469,7 +518,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'no pipeline is found' do - it 'does not return jobs' do + it 'does not return jobs', :aggregate_failures do get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/jobs", user) expect(json_response['message']).to eq '404 Project Not Found' @@ -481,7 +530,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when user is not logged in' do let(:api_user) { nil } - it 'does not return jobs' do + it 'does not return jobs', :aggregate_failures do expect(json_response['message']).to eq '404 Project Not Found' expect(response).to have_gitlab_http_status(:not_found) end @@ -523,13 +572,13 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'authorized user' do - it 'returns pipeline bridges' do + it 'returns pipeline bridges', :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers expect(json_response).to be_an Array end - it 'returns correct values' do + it 'returns correct values', :aggregate_failures do expect(json_response).not_to be_empty expect(json_response.first['commit']['id']).to eq project.commit.id expect(json_response.first['id']).to eq bridge.id @@ -537,7 +586,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_response.first['stage']).to eq bridge.stage end - it 'returns pipeline data' do + it 'returns pipeline data', :aggregate_failures do json_bridge = json_response.first expect(json_bridge['pipeline']).not_to be_empty @@ -548,7 +597,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status end - it 'returns downstream pipeline data' do + it 'returns downstream pipeline data', :aggregate_failures do json_bridge = json_response.first expect(json_bridge['downstream_pipeline']).not_to be_empty @@ -568,7 +617,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'with one scope element' do let(:query) { { 'scope' => 'pending' } } - it :skip_before_request do + it :skip_before_request, :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query expect(response).to have_gitlab_http_status(:ok) @@ -581,7 +630,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'with array of scope elements' do let(:query) { { scope: %w(pending running) } } - it :skip_before_request do + it :skip_before_request, :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query expect(response).to have_gitlab_http_status(:ok) @@ -635,7 +684,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'no pipeline is found' do - it 'does not return bridges' do + it 'does not return bridges', :aggregate_failures do get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/bridges", user) expect(json_response['message']).to eq '404 Project Not Found' @@ -647,7 +696,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when user is not logged in' do let(:api_user) { nil } - it 'does not return bridges' do + it 'does not return bridges', :aggregate_failures do expect(json_response['message']).to eq '404 Project Not Found' expect(response).to have_gitlab_http_status(:not_found) end @@ -704,7 +753,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do stub_ci_pipeline_to_return_yaml_file end - it 'creates and returns a new pipeline' do + it 'creates and returns a new pipeline', :aggregate_failures do expect do post api("/projects/#{project.id}/pipeline", user), params: { ref: project.default_branch } end.to change { project.ci_pipelines.count }.by(1) @@ -717,7 +766,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'variables given' do let(:variables) { [{ 'variable_type' => 'file', 'key' => 'UPLOAD_TO_S3', 'value' => 'true' }] } - it 'creates and returns a new pipeline using the given variables' do + it 'creates and returns a new pipeline using the given variables', :aggregate_failures do expect do post api("/projects/#{project.id}/pipeline", user), params: { ref: project.default_branch, variables: variables } end.to change { project.ci_pipelines.count }.by(1) @@ -738,7 +787,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do stub_ci_pipeline_yaml_file(config) end - it 'creates and returns a new pipeline using the given variables' do + it 'creates and returns a new pipeline using the given variables', :aggregate_failures do expect do post api("/projects/#{project.id}/pipeline", user), params: { ref: project.default_branch, variables: variables } end.to change { project.ci_pipelines.count }.by(1) @@ -763,7 +812,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end end - it 'fails when using an invalid ref' do + it 'fails when using an invalid ref', :aggregate_failures do post api("/projects/#{project.id}/pipeline", user), params: { ref: 'invalid_ref' } expect(response).to have_gitlab_http_status(:bad_request) @@ -778,7 +827,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do project.update!(auto_devops_attributes: { enabled: false }) end - it 'fails to create pipeline' do + it 'fails to create pipeline', :aggregate_failures do post api("/projects/#{project.id}/pipeline", user), params: { ref: project.default_branch } expect(response).to have_gitlab_http_status(:bad_request) @@ -790,7 +839,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'unauthorized user' do - it 'does not create pipeline' do + it 'does not create pipeline', :aggregate_failures do post api("/projects/#{project.id}/pipeline", non_member), params: { ref: project.default_branch } expect(response).to have_gitlab_http_status(:not_found) @@ -811,21 +860,22 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'authorized user' do - it 'exposes known attributes' do + it 'exposes known attributes', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/pipeline/detail') end - it 'returns project pipeline' do + it 'returns project pipeline', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) expect(response).to have_gitlab_http_status(:ok) expect(json_response['sha']).to match(/\A\h{40}\z/) + expect(json_response['name']).to eq('Build pipeline') end - it 'returns 404 when it does not exist' do + it 'returns 404 when it does not exist', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{non_existing_record_id}", user) expect(response).to have_gitlab_http_status(:not_found) @@ -844,10 +894,23 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_response["coverage"]).to eq('30.00') end end + + context 'with pipeline_name_in_api disabled' do + before do + stub_feature_flags(pipeline_name_in_api: false) + end + + it 'does not return name', :aggregate_failures do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.keys).not_to include('name') + end + end end context 'unauthorized user' do - it 'does not return a project pipeline' do + it 'does not return a project pipeline', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -863,7 +926,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do create(:ci_pipeline, source: dangling_source, project: project) end - it 'returns the specified pipeline' do + it 'returns the specified pipeline', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{dangling_pipeline.id}", user) expect(response).to have_gitlab_http_status(:ok) @@ -878,7 +941,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let!(:second_pipeline) do create(:ci_empty_pipeline, project: project, sha: second_branch.target, - ref: second_branch.name, user: user) + ref: second_branch.name, user: user, name: 'Build pipeline') end before do @@ -887,18 +950,19 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'default repository branch' do - it 'gets the latest pipleine' do + it 'gets the latest pipleine', :aggregate_failures do get api("/projects/#{project.id}/pipelines/latest", user) expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('public_api/v4/pipeline/detail') expect(json_response['ref']).to eq(project.default_branch) expect(json_response['sha']).to eq(project.commit.id) + expect(json_response['name']).to eq('Build pipeline') end end context 'ref parameter' do - it 'gets the latest pipleine' do + it 'gets the latest pipleine', :aggregate_failures do get api("/projects/#{project.id}/pipelines/latest", user), params: { ref: second_branch.name } expect(response).to have_gitlab_http_status(:ok) @@ -907,10 +971,23 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do expect(json_response['sha']).to eq(second_branch.target) end end + + context 'with pipeline_name_in_api disabled' do + before do + stub_feature_flags(pipeline_name_in_api: false) + end + + it 'does not return name', :aggregate_failures do + get api("/projects/#{project.id}/pipelines/latest", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.keys).not_to include('name') + end + end end context 'unauthorized user' do - it 'does not return a project pipeline' do + it 'does not return a project pipeline', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -926,7 +1003,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let(:api_user) { user } context 'user is a mantainer' do - it 'returns pipeline variables empty' do + it 'returns pipeline variables empty', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -936,7 +1013,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'with variables' do let!(:variable) { create(:ci_pipeline_variable, pipeline: pipeline, key: 'foo', value: 'bar') } - it 'returns pipeline variables' do + it 'returns pipeline variables', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -962,7 +1039,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let(:api_user) { pipeline_owner_user } let!(:variable) { create(:ci_pipeline_variable, pipeline: pipeline, key: 'foo', value: 'bar') } - it 'returns pipeline variables' do + it 'returns pipeline variables', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -987,7 +1064,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'user is not a project member' do - it 'does not return pipeline variables' do + it 'does not return pipeline variables', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/variables", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -1000,14 +1077,14 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'authorized user' do let(:owner) { project.first_owner } - it 'destroys the pipeline' do + it 'destroys the pipeline', :aggregate_failures do delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) expect(response).to have_gitlab_http_status(:no_content) expect { pipeline.reload }.to raise_error(ActiveRecord::RecordNotFound) end - it 'returns 404 when it does not exist' do + it 'returns 404 when it does not exist', :aggregate_failures do delete api("/projects/#{project.id}/pipelines/#{non_existing_record_id}", owner) expect(response).to have_gitlab_http_status(:not_found) @@ -1021,7 +1098,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when the pipeline has jobs' do let_it_be(:build) { create(:ci_build, project: project, pipeline: pipeline) } - it 'destroys associated jobs' do + it 'destroys associated jobs', :aggregate_failures do delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", owner) expect(response).to have_gitlab_http_status(:no_content) @@ -1044,7 +1121,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'unauthorized user' do context 'when user is not member' do - it 'returns a 404' do + it 'returns a 404', :aggregate_failures do delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -1059,7 +1136,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do project.add_developer(developer) end - it 'returns a 403' do + it 'returns a 403', :aggregate_failures do delete api("/projects/#{project.id}/pipelines/#{pipeline.id}", developer) expect(response).to have_gitlab_http_status(:forbidden) @@ -1078,7 +1155,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let_it_be(:build) { create(:ci_build, :failed, pipeline: pipeline) } - it 'retries failed builds' do + it 'retries failed builds', :aggregate_failures do expect do post api("/projects/#{project.id}/pipelines/#{pipeline.id}/retry", user) end.to change { pipeline.builds.count }.from(1).to(2) @@ -1089,7 +1166,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'unauthorized user' do - it 'does not return a project pipeline' do + it 'does not return a project pipeline', :aggregate_failures do post api("/projects/#{project.id}/pipelines/#{pipeline.id}/retry", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -1106,7 +1183,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end end - it 'returns error' do + it 'returns error', :aggregate_failures do post api("/projects/#{project.id}/pipelines/#{pipeline.id}/retry", user) expect(response).to have_gitlab_http_status(:forbidden) @@ -1124,7 +1201,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let_it_be(:build) { create(:ci_build, :running, pipeline: pipeline) } - context 'authorized user' do + context 'authorized user', :aggregate_failures do it 'retries failed builds', :sidekiq_might_not_need_inline do post api("/projects/#{project.id}/pipelines/#{pipeline.id}/cancel", user) @@ -1140,7 +1217,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do project.add_reporter(reporter) end - it 'rejects the action' do + it 'rejects the action', :aggregate_failures do post api("/projects/#{project.id}/pipelines/#{pipeline.id}/cancel", reporter) expect(response).to have_gitlab_http_status(:forbidden) @@ -1156,7 +1233,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let(:pipeline) { create(:ci_pipeline, project: project) } context 'when pipeline does not have a test report' do - it 'returns an empty test report' do + it 'returns an empty test report', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -1167,7 +1244,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when pipeline has a test report' do let(:pipeline) { create(:ci_pipeline, :with_test_reports, project: project) } - it 'returns the test report' do + it 'returns the test report', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -1180,7 +1257,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do create(:ci_build, :broken_test_reports, name: 'rspec', pipeline: pipeline) end - it 'returns a suite_error' do + it 'returns a suite_error', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -1190,7 +1267,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'unauthorized user' do - it 'does not return project pipelines' do + it 'does not return project pipelines', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/test_report", non_member) expect(response).to have_gitlab_http_status(:not_found) @@ -1208,7 +1285,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do let(:pipeline) { create(:ci_pipeline, project: project) } context 'when pipeline does not have a test report summary' do - it 'returns an empty test report summary' do + it 'returns an empty test report summary', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -1219,7 +1296,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do context 'when pipeline has a test report summary' do let(:pipeline) { create(:ci_pipeline, :with_report_results, project: project) } - it 'returns the test report summary' do + it 'returns the test report summary', :aggregate_failures do subject expect(response).to have_gitlab_http_status(:ok) @@ -1229,7 +1306,7 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end context 'unauthorized user' do - it 'does not return project pipelines' do + it 'does not return project pipelines', :aggregate_failures do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/test_report_summary", non_member) expect(response).to have_gitlab_http_status(:not_found) diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index 3d3d699542b..596af1110cc 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -174,8 +174,21 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego expect(json_response['RemoteObject']).to have_key('StoreURL') expect(json_response['RemoteObject']).to have_key('DeleteURL') expect(json_response['RemoteObject']).to have_key('MultipartUpload') + expect(json_response['RemoteObject']['SkipDelete']).to eq(true) expect(json_response['MaximumSize']).not_to be_nil end + + context 'when ci_artifacts_upload_to_final_location flag is disabled' do + before do + stub_feature_flags(ci_artifacts_upload_to_final_location: false) + end + + it 'does not skip delete' do + subject + + expect(json_response['RemoteObject']['SkipDelete']).to eq(false) + end + end end context 'when direct upload is disabled' do @@ -255,8 +268,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego it 'tracks code_intelligence usage ping' do tracking_params = { event_names: 'i_source_code_code_intelligence', - start_date: Date.yesterday, - end_date: Date.today + start_date: Date.today.beginning_of_week, + end_date: 1.week.from_now } expect { authorize_artifacts_with_token_in_headers(artifact_type: :lsif) } @@ -374,29 +387,53 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let(:object) do fog_connection.directories.new(key: 'artifacts').files.create( # rubocop:disable Rails/SaveBang - key: 'tmp/uploads/12312300', + key: remote_path, body: 'content' ) end let(:file_upload) { fog_to_uploaded_file(object) } - before do - upload_artifacts(file_upload, headers_with_token, 'file.remote_id' => remote_id) - end + context 'when uploaded file has matching pending remote upload to its final location' do + let(:remote_path) { '12345/foo-bar-123' } + let(:object_remote_id) { remote_path } + let(:remote_id) { remote_path } + + before do + allow(JobArtifactUploader).to receive(:generate_final_store_path).and_return(remote_path) - context 'when valid remote_id is used' do - let(:remote_id) { '12312300' } + ObjectStorage::PendingDirectUpload.prepare( + JobArtifactUploader.storage_location_identifier, + remote_path + ) + + upload_artifacts(file_upload, headers_with_token, 'file.remote_id' => remote_path) + end it_behaves_like 'successful artifacts upload' end - context 'when invalid remote_id is used' do - let(:remote_id) { 'invalid id' } + context 'when uploaded file is uploaded to temporary location' do + let(:object_remote_id) { JobArtifactUploader.generate_remote_id } + let(:remote_path) { File.join(ObjectStorage::TMP_UPLOAD_PATH, object_remote_id) } + + before do + upload_artifacts(file_upload, headers_with_token, 'file.remote_id' => remote_id) + end + + context 'and matching temporary remote_id is used' do + let(:remote_id) { object_remote_id } + + it_behaves_like 'successful artifacts upload' + end + + context 'and invalid remote_id is used' do + let(:remote_id) { JobArtifactUploader.generate_remote_id } - it 'responds with bad request' do - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(json_response['message']).to eq("Missing file") + it 'responds with internal server error' do + expect(response).to have_gitlab_http_status(:internal_server_error) + expect(json_response['message']).to eq("Missing file") + end end end end diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index ef3b38e3fc4..ab7ab4e74f8 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -21,13 +21,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let_it_be(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) } - let_it_be(:runner_machine) { create(:ci_runner_machine, runner: runner) } + let_it_be(:runner_manager) { create(:ci_runner_machine, runner: runner) } let_it_be(:user) { create(:user) } describe 'PUT /api/v4/jobs/:id' do let_it_be_with_reload(:job) do create(:ci_build, :pending, :trace_live, pipeline: pipeline, project: project, user: user, - runner_id: runner.id, runner_machine: runner_machine) + runner_id: runner.id, runner_manager: runner_manager) end before do @@ -40,7 +40,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego it 'updates runner info' do expect { update_job(state: 'success') }.to change { runner.reload.contacted_at } - .and change { runner_machine.reload.contacted_at } + .and change { runner_manager.reload.contacted_at } end context 'when status is given' do diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 6e721d40560..0164eda7680 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -122,56 +122,33 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego context 'when system_id parameter is specified' do subject(:request) { request_job(**args) } - context 'with create_runner_machine FF enabled' do - before do - stub_feature_flags(create_runner_machine: true) - end - - context 'when ci_runner_machines with same system_xid does not exist' do - let(:args) { { system_id: 's_some_system_id' } } - - it 'creates respective ci_runner_machines record', :freeze_time do - expect { request }.to change { runner.runner_machines.reload.count }.from(0).to(1) - - machine = runner.runner_machines.last - expect(machine.system_xid).to eq args[:system_id] - expect(machine.runner).to eq runner - expect(machine.contacted_at).to eq Time.current - end - end - - context 'when ci_runner_machines with same system_xid already exists', :freeze_time do - let(:args) { { system_id: 's_existing_system_id' } } - let!(:runner_machine) do - create(:ci_runner_machine, runner: runner, system_xid: args[:system_id], contacted_at: 1.hour.ago) - end - - it 'does not create new ci_runner_machines record' do - expect { request }.not_to change { Ci::RunnerMachine.count } - end + context 'when ci_runner_machines with same system_xid does not exist' do + let(:args) { { system_id: 's_some_system_id' } } - it 'updates the contacted_at field' do - request + it 'creates respective ci_runner_machines record', :freeze_time do + expect { request }.to change { runner.runner_managers.reload.count }.from(0).to(1) - expect(runner_machine.reload.contacted_at).to eq Time.current - end + runner_manager = runner.runner_managers.last + expect(runner_manager.system_xid).to eq args[:system_id] + expect(runner_manager.runner).to eq runner + expect(runner_manager.contacted_at).to eq Time.current end end - context 'with create_runner_machine FF disabled' do - before do - stub_feature_flags(create_runner_machine: false) + context 'when ci_runner_machines with same system_xid already exists', :freeze_time do + let(:args) { { system_id: 's_existing_system_id' } } + let!(:runner_manager) do + create(:ci_runner_machine, runner: runner, system_xid: args[:system_id], contacted_at: 1.hour.ago) end - context 'when ci_runner_machines with same system_xid does not exist' do - let(:args) { { system_id: 's_some_system_id' } } + it 'does not create new ci_runner_machines record' do + expect { request }.not_to change { Ci::RunnerManager.count } + end - it 'does not create respective ci_runner_machines record', :freeze_time, :aggregate_failures do - expect { request }.not_to change { runner.runner_machines.reload.count } + it 'updates the contacted_at field' do + request - expect(response).to have_gitlab_http_status(:created) - expect(runner.runner_machines).to be_empty - end + expect(runner_manager.reload.contacted_at).to eq Time.current end end end @@ -253,11 +230,14 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end let(:expected_cache) do - [{ 'key' => a_string_matching(/^cache_key-(?>protected|non_protected)$/), - 'untracked' => false, - 'paths' => ['vendor/*'], - 'policy' => 'pull-push', - 'when' => 'on_success' }] + [{ + 'key' => a_string_matching(/^cache_key-(?>protected|non_protected)$/), + 'untracked' => false, + 'paths' => ['vendor/*'], + 'policy' => 'pull-push', + 'when' => 'on_success', + 'fallback_keys' => [] + }] end let(:expected_features) do @@ -366,36 +346,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end - context 'when job filtered by job_age' do - let!(:job) do - create(:ci_build, :pending, :queued, :tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0, queued_at: 60.seconds.ago) - end - - before do - job.queuing_entry&.update!(created_at: 60.seconds.ago) - end - - context 'job is queued less than job_age parameter' do - let(:job_age) { 120 } - - it 'gives 204' do - request_job(job_age: job_age) - - expect(response).to have_gitlab_http_status(:no_content) - end - end - - context 'job is queued more than job_age parameter' do - let(:job_age) { 30 } - - it 'picks a job' do - request_job(job_age: job_age) - - expect(response).to have_gitlab_http_status(:created) - end - end - end - context 'when job is made for branch' do it 'sets tag as ref_type' do request_job @@ -831,19 +781,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end end - - context 'when the FF ci_hooks_pre_get_sources_script is disabled' do - before do - stub_feature_flags(ci_hooks_pre_get_sources_script: false) - end - - it 'does not return the pre_get_sources_script' do - request_job - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).not_to have_key('hooks') - end - end end describe 'port support' do diff --git a/spec/requests/api/ci/runner/runners_delete_spec.rb b/spec/requests/api/ci/runner/runners_delete_spec.rb index 65c287a9535..681dd4d701e 100644 --- a/spec/requests/api/ci/runner/runners_delete_spec.rb +++ b/spec/requests/api/ci/runner/runners_delete_spec.rb @@ -7,16 +7,19 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego include RedisHelpers include WorkhorseHelpers - let(:registration_token) { 'abcdefg123456' } - before do stub_feature_flags(ci_enable_live_trace: true) stub_gitlab_calls - stub_application_setting(runners_registration_token: registration_token) - allow_any_instance_of(::Ci::Runner).to receive(:cache_attributes) + allow_next_instance_of(::Ci::Runner) { |runner| allow(runner).to receive(:cache_attributes) } end describe '/api/v4/runners' do + let(:registration_token) { 'abcdefg123456' } + + before do + stub_application_setting(runners_registration_token: registration_token) + end + describe 'DELETE /api/v4/runners' do context 'when no token is provided' do it 'returns 400 error' do @@ -57,4 +60,85 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end end + + describe '/api/v4/runners/managers' do + describe 'DELETE /api/v4/runners/managers' do + subject(:delete_request) { delete api('/runners/managers'), params: delete_params } + + context 'with created runner' do + let!(:runner) { create(:ci_runner, :with_runner_manager, registration_type: :authenticated_user) } + + context 'with matching system_id' do + context 'when no token is provided' do + let(:delete_params) { { system_id: runner.runner_managers.first.system_xid } } + + it 'returns 400 error' do + delete_request + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'when invalid token is provided' do + let(:delete_params) { { token: 'invalid', system_id: runner.runner_managers.first.system_xid } } + + it 'returns 403 error' do + delete_request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + context 'when valid token is provided' do + context 'with created runner' do + let!(:runner) { create(:ci_runner, :with_runner_manager, registration_type: :authenticated_user) } + + context 'with matching system_id' do + let(:delete_params) { { token: runner.token, system_id: runner.runner_managers.first.system_xid } } + + it 'deletes runner manager' do + expect do + delete_request + + expect(response).to have_gitlab_http_status(:no_content) + end.to change { runner.runner_managers.count }.from(1).to(0) + + expect(::Ci::Runner.count).to eq(1) + end + + it_behaves_like '412 response' do + let(:request) { api('/runners/managers') } + let(:params) { delete_params } + end + + it_behaves_like 'storing arguments in the application context for the API' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end + end + + context 'with unknown system_id' do + let(:delete_params) { { token: runner.token, system_id: 'unknown_system_id' } } + + it 'returns 404 error' do + delete_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'without system_id' do + let(:delete_params) { { token: runner.token } } + + it 'does not delete runner manager nor runner' do + delete_request + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end + end + end + end end diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 73f8e87a9fb..a36ea2115cf 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -15,14 +15,10 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego context 'when invalid token is provided' do it 'returns 403 error' do - allow_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| - allow(service).to receive(:execute) - .and_return(ServiceResponse.error(message: 'invalid token supplied', http_status: :forbidden)) - end - post api('/runners'), params: { token: 'invalid' } expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('403 Forbidden - invalid token supplied') end end @@ -44,21 +40,24 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let_it_be(:new_runner) { create(:ci_runner) } before do - allow_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| - expected_params = { - description: 'server.hostname', - maintenance_note: 'Some maintainer notes', - run_untagged: false, - tag_list: %w(tag1 tag2), - locked: true, - active: true, - access_level: 'ref_protected', - maximum_timeout: 9000 - }.stringify_keys + expected_params = { + description: 'server.hostname', + maintenance_note: 'Some maintainer notes', + run_untagged: false, + tag_list: %w(tag1 tag2), + locked: true, + active: true, + access_level: 'ref_protected', + maximum_timeout: 9000 + }.stringify_keys + allow_next_instance_of( + ::Ci::Runners::RegisterRunnerService, + 'valid token', + a_hash_including(expected_params) + ) do |service| expect(service).to receive(:execute) .once - .with('valid token', a_hash_including(expected_params)) .and_return(ServiceResponse.success(payload: { runner: new_runner })) end end @@ -109,11 +108,14 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let(:new_runner) { create(:ci_runner) } it 'converts to maintenance_note param' do - allow_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| + allow_next_instance_of( + ::Ci::Runners::RegisterRunnerService, + 'valid token', + a_hash_including('maintenance_note' => 'Some maintainer notes') + .and(excluding('maintainter_note' => anything)) + ) do |service| expect(service).to receive(:execute) .once - .with('valid token', a_hash_including('maintenance_note' => 'Some maintainer notes') - .and(excluding('maintainter_note' => anything))) .and_return(ServiceResponse.success(payload: { runner: new_runner })) end @@ -134,12 +136,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let_it_be(:new_runner) { build(:ci_runner) } it 'uses active value in registration' do - expect_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| - expected_params = { active: false }.stringify_keys - + expect_next_instance_of( + ::Ci::Runners::RegisterRunnerService, + 'valid token', + a_hash_including({ active: false }.stringify_keys) + ) do |service| expect(service).to receive(:execute) .once - .with('valid token', a_hash_including(expected_params)) .and_return(ServiceResponse.success(payload: { runner: new_runner })) end @@ -197,12 +200,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let(:tag_list) { (1..::Ci::Runner::TAG_LIST_MAX_LENGTH + 1).map { |i| "tag#{i}" } } it 'uses tag_list value in registration and returns error' do - expect_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| - expected_params = { tag_list: tag_list }.stringify_keys - + expect_next_instance_of( + ::Ci::Runners::RegisterRunnerService, + registration_token, + a_hash_including({ tag_list: tag_list }.stringify_keys) + ) do |service| expect(service).to receive(:execute) .once - .with(registration_token, a_hash_including(expected_params)) .and_call_original end @@ -217,12 +221,13 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego let(:tag_list) { (1..20).map { |i| "tag#{i}" } } it 'uses tag_list value in registration and successfully creates runner' do - expect_next_instance_of(::Ci::Runners::RegisterRunnerService) do |service| - expected_params = { tag_list: tag_list }.stringify_keys - + expect_next_instance_of( + ::Ci::Runners::RegisterRunnerService, + registration_token, + a_hash_including({ tag_list: tag_list }.stringify_keys) + ) do |service| expect(service).to receive(:execute) .once - .with(registration_token, a_hash_including(expected_params)) .and_call_original end @@ -232,6 +237,18 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end end end + + context 'when runner registration is disallowed' do + before do + stub_application_setting(allow_runner_registration_token: false) + end + + it 'returns 410 Gone status' do + post api('/runners'), params: { token: registration_token } + + expect(response).to have_gitlab_http_status(:gone) + end + end end end end diff --git a/spec/requests/api/ci/runner/runners_verify_post_spec.rb b/spec/requests/api/ci/runner/runners_verify_post_spec.rb index a6a1ad947aa..f1b33826f5e 100644 --- a/spec/requests/api/ci/runner/runners_verify_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_verify_post_spec.rb @@ -17,7 +17,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego end describe '/api/v4/runners' do - describe 'POST /api/v4/runners/verify' do + describe 'POST /api/v4/runners/verify', :freeze_time do let_it_be_with_reload(:runner) { create(:ci_runner, token_expires_at: 3.days.from_now) } let(:params) {} @@ -45,9 +45,12 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego context 'when valid token is provided' do let(:params) { { token: runner.token } } - context 'with create_runner_machine FF enabled' do - before do - stub_feature_flags(create_runner_machine: true) + context 'with glrt-prefixed token' do + let_it_be(:registration_token) { 'glrt-abcdefg123456' } + let_it_be(:registration_type) { :authenticated_user } + let_it_be(:runner) do + create(:ci_runner, registration_type: registration_type, + token: registration_token, token_expires_at: 3.days.from_now) end it 'verifies Runner credentials' do @@ -61,39 +64,29 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego }) end - context 'with non-expiring runner token' do - before do - runner.update!(token_expires_at: nil) - end - - it 'verifies Runner credentials' do - verify - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq({ - 'id' => runner.id, - 'token' => runner.token, - 'token_expires_at' => nil - }) - end + it 'does not update contacted_at' do + expect { verify }.not_to change { runner.reload.contacted_at }.from(nil) end + end - it_behaves_like 'storing arguments in the application context for the API' do - let(:expected_params) { { client_id: "runner/#{runner.id}" } } - end + it 'verifies Runner credentials' do + verify - context 'when system_id is provided' do - let(:params) { { token: runner.token, system_id: 's_some_system_id' } } + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'id' => runner.id, + 'token' => runner.token, + 'token_expires_at' => runner.token_expires_at.iso8601(3) + }) + end - it 'creates a runner_machine' do - expect { verify }.to change { Ci::RunnerMachine.count }.by(1) - end - end + it 'updates contacted_at' do + expect { verify }.to change { runner.reload.contacted_at }.from(nil).to(Time.current) end - context 'with create_runner_machine FF disabled' do + context 'with non-expiring runner token' do before do - stub_feature_flags(create_runner_machine: false) + runner.update!(token_expires_at: nil) end it 'verifies Runner credentials' do @@ -103,18 +96,20 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego expect(json_response).to eq({ 'id' => runner.id, 'token' => runner.token, - 'token_expires_at' => runner.token_expires_at.iso8601(3) + 'token_expires_at' => nil }) end + end - context 'when system_id is provided' do - let(:params) { { token: runner.token, system_id: 's_some_system_id' } } + it_behaves_like 'storing arguments in the application context for the API' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end - it 'does not create a runner_machine', :aggregate_failures do - expect { verify }.not_to change { Ci::RunnerMachine.count } + context 'when system_id is provided' do + let(:params) { { token: runner.token, system_id: 's_some_system_id' } } - expect(response).to have_gitlab_http_status(:ok) - end + it 'creates a runner_manager' do + expect { verify }.to change { Ci::RunnerManager.count }.by(1) end end end diff --git a/spec/requests/api/ci/runners_reset_registration_token_spec.rb b/spec/requests/api/ci/runners_reset_registration_token_spec.rb index 1110dbf5fbc..98edde93e95 100644 --- a/spec/requests/api/ci/runners_reset_registration_token_spec.rb +++ b/spec/requests/api/ci/runners_reset_registration_token_spec.rb @@ -3,10 +3,12 @@ require 'spec_helper' RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do - subject { post api("#{prefix}/runners/reset_registration_token", user) } + let_it_be(:admin_mode) { false } + + subject { post api("#{prefix}/runners/reset_registration_token", user, admin_mode: admin_mode) } shared_examples 'bad request' do |result| - it 'returns 400 error' do + it 'returns 400 error', :aggregate_failures do expect { subject }.not_to change { get_token } expect(response).to have_gitlab_http_status(:bad_request) @@ -15,7 +17,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end shared_examples 'unauthenticated' do - it 'returns 401 error' do + it 'returns 401 error', :aggregate_failures do expect { subject }.not_to change { get_token } expect(response).to have_gitlab_http_status(:unauthorized) @@ -23,7 +25,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end shared_examples 'unauthorized' do - it 'returns 403 error' do + it 'returns 403 error', :aggregate_failures do expect { subject }.not_to change { get_token } expect(response).to have_gitlab_http_status(:forbidden) @@ -31,7 +33,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end shared_examples 'not found' do |scope| - it 'returns 404 error' do + it 'returns 404 error', :aggregate_failures do expect { subject }.not_to change { get_token } expect(response).to have_gitlab_http_status(:not_found) @@ -58,7 +60,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end shared_context 'when authorized' do |scope| - it 'resets runner registration token' do + it 'resets runner registration token', :aggregate_failures do expect { subject }.to change { get_token } expect(response).to have_gitlab_http_status(:success) @@ -99,6 +101,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do include_context 'when authorized', 'instance' do let_it_be(:user) { create(:user, :admin) } + let_it_be(:admin_mode) { true } def get_token ApplicationSetting.current_without_cache.runners_registration_token diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index ca051386265..2b2d2e0def8 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do +RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :runner_fleet do let_it_be(:admin) { create(:user, :admin) } let_it_be(:user) { create(:user) } let_it_be(:user2) { create(:user) } @@ -134,17 +134,21 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end describe 'GET /runners/all' do + let(:path) { '/runners/all' } + + it_behaves_like 'GET request permissions for admin mode' + context 'authorized user' do context 'with admin privileges' do it 'returns response status and headers' do - get api('/runners/all', admin) + get api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers end it 'returns all runners' do - get api('/runners/all', admin) + get api(path, admin, admin_mode: true) expect(json_response).to match_array [ a_hash_including('description' => 'Project runner', 'is_shared' => false, 'active' => true, 'paused' => false, 'runner_type' => 'project_type'), @@ -156,7 +160,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'filters runners by scope' do - get api('/runners/all?scope=shared', admin) + get api('/runners/all?scope=shared', admin, admin_mode: true) shared = json_response.all? { |r| r['is_shared'] } expect(response).to have_gitlab_http_status(:ok) @@ -167,7 +171,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'filters runners by scope' do - get api('/runners/all?scope=specific', admin) + get api('/runners/all?scope=specific', admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -181,12 +185,12 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'avoids filtering if scope is invalid' do - get api('/runners/all?scope=unknown', admin) + get api('/runners/all?scope=unknown', admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end it 'filters runners by project type' do - get api('/runners/all?type=project_type', admin) + get api('/runners/all?type=project_type', admin, admin_mode: true) expect(json_response).to match_array [ a_hash_including('description' => 'Project runner'), @@ -195,7 +199,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'filters runners by group type' do - get api('/runners/all?type=group_type', admin) + get api('/runners/all?type=group_type', admin, admin_mode: true) expect(json_response).to match_array [ a_hash_including('description' => 'Group runner A'), @@ -204,7 +208,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'does not filter by invalid type' do - get api('/runners/all?type=bogus', admin) + get api('/runners/all?type=bogus', admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end @@ -213,7 +217,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do let_it_be(:runner) { create(:ci_runner, :project, :inactive, description: 'Inactive project runner', projects: [project]) } it 'filters runners by status' do - get api('/runners/all?paused=true', admin) + get api('/runners/all?paused=true', admin, admin_mode: true) expect(json_response).to match_array [ a_hash_including('description' => 'Inactive project runner') @@ -221,7 +225,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'filters runners by status' do - get api('/runners/all?status=paused', admin) + get api('/runners/all?status=paused', admin, admin_mode: true) expect(json_response).to match_array [ a_hash_including('description' => 'Inactive project runner') @@ -230,7 +234,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'does not filter by invalid status' do - get api('/runners/all?status=bogus', admin) + get api('/runners/all?status=bogus', admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end @@ -239,7 +243,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do create(:ci_runner, :project, description: 'Runner tagged with tag1 and tag2', projects: [project], tag_list: %w[tag1 tag2]) create(:ci_runner, :project, description: 'Runner tagged with tag2', projects: [project], tag_list: ['tag2']) - get api('/runners/all?tag_list=tag1,tag2', admin) + get api('/runners/all?tag_list=tag1,tag2', admin, admin_mode: true) expect(json_response).to match_array [ a_hash_including('description' => 'Runner tagged with tag1 and tag2') @@ -249,7 +253,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'without admin privileges' do it 'does not return runners list' do - get api('/runners/all', user) + get api(path, user) expect(response).to have_gitlab_http_status(:forbidden) end @@ -266,6 +270,10 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end describe 'GET /runners/:id' do + let(:path) { "/runners/#{project_runner.id}" } + + it_behaves_like 'GET request permissions for admin mode' + context 'admin user' do context 'when runner is shared' do it "returns runner's details" do @@ -286,7 +294,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do it 'deletes unused runner' do expect do - delete api("/runners/#{unused_project_runner.id}", admin) + delete api("/runners/#{unused_project_runner.id}", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:no_content) end.to change { ::Ci::Runner.project_type.count }.by(-1) @@ -294,21 +302,21 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it "returns runner's details" do - get api("/runners/#{project_runner.id}", admin) + get api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(json_response['description']).to eq(project_runner.description) end it "returns the project's details for a project runner" do - get api("/runners/#{project_runner.id}", admin) + get api(path, admin, admin_mode: true) expect(json_response['projects'].first['id']).to eq(project.id) end end it 'returns 404 if runner does not exist' do - get api('/runners/0', admin) + get api("/runners/#{non_existing_record_id}", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:not_found) end @@ -316,7 +324,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when the runner is a group runner' do it "returns the runner's details" do - get api("/runners/#{group_runner_a.id}", admin) + get api("/runners/#{group_runner_a.id}", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(json_response['description']).to eq(group_runner_a.description) @@ -327,7 +335,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context "runner project's administrative user" do context 'when runner is not shared' do it "returns runner's details" do - get api("/runners/#{project_runner.id}", user) + get api(path, user) expect(response).to have_gitlab_http_status(:ok) expect(json_response['description']).to eq(project_runner.description) @@ -346,7 +354,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'other authorized user' do it "does not return project runner's details" do - get api("/runners/#{project_runner.id}", user2) + get api(path, user2) expect(response).to have_gitlab_http_status(:forbidden) end @@ -354,7 +362,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'unauthorized user' do it "does not return project runner's details" do - get api("/runners/#{project_runner.id}") + get api(path) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -362,6 +370,12 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end describe 'PUT /runners/:id' do + let(:path) { "/runners/#{project_runner.id}" } + + it_behaves_like 'PUT request permissions for admin mode' do + let(:params) { { description: 'test' } } + end + context 'admin user' do # see https://gitlab.com/gitlab-org/gitlab-foss/issues/48625 context 'single parameter update' do @@ -492,20 +506,22 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'returns 404 if runner does not exist' do - update_runner(0, admin, description: 'test') + update_runner(non_existing_record_id, admin, description: 'test') expect(response).to have_gitlab_http_status(:not_found) end def update_runner(id, user, args) - put api("/runners/#{id}", user), params: args + put api("/runners/#{id}", user, admin_mode: true), params: args end end context 'authorized user' do + let_it_be(:params) { { description: 'test' } } + context 'when runner is shared' do it 'does not update runner' do - put api("/runners/#{shared_runner.id}", user), params: { description: 'test' } + put api("/runners/#{shared_runner.id}", user), params: params expect(response).to have_gitlab_http_status(:forbidden) end @@ -513,17 +529,16 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when runner is not shared' do it 'does not update project runner without access to it' do - put api("/runners/#{project_runner.id}", user2), params: { description: 'test' } + put api(path, user2), params: { description: 'test' } expect(response).to have_gitlab_http_status(:forbidden) end it 'updates project runner with access to it' do description = project_runner.description - put api("/runners/#{project_runner.id}", admin), params: { description: 'test' } + put api(path, admin, admin_mode: true), params: params project_runner.reload - expect(response).to have_gitlab_http_status(:ok) expect(project_runner.description).to eq('test') expect(project_runner.description).not_to eq(description) end @@ -532,7 +547,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'unauthorized user' do it 'does not delete project runner' do - put api("/runners/#{project_runner.id}") + put api(path) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -540,6 +555,10 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end describe 'DELETE /runners/:id' do + let(:path) { "/runners/#{shared_runner.id}" } + + it_behaves_like 'DELETE request permissions for admin mode' + context 'admin user' do context 'when runner is shared' do it 'deletes runner' do @@ -548,14 +567,14 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end expect do - delete api("/runners/#{shared_runner.id}", admin) + delete api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:no_content) end.to change { ::Ci::Runner.instance_type.count }.by(-1) end it_behaves_like '412 response' do - let(:request) { api("/runners/#{shared_runner.id}", admin) } + let(:request) { api(path, admin, admin_mode: true) } end end @@ -566,7 +585,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end expect do - delete api("/runners/#{project_runner.id}", admin) + delete api("/runners/#{project_runner.id}", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:no_content) end.to change { ::Ci::Runner.project_type.count }.by(-1) @@ -578,7 +597,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do expect(service).not_to receive(:execute) end - delete api('/runners/0', admin) + delete api("/runners/#{non_existing_record_id}", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:not_found) end @@ -587,7 +606,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'authorized user' do context 'when runner is shared' do it 'does not delete runner' do - delete api("/runners/#{shared_runner.id}", user) + delete api(path, user) expect(response).to have_gitlab_http_status(:forbidden) end end @@ -671,10 +690,16 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end describe 'POST /runners/:id/reset_authentication_token' do + let(:path) { "/runners/#{shared_runner.id}/reset_authentication_token" } + + it_behaves_like 'POST request permissions for admin mode' do + let(:params) { {} } + end + context 'admin user' do it 'resets shared runner authentication token' do expect do - post api("/runners/#{shared_runner.id}/reset_authentication_token", admin) + post api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:success) expect(json_response).to eq({ 'token' => shared_runner.reload.token, 'token_expires_at' => nil }) @@ -682,7 +707,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'returns 404 if runner does not exist' do - post api('/runners/0/reset_authentication_token', admin) + post api("/runners/#{non_existing_record_id}/reset_authentication_token", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:not_found) end @@ -765,7 +790,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'unauthorized user' do it 'does not reset authentication token' do expect do - post api("/runners/#{shared_runner.id}/reset_authentication_token") + post api(path) expect(response).to have_gitlab_http_status(:unauthorized) end.not_to change { shared_runner.reload.token } @@ -779,12 +804,15 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do let_it_be(:job_3) { create(:ci_build, :failed, runner: shared_runner, project: project) } let_it_be(:job_4) { create(:ci_build, :running, runner: project_runner, project: project) } let_it_be(:job_5) { create(:ci_build, :failed, runner: project_runner, project: project) } + let(:path) { "/runners/#{project_runner.id}/jobs" } + + it_behaves_like 'GET request permissions for admin mode' context 'admin user' do context 'when runner exists' do context 'when runner is shared' do it 'return jobs' do - get api("/runners/#{shared_runner.id}/jobs", admin) + get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -796,7 +824,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when runner is a project runner' do it 'return jobs' do - get api("/runners/#{project_runner.id}/jobs", admin) + get api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -825,7 +853,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when valid status is provided' do it 'return filtered jobs' do - get api("/runners/#{project_runner.id}/jobs?status=failed", admin) + get api("/runners/#{project_runner.id}/jobs?status=failed", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -839,7 +867,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when valid order_by is provided' do context 'when sort order is not specified' do it 'return jobs in descending order' do - get api("/runners/#{project_runner.id}/jobs?order_by=id", admin) + get api("/runners/#{project_runner.id}/jobs?order_by=id", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -852,7 +880,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when sort order is specified as asc' do it 'return jobs sorted in ascending order' do - get api("/runners/#{project_runner.id}/jobs?order_by=id&sort=asc", admin) + get api("/runners/#{project_runner.id}/jobs?order_by=id&sort=asc", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -866,7 +894,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when invalid status is provided' do it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?status=non-existing", admin) + get api("/runners/#{project_runner.id}/jobs?status=non-existing", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end @@ -874,7 +902,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when invalid order_by is provided' do it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?order_by=non-existing", admin) + get api("/runners/#{project_runner.id}/jobs?order_by=non-existing", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end @@ -882,7 +910,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when invalid sort is provided' do it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?sort=non-existing", admin) + get api("/runners/#{project_runner.id}/jobs?sort=non-existing", admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end @@ -890,16 +918,16 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'avoids N+1 DB queries' do - get api("/runners/#{shared_runner.id}/jobs", admin) + get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) control = ActiveRecord::QueryRecorder.new do - get api("/runners/#{shared_runner.id}/jobs", admin) + get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) end create(:ci_build, :failed, runner: shared_runner, project: project) expect do - get api("/runners/#{shared_runner.id}/jobs", admin) + get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) end.not_to exceed_query_limit(control.count) end @@ -925,12 +953,12 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do ]).once.and_call_original end - get api("/runners/#{shared_runner.id}/jobs", admin), params: { per_page: 2, order_by: 'id', sort: 'desc' } + get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true), params: { per_page: 2, order_by: 'id', sort: 'desc' } end context "when runner doesn't exist" do it 'returns 404' do - get api('/runners/0/jobs', admin) + get api('/runners/0/jobs', admin, admin_mode: true) expect(response).to have_gitlab_http_status(:not_found) end @@ -949,7 +977,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'when runner is a project runner' do it 'return jobs' do - get api("/runners/#{project_runner.id}/jobs", user) + get api(path, user) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -992,7 +1020,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'other authorized user' do it 'does not return jobs' do - get api("/runners/#{project_runner.id}/jobs", user2) + get api(path, user2) expect(response).to have_gitlab_http_status(:forbidden) end @@ -1000,7 +1028,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'unauthorized user' do it 'does not return jobs' do - get api("/runners/#{project_runner.id}/jobs") + get api(path) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -1028,7 +1056,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do describe 'GET /projects/:id/runners' do context 'authorized user with maintainer privileges' do it 'returns response status and headers' do - get api('/runners/all', admin) + get api('/runners/all', admin, admin_mode: true) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -1200,19 +1228,27 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end describe 'POST /projects/:id/runners' do + let(:path) { "/projects/#{project.id}/runners" } + + it_behaves_like 'POST request permissions for admin mode' do + let!(:new_project_runner) { create(:ci_runner, :project) } + let(:params) { { runner_id: new_project_runner.id } } + let(:failed_status_code) { :not_found } + end + context 'authorized user' do let_it_be(:project_runner2) { create(:ci_runner, :project, projects: [project2]) } it 'enables project runner' do expect do - post api("/projects/#{project.id}/runners", user), params: { runner_id: project_runner2.id } + post api(path, user), params: { runner_id: project_runner2.id } end.to change { project.runners.count }.by(+1) expect(response).to have_gitlab_http_status(:created) end it 'avoids changes when enabling already enabled runner' do expect do - post api("/projects/#{project.id}/runners", user), params: { runner_id: project_runner.id } + post api(path, user), params: { runner_id: project_runner.id } end.to change { project.runners.count }.by(0) expect(response).to have_gitlab_http_status(:bad_request) end @@ -1221,20 +1257,20 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do project_runner2.update!(locked: true) expect do - post api("/projects/#{project.id}/runners", user), params: { runner_id: project_runner2.id } + post api(path, user), params: { runner_id: project_runner2.id } end.to change { project.runners.count }.by(0) expect(response).to have_gitlab_http_status(:forbidden) end it 'does not enable shared runner' do - post api("/projects/#{project.id}/runners", user), params: { runner_id: shared_runner.id } + post api(path, user), params: { runner_id: shared_runner.id } expect(response).to have_gitlab_http_status(:forbidden) end it 'does not enable group runner' do - post api("/projects/#{project.id}/runners", user), params: { runner_id: group_runner_a.id } + post api(path, user), params: { runner_id: group_runner_a.id } expect(response).to have_gitlab_http_status(:forbidden) end @@ -1245,7 +1281,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do it 'enables any project runner' do expect do - post api("/projects/#{project.id}/runners", admin), params: { runner_id: new_project_runner.id } + post api(path, admin, admin_mode: true), params: { runner_id: new_project_runner.id } end.to change { project.runners.count }.by(+1) expect(response).to have_gitlab_http_status(:created) end @@ -1257,7 +1293,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do it 'does not enable project runner' do expect do - post api("/projects/#{project.id}/runners", admin), params: { runner_id: new_project_runner.id } + post api(path, admin, admin_mode: true), params: { runner_id: new_project_runner.id } end.not_to change { project.runners.count } expect(response).to have_gitlab_http_status(:bad_request) end @@ -1266,7 +1302,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do end it 'raises an error when no runner_id param is provided' do - post api("/projects/#{project.id}/runners", admin) + post api(path, admin, admin_mode: true) expect(response).to have_gitlab_http_status(:bad_request) end @@ -1276,7 +1312,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do let!(:new_project_runner) { create(:ci_runner, :project) } it 'does not enable runner without access to' do - post api("/projects/#{project.id}/runners", user), params: { runner_id: new_project_runner.id } + post api(path, user), params: { runner_id: new_project_runner.id } expect(response).to have_gitlab_http_status(:forbidden) end @@ -1284,7 +1320,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'authorized user without permissions' do it 'does not enable runner' do - post api("/projects/#{project.id}/runners", user2) + post api(path, user2) expect(response).to have_gitlab_http_status(:forbidden) end @@ -1292,7 +1328,7 @@ RSpec.describe API::Ci::Runners, feature_category: :runner_fleet do context 'unauthorized user' do it 'does not enable runner' do - post api("/projects/#{project.id}/runners") + post api(path) expect(response).to have_gitlab_http_status(:unauthorized) end diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index fc988800b56..db12576154e 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -136,7 +136,7 @@ RSpec.describe API::Ci::SecureFiles, feature_category: :mobile_devops do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(secure_file_with_metadata.name) - expect(json_response['expires_at']).to eq('2022-04-26T19:20:40.000Z') + expect(json_response['expires_at']).to eq('2023-04-26T19:20:39.000Z') expect(json_response['metadata'].keys).to match_array(%w[id issuer subject expires_at]) expect(json_response['file_extension']).to eq('cer') end diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index 0f9f1bc80d6..e937c4c2b8f 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Ci::Variables, feature_category: :pipeline_authoring do +RSpec.describe API::Ci::Variables, feature_category: :secrets_management do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } |