Use closest visibility level for groups creation on Group Import

- When importing groups that had subgroups using
  Group Import/Export functionality,
  visibility levels were not considered.
- Which led to import failures due to validation errors,
  when a group that had internal visibility level is being
  imported into a private one.
- This MR changes visibility levels to, either use parent
  group's closest visibility level, or private.

changelogs/unreleased/georgekoltsov-fix-group-import-visibility-level.yml

+---
+title: Use closest allowed visibility level on group creation when importing groups
+  using Group Import/Export
+merge_request: 25026
+author:
+type: fixed

lib/api/group_import.rb

@@ -5,15 +5,25 @@ module API
     MAXIMUM_FILE_SIZE = 50.megabytes.freeze
 
     helpers do
-      def authorize_create_group!
-        parent_group = find_group!(params[:parent_id]) if params[:parent_id].present?
+      def parent_group
+        find_group!(params[:parent_id]) if params[:parent_id].present?
+      end
 
+      def authorize_create_group!
         if parent_group
           authorize! :create_subgroup, parent_group
         else
           authorize! :create_group
         end
       end
+
+      def closest_allowed_visibility_level
+        if parent_group
+          Gitlab::VisibilityLevel.closest_allowed_level(parent_group.visibility_level)
+        else
+          Gitlab::VisibilityLevel::PRIVATE
+        end
+      end
     end
 
     resource :groups, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do
@@ -59,6 +69,7 @@ module API
           path: params[:path],
           name: params[:name],
           parent_id: params[:parent_id],
+          visibility_level: closest_allowed_visibility_level,
           import_export_upload: ImportExportUpload.new(import_file: uploaded_file)
         }
 

lib/gitlab/import_export/group_import_export.yml

@@ -37,6 +37,7 @@ excluded_attributes:
     - :runners_token
     - :runners_token_encrypted
     - :saml_discovery_token
+    - :visibility_level
 
 methods:
   labels:

lib/gitlab/import_export/group_tree_restorer.rb

@@ -74,12 +74,23 @@ module Gitlab
         group_params = {
           name:      group_hash['name'],
           path:      group_hash['path'],
-          parent_id: parent_group&.id
+          parent_id: parent_group&.id,
+          visibility_level: sub_group_visibility_level(group_hash, parent_group)
         }
 
         ::Groups::CreateService.new(@user, group_params).execute
       end
 
+      def sub_group_visibility_level(group_hash, parent_group)
+        original_visibility_level = group_hash['visibility_level'] || Gitlab::VisibilityLevel::PRIVATE
+
+        if parent_group && parent_group.visibility_level < original_visibility_level
+          Gitlab::VisibilityLevel.closest_allowed_level(parent_group.visibility_level)
+        else
+          original_visibility_level
+        end
+      end
+
       def members_mapper
         @members_mapper ||= Gitlab::ImportExport::MembersMapper.new(exported_members: @group_members, user: @user, importable: @group)
       end

spec/fixtures/lib/gitlab/import_export/group_exports/visibility_levels/internal/group.json

+{
+  "id": 283,
+  "name": "internal",
+  "path": "internal",
+  "owner_id": null,
+  "created_at": "2020-02-12T16:56:34.924Z",
+  "updated_at": "2020-02-12T16:56:38.710Z",
+  "description": "",
+  "avatar": {
+    "url": null
+  },
+  "membership_lock": false,
+  "share_with_group_lock": false,
+  "visibility_level": 10,
+  "request_access_enabled": true,
+  "ldap_sync_status": "ready",
+  "ldap_sync_error": null,
+  "ldap_sync_last_update_at": null,
+  "ldap_sync_last_successful_update_at": null,
+  "ldap_sync_last_sync_at": null,
+  "lfs_enabled": null,
+  "parent_id": null,
+  "shared_runners_minutes_limit": null,
+  "repository_size_limit": null,
+  "require_two_factor_authentication": false,
+  "two_factor_grace_period": 48,
+  "plan_id": null,
+  "project_creation_level": 2,
+  "trial_ends_on": null,
+  "file_template_project_id": null,
+  "custom_project_templates_group_id": null,
+  "auto_devops_enabled": null,
+  "extra_shared_runners_minutes_limit": null,
+  "last_ci_minutes_notification_at": null,
+  "last_ci_minutes_usage_notification_level": null,
+  "subgroup_creation_level": 1,
+  "emails_disabled": null,
+  "max_pages_size": null,
+  "max_artifacts_size": null,
+  "mentions_disabled": null,
+  "children": [
+    {
+      "id": 284,
+      "name": "public",
+      "path": "public",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 20,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    },
+    {
+      "id": 285,
+      "name": "internal",
+      "path": "internal",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 10,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    },
+    {
+      "id": 286,
+      "name": "private",
+      "path": "private",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 0,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    }
+  ]
+}

