Merge branch '34519-extend-api-group-secret-variable' into 'master'

changelogs/unreleased/34519-extend-api-group-secret-variable.yml

+---
+title: Extend API for Group Secret Variable
+merge_request: 12936
+author:

doc/api/README.md

@@ -11,7 +11,8 @@ following locations:
 - [Award Emoji](award_emoji.md)
 - [Branches](branches.md)
 - [Broadcast Messages](broadcast_messages.md)
-- [Build Variables](build_variables.md)
+- [Project-level Variables](project_level_variables.md)
+- [Group-level Variables](group_level_variables.md)
 - [Commits](commits.md)
 - [Deployments](deployments.md)
 - [Deploy Keys](deploy_keys.md)

doc/api/group_level_variables.md

+# Group-level Variables  API
+
+## List group variables
+
+Get list of a group's variables.
+
+```
+GET /groups/:id/variables
+```
+
+| Attribute | Type    | required | Description         |
+|-----------|---------|----------|---------------------|
+| `id`      | integer/string | yes      | The ID of a group or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user |
+
+```
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/groups/1/variables"
+```
+
+```json
+[
+    {
+        "key": "TEST_VARIABLE_1",
+        "value": "TEST_1"
+    },
+    {
+        "key": "TEST_VARIABLE_2",
+        "value": "TEST_2"
+    }
+]
+```
+
+## Show variable details
+
+Get the details of a group's specific variable.
+
+```
+GET /groups/:id/variables/:key
+```
+
+| Attribute | Type    | required | Description           |
+|-----------|---------|----------|-----------------------|
+| `id`      | integer/string | yes      | The ID of a group or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user   |
+| `key`     | string  | yes      | The `key` of a variable |
+
+```
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/groups/1/variables/TEST_VARIABLE_1"
+```
+
+```json
+{
+    "key": "TEST_VARIABLE_1",
+    "value": "TEST_1"
+}
+```
+
+## Create variable
+
+Create a new variable.
+
+```
+POST /groups/:id/variables
+```
+
+| Attribute   | Type    | required | Description           |
+|-------------|---------|----------|-----------------------|
+| `id`        | integer/string | yes      | The ID of a group or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user   |
+| `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 |
+| `protected` | boolean | no       | Whether the variable is protected |
+
+```
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/groups/1/variables" --form "key=NEW_VARIABLE" --form "value=new value"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "new value",
+    "protected": false
+}
+```
+
+## Update variable
+
+Update a group's variable.
+
+```
+PUT /groups/:id/variables/:key
+```
+
+| Attribute   | Type    | required | Description             |
+|-------------|---------|----------|-------------------------|
+| `id`        | integer/string | yes      | The ID of a group or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user     |
+| `key`       | string  | yes      | The `key` of a variable   |
+| `value`     | string  | yes      | The `value` of a variable |
+| `protected` | boolean | no       | Whether the variable is protected |
+
+```
+curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/groups/1/variables/NEW_VARIABLE" --form "value=updated value"
+```
+
+```json
+{
+    "key": "NEW_VARIABLE",
+    "value": "updated value",
+    "protected": true
+}
+```
+
+## Remove variable
+
+Remove a group's variable.
+
+```
+DELETE /groups/:id/variables/:key
+```
+
+| Attribute | Type    | required | Description             |
+|-----------|---------|----------|-------------------------|
+| `id`      | integer/string | yes      | The ID of a group or [URL-encoded path of the group](README.md#namespaced-path-encoding) owned by the authenticated user     |
+| `key`     | string  | yes      | The `key` of a variable |
+
+```
+curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/groups/1/variables/VARIABLE_1"
+```

doc/api/build_variables.md → doc/api/project_level_variables.md

-# Build Variables  API
+# Project-level Variables  API
 
 ## List project variables
 
-Get list of a project's build variables.
+Get list of a project's variables.
 
 ```
 GET /projects/:id/variables
@@ -31,7 +31,7 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/
 
 ## Show variable details
 
-Get the details of a project's specific build variable.
+Get the details of a project's specific variable.
 
 ```
 GET /projects/:id/variables/:key
@@ -55,7 +55,7 @@ curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/
 
 ## Create variable
 
-Create a new build variable.
+Create a new variable.
 
 ```
 POST /projects/:id/variables
