diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index ef31bedc3a82510a7c2285eb4dd1323ba8f32499..f9fa4bd212c2faa8e3ab79740fc5dd6bf80eba53 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -10,15 +10,19 @@ class DeployKey < Key has_many :projects, through: :deploy_keys_projects has_many :deploy_keys_projects_with_write_access, -> { with_write_access }, class_name: "DeployKeysProject", inverse_of: :deploy_key + has_many :deploy_keys_projects_with_readonly_access, -> { with_readonly_access }, class_name: "DeployKeysProject", inverse_of: :deploy_key has_many :projects_with_write_access, -> { includes(:route) }, class_name: 'Project', through: :deploy_keys_projects_with_write_access, source: :project + has_many :projects_with_readonly_access, -> { includes(:route) }, class_name: 'Project', through: :deploy_keys_projects_with_readonly_access, source: :project has_many :protected_branch_push_access_levels, class_name: '::ProtectedBranch::PushAccessLevel', inverse_of: :deploy_key has_many :protected_tag_create_access_levels, class_name: '::ProtectedTag::CreateAccessLevel', inverse_of: :deploy_key scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where(deploy_keys_projects: { project_id: projects }) } scope :with_write_access, -> { joins(:deploy_keys_projects).merge(DeployKeysProject.with_write_access) } + scope :with_readonly_access, -> { joins(:deploy_keys_projects).merge(DeployKeysProject.with_readonly_access) } scope :are_public, -> { where(public: true) } scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, namespace: :route] }) } scope :including_projects_with_write_access, -> { includes(:projects_with_write_access) } + scope :including_projects_with_readonly_access, -> { includes(:projects_with_readonly_access) } accepts_nested_attributes_for :deploy_keys_projects, reject_if: :reject_deploy_keys_projects? diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb index 363ef0b1c9a0747994a492dba9cfbf1525770fe2..e114b7297ebda08fbe5d09bba626e2afd46f1bfe 100644 --- a/app/models/deploy_keys_project.rb +++ b/app/models/deploy_keys_project.rb @@ -5,6 +5,7 @@ class DeployKeysProject < ApplicationRecord belongs_to :deploy_key, inverse_of: :deploy_keys_projects scope :in_project, ->(project) { where(project: project) } scope :with_write_access, -> { where(can_push: true) } + scope :with_readonly_access, -> { where(can_push: false) } accepts_nested_attributes_for :deploy_key diff --git a/doc/api/deploy_keys.md b/doc/api/deploy_keys.md index 61e93b780674f25c2396ee518c23059bbe098bb1..003a5963ada3da758159119438b031782ed641fe 100644 --- a/doc/api/deploy_keys.md +++ b/doc/api/deploy_keys.md @@ -13,6 +13,8 @@ The deploy keys API can return in responses fingerprints of the public key in th ## List all deploy keys **(FREE SELF)** +> `projects_with_readonly_access` [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/119147) in GitLab 16.0. + Get a list of all deploy keys across all projects of the GitLab instance. This endpoint requires administrator access and is not available on GitLab.com. @@ -63,7 +65,8 @@ Example response: "path_with_namespace": "sidney_jones/project3", "created_at": "2021-10-25T18:33:17.666Z" } - ] + ], + "projects_with_readonly_access": [] }, { "id": 3, @@ -73,7 +76,18 @@ Example response: "fingerprint_sha256": "SHA256:lGI/Ys/Wx7PfMhUO1iuBH92JQKYN+3mhJZvWO4Q5ims", "created_at": "2013-10-02T11:12:29Z", "expires_at": null, - "projects_with_write_access": [] + "projects_with_write_access": [], + "projects_with_readonly_access": [ + { + "id": 74, + "description": null, + "name": "project3", + "name_with_namespace": "Sidney Jones / project3", + "path": "project3", + "path_with_namespace": "sidney_jones/project3", + "created_at": "2021-10-25T18:33:17.666Z" + } + ] } ] ``` diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 634d6052b997c3466b4365dcc3d3fdebbc5fbefd..f8379392531c87251a6312d92238414bd0ab741a 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -41,8 +41,10 @@ def find_by_deploy_key(project, key_id) authenticated_as_admin! deploy_keys = params[:public] ? DeployKey.are_public : DeployKey.all + deploy_keys = deploy_keys.including_projects_with_write_access.including_projects_with_readonly_access - present paginate(deploy_keys.including_projects_with_write_access), with: Entities::DeployKey, include_projects_with_write_access: true + present paginate(deploy_keys), + with: Entities::DeployKey, include_projects_with_write_access: true, include_projects_with_readonly_access: true end params do diff --git a/lib/api/entities/deploy_key.rb b/lib/api/entities/deploy_key.rb index 1bcd06f2c8839e43e32ae84b329ad069af9687b0..0e82d9abb63da20e640552eefe0e216afa1b3c47 100644 --- a/lib/api/entities/deploy_key.rb +++ b/lib/api/entities/deploy_key.rb @@ -14,6 +14,7 @@ class DeployKey < Entities::SSHKey documentation: { type: 'string', example: 'SHA256:Jrs3LD1Ji30xNLtTVf9NDCj7kkBgPBb2pjvTZ3HfIgU' } expose :projects_with_write_access, using: Entities::ProjectIdentity, if: -> (_, options) { options[:include_projects_with_write_access] } + expose :projects_with_readonly_access, using: Entities::ProjectIdentity, if: -> (_, options) { options[:include_projects_with_readonly_access] } end end end diff --git a/spec/factories/deploy_keys_projects.rb b/spec/factories/deploy_keys_projects.rb index 2a429bf8e56aa3e81a413969381dd5675d059d08..118336913299c2e1e8b801d0b99f76c1ff4a51a1 100644 --- a/spec/factories/deploy_keys_projects.rb +++ b/spec/factories/deploy_keys_projects.rb @@ -8,5 +8,9 @@ trait :write_access do can_push { true } end + + trait :readonly_access do + can_push { false } + end end end diff --git a/spec/fixtures/api/schemas/public_api/v4/deploy_key.json b/spec/fixtures/api/schemas/public_api/v4/deploy_key.json index 4f8b5c8422e83aa2dc26f6ed62291c71823fdc0f..77e533488f0fc32e60b40e10acd947cc037e82a4 100644 --- a/spec/fixtures/api/schemas/public_api/v4/deploy_key.json +++ b/spec/fixtures/api/schemas/public_api/v4/deploy_key.json @@ -50,6 +50,12 @@ "items": { "$ref": "project/identity.json" } + }, + "projects_with_readonly_access": { + "type": "array", + "items": { + "$ref": "project/identity.json" + } } }, "additionalProperties": false diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index 337fa40b4ba2bdfda9478032084026319a4f2604..528b36babc6898a229994d30f1fe0ecaf815c3cd 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -20,6 +20,20 @@ .source(:project) end + it do + is_expected.to have_many(:deploy_keys_projects_with_readonly_access) + .conditions(can_push: false) + .class_name('DeployKeysProject') + .inverse_of(:deploy_key) + end + + it do + is_expected.to have_many(:projects_with_readonly_access) + .class_name('Project') + .through(:deploy_keys_projects_with_readonly_access) + .source(:project) + end + it { is_expected.to have_many(:projects) } it { is_expected.to have_many(:protected_branch_push_access_levels).inverse_of(:deploy_key) } it { is_expected.to have_many(:protected_tag_create_access_levels).inverse_of(:deploy_key) } @@ -95,7 +109,7 @@ it { is_expected.to be_empty } end - context 'and this deploy key has not write access to the project' do + context 'and this deploy key has no write access to the project' do let(:specific_deploy_key) { create(:deploy_key, deploy_keys_projects: [create(:deploy_keys_project, project: project)]) } it { is_expected.to be_empty } diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index 18a9211df3ef0daf869f193e91354b98e7a10f23..30c345ef4584b3a688e56c1754b374360489aeea 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -59,6 +59,17 @@ def make_api_request(params = {}) expect { make_api_request }.not_to exceed_all_query_limit(control) end + it 'avoids N+1 database queries', :use_sql_query_cache, :request_store do + create(:deploy_keys_project, :readonly_access, project: project2, deploy_key: deploy_key) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { make_api_request } + + deploy_key2 = create(:deploy_key, public: true) + create(:deploy_keys_project, :readonly_access, project: project3, deploy_key: deploy_key2) + + expect { make_api_request }.not_to exceed_all_query_limit(control) + end + context 'when `public` parameter is `true`' do it 'only returns public deploy keys' do make_api_request({ public: true }) @@ -81,6 +92,21 @@ def make_api_request(params = {}) expect(response_projects_with_write_access[1]['id']).to eq(project3.id) end end + + context 'projects_with_readonly_access' do + let!(:deploy_keys_project2) { create(:deploy_keys_project, :readonly_access, project: project2, deploy_key: deploy_key) } + let!(:deploy_keys_project3) { create(:deploy_keys_project, :readonly_access, project: project3, deploy_key: deploy_key) } + + it 'returns projects with readonly access' do + make_api_request + + response_projects_with_readonly_access = json_response.first['projects_with_readonly_access'] + + expect(response_projects_with_readonly_access[0]['id']).to eq(project.id) + expect(response_projects_with_readonly_access[1]['id']).to eq(project2.id) + expect(response_projects_with_readonly_access[2]['id']).to eq(project3.id) + end + end end end @@ -103,6 +129,7 @@ def perform_request expect(json_response).to be_an Array expect(json_response.first['title']).to eq(deploy_key.title) expect(json_response.first).not_to have_key(:projects_with_write_access) + expect(json_response.first).not_to have_key(:projects_with_readonly_access) end it 'returns multiple deploy keys without N + 1' do @@ -129,6 +156,7 @@ def perform_request expect(json_response['title']).to eq(deploy_key.title) expect(json_response).not_to have_key(:projects_with_write_access) + expect(json_response).not_to have_key(:projects_with_readonly_access) end it 'returns 404 Not Found with invalid ID' do