Merge branch 'feature/sm/34518-extend-api-pipeline-schedule-variable-new' into 'master'

Extend API: Pipeline Schedule Variable

Closes #34518

See merge request !13653

changelogs/unreleased/feature-sm-34518-extend-api-pipeline-schedule-variable-new.yml

+---
+title: 'Extend API: Pipeline Schedule Variable'
+merge_request: 13653
+author:
+type: added

doc/api/pipeline_schedules.md

@@ -84,7 +84,13 @@ curl --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gitlab.example.com/
         "state": "active",
         "avatar_url": "http://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
         "web_url": "https://gitlab.example.com/root"
+    },
+    "variables": [
+        {
+            "key": "TEST_VARIABLE_1",
+            "value": "TEST_1"
         }
+    ]
 }
 ```
 
@@ -271,3 +277,86 @@ curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gi
     }
 }
 ```
+
+## Pipeline schedule variable
+
+> [Introduced][ce-34518] in GitLab 10.0.
+
+## Create a new pipeline schedule variable
+
+Create a new variable of a pipeline schedule.
+
+```
+POST /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables
+```
+
+| Attribute              | Type           | required | Description              |
+|------------------------|----------------|----------|--------------------------|
+| `id`                   | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user      |
+| `pipeline_schedule_id` | integer        | yes      | The pipeline schedule id |
+| `key`                  | string         | yes      | The `key` of a variable; must have no more than 255 characters; only `A-Z`, `a-z`, `0-9`, and `_` are allowed |
+| `value`                | string         | yes      | The `value` of a variable |
+
+```sh
+curl --request POST --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form "key=NEW_VARIABLE" --form "value=new value" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13/variables"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "new value"
+}
+```
+
+## Edit a pipeline schedule variable
+
+Updates the variable of a pipeline schedule.
+
+```
+PUT /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables/:key
+```
+
+| Attribute              | Type           | required | Description              |
+|------------------------|----------------|----------|--------------------------|
+| `id`                   | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user      |
+| `pipeline_schedule_id` | integer        | yes      | The pipeline schedule id |
+| `key`                  | string         | yes      | The `key` of a variable   |
+| `value`                | string         | yes      | The `value` of a variable |
+
+```sh
+curl --request PUT --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" --form "value=updated value" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13/variables/NEW_VARIABLE"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "updated value"
+}
+```
+
+## Delete a pipeline schedule variable
+
+Delete the variable of a pipeline schedule.
+
+```
+DELETE /projects/:id/pipeline_schedules/:pipeline_schedule_id/variables/:key
+```
+
+| Attribute              | Type           | required | Description              |
+|------------------------|----------------|----------|--------------------------|
+| `id`                   | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user      |
+| `pipeline_schedule_id` | integer        | yes      | The pipeline schedule id |
+| `key`                  | string         | yes      | The `key` of a variable |
+
+```sh
+curl --request DELETE --header "PRIVATE-TOKEN: k5ESFgWY2Qf5xEvDcFxZ" "https://gitlab.example.com/api/v4/projects/29/pipeline_schedules/13/variables/NEW_VARIABLE"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "updated value"
+}
+```
+
+[ce-34518]: https://gitlab.com/gitlab-org/gitlab-ce/issues/34518
\ No newline at end of file

lib/api/entities.rb

@@ -819,7 +819,7 @@ module API
 
     class Variable < Grape::Entity
       expose :key, :value
-      expose :protected?, as: :protected
+      expose :protected?, as: :protected, if: -> (entity, _) { entity.respond_to?(:protected?) }
     end
 
     class Pipeline < PipelineBasic
@@ -840,6 +840,7 @@ module API
 
     class PipelineScheduleDetails < PipelineSchedule
       expose :last_pipeline, using: Entities::PipelineBasic
+      expose :variables, using: Entities::Variable
     end
 
     class EnvironmentBasic < Grape::Entity

lib/api/pipeline_schedules.rb

@@ -31,10 +31,6 @@ module API
         requires :pipeline_schedule_id, type: Integer,  desc: 'The pipeline schedule id'
       end
       get ':id/pipeline_schedules/:pipeline_schedule_id' do
-        authorize! :read_pipeline_schedule, user_project
-
-        not_found!('PipelineSchedule') unless pipeline_schedule
-
         present pipeline_schedule, with: Entities::PipelineScheduleDetails
       end
 
@@ -74,9 +70,6 @@ module API
         optional :active, type: Boolean, desc: 'The activation of pipeline schedule'
       end
       put ':id/pipeline_schedules/:pipeline_schedule_id' do