@@ -82,7 +82,7 @@ curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitl
 
 ## Update variable
 
-Update a project's build variable.
+Update a project's variable.
 
 ```
 PUT /projects/:id/variables/:key
@@ -109,7 +109,7 @@ curl --request PUT --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitla
 
 ## Remove variable
 
-Remove a project's build variable.
+Remove a project's variable.
 
 ```
 DELETE /projects/:id/variables/:key

lib/api/api.rb

@@ -139,6 +139,7 @@ module API
     mount ::API::Triggers
     mount ::API::Users
     mount ::API::Variables
+    mount ::API::GroupVariables
     mount ::API::Version
 
     route :any, '*path' do

lib/api/group_variables.rb

+module API
+  class GroupVariables < Grape::API
+    include PaginationParams
+
+    before { authenticate! }
+    before { authorize! :admin_build, user_group }
+
+    params do
+      requires :id, type: String, desc: 'The ID of a group'
+    end
+
+    resource :groups, requirements: { id: %r{[^/]+} } do
+      desc 'Get group-level variables' do
+        success Entities::Variable
+      end
+      params do
+        use :pagination
+      end
+      get ':id/variables' do
+        variables = user_group.variables
+        present paginate(variables), with: Entities::Variable
+      end
+
+      desc 'Get a specific variable from a group' do
+        success Entities::Variable
+      end
+      params do
+        requires :key, type: String, desc: 'The key of the variable'
+      end
+      get ':id/variables/:key' do
+        key = params[:key]
+        variable = user_group.variables.find_by(key: key)
+
+        return not_found!('GroupVariable') unless variable
+
+        present variable, with: Entities::Variable
+      end
+
+      desc 'Create a new variable in a group' do
+        success Entities::Variable
+      end
+      params do
+        requires :key, type: String, desc: 'The key of the variable'
+        requires :value, type: String, desc: 'The value of the variable'
+        optional :protected, type: String, desc: 'Whether the variable is protected'
+      end
+      post ':id/variables' do
+        variable_params = declared_params(include_missing: false)
+
+        variable = user_group.variables.create(variable_params)
+
+        if variable.valid?
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
+      end
+
+      desc 'Update an existing variable from a group' do
+        success Entities::Variable
+      end
+      params do
+        optional :key, type: String, desc: 'The key of the variable'
+        optional :value, type: String, desc: 'The value of the variable'
+        optional :protected, type: String, desc: 'Whether the variable is protected'
+      end
+      put ':id/variables/:key' do
+        variable = user_group.variables.find_by(key: params[:key])
+
+        return not_found!('GroupVariable') unless variable
+
+        variable_params = declared_params(include_missing: false).except(:key)
+
+        if variable.update(variable_params)
+          present variable, with: Entities::Variable
+        else
+          render_validation_error!(variable)
+        end
+      end
+
+      desc 'Delete an existing variable from a group' do
+        success Entities::Variable
+      end
+      params do
+        requires :key, type: String, desc: 'The key of the variable'
+      end
+      delete ':id/variables/:key' do
+        variable = user_group.variables.find_by(key: params[:key])
+        not_found!('GroupVariable') unless variable
+
+        variable.destroy
+      end
+    end
+  end
+end

lib/api/helpers.rb

@@ -33,6 +33,10 @@ module API
       @project ||= find_project!(params[:id])
     end
 
+    def user_group
+      @group ||= find_group!(params[:id])
+    end
+
     def available_labels
       @available_labels ||= LabelsFinder.new(current_user, project_id: user_project.id).execute
     end

spec/requests/api/group_variables_spec.rb

