diff --git a/app/models/bulk_imports/entity.rb b/app/models/bulk_imports/entity.rb index d5d1d38784ea6d82ac69ef2775ecb7cc65d85a45..b3540917197dff514e85c1dddde21536436add81 100644 --- a/app/models/bulk_imports/entity.rb +++ b/app/models/bulk_imports/entity.rb @@ -56,7 +56,7 @@ class BulkImports::Entity < ApplicationRecord validates :destination_namespace, presence: true, - if: :project + if: :project? validate :validate_parent_is_a_group, if: :parent validate :validate_imported_entity_type diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index 5d4f84783776a4d7a4d4fc156acca6996154a212..61bbb186e6fd010fd6dead4278082ea8acc79cae 100644 --- a/app/policies/group_policy.rb +++ b/app/policies/group_policy.rb @@ -191,6 +191,7 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy enable :destroy_package enable :admin_package enable :create_projects + enable :import_projects enable :admin_pipeline enable :admin_build enable :add_cluster @@ -261,14 +262,20 @@ class GroupPolicy < Namespaces::GroupProjectNamespaceSharedPolicy end.enable :change_share_with_group_lock rule { developer & developer_maintainer_access }.enable :create_projects - rule { create_projects_disabled }.prevent :create_projects + rule { create_projects_disabled }.policy do + prevent :create_projects + prevent :import_projects + end rule { owner | admin }.policy do enable :owner_access enable :read_statistics end - rule { maintainer & can?(:create_projects) }.enable :transfer_projects + rule { maintainer & can?(:create_projects) }.policy do + enable :transfer_projects + enable :import_projects + end rule { read_package_registry_deploy_token }.policy do enable :read_package diff --git a/app/services/bulk_imports/create_service.rb b/app/services/bulk_imports/create_service.rb index aec32209b19eea0550045ce81c3d73da3e32a4b6..4c9c59ac50407e9373a097317a97c5f41d265788 100644 --- a/app/services/bulk_imports/create_service.rb +++ b/app/services/bulk_imports/create_service.rb @@ -2,7 +2,7 @@ # Entry point of the BulkImport/Direct Transfer feature. # This service receives a Gitlab Instance connection params -# and a list of groups to be imported. +# and a list of groups or projects to be imported. # # Process topography: # @@ -15,16 +15,17 @@ # P1 (sync) # # - Create a BulkImport record -# - Create a BulkImport::Entity for each group to be imported -# - Enqueue a BulkImportWorker job (P2) to import the given groups (entities) +# - Create a BulkImport::Entity for each group or project (entities) to be imported +# - Enqueue a BulkImportWorker job (P2) to import the given entity # # Pn (async) # # - For each group to be imported (BulkImport::Entity.with_status(:created)) # - Import the group data # - Create entities for each subgroup of the imported group -# - Enqueue a BulkImports::CreateService job (Pn) to import the new entities (subgroups) -# +# - Create entities for each project of the imported group +# - Enqueue a BulkImportWorker job (Pn) to import the new entities + module BulkImports class CreateService ENTITY_TYPES_MAPPING = { @@ -84,7 +85,7 @@ def create_bulk_import Array.wrap(params).each do |entity_params| track_access_level(entity_params) - validate_destination_namespace(entity_params[:destination_namespace]) + validate_destination_namespace(entity_params) validate_destination_slug(entity_params[:destination_slug] || entity_params[:destination_name]) validate_destination_full_path(entity_params) @@ -137,10 +138,18 @@ def source_equals_destination? credentials[:url].starts_with?(Settings.gitlab.base_url) end - def validate_destination_namespace(destination_namespace) - return if destination_namespace =~ Gitlab::Regex.bulk_import_destination_namespace_path_regex + def validate_destination_namespace(entity_params) + destination_namespace = entity_params[:destination_namespace] + source_type = entity_params[:source_type] - raise BulkImports::Error.destination_namespace_validation_failure + return if destination_namespace.blank? + + group = Group.find_by_full_path(destination_namespace) + if group.nil? || + (source_type == 'group_entity' && !current_user.can?(:create_subgroup, group)) || + (source_type == 'project_entity' && !current_user.can?(:import_projects, group)) + raise BulkImports::Error.destination_namespace_validation_failure(destination_namespace) + end end def validate_destination_slug(destination_slug) diff --git a/doc/api/bulk_imports.md b/doc/api/bulk_imports.md index 4816a5d506649335c1b3c15c4cdf03b054a0ce09..6070580ade29c8dd986900b08d5beed18f325fcd 100644 --- a/doc/api/bulk_imports.md +++ b/doc/api/bulk_imports.md @@ -18,7 +18,8 @@ prerequisites for [migrating groups by direct transfer](../user/group/import/ind ## Start a new group migration -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66353) in GitLab 14.2. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66353) in GitLab 14.2. +> - `project_entity` source type [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/390515) in GitLab 15.11. ```plaintext POST /bulk_imports @@ -30,7 +31,7 @@ POST /bulk_imports | `configuration[url]` | String | yes | Source GitLab instance URL. | | `configuration[access_token]` | String | yes | Access token to the source GitLab instance. | | `entities` | Array | yes | List of entities to import. | -| `entities[source_type]` | String | yes | Source entity type (only `group_entity` is supported). | +| `entities[source_type]` | String | yes | Source entity type. Valid values are `group_entity` (GitLab 14.2 and later) and `project_entity` (GitLab 15.11 and later). | | `entities[source_full_path]` | String | yes | Source full path of the entity to import. | | `entities[destination_name]` | String | yes | Deprecated: Use :destination_slug instead. Destination slug for the entity. | | `entities[destination_slug]` | String | yes | Destination slug for the entity. | diff --git a/doc/user/group/import/index.md b/doc/user/group/import/index.md index 433c842384c63c5ba95c647b633828a5a7b3949b..242ee1ac3d6e2da167a993e3d9e69c9e57b46408 100644 --- a/doc/user/group/import/index.md +++ b/doc/user/group/import/index.md @@ -225,6 +225,7 @@ Group items that are migrated to the destination GitLab instance include: > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/267945) in GitLab 14.4 [with a flag](../../feature_flags.md) named `bulk_import_projects`. Disabled by default. > - [Enabled on GitLab.com](https://gitlab.com/gitlab-org/gitlab/-/issues/339941) in GitLab 15.6. > - `bulk_import_projects` feature flag [removed](https://gitlab.com/gitlab-org/gitlab/-/issues/339941) in GitLab 15.10. +> - Project-only migrations using API [added](https://gitlab.com/gitlab-org/gitlab/-/issues/390515) in GitLab 15.11. If you choose to migrate projects when you [select groups to migrate](#select-the-groups-and-projects-to-import), project items are migrated with the projects. @@ -246,6 +247,9 @@ specific project item is migrated: Any other project items are **not** migrated. +If you choose not to migrate projects along with groups or if you want to retry a project migration, you can +initiate project-only migrations using the [API](../../../api/bulk_imports.md). + WARNING: Migrating projects when migrating groups by direct transfer is in [beta](../../../policy/alpha-beta-support.md#beta-features) and is not ready for production use. diff --git a/lib/api/bulk_imports.rb b/lib/api/bulk_imports.rb index e3dc9ea52cb9afca6b732a4ed8b0fece58cdcbeb..b4ace6cd6bcd273cda0f3918f5e99fe7682bf5b6 100644 --- a/lib/api/bulk_imports.rb +++ b/lib/api/bulk_imports.rb @@ -59,8 +59,8 @@ def bulk_import_entity requires :entities, type: Array, desc: 'List of entities to import' do requires :source_type, type: String, - desc: 'Source entity type (only `group_entity` is supported)', - values: %w[group_entity] + desc: 'Source entity type', + values: %w[group_entity project_entity] requires :source_full_path, type: String, desc: 'Relative path of the source entity to import', diff --git a/lib/bulk_imports/error.rb b/lib/bulk_imports/error.rb index 616b58d1852cd2b7c2e5218967a5b400470b73e0..c40b4bc7f34b018fc10643be86cc2065725eaaa6 100644 --- a/lib/bulk_imports/error.rb +++ b/lib/bulk_imports/error.rb @@ -15,9 +15,8 @@ def self.invalid_url self.new("Invalid source URL. Enter only the base URL of the source GitLab instance.") end - def self.destination_namespace_validation_failure - self.new("Import failed. Destination group or subgroup path " \ - "#{Gitlab::Regex.bulk_import_destination_namespace_path_regex_message}") + def self.destination_namespace_validation_failure(destination_namespace) + self.new("Import failed. Destination '#{destination_namespace}' is invalid, or you don't have permission.") end def self.destination_slug_validation_failure diff --git a/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb b/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb index 72b32ad5b3b90009d32314eb0e308e030c2f4bd2..1e025e91038eb5f2673784d44f261ded3c60cd94 100644 --- a/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb +++ b/lib/bulk_imports/projects/transformers/project_attributes_transformer.rb @@ -21,7 +21,7 @@ def transform(context, data) project[:created_at] = data['created_at'] project[:import_type] = PROJECT_IMPORT_TYPE project[:visibility_level] = visibility_level(entity, namespace, data['visibility']) - project[:namespace_id] = namespace.id if namespace + project[:namespace_id] = namespace.id project.with_indifferent_access end diff --git a/spec/lib/bulk_imports/groups/transformers/group_attributes_transformer_spec.rb b/spec/lib/bulk_imports/groups/transformers/group_attributes_transformer_spec.rb index ab0d57f574b4f42c80d3391c01c8713d88ca802c..9782f2aac2783cecaf4ffa68b5d856059ca4d54c 100644 --- a/spec/lib/bulk_imports/groups/transformers/group_attributes_transformer_spec.rb +++ b/spec/lib/bulk_imports/groups/transformers/group_attributes_transformer_spec.rb @@ -181,6 +181,49 @@ subject(:transformed_data) { described_class.new.transform(context, data) } include_examples 'visibility level settings' + + context 'when destination is blank' do + let(:destination_namespace) { '' } + + context 'when visibility level is public' do + let(:data) { { 'visibility' => 'public' } } + + it 'sets visibility level to public' do + expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PUBLIC) + end + end + + context 'when when visibility level is internal' do + let(:data) { { 'visibility' => 'internal' } } + + it 'sets visibility level to internal' do + expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::INTERNAL) + end + end + + context 'when private' do + let(:data) { { 'visibility' => 'private' } } + + it 'sets visibility level to private' do + expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) + end + end + + context 'when visibility level is restricted' do + let(:data) { { 'visibility' => 'internal' } } + + it 'sets visibility level to private' do + stub_application_setting( + restricted_visibility_levels: [ + Gitlab::VisibilityLevel::INTERNAL, + Gitlab::VisibilityLevel::PUBLIC + ] + ) + + expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) + end + end + end end end end diff --git a/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb b/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb index 40ad4ee8be52f98fdeecda98d5ab0b919504b10c..0e3d8b36fb27a7cb1ed0eeefb83d235673ff271a 100644 --- a/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb +++ b/spec/lib/bulk_imports/projects/transformers/project_attributes_transformer_spec.rb @@ -46,27 +46,8 @@ end describe 'namespace_id' do - context 'when destination namespace is present' do - it 'adds namespace_id' do - expect(transformed_data[:namespace_id]).to eq(destination_group.id) - end - end - - context 'when destination namespace is blank' do - it 'does not add namespace_id key' do - entity = create( - :bulk_import_entity, - source_type: :project_entity, - bulk_import: bulk_import, - source_full_path: 'source/full/path', - destination_slug: 'Destination-Project-Name', - destination_namespace: '' - ) - - context = double(entity: entity) - - expect(described_class.new.transform(context, data)).not_to have_key(:namespace_id) - end + it 'adds namespace_id' do + expect(transformed_data[:namespace_id]).to eq(destination_group.id) end end diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 1103ea4eaad4dc331bff2f67296262f5af4fc341..a133ef03d9bf257adbd30ae1a2f487401805db32 100644 --- a/spec/policies/group_policy_spec.rb +++ b/spec/policies/group_policy_spec.rb @@ -670,6 +670,124 @@ end end + context 'import_projects' do + before do + group.update!(project_creation_level: project_creation_level) + end + + context 'when group has no project creation level set' do + let(:project_creation_level) { nil } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + + context 'when group has project creation level set to no one' do + let(:project_creation_level) { ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_disallowed(:import_projects) } + end + end + + context 'when group has project creation level set to maintainer only' do + let(:project_creation_level) { ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + + context 'when group has project creation level set to developers + maintainer' do + let(:project_creation_level) { ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS } + + context 'reporter' do + let(:current_user) { reporter } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'developer' do + let(:current_user) { developer } + + it { is_expected.to be_disallowed(:import_projects) } + end + + context 'maintainer' do + let(:current_user) { maintainer } + + it { is_expected.to be_allowed(:import_projects) } + end + + context 'owner' do + let(:current_user) { owner } + + it { is_expected.to be_allowed(:import_projects) } + end + end + end + context 'create_subgroup' do context 'when group has subgroup creation level set to owner' do before do diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index ff76b9369af658c1ec293baa57508a9743f5b2ee..b159d4ad445bc255917e446e168c567f38cbd835 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -75,6 +75,8 @@ end describe 'POST /bulk_imports' do + let_it_be(:destination_namespace) { create(:group) } + let(:request) { post api('/bulk_imports', user), params: params } let(:destination_param) { { destination_slug: 'destination_slug' } } let(:params) do @@ -87,7 +89,7 @@ { source_type: 'group_entity', source_full_path: 'full_path', - destination_namespace: 'destination_namespace' + destination_namespace: destination_namespace.path }.merge(destination_param) ] } @@ -108,6 +110,8 @@ end stub_request(:get, "http://gitlab.example/api/v4/#{source_entity_type}/#{source_entity_identifier}/export_relations/status?page=1&per_page=30&private_token=access_token") .to_return(status: 200, body: "", headers: {}) + + destination_namespace.add_owner(user) end shared_examples 'starting a new migration' do @@ -197,7 +201,7 @@ { source_type: 'group_entity', source_full_path: 'full_path', - destination_namespace: 'destination_namespace' + destination_namespace: destination_namespace.path } ] } @@ -223,17 +227,13 @@ end end - context 'when the destination_namespace is invalid' do + context 'when the destination_namespace does not exist' do it 'returns invalid error' do - params[:entities][0][:destination_namespace] = "?not a destination-namespace" + params[:entities][0][:destination_namespace] = "invalid-destination-namespace" request - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq("entities[0][destination_namespace] must have a relative " \ - "path structure with no HTTP protocol characters, or leading or " \ - "trailing forward slashes. Path segments must not start or " \ - "end with a special character, and must not contain " \ - "consecutive special characters.") + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq("Import failed. Destination 'invalid-destination-namespace' is invalid, or you don't have permission.") end end diff --git a/spec/services/bulk_imports/create_service_spec.rb b/spec/services/bulk_imports/create_service_spec.rb index c68030a89a82813bd931ad7a7e4b4d9a6a097874..ff4afd6abd0061b6179175896dde2ac5608fcb13 100644 --- a/spec/services/bulk_imports/create_service_spec.rb +++ b/spec/services/bulk_imports/create_service_spec.rb @@ -135,10 +135,11 @@ body: { 'scopes' => ['api'] }.to_json, headers: { 'Content-Type' => 'application/json' } ) + + parent_group.add_owner(user) end it 'creates bulk import' do - parent_group.add_owner(user) expect { subject.execute }.to change { BulkImport.count }.by(1) last_bulk_import = BulkImport.last @@ -231,10 +232,11 @@ status: 200 ) end + + parent_group.add_owner(user) end it 'creates bulk import' do - parent_group.add_owner(user) expect { subject.execute }.to change { BulkImport.count }.by(1) last_bulk_import = BulkImport.last @@ -341,6 +343,8 @@ end it 'defines access_level as not a member' do + parent_group.members.delete_all + subject.execute expect_snowplow_event( category: 'BulkImports::CreateService', @@ -403,7 +407,7 @@ end end - describe '.validate_setting_enabled!' do + describe '#validate_setting_enabled!' do let(:entity_source_id) { 'gid://gitlab/Model/12345' } let(:graphql_client) { instance_double(BulkImports::Clients::Graphql) } let(:http_client) { instance_double(BulkImports::Clients::HTTP) } @@ -502,15 +506,15 @@ end end - describe '.validate_destination_namespace' do - context 'when the destination_namespace is invalid' do + describe '#validate_destination_namespace' do + context 'when the destination_namespace does not exist' do let(:params) do [ { source_type: 'group_entity', source_full_path: 'full/path/to/source', destination_slug: 'destination-slug', - destination_namespace: '---destination----namespace---', + destination_namespace: 'destination-namespace', migrate_projects: migrate_projects } ] @@ -522,17 +526,62 @@ expect(result).to be_a(ServiceResponse) expect(result).to be_error expect(result.message) - .to eq( - "Import failed. Destination group or subgroup path " \ - "must have a relative path structure with no HTTP protocol characters, or leading " \ - "or trailing forward slashes. Path segments must not start or end with a special " \ - "character, and must not contain consecutive special characters." - ) + .to eq("Import failed. Destination 'destination-namespace' is invalid, or you don't have permission.") + end + end + + context 'when the user does not have permission to create subgroups' do + let(:params) do + [ + { + source_type: 'group_entity', + source_full_path: 'full/path/to/source', + destination_slug: 'destination-slug', + destination_namespace: parent_group.path, + migrate_projects: migrate_projects + } + ] + end + + it 'returns ServiceResponse with an error message' do + parent_group.members.delete_all + + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result).to be_error + expect(result.message) + .to eq("Import failed. Destination '#{parent_group.path}' is invalid, or you don't have permission.") + end + end + + context 'when the user does not have permission to create projects' do + let(:params) do + [ + { + source_type: 'project_entity', + source_full_path: 'full/path/to/source', + destination_slug: 'destination-slug', + destination_namespace: parent_group.path, + migrate_projects: migrate_projects + } + ] + end + + it 'returns ServiceResponse with an error message' do + parent_group.members.delete_all + + result = subject.execute + + expect(result).to be_a(ServiceResponse) + expect(result).to be_error + expect(result.message) + .to eq("Import failed. Destination '#{parent_group.path}' is invalid, or you don't have permission.") end end end - describe '.validate_destination_slug' do + describe '#validate_destination_slug' do context 'when the destination_slug is invalid' do let(:params) do [ @@ -540,7 +589,7 @@ source_type: 'group_entity', source_full_path: 'full/path/to/source', destination_slug: 'destin-*-ation-slug', - destination_namespace: 'destination_namespace', + destination_namespace: parent_group.path, migrate_projects: migrate_projects } ] @@ -561,7 +610,7 @@ end end - describe '.validate_destination_full_path' do + describe '#validate_destination_full_path' do context 'when the source_type is a group' do context 'when the provided destination_slug already exists in the destination_namespace' do let_it_be(:existing_subgroup) { create(:group, path: 'existing-subgroup', parent_id: parent_group.id ) } @@ -657,6 +706,8 @@ end it 'returns ServiceResponse with an error message' do + existing_group.add_owner(user) + result = subject.execute expect(result).to be_a(ServiceResponse) @@ -684,6 +735,8 @@ end it 'returns success ServiceResponse' do + existing_group.add_owner(user) + result = subject.execute expect(result).to be_a(ServiceResponse) diff --git a/spec/support/shared_examples/bulk_imports/visibility_level_examples.rb b/spec/support/shared_examples/bulk_imports/visibility_level_examples.rb index 40e9726f89ca599b8726ba841adf27a0faa0371a..02eae250e6add1b57cd47307faca66f17cc0184c 100644 --- a/spec/support/shared_examples/bulk_imports/visibility_level_examples.rb +++ b/spec/support/shared_examples/bulk_imports/visibility_level_examples.rb @@ -27,14 +27,6 @@ expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) end end - - context 'when destination is blank' do - let(:destination_namespace) { '' } - - it 'sets visibility level to public' do - expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PUBLIC) - end - end end context 'when internal' do @@ -63,27 +55,6 @@ expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) end end - - context 'when destination is blank' do - let(:destination_namespace) { '' } - - it 'sets visibility level to internal' do - expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::INTERNAL) - end - - context 'when visibility level is restricted' do - it 'sets visibility level to private' do - stub_application_setting( - restricted_visibility_levels: [ - Gitlab::VisibilityLevel::INTERNAL, - Gitlab::VisibilityLevel::PUBLIC - ] - ) - - expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) - end - end - end end context 'when private' do @@ -112,13 +83,5 @@ expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) end end - - context 'when destination is blank' do - let(:destination_namespace) { '' } - - it 'sets visibility level to private' do - expect(transformed_data[:visibility_level]).to eq(Gitlab::VisibilityLevel::PRIVATE) - end - end end end