-        authorize! :read_pipeline_schedule, user_project
-
-        not_found!('PipelineSchedule') unless pipeline_schedule
         authorize! :update_pipeline_schedule, pipeline_schedule
 
         if pipeline_schedule.update(declared_params(include_missing: false))
@@ -93,9 +86,6 @@ module API
         requires :pipeline_schedule_id, type: Integer,  desc: 'The pipeline schedule id'
       end
       post ':id/pipeline_schedules/:pipeline_schedule_id/take_ownership' do
-        authorize! :read_pipeline_schedule, user_project
-
-        not_found!('PipelineSchedule') unless pipeline_schedule
         authorize! :update_pipeline_schedule, pipeline_schedule
 
         if pipeline_schedule.own!(current_user)
@@ -112,21 +102,84 @@ module API
         requires :pipeline_schedule_id, type: Integer,  desc: 'The pipeline schedule id'
       end
       delete ':id/pipeline_schedules/:pipeline_schedule_id' do
-        authorize! :read_pipeline_schedule, user_project
-
-        not_found!('PipelineSchedule') unless pipeline_schedule
         authorize! :admin_pipeline_schedule, pipeline_schedule
 
         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
+      post ':id/pipeline_schedules/:pipeline_schedule_id/variables' do
+        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::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! :update_pipeline_schedule, pipeline_schedule
+
+        if pipeline_schedule_variable.update(declared_params(include_missing: false))
+          present pipeline_schedule_variable, with: Entities::Variable
+        else
+          render_validation_error!(pipeline_schedule_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! :admin_pipeline_schedule, pipeline_schedule
+
+        status :accepted
+        present pipeline_schedule_variable.destroy, with: Entities::Variable
+      end
     end
 
     helpers do
       def pipeline_schedule
         @pipeline_schedule ||=
-          user_project.pipeline_schedules
+          user_project
+            .pipeline_schedules
             .preload(:owner, :last_pipeline)
-                      .find_by(id: params.delete(:pipeline_schedule_id))
+            .find_by(id: params.delete(:pipeline_schedule_id)).tap do |pipeline_schedule|
+              unless can?(current_user, :read_pipeline_schedule, pipeline_schedule)
+                not_found!('Pipeline Schedule')
+              end
+            end
+      end
+
+      def pipeline_schedule_variable
+        @pipeline_schedule_variable ||=
+          pipeline_schedule.variables.find_by(key: params[:key]).tap do |pipeline_schedule_variable|
+            unless pipeline_schedule_variable
+              not_found!('Pipeline Schedule Variable')
+            end
+          end
       end
     end
   end

spec/fixtures/api/schemas/pipeline_schedule.json

@@ -31,6 +31,10 @@
         "web_url": { "type": "uri" }
       },
       "additionalProperties": false
+    },
+    "variables": {
+      "type": ["array", "null"],
+      "items": { "$ref": "pipeline_schedule_variable.json" }
     }
   },
   "required": [

spec/fixtures/api/schemas/pipeline_schedule_variable.json

+{
+  "type": ["object", "null"],
+  "properties": {
+    "key": { "type": "string" },
+    "value": { "type": "string" }
+  },
+  "additionalProperties": false
+}

spec/requests/api/pipeline_schedules_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe API::PipelineSchedules do
   set(:developer) { create(:user) }
   set(:user) { create(:user) }
-  set(:project) { create(:project, :repository) }
+  set(:project) { create(:project, :repository, public_builds: false) }
 
   before do
     project.add_developer(developer)
@@ -110,6 +110,18 @@ describe API::PipelineSchedules do
       end
     end
 
+    context 'authenticated user with insufficient permissions' do
+      before do
+        project.add_guest(user)
+      end
+
+      it 'does not return pipeline_schedules list' do
+        get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", user)
+
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+
     context 'unauthenticated user' do
       it 'does not return pipeline_schedules list' do
         get api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}")
@@ -299,4 +311,150 @@ 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) }
+
+    set(: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 key 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
+    set(: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 pipeline_schedule_variable' do
+        put api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}/variables/#{pipeline_schedule_variable.key}", developer),
+          value: 'updated_value'
+
+        expect(response).to have_http_status(:ok)
+        expect(response).to match_response_schema('pipeline_schedule_variable')
+        expect(json_response['value']).to eq('updated_value')
+      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) }
+
+    set(: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 { Ci::PipelineScheduleVariable.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
+      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