spec/fixtures/lib/gitlab/import_export/group_exports/visibility_levels/private/group.json

+{
+  "id": 283,
+  "name": "private",
+  "path": "private",
+  "owner_id": null,
+  "created_at": "2020-02-12T16:56:34.924Z",
+  "updated_at": "2020-02-12T16:56:38.710Z",
+  "description": "",
+  "avatar": {
+    "url": null
+  },
+  "membership_lock": false,
+  "share_with_group_lock": false,
+  "visibility_level": 0,
+  "request_access_enabled": true,
+  "ldap_sync_status": "ready",
+  "ldap_sync_error": null,
+  "ldap_sync_last_update_at": null,
+  "ldap_sync_last_successful_update_at": null,
+  "ldap_sync_last_sync_at": null,
+  "lfs_enabled": null,
+  "parent_id": null,
+  "shared_runners_minutes_limit": null,
+  "repository_size_limit": null,
+  "require_two_factor_authentication": false,
+  "two_factor_grace_period": 48,
+  "plan_id": null,
+  "project_creation_level": 2,
+  "trial_ends_on": null,
+  "file_template_project_id": null,
+  "custom_project_templates_group_id": null,
+  "auto_devops_enabled": null,
+  "extra_shared_runners_minutes_limit": null,
+  "last_ci_minutes_notification_at": null,
+  "last_ci_minutes_usage_notification_level": null,
+  "subgroup_creation_level": 1,
+  "emails_disabled": null,
+  "max_pages_size": null,
+  "max_artifacts_size": null,
+  "mentions_disabled": null,
+  "children": [
+    {
+      "id": 284,
+      "name": "public",
+      "path": "public",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 20,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    },
+    {
+      "id": 285,
+      "name": "internal",
+      "path": "internal",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 10,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    },
+    {
+      "id": 286,
+      "name": "private",
+      "path": "private",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 0,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    }
+  ]
+}

spec/fixtures/lib/gitlab/import_export/group_exports/visibility_levels/public/group.json

+{
+  "id": 283,
+  "name": "public",
+  "path": "public",
+  "owner_id": null,
+  "created_at": "2020-02-12T16:56:34.924Z",
+  "updated_at": "2020-02-12T16:56:38.710Z",
+  "description": "",
+  "avatar": {
+    "url": null
+  },
+  "membership_lock": false,
+  "share_with_group_lock": false,
+  "visibility_level": 20,
+  "request_access_enabled": true,
+  "ldap_sync_status": "ready",
+  "ldap_sync_error": null,
+  "ldap_sync_last_update_at": null,
+  "ldap_sync_last_successful_update_at": null,
+  "ldap_sync_last_sync_at": null,
+  "lfs_enabled": null,
+  "parent_id": null,
+  "shared_runners_minutes_limit": null,
+  "repository_size_limit": null,
+  "require_two_factor_authentication": false,
+  "two_factor_grace_period": 48,
+  "plan_id": null,
+  "project_creation_level": 2,
+  "trial_ends_on": null,
+  "file_template_project_id": null,
+  "custom_project_templates_group_id": null,
+  "auto_devops_enabled": null,
+  "extra_shared_runners_minutes_limit": null,
+  "last_ci_minutes_notification_at": null,
+  "last_ci_minutes_usage_notification_level": null,
+  "subgroup_creation_level": 1,
+  "emails_disabled": null,
+  "max_pages_size": null,
+  "max_artifacts_size": null,
+  "mentions_disabled": null,
+  "children": [
+    {
+      "id": 284,
+      "name": "public",
+      "path": "public",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 20,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    },
+    {
+      "id": 285,
+      "name": "internal",
+      "path": "internal",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 10,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    },
+    {
+      "id": 286,
+      "name": "private",
+      "path": "private",
+      "owner_id": null,
+      "created_at": "2020-02-12T17:33:00.575Z",
+      "updated_at": "2020-02-12T17:33:00.575Z",
+      "description": "",
+      "avatar": {
+        "url": null
+      },
+      "membership_lock": false,
+      "share_with_group_lock": false,
+      "visibility_level": 0,
+      "request_access_enabled": true,
+      "ldap_sync_status": "ready",
+      "ldap_sync_error": null,
+      "ldap_sync_last_update_at": null,
+      "ldap_sync_last_successful_update_at": null,
+      "ldap_sync_last_sync_at": null,
+      "lfs_enabled": null,
+      "parent_id": 283,
+      "shared_runners_minutes_limit": null,
+      "repository_size_limit": null,
+      "require_two_factor_authentication": false,
+      "two_factor_grace_period": 48,
+      "plan_id": null,
+      "project_creation_level": 2,
+      "trial_ends_on": null,
+      "file_template_project_id": null,
+      "custom_project_templates_group_id": null,
+      "auto_devops_enabled": null,
+      "extra_shared_runners_minutes_limit": null,
+      "last_ci_minutes_notification_at": null,
+      "last_ci_minutes_usage_notification_level": null,
+      "subgroup_creation_level": 1,
+      "emails_disabled": null,
+      "max_pages_size": null,
+      "max_artifacts_size": null,
+      "mentions_disabled": null
+    }
+  ]
+}

