summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/ci/variable.rb6
-rw-r--r--lib/api/variables.rb41
-rw-r--r--spec/requests/api/variables_spec.rb42
3 files changed, 36 insertions, 53 deletions
diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 0e2712086ca..1d9ee91a31c 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -18,8 +18,12 @@ module Ci
belongs_to :project, class_name: '::Project', foreign_key: :gl_project_id
- validates_presence_of :key
validates_uniqueness_of :key, scope: :gl_project_id
+ validates :key,
+ presence: true,
+ length: { within: 0..255 },
+ format: { with: /\A[a-zA-Z0-9_]+\z/,
+ message: "can contain only letters, digits and '_'." }
attr_encrypted :value, mode: :per_attribute_iv_and_salt, key: Gitlab::Application.secrets.db_key_base
end
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index b8bbcb6ce3b..cc038e5731d 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -22,19 +22,12 @@ module API
#
# Parameters:
# id (required) - The ID of a project
- # variable_id (required) - The ID OR `key` of variable to show; if variable_id contains only digits it's treated
- # as ID other ways it's treated as `key`
+ # key (required) - The `key` of variable
# Example Request:
- # GET /projects/:id/variables/:variable_id
- get ':id/variables/:variable_id' do
- variable_id = params[:variable_id]
- variables = user_project.variables
- variables =
- if variable_id.match(/^\d+$/)
- variables.where(id: variable_id.to_i)
- else
- variables.where(key: variable_id)
- end
+ # GET /projects/:id/variables/:key
+ get ':id/variables/:key' do
+ key = params[:key]
+ variables = user_project.variables.where(key: key)
return not_found!('Variable') if variables.empty?
@@ -45,8 +38,8 @@ module API
#
# Parameters:
# id (required) - The ID of a project
- # key (required) - The key of variable being created
- # value (required) - The value of variable being created
+ # key (required) - The key of variable
+ # value (required) - The value of variable
# Example Request:
# POST /projects/:id/variables
post ':id/variables' do
@@ -63,17 +56,15 @@ module API
#
# Parameters:
# id (required) - The ID of a project
- # variable_id (required) - The ID of a variable
- # key (optional) - new value for `key` field of variable
- # value (optional) - new value for `value` field of variable
+ # key (optional) - The `key` of variable
+ # value (optional) - New value for `value` field of variable
# Example Request:
- # PUT /projects/:id/variables/:variable_id
- put ':id/variables/:variable_id' do
- variable = user_project.variables.where(id: params[:variable_id].to_i).first
+ # PUT /projects/:id/variables/:key
+ put ':id/variables/:key' do
+ variable = user_project.variables.where(key: params[:key]).first
return not_found!('Variable') unless variable
- variable.key = params[:key]
variable.value = params[:value]
variable.save!
@@ -84,11 +75,11 @@ module API
#
# Parameters:
# id (required) - The ID of a project
- # variable_id (required) - The ID of a variable
+ # key (required) - The ID of a variable
# Exanoke Reqyest:
- # DELETE /projects/:id/variables/:variable_id
- delete ':id/variables/:variable_id' do
- variable = user_project.variables.where(id: params[:variable_id].to_i).first
+ # DELETE /projects/:id/variables/:key
+ delete ':id/variables/:key' do
+ variable = user_project.variables.where(key: params[:key]).first
return not_found!('Variable') unless variable
diff --git a/spec/requests/api/variables_spec.rb b/spec/requests/api/variables_spec.rb
index bf0dd77473a..214d7d5a0cc 100644
--- a/spec/requests/api/variables_spec.rb
+++ b/spec/requests/api/variables_spec.rb
@@ -37,26 +37,17 @@ describe API::API, api: true do
end
end
- describe 'GET /projects/:id/variables/:variable_id' do
+ describe 'GET /projects/:id/variables/:key' do
context 'authorized user with proper permissions' do
- it 'should return project variable details when ID is used as :variable_id' do
- get api("/projects/#{project.id}/variables/#{variable.id}", user)
-
- expect(response.status).to eq(200)
- expect(json_response['key']).to eq(variable.key)
- expect(json_response['value']).to eq(variable.value)
- end
-
- it 'should return project variable details when `key` is used as :variable_id' do
+ it 'should return project variable details' do
get api("/projects/#{project.id}/variables/#{variable.key}", user)
expect(response.status).to eq(200)
- expect(json_response['id']).to eq(variable.id)
expect(json_response['value']).to eq(variable.value)
end
it 'should responde with 404 Not Found if requesting non-existing variable' do
- get api("/projects/#{project.id}/variables/9999", user)
+ get api("/projects/#{project.id}/variables/non_existing_variable", user)
expect(response.status).to eq(404)
end
@@ -64,7 +55,7 @@ describe API::API, api: true do
context 'authorized user with invalid permissions' do
it 'should not return project variable details' do
- get api("/projects/#{project.id}/variables/#{variable.id}", user2)
+ get api("/projects/#{project.id}/variables/#{variable.key}", user2)
expect(response.status).to eq(403)
end
@@ -72,7 +63,7 @@ describe API::API, api: true do
context 'unauthorized user' do
it 'should not return project variable details' do
- get api("/projects/#{project.id}/variables/#{variable.id}")
+ get api("/projects/#{project.id}/variables/#{variable.key}")
expect(response.status).to eq(401)
end
@@ -117,26 +108,23 @@ describe API::API, api: true do
end
end
- describe 'PUT /projects/:id/variables/:variable_id' do
+ describe 'PUT /projects/:id/variables/:key' do
context 'authorized user with proper permissions' do
it 'should update variable data' do
initial_variable = project.variables.first
- key_before = initial_variable.key
value_before = initial_variable.value
- put api("/projects/#{project.id}/variables/#{variable.id}", user), key: 'TEST_VARIABLE_1_UP', value: 'VALUE_1_UP'
+ put api("/projects/#{project.id}/variables/#{variable.key}", user), value: 'VALUE_1_UP'
updated_variable = project.variables.first
expect(response.status).to eq(200)
- expect(key_before).to eq(variable.key)
expect(value_before).to eq(variable.value)
- expect(updated_variable.key).to eq('TEST_VARIABLE_1_UP')
expect(updated_variable.value).to eq('VALUE_1_UP')
end
it 'should responde with 404 Not Found if requesting non-existing variable' do
- put api("/projects/#{project.id}/variables/9999", user)
+ put api("/projects/#{project.id}/variables/non_existing_variable", user)
expect(response.status).to eq(404)
end
@@ -144,7 +132,7 @@ describe API::API, api: true do
context 'authorized user with invalid permissions' do
it 'should not update variable' do
- put api("/projects/#{project.id}/variables/#{variable.id}", user2)
+ put api("/projects/#{project.id}/variables/#{variable.key}", user2)
expect(response.status).to eq(403)
end
@@ -152,24 +140,24 @@ describe API::API, api: true do
context 'unauthorized user' do
it 'should not update variable' do
- put api("/projects/#{project.id}/variables/#{variable.id}")
+ put api("/projects/#{project.id}/variables/#{variable.key}")
expect(response.status).to eq(401)
end
end
end
- describe 'DELETE /projects/:id/variables/:variable_id' do
+ describe 'DELETE /projects/:id/variables/:key' do
context 'authorized user with proper permissions' do
it 'should delete variable' do
expect do
- delete api("/projects/#{project.id}/variables/#{variable.id}", user)
+ delete api("/projects/#{project.id}/variables/#{variable.key}", user)
end.to change{project.variables.count}.by(-1)
expect(response.status).to eq(200)
end
it 'should responde with 404 Not Found if requesting non-existing variable' do
- delete api("/projects/#{project.id}/variables/9999", user)
+ delete api("/projects/#{project.id}/variables/non_existing_variable", user)
expect(response.status).to eq(404)
end
@@ -177,7 +165,7 @@ describe API::API, api: true do
context 'authorized user with invalid permissions' do
it 'should not delete variable' do
- delete api("/projects/#{project.id}/variables/#{variable.id}", user2)
+ delete api("/projects/#{project.id}/variables/#{variable.key}", user2)
expect(response.status).to eq(403)
end
@@ -185,7 +173,7 @@ describe API::API, api: true do
context 'unauthorized user' do
it 'should not delete variable' do
- delete api("/projects/#{project.id}/variables/#{variable.id}")
+ delete api("/projects/#{project.id}/variables/#{variable.key}")
expect(response.status).to eq(401)
end
View Lorry Controller Status