diff --git a/config/feature_flags/development/mirror_only_branches_match_regex.yml b/config/feature_flags/development/mirror_only_branches_match_regex.yml new file mode 100644 index 0000000000000000000000000000000000000000..6494569b364d014a0b7a62510e731bc2f3fa5e50 --- /dev/null +++ b/config/feature_flags/development/mirror_only_branches_match_regex.yml @@ -0,0 +1,8 @@ +--- +name: mirror_only_branches_match_regex +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/99201 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/381667 +milestone: '15.6' +type: development +group: "group::source code" +default_enabled: false diff --git a/ee/app/controllers/ee/projects/mirrors_controller.rb b/ee/app/controllers/ee/projects/mirrors_controller.rb index e885a218c8bff6260d56a78daf6d0de8ebb8561e..5ff52a97d6463c772a6fe9165009283c18094799 100644 --- a/ee/app/controllers/ee/projects/mirrors_controller.rb +++ b/ee/app/controllers/ee/projects/mirrors_controller.rb @@ -50,7 +50,9 @@ def update_now def mirror_params_attributes if can?(current_user, :admin_mirror, project) - super + mirror_params_attributes_ee + attributes = super + attributes[0][:remote_mirrors_attributes].push(:mirror_branch_regex) if mirror_branch_regex_enabled? + attributes + mirror_params_attributes_ee else super end @@ -89,8 +91,32 @@ def safe_mirror_params end end + # avoid enable only_protected_branches and mirror_branch_regex at the same time + remote_mirror_data = params[:remote_mirrors_attributes] + if remote_mirror_data.present? + remote_mirror_data.transform_values! do |value| + format_remote_mirrors_attributes(value) + value + end + end + params end + + # Pass mirror_branch_regex and only_protected_branches at same time will use mirror_branch_regex + def format_remote_mirrors_attributes(params) + return unless params.is_a?(ActionController::Parameters) + + if mirror_branch_regex_enabled? && params[:mirror_branch_regex].present? + params[:only_protected_branches] = false + end + + params[:mirror_branch_regex] = nil if params[:only_protected_branches].present? + end + + def mirror_branch_regex_enabled? + ::Feature.enabled?(:mirror_only_branches_match_regex, project) + end end end end diff --git a/ee/app/models/concerns/mirror_configuration.rb b/ee/app/models/concerns/mirror_configuration.rb new file mode 100644 index 0000000000000000000000000000000000000000..c3c719c283b5d246feb44fe22aa79dccb9442949 --- /dev/null +++ b/ee/app/models/concerns/mirror_configuration.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module MirrorConfiguration + MIRROR_BRANCHES_SETTING = { + all: 'all', + protected: 'protected', + regex: 'regex' + }.freeze + + def mirror_branches_setting + return MIRROR_BRANCHES_SETTING[:protected] if only_mirror_protected_branches_column + return MIRROR_BRANCHES_SETTING[:regex] if mirror_branch_regex.present? + + MIRROR_BRANCHES_SETTING[:all] + end + + def only_mirror_protected_branches_column + raise NotImplementedError + end +end diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index d27062774927861a054a90e202b9970aa745699e..6e62b2354ade5556d9cf23fd6d31f19348266e2a 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -34,6 +34,7 @@ def inapplicable_to_branch(branch) include UsageStatistics include ProjectSecurityScannersInformation include VulnerabilityFlagHelpers + include MirrorConfiguration before_update :update_legacy_open_source_license_available, if: -> { visibility_level_changed? } @@ -232,6 +233,7 @@ def lock_for_confirmation!(id) :only_allow_merge_if_all_status_checks_passed, :only_allow_merge_if_all_status_checks_passed=, to: :project_setting + delegate :mirror_branch_regex, :mirror_branch_regex=, to: :project_setting validates :repository_size_limit, numericality: { only_integer: true, greater_than_or_equal_to: 0, allow_nil: true } @@ -972,6 +974,10 @@ def any_external_status_checks_not_passed?(merge_request) status_checks.any? { |check| check.status(merge_request, merge_request.diff_head_sha) != 'passed' } end + def only_mirror_protected_branches_column + only_mirror_protected_branches + end + private def update_legacy_open_source_license_available diff --git a/ee/app/models/ee/project_setting.rb b/ee/app/models/ee/project_setting.rb index 759068c24db98bca241cf3c78324ad5236a52ace..bf0df6e65f41c5eedc697b44db74e3837c455596 100644 --- a/ee/app/models/ee/project_setting.rb +++ b/ee/app/models/ee/project_setting.rb @@ -8,6 +8,9 @@ module ProjectSetting belongs_to :push_rule scope :has_vulnerabilities, -> { where('has_vulnerabilities IS TRUE') } + + validates :mirror_branch_regex, absence: true, if: -> { project&.only_mirror_protected_branches? } + validates :mirror_branch_regex, untrusted_regexp: true, length: { maximum: 255 } end def selective_code_owner_removals diff --git a/ee/app/models/ee/remote_mirror.rb b/ee/app/models/ee/remote_mirror.rb index afa6e3155d2a3ef0d74dbfe980d22232cfbec500..c001fb2b83dcd75cad4ccfe6d0c2b946d2d4d5dd 100644 --- a/ee/app/models/ee/remote_mirror.rb +++ b/ee/app/models/ee/remote_mirror.rb @@ -4,8 +4,36 @@ module EE module RemoteMirror extend ActiveSupport::Concern + prepended do + include MirrorConfiguration + + validates :mirror_branch_regex, absence: true, if: -> { only_protected_branches? } + validates :mirror_branch_regex, untrusted_regexp: true, length: { maximum: 255 } + end + def sync? super && !::Gitlab::Geo.secondary? end + + def only_mirror_protected_branches_column + only_protected_branches + end + + def options_for_update + return super unless ::Feature.enabled?(:mirror_only_branches_match_regex, project) + + options = super + options[:only_branches_matching] = branches_to_sync if mirror_branch_regex.present? + + options + end + + def branches_to_sync + branch_filter = ::Gitlab::UntrustedRegexp.new(mirror_branch_regex) + + project.repository.branch_names.select do |branch_name| + branch_filter.match?(branch_name) + end + end end end diff --git a/ee/app/services/ee/projects/update_service.rb b/ee/app/services/ee/projects/update_service.rb index 4025dd142149d2e11e0c898e40185234e6d9060e..1347a0eb5cb19a63e1f8d3364d5f4e0058f633bb 100644 --- a/ee/app/services/ee/projects/update_service.rb +++ b/ee/app/services/ee/projects/update_service.rb @@ -14,6 +14,7 @@ module UpdateService only_mirror_protected_branches mirror_overwrites_diverged_branches import_data_attributes + mirror_branch_regex ].freeze override :execute @@ -23,6 +24,7 @@ def execute shared_runners_setting mirror_user_setting + mirror_branch_setting compliance_framework_setting return update_failed! if project.errors.any? @@ -113,6 +115,20 @@ def mirror_user_setting end end + def mirror_branch_setting + if mirror_branch_regex_enabled? && params[:mirror_branch_regex].present? + params[:only_mirror_protected_branches] = false + end + + params.delete(:mirror_branch_regex) unless mirror_branch_regex_enabled? + + params[:mirror_branch_regex] = nil if params[:only_mirror_protected_branches] + end + + def mirror_branch_regex_enabled? + ::Feature.enabled?(:mirror_only_branches_match_regex, project) + end + def compliance_framework_setting settings = params[:compliance_framework_setting_attributes] return unless settings.present? diff --git a/ee/app/services/projects/update_mirror_service.rb b/ee/app/services/projects/update_mirror_service.rb index 12b7435db45b151088c61818d4a354e4cf6c8ed5..a453851f71e30113172283264989d27e9108f6ab 100644 --- a/ee/app/services/projects/update_mirror_service.rb +++ b/ee/app/services/projects/update_mirror_service.rb @@ -193,10 +193,28 @@ def repository_tags_with_target repository.tags.select(&:dereferenced_target) end - def skip_branch?(name) + def skip_mismatched_branch?(name) + mirror_branch_regex_enabled? && + project.mirror_branch_regex.present? && + !branch_regex.match?(name) + end + + def mirror_branch_regex_enabled? + ::Feature.enabled?(:mirror_only_branches_match_regex, project) + end + + def branch_regex + @branch_regex ||= Gitlab::UntrustedRegexp.new(project.mirror_branch_regex) + end + + def skip_unprotected_branch?(name) project.only_mirror_protected_branches && !ProtectedBranch.protected?(project, name) end + def skip_branch?(name) + skip_unprotected_branch?(name) || skip_mismatched_branch?(name) + end + def service_logger @service_logger ||= Gitlab::UpdateMirrorServiceJsonLogger.build end diff --git a/ee/spec/controllers/projects/mirrors_controller_spec.rb b/ee/spec/controllers/projects/mirrors_controller_spec.rb index 90314b2ed06404e4fb3e67de7d1fd14d58ffbeda..3cab6330c9dc2b8269b6935b44d9751977d13c04 100644 --- a/ee/spec/controllers/projects/mirrors_controller_spec.rb +++ b/ee/spec/controllers/projects/mirrors_controller_spec.rb @@ -24,6 +24,42 @@ do_put(project, remote_mirrors_attributes: { '0' => { 'enabled' => 1, 'url' => url } }) end.to change { RemoteMirror.count }.to(1) end + + it 'allows to create a remote mirror with mirror_branch_regex' do + expect do + do_put(project, remote_mirrors_attributes: { '0' => { enabled: 1, url: url, mirror_branch_regex: 'test' } }) + end.to change { RemoteMirror.where(mirror_branch_regex: 'test').count }.to(1) + end + + it 'allows only mirror protected branches' do + expect do + do_put(project, remote_mirrors_attributes: { '0' => { enabled: 1, url: url, only_protected_branches: true } }) + end.to change { RemoteMirror.where(only_protected_branches: true, mirror_branch_regex: nil).count }.to(1) + end + + it 'allows mirror all branches' do + expect do + do_put(project, remote_mirrors_attributes: { '0' => { enabled: 1, url: url } }) + end.to change { RemoteMirror.where(only_protected_branches: false, mirror_branch_regex: nil).count }.to(1) + end + + it 'do not allow invalid regex' do + do_put(project, remote_mirrors_attributes: { '0' => { enabled: 1, url: url, mirror_branch_regex: '\\' } } ) + expect(response).to redirect_to(project_settings_repository_path(project, anchor: 'js-push-remote-settings')) + expect(flash[:alert]).to match(/Remote mirrors mirror branch regex not valid RE2 syntax: trailing/) + end + + context 'when `mirror_only_branches_match_regex` FF is disabled' do + before do + stub_feature_flags(mirror_only_branches_match_regex: false) + end + + it 'ignores mirror_branch_regex parameter' do + expect do + do_put(project, remote_mirrors_attributes: { '0' => { enabled: 1, url: url, mirror_branch_regex: 'test' } }) + end.not_to change { RemoteMirror.where(mirror_branch_regex: 'test').count } + end + end end context 'when the current project has a remote mirror' do @@ -225,6 +261,49 @@ expect(flash[:alert]).to match(/is blocked/) end end + + context 'setting up project mirror branches setting' do + it 'mirror all branches' do + do_put(project, only_mirror_protected_branches: false, mirror_branch_regex: nil) + project.reload + expect(project.only_mirror_protected_branches).to be_falsey + expect(project.mirror_branch_regex).to be_nil + end + + it 'enable mirror_branch_regex would ignore only_mirror_protected_branches' do + do_put(project, only_mirror_protected_branches: true, mirror_branch_regex: 'text') + project.reload + expect(project.only_mirror_protected_branches).to be_falsey + expect(project.mirror_branch_regex).to eq('text') + end + + it 'enable mirror_branch_regex would disable only_protected_branches' do + project.update!(only_mirror_protected_branches: true) + do_put(project, mirror_branch_regex: 'text') + project.reload + expect(project.only_mirror_protected_branches).to be_falsey + expect(project.mirror_branch_regex).to eq 'text' + end + + it 'do not allow invalid regex' do + do_put(project, mirror_branch_regex: '\\') + expect(response).to redirect_to(project_settings_repository_path(project, anchor: 'js-push-remote-settings')) + expect(flash[:alert]).to match(/Project setting mirror branch regex not valid RE2 syntax: trailing/) + end + + context 'when `mirror_only_branches_match_regex` FF is disabled' do + before do + stub_feature_flags(mirror_only_branches_match_regex: false) + end + + it 'ignores mirror_branch_regex parameter' do + do_put(project, { mirror_branch_regex: 'test' } ) + project.reload + expect(project.only_mirror_protected_branches).to be_falsey + expect(project.mirror_branch_regex).to be_nil + end + end + end end def do_put(project, options, extra_attrs = {}) diff --git a/ee/spec/models/concerns/mirror_configuration_spec.rb b/ee/spec/models/concerns/mirror_configuration_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..1c4c00ffdfed9462d8ecad212803437df798df9c --- /dev/null +++ b/ee/spec/models/concerns/mirror_configuration_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MirrorConfiguration do + let(:klazz) { Class.new { include MirrorConfiguration } } + + subject(:instance) { klazz.new } + + describe '#only_mirror_protected_branches_column?' do + it 'raises NotImplementedError' do + expect { subject.only_mirror_protected_branches_column }.to raise_error(NotImplementedError) + end + end +end diff --git a/ee/spec/models/ee/project_setting_spec.rb b/ee/spec/models/ee/project_setting_spec.rb index 323413377c6f6615e40493dad4cb169c689c1371..4acf0fd5396184a96d90a7e52c3f703d8a2390dc 100644 --- a/ee/spec/models/ee/project_setting_spec.rb +++ b/ee/spec/models/ee/project_setting_spec.rb @@ -13,4 +13,31 @@ it { is_expected.to contain_exactly(setting_1) } end + + describe 'validations' do + context 'when enable only_mirror_protected_branches and mirror_branch_regex' do + it 'is invalid' do + project = build(:project, only_mirror_protected_branches: true ) + setting = build(:project_setting, project: project, mirror_branch_regex: 'text') + + expect(setting).not_to be_valid + end + end + + context 'when disable only_mirror_protected_branches and enable mirror_branch_regex' do + let_it_be(:project) { build(:project, only_mirror_protected_branches: false) } + + it 'is valid' do + setting = build(:project_setting, project: project, mirror_branch_regex: 'test') + + expect(setting).to be_valid + end + + it 'is invalid with invalid regex' do + setting = build(:project_setting, project: project, mirror_branch_regex: '\\') + + expect(setting).not_to be_valid + end + end + end end diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb index 72cecf1227b7b5f28c7398b8f29f4717ef0a9b58..c936cae74c76ed8f7fb0faa871cd357c7a3e7951 100644 --- a/ee/spec/models/project_spec.rb +++ b/ee/spec/models/project_spec.rb @@ -3670,4 +3670,24 @@ def stub_default_url_options(host) expect(result).to be_empty end end + + describe '#mirror_branches_setting' do + it 'mirror all branches' do + project = build(:project, only_mirror_protected_branches: false, mirror_branch_regex: nil) + + expect(project.mirror_branches_setting).to eq('all') + end + + it 'mirror protected branches' do + project = build(:project, only_mirror_protected_branches: true, mirror_branch_regex: nil) + + expect(project.mirror_branches_setting).to eq('protected') + end + + it 'mirror branches match regex' do + project = build(:project, only_mirror_protected_branches: false, mirror_branch_regex: 'text') + + expect(project.mirror_branches_setting).to eq('regex') + end + end end diff --git a/ee/spec/models/remote_mirror_spec.rb b/ee/spec/models/remote_mirror_spec.rb index c842b385f8504dd67896c67d7d5a7df44a1686f9..22945152a62aa3763e3e97c76eecf7301b0fb0c9 100644 --- a/ee/spec/models/remote_mirror_spec.rb +++ b/ee/spec/models/remote_mirror_spec.rb @@ -5,6 +5,30 @@ RSpec.describe RemoteMirror do let(:project) { create(:project, :repository, :remote_mirror) } + describe 'validations' do + context 'when enable only_protected_branches and mirror_branch_regex' do + it 'is invalid' do + remote_mirror = build(:remote_mirror, only_protected_branches: true, mirror_branch_regex: 'text') + + expect(remote_mirror).not_to be_valid + end + end + + context 'when disable only_protected_branches and enable mirror_branch_regex' do + it 'is valid' do + remote_mirror = build(:remote_mirror, only_protected_branches: false, mirror_branch_regex: 'test') + + expect(remote_mirror).to be_valid + end + + it 'is invalid with invalid regex' do + remote_mirror = build(:remote_mirror, only_protected_branches: false, mirror_branch_regex: '\\') + + expect(remote_mirror).not_to be_valid + end + end + end + describe '#sync' do let(:remote_mirror) { project.remote_mirrors.first } @@ -16,4 +40,50 @@ end end end + + describe '#only_mirror_protected_branches_column' do + it 'returns true as only_protected_branches enabled' do + remote_mirror = build_stubbed(:remote_mirror, only_protected_branches: true) + + expect(remote_mirror.only_mirror_protected_branches_column).to be_truthy + end + + it 'returns false as only_protected_branches return' do + remote_mirror = build_stubbed(:remote_mirror, only_protected_branches: false) + + expect(remote_mirror.only_mirror_protected_branches_column).to be_falsy + end + end + + describe '#options_for_update' do + context 'when mirror_branch_regex is set' do + let(:user) { build(:user) } + let(:mirror) do + build(:remote_mirror, + project: project, + only_protected_branches: false, + mirror_branch_regex: '^matched*') + end + + before do + project.repository.create_branch('matched', create_commit) + project.repository.create_branch('mismatched', create_commit) + end + + it 'only sync matched and recently updated branch' do + options = mirror.options_for_update + + expect(options).to include(only_branches_matching: %w[matched]) + end + end + end + + def create_commit + project.repository.commit_files( + user, + branch_name: 'HEAD', + message: 'commit message', + actions: [] + ) + end end diff --git a/ee/spec/services/projects/update_mirror_service_spec.rb b/ee/spec/services/projects/update_mirror_service_spec.rb index d767bb2a32be9f035163bf2de45b2da8740c5009..5a71a229681437ecd8096badd5ee0f6ad300bc41 100644 --- a/ee/spec/services/projects/update_mirror_service_spec.rb +++ b/ee/spec/services/projects/update_mirror_service_spec.rb @@ -327,6 +327,47 @@ end end + context 'when mirror_branch_regex is set' do + let(:new_branch_name) { "new-branch" } + let(:existing_branch_name) { "existing-branch" } + let(:fake_regex) { instance_spy(Gitlab::UntrustedRegexp) } + let!(:project_setting) { create(:project_setting, project: project, mirror_branch_regex: 'fake_regex') } + + before do + allow(Gitlab::UntrustedRegexp).to receive(:new) + .with('fake_regex') + .and_return(fake_regex) + + allow(fake_regex).to receive(:match?).and_return(false) + end + + it 'create a new matched branch' do + allow(fake_regex).to receive(:match?).with(new_branch_name).and_return(true) + service.execute + expect(project.repository.branch_names).to include(new_branch_name) + end + + it 'does not create mismatched branch' do + service.execute + expect(project.repository.branch_names).not_to include(new_branch_name) + end + + it 'updates existing matched branches' do + allow(fake_regex).to receive(:match?).with('existing-branch').and_return(true) + service.execute + + expect(project.repository.find_branch(existing_branch_name).dereferenced_target) + .to eq(project.repository.find_branch(master).dereferenced_target) + end + + it 'does not update mismatched branches' do + service.execute + + expect(project.repository.find_branch(existing_branch_name).dereferenced_target) + .not_to eq(project.repository.find_branch(master).dereferenced_target) + end + end + context 'with diverged branches' do let(:diverged_branch) { "markdown" } diff --git a/ee/spec/services/projects/update_service_spec.rb b/ee/spec/services/projects/update_service_spec.rb index b920f72bcc2d9a4f768aeb7a5ee057437f100849..61bf47ba63a03a11a613a75563155fedbbc265f1 100644 --- a/ee/spec/services/projects/update_service_spec.rb +++ b/ee/spec/services/projects/update_service_spec.rb @@ -120,6 +120,48 @@ update_project(project, user, opts) end + + context 'when update mirror branch setting' do + before do + project.team.add_maintainer(admin) + end + + it 'allow mirror_branch_regex to be updated' do + result = update_project(project, admin, opts.merge(mirror_branch_regex: :test)) + + expect(result).to eq(status: :success) + expect(project.mirror_branch_regex).to match('test') + end + + it 'enable only_mirror_protected_branches would clean mirror_branch_regex' do + project.mirror_branch_regex = 'test' + result = update_project(project, admin, opts.merge(only_mirror_protected_branches: true)) + + expect(result).to eq(status: :success) + expect(project.mirror_branch_regex).to be_nil + end + + it 'fill mirror_branch_regex would disable only_mirror_protected_branches' do + project.only_mirror_protected_branches = true + result = update_project(project, admin, opts.merge(mirror_branch_regex: 'text')) + + expect(result).to eq(status: :success) + expect(project.only_mirror_protected_branches).to be_falsey + end + + context 'when `mirror_only_branches_match_regex` FF is disabled' do + before do + stub_feature_flags(mirror_only_branches_match_regex: false) + end + + it 'ignores mirror_branch_regex parameter' do + result = update_project(project, admin, opts.merge(mirror_branch_regex: 'text')) + + expect(result).to eq(status: :success) + expect(project.mirror_branch_regex).to be_nil + end + end + end end context 'audit events' do diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 2ff4cd72f1ea4e963645b6b094eef44457cf1b80..da478a3547e605c63608b3c7feb4b967e154d8c6 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -43,6 +43,7 @@ itself: # project - storage_version - topic_list - updated_at + - mirror_branch_regex remapped_attributes: avatar: avatar_url build_allow_git_fetch: build_git_strategy