diff options
Diffstat (limited to 'spec/services/import/github_service_spec.rb')
| -rw-r--r-- | spec/services/import/github_service_spec.rb | 395 |
1 files changed, 206 insertions, 189 deletions
diff --git a/spec/services/import/github_service_spec.rb b/spec/services/import/github_service_spec.rb index 982b8b11383..39832ee4b13 100644 --- a/spec/services/import/github_service_spec.rb +++ b/spec/services/import/github_service_spec.rb @@ -15,115 +15,157 @@ RSpec.describe Import::GithubService, feature_category: :importers do let(:settings) { instance_double(Gitlab::GithubImport::Settings) } let(:user_namespace_path) { user.namespace_path } let(:optional_stages) { nil } + let(:timeout_strategy) { "optimistic" } let(:params) do { repo_id: 123, new_name: 'new_repo', target_namespace: user_namespace_path, - optional_stages: optional_stages + optional_stages: optional_stages, + timeout_strategy: timeout_strategy } end + let(:client) { Gitlab::GithubImport::Client.new(token) } + let(:project_double) { instance_double(Project, persisted?: true) } + subject(:github_importer) { described_class.new(client, user, params) } - shared_examples 'handles errors' do |klass| - let(:client) { klass.new(token) } - let(:project_double) { instance_double(Project, persisted?: true) } + before do + allow(Gitlab::GithubImport::Settings).to receive(:new).with(project_double).and_return(settings) + allow(settings) + .to receive(:write) + .with( + optional_stages: optional_stages, + additional_access_tokens: access_params[:additional_access_tokens], + timeout_strategy: timeout_strategy + ) + end + + context 'do not raise an exception on input error' do + let(:exception) { Octokit::ClientError.new(status: 404, body: 'Not Found') } before do - allow(Gitlab::GithubImport::Settings).to receive(:new).with(project_double).and_return(settings) - allow(settings) - .to receive(:write) - .with( - optional_stages: optional_stages, - additional_access_tokens: access_params[:additional_access_tokens] - ) + expect(client).to receive(:repository).and_raise(exception) end - context 'do not raise an exception on input error' do - let(:exception) { Octokit::ClientError.new(status: 404, body: 'Not Found') } + it 'logs the original error' do + expect(Gitlab::Import::Logger).to receive(:error).with({ + message: 'Import failed due to a GitHub error', + status: 404, + error: 'Not Found' + }).and_call_original - before do - expect(client).to receive(:repository).and_raise(exception) - end + subject.execute(access_params, :github) + end - it 'logs the original error' do - expect(Gitlab::Import::Logger).to receive(:error).with({ - message: 'Import failed due to a GitHub error', - status: 404, - error: 'Not Found' - }).and_call_original + it 'returns an error with message and code' do + result = subject.execute(access_params, :github) - subject.execute(access_params, :github) - end + expect(result).to include( + message: 'Import failed due to a GitHub error: Not Found (HTTP 404)', + status: :error, + http_status: :unprocessable_entity + ) + end + end - it 'returns an error with message and code' do - result = subject.execute(access_params, :github) + it 'raises an exception for unknown error causes' do + exception = StandardError.new('Not Implemented') - expect(result).to include( - message: 'Import failed due to a GitHub error: Not Found (HTTP 404)', - status: :error, - http_status: :unprocessable_entity - ) - end - end + expect(client).to receive(:repository).and_raise(exception) - it 'raises an exception for unknown error causes' do - exception = StandardError.new('Not Implemented') + expect(Gitlab::Import::Logger).not_to receive(:error) - expect(client).to receive(:repository).and_raise(exception) + expect { subject.execute(access_params, :github) }.to raise_error(exception) + end + + context 'repository size validation' do + let(:repository_double) { { name: 'repository', size: 99 } } - expect(Gitlab::Import::Logger).not_to receive(:error) + before do + allow(subject).to receive(:authorized?).and_return(true) + expect(client).to receive(:repository).and_return(repository_double) - expect { subject.execute(access_params, :github) }.to raise_error(exception) + allow_next_instance_of(Gitlab::LegacyGithubImport::ProjectCreator) do |creator| + allow(creator).to receive(:execute).and_return(project_double) + end end - context 'repository size validation' do - let(:repository_double) { { name: 'repository', size: 99 } } + 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(optional_stages: nil, + additional_access_tokens: access_params[:additional_access_tokens], + timeout_strategy: timeout_strategy + ) + expect_snowplow_event( + category: 'Import::GithubService', + action: 'create', + label: 'import_access_level', + user: user, + extra: { import_type: 'github', user_role: 'Owner' } + ) + end + end - before do - allow(subject).to receive(:authorized?).and_return(true) - expect(client).to receive(:repository).and_return(repository_double) + context 'when the target namespace repository size limit is defined' do + let_it_be(:group) { create(:group, repository_size_limit: 100) } - allow_next_instance_of(Gitlab::LegacyGithubImport::ProjectCreator) do |creator| - allow(creator).to receive(:execute).and_return(project_double) - end + before do + params[:target_namespace] = group.full_path end - 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(optional_stages: nil, additional_access_tokens: access_params[:additional_access_tokens]) - expect_snowplow_event( - category: 'Import::GithubService', - action: 'create', - label: 'import_access_level', - user: user, - extra: { import_type: 'github', user_role: 'Owner' } + 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( + optional_stages: nil, + additional_access_tokens: access_params[:additional_access_tokens], + timeout_strategy: timeout_strategy ) - end + expect_snowplow_event( + category: 'Import::GithubService', + action: 'create', + label: 'import_access_level', + user: user, + extra: { import_type: 'github', user_role: 'Not a member' } + ) end - context 'when the target namespace repository size limit is defined' do - let_it_be(:group) { create(:group, repository_size_limit: 100) } + it 'returns error when the repository is larger than the limit' do + repository_double[:size] = 101 - before do - params[:target_namespace] = group.full_path - end + expect(subject.execute(access_params, :github)).to include(size_limit_error) + end + end + + context 'when target namespace repository limit is not defined' do + let_it_be(:group) { create(:group) } + before do + stub_application_setting(repository_size_limit: 100) + end + + 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(optional_stages: nil, additional_access_tokens: access_params[:additional_access_tokens]) + .with( + optional_stages: nil, + additional_access_tokens: access_params[:additional_access_tokens], + timeout_strategy: timeout_strategy + ) expect_snowplow_event( category: 'Import::GithubService', action: 'create', label: 'import_access_level', user: user, - extra: { import_type: 'github', user_role: 'Not a member' } + extra: { import_type: 'github', user_role: 'Owner' } ) end @@ -133,167 +175,142 @@ RSpec.describe Import::GithubService, feature_category: :importers do expect(subject.execute(access_params, :github)).to include(size_limit_error) end end - - context 'when target namespace repository limit is not defined' do - let_it_be(:group) { create(:group) } - - before do - stub_application_setting(repository_size_limit: 100) - end - - 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(optional_stages: nil, additional_access_tokens: access_params[:additional_access_tokens]) - expect_snowplow_event( - category: 'Import::GithubService', - action: 'create', - label: 'import_access_level', - user: user, - extra: { import_type: 'github', user_role: 'Owner' } - ) - end - - it 'returns error when the repository is larger than the limit' do - repository_double[:size] = 101 - - expect(subject.execute(access_params, :github)).to include(size_limit_error) - end - end - end - - context 'when optional stages params present' do - let(:optional_stages) do - { - single_endpoint_issue_events_import: true, - single_endpoint_notes_import: 'false', - attachments_import: false, - collaborators_import: true - } - end - - it 'saves optional stages choice to import_data' do - subject.execute(access_params, :github) - - 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 - context 'when import source is disabled' do - let(:repository_double) do + context 'when optional stages params present' do + let(:optional_stages) do { - name: 'vim', - description: 'test', - full_name: 'test/vim', - clone_url: 'http://repo.com/repo/repo.git', - private: false, - has_wiki?: false + single_endpoint_issue_events_import: true, + single_endpoint_notes_import: 'false', + attachments_import: false, + collaborators_import: true } end - before do - stub_application_setting(import_sources: nil) - allow(client).to receive(:repository).and_return(repository_double) + it 'saves optional stages choice to import_data' do + subject.execute(access_params, :github) + + expect(settings) + .to have_received(:write) + .with( + optional_stages: optional_stages, + additional_access_tokens: access_params[:additional_access_tokens], + timeout_strategy: timeout_strategy + ) end + end - it 'returns forbidden' do - result = subject.execute(access_params, :github) + context 'when timeout strategy param is present' do + let(:timeout_strategy) { 'pessimistic' } - expect(result).to include( - status: :error, - http_status: :forbidden - ) + it 'saves timeout strategy to import_data' do + subject.execute(access_params, :github) + + expect(settings) + .to have_received(:write) + .with( + optional_stages: optional_stages, + additional_access_tokens: access_params[:additional_access_tokens], + timeout_strategy: timeout_strategy + ) end end - context 'when a blocked/local URL is used as github_hostname' do - let(:message) { 'Error while attempting to import from GitHub' } - let(:error) { "Invalid URL: #{url}" } + context 'when additional access tokens are present' do + it 'saves additional access tokens to import_data' do + subject.execute(access_params, :github) - before do - stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) + expect(settings) + .to have_received(:write) + .with( + optional_stages: optional_stages, + additional_access_tokens: %w[foo bar], + timeout_strategy: timeout_strategy + ) end + end + end - where(url: %w[https://localhost https://10.0.0.1]) - - with_them do - it 'returns and logs an error' do - allow(github_importer).to receive(:url).and_return(url) + context 'when import source is disabled' do + let(:repository_double) do + { + name: 'vim', + description: 'test', + full_name: 'test/vim', + clone_url: 'http://repo.com/repo/repo.git', + private: false, + has_wiki?: false + } + end - expect(Gitlab::Import::Logger).to receive(:error).with({ - message: message, - error: error - }).and_call_original - expect(github_importer.execute(access_params, :github)).to include(blocked_url_error(url)) - end - end + before do + stub_application_setting(import_sources: nil) + allow(client).to receive(:repository).and_return(repository_double) end - context 'when target_namespace is blank' do - before do - params[:target_namespace] = '' - end + it 'returns forbidden' do + result = subject.execute(access_params, :github) - it 'raises an exception' do - expect { subject.execute(access_params, :github) }.to raise_error(ArgumentError, 'Target namespace is required') - end + expect(result).to include( + status: :error, + http_status: :forbidden + ) end + end - context 'when namespace to import repository into does not exist' do - before do - params[:target_namespace] = 'unknown_path' - end + context 'when a blocked/local URL is used as github_hostname' do + let(:message) { 'Error while attempting to import from GitHub' } + let(:error) { "Invalid URL: #{url}" } - it 'returns an error' do - expect(github_importer.execute(access_params, :github)).to include(not_existed_namespace_error) - end + before do + stub_application_setting(allow_local_requests_from_web_hooks_and_services: false) end - context 'when user has no permissions to import repository into the specified namespace' do - let_it_be(:group) { create(:group) } + where(url: %w[https://localhost https://10.0.0.1]) - before do - params[:target_namespace] = group.full_path - end + with_them do + it 'returns and logs an error' do + allow(github_importer).to receive(:url).and_return(url) - it 'returns an error' do - expect(github_importer.execute(access_params, :github)).to include(taken_namespace_error) + expect(Gitlab::Import::Logger).to receive(:error).with({ + message: message, + error: error + }).and_call_original + expect(github_importer.execute(access_params, :github)).to include(blocked_url_error(url)) end end end - context 'when remove_legacy_github_client feature flag is enabled' do + context 'when target_namespace is blank' do + before do + params[:target_namespace] = '' + end + + it 'raises an exception' do + expect { subject.execute(access_params, :github) }.to raise_error(ArgumentError, 'Target namespace is required') + end + end + + context 'when namespace to import repository into does not exist' do before do - stub_feature_flags(remove_legacy_github_client: true) + params[:target_namespace] = 'unknown_path' end - include_examples 'handles errors', Gitlab::GithubImport::Client + it 'returns an error' do + expect(github_importer.execute(access_params, :github)).to include(not_existed_namespace_error) + end end - context 'when remove_legacy_github_client feature flag is disabled' do + context 'when user has no permissions to import repository into the specified namespace' do + let_it_be(:group) { create(:group) } + before do - stub_feature_flags(remove_legacy_github_client: false) + params[:target_namespace] = group.full_path end - include_examples 'handles errors', Gitlab::LegacyGithubImport::Client + it 'returns an error' do + expect(github_importer.execute(access_params, :github)).to include(taken_namespace_error) + end end def size_limit_error |