diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 7e7f7ea9810bd4c2440487ca5805074696d25935..df255a7ae24dbc429023664acb179c710089e939 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -16,7 +16,7 @@ def execute(access_params, provider) track_access_level('github') if project.persisted? - store_import_settings(project) + store_import_settings(project, access_params) success(project) elsif project.errors[:import_source_disabled].present? error(project.errors[:import_source_disabled], :forbidden) @@ -134,8 +134,13 @@ def log_and_return_error(message, translated_message, http_status) error(translated_message, http_status) end - def store_import_settings(project) - Gitlab::GithubImport::Settings.new(project).write(params[:optional_stages]) + def store_import_settings(project, access_params) + Gitlab::GithubImport::Settings + .new(project) + .write( + optional_stages: params[:optional_stages], + additional_access_tokens: access_params[:additional_access_tokens] + ) end end end diff --git a/doc/api/import.md b/doc/api/import.md index be70868cca535feae5f87b9998050f253a68a5b7..7bbc19cb36a439a97ef4464070135f8091a7daba 100644 --- a/doc/api/import.md +++ b/doc/api/import.md @@ -26,14 +26,15 @@ Prerequisites: POST /import/github ``` -| Attribute | Type | Required | Description | -|-------------------------|---------|----------|-------------------------------------------------------------------------------------| -| `personal_access_token` | string | yes | GitHub personal access token | -| `repo_id` | integer | yes | GitHub repository ID | -| `new_name` | string | no | New repository name | -| `target_namespace` | string | yes | Namespace to import repository into. Supports subgroups like `/namespace/subgroup`. In GitLab 15.8 and later, must not be blank | -| `github_hostname` | string | no | Custom GitHub Enterprise hostname. Do not set for GitHub.com. | -| `optional_stages` | object | no | [Additional items to import](../user/project/import/github.md#select-additional-items-to-import). [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/373705) in GitLab 15.5 | +| Attribute | Type | Required | Description | +|----------------------------|---------|----------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `personal_access_token` | string | yes | GitHub personal access token | +| `repo_id` | integer | yes | GitHub repository ID | +| `new_name` | string | no | New repository name | +| `target_namespace` | string | yes | Namespace to import repository into. Supports subgroups like `/namespace/subgroup`. In GitLab 15.8 and later, must not be blank | +| `github_hostname` | string | no | Custom GitHub Enterprise hostname. Do not set for GitHub.com. | +| `optional_stages` | object | no | [Additional items to import](../user/project/import/github.md#select-additional-items-to-import). [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/373705) in GitLab 15.5 | +| `additional_access_tokens` | string | no | Additional list of comma-separated personal access tokens. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/337232) in GitLab 16.2 | ```shell curl --request POST \ @@ -51,7 +52,8 @@ curl --request POST \ "single_endpoint_notes_import": true, "attachments_import": true, "collaborators_import": true - } + }, + "additional_access_tokens": "foo,bar" }' ``` @@ -64,6 +66,8 @@ The following keys are available for `optional_stages`: For more information, see [Select additional items to import](../user/project/import/github.md#select-additional-items-to-import). +You can supply multiple personal access tokens in `additional_access_tokens` from different user accounts to import projects faster. + Example response: ```json diff --git a/lib/api/import_github.rb b/lib/api/import_github.rb index 6550808a5637a5f97833ccb73861d4c7c5f71f5b..ab7ac6624a8bc3f20d96e5ab809aa7ef18d7c3ed 100644 --- a/lib/api/import_github.rb +++ b/lib/api/import_github.rb @@ -20,7 +20,10 @@ def client end def access_params - { github_access_token: params[:personal_access_token] } + { + github_access_token: params[:personal_access_token], + additional_access_tokens: params[:additional_access_tokens] + } end def client_options @@ -59,6 +62,11 @@ def too_many_requests requires :target_namespace, type: String, allow_blank: false, desc: 'Namespace or group to import repository into' optional :github_hostname, type: String, desc: 'Custom GitHub enterprise hostname' optional :optional_stages, type: Hash, desc: 'Optional stages of import to be performed' + optional :additional_access_tokens, + type: Array[String], + coerce_with: ::API::Validations::Types::CommaSeparatedToArray.coerce, + desc: 'Additional list of personal access tokens', + documentation: { example: 'foo,bar' } end post 'import/github' do result = Import::GithubService.new(client, current_user, params).execute(access_params, provider) diff --git a/lib/gitlab/github_import.rb b/lib/gitlab/github_import.rb index 9556a9e98bae2ef06fcdeae70a1945bca10f948c..24e77363e1b8bf2dc2e6015598a731858dbcb4fe 100644 --- a/lib/gitlab/github_import.rb +++ b/lib/gitlab/github_import.rb @@ -8,12 +8,18 @@ def self.refmap def self.new_client_for(project, token: nil, host: nil, parallel: true) token_to_use = token || project.import_data&.credentials&.fetch(:user) - Client.new( - token_to_use, + token_pool = project.import_data&.credentials&.dig(:additional_access_tokens) + options = { host: host.presence || self.formatted_import_url(project), per_page: self.per_page(project), parallel: parallel - ) + } + + if token_pool + ClientPool.new(token_pool: token_pool, **options) + else + Client.new(token_to_use, **options) + end end # Returns the ID of the ghost user. diff --git a/lib/gitlab/github_import/client_pool.rb b/lib/gitlab/github_import/client_pool.rb new file mode 100644 index 0000000000000000000000000000000000000000..e8414942d1b52ce76426b7b9907b54d51588f304 --- /dev/null +++ b/lib/gitlab/github_import/client_pool.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Gitlab + module GithubImport + class ClientPool + delegate_missing_to :best_client + + def initialize(token_pool:, per_page:, parallel:, host: nil) + @token_pool = token_pool + @host = host + @per_page = per_page + @parallel = parallel + end + + # Returns the client with the most remaining requests, or the client with + # the closest rate limit reset time, if all clients are rate limited. + def best_client + clients_with_requests_remaining = clients.select(&:requests_remaining?) + + return clients_with_requests_remaining.max_by(&:remaining_requests) if clients_with_requests_remaining.any? + + clients.min_by(&:rate_limit_resets_in) + end + + private + + def clients + @clients ||= @token_pool.map do |token| + Client.new( + token, + host: @host, + per_page: @per_page, + parallel: @parallel + ) + end + end + end + end +end diff --git a/lib/gitlab/github_import/settings.rb b/lib/gitlab/github_import/settings.rb index 0b883de8ed042a156f53ff16f0e284137c7e0d00..73a5f49a9e3a71c6e144bf4496e288f5913adea1 100644 --- a/lib/gitlab/github_import/settings.rb +++ b/lib/gitlab/github_import/settings.rb @@ -56,8 +56,16 @@ def initialize(project) def write(user_settings) user_settings = user_settings.to_h.with_indifferent_access - optional_stages = fetch_stages_from_params(user_settings) - import_data = project.create_or_update_import_data(data: { optional_stages: optional_stages }) + optional_stages = fetch_stages_from_params(user_settings[:optional_stages]) + credentials = project.import_data&.credentials&.merge( + additional_access_tokens: user_settings[:additional_access_tokens] + ) + + import_data = project.create_or_update_import_data( + data: { optional_stages: optional_stages }, + credentials: credentials + ) + import_data.save! end @@ -74,6 +82,8 @@ def disabled?(stage_name) attr_reader :project def fetch_stages_from_params(user_settings) + user_settings = user_settings.to_h.with_indifferent_access + OPTIONAL_STAGES.keys.to_h do |stage_name| enabled = Gitlab::Utils.to_boolean(user_settings[stage_name], default: false) [stage_name, enabled] diff --git a/spec/lib/gitlab/github_import/client_pool_spec.rb b/spec/lib/gitlab/github_import/client_pool_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..aabb47c2cf1f642fe6f90493e727ae9ea5a68b5d --- /dev/null +++ b/spec/lib/gitlab/github_import/client_pool_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::GithubImport::ClientPool, feature_category: :importers do + subject(:pool) { described_class.new(token_pool: %w[foo bar], per_page: 1, parallel: true) } + + describe '#best_client' do + it 'returns the client with the most remaining requests' do + allow(Gitlab::GithubImport::Client).to receive(:new).and_return( + instance_double( + Gitlab::GithubImport::Client, + requests_remaining?: true, remaining_requests: 10, rate_limit_resets_in: 1 + ), + instance_double( + Gitlab::GithubImport::Client, + requests_remaining?: true, remaining_requests: 20, rate_limit_resets_in: 2 + ) + ) + + expect(pool.best_client.remaining_requests).to eq(20) + end + + context 'when all clients are rate limited' do + it 'returns the client with the closest rate limit reset time' do + allow(Gitlab::GithubImport::Client).to receive(:new).and_return( + instance_double( + Gitlab::GithubImport::Client, + requests_remaining?: false, remaining_requests: 10, rate_limit_resets_in: 10 + ), + instance_double( + Gitlab::GithubImport::Client, + requests_remaining?: false, remaining_requests: 20, rate_limit_resets_in: 20 + ) + ) + + expect(pool.best_client.rate_limit_resets_in).to eq(10) + end + end + end +end diff --git a/spec/lib/gitlab/github_import/settings_spec.rb b/spec/lib/gitlab/github_import/settings_spec.rb index 43e096863b8536c9c01a65e58e713e31e81b9335..d670aaea482ecd20a5c9e8f088c6fcfe421bddd8 100644 --- a/spec/lib/gitlab/github_import/settings_spec.rb +++ b/spec/lib/gitlab/github_import/settings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::Settings do +RSpec.describe Gitlab::GithubImport::Settings, feature_category: :importers do subject(:settings) { described_class.new(project) } let_it_be(:project) { create(:project) } @@ -55,19 +55,26 @@ describe '#write' do let(:data_input) do { - single_endpoint_issue_events_import: true, - single_endpoint_notes_import: 'false', - attachments_import: nil, - collaborators_import: false, - foo: :bar + optional_stages: { + single_endpoint_issue_events_import: true, + single_endpoint_notes_import: 'false', + attachments_import: nil, + collaborators_import: false, + foo: :bar + }, + additional_access_tokens: %w[foo bar] }.stringify_keys end - it 'puts optional steps flags into projects import_data' do + it 'puts optional steps & access tokens into projects import_data' do + project.create_or_update_import_data(credentials: { user: 'token' }) + settings.write(data_input) expect(project.import_data.data['optional_stages']) .to eq optional_stages.stringify_keys + expect(project.import_data.credentials.fetch(:additional_access_tokens)) + .to eq(data_input['additional_access_tokens']) end end diff --git a/spec/lib/gitlab/github_import_spec.rb b/spec/lib/gitlab/github_import_spec.rb index 1ea9f00309897d795d4457b563eda408617246bc..c4ed4b09f04fe0bce2d3e0abafeb69f6057f1363 100644 --- a/spec/lib/gitlab/github_import_spec.rb +++ b/spec/lib/gitlab/github_import_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport do +RSpec.describe Gitlab::GithubImport, feature_category: :importers do before do stub_feature_flags(github_importer_lower_per_page_limit: false) end @@ -11,6 +11,8 @@ let(:project) { double(:project, import_url: 'http://t0ken@github.com/user/repo.git', id: 1, group: nil) } it 'returns a new Client with a custom token' do + allow(project).to receive(:import_data) + expect(described_class::Client) .to receive(:new) .with('123', host: nil, parallel: true, per_page: 100) @@ -24,6 +26,7 @@ expect(project) .to receive(:import_data) .and_return(import_data) + .twice expect(described_class::Client) .to receive(:new) @@ -46,12 +49,31 @@ described_class.ghost_user_id end end + + context 'when there are additional access tokens' do + it 'returns a new ClientPool containing all tokens' do + import_data = double(:import_data, credentials: { user: '123', additional_access_tokens: %w[foo bar] }) + + expect(project) + .to receive(:import_data) + .and_return(import_data) + .twice + + expect(described_class::ClientPool) + .to receive(:new) + .with(token_pool: %w[foo bar], host: nil, parallel: true, per_page: 100) + + described_class.new_client_for(project) + end + end end context 'GitHub Enterprise' do let(:project) { double(:project, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git', group: nil) } it 'returns a new Client with a custom token' do + allow(project).to receive(:import_data) + expect(described_class::Client) .to receive(:new) .with('123', host: 'http://github.another-domain.com/api/v3', parallel: true, per_page: 100) @@ -65,6 +87,7 @@ expect(project) .to receive(:import_data) .and_return(import_data) + .twice expect(described_class::Client) .to receive(:new) diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index 9b5ae72526cd79ad164cb9bcb7c3d2230bb1f2cd..e394b92c0a2fab66ffe4d1fbd1d6f3d54708ff78 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -5,7 +5,8 @@ RSpec.describe API::ImportGithub, feature_category: :importers do let(:token) { "asdasd12345" } let(:provider) { :github } - let(:access_params) { { github_access_token: token } } + let(:access_params) { { github_access_token: token, additional_access_tokens: additional_access_tokens } } + let(:additional_access_tokens) { nil } let(:provider_username) { user.username } let(:provider_user) { double('provider', login: provider_username).as_null_object } let(:provider_repo) do @@ -51,7 +52,7 @@ it 'returns 201 response when the project is imported successfully' do allow(Gitlab::LegacyGithubImport::ProjectCreator) .to receive(:new).with(provider_repo, provider_repo[:name], user.namespace, user, type: provider, **access_params) - .and_return(double(execute: project)) + .and_return(double(execute: project)) post api("/import/github", user), params: { target_namespace: user.namespace_path, @@ -120,6 +121,28 @@ expect(response).to have_gitlab_http_status(:unauthorized) end end + + context 'when additional access tokens are provided' do + let(:additional_access_tokens) { 'token1,token2' } + + it 'returns 201' do + expected_access_params = { github_access_token: token, additional_access_tokens: %w[token1 token2] } + + expect(Gitlab::LegacyGithubImport::ProjectCreator) + .to receive(:new) + .with(provider_repo, provider_repo[:name], user.namespace, user, type: provider, **expected_access_params) + .and_return(double(execute: project)) + + post api("/import/github", user), params: { + target_namespace: user.namespace_path, + personal_access_token: token, + repo_id: non_existing_record_id, + additional_access_tokens: 'token1,token2' + } + + expect(response).to have_gitlab_http_status(:created) + end + end end describe "POST /import/github/cancel" do diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb index 21dc24e28f63be7a3713638318f46cd7213db085..982b8b1138315c2b217ee02f09116ba3f7c39b41 100644 --- a/spec/services/import/github_service_spec.rb +++ b/spec/services/import/github_service_spec.rb @@ -5,7 +5,13 @@ RSpec.describe Import::GithubService, feature_category: :importers do let_it_be(:user) { create(:user) } let_it_be(:token) { 'complex-token' } - let_it_be(:access_params) { { github_access_token: 'github-complex-token' } } + let_it_be(:access_params) do + { + github_access_token: 'github-complex-token', + additional_access_tokens: %w[foo bar] + } + end + let(:settings) { instance_double(Gitlab::GithubImport::Settings) } let(:user_namespace_path) { user.namespace_path } let(:optional_stages) { nil } @@ -26,7 +32,12 @@ before do allow(Gitlab::GithubImport::Settings).to receive(:new).with(project_double).and_return(settings) - allow(settings).to receive(:write).with(optional_stages) + allow(settings) + .to receive(:write) + .with( + optional_stages: optional_stages, + additional_access_tokens: access_params[:additional_access_tokens] + ) end context 'do not raise an exception on input error' do @@ -82,7 +93,9 @@ context 'when there is no repository size limit defined' do it 'skips the check, succeeds, and tracks an access level' do expect(subject.execute(access_params, :github)).to include(status: :success) - expect(settings).to have_received(:write).with(nil) + expect(settings) + .to have_received(:write) + .with(optional_stages: nil, additional_access_tokens: access_params[:additional_access_tokens]) expect_snowplow_event( category: 'Import::GithubService', action: 'create', @@ -102,7 +115,9 @@ it 'succeeds when the repository is smaller than the limit' do expect(subject.execute(access_params, :github)).to include(status: :success) - expect(settings).to have_received(:write).with(nil) + expect(settings) + .to have_received(:write) + .with(optional_stages: nil, additional_access_tokens: access_params[:additional_access_tokens]) expect_snowplow_event( category: 'Import::GithubService', action: 'create', @@ -129,7 +144,9 @@ context 'when application size limit is defined' do it 'succeeds when the repository is smaller than the limit' do expect(subject.execute(access_params, :github)).to include(status: :success) - expect(settings).to have_received(:write).with(nil) + expect(settings) + .to have_received(:write) + .with(optional_stages: nil, additional_access_tokens: access_params[:additional_access_tokens]) expect_snowplow_event( category: 'Import::GithubService', action: 'create', @@ -160,7 +177,22 @@ it 'saves optional stages choice to import_data' do subject.execute(access_params, :github) - expect(settings).to have_received(:write).with(optional_stages) + expect(settings) + .to have_received(:write) + .with( + optional_stages: optional_stages, + additional_access_tokens: access_params[:additional_access_tokens] + ) + end + end + + context 'when additional access tokens are present' do + it 'saves additional access tokens to import_data' do + subject.execute(access_params, :github) + + expect(settings) + .to have_received(:write) + .with(optional_stages: optional_stages, additional_access_tokens: %w[foo bar]) end end end diff --git a/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb index 2945bcbe64104bd343d3868469f61fea20355444..e385a5aaf3f07f77577326a49ec45e97405efebe 100644 --- a/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_attachments_worker_spec.rb @@ -10,7 +10,7 @@ let(:stage_enabled) { true } before do - settings.write({ attachments_import: stage_enabled }) + settings.write({ optional_stages: { attachments_import: stage_enabled } }) end describe '#import' do diff --git a/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb index 33ecf848997232990f8cb465187401c08779ed4a..808f6e827edef979fdcfeee86447d1a23ebce9ba 100644 --- a/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_collaborators_worker_spec.rb @@ -16,7 +16,7 @@ let(:push_rights_granted) { true } before do - settings.write({ collaborators_import: stage_enabled }) + settings.write({ optional_stages: { collaborators_import: stage_enabled } }) allow(client).to receive(:repository).with(project.import_source) .and_return({ permissions: { push: push_rights_granted } }) end diff --git a/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb index c70ee0250e82d20d8f0e42ea9796eeb46e17020d..7b0cf77bbbeb301b328f04612a09961d1a2a591e 100644 --- a/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_issue_events_worker_spec.rb @@ -11,7 +11,7 @@ let(:stage_enabled) { true } before do - settings.write({ single_endpoint_issue_events_import: stage_enabled }) + settings.write({ optional_stages: { single_endpoint_issue_events_import: stage_enabled } }) end describe '#import' do diff --git a/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb index 872201ece93734bc102a9e1a1fc30483f8a59fb9..188cf3530f77cb49e9c9fd88776518df0e76d34e 100644 --- a/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_issues_and_diff_notes_worker_spec.rb @@ -10,7 +10,7 @@ let(:single_endpoint_optional_stage) { true } before do - settings.write({ single_endpoint_notes_import: single_endpoint_optional_stage }) + settings.write({ optional_stages: { single_endpoint_notes_import: single_endpoint_optional_stage } }) end describe '#import' do diff --git a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb index 8c0004c0f3ff8ab8754cd69e9b100e99fe2cc44b..dcceeb1d6c2e03118141043f50ab7dbaf5dec6a5 100644 --- a/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_notes_worker_spec.rb @@ -10,7 +10,7 @@ let(:single_endpoint_optional_stage) { true } before do - settings.write({ single_endpoint_notes_import: single_endpoint_optional_stage }) + settings.write({ optional_stages: { single_endpoint_notes_import: single_endpoint_optional_stage } }) end describe '#import' do