Commit 5639d043 authored by James Fargher's avatar James Fargher

Merge branch 'add-github-host-to-api' into 'master'

Add github host to API and clients

See merge request gitlab-org/gitlab!45188
parents 734e1dfa 631ed9ab
...@@ -6,6 +6,10 @@ module Import ...@@ -6,6 +6,10 @@ module Import
attr_reader :params, :current_user attr_reader :params, :current_user
def execute(access_params, provider) def execute(access_params, provider)
if blocked_url?
return log_and_return_error("Invalid URL: #{url}", :bad_request)
end
unless authorized? unless authorized?
return error(_('This namespace has already been taken! Please choose another one.'), :unprocessable_entity) return error(_('This namespace has already been taken! Please choose another one.'), :unprocessable_entity)
end end
...@@ -56,6 +60,25 @@ module Import ...@@ -56,6 +60,25 @@ module Import
can?(current_user, :create_projects, target_namespace) can?(current_user, :create_projects, target_namespace)
end end
def url
@url ||= params[:github_hostname]
end
def allow_local_requests?
Gitlab::CurrentSettings.allow_local_requests_from_web_hooks_and_services?
end
def blocked_url?
Gitlab::UrlBlocker.blocked_url?(
url,
{
allow_localhost: allow_local_requests?,
allow_local_network: allow_local_requests?,
schemes: %w(http https)
}
)
end
private private
def log_error(exception) def log_error(exception)
......
---
title: Add hostname to GitHub import API
merge_request: 45188
author:
type: added
...@@ -14,6 +14,7 @@ POST /import/github ...@@ -14,6 +14,7 @@ POST /import/github
| `repo_id` | integer | yes | GitHub repository ID | | `repo_id` | integer | yes | GitHub repository ID |
| `new_name` | string | no | New repository name | | `new_name` | string | no | New repository name |
| `target_namespace` | string | yes | Namespace to import repository into. Supports subgroups like `/namespace/subgroup`. | | `target_namespace` | string | yes | Namespace to import repository into. Supports subgroups like `/namespace/subgroup`. |
| `github_hostname` | string | no | Custom GitHub enterprise hostname. Defaults to GitHub.com if `github_hostname` is not set. |
```shell ```shell
curl --request POST \ curl --request POST \
......
...@@ -11,7 +11,7 @@ module API ...@@ -11,7 +11,7 @@ module API
helpers do helpers do
def client def client
@client ||= if Feature.enabled?(:remove_legacy_github_client) @client ||= if Feature.enabled?(:remove_legacy_github_client)
Gitlab::GithubImport::Client.new(params[:personal_access_token]) Gitlab::GithubImport::Client.new(params[:personal_access_token], host: params[:github_hostname])
else else
Gitlab::LegacyGithubImport::Client.new(params[:personal_access_token], client_options) Gitlab::LegacyGithubImport::Client.new(params[:personal_access_token], client_options)
end end
...@@ -22,7 +22,7 @@ module API ...@@ -22,7 +22,7 @@ module API
end end
def client_options def client_options
{} { host: params[:github_hostname] }
end end
def provider def provider
...@@ -43,6 +43,7 @@ module API ...@@ -43,6 +43,7 @@ module API
requires :repo_id, type: Integer, desc: 'GitHub repository ID' requires :repo_id, type: Integer, desc: 'GitHub repository ID'
optional :new_name, type: String, desc: 'New repo name' optional :new_name, type: String, desc: 'New repo name'
requires :target_namespace, type: String, desc: 'Namespace to import repo into' requires :target_namespace, type: String, desc: 'Namespace to import repo into'
optional :github_hostname, type: String, desc: 'Custom GitHub enterprise hostname'
end end
post 'import/github' do post 'import/github' do
result = Import::GithubService.new(client, current_user, params).execute(access_params, provider) result = Import::GithubService.new(client, current_user, params).execute(access_params, provider)
......
...@@ -6,10 +6,13 @@ module Gitlab ...@@ -6,10 +6,13 @@ module Gitlab
[:heads, :tags, '+refs/pull/*/head:refs/merge-requests/*/head'] [:heads, :tags, '+refs/pull/*/head:refs/merge-requests/*/head']
end end
def self.new_client_for(project, token: nil, parallel: true) def self.new_client_for(project, token: nil, host: nil, parallel: true)
token_to_use = token || project.import_data&.credentials&.fetch(:user) token_to_use = token || project.import_data&.credentials&.fetch(:user)
Client.new(
Client.new(token_to_use, parallel: parallel) token_to_use,
host: host.presence || self.formatted_import_url(project),
parallel: parallel
)
end end
# Returns the ID of the ghost user. # Returns the ID of the ghost user.
...@@ -18,5 +21,17 @@ module Gitlab ...@@ -18,5 +21,17 @@ module Gitlab
Gitlab::Cache::Import::Caching.read_integer(key) || Gitlab::Cache::Import::Caching.write(key, User.select(:id).ghost.id) Gitlab::Cache::Import::Caching.read_integer(key) || Gitlab::Cache::Import::Caching.write(key, User.select(:id).ghost.id)
end end
# Get formatted GitHub import URL. If github.com is in the import URL, this will return nil and octokit will use the default github.com API URL
def self.formatted_import_url(project)
url = URI.parse(project.import_url)
unless url.host == 'github.com'
url.user = nil
url.password = nil
url.path = "/api/v3"
url.to_s
end
end
end end
end end
...@@ -31,6 +31,8 @@ module Gitlab ...@@ -31,6 +31,8 @@ module Gitlab
# token - The GitHub API token to use. # token - The GitHub API token to use.
# #
# host - The GitHub hostname. If nil, github.com will be used.
#
# per_page - The number of objects that should be displayed per page. # per_page - The number of objects that should be displayed per page.
# #
# parallel - When set to true hitting the rate limit will result in a # parallel - When set to true hitting the rate limit will result in a
...@@ -39,11 +41,13 @@ module Gitlab ...@@ -39,11 +41,13 @@ module Gitlab
# this value to `true` for parallel importing is crucial as # this value to `true` for parallel importing is crucial as
# otherwise hitting the rate limit will result in a thread # otherwise hitting the rate limit will result in a thread
# being blocked in a `sleep()` call for up to an hour. # being blocked in a `sleep()` call for up to an hour.
def initialize(token, per_page: 100, parallel: true) def initialize(token, host: nil, per_page: 100, parallel: true)
@host = host
@octokit = ::Octokit::Client.new( @octokit = ::Octokit::Client.new(
access_token: token, access_token: token,
per_page: per_page, per_page: per_page,
api_endpoint: api_endpoint api_endpoint: api_endpoint,
web_endpoint: web_endpoint
) )
@octokit.connection_options[:ssl] = { verify: verify_ssl } @octokit.connection_options[:ssl] = { verify: verify_ssl }
...@@ -181,7 +185,11 @@ module Gitlab ...@@ -181,7 +185,11 @@ module Gitlab
end end
def api_endpoint def api_endpoint
custom_api_endpoint || default_api_endpoint @host || custom_api_endpoint || default_api_endpoint
end
def web_endpoint
@host || custom_api_endpoint || ::Octokit::Default.web_endpoint
end end
def custom_api_endpoint def custom_api_endpoint
......
...@@ -25,10 +25,11 @@ module Gitlab ...@@ -25,10 +25,11 @@ module Gitlab
# project - The project to import the data into. # project - The project to import the data into.
# token - The token to use for the GitHub API. # token - The token to use for the GitHub API.
def initialize(project, token: nil) # host - The GitHub hostname. If nil, github.com will be used.
def initialize(project, token: nil, host: nil)
@project = project @project = project
@client = GithubImport @client = GithubImport
.new_client_for(project, token: token, parallel: false) .new_client_for(project, token: token, host: host, parallel: false)
end end
def execute def execute
......
...@@ -24,6 +24,7 @@ module Gitlab ...@@ -24,6 +24,7 @@ module Gitlab
@api ||= ::Octokit::Client.new( @api ||= ::Octokit::Client.new(
access_token: access_token, access_token: access_token,
api_endpoint: api_endpoint, api_endpoint: api_endpoint,
web_endpoint: web_endpoint,
# If there is no config, we're connecting to github.com and we # If there is no config, we're connecting to github.com and we
# should verify ssl. # should verify ssl.
connection_options: { connection_options: {
...@@ -85,6 +86,10 @@ module Gitlab ...@@ -85,6 +86,10 @@ module Gitlab
end end
end end
def web_endpoint
host.presence || ::Octokit::Default.web_endpoint
end
def config def config
Gitlab::Auth::OAuth::Provider.config_for('github') Gitlab::Auth::OAuth::Provider.config_for('github')
end end
......
...@@ -299,6 +299,32 @@ RSpec.describe Gitlab::GithubImport::Client do ...@@ -299,6 +299,32 @@ RSpec.describe Gitlab::GithubImport::Client do
end end
end end
describe '#web_endpoint' do
let(:client) { described_class.new('foo') }
context 'without a custom endpoint configured in Omniauth' do
it 'returns the default web endpoint' do
expect(client)
.to receive(:custom_api_endpoint)
.and_return(nil)
expect(client.web_endpoint).to eq('https://github.com')
end
end
context 'with a custom endpoint configured in Omniauth' do
it 'returns the custom endpoint' do
endpoint = 'https://github.kittens.com'
expect(client)
.to receive(:custom_api_endpoint)
.and_return(endpoint)
expect(client.web_endpoint).to eq(endpoint)
end
end
end
describe '#custom_api_endpoint' do describe '#custom_api_endpoint' do
let(:client) { described_class.new('foo') } let(:client) { described_class.new('foo') }
......
...@@ -6,7 +6,7 @@ RSpec.describe Gitlab::GithubImport::SequentialImporter do ...@@ -6,7 +6,7 @@ RSpec.describe Gitlab::GithubImport::SequentialImporter do
describe '#execute' do describe '#execute' do
it 'imports a project in sequence' do it 'imports a project in sequence' do
repository = double(:repository) repository = double(:repository)
project = double(:project, id: 1, repository: repository) project = double(:project, id: 1, repository: repository, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git')
importer = described_class.new(project, token: 'foo') importer = described_class.new(project, token: 'foo')
expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance| expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance|
......
...@@ -3,13 +3,13 @@ ...@@ -3,13 +3,13 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::GithubImport do RSpec.describe Gitlab::GithubImport do
let(:project) { double(:project) } context 'github.com' do
let(:project) { double(:project, import_url: 'http://t0ken@github.com/user/repo.git') }
describe '.new_client_for' do
it 'returns a new Client with a custom token' do it 'returns a new Client with a custom token' do
expect(described_class::Client) expect(described_class::Client)
.to receive(:new) .to receive(:new)
.with('123', parallel: true) .with('123', host: nil, parallel: true)
described_class.new_client_for(project, token: '123') described_class.new_client_for(project, token: '123')
end end
...@@ -23,18 +23,57 @@ RSpec.describe Gitlab::GithubImport do ...@@ -23,18 +23,57 @@ RSpec.describe Gitlab::GithubImport do
expect(described_class::Client) expect(described_class::Client)
.to receive(:new) .to receive(:new)
.with('123', parallel: true) .with('123', host: nil, parallel: true)
described_class.new_client_for(project) described_class.new_client_for(project)
end end
it 'returns the ID of the ghost user', :clean_gitlab_redis_cache do
expect(described_class.ghost_user_id).to eq(User.ghost.id)
end
it 'caches the ghost user ID', :clean_gitlab_redis_cache do
expect(Gitlab::Cache::Import::Caching)
.to receive(:write)
.once
.and_call_original
2.times do
described_class.ghost_user_id
end
end
end end
describe '.ghost_user_id', :clean_gitlab_redis_cache do context 'GitHub Enterprise' do
it 'returns the ID of the ghost user' do let(:project) { double(:project, import_url: 'http://t0ken@github.another-domain.com/repo-org/repo.git') }
it 'returns a new Client with a custom token' do
expect(described_class::Client)
.to receive(:new)
.with('123', host: 'http://github.another-domain.com/api/v3', parallel: true)
described_class.new_client_for(project, token: '123')
end
it 'returns a new Client with a token stored in the import data' do
import_data = double(:import_data, credentials: { user: '123' })
expect(project)
.to receive(:import_data)
.and_return(import_data)
expect(described_class::Client)
.to receive(:new)
.with('123', host: 'http://github.another-domain.com/api/v3', parallel: true)
described_class.new_client_for(project)
end
it 'returns the ID of the ghost user', :clean_gitlab_redis_cache do
expect(described_class.ghost_user_id).to eq(User.ghost.id) expect(described_class.ghost_user_id).to eq(User.ghost.id)
end end
it 'caches the ghost user ID' do it 'caches the ghost user ID', :clean_gitlab_redis_cache do
expect(Gitlab::Cache::Import::Caching) expect(Gitlab::Cache::Import::Caching)
.to receive(:write) .to receive(:write)
.once .once
...@@ -44,5 +83,9 @@ RSpec.describe Gitlab::GithubImport do ...@@ -44,5 +83,9 @@ RSpec.describe Gitlab::GithubImport do
described_class.ghost_user_id described_class.ghost_user_id
end end
end end
it 'formats the import url' do
expect(described_class.formatted_import_url(project)).to eq('http://github.another-domain.com/api/v3')
end
end end
end end
...@@ -57,6 +57,22 @@ RSpec.describe API::ImportGithub do ...@@ -57,6 +57,22 @@ RSpec.describe API::ImportGithub do
expect(json_response['name']).to eq(project.name) expect(json_response['name']).to eq(project.name)
end end
it 'returns 201 response when the project is imported successfully from GHE' do
allow(Gitlab::LegacyGithubImport::ProjectCreator)
.to receive(:new).with(provider_repo, provider_repo.name, user.namespace, user, access_params, type: provider)
.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,
github_hostname: "https://github.somecompany.com/"
}
expect(response).to have_gitlab_http_status(:created)
expect(json_response).to be_a Hash
expect(json_response['name']).to eq(project.name)
end
it 'returns 422 response when user can not create projects in the chosen namespace' do it 'returns 422 response when user can not create projects in the chosen namespace' do
other_namespace = create(:group, name: 'other_namespace') other_namespace = create(:group, name: 'other_namespace')
......
...@@ -26,7 +26,7 @@ RSpec.describe Gitlab::GithubImport::ReschedulingMethods do ...@@ -26,7 +26,7 @@ RSpec.describe Gitlab::GithubImport::ReschedulingMethods do
end end
context 'with an existing project' do context 'with an existing project' do
let(:project) { create(:project) } let(:project) { create(:project, import_url: 'https://t0ken@github.com/repo/repo.git') }
it 'notifies any waiters upon successfully importing the data' do it 'notifies any waiters upon successfully importing the data' do
expect(worker) expect(worker)
......
...@@ -9,6 +9,8 @@ RSpec.describe Gitlab::GithubImport::StageMethods do ...@@ -9,6 +9,8 @@ RSpec.describe Gitlab::GithubImport::StageMethods do
end end
describe '#perform' do describe '#perform' do
let(:project) { create(:project, import_url: 'https://t0ken@github.com/repo/repo.git') }
it 'returns if no project could be found' do it 'returns if no project could be found' do
expect(worker).not_to receive(:try_import) expect(worker).not_to receive(:try_import)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment