From e7c25e67d70c132daad92043c43632ce30b5ebf0 Mon Sep 17 00:00:00 2001 From: Martin Wortschack Date: Tue, 7 Jan 2025 15:37:57 +0100 Subject: [PATCH 1/3] Allow actor override when Direct Transfer disabled This change removes the override_bulk_import_disabled ops flag that was introduced in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132431 and allowed to selectively enable Direct Transfer when it has been globally disabled. Changelog: other --- .../import/bulk_imports_controller.rb | 3 +- ...roup_from_another_instance_panel.html.haml | 2 +- .../ops/override_bulk_import_disabled.yml | 8 ---- lib/api/bulk_imports.rb | 3 +- lib/api/group_export.rb | 3 +- lib/api/project_export.rb | 3 +- .../import/bulk_imports_controller_spec.rb | 26 ------------- .../import_export/connect_instance_spec.rb | 30 ++------------- spec/requests/api/bulk_imports_spec.rb | 18 --------- spec/requests/api/group_export_spec.rb | 18 --------- spec/requests/api/project_export_spec.rb | 38 +++---------------- 11 files changed, 15 insertions(+), 137 deletions(-) delete mode 100644 config/feature_flags/ops/override_bulk_import_disabled.yml diff --git a/app/controllers/import/bulk_imports_controller.rb b/app/controllers/import/bulk_imports_controller.rb index 8d01373d5a4902..33b49244eaf18f 100644 --- a/app/controllers/import/bulk_imports_controller.rb +++ b/app/controllers/import/bulk_imports_controller.rb @@ -157,8 +157,7 @@ def bulk_import_params end def ensure_bulk_import_enabled - render_404 unless Gitlab::CurrentSettings.bulk_import_enabled? || - Feature.enabled?(:override_bulk_import_disabled, current_user, type: :ops) + render_404 unless Gitlab::CurrentSettings.bulk_import_enabled? end def access_token_key diff --git a/app/views/groups/_import_group_from_another_instance_panel.html.haml b/app/views/groups/_import_group_from_another_instance_panel.html.haml index a219a385f0c152..44e331ca6f14ff 100644 --- a/app/views/groups/_import_group_from_another_instance_panel.html.haml +++ b/app/views/groups/_import_group_from_another_instance_panel.html.haml @@ -1,4 +1,4 @@ -- bulk_imports_enabled = Gitlab::CurrentSettings.bulk_import_enabled? || Feature.enabled?(:override_bulk_import_disabled, current_user, type: :ops) +- bulk_imports_enabled = Gitlab::CurrentSettings.bulk_import_enabled? = gitlab_ui_form_with url: configure_import_bulk_imports_path(namespace_id: params[:parent_id]), class: 'gl-show-field-errors' do |f| .gl-border-l-solid.gl-border-r-solid.gl-border-t-solid.gl-border-default.gl-border-1.gl-p-5.gl-mt-4 diff --git a/config/feature_flags/ops/override_bulk_import_disabled.yml b/config/feature_flags/ops/override_bulk_import_disabled.yml deleted file mode 100644 index 057170a30ad8a7..00000000000000 --- a/config/feature_flags/ops/override_bulk_import_disabled.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: override_bulk_import_disabled -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/132431 -rollout_issue_url: -milestone: '16.5' -type: ops -group: group::import and integrate -default_enabled: false diff --git a/lib/api/bulk_imports.rb b/lib/api/bulk_imports.rb index a42245bf732a62..2cbadbfa07ee97 100644 --- a/lib/api/bulk_imports.rb +++ b/lib/api/bulk_imports.rb @@ -33,8 +33,7 @@ def bulk_import_entity end before do - not_found! unless Gitlab::CurrentSettings.bulk_import_enabled? || - Feature.enabled?(:override_bulk_import_disabled, current_user, type: :ops) + not_found! unless Gitlab::CurrentSettings.bulk_import_enabled? authenticate! end diff --git a/lib/api/group_export.rb b/lib/api/group_export.rb index 086d712cc18bac..4e7c8b134dcfe5 100644 --- a/lib/api/group_export.rb +++ b/lib/api/group_export.rb @@ -70,8 +70,7 @@ class GroupExport < ::API::Base resource do before do - not_found! unless Gitlab::CurrentSettings.bulk_import_enabled? || - Feature.enabled?(:override_bulk_import_disabled, current_user, type: :ops) + not_found! unless Gitlab::CurrentSettings.bulk_import_enabled? end desc 'Start relations export' do diff --git a/lib/api/project_export.rb b/lib/api/project_export.rb index 1e445c3e12814f..6d4db1dd9804af 100644 --- a/lib/api/project_export.rb +++ b/lib/api/project_export.rb @@ -110,8 +110,7 @@ class ProjectExport < ::API::Base resource do before do - not_found! unless Gitlab::CurrentSettings.bulk_import_enabled? || - Feature.enabled?(:override_bulk_import_disabled, current_user, type: :ops) + not_found! unless Gitlab::CurrentSettings.bulk_import_enabled? authorize_admin_project end diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index 7227ddcf86aa65..a7adc589c38265 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -514,7 +514,6 @@ def get_status(params_override = {}, format = :json) context 'when importing groups and projects by direct transfer is disabled' do before do stub_application_setting(bulk_import_enabled: false) - stub_feature_flags(override_bulk_import_disabled: false) allow_next_instance_of(BulkImports::Clients::HTTP) do |instance| allow(instance).to receive(:validate_instance_version!).and_return(true) @@ -539,31 +538,6 @@ def get_status(params_override = {}, format = :json) expect(response).to have_gitlab_http_status(:not_found) end end - - context 'when the override_bulk_import_disabled feature flag is enabled' do - before do - stub_feature_flags(override_bulk_import_disabled: true) - end - - context 'POST configure' do - it 'does not return 404' do - post :configure, params: { - bulk_import_gitlab_access_token: 'token', bulk_import_gitlab_url: 'https://gitlab.example' - } - - expect(response).to have_gitlab_http_status(:found) - expect(response).to redirect_to(status_import_bulk_imports_url) - end - end - - context 'GET status' do - it 'does not return 404' do - get :status - - expect(response).to have_gitlab_http_status(:ok) - end - end - end end end diff --git a/spec/features/groups/import_export/connect_instance_spec.rb b/spec/features/groups/import_export/connect_instance_spec.rb index ba38cc51893139..3dd6b842b709fa 100644 --- a/spec/features/groups/import_export/connect_instance_spec.rb +++ b/spec/features/groups/import_export/connect_instance_spec.rb @@ -82,32 +82,10 @@ stub_application_setting(bulk_import_enabled: false) end - context 'when the override_bulk_import_disabled feature flag is disabled' do - before do - stub_feature_flags(override_bulk_import_disabled: false) - - open_import_group - end - - it 'renders fields and button disabled' do - expect(page).to have_field('GitLab source instance base URL', disabled: true) - expect(page).to have_field('Personal access token', disabled: true) - expect(page).to have_button('Connect instance', disabled: true) - end - end - - context 'when the override_bulk_import_disabled feature flag is enabled' do - before do - stub_feature_flags(override_bulk_import_disabled: true) - - open_import_group - end - - it 'renders fields and button enabled' do - expect(page).to have_field('GitLab source instance base URL', disabled: false) - expect(page).to have_field('Personal access token', disabled: false) - expect(page).to have_button('Connect instance', disabled: false) - end + it 'renders fields and button disabled' do + expect(page).to have_field('GitLab source instance base URL', disabled: true) + expect(page).to have_field('Personal access token', disabled: true) + expect(page).to have_button('Connect instance', disabled: true) end end diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index c4039a6675e8c5..bee4de01af2f79 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -52,29 +52,11 @@ shared_examples 'disabled feature' do before do stub_application_setting(bulk_import_enabled: false) - stub_feature_flags(override_bulk_import_disabled: false) end it_behaves_like '404 response' do let(:message) { '404 Not Found' } end - - it 'enables the feature when override flag is enabled for the user' do - stub_feature_flags(override_bulk_import_disabled: user) - - request - - expect(response).not_to have_gitlab_http_status(:not_found) - end - - it 'does not enable the feature when override flag is enabled for another user' do - other_user = create(:user) - stub_feature_flags(override_bulk_import_disabled: other_user) - - request - - expect(response).to have_gitlab_http_status(:not_found) - end end describe 'GET /bulk_imports' do diff --git a/spec/requests/api/group_export_spec.rb b/spec/requests/api/group_export_spec.rb index fd7c74c8193c39..e88516b6546ea8 100644 --- a/spec/requests/api/group_export_spec.rb +++ b/spec/requests/api/group_export_spec.rb @@ -423,29 +423,11 @@ before do stub_application_setting(bulk_import_enabled: false) - stub_feature_flags(override_bulk_import_disabled: false) end it_behaves_like '404 response' do let(:message) { '404 Not Found' } end - - it 'enables the feature when override flag is enabled for the user' do - stub_feature_flags(override_bulk_import_disabled: user) - - request - - expect(response).to have_gitlab_http_status(:accepted) - end - - it 'does not enable the feature when override flag is enabled for another user' do - other_user = create(:user) - stub_feature_flags(override_bulk_import_disabled: other_user) - - request - - expect(response).to have_gitlab_http_status(:not_found) - end end end end diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index 3dbd67621b72c1..b47b9236908fbc 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -755,63 +755,37 @@ context 'with bulk_import is disabled' do before do stub_application_setting(bulk_import_enabled: false) - stub_feature_flags(override_bulk_import_disabled: false) - end - - shared_examples 'flag override' do |expected_http_status:| - it 'enables the feature when override flag is enabled for the user' do - stub_feature_flags(override_bulk_import_disabled: user) - - request - - expect(response).to have_gitlab_http_status(expected_http_status) - end - - it 'does not enable the feature when override flag is enabled for another user' do - other_user = create(:user) - stub_feature_flags(override_bulk_import_disabled: other_user) - - request - - expect(response).to have_gitlab_http_status(:not_found) - end end describe 'POST /projects/:id/export_relations' do - subject(:request) { post api(path, user) } - it_behaves_like '404 response' do let(:message) { '404 Not Found' } - end - it_behaves_like 'flag override', expected_http_status: :accepted + subject(:request) { get api(download_path, user) } + end end describe 'GET /projects/:id/export_relations/download' do let_it_be(:export) { create(:bulk_import_export, project: project, relation: 'labels', user: user) } let_it_be(:upload) { create(:bulk_import_export_upload, export: export) } - subject(:request) { get api(download_path, user) } - before do upload.update!(export_file: fixture_file_upload('spec/fixtures/bulk_imports/gz/labels.ndjson.gz')) end it_behaves_like '404 response' do let(:message) { '404 Not Found' } - end - it_behaves_like 'flag override', expected_http_status: :ok + subject(:request) { get api(download_path, user) } + end end describe 'GET /projects/:id/export_relations/status' do - subject(:request) { get api(status_path, user) } - it_behaves_like '404 response' do let(:message) { '404 Not Found' } - end - it_behaves_like 'flag override', expected_http_status: :ok + subject(:request) { get api(status_path, user) } + end end end end -- GitLab From 700ca6ea60455ad1c30a3eeb59d41523f94197a1 Mon Sep 17 00:00:00 2001 From: Martin Wortschack Date: Wed, 8 Jan 2025 06:34:42 +0000 Subject: [PATCH 2/3] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Luke Duncalfe --- spec/features/groups/import_export/connect_instance_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/features/groups/import_export/connect_instance_spec.rb b/spec/features/groups/import_export/connect_instance_spec.rb index 3dd6b842b709fa..a75833114d852e 100644 --- a/spec/features/groups/import_export/connect_instance_spec.rb +++ b/spec/features/groups/import_export/connect_instance_spec.rb @@ -80,6 +80,8 @@ context 'when importing groups and projects by direct transfer is disabled' do before do stub_application_setting(bulk_import_enabled: false) + + open_import_group end it 'renders fields and button disabled' do -- GitLab From cf6f94e148e1cfbf2e8404d04f545f97caa51353 Mon Sep 17 00:00:00 2001 From: Martin Wortschack Date: Thu, 9 Jan 2025 07:26:54 +0000 Subject: [PATCH 3/3] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Luke Duncalfe --- spec/requests/api/project_export_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/api/project_export_spec.rb b/spec/requests/api/project_export_spec.rb index b47b9236908fbc..c694e6ed8d6035 100644 --- a/spec/requests/api/project_export_spec.rb +++ b/spec/requests/api/project_export_spec.rb @@ -761,7 +761,7 @@ it_behaves_like '404 response' do let(:message) { '404 Not Found' } - subject(:request) { get api(download_path, user) } + subject(:request) { post api(path, user) } end end -- GitLab