diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 09d7d9ad349..81cf7039260 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -819,7 +819,7 @@ module API class Variable < Grape::Entity expose :key, :value - expose :protected?, as: :protected, if: -> (entity, options) { entity.respond_to?(protected?) } + expose :protected?, as: :protected, if: -> (entity, options) { entity.respond_to?(:protected?) } end class Pipeline < PipelineBasic diff --git a/lib/api/pipeline_schedules.rb b/lib/api/pipeline_schedules.rb index e82b974c8cd..ffb9a09834d 100644 --- a/lib/api/pipeline_schedules.rb +++ b/lib/api/pipeline_schedules.rb @@ -120,75 +120,72 @@ module API destroy_conditionally!(pipeline_schedule) end + desc 'Create a new pipeline schedule variable' do + success Entities::Variable + end params do requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' + requires :key, type: String, desc: 'The key of the variable' + requires :value, type: String, desc: 'The value of the variable' end - resource :variables, requirements: { pipeline_schedule_id: %r{[^/]+} } do - desc 'Create a new pipeline schedule variable' do - success Entities::PipelineScheduleDetails - end - params do - requires :key, type: String, desc: 'The key of the variable' - requires :value, type: String, desc: 'The value of the variable' - end - post ':id/pipeline_schedules/:pipeline_schedule_id/variables' do - authorize! :read_pipeline_schedule, user_project + post ':id/pipeline_schedules/:pipeline_schedule_id/variables' do + authorize! :read_pipeline_schedule, user_project - not_found!('PipelineSchedule') unless pipeline_schedule - authorize! :update_pipeline_schedule, pipeline_schedule + not_found!('PipelineSchedule') unless pipeline_schedule + authorize! :update_pipeline_schedule, pipeline_schedule - variable_params = declared_params(include_missing: false) - variable = pipeline_schedule.variables.create(variable_params) - - if variable.persisted? - present variable, with: Entities::Variable - else - render_validation_error!(variable) - end - end - - desc 'Edit a pipeline schedule variable' do - success Entities::PipelineScheduleDetails - end - params do - optional :key, type: String, desc: 'The key of the variable' - optional :value, type: String, desc: 'The value of the variable' - end - put ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do - authorize! :read_pipeline_schedule, user_project - - not_found!('PipelineSchedule') unless pipeline_schedule - authorize! :update_pipeline_schedule, pipeline_schedule - - variable = pipeline_schedule.variables.find_by(key: params[:key]) - not_found!('Variable') unless variable - - if variable.update(declared_params(include_missing: false)) - present variable, with: Entities::Variable - else - render_validation_error!(variable) - end - end - - desc 'Delete a pipeline schedule variable' do - success Entities::PipelineScheduleDetails - end - params do - requires :key, type: String, desc: 'The key of the variable' - end - delete ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do - authorize! :read_pipeline_schedule, user_project - - not_found!('PipelineSchedule') unless pipeline_schedule - authorize! :admin_pipeline_schedule, pipeline_schedule - - variable = pipeline_schedule.variables.find_by(key: params[:key]) - not_found!('Variable') unless variable - - status :accepted + variable_params = declared_params(include_missing: false) + variable = pipeline_schedule.variables.create(variable_params) + if variable.persisted? present variable, with: Entities::Variable + else + render_validation_error!(variable) end end + + desc 'Edit a pipeline schedule variable' do + success Entities::Variable + end + params do + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' + requires :key, type: String, desc: 'The key of the variable' + optional :value, type: String, desc: 'The value of the variable' + end + put ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do + authorize! :read_pipeline_schedule, user_project + + not_found!('PipelineSchedule') unless pipeline_schedule + authorize! :update_pipeline_schedule, pipeline_schedule + + variable = pipeline_schedule.variables.find_by(key: params[:key]) + not_found!('Variable') unless variable + + if variable.update(declared_params(include_missing: false)) + present variable, with: Entities::Variable + else + render_validation_error!(variable) + end + end + + desc 'Delete a pipeline schedule variable' do + success Entities::Variable + end + params do + requires :pipeline_schedule_id, type: Integer, desc: 'The pipeline schedule id' + requires :key, type: String, desc: 'The key of the variable' + end + delete ':id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do + authorize! :read_pipeline_schedule, user_project + + not_found!('PipelineSchedule') unless pipeline_schedule + authorize! :admin_pipeline_schedule, pipeline_schedule + + variable = pipeline_schedule.variables.find_by(key: params[:key]) + not_found!('Variable') unless variable + + status :accepted + present variable, with: Entities::Variable + end end helpers do diff --git a/spec/fixtures/api/schemas/pipeline_schedule.json b/spec/fixtures/api/schemas/pipeline_schedule.json index 16bbeacddbd..c76c6945117 100644 --- a/spec/fixtures/api/schemas/pipeline_schedule.json +++ b/spec/fixtures/api/schemas/pipeline_schedule.json @@ -34,14 +34,7 @@ }, "variables": { "type": ["array", "null"], - "items": { - "type": ["object", "null"], - "properties": { - "key": { "type": "string" }, - "value": { "type": "string" } - }, - "additionalProperties": false - } + "items": { "$ref": "pipeline_schedule_variable.json" } } }, "required": [ diff --git a/spec/fixtures/api/schemas/pipeline_schedule_variable.json b/spec/fixtures/api/schemas/pipeline_schedule_variable.json new file mode 100644 index 00000000000..f7ccb2d44a0 --- /dev/null +++ b/spec/fixtures/api/schemas/pipeline_schedule_variable.json @@ -0,0 +1,8 @@ +{ + "type": ["object", "null"], + "properties": { + "key": { "type": "string" }, + "value": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index b6a5a7ffbb5..e55e3617b5a 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -299,4 +299,160 @@ describe API::PipelineSchedules do end end end + + describe 'POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables' do + let(:params) { attributes_for(:ci_pipeline_schedule_variable) } + + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + end + + context 'authenticated user with valid permissions' do + context 'with required parameters' do + it 'creates pipeline_schedule_variable' do + expect do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer), + params + end.to change { pipeline_schedule.variables.count }.by(1) + + expect(response).to have_http_status(:created) + expect(response).to match_response_schema('pipeline_schedule_variable') + expect(json_response['key']).to eq(params[:key]) + expect(json_response['value']).to eq(params[:value]) + end + end + + context 'without required parameters' do + it 'does not create pipeline_schedule_variable' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer) + + expect(response).to have_http_status(:bad_request) + end + end + + context 'when cron has validation error' do + it 'does not create pipeline_schedule_variable' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", developer), + params.merge('key' => '!?!?') + + expect(response).to have_http_status(:bad_request) + expect(json_response['message']).to have_key('key') + end + end + end + + context 'authenticated user with invalid permissions' do + it 'does not create pipeline_schedule_variable' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables", user), params + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not create pipeline_schedule_variable' do + post api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables"), params + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do + let(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + end + + let(:pipeline_schedule_variable) do + create(:ci_pipeline_schedule_variable, pipeline_schedule: pipeline_schedule) + end + + context 'authenticated user with valid permissions' do + it 'updates cron' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer), + key: pipeline_schedule_variable.key, value: 'updated_value' + + expect(response).to have_http_status(:ok) + expect(response).to match_response_schema('pipeline_schedule_variable') + expect(json_response['key']).to eq('updated_value') + end + + context 'when cron has validation error' do + it 'does not update pipeline_schedule_variable' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer), + key: '!?!?' + + expect(response).to have_http_status(:bad_request) + expect(json_response['message']).to have_key('key') + end + end + end + + context 'authenticated user with invalid permissions' do + it 'does not update pipeline_schedule_variable' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", user) + + expect(response).to have_http_status(:not_found) + end + end + + context 'unauthenticated user' do + it 'does not update pipeline_schedule_variable' do + put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}") + + expect(response).to have_http_status(:unauthorized) + end + end + end + + describe 'DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables/:key' do + let(:master) { create(:user) } + + let!(:pipeline_schedule) do + create(:ci_pipeline_schedule, project: project, owner: developer) + end + + let(:pipeline_schedule_variable) do + create(:ci_pipeline_schedule_variable, pipeline_schedule: pipeline_schedule) + end + + before do + project.add_master(master) + end + + context 'authenticated user with valid permissions' do + it 'deletes pipeline_schedule_variable' do + expect do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", master) + end.to change { project.pipeline_schedules.count }.by(-1) + + expect(response).to have_http_status(:accepted) + expect(response).to match_response_schema('pipeline_schedule_variable') + end + + it 'responds with 404 Not Found if requesting non-existing pipeline_schedule_variable' do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/____", master) + + expect(response).to have_http_status(:not_found) + end + end + + context 'authenticated user with invalid permissions' do # TODO: + let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: master) } + + it 'does not delete pipeline_schedule_variable' do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer) + + expect(response).to have_http_status(:forbidden) + end + end + + context 'unauthenticated user' do + it 'does not delete pipeline_schedule_variable' do + delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}") + + expect(response).to have_http_status(:unauthorized) + end + end + end end