spec/lib/gitlab/import_export/group_tree_restorer_spec.rb

@@ -125,4 +125,31 @@ describe Gitlab::ImportExport::GroupTreeRestorer do
       end
     end
   end
+
+  context 'group visibility levels' do
+    let(:user) { create(:user) }
+    let(:shared) { Gitlab::ImportExport::Shared.new(group) }
+    let(:group_tree_restorer) { described_class.new(user: user, shared: shared, group: group, group_hash: nil) }
+
+    before do
+      setup_import_export_config(filepath)
+
+      group_tree_restorer.restore
+    end
+
+    shared_examples 'with visibility level' do |visibility_level, expected_visibilities|
+      context "when visibility level is #{visibility_level}" do
+        let(:group) { create(:group, visibility_level) }
+        let(:filepath) { "group_exports/visibility_levels/#{visibility_level}" }
+
+        it "imports all subgroups as #{visibility_level}" do
+          expect(group.children.map(&:visibility_level)).to eq(expected_visibilities)
+        end
+      end
+    end
+
+    include_examples 'with visibility level', :public, [20, 10, 0]
+    include_examples 'with visibility level', :private, [0, 0, 0]
+    include_examples 'with visibility level', :internal, [10, 10, 0]
+  end
 end

spec/lib/gitlab/import_export/group_tree_saver_spec.rb

@@ -80,7 +80,7 @@ describe Gitlab::ImportExport::GroupTreeSaver do
       end
 
       it 'saves the correct json' do
-        expect(saved_group_json).to include({ 'description' => 'description', 'visibility_level' => 20 })
+        expect(saved_group_json).to include({ 'description' => 'description' })
       end
 
       it 'has milestones' do

spec/requests/api/group_import_spec.rb

@@ -45,6 +45,14 @@ describe API::GroupImport do
           expect(response).to have_gitlab_http_status(202)
         end
 
+        it 'creates private group' do
+          expect { subject }.to change { Group.count }.by(1)
+
+          group = Group.find_by(name: 'test-import-group')
+
+          expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+        end
+
         context 'when importing to a parent group' do
           before do
             group.add_owner(user)
@@ -59,6 +67,34 @@ describe API::GroupImport do
             expect(group.children.count).to eq(1)
           end
 
+          context 'when parent group is private or internal' do
+            let(:public_parent_group) { create(:group, :public) }
+            let(:internal_parent_group) { create(:group, :internal) }
+
+            before do
+              public_parent_group.add_owner(user)
+              internal_parent_group.add_owner(user)
+            end
+
+            it 'imports public group' do
+              params[:parent_id] = public_parent_group.id
+
+              subject
+
+              expect(response).to have_gitlab_http_status(202)
+              expect(public_parent_group.children.first.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
+            end
+
+            it 'imports internal group' do
+              params[:parent_id] = internal_parent_group.id
+
+              subject
+
+              expect(response).to have_gitlab_http_status(202)
+              expect(internal_parent_group.children.first.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
+            end
+          end
+
           context 'when parent group is invalid' do
             it 'returns 404 and does not create new group' do
               params[:parent_id] = 99999