From c9a1a1552a11a9ff862bf08df7267c553f2e0e99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sat, 13 Jan 2018 22:29:29 +0100 Subject: Fix N+1 in v3 builds API The N+1 issue was caused by loading the job_artifacts_archive for each job (build) individually. Including that in the builds AssociationRelation fixed the issue. --- lib/api/v3/builds.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index fa0bef39602..a256a5720e1 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -35,8 +35,7 @@ module API get ':id/builds' do builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) - - present paginate(builds), with: ::API::V3::Entities::Build + present paginate(builds.includes(:job_artifacts_archive)), with: ::API::V3::Entities::Build end desc 'Get builds for a specific commit of a project' do -- cgit v1.2.1 From c9840842f181904e7a94946066bb5ca98c1f657a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 14 Jan 2018 23:10:51 +0100 Subject: Eager load user, runner, pipeline and its creator --- lib/api/v3/builds.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index a256a5720e1..7bf7329625c 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -35,7 +35,9 @@ module API get ':id/builds' do builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) - present paginate(builds.includes(:job_artifacts_archive)), with: ::API::V3::Entities::Build + + builds = builds.includes(:user, :job_artifacts_archive, :runner, pipeline: :project) + present paginate(builds), with: ::API::V3::Entities::Build end desc 'Get builds for a specific commit of a project' do -- cgit v1.2.1 From 87cc03736ccaa91db51e171883ef2e2a05a74360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Sun, 14 Jan 2018 23:13:18 +0100 Subject: Fix N+1 builds query in Jobs list API --- lib/api/jobs.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/api') diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index a116ab3c9bd..57bad74c105 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -38,6 +38,7 @@ module API builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) + builds = builds.includes(:user, :job_artifacts_archive, :runner, pipeline: :project) present paginate(builds), with: Entities::Job end -- cgit v1.2.1 From b2dcb553160613220734bfd748cba40fa3c07605 Mon Sep 17 00:00:00 2001 From: Serdar Dogruyol Date: Mon, 15 Jan 2018 14:03:39 +0300 Subject: Fix duplication in API::CircuitBreakers --- lib/api/circuit_breakers.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/circuit_breakers.rb b/lib/api/circuit_breakers.rb index 598c76f6168..c13154dc0ec 100644 --- a/lib/api/circuit_breakers.rb +++ b/lib/api/circuit_breakers.rb @@ -17,11 +17,11 @@ module API end def storage_health - @failing_storage_health ||= Gitlab::Git::Storage::Health.for_all_storages + @storage_health ||= Gitlab::Git::Storage::Health.for_all_storages end end - desc 'Get all failing git storages' do + desc 'Get all git storages' do detail 'This feature was introduced in GitLab 9.5' success Entities::RepositoryStorageHealth end -- cgit v1.2.1 From 1b1cc6fb14be2d8c8d24ddd446030d2470bf9527 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 15 Jan 2018 14:24:16 +0200 Subject: [API] Fix creating issue when assignee_id is empty see https://gitlab.com/gitlab-org/gitlab-ce/issues/42025 --- lib/api/helpers/common_helpers.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/helpers/common_helpers.rb b/lib/api/helpers/common_helpers.rb index 322624c6092..9993caa5249 100644 --- a/lib/api/helpers/common_helpers.rb +++ b/lib/api/helpers/common_helpers.rb @@ -3,8 +3,10 @@ module API module CommonHelpers def convert_parameters_from_legacy_format(params) params.tap do |params| - if params[:assignee_id].present? - params[:assignee_ids] = [params.delete(:assignee_id)] + assignee_id = params.delete(:assignee_id) + + if assignee_id.present? + params[:assignee_ids] = [assignee_id] end end end -- cgit v1.2.1 From feb3449709ce4fce62227f67233bc4d061c66ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 15 Jan 2018 22:04:08 +0100 Subject: Use preload instead of includes to avoid joins --- lib/api/jobs.rb | 2 +- lib/api/v3/builds.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/api') diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb index 57bad74c105..9c205514b3a 100644 --- a/lib/api/jobs.rb +++ b/lib/api/jobs.rb @@ -38,7 +38,7 @@ module API builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) - builds = builds.includes(:user, :job_artifacts_archive, :runner, pipeline: :project) + builds = builds.preload(:user, :job_artifacts_archive, :runner, pipeline: :project) present paginate(builds), with: Entities::Job end diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb index 7bf7329625c..ac76fece931 100644 --- a/lib/api/v3/builds.rb +++ b/lib/api/v3/builds.rb @@ -36,7 +36,7 @@ module API builds = user_project.builds.order('id DESC') builds = filter_builds(builds, params[:scope]) - builds = builds.includes(:user, :job_artifacts_archive, :runner, pipeline: :project) + builds = builds.preload(:user, :job_artifacts_archive, :runner, pipeline: :project) present paginate(builds), with: ::API::V3::Entities::Build end -- cgit v1.2.1 From fa84b98796bb63eea42a8685e4a129433efd353f Mon Sep 17 00:00:00 2001 From: Jacopo Date: Mon, 15 Jan 2018 23:03:43 +0100 Subject: Enables Project Milestone Deletion via API Enables project milestone deletion via DELETE /projects/:id/milestones/:milestone_id --- lib/api/project_milestones.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'lib/api') diff --git a/lib/api/project_milestones.rb b/lib/api/project_milestones.rb index 0cb209a02d0..306dc0e63d7 100644 --- a/lib/api/project_milestones.rb +++ b/lib/api/project_milestones.rb @@ -60,6 +60,15 @@ module API update_milestone_for(user_project) end + desc 'Remove a project milestone' + delete ":id/milestones/:milestone_id" do + authorize! :admin_milestone, user_project + + user_project.milestones.find(params[:milestone_id]).destroy + + status(204) + end + desc 'Get all issues for a single project milestone' do success Entities::IssueBasic end -- cgit v1.2.1 From 0424801ec8854167d17c76b68e6ae8c5b5a6a52a Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sat, 6 Jan 2018 06:18:13 +0000 Subject: Merge branch 'security-10-3-do-not-expose-passwords-or-tokens-in-service-integrations-api' into 'security-10-3' Filter out sensitive fields from the project services API See merge request gitlab/gitlabhq!2281 (cherry picked from commit 476f2576444632f2a9a61b4cead9c1077f2c81d7) 2bcbbda0 Filter out sensitive fields from the project services API --- lib/api/entities.rb | 5 +---- lib/api/services.rb | 2 +- lib/api/v3/entities.rb | 5 +---- lib/api/v3/services.rb | 2 +- 4 files changed, 4 insertions(+), 10 deletions(-) (limited to 'lib/api') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c4ef2c74658..9618d6625bb 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -714,10 +714,7 @@ module API expose :job_events # Expose serialized properties expose :properties do |service, options| - field_names = service.fields - .select { |field| options[:include_passwords] || field[:type] != 'password' } - .map { |field| field[:name] } - service.properties.slice(*field_names) + service.properties.slice(*service.api_field_names) end end diff --git a/lib/api/services.rb b/lib/api/services.rb index a7f44e2869c..51e33e2c686 100644 --- a/lib/api/services.rb +++ b/lib/api/services.rb @@ -785,7 +785,7 @@ module API service_params = declared_params(include_missing: false).merge(active: true) if service.update_attributes(service_params) - present service, with: Entities::ProjectService, include_passwords: current_user.admin? + present service, with: Entities::ProjectService else render_api_error!('400 Bad Request', 400) end diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 64758dae7d3..2ccbb9da1c5 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -257,10 +257,7 @@ module API expose :job_events, as: :build_events # Expose serialized properties expose :properties do |service, options| - field_names = service.fields - .select { |field| options[:include_passwords] || field[:type] != 'password' } - .map { |field| field[:name] } - service.properties.slice(*field_names) + service.properties.slice(*service.api_field_names) end end diff --git a/lib/api/v3/services.rb b/lib/api/v3/services.rb index 44ed94d2869..20ca1021c71 100644 --- a/lib/api/v3/services.rb +++ b/lib/api/v3/services.rb @@ -622,7 +622,7 @@ module API end get ":id/services/:service_slug" do service = user_project.find_or_initialize_service(params[:service_slug].underscore) - present service, with: Entities::ProjectService, include_passwords: current_user.admin? + present service, with: Entities::ProjectService end end -- cgit v1.2.1 From 536a47b4b70df0f2a8438ed0ada7654593fa5cd0 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 5 Jan 2018 15:23:44 +0000 Subject: Merge branch 'sh-migrate-can-push-to-deploy-keys-projects-10-3' into 'security-10-3' [10.3] Migrate `can_push` column from `keys` to `deploy_keys_project` See merge request gitlab/gitlabhq!2276 (cherry picked from commit f6ca52d31bac350a23938e0aebf717c767b4710c) 1f2bd3c0 Backport to 10.3 --- lib/api/deploy_keys.rb | 64 +++++++++++++++++++++++++++++++++-------------- lib/api/entities.rb | 7 +++++- lib/api/v3/deploy_keys.rb | 46 ++++++++++++++++++++++++---------- 3 files changed, 84 insertions(+), 33 deletions(-) (limited to 'lib/api') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 281269b1190..a92f17937b8 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -4,6 +4,16 @@ module API before { authenticate! } + helpers do + def add_deploy_keys_project(project, attrs = {}) + project.deploy_keys_projects.create(attrs) + end + + def find_by_deploy_key(project, key_id) + project.deploy_keys_projects.find_by!(deploy_key: key_id) + end + end + desc 'Return all deploy keys' params do use :pagination @@ -21,28 +31,31 @@ module API before { authorize_admin_project } desc "Get a specific project's deploy keys" do - success Entities::SSHKey + success Entities::DeployKeysProject end params do use :pagination end get ":id/deploy_keys" do - present paginate(user_project.deploy_keys), with: Entities::SSHKey + keys = user_project.deploy_keys_projects.preload(:deploy_key) + + present paginate(keys), with: Entities::DeployKeysProject end desc 'Get single deploy key' do - success Entities::SSHKey + success Entities::DeployKeysProject end params do requires :key_id, type: Integer, desc: 'The ID of the deploy key' end get ":id/deploy_keys/:key_id" do - key = user_project.deploy_keys.find params[:key_id] - present key, with: Entities::SSHKey + key = find_by_deploy_key(user_project, params[:key_id]) + + present key, with: Entities::DeployKeysProject end desc 'Add new deploy key to currently authenticated user' do - success Entities::SSHKey + success Entities::DeployKeysProject end params do requires :key, type: String, desc: 'The new deploy key' @@ -53,24 +66,31 @@ module API params[:key].strip! # Check for an existing key joined to this project - key = user_project.deploy_keys.find_by(key: params[:key]) + key = user_project.deploy_keys_projects + .joins(:deploy_key) + .find_by(keys: { key: params[:key] }) + if key - present key, with: Entities::SSHKey + present key, with: Entities::DeployKeysProject break end # Check for available deploy keys in other projects key = current_user.accessible_deploy_keys.find_by(key: params[:key]) if key - user_project.deploy_keys << key - present key, with: Entities::SSHKey + added_key = add_deploy_keys_project(user_project, deploy_key: key, can_push: !!params[:can_push]) + + present added_key, with: Entities::DeployKeysProject break end # Create a new deploy key - key = DeployKey.new(declared_params(include_missing: false)) - if key.valid? && user_project.deploy_keys << key - present key, with: Entities::SSHKey + key_attributes = { can_push: !!params[:can_push], + deploy_key_attributes: declared_params.except(:can_push) } + key = add_deploy_keys_project(user_project, key_attributes) + + if key.valid? + present key, with: Entities::DeployKeysProject else render_validation_error!(key) end @@ -86,14 +106,20 @@ module API at_least_one_of :title, :can_push end put ":id/deploy_keys/:key_id" do - key = DeployKey.find(params.delete(:key_id)) + deploy_keys_project = find_by_deploy_key(user_project, params[:key_id]) - authorize!(:update_deploy_key, key) + authorize!(:update_deploy_key, deploy_keys_project.deploy_key) - if key.update_attributes(declared_params(include_missing: false)) - present key, with: Entities::SSHKey + can_push = params[:can_push].nil? ? deploy_keys_project.can_push : params[:can_push] + title = params[:title] || deploy_keys_project.deploy_key.title + + result = deploy_keys_project.update_attributes(can_push: can_push, + deploy_key_attributes: { id: params[:key_id], + title: title }) + if result + present deploy_keys_project, with: Entities::DeployKeysProject else - render_validation_error!(key) + render_validation_error!(deploy_keys_project) end end @@ -122,7 +148,7 @@ module API requires :key_id, type: Integer, desc: 'The ID of the deploy key' end delete ":id/deploy_keys/:key_id" do - key = user_project.deploy_keys_projects.find_by(deploy_key_id: params[:key_id]) + key = user_project.deploy_keys.find(params[:key_id]) not_found!('Deploy Key') unless key destroy_conditionally!(key) diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 9618d6625bb..971cb83a2d9 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -554,13 +554,18 @@ module API end class SSHKey < Grape::Entity - expose :id, :title, :key, :created_at, :can_push + expose :id, :title, :key, :created_at end class SSHKeyWithUser < SSHKey expose :user, using: Entities::UserPublic end + class DeployKeysProject < Grape::Entity + expose :deploy_key, merge: true, using: Entities::SSHKey + expose :can_push + end + class GPGKey < Grape::Entity expose :id, :key, :created_at end diff --git a/lib/api/v3/deploy_keys.rb b/lib/api/v3/deploy_keys.rb index b90e2061da3..47e54ca85a5 100644 --- a/lib/api/v3/deploy_keys.rb +++ b/lib/api/v3/deploy_keys.rb @@ -3,6 +3,16 @@ module API class DeployKeys < Grape::API before { authenticate! } + helpers do + def add_deploy_keys_project(project, attrs = {}) + project.deploy_keys_projects.create(attrs) + end + + def find_by_deploy_key(project, key_id) + project.deploy_keys_projects.find_by!(deploy_key: key_id) + end + end + get "deploy_keys" do authenticated_as_admin! @@ -18,25 +28,28 @@ module API %w(keys deploy_keys).each do |path| desc "Get a specific project's deploy keys" do - success ::API::Entities::SSHKey + success ::API::Entities::DeployKeysProject end get ":id/#{path}" do - present user_project.deploy_keys, with: ::API::Entities::SSHKey + keys = user_project.deploy_keys_projects.preload(:deploy_key) + + present keys, with: ::API::Entities::DeployKeysProject end desc 'Get single deploy key' do - success ::API::Entities::SSHKey + success ::API::Entities::DeployKeysProject end params do requires :key_id, type: Integer, desc: 'The ID of the deploy key' end get ":id/#{path}/:key_id" do - key = user_project.deploy_keys.find params[:key_id] - present key, with: ::API::Entities::SSHKey + key = find_by_deploy_key(user_project, params[:key_id]) + + present key, with: ::API::Entities::DeployKeysProject end desc 'Add new deploy key to currently authenticated user' do - success ::API::Entities::SSHKey + success ::API::Entities::DeployKeysProject end params do requires :key, type: String, desc: 'The new deploy key' @@ -47,24 +60,31 @@ module API params[:key].strip! # Check for an existing key joined to this project - key = user_project.deploy_keys.find_by(key: params[:key]) + key = user_project.deploy_keys_projects + .joins(:deploy_key) + .find_by(keys: { key: params[:key] }) + if key - present key, with: ::API::Entities::SSHKey + present key, with: ::API::Entities::DeployKeysProject break end # Check for available deploy keys in other projects key = current_user.accessible_deploy_keys.find_by(key: params[:key]) if key - user_project.deploy_keys << key - present key, with: ::API::Entities::SSHKey + added_key = add_deploy_keys_project(user_project, deploy_key: key, can_push: !!params[:can_push]) + + present added_key, with: ::API::Entities::DeployKeysProject break end # Create a new deploy key - key = DeployKey.new(declared_params(include_missing: false)) - if key.valid? && user_project.deploy_keys << key - present key, with: ::API::Entities::SSHKey + key_attributes = { can_push: !!params[:can_push], + deploy_key_attributes: declared_params.except(:can_push) } + key = add_deploy_keys_project(user_project, key_attributes) + + if key.valid? + present key, with: ::API::Entities::DeployKeysProject else render_validation_error!(key) end -- cgit v1.2.1 From 310d209b67938acdb1cde3cf5622440bf42c5bc4 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Thu, 11 Jan 2018 17:45:35 +0100 Subject: Adds sorting to deployments API Adds sorting to deployments API through the `order_by` and sort `fields`. --- lib/api/deployments.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib/api') diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb index 1efee9a1324..184fae0eb76 100644 --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -15,11 +15,13 @@ module API end params do use :pagination + optional :order_by, type: String, values: %w[id iid created_at ref], default: 'id', desc: 'Return deployments ordered by `id` or `iid` or `created_at` or `ref`' + optional :sort, type: String, values: %w[asc desc], default: 'asc', desc: 'Sort by asc (ascending) or desc (descending)' end get ':id/deployments' do authorize! :read_deployment, user_project - present paginate(user_project.deployments), with: Entities::Deployment + present paginate(user_project.deployments.order(params[:order_by] => params[:sort])), with: Entities::Deployment end desc 'Gets a specific deployment' do -- cgit v1.2.1 From d1eb3ff594b42d6e9625724119f52d3356045870 Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Wed, 17 Jan 2018 11:42:32 -0200 Subject: Make ruby lint happy --- lib/api/deploy_keys.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'lib/api') diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index a92f17937b8..b0b7b50998f 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -116,6 +116,7 @@ module API result = deploy_keys_project.update_attributes(can_push: can_push, deploy_key_attributes: { id: params[:key_id], title: title }) + if result present deploy_keys_project, with: Entities::DeployKeysProject else -- cgit v1.2.1