Merge branch 'bvl-improve-bare-project-import' into 'master'
Make the import take subgroups into account Closes #36664 and #30546 See merge request !13670
This commit is contained in:
commit
9ec60b5030
8 changed files with 272 additions and 81 deletions
45
app/services/groups/nested_create_service.rb
Normal file
45
app/services/groups/nested_create_service.rb
Normal file
|
@ -0,0 +1,45 @@
|
||||||
|
module Groups
|
||||||
|
class NestedCreateService < Groups::BaseService
|
||||||
|
attr_reader :group_path
|
||||||
|
|
||||||
|
def initialize(user, params)
|
||||||
|
@current_user, @params = user, params.dup
|
||||||
|
|
||||||
|
@group_path = @params.delete(:group_path)
|
||||||
|
end
|
||||||
|
|
||||||
|
def execute
|
||||||
|
return nil unless group_path
|
||||||
|
|
||||||
|
if group = Group.find_by_full_path(group_path)
|
||||||
|
return group
|
||||||
|
end
|
||||||
|
|
||||||
|
create_group_path
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def create_group_path
|
||||||
|
group_path_segments = group_path.split('/')
|
||||||
|
|
||||||
|
last_group = nil
|
||||||
|
partial_path_segments = []
|
||||||
|
while subgroup_name = group_path_segments.shift
|
||||||
|
partial_path_segments << subgroup_name
|
||||||
|
partial_path = partial_path_segments.join('/')
|
||||||
|
|
||||||
|
new_params = params.reverse_merge(
|
||||||
|
path: subgroup_name,
|
||||||
|
name: subgroup_name,
|
||||||
|
parent: last_group
|
||||||
|
)
|
||||||
|
new_params[:visibility_level] ||= Gitlab::CurrentSettings.current_application_settings.default_group_visibility
|
||||||
|
|
||||||
|
last_group = Group.find_by_full_path(partial_path) || Groups::CreateService.new(current_user, new_params).execute
|
||||||
|
end
|
||||||
|
|
||||||
|
last_group
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
title: 'Improve bare project import: Allow subgroups, take default visibility level
|
||||||
|
into account'
|
||||||
|
merge_request: 13670
|
||||||
|
author:
|
||||||
|
type: fixed
|
|
@ -107,7 +107,7 @@ module Github
|
||||||
# this means that repo has wiki enabled, but have no pages. So,
|
# this means that repo has wiki enabled, but have no pages. So,
|
||||||
# we can skip the import.
|
# we can skip the import.
|
||||||
if e.message !~ /repository not exported/
|
if e.message !~ /repository not exported/
|
||||||
errors(:wiki, wiki_url, e.message)
|
error(:wiki, wiki_url, e.message)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
96
lib/gitlab/bare_repository_importer.rb
Normal file
96
lib/gitlab/bare_repository_importer.rb
Normal file
|
@ -0,0 +1,96 @@
|
||||||
|
module Gitlab
|
||||||
|
class BareRepositoryImporter
|
||||||
|
NoAdminError = Class.new(StandardError)
|
||||||
|
|
||||||
|
def self.execute
|
||||||
|
Gitlab.config.repositories.storages.each do |storage_name, repository_storage|
|
||||||
|
git_base_path = repository_storage['path']
|
||||||
|
repos_to_import = Dir.glob(git_base_path + '/**/*.git')
|
||||||
|
|
||||||
|
repos_to_import.each do |repo_path|
|
||||||
|
if repo_path.end_with?('.wiki.git')
|
||||||
|
log " * Skipping wiki repo"
|
||||||
|
next
|
||||||
|
end
|
||||||
|
|
||||||
|
log "Processing #{repo_path}".color(:yellow)
|
||||||
|
|
||||||
|
repo_relative_path = repo_path[repository_storage['path'].length..-1]
|
||||||
|
.sub(/^\//, '') # Remove leading `/`
|
||||||
|
.sub(/\.git$/, '') # Remove `.git` at the end
|
||||||
|
new(storage_name, repo_relative_path).create_project_if_needed
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
attr_reader :storage_name, :full_path, :group_path, :project_path, :user
|
||||||
|
delegate :log, to: :class
|
||||||
|
|
||||||
|
def initialize(storage_name, repo_path)
|
||||||
|
@storage_name = storage_name
|
||||||
|
@full_path = repo_path
|
||||||
|
|
||||||
|
unless @user = User.admins.order_id_asc.first
|
||||||
|
raise NoAdminError.new('No admin user found to import repositories')
|
||||||
|
end
|
||||||
|
|
||||||
|
@group_path, @project_path = File.split(repo_path)
|
||||||
|
@group_path = nil if @group_path == '.'
|
||||||
|
end
|
||||||
|
|
||||||
|
def create_project_if_needed
|
||||||
|
if project = Project.find_by_full_path(full_path)
|
||||||
|
log " * #{project.name} (#{full_path}) exists"
|
||||||
|
return project
|
||||||
|
end
|
||||||
|
|
||||||
|
create_project
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def create_project
|
||||||
|
group = find_or_create_group
|
||||||
|
|
||||||
|
project_params = {
|
||||||
|
name: project_path,
|
||||||
|
path: project_path,
|
||||||
|
repository_storage: storage_name,
|
||||||
|
namespace_id: group&.id
|
||||||
|
}
|
||||||
|
|
||||||
|
project = Projects::CreateService.new(user, project_params).execute
|
||||||
|
|
||||||
|
if project.persisted?
|
||||||
|
log " * Created #{project.name} (#{full_path})".color(:green)
|
||||||
|
ProjectCacheWorker.perform_async(project.id)
|
||||||
|
else
|
||||||
|
log " * Failed trying to create #{project.name} (#{full_path})".color(:red)
|
||||||
|
log " Errors: #{project.errors.messages}".color(:red)
|
||||||
|
end
|
||||||
|
|
||||||
|
project
|
||||||
|
end
|
||||||
|
|
||||||
|
def find_or_create_group
|
||||||
|
return nil unless group_path
|
||||||
|
|
||||||
|
if namespace = Namespace.find_by_full_path(group_path)
|
||||||
|
log " * Namespace #{group_path} exists.".color(:green)
|
||||||
|
return namespace
|
||||||
|
end
|
||||||
|
|
||||||
|
log " * Creating Group: #{group_path}"
|
||||||
|
Groups::NestedCreateService.new(user, group_path: group_path).execute
|
||||||
|
end
|
||||||
|
|
||||||
|
# This is called from within a rake task only used by Admins, so allow writing
|
||||||
|
# to STDOUT
|
||||||
|
#
|
||||||
|
# rubocop:disable Rails/Output
|
||||||
|
def self.log(message)
|
||||||
|
puts message
|
||||||
|
end
|
||||||
|
# rubocop:enable Rails/Output
|
||||||
|
end
|
||||||
|
end
|
|
@ -9,6 +9,7 @@ namespace :gitlab do
|
||||||
# * The project owner will set to the first administator of the system
|
# * The project owner will set to the first administator of the system
|
||||||
# * Existing projects will be skipped
|
# * Existing projects will be skipped
|
||||||
#
|
#
|
||||||
|
#
|
||||||
desc "GitLab | Import bare repositories from repositories -> storages into GitLab project instance"
|
desc "GitLab | Import bare repositories from repositories -> storages into GitLab project instance"
|
||||||
task repos: :environment do
|
task repos: :environment do
|
||||||
if Project.current_application_settings.hashed_storage_enabled
|
if Project.current_application_settings.hashed_storage_enabled
|
||||||
|
@ -17,69 +18,7 @@ namespace :gitlab do
|
||||||
exit 1
|
exit 1
|
||||||
end
|
end
|
||||||
|
|
||||||
Gitlab.config.repositories.storages.each_value do |repository_storage|
|
Gitlab::BareRepositoryImporter.execute
|
||||||
git_base_path = repository_storage['path']
|
|
||||||
repos_to_import = Dir.glob(git_base_path + '/**/*.git')
|
|
||||||
|
|
||||||
repos_to_import.each do |repo_path|
|
|
||||||
# strip repo base path
|
|
||||||
repo_path[0..git_base_path.length] = ''
|
|
||||||
|
|
||||||
path = repo_path.sub(/\.git$/, '')
|
|
||||||
group_name, name = File.split(path)
|
|
||||||
group_name = nil if group_name == '.'
|
|
||||||
|
|
||||||
puts "Processing #{repo_path}".color(:yellow)
|
|
||||||
|
|
||||||
if path.end_with?('.wiki')
|
|
||||||
puts " * Skipping wiki repo"
|
|
||||||
next
|
|
||||||
end
|
|
||||||
|
|
||||||
project = Project.find_by_full_path(path)
|
|
||||||
|
|
||||||
if project
|
|
||||||
puts " * #{project.name} (#{repo_path}) exists"
|
|
||||||
else
|
|
||||||
user = User.admins.reorder("id").first
|
|
||||||
|
|
||||||
project_params = {
|
|
||||||
name: name,
|
|
||||||
path: name
|
|
||||||
}
|
|
||||||
|
|
||||||
# find group namespace
|
|
||||||
if group_name
|
|
||||||
group = Namespace.find_by(path: group_name)
|
|
||||||
# create group namespace
|
|
||||||
unless group
|
|
||||||
group = Group.new(name: group_name)
|
|
||||||
group.path = group_name
|
|
||||||
group.owner = user
|
|
||||||
if group.save
|
|
||||||
puts " * Created Group #{group.name} (#{group.id})".color(:green)
|
|
||||||
else
|
|
||||||
puts " * Failed trying to create group #{group.name}".color(:red)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
# set project group
|
|
||||||
project_params[:namespace_id] = group.id
|
|
||||||
end
|
|
||||||
|
|
||||||
project = Projects::CreateService.new(user, project_params).execute
|
|
||||||
|
|
||||||
if project.persisted?
|
|
||||||
puts " * Created #{project.name} (#{repo_path})".color(:green)
|
|
||||||
ProjectCacheWorker.perform_async(project.id)
|
|
||||||
else
|
|
||||||
puts " * Failed trying to create #{project.name} (#{repo_path})".color(:red)
|
|
||||||
puts " Errors: #{project.errors.messages}".color(:red)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
puts "Done!".color(:green)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -72,23 +72,7 @@ class GithubImport
|
||||||
return @current_user.namespace if names == @current_user.namespace_path
|
return @current_user.namespace if names == @current_user.namespace_path
|
||||||
return @current_user.namespace unless @current_user.can_create_group?
|
return @current_user.namespace unless @current_user.can_create_group?
|
||||||
|
|
||||||
full_path_namespace = Namespace.find_by_full_path(names)
|
Groups::NestedCreateService.new(@current_user, group_path: names).execute
|
||||||
|
|
||||||
return full_path_namespace if full_path_namespace
|
|
||||||
|
|
||||||
names.split('/').inject(nil) do |parent, name|
|
|
||||||
begin
|
|
||||||
namespace = Group.create!(name: name,
|
|
||||||
path: name,
|
|
||||||
owner: @current_user,
|
|
||||||
parent: parent)
|
|
||||||
namespace.add_owner(@current_user)
|
|
||||||
|
|
||||||
namespace
|
|
||||||
rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid
|
|
||||||
Namespace.where(parent: parent).find_by_path_or_name(name)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def full_path_namespace(names)
|
def full_path_namespace(names)
|
||||||
|
|
68
spec/lib/gitlab/bare_repository_importer_spec.rb
Normal file
68
spec/lib/gitlab/bare_repository_importer_spec.rb
Normal file
|
@ -0,0 +1,68 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe Gitlab::BareRepositoryImporter, repository: true do
|
||||||
|
subject(:importer) { described_class.new('default', project_path) }
|
||||||
|
let(:project_path) { 'a-group/a-sub-group/a-project' }
|
||||||
|
let!(:admin) { create(:admin) }
|
||||||
|
|
||||||
|
before do
|
||||||
|
allow(described_class).to receive(:log)
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '.execute' do
|
||||||
|
it 'creates a project for a repository in storage' do
|
||||||
|
FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git"))
|
||||||
|
fake_importer = double
|
||||||
|
|
||||||
|
expect(described_class).to receive(:new).with('default', project_path)
|
||||||
|
.and_return(fake_importer)
|
||||||
|
expect(fake_importer).to receive(:create_project_if_needed)
|
||||||
|
|
||||||
|
described_class.execute
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'skips wiki repos' do
|
||||||
|
FileUtils.mkdir_p(File.join(TestEnv.repos_path, 'the-group', 'the-project.wiki.git'))
|
||||||
|
|
||||||
|
expect(described_class).to receive(:log).with(' * Skipping wiki repo')
|
||||||
|
expect(described_class).not_to receive(:new)
|
||||||
|
|
||||||
|
described_class.execute
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#initialize' do
|
||||||
|
context 'without admin users' do
|
||||||
|
let(:admin) { nil }
|
||||||
|
|
||||||
|
it 'raises an error' do
|
||||||
|
expect { importer }.to raise_error(Gitlab::BareRepositoryImporter::NoAdminError)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#create_project_if_needed' do
|
||||||
|
it 'starts an import for a project that did not exist' do
|
||||||
|
expect(importer).to receive(:create_project)
|
||||||
|
|
||||||
|
importer.create_project_if_needed
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'skips importing when the project already exists' do
|
||||||
|
group = create(:group, path: 'a-group')
|
||||||
|
subgroup = create(:group, path: 'a-sub-group', parent: group)
|
||||||
|
project = create(:project, path: 'a-project', namespace: subgroup)
|
||||||
|
|
||||||
|
expect(importer).not_to receive(:create_project)
|
||||||
|
expect(importer).to receive(:log).with(" * #{project.name} (a-group/a-sub-group/a-project) exists")
|
||||||
|
|
||||||
|
importer.create_project_if_needed
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates a project with the correct path in the database' do
|
||||||
|
importer.create_project_if_needed
|
||||||
|
|
||||||
|
expect(Project.find_by_full_path(project_path)).not_to be_nil
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
53
spec/services/groups/nested_create_service_spec.rb
Normal file
53
spec/services/groups/nested_create_service_spec.rb
Normal file
|
@ -0,0 +1,53 @@
|
||||||
|
require 'spec_helper'
|
||||||
|
|
||||||
|
describe Groups::NestedCreateService do
|
||||||
|
let(:user) { create(:user) }
|
||||||
|
let(:params) { { group_path: 'a-group/a-sub-group' } }
|
||||||
|
|
||||||
|
subject(:service) { described_class.new(user, params) }
|
||||||
|
|
||||||
|
describe "#execute" do
|
||||||
|
it 'returns the group if it already existed' do
|
||||||
|
parent = create(:group, path: 'a-group', owner: user)
|
||||||
|
child = create(:group, path: 'a-sub-group', parent: parent, owner: user)
|
||||||
|
|
||||||
|
expect(service.execute).to eq(child)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'reuses a parent if it already existed' do
|
||||||
|
parent = create(:group, path: 'a-group')
|
||||||
|
parent.add_owner(user)
|
||||||
|
|
||||||
|
expect(service.execute.parent).to eq(parent)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates group and subgroup in the database' do
|
||||||
|
service.execute
|
||||||
|
|
||||||
|
parent = Group.find_by_full_path('a-group')
|
||||||
|
child = parent.children.find_by(path: 'a-sub-group')
|
||||||
|
|
||||||
|
expect(parent).not_to be_nil
|
||||||
|
expect(child).not_to be_nil
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'creates the group with correct visibility level' do
|
||||||
|
allow(Gitlab::CurrentSettings.current_application_settings)
|
||||||
|
.to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL }
|
||||||
|
|
||||||
|
group = service.execute
|
||||||
|
|
||||||
|
expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'adding a visibility level ' do
|
||||||
|
let(:params) { { group_path: 'a-group/a-sub-group', visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
|
||||||
|
|
||||||
|
it 'overwrites the visibility level' do
|
||||||
|
group = service.execute
|
||||||
|
|
||||||
|
expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
Loading…
Reference in a new issue