diff --git a/doc/user/group/import/index.md b/doc/user/group/import/index.md index 9ab786eec02921cf69fdf6c8de0c8e254cd6d7a9..49ae181b1864f0d1e9ce2be8efb338206ee43af9 100644 --- a/doc/user/group/import/index.md +++ b/doc/user/group/import/index.md @@ -161,6 +161,20 @@ After migration: If you used a private network on your source instance to hide content from the general public, make sure to have a similar setup on the destination instance, or to import into a private group. +## Memberships + +> - Importing of shared and inherited shared members was [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/129017) in GitLab 16.3. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/148220) in GitLab 16.11, shared and inherited shared members are no longer imported as direct members if they are already shared or inherited shared members of the imported group or project. + +Group and project members are imported if the [user account prerequisites](#user-accounts) are followed. + +All [direct and indirect](../../../user/project/members/index.md#membership-types) members are imported. + +Indirect members are imported as [direct members](../../project/members/index.md#membership-types) if: + +- They are not already an indirect member. +- They are an indirect member, but have a lower [permission](../../../user/permissions.md). + ## Prerequisites > - Requirement for Maintainer role instead of Developer role introduced in GitLab 16.0 and backported to GitLab 15.11.1 and GitLab 15.10.5. @@ -349,7 +363,7 @@ Group items that are migrated to the destination GitLab instance include: | Group labels 2 | [GitLab 13.9](https://gitlab.com/gitlab-org/gitlab/-/issues/292429) | | Iterations | [GitLab 13.10](https://gitlab.com/gitlab-org/gitlab/-/issues/292428) | | Iteration cadences | [GitLab 15.4](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96570) | -| Members 3 | [GitLab 13.9](https://gitlab.com/gitlab-org/gitlab/-/issues/299415) | +| Members 3 | [GitLab 13.9](https://gitlab.com/gitlab-org/gitlab/-/issues/299415) | | Group milestones | [GitLab 13.10](https://gitlab.com/gitlab-org/gitlab/-/issues/292427) | | Namespace settings | [GitLab 14.10](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/85128) | | Release milestones | [GitLab 15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/339422) | @@ -362,9 +376,7 @@ Group items that are migrated to the destination GitLab instance include: metadata [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/63551) in GitLab 14.0. 1. Group Labels cannot retain any associated Label Priorities during import. These labels will need to be re-prioritized manually once the relevant Project is migrated to the destination instance. -1. Group members are associated with the imported group if the user: - - Already exists in the destination GitLab instance. - - Has a public email in the source GitLab instance that matches a confirmed email in the destination GitLab instance. +1. See [Memberships](#memberships). ### Excluded items @@ -425,7 +437,7 @@ Project items that are migrated to the destination GitLab instance include: | Issue boards | [GitLab 14.4](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71661) | | Labels | [GitLab 14.4](https://gitlab.com/gitlab-org/gitlab/-/issues/339419) | | LFS Objects | [GitLab 14.8](https://gitlab.com/gitlab-org/gitlab/-/issues/339405) | -| Members | [GitLab 14.8](https://gitlab.com/gitlab-org/gitlab/-/issues/341886) | +| Members 2 | [GitLab 14.8](https://gitlab.com/gitlab-org/gitlab/-/issues/341886) | | Merge requests | [GitLab 14.5](https://gitlab.com/gitlab-org/gitlab/-/issues/339403) | | Push rules | [GitLab 14.6](https://gitlab.com/gitlab-org/gitlab/-/issues/339403) | | Milestones | [GitLab 14.5](https://gitlab.com/gitlab-org/gitlab/-/issues/339417) | @@ -445,6 +457,7 @@ Project items that are migrated to the destination GitLab instance include: 1. Imported branches respect the [default branch protection settings](../../project/protected_branches.md) of the destination group, which could cause an unprotected branch to be imported as protected. +1. See [Memberships](#memberships). ### Issue-related items diff --git a/lib/bulk_imports/common/pipelines/members_pipeline.rb b/lib/bulk_imports/common/pipelines/members_pipeline.rb index 90df8453d7789cb6f8f309e3895d8c5835b3e124..dbaae8f6ef23afaa45f897386fbd93ef441f3476 100644 --- a/lib/bulk_imports/common/pipelines/members_pipeline.rb +++ b/lib/bulk_imports/common/pipelines/members_pipeline.rb @@ -6,6 +6,9 @@ module Pipelines class MembersPipeline include Pipeline + GROUP_MEMBER_RELATIONS = %i[direct inherited shared_from_groups].freeze + PROJECT_MEMBER_RELATIONS = %i[direct inherited invited_groups shared_into_ancestors].freeze + transformer Common::Transformers::ProhibitedAttributesTransformer transformer Common::Transformers::MemberAttributesTransformer @@ -40,15 +43,27 @@ def graphql_extractor end def existing_user_membership(user_id) - members_finder.execute.find_by_user_id(user_id) + execute_finder.find_by_user_id(user_id) + end + + def finder + @finder ||= if context.entity.group? + ::GroupMembersFinder.new(portable, current_user) + else + ::MembersFinder.new(portable, current_user) + end + end + + def execute_finder + finder.execute(include_relations: finder_relations) end - def members_finder - @members_finder ||= if context.entity.group? - ::GroupMembersFinder.new(portable, current_user) - else - ::MembersFinder.new(portable, current_user) - end + def finder_relations + if context.entity.group? + GROUP_MEMBER_RELATIONS + else + PROJECT_MEMBER_RELATIONS + end end end end diff --git a/spec/lib/bulk_imports/common/pipelines/members_pipeline_spec.rb b/spec/lib/bulk_imports/common/pipelines/members_pipeline_spec.rb index 5fc0c8fa23956154d4d99b75053ff9643a28b5c7..39ccdeb344d064caa84578db6a7dea962da103f6 100644 --- a/spec/lib/bulk_imports/common/pipelines/members_pipeline_spec.rb +++ b/spec/lib/bulk_imports/common/pipelines/members_pipeline_spec.rb @@ -128,7 +128,7 @@ def extracted_data(email:, has_next_page: false) end end - context 'when membership with higher access level exists in parent group' do + context 'when membership has higher access level than membership in parent group' do it 'creates new direct membership' do data = member_data.merge(access_level: Gitlab::Access::MAINTAINER) @@ -140,7 +140,7 @@ def extracted_data(email:, has_next_page: false) end end - context 'when membership with lower access level exists in parent group' do + context 'when membership has lower access level than membership in parent group' do it 'does not create new membership' do data = member_data.merge(access_level: Gitlab::Access::GUEST) @@ -152,20 +152,96 @@ def extracted_data(email:, has_next_page: false) end context 'when importing to group' do - let(:portable) { create(:group) } + let_it_be(:portable) { create(:group) } + let(:portable_with_parent) { create(:group, parent: parent) } let(:entity) { create(:bulk_import_entity, :group_entity, group: portable, bulk_import: bulk_import) } let(:entity_with_parent) { create(:bulk_import_entity, :group_entity, group: portable_with_parent, bulk_import: bulk_import) } include_examples 'members import' + + context 'when user is a member of group through group sharing' do + before_all do + group = create(:group) + group.add_developer(member_user1) + create(:group_group_link, shared_group: portable, shared_with_group: group) + end + + it 'does not create new membership' do + expect { pipeline.load(context, member_data) }.not_to change(Member, :count) + end + + context 'when membership is a higher access level' do + it 'creates new direct membership' do + data = member_data.merge(access_level: Gitlab::Access::MAINTAINER) + + expect { pipeline.load(context, data) }.to change(portable.members, :count).by(1) + + member = portable.members.find_by_user_id(member_user1.id) + + expect(member.access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end end context 'when importing to project' do - let(:portable) { create(:project) } + let_it_be(:portable) { create(:project) } + let(:portable_with_parent) { create(:project, namespace: parent) } let(:entity) { create(:bulk_import_entity, :project_entity, project: portable, bulk_import: bulk_import) } let(:entity_with_parent) { create(:bulk_import_entity, :project_entity, project: portable_with_parent, bulk_import: bulk_import) } include_examples 'members import' + + context 'when project is shared with a group, and user is a direct member of the group' do + before_all do + group = create(:group) + group.add_developer(member_user1) + create(:project_group_link, project: portable, group: group) + end + + it 'does not create new membership' do + expect { pipeline.load(context, member_data) }.not_to change(Member, :count) + end + + context 'when membership is a higher access level' do + it 'creates new direct membership' do + data = member_data.merge(access_level: Gitlab::Access::MAINTAINER) + + expect { pipeline.load(context, data) }.to change(portable.members, :count).by(1) + + member = portable.members.find_by_user_id(member_user1.id) + + expect(member.access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end + + context 'when parent group is shared with other group, and user is a member of other group' do + let(:tracker) { create(:bulk_import_tracker, entity: entity_with_parent) } + + before do + group = create(:group) + group.add_developer(member_user1) + create(:group_group_link, shared_group: parent, shared_with_group: group) + end + + it 'does not create new membership' do + expect { pipeline.load(context, member_data) }.not_to change(Member, :count) + end + + context 'when membership is a higher access level' do + it 'creates new direct membership' do + data = member_data.merge(access_level: Gitlab::Access::MAINTAINER) + + expect { pipeline.load(context, data) }.to change(portable_with_parent.members, :count).by(1) + + member = portable_with_parent.members.find_by_user_id(member_user1.id) + + expect(member.access_level).to eq(Gitlab::Access::MAINTAINER) + end + end + end end end