+require 'spec_helper'
+
+describe API::GroupVariables do
+  let(:group) { create(:group) }
+  let(:user) { create(:user) }
+
+  describe 'GET /groups/:id/variables' do
+    let!(:variable) { create(:ci_group_variable, group: group) }
+
+    context 'authorized user with proper permissions' do
+      before do
+        group.add_master(user)
+      end
+
+      it 'returns group variables' do
+        get api("/groups/#{group.id}/variables", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to be_a(Array)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not return group variables' do
+        get api("/groups/#{group.id}/variables", user)
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not return group variables' do
+        get api("/groups/#{group.id}/variables")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+  end
+
+  describe 'GET /groups/:id/variables/:key' do
+    let!(:variable) { create(:ci_group_variable, group: group) }
+
+    context 'authorized user with proper permissions' do
+      before do
+        group.add_master(user)
+      end
+
+      it 'returns group variable details' do
+        get api("/groups/#{group.id}/variables/#{variable.key}", user)
+
+        expect(response).to have_http_status(200)
+        expect(json_response['value']).to eq(variable.value)
+        expect(json_response['protected']).to eq(variable.protected?)
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing variable' do
+        get api("/groups/#{group.id}/variables/non_existing_variable", user)
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not return group variable details' do
+        get api("/groups/#{group.id}/variables/#{variable.key}", user)
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not return group variable details' do
+        get api("/groups/#{group.id}/variables/#{variable.key}")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+  end
+
+  describe 'POST /groups/:id/variables' do
+    context 'authorized user with proper permissions' do
+      let!(:variable) { create(:ci_group_variable, group: group) }
+
+      before do
+        group.add_master(user)
+      end
+
+      it 'creates variable' do
+        expect do
+          post api("/groups/#{group.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2', protected: true
+        end.to change{group.variables.count}.by(1)
+
+        expect(response).to have_http_status(201)
+        expect(json_response['key']).to eq('TEST_VARIABLE_2')
+        expect(json_response['value']).to eq('VALUE_2')
+        expect(json_response['protected']).to be_truthy
+      end
+
+      it 'creates variable with optional attributes' do
+        expect do
+          post api("/groups/#{group.id}/variables", user), key: 'TEST_VARIABLE_2', value: 'VALUE_2'
+        end.to change{group.variables.count}.by(1)
+
+        expect(response).to have_http_status(201)
+        expect(json_response['key']).to eq('TEST_VARIABLE_2')
+        expect(json_response['value']).to eq('VALUE_2')
+        expect(json_response['protected']).to be_falsey
+      end
+
+      it 'does not allow to duplicate variable key' do
+        expect do
+          post api("/groups/#{group.id}/variables", user), key: variable.key, value: 'VALUE_2'
+        end.to change{group.variables.count}.by(0)
+
+        expect(response).to have_http_status(400)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not create variable' do
+        post api("/groups/#{group.id}/variables", user)
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not create variable' do
+        post api("/groups/#{group.id}/variables")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+  end
+
+  describe 'PUT /groups/:id/variables/:key' do
+    let!(:variable) { create(:ci_group_variable, group: group) }
+
+    context 'authorized user with proper permissions' do
+      before do
+        group.add_master(user)
+      end
+
+      it 'updates variable data' do
+        initial_variable = group.variables.first
+        value_before = initial_variable.value
+
+        put api("/groups/#{group.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP', protected: true
+
+        updated_variable = group.variables.first
+
+        expect(response).to have_http_status(200)
+        expect(value_before).to eq(variable.value)
+        expect(updated_variable.value).to eq('VALUE_1_UP')
+        expect(updated_variable).to be_protected
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing variable' do
+        put api("/groups/#{group.id}/variables/non_existing_variable", user)
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not update variable' do
+        put api("/groups/#{group.id}/variables/#{variable.key}", user)
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not update variable' do
+        put api("/groups/#{group.id}/variables/#{variable.key}")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+  end
+
+  describe 'DELETE /groups/:id/variables/:key' do
+    let!(:variable) { create(:ci_group_variable, group: group) }
+
+    context 'authorized user with proper permissions' do
+      before do
+        group.add_master(user)
+      end
+
+      it 'deletes variable' do
+        expect do
+          delete api("/groups/#{group.id}/variables/#{variable.key}", user)
+
+          expect(response).to have_http_status(204)
+        end.to change{group.variables.count}.by(-1)
+      end
+
+      it 'responds with 404 Not Found if requesting non-existing variable' do
+        delete api("/groups/#{group.id}/variables/non_existing_variable", user)
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'authorized user with invalid permissions' do
+      it 'does not delete variable' do
+        delete api("/groups/#{group.id}/variables/#{variable.key}", user)
+
+        expect(response).to have_http_status(403)
+      end
+    end
+
+    context 'unauthorized user' do
+      it 'does not delete variable' do
+        delete api("/groups/#{group.id}/variables/#{variable.key}")
+
+        expect(response).to have_http_status(401)
+      end
+    end
+  end
+end